| 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, 4 Sep 2020 14:59:23 +0200
From: Alexandre Belloni <alexandre.belloni@...tlin.com>
To: Helmut Grohne <helmut.grohne@...enta.de>
Cc: Nicolas Ferre <nicolas.ferre@...rochip.com>,
Ludovic Desroches <ludovic.desroches@...rochip.com>,
Woojung Huh <woojung.huh@...rochip.com>,
Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Florian Fainelli <f.fainelli@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org,
netdev@...r.kernel.org
Subject: Re: [PATCH v2] net: dsa: microchip: look for phy-mode in port nodes
On 04/09/2020 10:14:42+0200, Helmut Grohne wrote:
> Documentation/devicetree/bindings/net/dsa/dsa.txt says that the phy-mode
> property should be specified on port nodes. However, the microchip
> drivers read it from the switch node.
>
> Let the driver use the per-port property and fall back to the old
> location with a warning.
>
> Fix in-tree users.
>
> Signed-off-by: Helmut Grohne <helmut.grohne@...enta.de>
Acked-by: Alexandre Belloni <alexandre.belloni@...tlin.com>
> Link: https://lore.kernel.org/netdev/20200617082235.GA1523@laureti-dev/
> ---
> arch/arm/boot/dts/at91-sama5d2_icp.dts | 2 +-
> drivers/net/dsa/microchip/ksz8795.c | 17 +++++++++++-----
> drivers/net/dsa/microchip/ksz9477.c | 28 +++++++++++++++++---------
> drivers/net/dsa/microchip/ksz_common.c | 13 +++++++++++-
> drivers/net/dsa/microchip/ksz_common.h | 3 ++-
> 5 files changed, 45 insertions(+), 18 deletions(-)
>
> Changes since v1:
> * Preserve the reverse christmas tree ordering of local variables.
> Reported by David Miller.
>
> Reason for resending v1:
> * While Andrew Lunn agreed to the semantic change, he found the
> implementation unnecessarily complex. He suggested going without a
> per-port interface attribute, but that happened to not work out. The
> information of which port will become the cpu port is only realized
> in a later initialization step.
>
> There were no further replies, so here goes a v2 with minimal changes.
>
> Helmut
>
> diff --git a/arch/arm/boot/dts/at91-sama5d2_icp.dts b/arch/arm/boot/dts/at91-sama5d2_icp.dts
> index 8d19925fc09e..6783cf16ff81 100644
> --- a/arch/arm/boot/dts/at91-sama5d2_icp.dts
> +++ b/arch/arm/boot/dts/at91-sama5d2_icp.dts
> @@ -116,7 +116,6 @@
> switch0: ksz8563@0 {
> compatible = "microchip,ksz8563";
> reg = <0>;
> - phy-mode = "mii";
> reset-gpios = <&pioA PIN_PD4 GPIO_ACTIVE_LOW>;
>
> spi-max-frequency = <500000>;
> @@ -140,6 +139,7 @@
> reg = <2>;
> label = "cpu";
> ethernet = <&macb0>;
> + phy-mode = "mii";
> fixed-link {
> speed = <100>;
> full-duplex;
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> index 8f1d15ea15d9..cae77eafd533 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -932,11 +932,18 @@ static void ksz8795_port_setup(struct ksz_device *dev, int port, bool cpu_port)
> ksz_port_cfg(dev, port, P_PRIO_CTRL, PORT_802_1P_ENABLE, true);
>
> if (cpu_port) {
> + if (!p->interface && dev->compat_interface) {
> + dev_warn(dev->dev,
> + "Using legacy switch \"phy-mode\" missing on port %d node. Please update your device tree.\n",
> + port);
> + p->interface = dev->compat_interface;
> + }
> +
> /* Configure MII interface for proper network communication. */
> ksz_read8(dev, REG_PORT_5_CTRL_6, &data8);
> data8 &= ~PORT_INTERFACE_TYPE;
> data8 &= ~PORT_GMII_1GPS_MODE;
> - switch (dev->interface) {
> + switch (p->interface) {
> case PHY_INTERFACE_MODE_MII:
> p->phydev.speed = SPEED_100;
> break;
> @@ -952,11 +959,11 @@ static void ksz8795_port_setup(struct ksz_device *dev, int port, bool cpu_port)
> default:
> data8 &= ~PORT_RGMII_ID_IN_ENABLE;
> data8 &= ~PORT_RGMII_ID_OUT_ENABLE;
> - if (dev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - dev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
> + if (p->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + p->interface == PHY_INTERFACE_MODE_RGMII_RXID)
> data8 |= PORT_RGMII_ID_IN_ENABLE;
> - if (dev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - dev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
> + if (p->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + p->interface == PHY_INTERFACE_MODE_RGMII_TXID)
> data8 |= PORT_RGMII_ID_OUT_ENABLE;
> data8 |= PORT_GMII_1GPS_MODE;
> data8 |= PORT_INTERFACE_RGMII;
> diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
> index 3cb22d149813..89e8934bc60b 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -1208,7 +1208,7 @@ static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
>
> /* configure MAC to 1G & RGMII mode */
> ksz_pread8(dev, port, REG_PORT_XMII_CTRL_1, &data8);
> - switch (dev->interface) {
> + switch (p->interface) {
> case PHY_INTERFACE_MODE_MII:
> ksz9477_set_xmii(dev, 0, &data8);
> ksz9477_set_gbit(dev, false, &data8);
> @@ -1229,11 +1229,11 @@ static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
> ksz9477_set_gbit(dev, true, &data8);
> data8 &= ~PORT_RGMII_ID_IG_ENABLE;
> data8 &= ~PORT_RGMII_ID_EG_ENABLE;
> - if (dev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - dev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
> + if (p->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + p->interface == PHY_INTERFACE_MODE_RGMII_RXID)
> data8 |= PORT_RGMII_ID_IG_ENABLE;
> - if (dev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - dev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
> + if (p->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + p->interface == PHY_INTERFACE_MODE_RGMII_TXID)
> data8 |= PORT_RGMII_ID_EG_ENABLE;
> p->phydev.speed = SPEED_1000;
> break;
> @@ -1269,23 +1269,31 @@ static void ksz9477_config_cpu_port(struct dsa_switch *ds)
> dev->cpu_port = i;
> dev->host_mask = (1 << dev->cpu_port);
> dev->port_mask |= dev->host_mask;
> + p = &dev->ports[i];
>
> /* Read from XMII register to determine host port
> * interface. If set specifically in device tree
> * note the difference to help debugging.
> */
> interface = ksz9477_get_interface(dev, i);
> - if (!dev->interface)
> - dev->interface = interface;
> - if (interface && interface != dev->interface)
> + if (!p->interface) {
> + if (dev->compat_interface) {
> + dev_warn(dev->dev,
> + "Using legacy switch \"phy-mode\" missing on port %d node. Please update your device tree.\n",
> + i);
> + p->interface = dev->compat_interface;
> + } else {
> + p->interface = interface;
> + }
> + }
> + if (interface && interface != p->interface)
> dev_info(dev->dev,
> "use %s instead of %s\n",
> - phy_modes(dev->interface),
> + phy_modes(p->interface),
> phy_modes(interface));
>
> /* enable cpu port */
> ksz9477_port_setup(dev, i, true);
> - p = &dev->ports[dev->cpu_port];
> p->vid_member = dev->port_mask;
> p->on = 1;
> }
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 8d53b12d40a8..8e755b50c9c1 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -388,6 +388,8 @@ int ksz_switch_register(struct ksz_device *dev,
> const struct ksz_dev_ops *ops)
> {
> phy_interface_t interface;
> + struct device_node *port;
> + unsigned int port_num;
> int ret;
>
> if (dev->pdata)
> @@ -421,10 +423,19 @@ int ksz_switch_register(struct ksz_device *dev,
> /* Host port interface will be self detected, or specifically set in
> * device tree.
> */
> + for (port_num = 0; port_num < dev->port_cnt; ++port_num)
> + dev->ports[port_num].interface = PHY_INTERFACE_MODE_NA;
> if (dev->dev->of_node) {
> ret = of_get_phy_mode(dev->dev->of_node, &interface);
> if (ret == 0)
> - dev->interface = interface;
> + dev->compat_interface = interface;
> + for_each_available_child_of_node(dev->dev->of_node, port) {
> + if (of_property_read_u32(port, "reg", &port_num))
> + continue;
> + if (port_num >= dev->port_cnt)
> + return -EINVAL;
> + of_get_phy_mode(port, &dev->ports[port_num].interface);
> + }
> dev->synclko_125 = of_property_read_bool(dev->dev->of_node,
> "microchip,synclko-125");
> }
> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index 206838160f49..cf866e48ff66 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -39,6 +39,7 @@ struct ksz_port {
> u32 freeze:1; /* MIB counter freeze is enabled */
>
> struct ksz_port_mib mib;
> + phy_interface_t interface;
> };
>
> struct ksz_device {
> @@ -72,7 +73,7 @@ struct ksz_device {
> int mib_cnt;
> int mib_port_cnt;
> int last_port; /* ports after that not used */
> - phy_interface_t interface;
> + phy_interface_t compat_interface;
> u32 regs_size;
> bool phy_errata_9477;
> bool synclko_125;
> --
> 2.20.1
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Powered by blists - more mailing lists