Hi Wolfram,
On Wed, Jun 27, 2018 at 10:44 AM Wolfram Sang <wsa@xxxxxxxxxxxxx> wrote:
> > > The hardware team said:
> > > - In CPG point of view:
> > > - such polling doesn't need (because the reset pulse is generated correctly).
> > > - About the interval after deassert the reset, this is depend on each hardware module.
> > > (In other words, if each hardware module has a special handling about after the deassert interval,
> > > we should follow the special handling.)
> > > - The I2C controller needs an interval of reading SRCR at least (this is a special handling).
> > >
> > > So, I think adding this code is not good fit in CPG point of view.
> >
> > Calling reset_control_status() from i2c-rcar is fine for me.
> >
>
> Doesn't make this writing device drivers a bit harder, I wonder? If we
> follow the above, we need to know per driver (like i2c-rcar.c) if
> reset_control_reset() is enough or if we need to call
> reset_control_status() additionally. For a driver author, it would be
> much less error prone IMHO, if reset_control_reset() just does the right
> thing. It has also the advantage that we can handle special handling on
> different SoCs differently (if that is ever needed) because MSSR driver
> is per SoC.
True.
But this seems to be a bug in the R-Car Gen3 I2C controller implementation.
> > Note that reset controller support is optional, so we want to add
> >
> > select RESET_CONTROLLER if ARCH_RENESAS && ARM64
> >
> > to the I2C_RCAR section drivers/i2c/busses/Kconfig. Else reset will fail
> > silently.
>
> Technically, it should also be "&& HAS_DMA". But this is maybe a tad too
> defensive?
s/HAS_DMA/RCAR_DMAC/ :-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
