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 <[email protected]>
---
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);
+ }
err_main_clocks:
if (error)
--
2.44.0
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 [email protected]?
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
> 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);
> + }
>
--
Best regards,
Dhruva
* Dhruva Gole <[email protected]> [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 [email protected]?
Let's just leave out the fixes tag as there are no known bugs caused
by this.
Regards,
Tony