| 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
| ||
|
Message-ID: <CAJq09z7XH-xSNQdi5JMe9kLcgLqwq75Ea-fEKiNG3GJXu23pqw@mail.gmail.com>
Date: Mon, 11 Dec 2023 02:16:49 -0300
From: Luiz Angelo Daros de Luca <luizluca@...il.com>
To: Alvin Šipraga <ALSI@...g-olufsen.dk>
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 6/7] net: dsa: realtek: migrate user_mii setup to common
> On Fri, Dec 08, 2023 at 01:41:42AM -0300, Luiz Angelo Daros de Luca wrote:
> > Although there are many mentions to SMI in in the user mdio driver,
> > including its compatible string, there is nothing special about the SMI
> > interface in the user mdio bus. That way, the code was migrated to the
> > common module.
> >
> > All references to SMI were removed, except for the compatible string
> > that will still work but warn about using the mdio node name instead.
> >
> > The variant ds_ops_{smi,mdio} fields were rename to, respectively,
> > ds_ops_custom_user_mdio and ds_ops_default_user_mdio.
> >
> > The priv->setup_interface() is also gone. If the ds_ops defines
> > phy_read/write, it means DSA will handle the user_mii_bus. We don't need
> > to check in another place. Also, with the function that would define
> > setup_interface() in common, we can call it directly.
>
> What is default? It's MDIO. What is custom? It's SMI right now. If
> there is another interface (SPI) then we may need a third way. Frankly I
> do not see the benefit of this change, sorry.
Default should actually be "generic", the one created by DSA when
user_mii is not defined.
But I'll solve this by squashing with the next patch.
>
> >
> > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@...il.com>
> > ---
> > drivers/net/dsa/realtek/realtek-common.c | 67 ++++++++++++++++++++++++
> > drivers/net/dsa/realtek/realtek-common.h | 1 +
> > drivers/net/dsa/realtek/realtek-mdio.c | 2 +-
> > drivers/net/dsa/realtek/realtek-smi.c | 61 +--------------------
> > drivers/net/dsa/realtek/realtek.h | 5 +-
> > drivers/net/dsa/realtek/rtl8365mb.c | 12 ++---
> > drivers/net/dsa/realtek/rtl8366rb.c | 12 ++---
> > 7 files changed, 84 insertions(+), 76 deletions(-)
> >
> > diff --git a/drivers/net/dsa/realtek/realtek-common.c b/drivers/net/dsa/realtek/realtek-common.c
> > index 73c25d114dd3..64a55cb1ea05 100644
> > --- a/drivers/net/dsa/realtek/realtek-common.c
> > +++ b/drivers/net/dsa/realtek/realtek-common.c
> > @@ -1,6 +1,7 @@
> > // SPDX-License-Identifier: GPL-2.0+
> >
> > #include <linux/module.h>
> > +#include <linux/of_mdio.h>
> >
> > #include "realtek.h"
> > #include "realtek-common.h"
> > @@ -21,6 +22,72 @@ void realtek_common_unlock(void *ctx)
> > }
> > EXPORT_SYMBOL_GPL(realtek_common_unlock);
> >
> > +static int realtek_common_user_mdio_read(struct mii_bus *bus, int addr,
> > + int regnum)
> > +{
> > + struct realtek_priv *priv = bus->priv;
> > +
> > + return priv->ops->phy_read(priv, addr, regnum);
> > +}
> > +
> > +static int realtek_common_user_mdio_write(struct mii_bus *bus, int addr,
> > + int regnum, u16 val)
> > +{
> > + struct realtek_priv *priv = bus->priv;
> > +
> > + return priv->ops->phy_write(priv, addr, regnum, val);
> > +}
> > +
> > +int realtek_common_setup_user_mdio(struct dsa_switch *ds)
> > +{
> > + struct realtek_priv *priv = ds->priv;
> > + struct device_node *mdio_np;
> > + const char compatible = "realtek,smi-mdio";
>
> const char *compatible
>
> > + int ret;
> > +
> > + mdio_np = of_get_child_by_name(priv->dev->of_node, "mdio");
> > + if (!mdio_np) {
> > + mdio_np = of_get_compatible_child(priv->dev->of_node, compatible);
> > + if (!mdio_np) {
> > + dev_err(priv->dev, "no MDIO bus node\n");
> > + return -ENODEV;
> > + }
> > + dev_warn(priv->dev,
> > + "Rename node '%s' to 'mdio' and remove compatible '%s'",
> > + mdio_np->name, compatible);
> > + }
> > +
> > + priv->user_mii_bus = devm_mdiobus_alloc(priv->dev);
> > + if (!priv->user_mii_bus) {
> > + ret = -ENOMEM;
> > + goto err_put_node;
> > + }
> > + priv->user_mii_bus->priv = priv;
> > + priv->user_mii_bus->name = "Realtek user MII";
> > + priv->user_mii_bus->read = realtek_common_user_mdio_read;
> > + priv->user_mii_bus->write = realtek_common_user_mdio_write;
> > + snprintf(priv->user_mii_bus->id, MII_BUS_ID_SIZE, "Realtek-%d",
> > + ds->index);
> > + priv->user_mii_bus->parent = priv->dev;
> > + ds->user_mii_bus = priv->user_mii_bus;
> > +
> > + ret = devm_of_mdiobus_register(priv->dev, priv->user_mii_bus, mdio_np);
> > + of_node_put(mdio_np);
> > + if (ret) {
> > + dev_err(priv->dev, "unable to register MDIO bus %s\n",
> > + priv->user_mii_bus->id);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +
> > +err_put_node:
> > + of_node_put(mdio_np);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(realtek_common_setup_user_mdio);
> > +
> > struct realtek_priv *
> > realtek_common_probe_pre(struct device *dev, struct regmap_config rc,
> > struct regmap_config rc_nolock)
> > diff --git a/drivers/net/dsa/realtek/realtek-common.h b/drivers/net/dsa/realtek/realtek-common.h
> > index 405bd0d85d2b..4f8c66167b15 100644
> > --- a/drivers/net/dsa/realtek/realtek-common.h
> > +++ b/drivers/net/dsa/realtek/realtek-common.h
> > @@ -7,6 +7,7 @@
> >
> > void realtek_common_lock(void *ctx);
> > void realtek_common_unlock(void *ctx);
> > +int realtek_common_setup_user_mdio(struct dsa_switch *ds);
> > struct realtek_priv *
> > realtek_common_probe_pre(struct device *dev, struct regmap_config rc,
> > struct regmap_config rc_nolock);
> > diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c
> > index bb5bff719ae9..37a41bab20b4 100644
> > --- a/drivers/net/dsa/realtek/realtek-mdio.c
> > +++ b/drivers/net/dsa/realtek/realtek-mdio.c
> > @@ -141,7 +141,7 @@ int realtek_mdio_probe(struct mdio_device *mdiodev)
> > priv->bus = mdiodev->bus;
> > priv->mdio_addr = mdiodev->addr;
> > priv->write_reg_noack = realtek_mdio_write;
> > - priv->ds_ops = priv->variant->ds_ops_mdio;
> > + priv->ds_ops = priv->variant->ds_ops_default_user_mdio;
> >
> > return realtek_common_probe_post(priv);
> >
> > diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
> > index 1ca2aa784d24..84dde2123b09 100644
> > --- a/drivers/net/dsa/realtek/realtek-smi.c
> > +++ b/drivers/net/dsa/realtek/realtek-smi.c
> > @@ -31,7 +31,6 @@
> > #include <linux/spinlock.h>
> > #include <linux/skbuff.h>
> > #include <linux/of.h>
> > -#include <linux/of_mdio.h>
> > #include <linux/delay.h>
> > #include <linux/gpio/consumer.h>
> > #include <linux/platform_device.h>
> > @@ -339,63 +338,6 @@ static const struct regmap_config realtek_smi_nolock_regmap_config = {
> > .disable_locking = true,
> > };
> >
> > -static int realtek_smi_mdio_read(struct mii_bus *bus, int addr, int regnum)
> > -{
> > - struct realtek_priv *priv = bus->priv;
> > -
> > - return priv->ops->phy_read(priv, addr, regnum);
> > -}
> > -
> > -static int realtek_smi_mdio_write(struct mii_bus *bus, int addr, int regnum,
> > - u16 val)
> > -{
> > - struct realtek_priv *priv = bus->priv;
> > -
> > - return priv->ops->phy_write(priv, addr, regnum, val);
> > -}
> > -
> > -static int realtek_smi_setup_mdio(struct dsa_switch *ds)
> > -{
> > - struct realtek_priv *priv = ds->priv;
> > - struct device_node *mdio_np;
> > - int ret;
> > -
> > - mdio_np = of_get_compatible_child(priv->dev->of_node, "realtek,smi-mdio");
> > - if (!mdio_np) {
> > - dev_err(priv->dev, "no MDIO bus node\n");
> > - return -ENODEV;
> > - }
> > -
> > - priv->user_mii_bus = devm_mdiobus_alloc(priv->dev);
> > - if (!priv->user_mii_bus) {
> > - ret = -ENOMEM;
> > - goto err_put_node;
> > - }
> > - priv->user_mii_bus->priv = priv;
> > - priv->user_mii_bus->name = "SMI user MII";
> > - priv->user_mii_bus->read = realtek_smi_mdio_read;
> > - priv->user_mii_bus->write = realtek_smi_mdio_write;
> > - snprintf(priv->user_mii_bus->id, MII_BUS_ID_SIZE, "SMI-%d",
> > - ds->index);
> > - priv->user_mii_bus->parent = priv->dev;
> > - ds->user_mii_bus = priv->user_mii_bus;
> > -
> > - ret = devm_of_mdiobus_register(priv->dev, priv->user_mii_bus, mdio_np);
> > - of_node_put(mdio_np);
> > - if (ret) {
> > - dev_err(priv->dev, "unable to register MDIO bus %s\n",
> > - priv->user_mii_bus->id);
> > - return ret;
> > - }
> > -
> > - return 0;
> > -
> > -err_put_node:
> > - of_node_put(mdio_np);
> > -
> > - return ret;
> > -}
> > -
> > int realtek_smi_probe(struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev;
> > @@ -416,8 +358,7 @@ int realtek_smi_probe(struct platform_device *pdev)
> > return PTR_ERR(priv->mdio);
> >
> > priv->write_reg_noack = realtek_smi_write_reg_noack;
> > - priv->setup_interface = realtek_smi_setup_mdio;
> > - priv->ds_ops = priv->variant->ds_ops_smi;
> > + priv->ds_ops = priv->variant->ds_ops_custom_user_mdio;
> >
> > return realtek_common_probe_post(priv);
> > }
> > diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h
> > index fbd0616c1df3..3fa8479c396f 100644
> > --- a/drivers/net/dsa/realtek/realtek.h
> > +++ b/drivers/net/dsa/realtek/realtek.h
> > @@ -71,7 +71,6 @@ struct realtek_priv {
> > struct rtl8366_mib_counter *mib_counters;
> >
> > const struct realtek_ops *ops;
> > - int (*setup_interface)(struct dsa_switch *ds);
> > int (*write_reg_noack)(void *ctx, u32 addr, u32 data);
> >
> > int vlan_enabled;
> > @@ -115,8 +114,8 @@ struct realtek_ops {
> > };
> >
> > struct realtek_variant {
> > - const struct dsa_switch_ops *ds_ops_smi;
> > - const struct dsa_switch_ops *ds_ops_mdio;
> > + const struct dsa_switch_ops *ds_ops_default_user_mdio;
> > + const struct dsa_switch_ops *ds_ops_custom_user_mdio;
> > const struct realtek_ops *ops;
> > unsigned int clk_delay;
> > u8 cmd_read;
> > diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
> > index ac848b965f84..a52fb07504b5 100644
> > --- a/drivers/net/dsa/realtek/rtl8365mb.c
> > +++ b/drivers/net/dsa/realtek/rtl8365mb.c
> > @@ -2017,8 +2017,8 @@ static int rtl8365mb_setup(struct dsa_switch *ds)
> > if (ret)
> > goto out_teardown_irq;
> >
> > - if (priv->setup_interface) {
> > - ret = priv->setup_interface(ds);
> > + if (!priv->ds_ops->phy_read) {
> > + ret = realtek_common_setup_user_mdio(ds);
> > if (ret) {
> > dev_err(priv->dev, "could not set up MDIO bus\n");
> > goto out_teardown_irq;
> > @@ -2116,7 +2116,7 @@ static int rtl8365mb_detect(struct realtek_priv *priv)
> > return 0;
> > }
> >
> > -static const struct dsa_switch_ops rtl8365mb_switch_ops_smi = {
> > +static const struct dsa_switch_ops rtl8365mb_switch_ops_custom_user_mdio = {
> > .get_tag_protocol = rtl8365mb_get_tag_protocol,
> > .change_tag_protocol = rtl8365mb_change_tag_protocol,
> > .setup = rtl8365mb_setup,
> > @@ -2137,7 +2137,7 @@ static const struct dsa_switch_ops rtl8365mb_switch_ops_smi = {
> > .port_max_mtu = rtl8365mb_port_max_mtu,
> > };
> >
> > -static const struct dsa_switch_ops rtl8365mb_switch_ops_mdio = {
> > +static const struct dsa_switch_ops rtl8365mb_switch_ops_default_user_mdio = {
> > .get_tag_protocol = rtl8365mb_get_tag_protocol,
> > .change_tag_protocol = rtl8365mb_change_tag_protocol,
> > .setup = rtl8365mb_setup,
> > @@ -2167,8 +2167,8 @@ static const struct realtek_ops rtl8365mb_ops = {
> > };
> >
> > const struct realtek_variant rtl8365mb_variant = {
> > - .ds_ops_smi = &rtl8365mb_switch_ops_smi,
> > - .ds_ops_mdio = &rtl8365mb_switch_ops_mdio,
> > + .ds_ops_default_user_mdio = &rtl8365mb_switch_ops_default_user_mdio,
> > + .ds_ops_custom_user_mdio = &rtl8365mb_switch_ops_custom_user_mdio,
> > .ops = &rtl8365mb_ops,
> > .clk_delay = 10,
> > .cmd_read = 0xb9,
> > diff --git a/drivers/net/dsa/realtek/rtl8366rb.c b/drivers/net/dsa/realtek/rtl8366rb.c
> > index 1cc4de3cf54f..9b6997574d2c 100644
> > --- a/drivers/net/dsa/realtek/rtl8366rb.c
> > +++ b/drivers/net/dsa/realtek/rtl8366rb.c
> > @@ -1027,8 +1027,8 @@ static int rtl8366rb_setup(struct dsa_switch *ds)
> > if (ret)
> > dev_info(priv->dev, "no interrupt support\n");
> >
> > - if (priv->setup_interface) {
> > - ret = priv->setup_interface(ds);
> > + if (!priv->ds_ops->phy_read) {
> > + ret = realtek_common_setup_user_mdio(ds);
> > if (ret) {
> > dev_err(priv->dev, "could not set up MDIO bus\n");
> > return -ENODEV;
> > @@ -1848,7 +1848,7 @@ static int rtl8366rb_detect(struct realtek_priv *priv)
> > return 0;
> > }
> >
> > -static const struct dsa_switch_ops rtl8366rb_switch_ops_smi = {
> > +static const struct dsa_switch_ops rtl8366rb_switch_ops_custom_user_mdio = {
> > .get_tag_protocol = rtl8366_get_tag_protocol,
> > .setup = rtl8366rb_setup,
> > .phylink_get_caps = rtl8366rb_phylink_get_caps,
> > @@ -1872,7 +1872,7 @@ static const struct dsa_switch_ops rtl8366rb_switch_ops_smi = {
> > .port_max_mtu = rtl8366rb_max_mtu,
> > };
> >
> > -static const struct dsa_switch_ops rtl8366rb_switch_ops_mdio = {
> > +static const struct dsa_switch_ops rtl8366rb_switch_ops_default_user_mdio = {
> > .get_tag_protocol = rtl8366_get_tag_protocol,
> > .setup = rtl8366rb_setup,
> > .phy_read = rtl8366rb_dsa_phy_read,
> > @@ -1915,8 +1915,8 @@ static const struct realtek_ops rtl8366rb_ops = {
> > };
> >
> > const struct realtek_variant rtl8366rb_variant = {
> > - .ds_ops_smi = &rtl8366rb_switch_ops_smi,
> > - .ds_ops_mdio = &rtl8366rb_switch_ops_mdio,
> > + .ds_ops_default_user_mdio = &rtl8366rb_switch_ops_default_user_mdio,
> > + .ds_ops_custom_user_mdio = &rtl8366rb_switch_ops_custom_user_mdio,
> > .ops = &rtl8366rb_ops,
> > .clk_delay = 10,
> > .cmd_read = 0xa9,
> > --
> > 2.43.0
> >
Regards,
Luiz
Powered by blists - more mailing lists