dev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should
be used instead. Migrate to the new API.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/devfreq/tegra30-devfreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 117cad7968ab..d2477d7d1f66 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
return PTR_ERR(opp);
}
- ret = dev_pm_opp_set_bw(dev, opp);
+ ret = dev_pm_opp_set_opp(dev, opp);
dev_pm_opp_put(opp);
return ret;
--
2.25.0.rc1.19.g042ed3e048af
21.01.2021 14:17, Viresh Kumar пишет:
> dev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should
> be used instead. Migrate to the new API.
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> drivers/devfreq/tegra30-devfreq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
> index 117cad7968ab..d2477d7d1f66 100644
> --- a/drivers/devfreq/tegra30-devfreq.c
> +++ b/drivers/devfreq/tegra30-devfreq.c
> @@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
> return PTR_ERR(opp);
> }
>
> - ret = dev_pm_opp_set_bw(dev, opp);
> + ret = dev_pm_opp_set_opp(dev, opp);
> dev_pm_opp_put(opp);
>
> return ret;
>
This patch introduces a very serious change that needs to be fixed.
Now dev_pm_opp_set_opp() changes both clock rate and bandwidth, this is
unacceptable for this driver because it shall not touch the clock rate.
I think dev_pm_opp_set_bw() can't be removed.
On 22-01-21, 00:36, Dmitry Osipenko wrote:
> 21.01.2021 14:17, Viresh Kumar пишет:
> > dev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should
> > be used instead. Migrate to the new API.
> >
> > Signed-off-by: Viresh Kumar <[email protected]>
> > ---
> > drivers/devfreq/tegra30-devfreq.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
> > index 117cad7968ab..d2477d7d1f66 100644
> > --- a/drivers/devfreq/tegra30-devfreq.c
> > +++ b/drivers/devfreq/tegra30-devfreq.c
> > @@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
> > return PTR_ERR(opp);
> > }
> >
> > - ret = dev_pm_opp_set_bw(dev, opp);
> > + ret = dev_pm_opp_set_opp(dev, opp);
> > dev_pm_opp_put(opp);
> >
> > return ret;
> >
>
> This patch introduces a very serious change that needs to be fixed.
>
> Now dev_pm_opp_set_opp() changes both clock rate and bandwidth, this is
> unacceptable for this driver because it shall not touch the clock rate.
>
> I think dev_pm_opp_set_bw() can't be removed.
I am wondering here on what would be a better solution, do what you
said or introduce another helper like dev_pm_opp_clear_clk(), which
will make sure the OPP core doesn't play with device's clk.
--
viresh
22.01.2021 09:26, Viresh Kumar пишет:
> On 22-01-21, 00:36, Dmitry Osipenko wrote:
>> 21.01.2021 14:17, Viresh Kumar пишет:
>>> dev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should
>>> be used instead. Migrate to the new API.
>>>
>>> Signed-off-by: Viresh Kumar <[email protected]>
>>> ---
>>> drivers/devfreq/tegra30-devfreq.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
>>> index 117cad7968ab..d2477d7d1f66 100644
>>> --- a/drivers/devfreq/tegra30-devfreq.c
>>> +++ b/drivers/devfreq/tegra30-devfreq.c
>>> @@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
>>> return PTR_ERR(opp);
>>> }
>>>
>>> - ret = dev_pm_opp_set_bw(dev, opp);
>>> + ret = dev_pm_opp_set_opp(dev, opp);
>>> dev_pm_opp_put(opp);
>>>
>>> return ret;
>>>
>>
>> This patch introduces a very serious change that needs to be fixed.
>>
>> Now dev_pm_opp_set_opp() changes both clock rate and bandwidth, this is
>> unacceptable for this driver because it shall not touch the clock rate.
>>
>> I think dev_pm_opp_set_bw() can't be removed.
>
> I am wondering here on what would be a better solution, do what you
> said or introduce another helper like dev_pm_opp_clear_clk(), which
> will make sure the OPP core doesn't play with device's clk.
>
Either way will work, but maybe keeping the dev_pm_opp_set_bw() is a bit
more straightforward variant for now since it will avoid the code's
changes and it's probably a bit more obvious variant for the OPP users.
On 22-01-21, 18:28, Dmitry Osipenko wrote:
> Either way will work, but maybe keeping the dev_pm_opp_set_bw() is a bit
> more straightforward variant for now since it will avoid the code's
> changes and it's probably a bit more obvious variant for the OPP users.
The problem is it creates unnecessary paths which we need to support. For
example, in case of bandwidth itself we may want to update regulator/pm
domain/required OPPs and this should all be done for everyone. I really do not
want to keep separate paths for such stuff, it makes it hard to maintain..
--
viresh
25.01.2021 06:14, Viresh Kumar пишет:
> On 22-01-21, 18:28, Dmitry Osipenko wrote:
>> Either way will work, but maybe keeping the dev_pm_opp_set_bw() is a bit
>> more straightforward variant for now since it will avoid the code's
>> changes and it's probably a bit more obvious variant for the OPP users.
>
> The problem is it creates unnecessary paths which we need to support. For
> example, in case of bandwidth itself we may want to update regulator/pm
> domain/required OPPs and this should all be done for everyone. I really do not
> want to keep separate paths for such stuff, it makes it hard to maintain..
>
Maybe we could add dev_pm_opp_of_add_table_without_clock(), at least
that should be a bit nicer from a drivers perspective than having to
care about dev_pm_opp_clear_clk(), IMO.
dev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should
be used instead. Migrate to the new API.
We don't want the OPP core to manage the clk for this driver, migrate to
dev_pm_opp_of_add_table_noclk() to make sure dev_pm_opp_set_opp()
doesn't have any side effects.
Signed-off-by: Viresh Kumar <[email protected]>
---
Dmitry,
This is based over the patches sent here:
https://lore.kernel.org/lkml/6c2160ff30a8f421563793020264cf9f533f293c.1611738228.git.viresh.kumar@linaro.org/
This should fix the problem you mentioned earlier. Will push this for
linux-next unless you have any issues with it.
drivers/devfreq/tegra30-devfreq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 117cad7968ab..31f7dec5990b 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
return PTR_ERR(opp);
}
- ret = dev_pm_opp_set_bw(dev, opp);
+ ret = dev_pm_opp_set_opp(dev, opp);
dev_pm_opp_put(opp);
return ret;
@@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
return err;
}
- err = dev_pm_opp_of_add_table(&pdev->dev);
+ err = dev_pm_opp_of_add_table_noclk(&pdev->dev);
if (err) {
dev_err(&pdev->dev, "Failed to add OPP table: %d\n", err);
goto put_hw;
--
2.25.0.rc1.19.g042ed3e048af
On 27-01-21, 14:40, Viresh Kumar wrote:
> dev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should
> be used instead. Migrate to the new API.
>
> We don't want the OPP core to manage the clk for this driver, migrate to
> dev_pm_opp_of_add_table_noclk() to make sure dev_pm_opp_set_opp()
> doesn't have any side effects.
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> Dmitry,
>
> This is based over the patches sent here:
>
> https://lore.kernel.org/lkml/6c2160ff30a8f421563793020264cf9f533f293c.1611738228.git.viresh.kumar@linaro.org/
>
> This should fix the problem you mentioned earlier. Will push this for
> linux-next unless you have any issues with it.
>
> drivers/devfreq/tegra30-devfreq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
> index 117cad7968ab..31f7dec5990b 100644
> --- a/drivers/devfreq/tegra30-devfreq.c
> +++ b/drivers/devfreq/tegra30-devfreq.c
> @@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
> return PTR_ERR(opp);
> }
>
> - ret = dev_pm_opp_set_bw(dev, opp);
> + ret = dev_pm_opp_set_opp(dev, opp);
> dev_pm_opp_put(opp);
>
> return ret;
> @@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
> return err;
> }
>
> - err = dev_pm_opp_of_add_table(&pdev->dev);
> + err = dev_pm_opp_of_add_table_noclk(&pdev->dev);
Plus this, somehow was left uncommited in my tree :(
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 31f7dec5990b..ce83f883ca65 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
return err;
}
- err = dev_pm_opp_of_add_table_noclk(&pdev->dev);
+ err = dev_pm_opp_of_add_table_noclk(&pdev->dev, 0);
if (err) {
dev_err(&pdev->dev, "Failed to add OPP table: %d\n", err);
goto put_hw;
--
viresh
27.01.2021 13:02, Viresh Kumar пишет:
> On 27-01-21, 14:40, Viresh Kumar wrote:
>> dev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should
>> be used instead. Migrate to the new API.
>>
>> We don't want the OPP core to manage the clk for this driver, migrate to
>> dev_pm_opp_of_add_table_noclk() to make sure dev_pm_opp_set_opp()
>> doesn't have any side effects.
>>
>> Signed-off-by: Viresh Kumar <[email protected]>
>> ---
>> Dmitry,
>>
>> This is based over the patches sent here:
>>
>> https://lore.kernel.org/lkml/6c2160ff30a8f421563793020264cf9f533f293c.1611738228.git.viresh.kumar@linaro.org/
>>
>> This should fix the problem you mentioned earlier. Will push this for
>> linux-next unless you have any issues with it.
>>
>> drivers/devfreq/tegra30-devfreq.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
>> index 117cad7968ab..31f7dec5990b 100644
>> --- a/drivers/devfreq/tegra30-devfreq.c
>> +++ b/drivers/devfreq/tegra30-devfreq.c
>> @@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
>> return PTR_ERR(opp);
>> }
>>
>> - ret = dev_pm_opp_set_bw(dev, opp);
>> + ret = dev_pm_opp_set_opp(dev, opp);
>> dev_pm_opp_put(opp);
>>
>> return ret;
>> @@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>> return err;
>> }
>>
>> - err = dev_pm_opp_of_add_table(&pdev->dev);
>> + err = dev_pm_opp_of_add_table_noclk(&pdev->dev);
>
> Plus this, somehow was left uncommited in my tree :(
>
> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
> index 31f7dec5990b..ce83f883ca65 100644
> --- a/drivers/devfreq/tegra30-devfreq.c
> +++ b/drivers/devfreq/tegra30-devfreq.c
> @@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
> return err;
> }
>
> - err = dev_pm_opp_of_add_table_noclk(&pdev->dev);
> + err = dev_pm_opp_of_add_table_noclk(&pdev->dev, 0);
> if (err) {
> dev_err(&pdev->dev, "Failed to add OPP table: %d\n", err);
> goto put_hw;
>
Sadly this doesn't work because we missed that clk is assigned to
opp_table when OPP table is allocated and not when it's added to device.
Hence we're now set back to the dev_pm_opp_clear_clk() variant.
What about to add a new OPP API which will allow OPP users to configure
behaviour that user wants from OPP core in a generic way, something like
this:
struct opp_config {
bool no_clk;
...
};
devm_pm_opp_set_config(dev, struct opp_config);
dev_pm_opp_set_config(dev, struct opp_config);
dev_pm_opp_unset_config(dev);
Or maybe even rename it dev_pm_opp_allocate_table(dev, struct
opp_config), which will allow users to directly allocate OPP table
instead of relying on the implicit allocations. Then there won't be a
need for drivers to use a dummy devm_pm_opp_set_clkname(dev, NULL) just
to allocate the table usable for dev_pm_opp_set_rate().
On 27-01-21, 18:58, Dmitry Osipenko wrote:
> Sadly this doesn't work because we missed that clk is assigned to
> opp_table when OPP table is allocated and not when it's added to device.
Ahh, I missed that.
I have bumped up the other patchset to V2, that should work fine with
this patch for tegra30 (this shouldn't require any update).
Everything is pushed in opp/linux-next, fetch and try it. Thanks.
--
viresh
On 1/27/21 6:10 PM, Viresh Kumar wrote:
> dev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should
> be used instead. Migrate to the new API.
>
> We don't want the OPP core to manage the clk for this driver, migrate to
> dev_pm_opp_of_add_table_noclk() to make sure dev_pm_opp_set_opp()
> doesn't have any side effects.
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> Dmitry,
>
> This is based over the patches sent here:
>
> https://protect2.fireeye.com/v1/url?k=72d88562-2d43bc78-72d90e2d-000babff24ad-e4764030101eaedc&q=1&e=a25b5db7-346b-47b2-9c0a-3945e579f389&u=https%3A%2F%2Flore.kernel.org%2Flkml%2F6c2160ff30a8f421563793020264cf9f533f293c.1611738228.git.viresh.kumar%40linaro.org%2F
>
> This should fix the problem you mentioned earlier. Will push this for
> linux-next unless you have any issues with it.
>
> drivers/devfreq/tegra30-devfreq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
> index 117cad7968ab..31f7dec5990b 100644
> --- a/drivers/devfreq/tegra30-devfreq.c
> +++ b/drivers/devfreq/tegra30-devfreq.c
> @@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
> return PTR_ERR(opp);
> }
>
> - ret = dev_pm_opp_set_bw(dev, opp);
> + ret = dev_pm_opp_set_opp(dev, opp);
> dev_pm_opp_put(opp);
>
> return ret;
> @@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
> return err;
> }
>
> - err = dev_pm_opp_of_add_table(&pdev->dev);
> + err = dev_pm_opp_of_add_table_noclk(&pdev->dev);
> if (err) {
> dev_err(&pdev->dev, "Failed to add OPP table: %d\n", err);
> goto put_hw;
>
Acked-by: Chanwoo Choi <[email protected]>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
27.01.2021 12:10, Viresh Kumar пишет:
> dev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should
> be used instead. Migrate to the new API.
>
> We don't want the OPP core to manage the clk for this driver, migrate to
> dev_pm_opp_of_add_table_noclk() to make sure dev_pm_opp_set_opp()
> doesn't have any side effects.
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> Dmitry,
>
> This is based over the patches sent here:
>
> https://lore.kernel.org/lkml/6c2160ff30a8f421563793020264cf9f533f293c.1611738228.git.viresh.kumar@linaro.org/
>
> This should fix the problem you mentioned earlier. Will push this for
> linux-next unless you have any issues with it.
>
> drivers/devfreq/tegra30-devfreq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
> index 117cad7968ab..31f7dec5990b 100644
> --- a/drivers/devfreq/tegra30-devfreq.c
> +++ b/drivers/devfreq/tegra30-devfreq.c
> @@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
> return PTR_ERR(opp);
> }
>
> - ret = dev_pm_opp_set_bw(dev, opp);
> + ret = dev_pm_opp_set_opp(dev, opp);
> dev_pm_opp_put(opp);
>
> return ret;
> @@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
> return err;
> }
>
> - err = dev_pm_opp_of_add_table(&pdev->dev);
> + err = dev_pm_opp_of_add_table_noclk(&pdev->dev);
> if (err) {
> dev_err(&pdev->dev, "Failed to add OPP table: %d\n", err);
> goto put_hw;
>
Tested-by: Dmitry Osipenko <[email protected]>