diff mbox series

phy: PHY_FSL_LYNX_28G should depend on ARCH_LAYERSCAPE

Message ID 20220329125631.2915388-1-geert@linux-m68k.org
State Accepted
Commit 37fcacb50be7071d146144a6c5c5bf0194b9a1cf
Headers show
Series phy: PHY_FSL_LYNX_28G should depend on ARCH_LAYERSCAPE | expand

Commit Message

Geert Uytterhoeven March 29, 2022, 12:56 p.m. UTC
From: Geert Uytterhoeven <geert+renesas@glider.be>

Freescale Layerscape Lynx 28G SerDes PHYs are only present on
Freescale/NXP Layerscape SoCs.

Move PHY_FSL_LYNX_28G outside the block for ARCH_MXC, as the latter
is meant for i.MX8 SoCs, which is a different family than Layerscape.
Add a dependency on ARCH_LAYERSCAPE, to prevent asking the user about
this driver when configuring a kernel without Layerscape SoC support.

Fixes: 02e2af20f4f9f2aa ("Merge tag 'char-misc-5.18-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc")
Fixes: 8f73b37cf3fbda67 ("phy: add support for the Layerscape SerDes 28G")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/phy/freescale/Kconfig | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Greg Kroah-Hartman March 29, 2022, 2:18 p.m. UTC | #1
On Tue, Mar 29, 2022 at 02:56:31PM +0200, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Freescale Layerscape Lynx 28G SerDes PHYs are only present on
> Freescale/NXP Layerscape SoCs.
> 
> Move PHY_FSL_LYNX_28G outside the block for ARCH_MXC, as the latter
> is meant for i.MX8 SoCs, which is a different family than Layerscape.
> Add a dependency on ARCH_LAYERSCAPE, to prevent asking the user about
> this driver when configuring a kernel without Layerscape SoC support.

Why the artificial ARCH dependency?  What happens when people want to
build kernels for multiple arches at the same time?  We shouldn't put
these restrictions on just to have to go back later and remove them.


> 
> Fixes: 02e2af20f4f9f2aa ("Merge tag 'char-misc-5.18-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc")
> Fixes: 8f73b37cf3fbda67 ("phy: add support for the Layerscape SerDes 28G")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/phy/freescale/Kconfig | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
> index 8d945211c7b40727..f9c54cd02036285b 100644
> --- a/drivers/phy/freescale/Kconfig
> +++ b/drivers/phy/freescale/Kconfig
> @@ -26,14 +26,15 @@ config PHY_FSL_IMX8M_PCIE
>  	  Enable this to add support for the PCIE PHY as found on
>  	  i.MX8M family of SOCs.
>  
> +endif
> +
>  config PHY_FSL_LYNX_28G
>  	tristate "Freescale Layerscape Lynx 28G SerDes PHY support"
>  	depends on OF
> +	depends on ARCH_LAYERSCAPE || COMPILE_TEST
>  	select GENERIC_PHY
>  	help
>  	  Enable this to add support for the Lynx SerDes 28G PHY as
>  	  found on NXP's Layerscape platforms such as LX2160A.
>  	  Used to change the protocol running on SerDes lanes at runtime.
>  	  Only useful for a restricted set of Ethernet protocols.
> -
> -endif

The movement of the #endif is fine, how about just sending that change
as that is the merge issue here.

thanks,

greg k-h
Geert Uytterhoeven March 29, 2022, 3:14 p.m. UTC | #2
Hi Greg,

On Tue, Mar 29, 2022 at 4:18 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Mar 29, 2022 at 02:56:31PM +0200, Geert Uytterhoeven wrote:
> > From: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > Freescale Layerscape Lynx 28G SerDes PHYs are only present on
> > Freescale/NXP Layerscape SoCs.
> >
> > Move PHY_FSL_LYNX_28G outside the block for ARCH_MXC, as the latter
> > is meant for i.MX8 SoCs, which is a different family than Layerscape.
> > Add a dependency on ARCH_LAYERSCAPE, to prevent asking the user about
> > this driver when configuring a kernel without Layerscape SoC support.
>
> Why the artificial ARCH dependency?  What happens when people want to

Because it does not make sense to ask Linus^Weverybody about this
driver when configuring a kernel that cannot even run on a platform
that has this hardware.

Please read "Architecture and platform dependencies"
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/kbuild/kconfig-language.rst#n552

> build kernels for multiple arches at the same time?  We shouldn't put

If you build a kernel to run on multiple arches, you should have
support for all of these arches enabled anyway, else it won't run.
E.g. both are enabled in the arm64 defconfig:
$ git grep -E "ARCH_(MXC|LAYERSCAPE)" -- arch/arm64/configs/defconfig
arch/arm64/configs/defconfig:CONFIG_ARCH_LAYERSCAPE=y
arch/arm64/configs/defconfig:CONFIG_ARCH_MXC=y

> these restrictions on just to have to go back later and remove them.

What do you gain by removing them, apart from having to spend more
time making love with your kernel configuration?
Do you (and everyone else) really want to see all 19190 config
symbol questions?

> > Fixes: 02e2af20f4f9f2aa ("Merge tag 'char-misc-5.18-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc")
> > Fixes: 8f73b37cf3fbda67 ("phy: add support for the Layerscape SerDes 28G")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

> > --- a/drivers/phy/freescale/Kconfig
> > +++ b/drivers/phy/freescale/Kconfig
> > @@ -26,14 +26,15 @@ config PHY_FSL_IMX8M_PCIE
> >         Enable this to add support for the PCIE PHY as found on
> >         i.MX8M family of SOCs.
> >
> > +endif
> > +
> >  config PHY_FSL_LYNX_28G
> >       tristate "Freescale Layerscape Lynx 28G SerDes PHY support"
> >       depends on OF
> > +     depends on ARCH_LAYERSCAPE || COMPILE_TEST
> >       select GENERIC_PHY
> >       help
> >         Enable this to add support for the Lynx SerDes 28G PHY as
> >         found on NXP's Layerscape platforms such as LX2160A.
> >         Used to change the protocol running on SerDes lanes at runtime.
> >         Only useful for a restricted set of Ethernet protocols.
> > -
> > -endif
>
> The movement of the #endif is fine, how about just sending that change
> as that is the merge issue here.

The Lynx 28G SerDes PHY driver should depend on ARCH_LAYERSCAPE
instead of ARCH_MXC.  The former needs a new dependency, the latter
needs the if to bemoved.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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
Linus Torvalds March 29, 2022, 3:45 p.m. UTC | #3
On Tue, Mar 29, 2022 at 5:56 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Freescale Layerscape Lynx 28G SerDes PHYs are only present on
> Freescale/NXP Layerscape SoCs.
>
> Move PHY_FSL_LYNX_28G outside the block for ARCH_MXC, as the latter
> is meant for i.MX8 SoCs, which is a different family than Layerscape.
> Add a dependency on ARCH_LAYERSCAPE, to prevent asking the user about
> this driver when configuring a kernel without Layerscape SoC support.

I took this directly, since one of the bugs it fixed was mine.

Mea culpa,
             Linus
diff mbox series

Patch

diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
index 8d945211c7b40727..f9c54cd02036285b 100644
--- a/drivers/phy/freescale/Kconfig
+++ b/drivers/phy/freescale/Kconfig
@@ -26,14 +26,15 @@  config PHY_FSL_IMX8M_PCIE
 	  Enable this to add support for the PCIE PHY as found on
 	  i.MX8M family of SOCs.
 
+endif
+
 config PHY_FSL_LYNX_28G
 	tristate "Freescale Layerscape Lynx 28G SerDes PHY support"
 	depends on OF
+	depends on ARCH_LAYERSCAPE || COMPILE_TEST
 	select GENERIC_PHY
 	help
 	  Enable this to add support for the Lynx SerDes 28G PHY as
 	  found on NXP's Layerscape platforms such as LX2160A.
 	  Used to change the protocol running on SerDes lanes at runtime.
 	  Only useful for a restricted set of Ethernet protocols.
-
-endif