| Message ID | 20231208045054.27966-6-luizluca@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: realtek: variants to drivers, interfaces to a common module | expand |
On Fri, Dec 08, 2023 at 01:41:41AM -0300, Luiz Angelo Daros de Luca wrote: > As both realtek-common and realtek-{smi,mdio} must always be loaded > together, we can save some resources merging them into a single module. > > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> > --- > drivers/net/dsa/realtek/Kconfig | 4 ++-- > drivers/net/dsa/realtek/Makefile | 8 +++++--- > drivers/net/dsa/realtek/realtek-common.c | 1 + > drivers/net/dsa/realtek/realtek-mdio.c | 4 ---- > drivers/net/dsa/realtek/realtek-smi.c | 4 ---- > 5 files changed, 8 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/dsa/realtek/Kconfig b/drivers/net/dsa/realtek/Kconfig > index 9d182fde11b4..6989972eebc3 100644 > --- a/drivers/net/dsa/realtek/Kconfig > +++ b/drivers/net/dsa/realtek/Kconfig > @@ -16,14 +16,14 @@ menuconfig NET_DSA_REALTEK > if NET_DSA_REALTEK > > config NET_DSA_REALTEK_MDIO > - tristate "Realtek MDIO interface support" > + bool "Realtek MDIO interface support" > depends on OF > help > Select to enable support for registering switches configured > through MDIO. > > config NET_DSA_REALTEK_SMI > - tristate "Realtek SMI interface support" > + bool "Realtek SMI interface support" > depends on OF > help > Select to enable support for registering switches connected > diff --git a/drivers/net/dsa/realtek/Makefile b/drivers/net/dsa/realtek/Makefile > index 5e0c1ef200a3..88f6652f9850 100644 > --- a/drivers/net/dsa/realtek/Makefile > +++ b/drivers/net/dsa/realtek/Makefile > @@ -1,7 +1,9 @@ > # SPDX-License-Identifier: GPL-2.0 > -obj-$(CONFIG_NET_DSA_REALTEK) += realtek-common.o > -obj-$(CONFIG_NET_DSA_REALTEK_MDIO) += realtek-mdio.o > -obj-$(CONFIG_NET_DSA_REALTEK_SMI) += realtek-smi.o > +obj-$(CONFIG_NET_DSA_REALTEK) += realtek_common.o > +realtek_common-objs-y := realtek-common.o This is weird with the - and _. Also realtek-common is not a very descriptive module name. Maybe realtek-dsa? obj-$(CONFIG_NET_DSA_REALTEK) += realtek-dsa.o realtek-dsa-objs-y += realtek-common.o realtek-dsa-objs-$(..._MDIO) += realtek-mdio.o realtek-dsa-objs-$(..._SMI) += realtek-smi.o Also what happens if I just enable CONFIG_NET_DSA_REALTEK and nothing else. Do I get a module that doesn't do anything? Not sure if it's a big deal. > +realtek_common-objs-$(CONFIG_NET_DSA_REALTEK_MDIO) += realtek-mdio.o > +realtek_common-objs-$(CONFIG_NET_DSA_REALTEK_SMI) += realtek-smi.o > +realtek_common-objs := $(realtek_common-objs-y) > obj-$(CONFIG_NET_DSA_REALTEK_RTL8366RB) += rtl8366.o > rtl8366-objs := rtl8366-core.o rtl8366rb.o > obj-$(CONFIG_NET_DSA_REALTEK_RTL8365MB) += rtl8365mb.o > diff --git a/drivers/net/dsa/realtek/realtek-common.c b/drivers/net/dsa/realtek/realtek-common.c > index 75b6aa071990..73c25d114dd3 100644 > --- a/drivers/net/dsa/realtek/realtek-common.c > +++ b/drivers/net/dsa/realtek/realtek-common.c > @@ -132,5 +132,6 @@ void realtek_common_remove(struct realtek_priv *priv) > EXPORT_SYMBOL(realtek_common_remove); > > MODULE_AUTHOR("Luiz Angelo Daros de Luca <luizluca@gmail.com>"); > +MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>"); > MODULE_DESCRIPTION("Realtek DSA switches common module"); > MODULE_LICENSE("GPL"); > diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c > index 4c9a744b72f8..bb5bff719ae9 100644 > --- a/drivers/net/dsa/realtek/realtek-mdio.c > +++ b/drivers/net/dsa/realtek/realtek-mdio.c > @@ -168,7 +168,3 @@ void realtek_mdio_shutdown(struct mdio_device *mdiodev) > dev_set_drvdata(&mdiodev->dev, NULL); > } > EXPORT_SYMBOL_GPL(realtek_mdio_shutdown); > - > -MODULE_AUTHOR("Luiz Angelo Daros de Luca <luizluca@gmail.com>"); > -MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via MDIO interface"); > -MODULE_LICENSE("GPL"); > diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c > index 246024eec3bd..1ca2aa784d24 100644 > --- a/drivers/net/dsa/realtek/realtek-smi.c > +++ b/drivers/net/dsa/realtek/realtek-smi.c > @@ -443,7 +443,3 @@ void realtek_smi_shutdown(struct platform_device *pdev) > platform_set_drvdata(pdev, NULL); > } > EXPORT_SYMBOL_GPL(realtek_smi_shutdown); > - > -MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>"); > -MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via SMI interface"); > -MODULE_LICENSE("GPL"); > -- > 2.43.0 >
> > As both realtek-common and realtek-{smi,mdio} must always be loaded > > together, we can save some resources merging them into a single module. > > > > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> > > --- > > drivers/net/dsa/realtek/Kconfig | 4 ++-- > > drivers/net/dsa/realtek/Makefile | 8 +++++--- > > drivers/net/dsa/realtek/realtek-common.c | 1 + > > drivers/net/dsa/realtek/realtek-mdio.c | 4 ---- > > drivers/net/dsa/realtek/realtek-smi.c | 4 ---- > > 5 files changed, 8 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/net/dsa/realtek/Kconfig b/drivers/net/dsa/realtek/Kconfig > > index 9d182fde11b4..6989972eebc3 100644 > > --- a/drivers/net/dsa/realtek/Kconfig > > +++ b/drivers/net/dsa/realtek/Kconfig > > @@ -16,14 +16,14 @@ menuconfig NET_DSA_REALTEK > > if NET_DSA_REALTEK > > > > config NET_DSA_REALTEK_MDIO > > - tristate "Realtek MDIO interface support" > > + bool "Realtek MDIO interface support" > > depends on OF > > help > > Select to enable support for registering switches configured > > through MDIO. > > > > config NET_DSA_REALTEK_SMI > > - tristate "Realtek SMI interface support" > > + bool "Realtek SMI interface support" > > depends on OF > > help > > Select to enable support for registering switches connected > > diff --git a/drivers/net/dsa/realtek/Makefile b/drivers/net/dsa/realtek/Makefile > > index 5e0c1ef200a3..88f6652f9850 100644 > > --- a/drivers/net/dsa/realtek/Makefile > > +++ b/drivers/net/dsa/realtek/Makefile > > @@ -1,7 +1,9 @@ > > # SPDX-License-Identifier: GPL-2.0 > > -obj-$(CONFIG_NET_DSA_REALTEK) += realtek-common.o > > -obj-$(CONFIG_NET_DSA_REALTEK_MDIO) += realtek-mdio.o > > -obj-$(CONFIG_NET_DSA_REALTEK_SMI) += realtek-smi.o > > +obj-$(CONFIG_NET_DSA_REALTEK) += realtek_common.o > > +realtek_common-objs-y := realtek-common.o > > This is weird with the - and _. Also realtek-common is not a very > descriptive module name. Maybe realtek-dsa? > > obj-$(CONFIG_NET_DSA_REALTEK) += realtek-dsa.o > realtek-dsa-objs-y += realtek-common.o > realtek-dsa-objs-$(..._MDIO) += realtek-mdio.o > realtek-dsa-objs-$(..._SMI) += realtek-smi.o Yes, I'm not proud of it. The realtek_common/realtek-common trick is just to bypass the fact that I cannot link multiple files into a module that has the same name as one of these files. But realtek-dsa is fine and it would avoid conflicts with other realtek stuff in the kernel that might have a common module. However, I would introduce that name already in the previous patch. > Also what happens if I just enable CONFIG_NET_DSA_REALTEK and nothing > else. Do I get a module that doesn't do anything? Not sure if it's a big > deal. The config language might not be good enough to handle that nicely. There is the "imply" keyword but it does not force anything. I don't know how to require "at least one of these two interfaces" without creating a dependency cycle. I would just let it build a useless common module. Building without variants does make a little bit of sense if you want to build a new driver out-of-tree. > > +realtek_common-objs-$(CONFIG_NET_DSA_REALTEK_MDIO) += realtek-mdio.o > > +realtek_common-objs-$(CONFIG_NET_DSA_REALTEK_SMI) += realtek-smi.o > > +realtek_common-objs := $(realtek_common-objs-y) > > obj-$(CONFIG_NET_DSA_REALTEK_RTL8366RB) += rtl8366.o > > rtl8366-objs := rtl8366-core.o rtl8366rb.o > > obj-$(CONFIG_NET_DSA_REALTEK_RTL8365MB) += rtl8365mb.o > > diff --git a/drivers/net/dsa/realtek/realtek-common.c b/drivers/net/dsa/realtek/realtek-common.c > > index 75b6aa071990..73c25d114dd3 100644 > > --- a/drivers/net/dsa/realtek/realtek-common.c > > +++ b/drivers/net/dsa/realtek/realtek-common.c > > @@ -132,5 +132,6 @@ void realtek_common_remove(struct realtek_priv *priv) > > EXPORT_SYMBOL(realtek_common_remove); > > > > MODULE_AUTHOR("Luiz Angelo Daros de Luca <luizluca@gmail.com>"); > > +MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>"); > > MODULE_DESCRIPTION("Realtek DSA switches common module"); > > MODULE_LICENSE("GPL"); > > diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c > > index 4c9a744b72f8..bb5bff719ae9 100644 > > --- a/drivers/net/dsa/realtek/realtek-mdio.c > > +++ b/drivers/net/dsa/realtek/realtek-mdio.c > > @@ -168,7 +168,3 @@ void realtek_mdio_shutdown(struct mdio_device *mdiodev) > > dev_set_drvdata(&mdiodev->dev, NULL); > > } > > EXPORT_SYMBOL_GPL(realtek_mdio_shutdown); > > - > > -MODULE_AUTHOR("Luiz Angelo Daros de Luca <luizluca@gmail.com>"); > > -MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via MDIO interface"); > > -MODULE_LICENSE("GPL"); > > diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c > > index 246024eec3bd..1ca2aa784d24 100644 > > --- a/drivers/net/dsa/realtek/realtek-smi.c > > +++ b/drivers/net/dsa/realtek/realtek-smi.c > > @@ -443,7 +443,3 @@ void realtek_smi_shutdown(struct platform_device *pdev) > > platform_set_drvdata(pdev, NULL); > > } > > EXPORT_SYMBOL_GPL(realtek_smi_shutdown); > > - > > -MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>"); > > -MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via SMI interface"); > > -MODULE_LICENSE("GPL"); > > -- > > 2.43.0 > >
On Mon, Dec 11, 2023 at 02:13:56AM -0300, Luiz Angelo Daros de Luca wrote: > > > As both realtek-common and realtek-{smi,mdio} must always be loaded > > > together, we can save some resources merging them into a single module. > > > > > > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> > > > --- > > > drivers/net/dsa/realtek/Kconfig | 4 ++-- > > > drivers/net/dsa/realtek/Makefile | 8 +++++--- > > > drivers/net/dsa/realtek/realtek-common.c | 1 + > > > drivers/net/dsa/realtek/realtek-mdio.c | 4 ---- > > > drivers/net/dsa/realtek/realtek-smi.c | 4 ---- > > > 5 files changed, 8 insertions(+), 13 deletions(-) > > > > > > diff --git a/drivers/net/dsa/realtek/Kconfig b/drivers/net/dsa/realtek/Kconfig > > > index 9d182fde11b4..6989972eebc3 100644 > > > --- a/drivers/net/dsa/realtek/Kconfig > > > +++ b/drivers/net/dsa/realtek/Kconfig > > > @@ -16,14 +16,14 @@ menuconfig NET_DSA_REALTEK > > > if NET_DSA_REALTEK > > > > > > config NET_DSA_REALTEK_MDIO > > > - tristate "Realtek MDIO interface support" > > > + bool "Realtek MDIO interface support" > > > depends on OF > > > help > > > Select to enable support for registering switches configured > > > through MDIO. > > > > > > config NET_DSA_REALTEK_SMI > > > - tristate "Realtek SMI interface support" > > > + bool "Realtek SMI interface support" > > > depends on OF > > > help > > > Select to enable support for registering switches connected > > > diff --git a/drivers/net/dsa/realtek/Makefile b/drivers/net/dsa/realtek/Makefile > > > index 5e0c1ef200a3..88f6652f9850 100644 > > > --- a/drivers/net/dsa/realtek/Makefile > > > +++ b/drivers/net/dsa/realtek/Makefile > > > @@ -1,7 +1,9 @@ > > > # SPDX-License-Identifier: GPL-2.0 > > > -obj-$(CONFIG_NET_DSA_REALTEK) += realtek-common.o > > > -obj-$(CONFIG_NET_DSA_REALTEK_MDIO) += realtek-mdio.o > > > -obj-$(CONFIG_NET_DSA_REALTEK_SMI) += realtek-smi.o > > > +obj-$(CONFIG_NET_DSA_REALTEK) += realtek_common.o > > > +realtek_common-objs-y := realtek-common.o > > > > This is weird with the - and _. Also realtek-common is not a very > > descriptive module name. Maybe realtek-dsa? > > > > obj-$(CONFIG_NET_DSA_REALTEK) += realtek-dsa.o > > realtek-dsa-objs-y += realtek-common.o > > realtek-dsa-objs-$(..._MDIO) += realtek-mdio.o > > realtek-dsa-objs-$(..._SMI) += realtek-smi.o > > Yes, I'm not proud of it. The realtek_common/realtek-common trick is > just to bypass the fact that I cannot link multiple files into a > module that has the same name as one of these files. But realtek-dsa > is fine and it would avoid conflicts with other realtek stuff in the > kernel that might have a common module. However, I would introduce > that name already in the previous patch. OK > > > Also what happens if I just enable CONFIG_NET_DSA_REALTEK and nothing > > else. Do I get a module that doesn't do anything? Not sure if it's a big > > deal. > > The config language might not be good enough to handle that nicely. > There is the "imply" keyword but it does not force anything. I don't > know how to require "at least one of these two interfaces" without > creating a dependency cycle. I would just let it build a useless > common module. Building without variants does make a little bit of > sense if you want to build a new driver out-of-tree. Agree, I don't know a good way to prevent it. I don't think it's a big deal. > > > > +realtek_common-objs-$(CONFIG_NET_DSA_REALTEK_MDIO) += realtek-mdio.o > > > +realtek_common-objs-$(CONFIG_NET_DSA_REALTEK_SMI) += realtek-smi.o > > > +realtek_common-objs := $(realtek_common-objs-y) > > > obj-$(CONFIG_NET_DSA_REALTEK_RTL8366RB) += rtl8366.o > > > rtl8366-objs := rtl8366-core.o rtl8366rb.o > > > obj-$(CONFIG_NET_DSA_REALTEK_RTL8365MB) += rtl8365mb.o > > > diff --git a/drivers/net/dsa/realtek/realtek-common.c b/drivers/net/dsa/realtek/realtek-common.c > > > index 75b6aa071990..73c25d114dd3 100644 > > > --- a/drivers/net/dsa/realtek/realtek-common.c > > > +++ b/drivers/net/dsa/realtek/realtek-common.c > > > @@ -132,5 +132,6 @@ void realtek_common_remove(struct realtek_priv *priv) > > > EXPORT_SYMBOL(realtek_common_remove); > > > > > > MODULE_AUTHOR("Luiz Angelo Daros de Luca <luizluca@gmail.com>"); > > > +MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>"); > > > MODULE_DESCRIPTION("Realtek DSA switches common module"); > > > MODULE_LICENSE("GPL"); > > > diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c > > > index 4c9a744b72f8..bb5bff719ae9 100644 > > > --- a/drivers/net/dsa/realtek/realtek-mdio.c > > > +++ b/drivers/net/dsa/realtek/realtek-mdio.c > > > @@ -168,7 +168,3 @@ void realtek_mdio_shutdown(struct mdio_device *mdiodev) > > > dev_set_drvdata(&mdiodev->dev, NULL); > > > } > > > EXPORT_SYMBOL_GPL(realtek_mdio_shutdown); > > > - > > > -MODULE_AUTHOR("Luiz Angelo Daros de Luca <luizluca@gmail.com>"); > > > -MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via MDIO interface"); > > > -MODULE_LICENSE("GPL"); > > > diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c > > > index 246024eec3bd..1ca2aa784d24 100644 > > > --- a/drivers/net/dsa/realtek/realtek-smi.c > > > +++ b/drivers/net/dsa/realtek/realtek-smi.c > > > @@ -443,7 +443,3 @@ void realtek_smi_shutdown(struct platform_device *pdev) > > > platform_set_drvdata(pdev, NULL); > > > } > > > EXPORT_SYMBOL_GPL(realtek_smi_shutdown); > > > - > > > -MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>"); > > > -MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via SMI interface"); > > > -MODULE_LICENSE("GPL"); > > > -- > > > 2.43.0 > > >
diff --git a/drivers/net/dsa/realtek/Kconfig b/drivers/net/dsa/realtek/Kconfig index 9d182fde11b4..6989972eebc3 100644 --- a/drivers/net/dsa/realtek/Kconfig +++ b/drivers/net/dsa/realtek/Kconfig @@ -16,14 +16,14 @@ menuconfig NET_DSA_REALTEK if NET_DSA_REALTEK config NET_DSA_REALTEK_MDIO - tristate "Realtek MDIO interface support" + bool "Realtek MDIO interface support" depends on OF help Select to enable support for registering switches configured through MDIO. config NET_DSA_REALTEK_SMI - tristate "Realtek SMI interface support" + bool "Realtek SMI interface support" depends on OF help Select to enable support for registering switches connected diff --git a/drivers/net/dsa/realtek/Makefile b/drivers/net/dsa/realtek/Makefile index 5e0c1ef200a3..88f6652f9850 100644 --- a/drivers/net/dsa/realtek/Makefile +++ b/drivers/net/dsa/realtek/Makefile @@ -1,7 +1,9 @@ # SPDX-License-Identifier: GPL-2.0 -obj-$(CONFIG_NET_DSA_REALTEK) += realtek-common.o -obj-$(CONFIG_NET_DSA_REALTEK_MDIO) += realtek-mdio.o -obj-$(CONFIG_NET_DSA_REALTEK_SMI) += realtek-smi.o +obj-$(CONFIG_NET_DSA_REALTEK) += realtek_common.o +realtek_common-objs-y := realtek-common.o +realtek_common-objs-$(CONFIG_NET_DSA_REALTEK_MDIO) += realtek-mdio.o +realtek_common-objs-$(CONFIG_NET_DSA_REALTEK_SMI) += realtek-smi.o +realtek_common-objs := $(realtek_common-objs-y) obj-$(CONFIG_NET_DSA_REALTEK_RTL8366RB) += rtl8366.o rtl8366-objs := rtl8366-core.o rtl8366rb.o obj-$(CONFIG_NET_DSA_REALTEK_RTL8365MB) += rtl8365mb.o diff --git a/drivers/net/dsa/realtek/realtek-common.c b/drivers/net/dsa/realtek/realtek-common.c index 75b6aa071990..73c25d114dd3 100644 --- a/drivers/net/dsa/realtek/realtek-common.c +++ b/drivers/net/dsa/realtek/realtek-common.c @@ -132,5 +132,6 @@ void realtek_common_remove(struct realtek_priv *priv) EXPORT_SYMBOL(realtek_common_remove); MODULE_AUTHOR("Luiz Angelo Daros de Luca <luizluca@gmail.com>"); +MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>"); MODULE_DESCRIPTION("Realtek DSA switches common module"); MODULE_LICENSE("GPL"); diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c index 4c9a744b72f8..bb5bff719ae9 100644 --- a/drivers/net/dsa/realtek/realtek-mdio.c +++ b/drivers/net/dsa/realtek/realtek-mdio.c @@ -168,7 +168,3 @@ void realtek_mdio_shutdown(struct mdio_device *mdiodev) dev_set_drvdata(&mdiodev->dev, NULL); } EXPORT_SYMBOL_GPL(realtek_mdio_shutdown); - -MODULE_AUTHOR("Luiz Angelo Daros de Luca <luizluca@gmail.com>"); -MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via MDIO interface"); -MODULE_LICENSE("GPL"); diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c index 246024eec3bd..1ca2aa784d24 100644 --- a/drivers/net/dsa/realtek/realtek-smi.c +++ b/drivers/net/dsa/realtek/realtek-smi.c @@ -443,7 +443,3 @@ void realtek_smi_shutdown(struct platform_device *pdev) platform_set_drvdata(pdev, NULL); } EXPORT_SYMBOL_GPL(realtek_smi_shutdown); - -MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>"); -MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via SMI interface"); -MODULE_LICENSE("GPL");
As both realtek-common and realtek-{smi,mdio} must always be loaded together, we can save some resources merging them into a single module. Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> --- drivers/net/dsa/realtek/Kconfig | 4 ++-- drivers/net/dsa/realtek/Makefile | 8 +++++--- drivers/net/dsa/realtek/realtek-common.c | 1 + drivers/net/dsa/realtek/realtek-mdio.c | 4 ---- drivers/net/dsa/realtek/realtek-smi.c | 4 ---- 5 files changed, 8 insertions(+), 13 deletions(-)