clk: tegra: ensure all provided clock values are valid cookies
Subsystems (3)
PATCH 1/1 clk: tegra: ensure all provided clock values are valid cookies
From: Stephen Warren <[email protected]>
Tegra's clock implementation uses pointers as the clock cookies, and hence chooses to make NULL an invalid clock cookie. However, there are gaps in the assigned device tree clock IDs, and hence the array mapping DT clock ID contains entries with NULL values (uninitialized BSS) unless explicit action is taken. This patch enhances the Tegra clock code to detect this case and explicitly initialize those lookup table entries to an error value. This prevents clk_get() from ever returning NULL. Hence, Tegra's clock APIs don't have to check the clock cookie they're passed for NULL.
---
Mike, this also will need to go through the Tegra tree; just looking for any review/ack from you. Thanks.
drivers/clk/tegra/clk-tegra20.c | 5 ++++- drivers/clk/tegra/clk-tegra30.c | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-)
drivers/clk/tegra/clk-tegra20.c +4 -1
@@ -1243,12 +1243,15 @@ void __init tegra20_clock_init(struct device_node *np)tegra20_audio_clk_init (); -for (i =0 ; i <ARRAY_SIZE (clks); i++) +for (i =0 ; i <ARRAY_SIZE (clks); i++) {if (IS_ERR (clks[i])) {pr_err ("Tegra20 clk %d: register failed with %ld\n" , i,PTR_ERR (clks[i]));BUG (); } +if (!clks[i]) + clks[i] =ERR_PTR (-EINVAL ); + }tegra_init_dup_clks (tegra_clk_duplicates, clks, clk_max);
drivers/clk/tegra/clk-tegra30.c +4 -1
@@ -2022,12 +2022,15 @@ void __init tegra30_clock_init(struct device_node *np)tegra30_audio_clk_init ();tegra30_pmc_clk_init (); -for (i =0 ; i <ARRAY_SIZE (clks); i++) +for (i =0 ; i <ARRAY_SIZE (clks); i++) {if (IS_ERR (clks[i])) {pr_err ("Tegra30 clk %d: register failed with %ld\n" , i,PTR_ERR (clks[i]));BUG (); } +if (!clks[i]) + clks[i] =ERR_PTR (-EINVAL ); + }tegra_init_dup_clks (tegra_clk_duplicates, clks, clk_max);
Signature · 1 line
1.7.10.4
PATCH 1/1 clk: tegra: ensure all provided clock values are valid cookies
Quoting Stephen Warren (2013-01-14 10:16:40)
Quoted 12 lines
From: Stephen Warren <[email protected]>
Tegra's clock implementation uses pointers as the clock cookies, and hence chooses to make NULL an invalid clock cookie. However, there are gaps in the assigned device tree clock IDs, and hence the array mapping DT clock ID contains entries with NULL values (uninitialized BSS) unless explicit action is taken. This patch enhances the Tegra clock code to detect this case and explicitly initialize those lookup table entries to an error value. This prevents
clk_get()from ever returning NULL. Hence, Tegra's clock APIs don't have to check the clock cookie they're passed for NULL.
Quoted 5 lines
Signed-off-by Stephen Warren <[email protected]>---
Mike, this also will need to go through the Tegra tree; just looking for any review/ack from you. Thanks.
Stephen,
I think this is a bit strange. Will you have gaps in the DT clock id's forever or is this a temporary issue?
Also it is important than any driver using the clock api does not dereference the struct clk pointer and assume that a NULL clk is invalid. I know that this series doesn't imply any such thing but I wanted to state that anyways.
Also are you planning to roll this into the existing tegra ccf series?
Regards,
Mike
Quoted 48 lines
drivers/clk/tegra/clk-tegra20.c | 5 ++++- drivers/clk/tegra/clk-tegra30.c | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-)drivers/clk/tegra/clk-tegra20.c +4
-1@@ -1243,12 +1243,15 @@ void __init tegra20_clock_init(struct device_node *np)tegra20_audio_clk_init (); -for (i =0 ; i <ARRAY_SIZE (clks); i++) +for (i =0 ; i <ARRAY_SIZE (clks); i++) {if (IS_ERR (clks[i])) {pr_err ("Tegra20 clk %d: register failed with %ld\n" , i,PTR_ERR (clks[i]));BUG (); } +if (!clks[i]) + clks[i] =ERR_PTR (-EINVAL ); + }tegra_init_dup_clks (tegra_clk_duplicates, clks, clk_max);drivers/clk/tegra/clk-tegra30.c +4
-1@@ -2022,12 +2022,15 @@ void __init tegra30_clock_init(struct device_node *np)tegra30_audio_clk_init ();tegra30_pmc_clk_init (); -for (i =0 ; i <ARRAY_SIZE (clks); i++) +for (i =0 ; i <ARRAY_SIZE (clks); i++) {if (IS_ERR (clks[i])) {pr_err ("Tegra30 clk %d: register failed with %ld\n" , i,PTR_ERR (clks[i]));BUG (); } +if (!clks[i]) + clks[i] =ERR_PTR (-EINVAL ); + }tegra_init_dup_clks (tegra_clk_duplicates, clks, clk_max);Signature · 1 line
1.7.10.4
PATCH 1/1 clk: tegra: ensure all provided clock values are valid cookies
Quoted 18 lines · Mike Turquette
Quoting Stephen Warren (2013-01-14 10:16:40)
Quoted 12 lines
From: Stephen Warren <[email protected]>
Tegra's clock implementation uses pointers as the clock cookies, and hence chooses to make NULL an invalid clock cookie. However, there are gaps in the assigned device tree clock IDs, and hence the array mapping DT clock ID contains entries with NULL values (uninitialized BSS) unless explicit action is taken. This patch enhances the Tegra clock code to detect this case and explicitly initialize those lookup table entries to an error value. This prevents
clk_get()from ever returning NULL. Hence, Tegra's clock APIs don't have to check the clock cookie they're passed for NULL.Quoted 4 lines
Signed-off-by Stephen Warren <[email protected]>---
Mike, this also will need to go through the Tegra tree; just looking for any review/ack from you. Thanks.
Quoted 4 lines
Stephen,
I think this is a bit strange. Will you have gaps in the DT clock id's forever or is this a temporary issue?
Forever as far as I know. Since the clock IDs are just arbitrary tokens, I don't see any particular need for them to be strictly packed.
Quoted 4 lines
Also it is important than any driver using the clock api does not dereference the struct clk pointer and assume that a NULL clk is invalid. I know that this series doesn't imply any such thing but I wanted to state that anyways.
Right. This ensures that clk_get() returns an IS_ERR() pointer, or a valid cookie that the clock driver is prepared to handle (which doesn't include NULL).
Quoted 1 line
Also are you planning to roll this into the existing tegra ccf series?
It's already rolled into the latest version of the series which I posted.