| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 8 Dec 2023 10:23:28 +0000
From: Alvin Šipraga <ALSI@...g-olufsen.dk>
To: Luiz Angelo Daros de Luca <luizluca@...il.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linus.walleij@...aro.org" <linus.walleij@...aro.org>, "andrew@...n.ch"
<andrew@...n.ch>, "f.fainelli@...il.com" <f.fainelli@...il.com>,
"olteanv@...il.com" <olteanv@...il.com>, "davem@...emloft.net"
<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
"arinc.unal@...nc9.com" <arinc.unal@...nc9.com>
Subject: Re: [PATCH net-next 3/7] net: dsa: realtek: convert variants into a
real driver
On Fri, Dec 08, 2023 at 01:41:39AM -0300, Luiz Angelo Daros de Luca wrote:
> Previously, the interface modules realtek-smi and realtek-mdio were,
> respectively, a platform and an MDIO driver. Each interface module
> repeated the same compatible strings for both variants and they
> referenced symbols from variants.
>
> Now each variant module was converted into both a platform and an MDIO
> driver. It reverses the relationship between interface and variant
> module, with the variant now using symbols from interface modules.
>
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@...il.com>
> ---
> drivers/net/dsa/realtek/Kconfig | 20 ++++-------
> drivers/net/dsa/realtek/realtek-mdio.c | 33 ++++--------------
> drivers/net/dsa/realtek/realtek-mdio.h | 48 ++++++++++++++++++++++++++
> drivers/net/dsa/realtek/realtek-smi.c | 38 ++++----------------
> drivers/net/dsa/realtek/realtek-smi.h | 48 ++++++++++++++++++++++++++
> drivers/net/dsa/realtek/rtl8365mb.c | 48 +++++++++++++++++++++++++-
> drivers/net/dsa/realtek/rtl8366rb.c | 48 +++++++++++++++++++++++++-
> 7 files changed, 210 insertions(+), 73 deletions(-)
> create mode 100644 drivers/net/dsa/realtek/realtek-mdio.h
> create mode 100644 drivers/net/dsa/realtek/realtek-smi.h
>
> diff --git a/drivers/net/dsa/realtek/Kconfig b/drivers/net/dsa/realtek/Kconfig
> index 060165a85fb7..9d182fde11b4 100644
> --- a/drivers/net/dsa/realtek/Kconfig
> +++ b/drivers/net/dsa/realtek/Kconfig
> @@ -16,37 +16,29 @@ menuconfig NET_DSA_REALTEK
> if NET_DSA_REALTEK
>
> config NET_DSA_REALTEK_MDIO
> - tristate "Realtek MDIO interface driver"
> + tristate "Realtek MDIO interface support"
> depends on OF
> - depends on NET_DSA_REALTEK_RTL8365MB || NET_DSA_REALTEK_RTL8366RB
> - depends on NET_DSA_REALTEK_RTL8365MB || !NET_DSA_REALTEK_RTL8365MB
> - depends on NET_DSA_REALTEK_RTL8366RB || !NET_DSA_REALTEK_RTL8366RB
> help
> Select to enable support for registering switches configured
> through MDIO.
>
> config NET_DSA_REALTEK_SMI
> - tristate "Realtek SMI interface driver"
> + tristate "Realtek SMI interface support"
> depends on OF
> - depends on NET_DSA_REALTEK_RTL8365MB || NET_DSA_REALTEK_RTL8366RB
> - depends on NET_DSA_REALTEK_RTL8365MB || !NET_DSA_REALTEK_RTL8365MB
> - depends on NET_DSA_REALTEK_RTL8366RB || !NET_DSA_REALTEK_RTL8366RB
> help
> Select to enable support for registering switches connected
> through SMI.
>
> config NET_DSA_REALTEK_RTL8365MB
> - tristate "Realtek RTL8365MB switch subdriver"
> - imply NET_DSA_REALTEK_SMI
> - imply NET_DSA_REALTEK_MDIO
> + tristate "Realtek RTL8365MB switch driver"
> + depends on NET_DSA_REALTEK_SMI || NET_DSA_REALTEK_MDIO
> select NET_DSA_TAG_RTL8_4
> help
> Select to enable support for Realtek RTL8365MB-VC and RTL8367S.
>
> config NET_DSA_REALTEK_RTL8366RB
> - tristate "Realtek RTL8366RB switch subdriver"
> - imply NET_DSA_REALTEK_SMI
> - imply NET_DSA_REALTEK_MDIO
> + tristate "Realtek RTL8366RB switch driver"
> + depends on NET_DSA_REALTEK_SMI || NET_DSA_REALTEK_MDIO
> select NET_DSA_TAG_RTL4_A
> help
> Select to enable support for Realtek RTL8366RB.
> diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c
> index 292e6d087e8b..58966d0625c8 100644
> --- a/drivers/net/dsa/realtek/realtek-mdio.c
> +++ b/drivers/net/dsa/realtek/realtek-mdio.c
> @@ -25,6 +25,7 @@
> #include <linux/regmap.h>
>
> #include "realtek.h"
> +#include "realtek-mdio.h"
>
> /* Read/write via mdiobus */
> #define REALTEK_MDIO_CTRL0_REG 31
> @@ -140,7 +141,7 @@ static const struct regmap_config realtek_mdio_nolock_regmap_config = {
> .disable_locking = true,
> };
>
> -static int realtek_mdio_probe(struct mdio_device *mdiodev)
> +int realtek_mdio_probe(struct mdio_device *mdiodev)
> {
> struct realtek_priv *priv;
> struct device *dev = &mdiodev->dev;
> @@ -235,8 +236,9 @@ static int realtek_mdio_probe(struct mdio_device *mdiodev)
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(realtek_mdio_probe);
>
> -static void realtek_mdio_remove(struct mdio_device *mdiodev)
> +void realtek_mdio_remove(struct mdio_device *mdiodev)
> {
> struct realtek_priv *priv = dev_get_drvdata(&mdiodev->dev);
>
> @@ -249,8 +251,9 @@ static void realtek_mdio_remove(struct mdio_device *mdiodev)
> if (priv->reset)
> gpiod_set_value(priv->reset, 1);
> }
> +EXPORT_SYMBOL_GPL(realtek_mdio_remove);
>
> -static void realtek_mdio_shutdown(struct mdio_device *mdiodev)
> +void realtek_mdio_shutdown(struct mdio_device *mdiodev)
> {
> struct realtek_priv *priv = dev_get_drvdata(&mdiodev->dev);
>
> @@ -261,29 +264,7 @@ static void realtek_mdio_shutdown(struct mdio_device *mdiodev)
>
> dev_set_drvdata(&mdiodev->dev, NULL);
> }
> -
> -static const struct of_device_id realtek_mdio_of_match[] = {
> -#if IS_ENABLED(CONFIG_NET_DSA_REALTEK_RTL8366RB)
> - { .compatible = "realtek,rtl8366rb", .data = &rtl8366rb_variant, },
> -#endif
> -#if IS_ENABLED(CONFIG_NET_DSA_REALTEK_RTL8365MB)
> - { .compatible = "realtek,rtl8365mb", .data = &rtl8365mb_variant, },
> -#endif
> - { /* sentinel */ },
> -};
> -MODULE_DEVICE_TABLE(of, realtek_mdio_of_match);
> -
> -static struct mdio_driver realtek_mdio_driver = {
> - .mdiodrv.driver = {
> - .name = "realtek-mdio",
> - .of_match_table = realtek_mdio_of_match,
> - },
> - .probe = realtek_mdio_probe,
> - .remove = realtek_mdio_remove,
> - .shutdown = realtek_mdio_shutdown,
> -};
> -
> -mdio_module_driver(realtek_mdio_driver);
> +EXPORT_SYMBOL_GPL(realtek_mdio_shutdown);
>
> MODULE_AUTHOR("Luiz Angelo Daros de Luca <luizluca@...il.com>");
> MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via MDIO interface");
> diff --git a/drivers/net/dsa/realtek/realtek-mdio.h b/drivers/net/dsa/realtek/realtek-mdio.h
> new file mode 100644
> index 000000000000..b938a55249c9
> --- /dev/null
> +++ b/drivers/net/dsa/realtek/realtek-mdio.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#ifndef _REALTEK_MDIO_H
> +#define _REALTEK_MDIO_H
> +
> +#if IS_ENABLED(CONFIG_NET_DSA_REALTEK_MDIO)
> +
> +static inline int realtek_mdio_driver_register(struct mdio_driver *drv)
> +{
> + return mdio_driver_register(drv);
> +}
> +
> +static inline void realtek_mdio_driver_unregister(struct mdio_driver *drv)
> +{
> + mdio_driver_unregister(drv);
> +}
> +
> +int realtek_mdio_probe(struct mdio_device *mdiodev);
> +void realtek_mdio_remove(struct mdio_device *mdiodev);
> +void realtek_mdio_shutdown(struct mdio_device *mdiodev);
> +
> +#else
> +
> +static inline int realtek_mdio_driver_register(struct mdio_driver *drv)
> +{
> + return 0;
> +}
> +
> +static inline void realtek_mdio_driver_unregister(struct mdio_driver *drv)
> +{
> +}
> +
> +static inline int realtek_mdio_probe(struct mdio_device *mdiodev)
> +{
> + return -ENOENT;
> +}
> +
> +static inline void realtek_mdio_remove(struct mdio_device *mdiodev)
> +{
> +}
> +
> +static inline void realtek_mdio_shutdown(struct mdio_device *mdiodev)
> +{
> +}
> +
> +#endif
> +
> +#endif
> diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
> index ddcae546afbc..55586d158c0e 100644
> --- a/drivers/net/dsa/realtek/realtek-smi.c
> +++ b/drivers/net/dsa/realtek/realtek-smi.c
> @@ -40,6 +40,7 @@
> #include <linux/if_bridge.h>
>
> #include "realtek.h"
> +#include "realtek-smi.h"
>
> #define REALTEK_SMI_ACK_RETRY_COUNT 5
>
> @@ -408,7 +409,7 @@ static int realtek_smi_setup_mdio(struct dsa_switch *ds)
> return ret;
> }
>
> -static int realtek_smi_probe(struct platform_device *pdev)
> +int realtek_smi_probe(struct platform_device *pdev)
> {
> const struct realtek_variant *var;
> struct device *dev = &pdev->dev;
> @@ -505,8 +506,9 @@ static int realtek_smi_probe(struct platform_device *pdev)
> }
> return 0;
> }
> +EXPORT_SYMBOL_GPL(realtek_smi_probe);
>
> -static void realtek_smi_remove(struct platform_device *pdev)
> +void realtek_smi_remove(struct platform_device *pdev)
> {
> struct realtek_priv *priv = platform_get_drvdata(pdev);
>
> @@ -519,8 +521,9 @@ static void realtek_smi_remove(struct platform_device *pdev)
> if (priv->reset)
> gpiod_set_value(priv->reset, 1);
> }
> +EXPORT_SYMBOL_GPL(realtek_smi_remove);
>
> -static void realtek_smi_shutdown(struct platform_device *pdev)
> +void realtek_smi_shutdown(struct platform_device *pdev)
> {
> struct realtek_priv *priv = platform_get_drvdata(pdev);
>
> @@ -531,34 +534,7 @@ static void realtek_smi_shutdown(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, NULL);
> }
> -
> -static const struct of_device_id realtek_smi_of_match[] = {
> -#if IS_ENABLED(CONFIG_NET_DSA_REALTEK_RTL8366RB)
> - {
> - .compatible = "realtek,rtl8366rb",
> - .data = &rtl8366rb_variant,
> - },
> -#endif
> -#if IS_ENABLED(CONFIG_NET_DSA_REALTEK_RTL8365MB)
> - {
> - .compatible = "realtek,rtl8365mb",
> - .data = &rtl8365mb_variant,
> - },
> -#endif
> - { /* sentinel */ },
> -};
> -MODULE_DEVICE_TABLE(of, realtek_smi_of_match);
> -
> -static struct platform_driver realtek_smi_driver = {
> - .driver = {
> - .name = "realtek-smi",
> - .of_match_table = realtek_smi_of_match,
> - },
> - .probe = realtek_smi_probe,
> - .remove_new = realtek_smi_remove,
> - .shutdown = realtek_smi_shutdown,
> -};
> -module_platform_driver(realtek_smi_driver);
> +EXPORT_SYMBOL_GPL(realtek_smi_shutdown);
>
> MODULE_AUTHOR("Linus Walleij <linus.walleij@...aro.org>");
> MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via SMI interface");
> diff --git a/drivers/net/dsa/realtek/realtek-smi.h b/drivers/net/dsa/realtek/realtek-smi.h
> new file mode 100644
> index 000000000000..76fb58df728b
> --- /dev/null
> +++ b/drivers/net/dsa/realtek/realtek-smi.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#ifndef _REALTEK_SMI_H
> +#define _REALTEK_SMI_H
> +
> +#if IS_ENABLED(CONFIG_NET_DSA_REALTEK_SMI)
> +
> +static inline int realtek_smi_driver_register(struct platform_driver *drv)
> +{
> + return platform_driver_register(drv);
> +}
> +
> +static inline void realtek_smi_driver_unregister(struct platform_driver *drv)
> +{
> + platform_driver_unregister(drv);
> +}
> +
> +int realtek_smi_probe(struct platform_device *pdev);
> +void realtek_smi_remove(struct platform_device *pdev);
> +void realtek_smi_shutdown(struct platform_device *pdev);
> +
> +#else
> +
> +static inline int realtek_smi_driver_register(struct platform_driver *drv)
> +{
> + return 0;
> +}
> +
> +static inline void realtek_smi_driver_unregister(struct platform_driver *drv)
> +{
> +}
> +
> +static inline int realtek_smi_probe(struct platform_device *pdev)
> +{
> + return -ENOENT;
> +}
> +
> +static inline void realtek_smi_remove(struct platform_device *pdev)
> +{
> +}
> +
> +static inline void realtek_smi_shutdown(struct platform_device *pdev)
> +{
> +}
> +
> +#endif
> +
> +#endif
nit: please add /* _REALTEK_SMI_H */ and /* IS_ENABLED(etc.) */ after
the #endifs to make it more readable.
> diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
> index 0875e4fc9f57..526bf98cef1d 100644
> --- a/drivers/net/dsa/realtek/rtl8365mb.c
> +++ b/drivers/net/dsa/realtek/rtl8365mb.c
> @@ -101,6 +101,8 @@
> #include <linux/if_vlan.h>
>
> #include "realtek.h"
> +#include "realtek-smi.h"
> +#include "realtek-mdio.h"
>
> /* Family-specific data and limits */
> #define RTL8365MB_PHYADDRMAX 7
> @@ -2172,7 +2174,51 @@ const struct realtek_variant rtl8365mb_variant = {
> .cmd_write = 0xb8,
> .chip_data_sz = sizeof(struct rtl8365mb),
> };
> -EXPORT_SYMBOL_GPL(rtl8365mb_variant);
> +
> +static const struct of_device_id rtl8365mb_of_match[] = {
> + { .compatible = "realtek,rtl8365mb", .data = &rtl8365mb_variant, },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, rtl8365mb_of_match);
> +
> +static struct platform_driver rtl8365mb_smi_driver = {
> + .driver = {
> + .name = "rtl8365mb-smi",
> + .of_match_table = rtl8365mb_of_match,
> + },
> + .probe = realtek_smi_probe,
> + .remove_new = realtek_smi_remove,
> + .shutdown = realtek_smi_shutdown,
> +};
> +
> +static struct mdio_driver rtl8365mb_mdio_driver = {
> + .mdiodrv.driver = {
> + .name = "rtl8365mb-mdio",
> + .of_match_table = rtl8365mb_of_match,
> + },
> + .probe = realtek_mdio_probe,
> + .remove = realtek_mdio_remove,
> + .shutdown = realtek_mdio_shutdown,
> +};
> +
> +static int rtl8365mb_init(void)
> +{
> + int ret;
> +
> + ret = realtek_mdio_driver_register(&rtl8365mb_mdio_driver);
> + if (ret)
> + return ret;
> +
> + return platform_driver_register(&rtl8365mb_smi_driver);
If this fails I think you should also unwind by unregistering the mdio
driver.
> +}
> +module_init(rtl8365mb_init);
> +
> +static void __exit rtl8365mb_exit(void)
> +{
> + realtek_mdio_driver_unregister(&rtl8365mb_mdio_driver);
> + platform_driver_unregister(&rtl8365mb_smi_driver);
Normally in these unwind functions I like to see the unregistration
performed in an RAII manner, so unregistering the last-registered thing
first. i.e. reverse the order of these lines. Same comments hold for SMI
below.
> +}
> +module_exit(rtl8365mb_exit);
>
> MODULE_AUTHOR("Alvin Šipraga <alsi@...g-olufsen.dk>");
> MODULE_DESCRIPTION("Driver for RTL8365MB-VC ethernet switch");
> diff --git a/drivers/net/dsa/realtek/rtl8366rb.c b/drivers/net/dsa/realtek/rtl8366rb.c
> index b39b719a5b8f..09c17de19457 100644
> --- a/drivers/net/dsa/realtek/rtl8366rb.c
> +++ b/drivers/net/dsa/realtek/rtl8366rb.c
> @@ -22,6 +22,8 @@
> #include <linux/regmap.h>
>
> #include "realtek.h"
> +#include "realtek-smi.h"
> +#include "realtek-mdio.h"
>
> #define RTL8366RB_PORT_NUM_CPU 5
> #define RTL8366RB_NUM_PORTS 6
> @@ -1920,7 +1922,51 @@ const struct realtek_variant rtl8366rb_variant = {
> .cmd_write = 0xa8,
> .chip_data_sz = sizeof(struct rtl8366rb),
> };
> -EXPORT_SYMBOL_GPL(rtl8366rb_variant);
> +
> +static const struct of_device_id rtl8366rb_of_match[] = {
> + { .compatible = "realtek,rtl8366rb", .data = &rtl8366rb_variant, },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, rtl8366rb_of_match);
> +
> +static struct platform_driver rtl8366rb_smi_driver = {
> + .driver = {
> + .name = "rtl8366rb-smi",
> + .of_match_table = rtl8366rb_of_match,
> + },
> + .probe = realtek_smi_probe,
> + .remove_new = realtek_smi_remove,
> + .shutdown = realtek_smi_shutdown,
> +};
> +
> +static struct mdio_driver rtl8366rb_mdio_driver = {
> + .mdiodrv.driver = {
> + .name = "rtl8366rb-mdio",
> + .of_match_table = rtl8366rb_of_match,
> + },
> + .probe = realtek_mdio_probe,
> + .remove = realtek_mdio_remove,
> + .shutdown = realtek_mdio_shutdown,
> +};
> +
> +static int rtl8366rb_init(void)
> +{
> + int ret;
> +
> + ret = realtek_mdio_driver_register(&rtl8366rb_mdio_driver);
> + if (ret)
> + return ret;
> +
> + return realtek_smi_driver_register(&rtl8366rb_smi_driver);
> +}
> +module_init(rtl8366rb_init);
> +
> +static void __exit rtl8366rb_exit(void)
> +{
> + realtek_mdio_driver_unregister(&rtl8366rb_mdio_driver);
> + realtek_smi_driver_unregister(&rtl8366rb_smi_driver);
> +}
> +module_exit(rtl8366rb_exit);
>
> MODULE_AUTHOR("Linus Walleij <linus.walleij@...aro.org>");
> MODULE_DESCRIPTION("Driver for RTL8366RB ethernet switch");
> --
> 2.43.0
>
Powered by blists - more mailing lists