Hi Claudiu, On Mon, Aug 19, 2024 at 01:23:42PM GMT, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> > > Add suspend/resume support for the RIIC driver. This is necessary for the > Renesas RZ/G3S SoC which support suspend to deep sleep state where power > to most of the SoC components is turned off. As a result the I2C controller > needs to be reconfigured after suspend/resume. For this, the reset line > was stored in the driver private data structure as well as i2c timings. > The reset line and I2C timings are necessary to re-initialize the > controller after resume. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> This patch doesn't have tags, so I'll add mine :-) Reviewed-by: Andi Shyti <andi.shyti@xxxxxxxxxx> Just one thing, though... ... > +static int riic_i2c_resume(struct device *dev) > +{ > + struct riic_dev *riic = dev_get_drvdata(dev); > + int ret; > + > + ret = reset_control_deassert(riic->rstc); > + if (ret) > + return ret; > + > + ret = riic_init_hw(riic); > + if (ret) { > + reset_control_assert(riic->rstc); > + return ret; Can I add a comment here saying: /* * Since the driver remains loaded after resume, * we want the reset line to be asserted. */ reset_control_assert(riic->rstc); Unless I missed the point :-) Thanks, Andi