On 7/17/26 3:01 PM, Konrad Dybcio wrote: > The first two labels only unwind a single regulator
Thanks for taking a look. Could you clarify whether this is a correctness concern (e.g. the fallthrough from err_enable/err_set_load into err_set_voltage not doing what you'd expect), or more a structural/style preference (e.g. avoiding three chained labels in favor of a different approach)? Happy to send a v2 once I understand what you'd like changed. Thanks, Sailesh On Fri, Jul 17, 2026 at 3:01 PM Konrad Dybcio <[email protected]> wrote: > > On 7/10/26 9:46 PM, Sailesh Nandanavanam wrote: > > In q6v5_regulator_enable(), when any operation fails for regulator at > > index 'i', the error cleanup path unconditionally calls > > regulator_disable() starting from index 'i'. However, regulator 'i' > > was never successfully enabled at this point, resulting in an > > unbalanced disable. > > > > There are three distinct failure points: > > - regulator_set_voltage() failure: voltage was never set, load was > > never set, regulator was never enabled. > > - regulator_set_load() failure: voltage was set, but regulator was > > never enabled. > > - regulator_enable() failure: voltage and load were set, but > > regulator was never enabled. > > > > Fix this by introducing three separate error labels to handle each > > failure point correctly. For the failing regulator at index 'i', > > only reset the resources that were actually configured, without > > calling regulator_disable(). Then roll back all previously enabled > > regulators using 'i--' in the for loop initializer to skip the > > never-enabled regulator. > > > > Fixes: 19f902b53b47 ("remoteproc: qcom: Initialize and enable proxy and > > active regulators.") > > Cc: [email protected] > > Signed-off-by: Sailesh Nandanavanam <[email protected]> > > --- > > [...] > > > -err: > > - for (; i >= 0; i--) { > > +err_enable: > > + if (regs[i].uA > 0) > > + regulator_set_load(regs[i].reg, 0); > > +err_set_load: > > + if (regs[i].uV > 0) > > + regulator_set_voltage(regs[i].reg, 0, INT_MAX); > > The first two labels only unwind a single regulator > > Konrad
