Re: [PATCH RFT 1/2] i2c: rcar: reset controller is mandatory for Gen3+

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]

Hi Geert,

> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks!

> > +               priv->flags &= ~ID_P_NO_RXDMA;
> > +               ret = rcar_i2c_do_reset(priv);
> > +               if (ret)
> > +                       priv->flags |= ID_P_NO_RXDMA;
> 
> This is pre-existing, but if rcar_i2c_do_reset() returns an error,
> that means the I2C block couldn't get out of reset.  Are we sure we
> can still do PIO transfers in that case, or should this be considered
> a fatal error?

Makes sense. I will double check what to do here.

> > +               ret = reset_control_status(priv->rstc);
> > +               if (ret < 0)
> > +                       return ret;
> 
> This is a pre-existing check, but do you really need it?
> This condition will be true if the reset is still asserted, which
> could happen due to some glitch, or force-booting into a new kernel
> using kexec.  And AFAIUI, that should be resolved by the call to
> rcar_i2c_do_reset() above.

This check is needed to ensure reset_control_status() really works
because we need it in rcar_i2c_do_reset(). From the docs:

"reset_control_status - returns a negative errno if not supported,..."

The code only checks for that, not for the status of the reset line.

All the best,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]