Hi all,
When seeing commit bde4975310eb1982 ("net: stmmac: fix build failure due
to missing COMMON_CLK dependency"), I wondered why this dependency is
needed, as all implementations of the clock API should implement all
required functionality, or provide dummies.
It turns out there were still two implementations that lacked the
clk_set_rate() function: Coldfire and AR7.
This series contains three patches:
- The first two patches add dummies for clk_set_rate(),
clk_set_rate(), clk_set_parent(), and clk_get_parent() to the
Coldfire and AR7, like Arnd has done for other legacy clock
implementations a while ago.
- The second patch removes the COMMON_CLK dependency from the stmmac
network drivers again, as it is no longer needed.
Obviously this patch has a hard dependency on the first two patches.
Thanks!
Geert Uytterhoeven (3):
m68k: coldfire: Normalize clk API
MIPS: AR7: Normalize clk API
[RFC] Revert "net: stmmac: fix build failure due to missing COMMON_CLK
dependency"
arch/m68k/coldfire/clk.c | 29 +++++++++++++++++++++++++++++
arch/mips/ar7/clock.c | 29 +++++++++++++++++++++++++++++
drivers/net/ethernet/stmicro/stmmac/Kconfig | 10 +++++-----
3 files changed, 63 insertions(+), 5 deletions(-)
--
2.7.4
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
Coldfire still provides its own variant of the clk API rather than using
the generic COMMON_CLK API. This generally works, but it causes some
link errors with drivers using the clk_round_rate(), clk_set_rate(),
clk_set_parent(), or clk_get_parent() functions when a platform lacks
those interfaces.
This adds empty stub implementations for each of them, and I don't even
try to do something useful here but instead just print a WARN() message
to make it obvious what is going on if they ever end up being called.
The drivers that call these won't be used on these platforms (otherwise
we'd get a link error today), so the added code is harmless bloat and
will warn about accidental use.
Based on commit bd7fefe1f06ca6cc ("ARM: w90x900: normalize clk API").
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
arch/mips/ar7/clock.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/arch/mips/ar7/clock.c b/arch/mips/ar7/clock.c
index 0137656107a9c5b5..6b64fd96dba8fb26 100644
--- a/arch/mips/ar7/clock.c
+++ b/arch/mips/ar7/clock.c
@@ -476,3 +476,32 @@ void __init ar7_init_clocks(void)
/* adjust vbus clock rate */
vbus_clk.rate = bus_clk.rate / 2;
}
+
+/* dummy functions, should not be called */
+long clk_round_rate(struct clk *clk, unsigned long rate)
+{
+ WARN_ON(clk);
+ return 0;
+}
+EXPORT_SYMBOL(clk_round_rate);
+
+int clk_set_rate(struct clk *clk, unsigned long rate)
+{
+ WARN_ON(clk);
+ return 0;
+}
+EXPORT_SYMBOL(clk_set_rate);
+
+int clk_set_parent(struct clk *clk, struct clk *parent)
+{
+ WARN_ON(clk);
+ return 0;
+}
+EXPORT_SYMBOL(clk_set_parent);
+
+struct clk *clk_get_parent(struct clk *clk)
+{
+ WARN_ON(clk);
+ return NULL;
+}
+EXPORT_SYMBOL(clk_get_parent);
--
2.7.4
Coldfire still provides its own variant of the clk API rather than using
the generic COMMON_CLK API. This generally works, but it causes some
link errors with drivers using the clk_round_rate(), clk_set_rate(),
clk_set_parent(), or clk_get_parent() functions when a platform lacks
those interfaces.
This adds empty stub implementations for each of them, and I don't even
try to do something useful here but instead just print a WARN() message
to make it obvious what is going on if they ever end up being called.
The drivers that call these won't be used on these platforms (otherwise
we'd get a link error today), so the added code is harmless bloat and
will warn about accidental use.
Based on commit bd7fefe1f06ca6cc ("ARM: w90x900: normalize clk API").
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
arch/m68k/coldfire/clk.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/arch/m68k/coldfire/clk.c b/arch/m68k/coldfire/clk.c
index 849cd208e2ed99e6..7bc666e482ebe82f 100644
--- a/arch/m68k/coldfire/clk.c
+++ b/arch/m68k/coldfire/clk.c
@@ -129,4 +129,33 @@ unsigned long clk_get_rate(struct clk *clk)
}
EXPORT_SYMBOL(clk_get_rate);
+/* dummy functions, should not be called */
+long clk_round_rate(struct clk *clk, unsigned long rate)
+{
+ WARN_ON(clk);
+ return 0;
+}
+EXPORT_SYMBOL(clk_round_rate);
+
+int clk_set_rate(struct clk *clk, unsigned long rate)
+{
+ WARN_ON(clk);
+ return 0;
+}
+EXPORT_SYMBOL(clk_set_rate);
+
+int clk_set_parent(struct clk *clk, struct clk *parent)
+{
+ WARN_ON(clk);
+ return 0;
+}
+EXPORT_SYMBOL(clk_set_parent);
+
+struct clk *clk_get_parent(struct clk *clk)
+{
+ WARN_ON(clk);
+ return NULL;
+}
+EXPORT_SYMBOL(clk_get_parent);
+
/***************************************************************************/
--
2.7.4
This reverts commit bde4975310eb1982bd0bbff673989052d92fd481.
All legacy clock implementations now implement clk_set_rate() (Some
implementations may be dummies, though).
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Marked "RFC", as this depends on "m68k: coldfire: Normalize clk API" and
"MIPS: AR7: Normalize clk API".
---
drivers/net/ethernet/stmicro/stmmac/Kconfig | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index cb5b0f58c395c2bd..e28c0d2c58e911ed 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -33,7 +33,7 @@ config DWMAC_DWC_QOS_ETH
select PHYLIB
select CRC32
select MII
- depends on OF && COMMON_CLK && HAS_DMA
+ depends on OF && HAS_DMA
help
Support for chips using the snps,dwc-qos-ethernet.txt DT binding.
@@ -57,7 +57,7 @@ config DWMAC_ANARION
config DWMAC_IPQ806X
tristate "QCA IPQ806x DWMAC support"
default ARCH_QCOM
- depends on OF && COMMON_CLK && (ARCH_QCOM || COMPILE_TEST)
+ depends on OF && (ARCH_QCOM || COMPILE_TEST)
select MFD_SYSCON
help
Support for QCA IPQ806X DWMAC Ethernet.
@@ -100,7 +100,7 @@ config DWMAC_OXNAS
config DWMAC_ROCKCHIP
tristate "Rockchip dwmac support"
default ARCH_ROCKCHIP
- depends on OF && COMMON_CLK && (ARCH_ROCKCHIP || COMPILE_TEST)
+ depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
select MFD_SYSCON
help
Support for Ethernet controller on Rockchip RK3288 SoC.
@@ -123,7 +123,7 @@ config DWMAC_SOCFPGA
config DWMAC_STI
tristate "STi GMAC support"
default ARCH_STI
- depends on OF && COMMON_CLK && (ARCH_STI || COMPILE_TEST)
+ depends on OF && (ARCH_STI || COMPILE_TEST)
select MFD_SYSCON
---help---
Support for ethernet controller on STi SOCs.
@@ -147,7 +147,7 @@ config DWMAC_STM32
config DWMAC_SUNXI
tristate "Allwinner GMAC support"
default ARCH_SUNXI
- depends on OF && COMMON_CLK && (ARCH_SUNXI || COMPILE_TEST)
+ depends on OF && (ARCH_SUNXI || COMPILE_TEST)
---help---
Support for Allwinner A20/A31 GMAC ethernet controllers.
--
2.7.4
On Mon, Jun 11, 2018 at 10:44 AM, Geert Uytterhoeven
<[email protected]> wrote:
> This reverts commit bde4975310eb1982bd0bbff673989052d92fd481.
>
> All legacy clock implementations now implement clk_set_rate() (Some
> implementations may be dummies, though).
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> Marked "RFC", as this depends on "m68k: coldfire: Normalize clk API" and
> "MIPS: AR7: Normalize clk API".
This seems reasonable. It's possible that it will cause regressions because the
COMMON_CLK dependency hides another dependency on something else
that not everything implements, but we should fix that properly if that happens.
Arnd
On Mon, Jun 11, 2018 at 10:44 AM, Geert Uytterhoeven
<[email protected]> wrote:
> Hi all,
>
> When seeing commit bde4975310eb1982 ("net: stmmac: fix build failure due
> to missing COMMON_CLK dependency"), I wondered why this dependency is
> needed, as all implementations of the clock API should implement all
> required functionality, or provide dummies.
>
> It turns out there were still two implementations that lacked the
> clk_set_rate() function: Coldfire and AR7.
>
> This series contains three patches:
> - The first two patches add dummies for clk_set_rate(),
> clk_set_rate(), clk_set_parent(), and clk_get_parent() to the
> Coldfire and AR7, like Arnd has done for other legacy clock
> implementations a while ago.
> - The second patch removes the COMMON_CLK dependency from the stmmac
> network drivers again, as it is no longer needed.
> Obviously this patch has a hard dependency on the first two patches.
Yes, good idea.
Acked-by: Arnd Bergmann <[email protected]>
One question: what happens on machines that don't support any CLK
interface, i.e.
that don't have any of COMMON_CLK/HAVE_CLK/CLKDEV_LOOKUP?
I guess those are already hopelessly broken for many drivers, right?
Arnd
Hi Arnd,
On Mon, Jun 11, 2018 at 11:02 AM Arnd Bergmann <[email protected]> wrote:
> On Mon, Jun 11, 2018 at 10:44 AM, Geert Uytterhoeven
> <[email protected]> wrote:
> > When seeing commit bde4975310eb1982 ("net: stmmac: fix build failure due
> > to missing COMMON_CLK dependency"), I wondered why this dependency is
> > needed, as all implementations of the clock API should implement all
> > required functionality, or provide dummies.
> >
> > It turns out there were still two implementations that lacked the
> > clk_set_rate() function: Coldfire and AR7.
> >
> > This series contains three patches:
> > - The first two patches add dummies for clk_set_rate(),
> > clk_set_rate(), clk_set_parent(), and clk_get_parent() to the
> > Coldfire and AR7, like Arnd has done for other legacy clock
> > implementations a while ago.
> > - The second patch removes the COMMON_CLK dependency from the stmmac
> > network drivers again, as it is no longer needed.
> > Obviously this patch has a hard dependency on the first two patches.
>
> Yes, good idea.
>
> Acked-by: Arnd Bergmann <[email protected]>
Thanks!
> One question: what happens on machines that don't support any CLK
> interface, i.e.
> that don't have any of COMMON_CLK/HAVE_CLK/CLKDEV_LOOKUP?
>
> I guess those are already hopelessly broken for many drivers, right?
Nope, they (e.g. m68k allmodconfig) build fine, as they don't define
CONFIG_HAVE_CLK.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
Hi Arnd,
On Mon, Jun 11, 2018 at 10:59 AM Arnd Bergmann <[email protected]> wrote:
> On Mon, Jun 11, 2018 at 10:44 AM, Geert Uytterhoeven
> <[email protected]> wrote:
> > This reverts commit bde4975310eb1982bd0bbff673989052d92fd481.
> >
> > All legacy clock implementations now implement clk_set_rate() (Some
> > implementations may be dummies, though).
> >
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > ---
> > Marked "RFC", as this depends on "m68k: coldfire: Normalize clk API" and
> > "MIPS: AR7: Normalize clk API".
>
> This seems reasonable. It's possible that it will cause regressions because the
> COMMON_CLK dependency hides another dependency on something else
> that not everything implements, but we should fix that properly if that happens.
Compile-testing was enabled 2 years ago, in commit 2e280c188f06b190
("stmmac: make platform drivers depend on their associated SoC"), but the
dependency on COMMON_CLK was added only recently.
That's what triggered me: the drivers were suddenly disabled in m68k
allmodconfig,
while they built fine for years before.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
Hi Geert,
On 11/06/18 18:44, Geert Uytterhoeven wrote:
> Coldfire still provides its own variant of the clk API rather than using
> the generic COMMON_CLK API. This generally works, but it causes some
> link errors with drivers using the clk_round_rate(), clk_set_rate(),
> clk_set_parent(), or clk_get_parent() functions when a platform lacks
> those interfaces.
>
> This adds empty stub implementations for each of them, and I don't even
> try to do something useful here but instead just print a WARN() message
> to make it obvious what is going on if they ever end up being called.
>
> The drivers that call these won't be used on these platforms (otherwise
> we'd get a link error today), so the added code is harmless bloat and
> will warn about accidental use.
>
> Based on commit bd7fefe1f06ca6cc ("ARM: w90x900: normalize clk API").
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
I am fine with this for ColdFire, so
Acked-by: Greg Ungerer <[email protected]>
Are you going to take this/these via your m68k git tree?
Regards
Greg
> ---
> arch/m68k/coldfire/clk.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/arch/m68k/coldfire/clk.c b/arch/m68k/coldfire/clk.c
> index 849cd208e2ed99e6..7bc666e482ebe82f 100644
> --- a/arch/m68k/coldfire/clk.c
> +++ b/arch/m68k/coldfire/clk.c
> @@ -129,4 +129,33 @@ unsigned long clk_get_rate(struct clk *clk)
> }
> EXPORT_SYMBOL(clk_get_rate);
>
> +/* dummy functions, should not be called */
> +long clk_round_rate(struct clk *clk, unsigned long rate)
> +{
> + WARN_ON(clk);
> + return 0;
> +}
> +EXPORT_SYMBOL(clk_round_rate);
> +
> +int clk_set_rate(struct clk *clk, unsigned long rate)
> +{
> + WARN_ON(clk);
> + return 0;
> +}
> +EXPORT_SYMBOL(clk_set_rate);
> +
> +int clk_set_parent(struct clk *clk, struct clk *parent)
> +{
> + WARN_ON(clk);
> + return 0;
> +}
> +EXPORT_SYMBOL(clk_set_parent);
> +
> +struct clk *clk_get_parent(struct clk *clk)
> +{
> + WARN_ON(clk);
> + return NULL;
> +}
> +EXPORT_SYMBOL(clk_get_parent);
> +
> /***************************************************************************/
>
Hi Greg,
On Tue, Jun 12, 2018 at 9:27 AM Greg Ungerer <[email protected]> wrote:
> On 11/06/18 18:44, Geert Uytterhoeven wrote:
> > Coldfire still provides its own variant of the clk API rather than using
> > the generic COMMON_CLK API. This generally works, but it causes some
> > link errors with drivers using the clk_round_rate(), clk_set_rate(),
> > clk_set_parent(), or clk_get_parent() functions when a platform lacks
> > those interfaces.
> >
> > This adds empty stub implementations for each of them, and I don't even
> > try to do something useful here but instead just print a WARN() message
> > to make it obvious what is going on if they ever end up being called.
> >
> > The drivers that call these won't be used on these platforms (otherwise
> > we'd get a link error today), so the added code is harmless bloat and
> > will warn about accidental use.
> >
> > Based on commit bd7fefe1f06ca6cc ("ARM: w90x900: normalize clk API").
> >
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
>
> I am fine with this for ColdFire, so
>
> Acked-by: Greg Ungerer <[email protected]>
Thanks!
> Are you going to take this/these via your m68k git tree?
I''m fine delagating this to you.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
Hi Geert,
On 12/06/18 17:31, Geert Uytterhoeven wrote:
> On Tue, Jun 12, 2018 at 9:27 AM Greg Ungerer <[email protected]> wrote:
>> On 11/06/18 18:44, Geert Uytterhoeven wrote:
>>> Coldfire still provides its own variant of the clk API rather than using
>>> the generic COMMON_CLK API. This generally works, but it causes some
>>> link errors with drivers using the clk_round_rate(), clk_set_rate(),
>>> clk_set_parent(), or clk_get_parent() functions when a platform lacks
>>> those interfaces.
>>>
>>> This adds empty stub implementations for each of them, and I don't even
>>> try to do something useful here but instead just print a WARN() message
>>> to make it obvious what is going on if they ever end up being called.
>>>
>>> The drivers that call these won't be used on these platforms (otherwise
>>> we'd get a link error today), so the added code is harmless bloat and
>>> will warn about accidental use.
>>>
>>> Based on commit bd7fefe1f06ca6cc ("ARM: w90x900: normalize clk API").
>>>
>>> Signed-off-by: Geert Uytterhoeven <[email protected]>
>>
>> I am fine with this for ColdFire, so
>>
>> Acked-by: Greg Ungerer <[email protected]>
>
> Thanks!
>
>> Are you going to take this/these via your m68k git tree?
>
> I''m fine delagating this to you.
No problem. I'll add it to the m68knommu git tree (for-next branch)
when the merge window closes.
Thanks
Greg
Hi,
On 11-06-2018 09:44, Geert Uytterhoeven wrote:
> This reverts commit bde4975310eb1982bd0bbff673989052d92fd481.
>
> All legacy clock implementations now implement clk_set_rate() (Some
> implementations may be dummies, though).
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
>
This seems okay by me. You can send a non-rfc patch with my ack
once other 2 patches get accepted:
Acked-by: Jose Abreu <[email protected]>
Thanks and Best Regards,
Jose Miguel Abreu
Hi Geert,
On Mon, Jun 11, 2018 at 10:44:22AM +0200, Geert Uytterhoeven wrote:
> Coldfire still provides its own variant of the clk API rather than using
> the generic COMMON_CLK API. This generally works, but it causes some
> link errors with drivers using the clk_round_rate(), clk_set_rate(),
> clk_set_parent(), or clk_get_parent() functions when a platform lacks
> those interfaces.
>
> This adds empty stub implementations for each of them, and I don't even
> try to do something useful here but instead just print a WARN() message
> to make it obvious what is going on if they ever end up being called.
>
> The drivers that call these won't be used on these platforms (otherwise
> we'd get a link error today), so the added code is harmless bloat and
> will warn about accidental use.
>
> Based on commit bd7fefe1f06ca6cc ("ARM: w90x900: normalize clk API").
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> arch/mips/ar7/clock.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
Applied to mips-next for 4.19.
Thanks,
Paul