| Message ID | 20240606070127.3506240-1-primoz.fiser@norik.com |
|---|---|
| State | Accepted |
| Commit | 3a1ac6b8f603a9310274990a0ad563a5fb709f59 |
| Headers | show |
| Series | OPP: ti: Fix ti_opp_supply_probe wrong return values | expand |
On 06-06-24, 09:01, Primoz Fiser wrote: > Function ti_opp_supply_probe() since commit 6baee034cb55 ("OPP: ti: > Migrate to dev_pm_opp_set_config_regulators()") returns wrong values > when all goes well and hence driver probing eventually fails. > > Fixes: 6baee034cb55 ("OPP: ti: Migrate to dev_pm_opp_set_config_regulators()") > Signed-off-by: Primoz Fiser <primoz.fiser@norik.com> > --- > drivers/opp/ti-opp-supply.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/opp/ti-opp-supply.c b/drivers/opp/ti-opp-supply.c > index e3b97cd1fbbf..ec0056a4bb13 100644 > --- a/drivers/opp/ti-opp-supply.c > +++ b/drivers/opp/ti-opp-supply.c > @@ -393,10 +393,12 @@ static int ti_opp_supply_probe(struct platform_device *pdev) > } > > ret = dev_pm_opp_set_config_regulators(cpu_dev, ti_opp_config_regulators); > - if (ret < 0) > + if (ret < 0) { > _free_optimized_voltages(dev, &opp_data); > + return ret; > + } > > - return ret; > + return 0; > } > > static struct platform_driver ti_opp_supply_driver = { Not sure I understand the problem here. Can you please explain with an example ?
On 06-06-24, 12:34, Primoz Fiser wrote: > > Ah, I forgot about the token that is returned here. I think the driver > > should be fixed properly once and for all here. > > > > The token must be used to clean the module removal part. Else if you > > try to insert this module, remove it, insert again, you will get some > > errors as the resources aren't cleaned well. > > > > So either provide a remove() callback to the driver, or use > > devm_pm_opp_set_config() here. > > > > So basically, you want v2 with: > > > diff --git a/drivers/opp/ti-opp-supply.c b/drivers/opp/ti-opp-supply.c > > index e3b97cd1fbbf..8a4bcc5fb9dc 100644 > > --- a/drivers/opp/ti-opp-supply.c > > +++ b/drivers/opp/ti-opp-supply.c > > @@ -392,7 +392,7 @@ static int ti_opp_supply_probe(struct platform_device *pdev) > > return ret; > > } > > > > - ret = dev_pm_opp_set_config_regulators(cpu_dev, ti_opp_config_regulators); > > + ret = devm_pm_opp_set_config_regulators(cpu_dev, ti_opp_config_regulators); > > if (ret < 0) > > _free_optimized_voltages(dev, &opp_data); > > > > diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h > > index dd7c8441af42..a2401878d1d9 100644 > > --- a/include/linux/pm_opp.h > > +++ b/include/linux/pm_opp.h > > @@ -654,14 +654,14 @@ static inline int devm_pm_opp_set_clkname(struct device *dev, const char *name) > > } > > > > /* config-regulators helpers */ > > -static inline int dev_pm_opp_set_config_regulators(struct device *dev, Don't remove this, add a new devm_ counterpart. > > +static inline int devm_pm_opp_set_config_regulators(struct device *dev, > > config_regulators_t helper) > > { > > struct dev_pm_opp_config config = { > > .config_regulators = helper, > > }; > > > > - return dev_pm_opp_set_config(dev, &config); > > + return devm_pm_opp_set_config(dev, &config); > > }
diff --git a/drivers/opp/ti-opp-supply.c b/drivers/opp/ti-opp-supply.c index e3b97cd1fbbf..ec0056a4bb13 100644 --- a/drivers/opp/ti-opp-supply.c +++ b/drivers/opp/ti-opp-supply.c @@ -393,10 +393,12 @@ static int ti_opp_supply_probe(struct platform_device *pdev) } ret = dev_pm_opp_set_config_regulators(cpu_dev, ti_opp_config_regulators); - if (ret < 0) + if (ret < 0) { _free_optimized_voltages(dev, &opp_data); + return ret; + } - return ret; + return 0; } static struct platform_driver ti_opp_supply_driver = {
Function ti_opp_supply_probe() since commit 6baee034cb55 ("OPP: ti: Migrate to dev_pm_opp_set_config_regulators()") returns wrong values when all goes well and hence driver probing eventually fails. Fixes: 6baee034cb55 ("OPP: ti: Migrate to dev_pm_opp_set_config_regulators()") Signed-off-by: Primoz Fiser <primoz.fiser@norik.com> --- drivers/opp/ti-opp-supply.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)