| Message ID | 20240410064010.57142-2-tony@atomide.com |
|---|---|
| State | New |
| Headers | show |
| Series | Update ti-sysc description and drop legacy quirk handling | expand |
On Apr 10, 2024 at 09:40:05 +0300, Tony Lindgren wrote: > We are wrongly checking SYSC_QUIRK_NO_RESET_ON_INIT flag in sysc_reset(), > it can be called also after init from sysc_reinit_module(). Let's fix the > issue by moving the check to the init code. I am not able to understand exactly the potential bug here, what was the issue exactly? What I am able to infer is this is more of an improvement than fixing a bug? Maybe I am missing some context, can you help me understand the potential bug here? > > Fixes: 6a52bc2b81fa ("bus: ti-sysc: Add quirk handling for reset on re-init") Fixes tag, you might want to CC stable@vger.kernel.org? > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > drivers/bus/ti-sysc.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c > --- a/drivers/bus/ti-sysc.c > +++ b/drivers/bus/ti-sysc.c > @@ -2145,8 +2145,7 @@ static int sysc_reset(struct sysc *ddata) > sysc_offset = ddata->offsets[SYSC_SYSCONFIG]; > > if (ddata->legacy_mode || > - ddata->cap->regbits->srst_shift < 0 || > - ddata->cfg.quirks & SYSC_QUIRK_NO_RESET_ON_INIT) > + ddata->cap->regbits->srst_shift < 0) > return 0; > > sysc_mask = BIT(ddata->cap->regbits->srst_shift); > @@ -2240,12 +2239,14 @@ static int sysc_init_module(struct sysc *ddata) > goto err_main_clocks; > } > > - error = sysc_reset(ddata); > - if (error) > - dev_err(ddata->dev, "Reset failed with %d\n", error); > + if (!(ddata->cfg.quirks & SYSC_QUIRK_NO_RESET_ON_INIT)) { > + error = sysc_reset(ddata); > + if (error) > + dev_err(ddata->dev, "Reset failed with %d\n", error); > > - if (error && !ddata->legacy_mode) > - sysc_disable_module(ddata->dev); > + if (error && !ddata->legacy_mode) > + sysc_disable_module(ddata->dev); > + } >
* Dhruva Gole <d-gole@ti.com> [240410 18:03]: > On Apr 10, 2024 at 09:40:05 +0300, Tony Lindgren wrote: > > We are wrongly checking SYSC_QUIRK_NO_RESET_ON_INIT flag in sysc_reset(), > > it can be called also after init from sysc_reinit_module(). Let's fix the > > issue by moving the check to the init code. > > I am not able to understand exactly the potential bug here, what was the > issue exactly? With this flag, reset should be skipped on init, for example for an SDRAM controller during booting or to preserve a boot logo etc. However, if a reset is requested later on after init, we must ignore this flag. > What I am able to infer is this is more of an improvement than fixing a > bug? Maybe I am missing some context, can you help me understand the > potential bug here? We are now also calling sysc_reset() during runtime, so in theory some device would not reset during usage as requested. I don't think we have such cases in reality though. So yeah this is more cleanup rather than a fix AFAIK. > > Fixes: 6a52bc2b81fa ("bus: ti-sysc: Add quirk handling for reset on re-init") > > Fixes tag, you might want to CC stable@vger.kernel.org? Let's just leave out the fixes tag as there are no known bugs caused by this. Regards, Tony
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c --- a/drivers/bus/ti-sysc.c +++ b/drivers/bus/ti-sysc.c @@ -2145,8 +2145,7 @@ static int sysc_reset(struct sysc *ddata) sysc_offset = ddata->offsets[SYSC_SYSCONFIG]; if (ddata->legacy_mode || - ddata->cap->regbits->srst_shift < 0 || - ddata->cfg.quirks & SYSC_QUIRK_NO_RESET_ON_INIT) + ddata->cap->regbits->srst_shift < 0) return 0; sysc_mask = BIT(ddata->cap->regbits->srst_shift); @@ -2240,12 +2239,14 @@ static int sysc_init_module(struct sysc *ddata) goto err_main_clocks; } - error = sysc_reset(ddata); - if (error) - dev_err(ddata->dev, "Reset failed with %d\n", error); + if (!(ddata->cfg.quirks & SYSC_QUIRK_NO_RESET_ON_INIT)) { + error = sysc_reset(ddata); + if (error) + dev_err(ddata->dev, "Reset failed with %d\n", error); - if (error && !ddata->legacy_mode) - sysc_disable_module(ddata->dev); + if (error && !ddata->legacy_mode) + sysc_disable_module(ddata->dev); + } err_main_clocks: if (error)
We are wrongly checking SYSC_QUIRK_NO_RESET_ON_INIT flag in sysc_reset(), it can be called also after init from sysc_reinit_module(). Let's fix the issue by moving the check to the init code. Fixes: 6a52bc2b81fa ("bus: ti-sysc: Add quirk handling for reset on re-init") Signed-off-by: Tony Lindgren <tony@atomide.com> --- drivers/bus/ti-sysc.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)