diff mbox

[OpenWrt-Devel,v2] lantiq, xrx200-net: add devicetree aliases support for port->mac mapping

Message ID 1450084729-3816-1-git-send-email-mschiller@tdt.de
State Changes Requested
Headers show

Commit Message

Martin Schiller Dec. 14, 2015, 9:18 a.m. UTC
This patch adds devicetree aliases support, which makes it possible to
renumber the physical mac ports.

This change has only cosmetical reasons (e.g. appearance in swconfig,
kernel messages on link change).

Signed-off-by: Martin Schiller <mschiller@tdt.de>
---
Changes in v2:
- use devicetree aliases instead of 'id' property
- only map ports, which are "configured" in the devicetree

 .../0025-NET-MIPS-lantiq-adds-xrx200-net.patch     | 46 ++++++++++++++++------
 1 file changed, 33 insertions(+), 13 deletions(-)

Comments

John Crispin Dec. 17, 2015, 8:20 a.m. UTC | #1
On 14/12/2015 10:18, Martin Schiller wrote:
> This patch adds devicetree aliases support, which makes it possible to
> renumber the physical mac ports.
> 
> This change has only cosmetical reasons (e.g. appearance in swconfig,
> kernel messages on link change).
> 
> Signed-off-by: Martin Schiller <mschiller@tdt.de>

i have a problem with this patch. port 5 is port 5 *always*. i dont see
how this fixes an issue. did you HW guys build broken HW with badly
ordered mac ports ?




> ---
> Changes in v2:
> - use devicetree aliases instead of 'id' property
> - only map ports, which are "configured" in the devicetree
> 
>  .../0025-NET-MIPS-lantiq-adds-xrx200-net.patch     | 46 ++++++++++++++++------
>  1 file changed, 33 insertions(+), 13 deletions(-)
> 
> diff --git a/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-adds-xrx200-net.patch b/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-adds-xrx200-net.patch
> index 48c7395..c76c165 100644
> --- a/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-adds-xrx200-net.patch
> +++ b/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-adds-xrx200-net.patch
> @@ -209,7 +209,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
>  +};
>  --- /dev/null
>  +++ b/drivers/net/ethernet/lantiq_xrx200.c
> -@@ -0,0 +1,1798 @@
> +@@ -0,0 +1,1818 @@
>  +/*
>  + *   This program is free software; you can redistribute it and/or modify it
>  + *   under the terms of the GNU General Public License version 2 as published
> @@ -404,6 +404,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
>  +
>  +struct xrx200_port {
>  +	u8 num;
> ++	u8 id;
>  +	u8 phy_addr;
>  +	u16 flags;
>  +	phy_interface_t phy_if;
> @@ -461,6 +462,9 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
>  +	struct xrx200_hw *hw;
>  +};
>  +
> ++static int id_to_mac[XRX200_MAX_PORT] = {-1,-1,-1,-1,-1,-1,6};
> ++static int mac_to_id[XRX200_MAX_PORT] = {-1,-1,-1,-1,-1,-1,6};
> ++
>  +static __iomem void *xrx200_switch_membase;
>  +static __iomem void *xrx200_mii_membase;
>  +static __iomem void *xrx200_mdio_membase;
> @@ -789,9 +793,13 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
>  +	{
>  +		struct switch_port *p = &val->value.ports[i];
>  +
> -+		portmap |= (1 << p->id);
> ++		if (id_to_mac[p->id] < 0) {
> ++			continue;
> ++		}
> ++
> ++		portmap |= (1 << id_to_mac[p->id]);
>  +		if (p->flags & (1 << SWITCH_PORT_FLAG_TAGGED))
> -+			tagmap |= (1 << p->id);
> ++			tagmap |= (1 << id_to_mac[p->id]);
>  +	}
>  +
>  +	tem.table = XRX200_PCE_VLANMAP_IDX;
> @@ -855,7 +863,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
>  +			continue;
>  +
>  +		p = &val->value.ports[val->len++];
> -+		p->id = i;
> ++		p->id = mac_to_id[i];
>  +		if (tags & (1 << i))
>  +			p->flags = (1 << SWITCH_PORT_FLAG_TAGGED);
>  +		else
> @@ -898,11 +906,12 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
>  +}
>  +
>  +// port
> -+static int xrx200sw_get_port_pvid(struct switch_dev *dev, int port, int *val)
> ++static int xrx200sw_get_port_pvid(struct switch_dev *dev, int port_id, int *val)
>  +{
> ++	int port = id_to_mac[port_id];
>  +	struct xrx200_pce_table_entry tev;
>  +
> -+	if (port >= XRX200_MAX_PORT)
> ++	if (port < 0 || port >= XRX200_MAX_PORT)
>  +		return -EINVAL;
>  +
>  +	tev.table = XRX200_PCE_ACTVLAN_IDX;
> @@ -914,10 +923,12 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
>  +}
>  +
>  +static int xrx200sw_get_port_link(struct switch_dev *dev,
> -+				  int port,
> ++				  int port_id,
>  +				  struct switch_port_link *link)
>  +{
> -+	if (port >= XRX200_MAX_PORT)
> ++	int port = id_to_mac[port_id];
> ++
> ++	if (port < 0 || port >= XRX200_MAX_PORT)
>  +		return -EINVAL;
>  +
>  +	link->link = xrx200sw_read_x(XRX200_MAC_PSTAT_LSTAT, port);
> @@ -1432,7 +1443,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
>  +			xrx200_gmac_update(&priv->port[i]);
>  +			priv->port[i].link = priv->port[i].phydev->link;
>  +			netdev_info(dev, "port %d %s link\n",
> -+				priv->port[i].num,
> ++				priv->port[i].id,
>  +				(priv->port[i].link)?("got"):("lost"));
>  +		}
>  +	}
> @@ -1633,7 +1644,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
>  +	for (i = 0; i < priv->num_port; i++)
>  +		if (xrx200_mdio_probe(dev, &priv->port[i]))
>  +			pr_warn("xrx200-mdio: probing phy of port %d failed\n",
> -+					 priv->port[i].num);
> ++					 priv->port[i].id);
>  +
>  +	return 0;
>  +
> @@ -1787,19 +1798,24 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
>  +
>  +static void xrx200_of_port(struct xrx200_priv *priv, struct device_node *port)
>  +{
> -+	const __be32 *addr, *id = of_get_property(port, "reg", NULL);
> ++	const __be32 *addr, *mac = of_get_property(port, "reg", NULL);
>  +	struct xrx200_port *p = &priv->port[priv->num_port];
> ++	int id = of_alias_get_id(port, "ethernet");
>  +
> -+	if (!id)
> ++	if (!mac)
>  +		return;
>  +
> ++	if (id < 0)
> ++		id = *mac;
> ++
>  +	memset(p, 0, sizeof(struct xrx200_port));
>  +	p->phy_node = of_parse_phandle(port, "phy-handle", 0);
>  +	addr = of_get_property(p->phy_node, "reg", NULL);
>  +	if (!addr)
>  +		return;
>  +
> -+	p->num = *id;
> ++	p->num = *mac;
> ++	p->id = id;
>  +	p->phy_addr = *addr;
>  +	p->phy_if = of_get_phy_mode(port);
>  +	if (p->phy_addr > 0x10)
> @@ -1824,6 +1840,10 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
>  +
>  +	/* store the port id in the hw struct so we can map ports -> devices */
>  +	priv->hw->port_map[p->num] = priv->hw->num_devs;
> ++
> ++	/* store the id <-> mac (port) mapping */
> ++	id_to_mac[p->id] = p->num;
> ++	mac_to_id[p->num] = p->id;
>  +}
>  +
>  +static const struct net_device_ops xrx200_netdev_ops = {
>
Martin Schiller Dec. 17, 2015, 11:03 a.m. UTC | #2
On 12/17/2015 at 9:20 AM, John Crispin wrote:
>
>
> On 14/12/2015 10:18, Martin Schiller wrote:
> > This patch adds devicetree aliases support, which makes it possible
> to
> > renumber the physical mac ports.
> >
> > This change has only cosmetical reasons (e.g. appearance in swconfig,
> > kernel messages on link change).
> >
> > Signed-off-by: Martin Schiller <mschiller@tdt.de>
>
> i have a problem with this patch. port 5 is port 5 *always*. i dont see
> how this fixes an issue. did you HW guys build broken HW with badly
> ordered mac ports ?

Yes, you are right. This patch is intended to reorder broken/unlovely
placed mac ports.

The user should see ports in the order 1,2,3,4,5 instead of 1,4,5,2,3.

Our HW guys told us it would be much more work to solve this in hardware
by doing a lot of "wire crossings" (with unwanted hf side-effects) in the
pcb layout.

And there are also situations, where you even can't do it in hardware:
when you use the xrx200 in Gigabit mode, you can use mac 0,1,2,4,5.
mac/port 3 is dead in this case.

But you can't tell an enduser/customer that there is a port 2 and a port 4
but no port 3.


>
>
>
> > ---
> > Changes in v2:
> > - use devicetree aliases instead of 'id' property
> > - only map ports, which are "configured" in the devicetree
> >
> >  .../0025-NET-MIPS-lantiq-adds-xrx200-net.patch     | 46
> ++++++++++++++++------
> >  1 file changed, 33 insertions(+), 13 deletions(-)
> >
> > diff --git a/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-
> adds-xrx200-net.patch b/target/linux/lantiq/patches-4.1/0025-NET-MIPS-
> lantiq-adds-xrx200-net.patch
> > index 48c7395..c76c165 100644
> > --- a/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-adds-
> xrx200-net.patch
> > +++ b/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-adds-
> xrx200-net.patch
> > @@ -209,7 +209,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> xrx200-net
> >  +};
> >  --- /dev/null
> >  +++ b/drivers/net/ethernet/lantiq_xrx200.c
> > -@@ -0,0 +1,1798 @@
> > +@@ -0,0 +1,1818 @@
> >  +/*
> >  + *   This program is free software; you can redistribute it and/or
> modify it
> >  + *   under the terms of the GNU General Public License version 2 as
> published
> > @@ -404,6 +404,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> xrx200-net
> >  +
> >  +struct xrx200_port {
> >  +u8 num;
> > ++u8 id;
> >  +u8 phy_addr;
> >  +u16 flags;
> >  +phy_interface_t phy_if;
> > @@ -461,6 +462,9 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> xrx200-net
> >  +struct xrx200_hw *hw;
> >  +};
> >  +
> > ++static int id_to_mac[XRX200_MAX_PORT] = {-1,-1,-1,-1,-1,-1,6};
> > ++static int mac_to_id[XRX200_MAX_PORT] = {-1,-1,-1,-1,-1,-1,6};
> > ++
> >  +static __iomem void *xrx200_switch_membase;
> >  +static __iomem void *xrx200_mii_membase;
> >  +static __iomem void *xrx200_mdio_membase;
> > @@ -789,9 +793,13 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> xrx200-net
> >  +{
> >  +struct switch_port *p = &val->value.ports[i];
> >  +
> > -+portmap |= (1 << p->id);
> > ++if (id_to_mac[p->id] < 0) {
> > ++continue;
> > ++}
> > ++
> > ++portmap |= (1 << id_to_mac[p->id]);
> >  +if (p->flags & (1 << SWITCH_PORT_FLAG_TAGGED))
> > -+tagmap |= (1 << p->id);
> > ++tagmap |= (1 << id_to_mac[p->id]);
> >  +}
> >  +
> >  +tem.table = XRX200_PCE_VLANMAP_IDX;
> > @@ -855,7 +863,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> xrx200-net
> >  +continue;
> >  +
> >  +p = &val->value.ports[val->len++];
> > -+p->id = i;
> > ++p->id = mac_to_id[i];
> >  +if (tags & (1 << i))
> >  +p->flags = (1 << SWITCH_PORT_FLAG_TAGGED);
> >  +else
> > @@ -898,11 +906,12 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> xrx200-net
> >  +}
> >  +
> >  +// port
> > -+static int xrx200sw_get_port_pvid(struct switch_dev *dev, int port,
> int *val)
> > ++static int xrx200sw_get_port_pvid(struct switch_dev *dev, int
> port_id, int *val)
> >  +{
> > ++int port = id_to_mac[port_id];
> >  +struct xrx200_pce_table_entry tev;
> >  +
> > -+if (port >= XRX200_MAX_PORT)
> > ++if (port < 0 || port >= XRX200_MAX_PORT)
> >  +return -EINVAL;
> >  +
> >  +tev.table = XRX200_PCE_ACTVLAN_IDX;
> > @@ -914,10 +923,12 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> xrx200-net
> >  +}
> >  +
> >  +static int xrx200sw_get_port_link(struct switch_dev *dev,
> > -+  int port,
> > ++  int port_id,
> >  +  struct switch_port_link *link)
> >  +{
> > -+if (port >= XRX200_MAX_PORT)
> > ++int port = id_to_mac[port_id];
> > ++
> > ++if (port < 0 || port >= XRX200_MAX_PORT)
> >  +return -EINVAL;
> >  +
> >  +link->link = xrx200sw_read_x(XRX200_MAC_PSTAT_LSTAT, port);
> > @@ -1432,7 +1443,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> xrx200-net
> >  +xrx200_gmac_update(&priv->port[i]);
> >  +priv->port[i].link = priv->port[i].phydev->link;
> >  +netdev_info(dev, "port %d %s link\n",
> > -+priv->port[i].num,
> > ++priv->port[i].id,
> >  +(priv->port[i].link)?("got"):("lost"));
> >  +}
> >  +}
> > @@ -1633,7 +1644,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> xrx200-net
> >  +for (i = 0; i < priv->num_port; i++)
> >  +if (xrx200_mdio_probe(dev, &priv->port[i]))
> >  +pr_warn("xrx200-mdio: probing phy of port %d
> failed\n",
> > -+ priv->port[i].num);
> > ++ priv->port[i].id);
> >  +
> >  +return 0;
> >  +
> > @@ -1787,19 +1798,24 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq:
> adds xrx200-net
> >  +
> >  +static void xrx200_of_port(struct xrx200_priv *priv, struct
> device_node *port)
> >  +{
> > -+const __be32 *addr, *id = of_get_property(port, "reg", NULL);
> > ++const __be32 *addr, *mac = of_get_property(port, "reg", NULL);
> >  +struct xrx200_port *p = &priv->port[priv->num_port];
> > ++int id = of_alias_get_id(port, "ethernet");
> >  +
> > -+if (!id)
> > ++if (!mac)
> >  +return;
> >  +
> > ++if (id < 0)
> > ++id = *mac;
> > ++
> >  +memset(p, 0, sizeof(struct xrx200_port));
> >  +p->phy_node = of_parse_phandle(port, "phy-handle", 0);
> >  +addr = of_get_property(p->phy_node, "reg", NULL);
> >  +if (!addr)
> >  +return;
> >  +
> > -+p->num = *id;
> > ++p->num = *mac;
> > ++p->id = id;
> >  +p->phy_addr = *addr;
> >  +p->phy_if = of_get_phy_mode(port);
> >  +if (p->phy_addr > 0x10)
> > @@ -1824,6 +1840,10 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> xrx200-net
> >  +
> >  +/* store the port id in the hw struct so we can map ports ->
> devices */
> >  +priv->hw->port_map[p->num] = priv->hw->num_devs;
> > ++
> > ++/* store the id <-> mac (port) mapping */
> > ++id_to_mac[p->id] = p->num;
> > ++mac_to_id[p->num] = p->id;
> >  +}
> >  +
> >  +static const struct net_device_ops xrx200_netdev_ops = {
> >
John Crispin Dec. 17, 2015, 11:13 a.m. UTC | #3
On 17/12/2015 12:03, Martin Schiller wrote:
> On 12/17/2015 at 9:20 AM, John Crispin wrote:
>>
>>
>> On 14/12/2015 10:18, Martin Schiller wrote:
>>> This patch adds devicetree aliases support, which makes it possible
>> to
>>> renumber the physical mac ports.
>>>
>>> This change has only cosmetical reasons (e.g. appearance in swconfig,
>>> kernel messages on link change).
>>>
>>> Signed-off-by: Martin Schiller <mschiller@tdt.de>
>>
>> i have a problem with this patch. port 5 is port 5 *always*. i dont see
>> how this fixes an issue. did you HW guys build broken HW with badly
>> ordered mac ports ?
> 
> Yes, you are right. This patch is intended to reorder broken/unlovely
> placed mac ports.
> 
> The user should see ports in the order 1,2,3,4,5 instead of 1,4,5,2,3.
> 

ah of coruse the ports are ordered in a wonky way on the SoC

> Our HW guys told us it would be much more work to solve this in hardware
> by doing a lot of "wire crossings" (with unwanted hf side-effects) in the
> pcb layout.
> 

correct

> And there are also situations, where you even can't do it in hardware:
> when you use the xrx200 in Gigabit mode, you can use mac 0,1,2,4,5.
> mac/port 3 is dead in this case.
> 
> But you can't tell an enduser/customer that there is a port 2 and a port 4
> but no port 3.

sure but solving this in the driver by using a virtual port abstraction
is imho not good.

i'll see if i can think of a better solution for this issue





> 
>>
>>
>>
>>> ---
>>> Changes in v2:
>>> - use devicetree aliases instead of 'id' property
>>> - only map ports, which are "configured" in the devicetree
>>>
>>>  .../0025-NET-MIPS-lantiq-adds-xrx200-net.patch     | 46
>> ++++++++++++++++------
>>>  1 file changed, 33 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-
>> adds-xrx200-net.patch b/target/linux/lantiq/patches-4.1/0025-NET-MIPS-
>> lantiq-adds-xrx200-net.patch
>>> index 48c7395..c76c165 100644
>>> --- a/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-adds-
>> xrx200-net.patch
>>> +++ b/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-adds-
>> xrx200-net.patch
>>> @@ -209,7 +209,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
>> xrx200-net
>>>  +};
>>>  --- /dev/null
>>>  +++ b/drivers/net/ethernet/lantiq_xrx200.c
>>> -@@ -0,0 +1,1798 @@
>>> +@@ -0,0 +1,1818 @@
>>>  +/*
>>>  + *   This program is free software; you can redistribute it and/or
>> modify it
>>>  + *   under the terms of the GNU General Public License version 2 as
>> published
>>> @@ -404,6 +404,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
>> xrx200-net
>>>  +
>>>  +struct xrx200_port {
>>>  +u8 num;
>>> ++u8 id;
>>>  +u8 phy_addr;
>>>  +u16 flags;
>>>  +phy_interface_t phy_if;
>>> @@ -461,6 +462,9 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
>> xrx200-net
>>>  +struct xrx200_hw *hw;
>>>  +};
>>>  +
>>> ++static int id_to_mac[XRX200_MAX_PORT] = {-1,-1,-1,-1,-1,-1,6};
>>> ++static int mac_to_id[XRX200_MAX_PORT] = {-1,-1,-1,-1,-1,-1,6};
>>> ++
>>>  +static __iomem void *xrx200_switch_membase;
>>>  +static __iomem void *xrx200_mii_membase;
>>>  +static __iomem void *xrx200_mdio_membase;
>>> @@ -789,9 +793,13 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
>> xrx200-net
>>>  +{
>>>  +struct switch_port *p = &val->value.ports[i];
>>>  +
>>> -+portmap |= (1 << p->id);
>>> ++if (id_to_mac[p->id] < 0) {
>>> ++continue;
>>> ++}
>>> ++
>>> ++portmap |= (1 << id_to_mac[p->id]);
>>>  +if (p->flags & (1 << SWITCH_PORT_FLAG_TAGGED))
>>> -+tagmap |= (1 << p->id);
>>> ++tagmap |= (1 << id_to_mac[p->id]);
>>>  +}
>>>  +
>>>  +tem.table = XRX200_PCE_VLANMAP_IDX;
>>> @@ -855,7 +863,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
>> xrx200-net
>>>  +continue;
>>>  +
>>>  +p = &val->value.ports[val->len++];
>>> -+p->id = i;
>>> ++p->id = mac_to_id[i];
>>>  +if (tags & (1 << i))
>>>  +p->flags = (1 << SWITCH_PORT_FLAG_TAGGED);
>>>  +else
>>> @@ -898,11 +906,12 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
>> xrx200-net
>>>  +}
>>>  +
>>>  +// port
>>> -+static int xrx200sw_get_port_pvid(struct switch_dev *dev, int port,
>> int *val)
>>> ++static int xrx200sw_get_port_pvid(struct switch_dev *dev, int
>> port_id, int *val)
>>>  +{
>>> ++int port = id_to_mac[port_id];
>>>  +struct xrx200_pce_table_entry tev;
>>>  +
>>> -+if (port >= XRX200_MAX_PORT)
>>> ++if (port < 0 || port >= XRX200_MAX_PORT)
>>>  +return -EINVAL;
>>>  +
>>>  +tev.table = XRX200_PCE_ACTVLAN_IDX;
>>> @@ -914,10 +923,12 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
>> xrx200-net
>>>  +}
>>>  +
>>>  +static int xrx200sw_get_port_link(struct switch_dev *dev,
>>> -+  int port,
>>> ++  int port_id,
>>>  +  struct switch_port_link *link)
>>>  +{
>>> -+if (port >= XRX200_MAX_PORT)
>>> ++int port = id_to_mac[port_id];
>>> ++
>>> ++if (port < 0 || port >= XRX200_MAX_PORT)
>>>  +return -EINVAL;
>>>  +
>>>  +link->link = xrx200sw_read_x(XRX200_MAC_PSTAT_LSTAT, port);
>>> @@ -1432,7 +1443,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
>> xrx200-net
>>>  +xrx200_gmac_update(&priv->port[i]);
>>>  +priv->port[i].link = priv->port[i].phydev->link;
>>>  +netdev_info(dev, "port %d %s link\n",
>>> -+priv->port[i].num,
>>> ++priv->port[i].id,
>>>  +(priv->port[i].link)?("got"):("lost"));
>>>  +}
>>>  +}
>>> @@ -1633,7 +1644,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
>> xrx200-net
>>>  +for (i = 0; i < priv->num_port; i++)
>>>  +if (xrx200_mdio_probe(dev, &priv->port[i]))
>>>  +pr_warn("xrx200-mdio: probing phy of port %d
>> failed\n",
>>> -+ priv->port[i].num);
>>> ++ priv->port[i].id);
>>>  +
>>>  +return 0;
>>>  +
>>> @@ -1787,19 +1798,24 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq:
>> adds xrx200-net
>>>  +
>>>  +static void xrx200_of_port(struct xrx200_priv *priv, struct
>> device_node *port)
>>>  +{
>>> -+const __be32 *addr, *id = of_get_property(port, "reg", NULL);
>>> ++const __be32 *addr, *mac = of_get_property(port, "reg", NULL);
>>>  +struct xrx200_port *p = &priv->port[priv->num_port];
>>> ++int id = of_alias_get_id(port, "ethernet");
>>>  +
>>> -+if (!id)
>>> ++if (!mac)
>>>  +return;
>>>  +
>>> ++if (id < 0)
>>> ++id = *mac;
>>> ++
>>>  +memset(p, 0, sizeof(struct xrx200_port));
>>>  +p->phy_node = of_parse_phandle(port, "phy-handle", 0);
>>>  +addr = of_get_property(p->phy_node, "reg", NULL);
>>>  +if (!addr)
>>>  +return;
>>>  +
>>> -+p->num = *id;
>>> ++p->num = *mac;
>>> ++p->id = id;
>>>  +p->phy_addr = *addr;
>>>  +p->phy_if = of_get_phy_mode(port);
>>>  +if (p->phy_addr > 0x10)
>>> @@ -1824,6 +1840,10 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
>> xrx200-net
>>>  +
>>>  +/* store the port id in the hw struct so we can map ports ->
>> devices */
>>>  +priv->hw->port_map[p->num] = priv->hw->num_devs;
>>> ++
>>> ++/* store the id <-> mac (port) mapping */
>>> ++id_to_mac[p->id] = p->num;
>>> ++mac_to_id[p->num] = p->id;
>>>  +}
>>>  +
>>>  +static const struct net_device_ops xrx200_netdev_ops = {
>>>
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
>
Martin Schiller Dec. 22, 2015, 9:36 a.m. UTC | #4
On 12/17/2015 at 12:13 PM, John Crispin wrote:
>
>
> On 17/12/2015 12:03, Martin Schiller wrote:
> > On 12/17/2015 at 9:20 AM, John Crispin wrote:
> >>
> >>
> >> On 14/12/2015 10:18, Martin Schiller wrote:
> >>> This patch adds devicetree aliases support, which makes it possible
> >> to
> >>> renumber the physical mac ports.
> >>>
> >>> This change has only cosmetical reasons (e.g. appearance in
> swconfig,
> >>> kernel messages on link change).
> >>>
> >>> Signed-off-by: Martin Schiller <mschiller@tdt.de>
> >>
> >> i have a problem with this patch. port 5 is port 5 *always*. i dont
> see
> >> how this fixes an issue. did you HW guys build broken HW with badly
> >> ordered mac ports ?
> >
> > Yes, you are right. This patch is intended to reorder broken/unlovely
> > placed mac ports.
> >
> > The user should see ports in the order 1,2,3,4,5 instead of
> 1,4,5,2,3.
> >
>
> ah of coruse the ports are ordered in a wonky way on the SoC
>
> > Our HW guys told us it would be much more work to solve this in
> hardware
> > by doing a lot of "wire crossings" (with unwanted hf side-effects) in
> the
> > pcb layout.
> >
>
> correct
>
> > And there are also situations, where you even can't do it in
> hardware:
> > when you use the xrx200 in Gigabit mode, you can use mac 0,1,2,4,5.
> > mac/port 3 is dead in this case.
> >
> > But you can't tell an enduser/customer that there is a port 2 and a
> port 4
> > but no port 3.
>
> sure but solving this in the driver by using a virtual port abstraction
> is imho not good.
>
> i'll see if i can think of a better solution for this issue

As this is a board specific issue, I thought solving this in the
devicetree would be a nice way.

But any better solution would be welcome.

>
>
>
>
>
> >
> >>
> >>
> >>
> >>> ---
> >>> Changes in v2:
> >>> - use devicetree aliases instead of 'id' property
> >>> - only map ports, which are "configured" in the devicetree
> >>>
> >>>  .../0025-NET-MIPS-lantiq-adds-xrx200-net.patch     | 46
> >> ++++++++++++++++------
> >>>  1 file changed, 33 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-
> >> adds-xrx200-net.patch b/target/linux/lantiq/patches-4.1/0025-NET-
> MIPS-
> >> lantiq-adds-xrx200-net.patch
> >>> index 48c7395..c76c165 100644
> >>> --- a/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-adds-
> >> xrx200-net.patch
> >>> +++ b/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-adds-
> >> xrx200-net.patch
> >>> @@ -209,7 +209,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> >> xrx200-net
> >>>  +};
> >>>  --- /dev/null
> >>>  +++ b/drivers/net/ethernet/lantiq_xrx200.c
> >>> -@@ -0,0 +1,1798 @@
> >>> +@@ -0,0 +1,1818 @@
> >>>  +/*
> >>>  + *   This program is free software; you can redistribute it
> and/or
> >> modify it
> >>>  + *   under the terms of the GNU General Public License version 2
> as
> >> published
> >>> @@ -404,6 +404,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> >> xrx200-net
> >>>  +
> >>>  +struct xrx200_port {
> >>>  +u8 num;
> >>> ++u8 id;
> >>>  +u8 phy_addr;
> >>>  +u16 flags;
> >>>  +phy_interface_t phy_if;
> >>> @@ -461,6 +462,9 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> >> xrx200-net
> >>>  +struct xrx200_hw *hw;
> >>>  +};
> >>>  +
> >>> ++static int id_to_mac[XRX200_MAX_PORT] = {-1,-1,-1,-1,-1,-1,6};
> >>> ++static int mac_to_id[XRX200_MAX_PORT] = {-1,-1,-1,-1,-1,-1,6};
> >>> ++
> >>>  +static __iomem void *xrx200_switch_membase;
> >>>  +static __iomem void *xrx200_mii_membase;
> >>>  +static __iomem void *xrx200_mdio_membase;
> >>> @@ -789,9 +793,13 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> >> xrx200-net
> >>>  +{
> >>>  +struct switch_port *p = &val->value.ports[i];
> >>>  +
> >>> -+portmap |= (1 << p->id);
> >>> ++if (id_to_mac[p->id] < 0) {
> >>> ++continue;
> >>> ++}
> >>> ++
> >>> ++portmap |= (1 << id_to_mac[p->id]);
> >>>  +if (p->flags & (1 << SWITCH_PORT_FLAG_TAGGED))
> >>> -+tagmap |= (1 << p->id);
> >>> ++tagmap |= (1 << id_to_mac[p->id]);
> >>>  +}
> >>>  +
> >>>  +tem.table = XRX200_PCE_VLANMAP_IDX;
> >>> @@ -855,7 +863,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq: adds
> >> xrx200-net
> >>>  +continue;
> >>>  +
> >>>  +p = &val->value.ports[val->len++];
> >>> -+p->id = i;
> >>> ++p->id = mac_to_id[i];
> >>>  +if (tags & (1 << i))
> >>>  +p->flags = (1 << SWITCH_PORT_FLAG_TAGGED);
> >>>  +else
> >>> @@ -898,11 +906,12 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq:
> adds
> >> xrx200-net
> >>>  +}
> >>>  +
> >>>  +// port
> >>> -+static int xrx200sw_get_port_pvid(struct switch_dev *dev, int
> port,
> >> int *val)
> >>> ++static int xrx200sw_get_port_pvid(struct switch_dev *dev, int
> >> port_id, int *val)
> >>>  +{
> >>> ++int port = id_to_mac[port_id];
> >>>  +struct xrx200_pce_table_entry tev;
> >>>  +
> >>> -+if (port >= XRX200_MAX_PORT)
> >>> ++if (port < 0 || port >= XRX200_MAX_PORT)
> >>>  +return -EINVAL;
> >>>  +
> >>>  +tev.table = XRX200_PCE_ACTVLAN_IDX;
> >>> @@ -914,10 +923,12 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq:
> adds
> >> xrx200-net
> >>>  +}
> >>>  +
> >>>  +static int xrx200sw_get_port_link(struct switch_dev *dev,
> >>> -+  int port,
> >>> ++  int port_id,
> >>>  +  struct switch_port_link *link)
> >>>  +{
> >>> -+if (port >= XRX200_MAX_PORT)
> >>> ++int port = id_to_mac[port_id];
> >>> ++
> >>> ++if (port < 0 || port >= XRX200_MAX_PORT)
> >>>  +return -EINVAL;
> >>>  +
> >>>  +link->link = xrx200sw_read_x(XRX200_MAC_PSTAT_LSTAT, port);
> >>> @@ -1432,7 +1443,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq:
> adds
> >> xrx200-net
> >>>  +xrx200_gmac_update(&priv->port[i]);
> >>>  +priv->port[i].link = priv->port[i].phydev->link;
> >>>  +netdev_info(dev, "port %d %s link\n",
> >>> -+priv->port[i].num,
> >>> ++priv->port[i].id,
> >>>  +(priv->port[i].link)?("got"):("lost"));
> >>>  +}
> >>>  +}
> >>> @@ -1633,7 +1644,7 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq:
> adds
> >> xrx200-net
> >>>  +for (i = 0; i < priv->num_port; i++)
> >>>  +if (xrx200_mdio_probe(dev, &priv->port[i]))
> >>>  +pr_warn("xrx200-mdio: probing phy of port %d
> >> failed\n",
> >>> -+ priv->port[i].num);
> >>> ++ priv->port[i].id);
> >>>  +
> >>>  +return 0;
> >>>  +
> >>> @@ -1787,19 +1798,24 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq:
> >> adds xrx200-net
> >>>  +
> >>>  +static void xrx200_of_port(struct xrx200_priv *priv, struct
> >> device_node *port)
> >>>  +{
> >>> -+const __be32 *addr, *id = of_get_property(port, "reg", NULL);
> >>> ++const __be32 *addr, *mac = of_get_property(port, "reg", NULL);
> >>>  +struct xrx200_port *p = &priv->port[priv->num_port];
> >>> ++int id = of_alias_get_id(port, "ethernet");
> >>>  +
> >>> -+if (!id)
> >>> ++if (!mac)
> >>>  +return;
> >>>  +
> >>> ++if (id < 0)
> >>> ++id = *mac;
> >>> ++
> >>>  +memset(p, 0, sizeof(struct xrx200_port));
> >>>  +p->phy_node = of_parse_phandle(port, "phy-handle", 0);
> >>>  +addr = of_get_property(p->phy_node, "reg", NULL);
> >>>  +if (!addr)
> >>>  +return;
> >>>  +
> >>> -+p->num = *id;
> >>> ++p->num = *mac;
> >>> ++p->id = id;
> >>>  +p->phy_addr = *addr;
> >>>  +p->phy_if = of_get_phy_mode(port);
> >>>  +if (p->phy_addr > 0x10)
> >>> @@ -1824,6 +1840,10 @@ Subject: [PATCH 25/36] NET: MIPS: lantiq:
> adds
> >> xrx200-net
> >>>  +
> >>>  +/* store the port id in the hw struct so we can map ports ->
> >> devices */
> >>>  +priv->hw->port_map[p->num] = priv->hw->num_devs;
> >>> ++
> >>> ++/* store the id <-> mac (port) mapping */
> >>> ++id_to_mac[p->id] = p->num;
> >>> ++mac_to_id[p->num] = p->id;
> >>>  +}
> >>>  +
> >>>  +static const struct net_device_ops xrx200_netdev_ops = {
> >>>
> >
> >
> > _______________________________________________
> > openwrt-devel mailing list
> > openwrt-devel@lists.openwrt.org
> > https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
> >
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
John Crispin Feb. 26, 2016, 8:25 a.m. UTC | #5
On 22/12/2015 10:36, Martin Schiller wrote:
>>> And there are also situations, where you even can't do it in
>> > hardware:
>>> > > when you use the xrx200 in Gigabit mode, you can use mac 0,1,2,4,5.
>>> > > mac/port 3 is dead in this case.
>>> > >
>>> > > But you can't tell an enduser/customer that there is a port 2 and a
>> > port 4
>>> > > but no port 3.
>> >
>> > sure but solving this in the driver by using a virtual port abstraction
>> > is imho not good.
>> >
>> > i'll see if i can think of a better solution for this issue
> As this is a board specific issue, I thought solving this in the
> devicetree would be a nice way.
> 
> But any better solution would be welcome.
> 

can you use swconfig,port to solve this ? and it not, maybe extend the
swconfig code to also have a swconfig,portname option ?

that would be a far more generic solution.

	John
diff mbox

Patch

diff --git a/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-adds-xrx200-net.patch b/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-adds-xrx200-net.patch
index 48c7395..c76c165 100644
--- a/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-adds-xrx200-net.patch
+++ b/target/linux/lantiq/patches-4.1/0025-NET-MIPS-lantiq-adds-xrx200-net.patch
@@ -209,7 +209,7 @@  Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
 +};
 --- /dev/null
 +++ b/drivers/net/ethernet/lantiq_xrx200.c
-@@ -0,0 +1,1798 @@
+@@ -0,0 +1,1818 @@
 +/*
 + *   This program is free software; you can redistribute it and/or modify it
 + *   under the terms of the GNU General Public License version 2 as published
@@ -404,6 +404,7 @@  Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
 +
 +struct xrx200_port {
 +	u8 num;
++	u8 id;
 +	u8 phy_addr;
 +	u16 flags;
 +	phy_interface_t phy_if;
@@ -461,6 +462,9 @@  Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
 +	struct xrx200_hw *hw;
 +};
 +
++static int id_to_mac[XRX200_MAX_PORT] = {-1,-1,-1,-1,-1,-1,6};
++static int mac_to_id[XRX200_MAX_PORT] = {-1,-1,-1,-1,-1,-1,6};
++
 +static __iomem void *xrx200_switch_membase;
 +static __iomem void *xrx200_mii_membase;
 +static __iomem void *xrx200_mdio_membase;
@@ -789,9 +793,13 @@  Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
 +	{
 +		struct switch_port *p = &val->value.ports[i];
 +
-+		portmap |= (1 << p->id);
++		if (id_to_mac[p->id] < 0) {
++			continue;
++		}
++
++		portmap |= (1 << id_to_mac[p->id]);
 +		if (p->flags & (1 << SWITCH_PORT_FLAG_TAGGED))
-+			tagmap |= (1 << p->id);
++			tagmap |= (1 << id_to_mac[p->id]);
 +	}
 +
 +	tem.table = XRX200_PCE_VLANMAP_IDX;
@@ -855,7 +863,7 @@  Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
 +			continue;
 +
 +		p = &val->value.ports[val->len++];
-+		p->id = i;
++		p->id = mac_to_id[i];
 +		if (tags & (1 << i))
 +			p->flags = (1 << SWITCH_PORT_FLAG_TAGGED);
 +		else
@@ -898,11 +906,12 @@  Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
 +}
 +
 +// port
-+static int xrx200sw_get_port_pvid(struct switch_dev *dev, int port, int *val)
++static int xrx200sw_get_port_pvid(struct switch_dev *dev, int port_id, int *val)
 +{
++	int port = id_to_mac[port_id];
 +	struct xrx200_pce_table_entry tev;
 +
-+	if (port >= XRX200_MAX_PORT)
++	if (port < 0 || port >= XRX200_MAX_PORT)
 +		return -EINVAL;
 +
 +	tev.table = XRX200_PCE_ACTVLAN_IDX;
@@ -914,10 +923,12 @@  Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
 +}
 +
 +static int xrx200sw_get_port_link(struct switch_dev *dev,
-+				  int port,
++				  int port_id,
 +				  struct switch_port_link *link)
 +{
-+	if (port >= XRX200_MAX_PORT)
++	int port = id_to_mac[port_id];
++
++	if (port < 0 || port >= XRX200_MAX_PORT)
 +		return -EINVAL;
 +
 +	link->link = xrx200sw_read_x(XRX200_MAC_PSTAT_LSTAT, port);
@@ -1432,7 +1443,7 @@  Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
 +			xrx200_gmac_update(&priv->port[i]);
 +			priv->port[i].link = priv->port[i].phydev->link;
 +			netdev_info(dev, "port %d %s link\n",
-+				priv->port[i].num,
++				priv->port[i].id,
 +				(priv->port[i].link)?("got"):("lost"));
 +		}
 +	}
@@ -1633,7 +1644,7 @@  Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
 +	for (i = 0; i < priv->num_port; i++)
 +		if (xrx200_mdio_probe(dev, &priv->port[i]))
 +			pr_warn("xrx200-mdio: probing phy of port %d failed\n",
-+					 priv->port[i].num);
++					 priv->port[i].id);
 +
 +	return 0;
 +
@@ -1787,19 +1798,24 @@  Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
 +
 +static void xrx200_of_port(struct xrx200_priv *priv, struct device_node *port)
 +{
-+	const __be32 *addr, *id = of_get_property(port, "reg", NULL);
++	const __be32 *addr, *mac = of_get_property(port, "reg", NULL);
 +	struct xrx200_port *p = &priv->port[priv->num_port];
++	int id = of_alias_get_id(port, "ethernet");
 +
-+	if (!id)
++	if (!mac)
 +		return;
 +
++	if (id < 0)
++		id = *mac;
++
 +	memset(p, 0, sizeof(struct xrx200_port));
 +	p->phy_node = of_parse_phandle(port, "phy-handle", 0);
 +	addr = of_get_property(p->phy_node, "reg", NULL);
 +	if (!addr)
 +		return;
 +
-+	p->num = *id;
++	p->num = *mac;
++	p->id = id;
 +	p->phy_addr = *addr;
 +	p->phy_if = of_get_phy_mode(port);
 +	if (p->phy_addr > 0x10)
@@ -1824,6 +1840,10 @@  Subject: [PATCH 25/36] NET: MIPS: lantiq: adds xrx200-net
 +
 +	/* store the port id in the hw struct so we can map ports -> devices */
 +	priv->hw->port_map[p->num] = priv->hw->num_devs;
++
++	/* store the id <-> mac (port) mapping */
++	id_to_mac[p->id] = p->num;
++	mac_to_id[p->num] = p->id;
 +}
 +
 +static const struct net_device_ops xrx200_netdev_ops = {