* [PATCH] memory: tegra: Remove calls to dev_pm_opp_set_clkname()
@ 2021-01-22 6:00 Viresh Kumar
2021-01-22 15:00 ` Dmitry Osipenko
0 siblings, 1 reply; 7+ messages in thread
From: Viresh Kumar @ 2021-01-22 6:00 UTC (permalink / raw)
To: Dmitry Osipenko, Krzysztof Kozlowski, Thierry Reding, Jonathan Hunter
Cc: Viresh Kumar, Vincent Guittot, linux-kernel, linux-tegra
There is no point calling dev_pm_opp_set_clkname() with the "name"
parameter set to NULL, this is already done by the OPP core at setup
time and should work as it is.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Dmitry, am I missing something obvious here ?
drivers/memory/tegra/tegra20-emc.c | 13 ++-----------
drivers/memory/tegra/tegra30-emc.c | 13 ++-----------
2 files changed, 4 insertions(+), 22 deletions(-)
diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c
index 686aaf477d8a..d653a6be8d7f 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -911,21 +911,14 @@ static int tegra_emc_interconnect_init(struct tegra_emc *emc)
static int tegra_emc_opp_table_init(struct tegra_emc *emc)
{
u32 hw_version = BIT(tegra_sku_info.soc_process_id);
- struct opp_table *clk_opp_table, *hw_opp_table;
+ struct opp_table *hw_opp_table;
int err;
- clk_opp_table = dev_pm_opp_set_clkname(emc->dev, NULL);
- err = PTR_ERR_OR_ZERO(clk_opp_table);
- if (err) {
- dev_err(emc->dev, "failed to set OPP clk: %d\n", err);
- return err;
- }
-
hw_opp_table = dev_pm_opp_set_supported_hw(emc->dev, &hw_version, 1);
err = PTR_ERR_OR_ZERO(hw_opp_table);
if (err) {
dev_err(emc->dev, "failed to set OPP supported HW: %d\n", err);
- goto put_clk_table;
+ return err;
}
err = dev_pm_opp_of_add_table(emc->dev);
@@ -954,8 +947,6 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
dev_pm_opp_of_remove_table(emc->dev);
put_hw_table:
dev_pm_opp_put_supported_hw(hw_opp_table);
-put_clk_table:
- dev_pm_opp_put_clkname(clk_opp_table);
return err;
}
diff --git a/drivers/memory/tegra/tegra30-emc.c b/drivers/memory/tegra/tegra30-emc.c
index 44ac155936aa..6985da0ffb35 100644
--- a/drivers/memory/tegra/tegra30-emc.c
+++ b/drivers/memory/tegra/tegra30-emc.c
@@ -1483,21 +1483,14 @@ static int tegra_emc_interconnect_init(struct tegra_emc *emc)
static int tegra_emc_opp_table_init(struct tegra_emc *emc)
{
u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
- struct opp_table *clk_opp_table, *hw_opp_table;
+ struct opp_table *hw_opp_table;
int err;
- clk_opp_table = dev_pm_opp_set_clkname(emc->dev, NULL);
- err = PTR_ERR_OR_ZERO(clk_opp_table);
- if (err) {
- dev_err(emc->dev, "failed to set OPP clk: %d\n", err);
- return err;
- }
-
hw_opp_table = dev_pm_opp_set_supported_hw(emc->dev, &hw_version, 1);
err = PTR_ERR_OR_ZERO(hw_opp_table);
if (err) {
dev_err(emc->dev, "failed to set OPP supported HW: %d\n", err);
- goto put_clk_table;
+ return err;
}
err = dev_pm_opp_of_add_table(emc->dev);
@@ -1526,8 +1519,6 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
dev_pm_opp_of_remove_table(emc->dev);
put_hw_table:
dev_pm_opp_put_supported_hw(hw_opp_table);
-put_clk_table:
- dev_pm_opp_put_clkname(clk_opp_table);
return err;
}
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] memory: tegra: Remove calls to dev_pm_opp_set_clkname()
2021-01-22 6:00 [PATCH] memory: tegra: Remove calls to dev_pm_opp_set_clkname() Viresh Kumar
@ 2021-01-22 15:00 ` Dmitry Osipenko
2021-01-25 3:10 ` Viresh Kumar
0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Osipenko @ 2021-01-22 15:00 UTC (permalink / raw)
To: Viresh Kumar, Krzysztof Kozlowski, Thierry Reding, Jonathan Hunter
Cc: Vincent Guittot, linux-kernel, linux-tegra
22.01.2021 09:00, Viresh Kumar пишет:
> There is no point calling dev_pm_opp_set_clkname() with the "name"
> parameter set to NULL, this is already done by the OPP core at setup
> time and should work as it is.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Dmitry, am I missing something obvious here ?
The dev_pm_opp_set_clkname() ensures that the OPP table has a clk
assigned to the table.
The _allocate_opp_table() ignores all clk_get() errors except the
-EPROBE_DEFER.
The opp_table->clk must be not a PTR_ERR for these memory drivers.
On the other hand, the memory drivers also do the devm_clk_get()
themselves, hence technically clk_get() of _allocate_opp_table()
shouldn't fail. But I'll need to take a closer look at the clk core, to
check whether clk_get() could fail for the implicit _allocate_opp_table().
Tegra124-emc driver also has dev_pm_opp_set_clkname(), BTW.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] memory: tegra: Remove calls to dev_pm_opp_set_clkname()
2021-01-22 15:00 ` Dmitry Osipenko
@ 2021-01-25 3:10 ` Viresh Kumar
0 siblings, 0 replies; 7+ messages in thread
From: Viresh Kumar @ 2021-01-25 3:10 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Krzysztof Kozlowski, Thierry Reding, Jonathan Hunter,
Vincent Guittot, linux-kernel, linux-tegra
On 22-01-21, 18:00, Dmitry Osipenko wrote:
> 22.01.2021 09:00, Viresh Kumar пишет:
> > There is no point calling dev_pm_opp_set_clkname() with the "name"
> > parameter set to NULL, this is already done by the OPP core at setup
> > time and should work as it is.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> > Dmitry, am I missing something obvious here ?
>
> The dev_pm_opp_set_clkname() ensures that the OPP table has a clk
> assigned to the table.
>
> The _allocate_opp_table() ignores all clk_get() errors except the
> -EPROBE_DEFER.
That is the case for everyone relying on OPP core to get it. It should be fine
for this case as well.
> The opp_table->clk must be not a PTR_ERR for these memory drivers.
>
> On the other hand, the memory drivers also do the devm_clk_get()
> themselves, hence technically clk_get() of _allocate_opp_table()
> shouldn't fail. But I'll need to take a closer look at the clk core, to
> check whether clk_get() could fail for the implicit _allocate_opp_table().
>
> Tegra124-emc driver also has dev_pm_opp_set_clkname(), BTW.
--
viresh
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] memory: tegra: Remove calls to dev_pm_opp_set_clkname()
@ 2021-01-27 10:16 Viresh Kumar
2021-01-27 19:27 ` Dmitry Osipenko
2021-01-29 6:59 ` Krzysztof Kozlowski
0 siblings, 2 replies; 7+ messages in thread
From: Viresh Kumar @ 2021-01-27 10:16 UTC (permalink / raw)
To: Dmitry Osipenko, Krzysztof Kozlowski, Thierry Reding, Jonathan Hunter
Cc: Viresh Kumar, Vincent Guittot, linux-kernel, linux-tegra
There is no point calling dev_pm_opp_set_clkname() with the "name"
parameter set to NULL, this is already done by the OPP core at setup
time and should work as it is.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V2: Update tegra124 as well.
Krzysztof, please take this through your tree, it doesn't have any
dependency in the OPP tree.
---
drivers/memory/tegra/tegra124-emc.c | 13 ++-----------
drivers/memory/tegra/tegra20-emc.c | 13 ++-----------
drivers/memory/tegra/tegra30-emc.c | 13 ++-----------
3 files changed, 6 insertions(+), 33 deletions(-)
diff --git a/drivers/memory/tegra/tegra124-emc.c b/drivers/memory/tegra/tegra124-emc.c
index 9706550bdd5d..bee8d9f79b04 100644
--- a/drivers/memory/tegra/tegra124-emc.c
+++ b/drivers/memory/tegra/tegra124-emc.c
@@ -1399,21 +1399,14 @@ static int tegra_emc_interconnect_init(struct tegra_emc *emc)
static int tegra_emc_opp_table_init(struct tegra_emc *emc)
{
u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
- struct opp_table *clk_opp_table, *hw_opp_table;
+ struct opp_table *hw_opp_table;
int err;
- clk_opp_table = dev_pm_opp_set_clkname(emc->dev, NULL);
- err = PTR_ERR_OR_ZERO(clk_opp_table);
- if (err) {
- dev_err(emc->dev, "failed to set OPP clk: %d\n", err);
- return err;
- }
-
hw_opp_table = dev_pm_opp_set_supported_hw(emc->dev, &hw_version, 1);
err = PTR_ERR_OR_ZERO(hw_opp_table);
if (err) {
dev_err(emc->dev, "failed to set OPP supported HW: %d\n", err);
- goto put_clk_table;
+ return err;
}
err = dev_pm_opp_of_add_table(emc->dev);
@@ -1442,8 +1435,6 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
dev_pm_opp_of_remove_table(emc->dev);
put_hw_table:
dev_pm_opp_put_supported_hw(hw_opp_table);
-put_clk_table:
- dev_pm_opp_put_clkname(clk_opp_table);
return err;
}
diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c
index 686aaf477d8a..d653a6be8d7f 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -911,21 +911,14 @@ static int tegra_emc_interconnect_init(struct tegra_emc *emc)
static int tegra_emc_opp_table_init(struct tegra_emc *emc)
{
u32 hw_version = BIT(tegra_sku_info.soc_process_id);
- struct opp_table *clk_opp_table, *hw_opp_table;
+ struct opp_table *hw_opp_table;
int err;
- clk_opp_table = dev_pm_opp_set_clkname(emc->dev, NULL);
- err = PTR_ERR_OR_ZERO(clk_opp_table);
- if (err) {
- dev_err(emc->dev, "failed to set OPP clk: %d\n", err);
- return err;
- }
-
hw_opp_table = dev_pm_opp_set_supported_hw(emc->dev, &hw_version, 1);
err = PTR_ERR_OR_ZERO(hw_opp_table);
if (err) {
dev_err(emc->dev, "failed to set OPP supported HW: %d\n", err);
- goto put_clk_table;
+ return err;
}
err = dev_pm_opp_of_add_table(emc->dev);
@@ -954,8 +947,6 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
dev_pm_opp_of_remove_table(emc->dev);
put_hw_table:
dev_pm_opp_put_supported_hw(hw_opp_table);
-put_clk_table:
- dev_pm_opp_put_clkname(clk_opp_table);
return err;
}
diff --git a/drivers/memory/tegra/tegra30-emc.c b/drivers/memory/tegra/tegra30-emc.c
index 44ac155936aa..6985da0ffb35 100644
--- a/drivers/memory/tegra/tegra30-emc.c
+++ b/drivers/memory/tegra/tegra30-emc.c
@@ -1483,21 +1483,14 @@ static int tegra_emc_interconnect_init(struct tegra_emc *emc)
static int tegra_emc_opp_table_init(struct tegra_emc *emc)
{
u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
- struct opp_table *clk_opp_table, *hw_opp_table;
+ struct opp_table *hw_opp_table;
int err;
- clk_opp_table = dev_pm_opp_set_clkname(emc->dev, NULL);
- err = PTR_ERR_OR_ZERO(clk_opp_table);
- if (err) {
- dev_err(emc->dev, "failed to set OPP clk: %d\n", err);
- return err;
- }
-
hw_opp_table = dev_pm_opp_set_supported_hw(emc->dev, &hw_version, 1);
err = PTR_ERR_OR_ZERO(hw_opp_table);
if (err) {
dev_err(emc->dev, "failed to set OPP supported HW: %d\n", err);
- goto put_clk_table;
+ return err;
}
err = dev_pm_opp_of_add_table(emc->dev);
@@ -1526,8 +1519,6 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
dev_pm_opp_of_remove_table(emc->dev);
put_hw_table:
dev_pm_opp_put_supported_hw(hw_opp_table);
-put_clk_table:
- dev_pm_opp_put_clkname(clk_opp_table);
return err;
}
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] memory: tegra: Remove calls to dev_pm_opp_set_clkname()
2021-01-27 10:16 Viresh Kumar
@ 2021-01-27 19:27 ` Dmitry Osipenko
2021-01-28 4:18 ` Viresh Kumar
2021-01-29 6:59 ` Krzysztof Kozlowski
1 sibling, 1 reply; 7+ messages in thread
From: Dmitry Osipenko @ 2021-01-27 19:27 UTC (permalink / raw)
To: Viresh Kumar, Krzysztof Kozlowski, Thierry Reding, Jonathan Hunter
Cc: Vincent Guittot, linux-kernel, linux-tegra
27.01.2021 13:16, Viresh Kumar пишет:
> There is no point calling dev_pm_opp_set_clkname() with the "name"
> parameter set to NULL, this is already done by the OPP core at setup
> time and should work as it is.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> ---
> V2: Update tegra124 as well.
>
> Krzysztof, please take this through your tree, it doesn't have any
> dependency in the OPP tree.
> ---
> drivers/memory/tegra/tegra124-emc.c | 13 ++-----------
> drivers/memory/tegra/tegra20-emc.c | 13 ++-----------
> drivers/memory/tegra/tegra30-emc.c | 13 ++-----------
> 3 files changed, 6 insertions(+), 33 deletions(-)
Ideally drivers should be able to ensure that OPP table received the
clk, IMO. But this is also almost fine with me since realistically
clk_get() shouldn't fail if it already succeeded a moment ago.
Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
Could we please fix the _allocate_opp_table() to not ignore clk_get()
errors like -ENOMEM and etc?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] memory: tegra: Remove calls to dev_pm_opp_set_clkname()
2021-01-27 19:27 ` Dmitry Osipenko
@ 2021-01-28 4:18 ` Viresh Kumar
0 siblings, 0 replies; 7+ messages in thread
From: Viresh Kumar @ 2021-01-28 4:18 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Krzysztof Kozlowski, Thierry Reding, Jonathan Hunter,
Vincent Guittot, linux-kernel, linux-tegra
On 27-01-21, 22:27, Dmitry Osipenko wrote:
> 27.01.2021 13:16, Viresh Kumar пишет:
> > There is no point calling dev_pm_opp_set_clkname() with the "name"
> > parameter set to NULL, this is already done by the OPP core at setup
> > time and should work as it is.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> >
> > ---
> > V2: Update tegra124 as well.
> >
> > Krzysztof, please take this through your tree, it doesn't have any
> > dependency in the OPP tree.
> > ---
> > drivers/memory/tegra/tegra124-emc.c | 13 ++-----------
> > drivers/memory/tegra/tegra20-emc.c | 13 ++-----------
> > drivers/memory/tegra/tegra30-emc.c | 13 ++-----------
> > 3 files changed, 6 insertions(+), 33 deletions(-)
>
> Ideally drivers should be able to ensure that OPP table received the
> clk, IMO.
I don't really agree with that, this stuff is internal to the OPP
core. And it should all work unless there is a bug, whose effects we
will see anyway.
> But this is also almost fine with me since realistically
> clk_get() shouldn't fail if it already succeeded a moment ago.
>
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> Tested-by: Dmitry Osipenko <digetx@gmail.com>
Thanks.
Krzysztof, please apply the patch to your tree. Thanks.
> Could we please fix the _allocate_opp_table() to not ignore clk_get()
> errors like -ENOMEM and etc?
Maybe we should fail if the error is anything other than -ENODEV ?
--
viresh
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] memory: tegra: Remove calls to dev_pm_opp_set_clkname()
2021-01-27 10:16 Viresh Kumar
2021-01-27 19:27 ` Dmitry Osipenko
@ 2021-01-29 6:59 ` Krzysztof Kozlowski
1 sibling, 0 replies; 7+ messages in thread
From: Krzysztof Kozlowski @ 2021-01-29 6:59 UTC (permalink / raw)
To: Viresh Kumar
Cc: Dmitry Osipenko, Thierry Reding, Jonathan Hunter,
Vincent Guittot, linux-kernel, linux-tegra
On Wed, Jan 27, 2021 at 03:46:22PM +0530, Viresh Kumar wrote:
> There is no point calling dev_pm_opp_set_clkname() with the "name"
> parameter set to NULL, this is already done by the OPP core at setup
> time and should work as it is.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> ---
> V2: Update tegra124 as well.
>
> Krzysztof, please take this through your tree, it doesn't have any
> dependency in the OPP tree.
> ---
> drivers/memory/tegra/tegra124-emc.c | 13 ++-----------
> drivers/memory/tegra/tegra20-emc.c | 13 ++-----------
> drivers/memory/tegra/tegra30-emc.c | 13 ++-----------
> 3 files changed, 6 insertions(+), 33 deletions(-)
Thanks, applied.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-01-29 7:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-22 6:00 [PATCH] memory: tegra: Remove calls to dev_pm_opp_set_clkname() Viresh Kumar
2021-01-22 15:00 ` Dmitry Osipenko
2021-01-25 3:10 ` Viresh Kumar
2021-01-27 10:16 Viresh Kumar
2021-01-27 19:27 ` Dmitry Osipenko
2021-01-28 4:18 ` Viresh Kumar
2021-01-29 6:59 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).