Hi, Andi, On 19.08.2024 22:37, Andi Shyti wrote: > 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. > */ Sure, thank you! > reset_control_assert(riic->rstc); > > Unless I missed the point :-) > > Thanks, > Andi