| 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: <aXIUJbEwnAvIkeKK@smile.fi.intel.com>
Date: Thu, 22 Jan 2026 14:12:21 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
Heiner Kallweit <hkallweit1@...il.com>,
Russell King <linux@...linux.org.uk>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
linux-kernel@...r.kernel.org,
Herve Codina <herve.codina@...tlin.com>,
Mark Brown <broonie@...nel.org>,
Serge Semin <fancer.lancer@...il.com>,
Maxime Chevallier <maxime.chevallier@...tlin.com>,
Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, devicetree@...r.kernel.org,
Choong Yong Liang <yong.liang.choong@...ux.intel.com>,
Jiawen Wu <jiawenwu@...stnetic.com>
Subject: Re: [PATCH v2 net-next 02/15] net: mdio: add driver for NXP SJA1110
100BASE-T1 embedded PHYs
On Thu, Jan 22, 2026 at 12:56:41PM +0200, Vladimir Oltean wrote:
> This driver is the standalone variant of drivers/net/dsa/sja1105/sja1105_mdio.c.
> In terms of differences:
>
> - this one uses regmaps provided by the parent as a method to abstract
> away the sja1105_xfer_u32() calls for register access
> - the driver prefix has been changed from sja1105 to sja1110 (this MDIO
> controller is not present on the older SJA1105 family)
> - in the sja1105 driver, each memory word has 32 bits, so addresses as
> seen by regmap need to be multiplied by 4. This affects what
> sja1110_base_t1_encode_addr() returns, and is different compared to
> sja1105_base_t1_encode_addr().
...
> +static int sja1110_base_t1_mdio_read_c22(struct mii_bus *bus, int phy, int reg)
> +{
> + struct sja1110_base_t1_private *priv = bus->priv;
> + struct regmap *regmap = priv->regmap;
> + unsigned int addr, val;
> + int err;
> +
> + addr = sja1110_base_t1_encode_addr(phy, SJA1110_C22, reg & 0x1f);
GENMASK() ? Or do you have already a defined mask for this?
> + err = regmap_read(regmap, priv->base + addr, &val);
> + if (err)
> + return err;
> +
> + return val & 0xffff;
lower_16_bits() from wordpart.h?
> +}
...
> +static int sja1110_base_t1_mdio_read_c45(struct mii_bus *bus, int phy,
> + int mmd, int reg)
> +{
> + struct sja1110_base_t1_private *priv = bus->priv;
> + struct regmap *regmap = priv->regmap;
> + unsigned int addr, val;
> + int err;
> +
> + addr = sja1110_base_t1_encode_addr(phy, SJA1110_C45_ADDR, mmd);
> + err = regmap_write(regmap, priv->base + addr, reg);
> + if (err)
> + return err;
> +
> + addr = sja1110_base_t1_encode_addr(phy, SJA1110_C45_DATA, mmd);
> + err = regmap_read(regmap, priv->base + addr, &val);
> + if (err)
> + return err;
> +
> + return val & 0xffff;
Ditto.
> +}
...
> +static int sja1110_base_t1_mdio_write_c22(struct mii_bus *bus, int phy, int reg,
> + u16 val)
> +{
> + struct sja1110_base_t1_private *priv = bus->priv;
> + struct regmap *regmap = priv->regmap;
> + unsigned int addr;
> +
> + addr = sja1110_base_t1_encode_addr(phy, SJA1110_C22, reg & 0x1f);
> + return regmap_write(regmap, priv->base + addr, val & 0xffff);
val is already u16.
> +}
...
> +static int sja1110_base_t1_mdio_write_c45(struct mii_bus *bus, int phy,
> + int mmd, int reg, u16 val)
> +{
> + struct sja1110_base_t1_private *priv = bus->priv;
> + struct regmap *regmap = priv->regmap;
> + unsigned int addr;
> + int err;
> +
> + addr = sja1110_base_t1_encode_addr(phy, SJA1110_C45_ADDR, mmd);
> + err = regmap_write(regmap, priv->base + addr, reg);
> + if (err)
> + return err;
> +
> + addr = sja1110_base_t1_encode_addr(phy, SJA1110_C45_DATA, mmd);
> + return regmap_write(regmap, priv->base + addr, val & 0xffff);
Ditto.
> +}
...
> +static int sja1110_base_t1_mdio_probe(struct platform_device *pdev)
> +{
> + struct sja1110_base_t1_private *priv;
> + struct device *dev = &pdev->dev;
> + struct regmap *regmap;
> + struct resource *res;
> + struct mii_bus *bus;
> + int err;
> + if (!dev->of_node || !dev->parent)
Can we avoid dereferencing? And perhaps dev_fwnode(dev)?
> + return -ENODEV;
> +
> + regmap = dev_get_regmap(dev->parent, NULL);
> + if (!regmap)
> + return -ENODEV;
> +
> + bus = mdiobus_alloc_size(sizeof(*priv));
> + if (!bus)
> + return -ENOMEM;
> +
> + bus->name = "SJA1110 100base-T1 MDIO bus";
> + snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
> + bus->read = sja1110_base_t1_mdio_read_c22;
> + bus->write = sja1110_base_t1_mdio_write_c22;
> + bus->read_c45 = sja1110_base_t1_mdio_read_c45;
> + bus->write_c45 = sja1110_base_t1_mdio_write_c45;
> + bus->parent = dev;
> + priv = bus->priv;
> + priv->regmap = regmap;
> +
> + res = platform_get_resource(pdev, IORESOURCE_REG, 0);
> + if (res)
> + priv->base = res->start;
> +
> + err = of_mdiobus_register(bus, dev->of_node);
> + if (err)
> + goto err_free_bus;
> +
> + priv->bus = bus;
> + platform_set_drvdata(pdev, priv);
> +
> + return 0;
> +
> +err_free_bus:
> + mdiobus_free(bus);
> +
> + return err;
> +}
...
> +static const struct of_device_id sja1110_base_t1_mdio_match[] = {
> + { .compatible = "nxp,sja1110-base-t1-mdio", },
Inner comma is redundant.
> + {},
Terminator is terminator, trailing comma is confusing here.
> +};
...
> +static struct platform_driver sja1110_base_t1_mdio_driver = {
> + .probe = sja1110_base_t1_mdio_probe,
> + .remove = sja1110_base_t1_mdio_remove,
> + .driver = {
> + .name = "sja1110-base-t1-mdio",
> + .of_match_table = sja1110_base_t1_mdio_match,
> + },
> +};
> +
Redundant blank line.
> +module_platform_driver(sja1110_base_t1_mdio_driver);
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists