| Message ID | 1d7c97524b9e1fbc60271d9c246c5461ca8a106c.1598594714.git.viresh.kumar@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | opp: Unconditionally call dev_pm_opp_of_remove_table() | expand |
Hi, On Fri, Aug 28, 2020 at 1:44 AM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Fri, 28 Aug 2020 at 08:08, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > dev_pm_opp_of_remove_table() doesn't report any errors when it fails to > > find the OPP table with error -ENODEV (i.e. OPP table not present for > > the device). And we can call dev_pm_opp_of_remove_table() > > unconditionally here. > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > Replaced v1 with v2 on my next branch, thanks! Actually, I don't see it on there yet, but at least the old broken v1 isn't there anymore. ;-) I picked v2 and tried it on my sc7180-based device (which does have OPP tables). It worked fine. Thus: Tested-by: Douglas Anderson <dianders@chromium.org> I looked at the code and it looks right to me. Thus: Reviewed-by: Douglas Anderson <dianders@chromium.org> > Just to be sure, this patch doesn't depend on any changes for the opp > core that are queued for v5.10? Running atop mmc-next, I see the check for -ENODEV, so I'm gonna assume that the required change is there. $ git grep -A10 'void _dev_pm_opp_find_and_remove_table' -- drivers/opp/core.c drivers/opp/core.c:void _dev_pm_opp_find_and_remove_table(struct device *dev) drivers/opp/core.c-{ drivers/opp/core.c- struct opp_table *opp_table; drivers/opp/core.c- drivers/opp/core.c- /* Check for existing table for 'dev' */ drivers/opp/core.c- opp_table = _find_opp_table(dev); drivers/opp/core.c- if (IS_ERR(opp_table)) { drivers/opp/core.c- int error = PTR_ERR(opp_table); drivers/opp/core.c- drivers/opp/core.c- if (error != -ENODEV) drivers/opp/core.c- WARN(1, "%s: opp_table: %d\n",
On 28-08-20, 10:43, Ulf Hansson wrote: > On Fri, 28 Aug 2020 at 08:08, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > dev_pm_opp_of_remove_table() doesn't report any errors when it fails to > > find the OPP table with error -ENODEV (i.e. OPP table not present for > > the device). And we can call dev_pm_opp_of_remove_table() > > unconditionally here. > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > Replaced v1 with v2 on my next branch, thanks! > > Just to be sure, this patch doesn't depend on any changes for the opp > core that are queued for v5.10? No it doesn't.
On 28-08-20, 10:43, Ulf Hansson wrote: > On Fri, 28 Aug 2020 at 08:08, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > dev_pm_opp_of_remove_table() doesn't report any errors when it fails to > > find the OPP table with error -ENODEV (i.e. OPP table not present for > > the device). And we can call dev_pm_opp_of_remove_table() > > unconditionally here. > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > Replaced v1 with v2 on my next branch, thanks! > > Just to be sure, this patch doesn't depend on any changes for the opp > core that are queued for v5.10? The recent crashes reported by Anders and Naresh were related to a OPP core bug, for which I have just sent the fix here: https://lore.kernel.org/lkml/922ff0759a16299e24cacfc981ac07914d8f1826.1598865786.git.viresh.kumar@linaro.org/ This is already tested by Naresh now and finally everything works as expected. I am going to get this fix merged in 5.9-rc4, but we do have a dependency now with that fix. What's the best way to handle this stuff now ? The easiest IMO would be for me to send these patches through the OPP tree, otherwise people need to carry this and the OPP fix (for which I can provide the branch/tag).
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c index 5a33389037cd..f7beaec6412e 100644 --- a/drivers/mmc/host/sdhci-msm.c +++ b/drivers/mmc/host/sdhci-msm.c @@ -263,7 +263,6 @@ struct sdhci_msm_host { unsigned long clk_rate; struct mmc_host *mmc; struct opp_table *opp_table; - bool has_opp_table; bool use_14lpp_dll_reset; bool tuning_done; bool calibration_done; @@ -2285,11 +2284,9 @@ static int sdhci_msm_probe(struct platform_device *pdev) /* OPP table is optional */ ret = dev_pm_opp_of_add_table(&pdev->dev); - if (!ret) { - msm_host->has_opp_table = true; - } else if (ret != -ENODEV) { + if (ret && ret != -ENODEV) { dev_err(&pdev->dev, "Invalid OPP table in Device tree\n"); - goto opp_cleanup; + goto opp_put_clkname; } /* Vote for maximum clock rate for maximum performance */ @@ -2453,8 +2450,8 @@ static int sdhci_msm_probe(struct platform_device *pdev) clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks), msm_host->bulk_clks); opp_cleanup: - if (msm_host->has_opp_table) - dev_pm_opp_of_remove_table(&pdev->dev); + dev_pm_opp_of_remove_table(&pdev->dev); +opp_put_clkname: dev_pm_opp_put_clkname(msm_host->opp_table); bus_clk_disable: if (!IS_ERR(msm_host->bus_clk)) @@ -2474,8 +2471,7 @@ static int sdhci_msm_remove(struct platform_device *pdev) sdhci_remove_host(host, dead); - if (msm_host->has_opp_table) - dev_pm_opp_of_remove_table(&pdev->dev); + dev_pm_opp_of_remove_table(&pdev->dev); dev_pm_opp_put_clkname(msm_host->opp_table); pm_runtime_get_sync(&pdev->dev); pm_runtime_disable(&pdev->dev);
dev_pm_opp_of_remove_table() doesn't report any errors when it fails to find the OPP table with error -ENODEV (i.e. OPP table not present for the device). And we can call dev_pm_opp_of_remove_table() unconditionally here. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- V2: - Compare with -ENODEV only for failures. - Create new label to put clkname. --- drivers/mmc/host/sdhci-msm.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)