| Message ID | 20210316175725.79981-1-krzysztof.kozlowski@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | MIPS: ralink: define stubs for clk_set_parent to fix compile testing | expand |
17.03.2021 00:58, Thomas Bogendoerfer пишет: > On Tue, Mar 16, 2021 at 06:57:25PM +0100, Krzysztof Kozlowski wrote: >> The Ralink MIPS platform does not use Common Clock Framework and does >> not define certain clock operations leading to compile test failures: >> >> /usr/bin/mips-linux-gnu-ld: drivers/usb/phy/phy-tegra-usb.o: in function `tegra_usb_phy_init': >> phy-tegra-usb.c:(.text+0x1dd4): undefined reference to `clk_get_parent' > > hmm, why not make it use common clock framework ? And shouldn't > include/linux/clk.h provide what you need, if CONFIG_HAVE_CLK is not set ? That should increase kernel size by a couple kbytes. If size isn't important, then somebody should dedicate time and energy on creating the patches.
On 17/03/2021 01:10, Dmitry Osipenko wrote: > 16.03.2021 20:57, Krzysztof Kozlowski пишет: >> The Ralink MIPS platform does not use Common Clock Framework and does >> not define certain clock operations leading to compile test failures: >> >> /usr/bin/mips-linux-gnu-ld: drivers/usb/phy/phy-tegra-usb.o: in function `tegra_usb_phy_init': >> phy-tegra-usb.c:(.text+0x1dd4): undefined reference to `clk_get_parent' >> >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> >> --- >> arch/mips/ralink/clk.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c >> index 2f9d5acb38ea..8387177a47ef 100644 >> --- a/arch/mips/ralink/clk.c >> +++ b/arch/mips/ralink/clk.c >> @@ -70,6 +70,20 @@ long clk_round_rate(struct clk *clk, unsigned long rate) >> } >> EXPORT_SYMBOL_GPL(clk_round_rate); >> >> +int clk_set_parent(struct clk *clk, struct clk *parent) >> +{ >> + WARN_ON(clk); >> + return -1; >> +} >> +EXPORT_SYMBOL(clk_set_parent); >> + >> +struct clk *clk_get_parent(struct clk *clk) >> +{ >> + WARN_ON(clk); >> + return NULL; >> +} >> +EXPORT_SYMBOL(clk_get_parent); > > I'm wondering whether symbols should be GPL or it doesn't matter in this > case. Otherwise this looks good to me. The ones in arch/mips/ar7/clock.c were not GPL but other stubs already defined here are, so indeed I'll make them GPL for consistency. > > Also, I guess it should be possible to create a generic clk stubs that > will use weak functions, allowing platforms to override only the wanted > stubs and then we won't need to worry about the missing stubs for each > platform individually. But of course that will be a much bigger change. > Just wanted to share my thoughts. Yes, it would be a good idea but also a bigger task. I am not sure if these platforms are alive enough to get that attention. Best regards, Krzysztof
On 17/03/2021 10:52, Sergei Shtylyov wrote: > Hello! > > On 16.03.2021 20:57, Krzysztof Kozlowski wrote: > >> The Ralink MIPS platform does not use Common Clock Framework and does >> not define certain clock operations leading to compile test failures: >> >> /usr/bin/mips-linux-gnu-ld: drivers/usb/phy/phy-tegra-usb.o: in function `tegra_usb_phy_init': >> phy-tegra-usb.c:(.text+0x1dd4): undefined reference to `clk_get_parent' >> >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> >> --- >> arch/mips/ralink/clk.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c >> index 2f9d5acb38ea..8387177a47ef 100644 >> --- a/arch/mips/ralink/clk.c >> +++ b/arch/mips/ralink/clk.c >> @@ -70,6 +70,20 @@ long clk_round_rate(struct clk *clk, unsigned long rate) >> } >> EXPORT_SYMBOL_GPL(clk_round_rate); >> >> +int clk_set_parent(struct clk *clk, struct clk *parent) >> +{ >> + WARN_ON(clk); >> + return -1; > > Shouldn't this be a proepr error code (-1 corresponds to -EPRERM)? Could be ENODEV or EINVAL but all other stubs here and in ar7/clock.c use -1. Do you prefer it then to have it inconsistent with others? Best regards, Krzysztof
On 17.03.2021 12:56, Krzysztof Kozlowski wrote: [...] >>> The Ralink MIPS platform does not use Common Clock Framework and does >>> not define certain clock operations leading to compile test failures: >>> >>> /usr/bin/mips-linux-gnu-ld: drivers/usb/phy/phy-tegra-usb.o: in function `tegra_usb_phy_init': >>> phy-tegra-usb.c:(.text+0x1dd4): undefined reference to `clk_get_parent' >>> >>> Reported-by: kernel test robot <lkp@intel.com> >>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> >>> --- >>> arch/mips/ralink/clk.c | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >>> >>> diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c >>> index 2f9d5acb38ea..8387177a47ef 100644 >>> --- a/arch/mips/ralink/clk.c >>> +++ b/arch/mips/ralink/clk.c >>> @@ -70,6 +70,20 @@ long clk_round_rate(struct clk *clk, unsigned long rate) >>> } >>> EXPORT_SYMBOL_GPL(clk_round_rate); >>> >>> +int clk_set_parent(struct clk *clk, struct clk *parent) >>> +{ >>> + WARN_ON(clk); >>> + return -1; >> >> Shouldn't this be a proepr error code (-1 corresponds to -EPRERM)? > > Could be ENODEV or EINVAL but all other stubs here and in ar7/clock.c > use -1. Do you prefer it then to have it inconsistent with others? No. :-) > Best regards, > Krzysztof MBR, Sergei
On Wed, 17 Mar 2021 at 20:37, Dmitry Osipenko <digetx@gmail.com> wrote: > > 17.03.2021 12:56, Krzysztof Kozlowski пишет: > > On 17/03/2021 10:52, Sergei Shtylyov wrote: > >> Hello! > >> > >> On 16.03.2021 20:57, Krzysztof Kozlowski wrote: > >> > >>> The Ralink MIPS platform does not use Common Clock Framework and does > >>> not define certain clock operations leading to compile test failures: > >>> > >>> /usr/bin/mips-linux-gnu-ld: drivers/usb/phy/phy-tegra-usb.o: in function `tegra_usb_phy_init': > >>> phy-tegra-usb.c:(.text+0x1dd4): undefined reference to `clk_get_parent' > >>> > >>> Reported-by: kernel test robot <lkp@intel.com> > >>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> > >>> --- > >>> arch/mips/ralink/clk.c | 14 ++++++++++++++ > >>> 1 file changed, 14 insertions(+) > >>> > >>> diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c > >>> index 2f9d5acb38ea..8387177a47ef 100644 > >>> --- a/arch/mips/ralink/clk.c > >>> +++ b/arch/mips/ralink/clk.c > >>> @@ -70,6 +70,20 @@ long clk_round_rate(struct clk *clk, unsigned long rate) > >>> } > >>> EXPORT_SYMBOL_GPL(clk_round_rate); > >>> > >>> +int clk_set_parent(struct clk *clk, struct clk *parent) > >>> +{ > >>> + WARN_ON(clk); > >>> + return -1; > >> > >> Shouldn't this be a proepr error code (-1 corresponds to -EPRERM)? > > > > Could be ENODEV or EINVAL but all other stubs here and in ar7/clock.c > > use -1. Do you prefer it then to have it inconsistent with others? > > I don't see where -1 is used, ar7/clock.c returns 0. Other drivers > either return 0 or EINVAL. > > Since linux/clk.h returns 0 in the stub, I think 0 is the correct variant. The ar7 returns 0 but the other stubs in ralink return -1. Best regards, Krzysztof
diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c index 2f9d5acb38ea..8387177a47ef 100644 --- a/arch/mips/ralink/clk.c +++ b/arch/mips/ralink/clk.c @@ -70,6 +70,20 @@ long clk_round_rate(struct clk *clk, unsigned long rate) } EXPORT_SYMBOL_GPL(clk_round_rate); +int clk_set_parent(struct clk *clk, struct clk *parent) +{ + WARN_ON(clk); + return -1; +} +EXPORT_SYMBOL(clk_set_parent); + +struct clk *clk_get_parent(struct clk *clk) +{ + WARN_ON(clk); + return NULL; +} +EXPORT_SYMBOL(clk_get_parent); + void __init plat_time_init(void) { struct clk *clk;
The Ralink MIPS platform does not use Common Clock Framework and does not define certain clock operations leading to compile test failures: /usr/bin/mips-linux-gnu-ld: drivers/usb/phy/phy-tegra-usb.o: in function `tegra_usb_phy_init': phy-tegra-usb.c:(.text+0x1dd4): undefined reference to `clk_get_parent' Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> --- arch/mips/ralink/clk.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)