| Message ID | 20210316075551.10259-1-krzysztof.kozlowski@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] phy: tegra: depend on COMMON_CLK to fix compile tests | expand |
16.03.2021 10:55, Krzysztof Kozlowski пишет: > From: Krzysztof Kozlowski <krzk@kernel.org> > > The Tegra USB PHY driver uses Common Clock Framework thus it cannot be > built on platforms without it (e.g. compile test on MIPS with RALINK and > SOC_RT305X): > > /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 <krzk@kernel.org> > > --- > > Changes since v1: > 1. Depend on COMMON_CLK always, not only for compile test. > --- > drivers/usb/phy/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig > index 52eebcb88c1f..7500e77a7d01 100644 > --- a/drivers/usb/phy/Kconfig > +++ b/drivers/usb/phy/Kconfig > @@ -163,6 +163,7 @@ config USB_MXS_PHY > config USB_TEGRA_PHY > tristate "NVIDIA Tegra USB PHY Driver" > depends on ARCH_TEGRA || COMPILE_TEST > + depends on COMMON_CLK > select USB_COMMON > select USB_PHY > select USB_ULPI > But if COMMON_CLK is disabled, then include/linux/clk.h provides a stub for clk_get_parent(), meaning that MIPS has its own COMMON_CLK, no?
On Tue, 16 Mar 2021 at 16:43, Dmitry Osipenko <digetx@gmail.com> wrote: > > 16.03.2021 10:55, Krzysztof Kozlowski пишет: > > From: Krzysztof Kozlowski <krzk@kernel.org> > > > > The Tegra USB PHY driver uses Common Clock Framework thus it cannot be > > built on platforms without it (e.g. compile test on MIPS with RALINK and > > SOC_RT305X): > > > > /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 <krzk@kernel.org> > > > > --- > > > > Changes since v1: > > 1. Depend on COMMON_CLK always, not only for compile test. > > --- > > drivers/usb/phy/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig > > index 52eebcb88c1f..7500e77a7d01 100644 > > --- a/drivers/usb/phy/Kconfig > > +++ b/drivers/usb/phy/Kconfig > > @@ -163,6 +163,7 @@ config USB_MXS_PHY > > config USB_TEGRA_PHY > > tristate "NVIDIA Tegra USB PHY Driver" > > depends on ARCH_TEGRA || COMPILE_TEST > > + depends on COMMON_CLK > > select USB_COMMON > > select USB_PHY > > select USB_ULPI > > > > But if COMMON_CLK is disabled, then include/linux/clk.h provides a stub > for clk_get_parent(), meaning that MIPS has its own COMMON_CLK, no? Hi, It depends on the platform. Not all of them implement every clk API, so you can have failures: https://lore.kernel.org/lkml/202102170017.MgPVy7aZ-lkp@intel.com/ Best regards, Krzysztof
On Tue, 16 Mar 2021 at 16:47, Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> wrote: > > On Tue, 16 Mar 2021 at 16:43, Dmitry Osipenko <digetx@gmail.com> wrote: > > > > 16.03.2021 10:55, Krzysztof Kozlowski пишет: > > > From: Krzysztof Kozlowski <krzk@kernel.org> > > > > > > The Tegra USB PHY driver uses Common Clock Framework thus it cannot be > > > built on platforms without it (e.g. compile test on MIPS with RALINK and > > > SOC_RT305X): > > > > > > /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 <krzk@kernel.org> > > > > > > --- > > > > > > Changes since v1: > > > 1. Depend on COMMON_CLK always, not only for compile test. > > > --- > > > drivers/usb/phy/Kconfig | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig > > > index 52eebcb88c1f..7500e77a7d01 100644 > > > --- a/drivers/usb/phy/Kconfig > > > +++ b/drivers/usb/phy/Kconfig > > > @@ -163,6 +163,7 @@ config USB_MXS_PHY > > > config USB_TEGRA_PHY > > > tristate "NVIDIA Tegra USB PHY Driver" > > > depends on ARCH_TEGRA || COMPILE_TEST > > > + depends on COMMON_CLK > > > select USB_COMMON > > > select USB_PHY > > > select USB_ULPI > > > > > > > But if COMMON_CLK is disabled, then include/linux/clk.h provides a stub > > for clk_get_parent(), meaning that MIPS has its own COMMON_CLK, no? > > Hi, > > It depends on the platform. Not all of them implement every clk API, > so you can have failures: > https://lore.kernel.org/lkml/202102170017.MgPVy7aZ-lkp@intel.com/ Ah, you mentioned the stub, so let me clarify more. The common clk stubs are not used for cases like !COMMON_CLK && HAVE_LEGACY_CLK (or HAVE_CLK, I don't remember). This is why you can have a MIPS platform defining some of the clock operations thus not using COMMON_CLK at all (and neither the stubs). Best regards, Krzysztof
16.03.2021 18:51, Krzysztof Kozlowski пишет: > On Tue, 16 Mar 2021 at 16:47, Krzysztof Kozlowski > <krzysztof.kozlowski@canonical.com> wrote: >> >> On Tue, 16 Mar 2021 at 16:43, Dmitry Osipenko <digetx@gmail.com> wrote: >>> >>> 16.03.2021 10:55, Krzysztof Kozlowski пишет: >>>> From: Krzysztof Kozlowski <krzk@kernel.org> >>>> >>>> The Tegra USB PHY driver uses Common Clock Framework thus it cannot be >>>> built on platforms without it (e.g. compile test on MIPS with RALINK and >>>> SOC_RT305X): >>>> >>>> /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 <krzk@kernel.org> >>>> >>>> --- >>>> >>>> Changes since v1: >>>> 1. Depend on COMMON_CLK always, not only for compile test. >>>> --- >>>> drivers/usb/phy/Kconfig | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig >>>> index 52eebcb88c1f..7500e77a7d01 100644 >>>> --- a/drivers/usb/phy/Kconfig >>>> +++ b/drivers/usb/phy/Kconfig >>>> @@ -163,6 +163,7 @@ config USB_MXS_PHY >>>> config USB_TEGRA_PHY >>>> tristate "NVIDIA Tegra USB PHY Driver" >>>> depends on ARCH_TEGRA || COMPILE_TEST >>>> + depends on COMMON_CLK >>>> select USB_COMMON >>>> select USB_PHY >>>> select USB_ULPI >>>> >>> >>> But if COMMON_CLK is disabled, then include/linux/clk.h provides a stub >>> for clk_get_parent(), meaning that MIPS has its own COMMON_CLK, no? >> >> Hi, >> >> It depends on the platform. Not all of them implement every clk API, >> so you can have failures: >> https://lore.kernel.org/lkml/202102170017.MgPVy7aZ-lkp@intel.com/ > > Ah, you mentioned the stub, so let me clarify more. The common clk > stubs are not used for cases like !COMMON_CLK && HAVE_LEGACY_CLK (or > HAVE_CLK, I don't remember). This is why you can have a MIPS platform > defining some of the clock operations thus not using COMMON_CLK at all > (and neither the stubs). I see now that the stubs depend on CONFIG_HAVE_CLK and not COMMON_CLK, thanks. This raises question about why those platforms select CONFIG_HAVE_CLK, while not implementing it fully. Sounds like a better fix should be to add the missing stubs to the MIPS clk implementation, which should avoid the need to patch each driver individually. https://elixir.bootlin.com/linux/v5.12-rc3/source/arch/mips/ar7/clock.c#L489
On 16/03/2021 17:14, Dmitry Osipenko wrote: > 16.03.2021 18:51, Krzysztof Kozlowski пишет: >>>> >>>> But if COMMON_CLK is disabled, then include/linux/clk.h provides a stub >>>> for clk_get_parent(), meaning that MIPS has its own COMMON_CLK, no? >>> >>> Hi, >>> >>> It depends on the platform. Not all of them implement every clk API, >>> so you can have failures: >>> https://lore.kernel.org/lkml/202102170017.MgPVy7aZ-lkp@intel.com/ >> >> Ah, you mentioned the stub, so let me clarify more. The common clk >> stubs are not used for cases like !COMMON_CLK && HAVE_LEGACY_CLK (or >> HAVE_CLK, I don't remember). This is why you can have a MIPS platform >> defining some of the clock operations thus not using COMMON_CLK at all >> (and neither the stubs). > > I see now that the stubs depend on CONFIG_HAVE_CLK and not COMMON_CLK, > thanks. > > This raises question about why those platforms select CONFIG_HAVE_CLK, > while not implementing it fully. Sounds like a better fix should be to > add the missing stubs to the MIPS clk implementation, which should avoid > the need to patch each driver individually. > > https://elixir.bootlin.com/linux/v5.12-rc3/source/arch/mips/ar7/clock.c#L489 If such stubs as pointed by you are accepted, then indeed it would be better approach. Best regards, Krzysztof
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index 52eebcb88c1f..7500e77a7d01 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -163,6 +163,7 @@ config USB_MXS_PHY config USB_TEGRA_PHY tristate "NVIDIA Tegra USB PHY Driver" depends on ARCH_TEGRA || COMPILE_TEST + depends on COMMON_CLK select USB_COMMON select USB_PHY select USB_ULPI