On 02/12/2023 18:18, Marek Vasut wrote: > On 11/19/23 21:48, Paul Barker wrote: >> On Sun, Nov 19, 2023 at 09:15:36PM +0100, Marek Vasut wrote: >>> On 11/15/23 18:40, Paul Barker wrote: >>>> This driver supports the I2C module on the Renesas RZ/G2L (R9A07G044) >>>> SoC, also known as the RIIC module. >>>> >>>> This patch is based on both the u-boot driver in the Renesas RZ BSP >>>> 3.0.5 release [1] (commit 7fcc1fdc2534), and the Linux v6.6 driver >>>> (commit ffc253263a13). >>>> >>>> Support for deblocking the I2C bus is included as this may be needed >>>> after triggering a reset via the Power Management IC (PMIC) over I2C >>>> (the PMIC asserts the reset line before the SoC completes the I2C write >>>> transaction with obvious bus locking effects). If the SDA line is >>>> observed to be low during initialisation, we automatically attempt to >>>> deblock. >>>> >>>> [1]: https://github.com/renesas-rz/renesas-u-boot-cip >>>> >>>> Signed-off-by: Paul Barker <paul.barker...@bp.renesas.com> >>> >>> The driver seems very similar to drivers/i2c/rcar_iic.c , can there be some >>> code reuse ? >> >> My initial idea was to extend the R-Car iic driver but I quickly saw >> that would be very messy. All the registers and bits are in different >> places, the calculations are different (particularly in regard to >> setting bus speed) and the read/write process has more edge cases for >> the RZ/G2L. The bus recovery mechanism we use also doesn't exist on >> previous R-Car SoCs. >> >> The Linux drivers for these modules are also separate, see >> drivers/i2c/busses/i2c-sh_mobile.c (for R-Car and earlier SoCs) and >> drivers/i2c/busses/i2c-riic.c (for RZ/G2L) in the Linux kernel. > > Ah, I thought some of the registers were the same, but indeed the RZG > core is much more extensive. > > You should either use devm_clk_*() in probe() or implement .remove > callback. With that fixed: > > Reviewed-by: Marek Vasut <marek.vasut+rene...@mailbox.org> > > And sorry for the delayed reply.
Here's an even more delayed reply from me! I've been busy on other work but getting back to u-boot now. Using devm_clk_get() in the probe function for this driver doesn't seem to be needed. The `struct clk` is stored in a `struct riic_priv`, this is allocated and freed automatically for each device as we set `priv_auto` in the driver definition. Thanks, -- Paul Barker
OpenPGP_0x27F4B3459F002257.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
