| 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: Tue, 21 Mar 2023 10:32:06 +0100
From: Álvaro Fernández Rojas <noltari@...il.com>
To: f.fainelli@...il.com, jonas.gorski@...il.com, andrew@...n.ch,
olteanv@...il.com, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: Álvaro Fernández Rojas <noltari@...il.com>
Subject: Re: [RFC PATCH] drivers: net: dsa: b53: mmap: add phy ops
2023-03-20 19:28 GMT+01:00, Álvaro Fernández Rojas <noltari@...il.com>:
> Currently, B53 MMAP BCM63xx devices with an external switch hang when
> performing PHY read and write operations due to invalid registers access.
> This adds support for PHY ops by using the internal bus from
> mdio-mux-bcm6368
> when probed by device tree and also falls back to direct MDIO registers if
> not.
>
> This is an alternative to:
> -
> https://patchwork.kernel.org/project/netdevbpf/cover/20230317113427.302162-1-noltari@gmail.com/
> -
> https://patchwork.kernel.org/project/netdevbpf/patch/20230317113427.302162-2-noltari@gmail.com/
> -
> https://patchwork.kernel.org/project/netdevbpf/patch/20230317113427.302162-3-noltari@gmail.com/
> -
> https://patchwork.kernel.org/project/netdevbpf/patch/20230317113427.302162-4-noltari@gmail.com/
> As discussed, it was an ABI break and not the correct way of fixing the
> issue.
>
> Signed-off-by: Álvaro Fernández Rojas <noltari@...il.com>
> ---
> drivers/net/dsa/b53/b53_mmap.c | 86 +++++++++++++++++++++++++++++++
> include/linux/platform_data/b53.h | 1 +
> 2 files changed, 87 insertions(+)
>
> diff --git a/drivers/net/dsa/b53/b53_mmap.c
> b/drivers/net/dsa/b53/b53_mmap.c
> index 706df04b6cee..7deca1c557c5 100644
> --- a/drivers/net/dsa/b53/b53_mmap.c
> +++ b/drivers/net/dsa/b53/b53_mmap.c
> @@ -19,14 +19,25 @@
> #include <linux/bits.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/of_mdio.h>
> #include <linux/io.h>
> #include <linux/platform_device.h>
> #include <linux/platform_data/b53.h>
>
> #include "b53_priv.h"
>
> +#define REG_MDIOC 0xb0
> +#define REG_MDIOC_EXT_MASK BIT(16)
> +#define REG_MDIOC_REG_SHIFT 20
> +#define REG_MDIOC_PHYID_SHIFT 25
> +#define REG_MDIOC_RD_MASK BIT(30)
> +#define REG_MDIOC_WR_MASK BIT(31)
> +
> +#define REG_MDIOD 0xb4
> +
> struct b53_mmap_priv {
> void __iomem *regs;
> + struct mii_bus *bus;
Can we reuse "bus" from b53_device instead of adding a new one for
b53_mmap_priv?
> };
>
> static int b53_mmap_read8(struct b53_device *dev, u8 page, u8 reg, u8
> *val)
> @@ -216,6 +227,69 @@ static int b53_mmap_write64(struct b53_device *dev, u8
> page, u8 reg,
> return 0;
> }
>
> +static inline void b53_mmap_mdio_read(struct b53_device *dev, int phy_id,
> + int loc, u16 *val)
> +{
> + uint32_t reg;
> +
> + b53_mmap_write32(dev, 0, REG_MDIOC, 0);
> +
> + reg = REG_MDIOC_RD_MASK |
> + (phy_id << REG_MDIOC_PHYID_SHIFT) |
> + (loc << REG_MDIOC_REG_SHIFT);
> +
> + b53_mmap_write32(dev, 0, REG_MDIOC, reg);
> + udelay(50);
> + b53_mmap_read16(dev, 0, REG_MDIOD, val);
> +}
> +
> +static inline int b53_mmap_mdio_write(struct b53_device *dev, int phy_id,
> + int loc, u16 val)
> +{
> + uint32_t reg;
> +
> + b53_mmap_write32(dev, 0, REG_MDIOC, 0);
> +
> + reg = REG_MDIOC_WR_MASK |
> + (phy_id << REG_MDIOC_PHYID_SHIFT) |
> + (loc << REG_MDIOC_REG_SHIFT) |
> + val;
> +
> + b53_mmap_write32(dev, 0, REG_MDIOC, reg);
> + udelay(50);
> +
> + return 0;
> +}
> +
> +static int b53_mmap_phy_read16(struct b53_device *dev, int addr, int reg,
> + u16 *value)
> +{
> + struct b53_mmap_priv *priv = dev->priv;
> + struct mii_bus *bus = priv->bus;
> +
> + if (bus)
> + *value = mdiobus_read_nested(bus, addr, reg);
> + else
> + b53_mmap_mdio_read(dev, addr, reg, value);
> +
> + return 0;
> +}
> +
> +static int b53_mmap_phy_write16(struct b53_device *dev, int addr, int reg,
> + u16 value)
> +{
> + struct b53_mmap_priv *priv = dev->priv;
> + struct mii_bus *bus = priv->bus;
> + int ret;
> +
> + if (bus)
> + ret = mdiobus_write_nested(bus, addr, reg, value);
> + else
> + ret = b53_mmap_mdio_write(dev, addr, reg, value);
> +
> + return ret;
> +}
> +
> static const struct b53_io_ops b53_mmap_ops = {
> .read8 = b53_mmap_read8,
> .read16 = b53_mmap_read16,
> @@ -227,6 +301,8 @@ static const struct b53_io_ops b53_mmap_ops = {
> .write32 = b53_mmap_write32,
> .write48 = b53_mmap_write48,
> .write64 = b53_mmap_write64,
> + .phy_read16 = b53_mmap_phy_read16,
> + .phy_write16 = b53_mmap_phy_write16,
> };
>
> static int b53_mmap_probe_of(struct platform_device *pdev,
> @@ -234,6 +310,7 @@ static int b53_mmap_probe_of(struct platform_device
> *pdev,
> {
> struct device_node *np = pdev->dev.of_node;
> struct device_node *of_ports, *of_port;
> + struct device_node *mdio;
> struct device *dev = &pdev->dev;
> struct b53_platform_data *pdata;
> void __iomem *mem;
> @@ -251,6 +328,14 @@ static int b53_mmap_probe_of(struct platform_device
> *pdev,
> pdata->chip_id = (u32)device_get_match_data(dev);
> pdata->big_endian = of_property_read_bool(np, "big-endian");
>
> + mdio = of_parse_phandle(np, "mii-bus", 0);
Is "mii-bus" OK or shall we use something like "brcm,mii-bus"?
> + if (!mdio)
> + return -EINVAL;
> +
> + pdata->bus = of_mdio_find_bus(mdio);
> + if (!pdata->bus)
> + return -EPROBE_DEFER;
> +
> of_ports = of_get_child_by_name(np, "ports");
> if (!of_ports) {
> dev_err(dev, "no ports child node found\n");
> @@ -297,6 +382,7 @@ static int b53_mmap_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> priv->regs = pdata->regs;
> + priv->bus = pdata->bus;
>
> dev = b53_switch_alloc(&pdev->dev, &b53_mmap_ops, priv);
> if (!dev)
> diff --git a/include/linux/platform_data/b53.h
> b/include/linux/platform_data/b53.h
> index 6f6fed2b171d..be0c5bfdedad 100644
> --- a/include/linux/platform_data/b53.h
> +++ b/include/linux/platform_data/b53.h
> @@ -32,6 +32,7 @@ struct b53_platform_data {
> /* only used by MMAP'd driver */
> unsigned big_endian:1;
> void __iomem *regs;
> + struct mii_bus *bus;
> };
>
> #endif
> --
> 2.30.2
>
>
Powered by blists - more mailing lists