Hi, On 21 January 2016 at 14:55, Rafał Miłecki <zaj...@gmail.com> wrote: > It's quite common for switches to have PHY per port so we may use a > generic function for setting port link. We just need an API to access > PHYs which this patch also adds. > > Signed-off-by: Rafał Miłecki <zaj...@gmail.com> > --- > .../linux/generic/files/drivers/net/phy/swconfig.c | 44 > ++++++++++++++++++++-- > target/linux/generic/files/include/linux/switch.h | 3 ++ > 2 files changed, 44 insertions(+), 3 deletions(-) > > diff --git a/target/linux/generic/files/drivers/net/phy/swconfig.c > b/target/linux/generic/files/drivers/net/phy/swconfig.c > index 9a5f1e9..8b9bb51 100644 > --- a/target/linux/generic/files/drivers/net/phy/swconfig.c > +++ b/target/linux/generic/files/drivers/net/phy/swconfig.c > @@ -25,6 +25,7 @@ > #include <linux/switch.h> > #include <linux/of.h> > #include <linux/version.h> > +#include <uapi/linux/mii.h> > > #define SWCONFIG_DEVNAME "switch%d" > > @@ -131,10 +132,47 @@ static int > swconfig_set_link(struct switch_dev *dev, const struct switch_attr *attr, > struct switch_val *val) > { > - if (!dev->ops->set_port_link) > - return -EOPNOTSUPP; > + struct switch_port_link *link = val->value.link; > + int port = val->port_vlan; > + > + if (port == dev->cpu_port) > + return -EINVAL;
The cpu port might not be the only port that may not be modified; sometimes there is more than one fixed connection, sometimes the phy ports aren't contiguous. I think it would make more sense to add a function for switch drivers to call than to do it directly in the callback, so they can do something like int b53_set_link(...) { /* TODO: BCM63XX requires special handling as it can have external phys, and ports might be GE or only FE */ if (is63xx(dev)) return -EINVAL; if (port == dev->CPU_PORT) return -EINVAL; if (!(BIT(port) & dev->enabled_ports)) return -EINVAL; if (link->speed == SWITCH_PORT_SPEED_1000 && (is5325() || is5365()) return -EINVAL; if (link->speed == SWITCH_PORT_SPEED_1000 && !link->duplex) return -EINVAL; return switch_generic_set_link(...); } > + > + /* Custom implementation */ > + if (dev->ops->set_port_link) > + return dev->ops->set_port_link(dev, port, link); > + And the following being the generic function to call: > + /* Chceck if we can use generic implementation */ *Check > + if (!dev->ops->phy_write16) > + return -ENOTSUPP; > + this-^ one maybe with a WARN_ON() to spot misusage. > + /* Generic implementation */ > + if (link->aneg) { > + dev->ops->phy_write16(dev, port, MII_BMCR, 0x0000); > + dev->ops->phy_write16(dev, port, MII_BMCR, BMCR_ANENABLE | > BMCR_ANRESTART); > + } else { > + u16 bmcr = 0; > > - return dev->ops->set_port_link(dev, val->port_vlan, val->value.link); > + if (link->duplex) > + bmcr |= BMCR_FULLDPLX; > + > + switch (link->speed) { > + case SWITCH_PORT_SPEED_10: > + break; > + case SWITCH_PORT_SPEED_100: > + bmcr |= BMCR_SPEED100; > + break; > + case SWITCH_PORT_SPEED_1000: > + bmcr |= BMCR_SPEED1000; > + break; > + default: > + return -ENOTSUPP; > + } > + > + dev->ops->phy_write16(dev, port, MII_BMCR, bmcr); > + } > + > + return 0; > } > > static int > diff --git a/target/linux/generic/files/include/linux/switch.h > b/target/linux/generic/files/include/linux/switch.h > index 4ada0e5..ab587ea 100644 > --- a/target/linux/generic/files/include/linux/switch.h > +++ b/target/linux/generic/files/include/linux/switch.h > @@ -99,6 +99,9 @@ struct switch_dev_ops { > struct switch_port_link *link); > int (*get_port_stats)(struct switch_dev *dev, int port, > struct switch_port_stats *stats); > + > + int (*phy_read16)(struct switch_dev *dev, int addr, u8 reg, u16 > *value); > + int (*phy_write16)(struct switch_dev *dev, int addr, u8 reg, u16 > value); > }; > > struct switch_dev { > -- > 1.8.4.5 Jonas _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel