Discussion:
[PATCH 0/9] make clk_get_rate implementations behavior more consistent
Jonas Gorski
2017-07-18 10:17:21 UTC
Permalink
The common clock and several other clock API implementations allow
calling clk_get_rate with a NULL pointer. While not specified as
expected behavior of the API, device drivers have come to rely on that,
causing them to OOPS when run on a platform with a different clock API
implementation.

Fix this by making sure all clk_get_rate implementations handle
NULL clocks instead of OOPSing.

While some custom implementations even allow ERR_PTR()s, I decided
against that as IIRC the usual idea is that errors should be handled and
not silently carried over.

Cc: adi-buildroot-***@lists.sourceforge.net
Cc: bcm-kernel-feedback-***@broadcom.com
Cc: linux-arm-***@lists.infradead.org
Cc: linux-***@vger.kernel.org
Cc: linux-***@lists.linux-m68k.org
Cc: linux-***@linux-mips.org

Jonas Gorski (9):
ARM: ep93xx: allow NULL clock for clk_get_rate
ARM: mmp: allow NULL clock for clk_get_rate
blackfin: bf609: allow NULL clock for clk_get_rate
m68k: allow NULL clock for clk_get_rate
MIPS: AR7: allow NULL clock for clk_get_rate
MIPS: BCM63XX: allow NULL clock for clk_get_rate
MIPS: Loongson 2F: allow NULL clock for clk_get_rate
MIPS: ralink: allow NULL clock for clk_get_rate
unicore32: allow NULL clock for clk_get_rate

arch/arm/mach-ep93xx/clock.c | 3 +++
arch/arm/mach-mmp/clock.c | 4 +++-
arch/blackfin/mach-bf609/clock.c | 2 +-
arch/m68k/coldfire/clk.c | 3 +++
arch/mips/ar7/clock.c | 3 +++
arch/mips/bcm63xx/clk.c | 3 +++
arch/mips/loongson64/lemote-2f/clock.c | 3 +++
arch/mips/ralink/clk.c | 3 +++
arch/unicore32/kernel/clock.c | 3 +++
9 files changed, 25 insertions(+), 2 deletions(-)
--
2.11.0
Jonas Gorski
2017-07-18 10:17:23 UTC
Permalink
Make the behaviour of clk_get_rate consistent with common clk's
clk_get_rate by accepting NULL clocks as parameter. Some device
drivers rely on this, and will cause an OOPS otherwise.

Fixes: 49cbe78637eb ("[ARM] pxa: add base support for Marvell's PXA168 processor line")
Cc: Eric Miao <***@gmail.com>
Cc: Haojian Zhuang <***@gmail.com>
Cc: Russell King <***@armlinux.org.uk>
Cc: linux-arm-***@lists.infradead.org
Cc: linux-***@vger.kernel.org
Reported-by: Mathias Kresin <***@kresin.me>
Signed-off-by: Jonas Gorski <***@gmail.com>
---
arch/arm/mach-mmp/clock.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-mmp/clock.c b/arch/arm/mach-mmp/clock.c
index 28fe64c6e2f5..bdfb113431ec 100644
--- a/arch/arm/mach-mmp/clock.c
+++ b/arch/arm/mach-mmp/clock.c
@@ -83,7 +83,9 @@ unsigned long clk_get_rate(struct clk *clk)
{
unsigned long rate;

- if (clk->ops->getrate)
+ if (!clk)
+ rate = 0;
+ else if (clk->ops->getrate)
rate = clk->ops->getrate(clk);
else
rate = clk->rate;
--
2.11.0
Jonas Gorski
2017-07-18 10:17:22 UTC
Permalink
Make the behaviour of clk_get_rate consistent with common clk's
clk_get_rate by accepting NULL clocks as parameter. Some device
drivers rely on this, and will cause an OOPS otherwise.

Fixes: 1d81eedb8f6c ("[ARM] 3634/1: ep93xx: initial implementation of the clk_* API")
Cc: Hartley Sweeten <***@visionengravers.com>
Cc: Alexander Sverdlin <***@gmail.com>
Cc: Russell King <***@armlinux.org.uk>
Cc: linux-arm-***@lists.infradead.org
Cc: linux-***@vger.kernel.org
Reported-by: Mathias Kresin <***@kresin.me>
Signed-off-by: Jonas Gorski <***@gmail.com>
---
arch/arm/mach-ep93xx/clock.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-ep93xx/clock.c b/arch/arm/mach-ep93xx/clock.c
index 39ef3b613912..f0768befafe8 100644
--- a/arch/arm/mach-ep93xx/clock.c
+++ b/arch/arm/mach-ep93xx/clock.c
@@ -316,6 +316,9 @@ static unsigned long get_uart_rate(struct clk *clk)

unsigned long clk_get_rate(struct clk *clk)
{
+ if (!clk)
+ return 0;
+
if (clk->get_rate)
return clk->get_rate(clk);
--
2.11.0
Alexander Sverdlin
2017-07-19 05:04:40 UTC
Permalink
Post by Jonas Gorski
Make the behaviour of clk_get_rate consistent with common clk's
clk_get_rate by accepting NULL clocks as parameter. Some device
drivers rely on this, and will cause an OOPS otherwise.
Fixes: 1d81eedb8f6c ("[ARM] 3634/1: ep93xx: initial implementation of the clk_* API")
---
arch/arm/mach-ep93xx/clock.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/mach-ep93xx/clock.c b/arch/arm/mach-ep93xx/clock.c
index 39ef3b613912..f0768befafe8 100644
--- a/arch/arm/mach-ep93xx/clock.c
+++ b/arch/arm/mach-ep93xx/clock.c
@@ -316,6 +316,9 @@ static unsigned long get_uart_rate(struct clk *clk)
unsigned long clk_get_rate(struct clk *clk)
{
+ if (!clk)
+ return 0;
+
if (clk->get_rate)
return clk->get_rate(clk);
Geert Uytterhoeven
2017-07-18 12:01:19 UTC
Permalink
Hi Jonas,
Post by Jonas Gorski
The common clock and several other clock API implementations allow
calling clk_get_rate with a NULL pointer. While not specified as
expected behavior of the API, device drivers have come to rely on that,
causing them to OOPS when run on a platform with a different clock API
implementation.
Fix this by making sure all clk_get_rate implementations handle
NULL clocks instead of OOPSing.
While some custom implementations even allow ERR_PTR()s, I decided
against that as IIRC the usual idea is that errors should be handled and
not silently carried over.
ARM: ep93xx: allow NULL clock for clk_get_rate
ARM: mmp: allow NULL clock for clk_get_rate
blackfin: bf609: allow NULL clock for clk_get_rate
m68k: allow NULL clock for clk_get_rate
MIPS: AR7: allow NULL clock for clk_get_rate
MIPS: BCM63XX: allow NULL clock for clk_get_rate
MIPS: Loongson 2F: allow NULL clock for clk_get_rate
MIPS: ralink: allow NULL clock for clk_get_rate
unicore32: allow NULL clock for clk_get_rate
arch/arm/mach-ep93xx/clock.c | 3 +++
arch/arm/mach-mmp/clock.c | 4 +++-
arch/blackfin/mach-bf609/clock.c | 2 +-
arch/m68k/coldfire/clk.c | 3 +++
arch/mips/ar7/clock.c | 3 +++
arch/mips/bcm63xx/clk.c | 3 +++
arch/mips/loongson64/lemote-2f/clock.c | 3 +++
arch/mips/ralink/clk.c | 3 +++
arch/unicore32/kernel/clock.c | 3 +++
9 files changed, 25 insertions(+), 2 deletions(-)
For the whole series:
Reviewed-by: Geert Uytterhoeven <***@linux-m68k.org>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ***@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Loading...