On Mon, Aug 24, 2020 at 02:39:32PM +0530, Viresh Kumar wrote: > From: Stephan Gerhold <stephan@xxxxxxxxxxx> > > The OPP core manages various resources, e.g. clocks or interconnect paths. > These resources are looked up when the OPP table is allocated once > dev_pm_opp_get_opp_table() is called the first time (either directly > or indirectly through one of the many helper functions). > > At this point, the resources may not be available yet, i.e. looking them > up will result in -EPROBE_DEFER. Unfortunately, dev_pm_opp_get_opp_table() > is currently unable to propagate this error code since it only returns > the allocated OPP table or NULL. > > This means that all consumers of the OPP core are required to make sure > that all necessary resources are available. Usually this happens by > requesting them, checking the result and releasing them immediately after. > > For example, we have added "dev_pm_opp_of_find_icc_paths(dev, NULL)" to > several drivers now just to make sure the interconnect providers are > ready before the OPP table is allocated. If this call is missing, > the OPP core will only warn about this and then attempt to continue > without interconnect. This will eventually fail horribly, e.g.: > > cpu cpu0: _allocate_opp_table: Error finding interconnect paths: -517 > ... later ... > of: _read_bw: Mismatch between opp-peak-kBps and paths (1 0) > cpu cpu0: _opp_add_static_v2: opp key field not found > cpu cpu0: _of_add_opp_table_v2: Failed to add OPP, -22 > > This example happens when trying to use interconnects for a CPU OPP > table together with qcom-cpufreq-nvmem.c. qcom-cpufreq-nvmem calls > dev_pm_opp_set_supported_hw(), which ends up allocating the OPP table > early. To fix the problem with the current approach we would need to add > yet another call to dev_pm_opp_of_find_icc_paths(dev, NULL). > But actually qcom-cpufreq-nvmem.c has nothing to do with interconnects... > > This commit attempts to make this more robust by allowing > dev_pm_opp_get_opp_table() to return an error pointer. Fixing all > the usages is trivial because the function is usually used indirectly > through another helper (e.g. dev_pm_opp_set_supported_hw() above). > These other helpers already return an error pointer. > > The example above then works correctly because set_supported_hw() will > return -EPROBE_DEFER, and qcom-cpufreq-nvmem.c already propagates that > error. It should also be possible to remove the remaining usages of > "dev_pm_opp_of_find_icc_paths(dev, NULL)" from other drivers as well. > > Note that this commit currently only handles -EPROBE_DEFER for the > clock/interconnects within _allocate_opp_table(). Other errors are just > ignored as before. Eventually those should be propagated as well. > > Signed-off-by: Stephan Gerhold <stephan@xxxxxxxxxxx> > [ Viresh: skip checking return value of dev_pm_opp_get_opp_table() for > EPROBE_DEFER in domain.c, fix NULL return value and reorder > code a bit in core.c, and update exynos-asv.c ] > Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx> > --- > Stephan, I have made some changes to the code. Please try it again and > lemme know if it works fine. > > drivers/base/power/domain.c | 14 +++++---- > drivers/opp/core.c | 53 +++++++++++++++++++------------- > drivers/opp/of.c | 8 ++--- > drivers/soc/samsung/exynos-asv.c | 2 +- > 4 files changed, 44 insertions(+), 33 deletions(-) For Samsung: Acked-by: Krzysztof Kozlowski <krzk@xxxxxxxxxx> Best regards, Krzysztof
