Message ID | 1453384529-17814-1-git-send-email-zajec5@gmail.com |
---|---|
State | Changes Requested |
Headers | show |
Hi, On 21 January 2016 at 14:55, Rafał Miłecki <zajec5@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 <zajec5@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
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; + + /* Custom implementation */ + if (dev->ops->set_port_link) + return dev->ops->set_port_link(dev, port, link); + + /* Chceck if we can use generic implementation */ + if (!dev->ops->phy_write16) + return -ENOTSUPP; + + /* 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 {
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 <zajec5@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(-)