RE: cpufreq regression on imx6q sabresd

RE: cpufreq regression on imx6q sabresd

[Aisheng Dong]

+ linux-arm-kernel

> From: Aisheng Dong
> Sent: Tuesday, October 13, 2020 5:00 PM
> Hi
> 
> Cpufreq on MX6Q SDB becomes unwork since next-20200902 and this issue
> also exist with latest
> linux-next next-20201012.
> 
> Error log as follows:
> [    6.782395] cpu cpu0: failed to disable 852MHz OPP
> [    6.787246] cpu cpu0: failed to disable 1.2GHz OPP
> [    6.792189] cpu cpu0: failed to init cpufreq table: -61
> [    6.800234] imx6q-cpufreq: probe of imx6q-cpufreq failed with error -61
> 
> Simply back to next-20200828 version for drivers/opp can avoid this issue.
> e.g. git checkout next-20200828 drivers/opp/
> So the issue seems to be related to OPP changes.
> 
> $ git log --oneline --no-merges next-20200828..next-20201012 drivers/opp/
> a5663c9b1e31 opp: Allow opp-level to be set to 0
> cb60e9602cce opp: Prevent memory leak in dev_pm_opp_attach_genpd()
> 0ff25c99042a opp: Allow opp-supported-hw to contain multiple versions
> 2c59138c22f1 opp: Set required OPPs in reverse order when scaling down
> 60cdeae0d627 opp: Reduce code duplication in _set_required_opps()
> 475ac8ead803 opp: Drop unnecessary check from
> dev_pm_opp_attach_genpd()
> 90d46d71cce2 opp: Handle multiple calls for same OPP table in
> _of_add_opp_table_v1()
> 922ff0759a16 opp: Don't drop reference for an OPP table that was never
> parsed
> 
> NOTE:
> Due to another know upstream kernel boot issue caused by pcie, we have to
> add below workaround in order to
> Boot Mx6q sabresd.
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c
> b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 317ff512f8df..afee1a0e8883 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -441,6 +441,9 @@ static void __iomem
> *dw_pcie_other_conf_map_bus(struct pci_bus *bus,
>         struct pcie_port *pp = bus->sysdata;
>         struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> 
> +       if (!dw_pcie_link_up(pci))
> +               return NULL;
> +
>         busdev = PCIE_ATU_BUS(bus->number) |
> PCIE_ATU_DEV(PCI_SLOT(devfn)) |
>                  PCIE_ATU_FUNC(PCI_FUNC(devfn));
> 
> Regards
> Aisheng
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: cpufreq regression on imx6q sabresd

[Aisheng Dong]

> From: Viresh Kumar <viresh.kumar@linaro.org>
> Sent: Tuesday, October 13, 2020 5:28 PM
> 
> On 13-10-20, 09:00, Aisheng Dong wrote:
> > Hi
> >
> > Cpufreq on MX6Q SDB becomes unwork since next-20200902 and this issue
> > also exist with latest linux-next next-20201012.
> >
> > Error log as follows:
> > [    6.782395] cpu cpu0: failed to disable 852MHz OPP
> > [    6.787246] cpu cpu0: failed to disable 1.2GHz OPP
> > [    6.792189] cpu cpu0: failed to init cpufreq table: -61
> > [    6.800234] imx6q-cpufreq: probe of imx6q-cpufreq failed with error -61
> >
> > Simply back to next-20200828 version for drivers/opp can avoid this issue.
> > e.g. git checkout next-20200828 drivers/opp/ So the issue seems to be
> > related to OPP changes.
> >
> > $ git log --oneline --no-merges next-20200828..next-20201012
> > drivers/opp/
> 
> diff with 20200902 would have been more interesting as you said it didn't work
> there either ? I had a look at it and am not sure why would it fail this way.
> 
> I tried to look into this but I am afraid you need to dig in a bit further to see
> where it is going bad.
> 

I think it's caused by the patch below:
90d46d71cce2 opp: Handle multiple calls for same OPP table in _of_add_opp_table_v1()

My kernel is next-20201012.
$ git log --no-merges --oneline -n10 drivers/opp/
a5663c9b1e31 opp: Allow opp-level to be set to 0
cb60e9602cce opp: Prevent memory leak in dev_pm_opp_attach_genpd()
0ff25c99042a opp: Allow opp-supported-hw to contain multiple versions
2c59138c22f1 opp: Set required OPPs in reverse order when scaling down
60cdeae0d627 opp: Reduce code duplication in _set_required_opps()
475ac8ead803 opp: Drop unnecessary check from dev_pm_opp_attach_genpd()
90d46d71cce2 opp: Handle multiple calls for same OPP table in _of_add_opp_table_v1()
922ff0759a16 opp: Don't drop reference for an OPP table that was never parsed
dd461cd9183f opp: Allow dev_pm_opp_get_opp_table() to return -EPROBE_DEFER
8aaf6264fc7f opp: Remove _dev_pm_opp_find_and_remove_table() wrapper

If back to the version earlier than that commit, the issue was gone.
e.g.
git checkout 922ff0759a16 drivers/opp/
NOTE: no dt changes. Only opp changes.

As I'm busy with some other urgent things, so still did not spend too much time to dig
Into the code to see why the issue happened.
I may find some time to look into it later if no one else look at it.

Regards
Aisheng

> Hint, from the error message and the return error (ENOENT), it looks like there
> are no entires in the OPP table that are valid and so a count of 0 is returned,
> which made OPP core return ENOENT. You need to look into why the count is
> zero, while you were able to add the OPP table.
> 
> --
> viresh
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: cpufreq regression on imx6q sabresd

[Viresh Kumar]

On 13-10-20, 11:58, Aisheng Dong wrote:
> I think it's caused by the patch below:
> 90d46d71cce2 opp: Handle multiple calls for same OPP table in _of_add_opp_table_v1()
> 
> My kernel is next-20201012.
> $ git log --no-merges --oneline -n10 drivers/opp/
> a5663c9b1e31 opp: Allow opp-level to be set to 0
> cb60e9602cce opp: Prevent memory leak in dev_pm_opp_attach_genpd()
> 0ff25c99042a opp: Allow opp-supported-hw to contain multiple versions
> 2c59138c22f1 opp: Set required OPPs in reverse order when scaling down
> 60cdeae0d627 opp: Reduce code duplication in _set_required_opps()
> 475ac8ead803 opp: Drop unnecessary check from dev_pm_opp_attach_genpd()
> 90d46d71cce2 opp: Handle multiple calls for same OPP table in _of_add_opp_table_v1()
> 922ff0759a16 opp: Don't drop reference for an OPP table that was never parsed
> dd461cd9183f opp: Allow dev_pm_opp_get_opp_table() to return -EPROBE_DEFER
> 8aaf6264fc7f opp: Remove _dev_pm_opp_find_and_remove_table() wrapper
> 
> If back to the version earlier than that commit, the issue was gone.
> e.g.
> git checkout 922ff0759a16 drivers/opp/
> NOTE: no dt changes. Only opp changes.
> 
> As I'm busy with some other urgent things, so still did not spend too much time to dig
> Into the code to see why the issue happened.
> I may find some time to look into it later if no one else look at it.

This should fix it.

Subject: [PATCH] opp: Don't always remove static OPPs in
 _of_add_opp_table_v1()

The patch missed returning 0 early in case of success and hence the
static OPPs got removed by mistake. Fix it.

Fixes: 90d46d71cce2 ("opp: Handle multiple calls for same OPP table in _of_add_opp_table_v1()")
Reported-by: Aisheng Dong <aisheng.dong@nxp.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/opp/of.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 874b58756220..9faeb83e4b32 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -944,6 +944,8 @@ static int _of_add_opp_table_v1(struct device *dev, struct opp_table *opp_table)
 		nr -= 2;
 	}
 
+	return 0;
+
 remove_static_opp:
 	_opp_remove_all_static(opp_table);
 
-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel