On Wed, May 24, 2017 at 09:25:48AM +0000, Bogdan Purcareata wrote: > Add basic support for Cortina PHY drivers. Support only CS4340 for now. > The phys are not compatible with IEEE 802.3 clause 45 registers. Implement > proper read_status support, so that phy polling does not cause bus > register access errors. > > The driver should be described using the "ethernet-phy-id" device tree > compatible.
Hi Bogdan Thanks for testing that ethernet-phy-id works. I suggest you write a Documentation/devicetree/binding/net/phy/cortina.txt giving an example device tree node. > > Signed-off-by: Bogdan Purcareata <bogdan.purcare...@nxp.com> > --- > v1 -> v2: > - Rename "mdio-cortina.c" to "cortina.c" since it's a phy driver. > - Test probing based on the "ethernet-phy-id" compatible. In the previous > version, getting the phy_id via get_phy_c45_ids() involved an additional > hack. Drop that approach and document probing in the commit message. > > drivers/net/phy/Kconfig | 5 +++ > drivers/net/phy/Makefile | 1 + > drivers/net/phy/cortina.c | 90 > +++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 96 insertions(+) > create mode 100644 drivers/net/phy/cortina.c > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index 22dea7f..ad09e2d 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -240,6 +240,11 @@ config CICADA_PHY > ---help--- > Currently supports the cis8204 > > +config CORTINA_PHY > + tristate "Cortina quad-10G Ethernet PHY" > + ---help--- > + Currently supports the CS4340 phy. > + > config DAVICOM_PHY > tristate "Davicom PHYs" > ---help--- > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > index 79365be..0de3e20 100644 > --- a/drivers/net/phy/Makefile > +++ b/drivers/net/phy/Makefile > @@ -47,6 +47,7 @@ obj-$(CONFIG_BCM_CYGNUS_PHY) += bcm-cygnus.o > obj-$(CONFIG_BCM_NET_PHYLIB) += bcm-phy-lib.o > obj-$(CONFIG_BROADCOM_PHY) += broadcom.o > obj-$(CONFIG_CICADA_PHY) += cicada.o > +obj-$(CONFIG_CORTINA_PHY) += cortina.o > obj-$(CONFIG_DAVICOM_PHY) += davicom.o > obj-$(CONFIG_DP83640_PHY) += dp83640.o > obj-$(CONFIG_DP83848_PHY) += dp83848.o > diff --git a/drivers/net/phy/cortina.c b/drivers/net/phy/cortina.c > new file mode 100644 > index 0000000..6f054ed > --- /dev/null > +++ b/drivers/net/phy/cortina.c > @@ -0,0 +1,90 @@ > +/* > + * Based on code from Cortina Systems, Inc. > + * > + * Copyright 2011 Cortina Systems, Inc. > + * Copyright 2017 NXP > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > +#include <linux/module.h> > +#include <linux/phy.h> > + > +#define PHY_ID_CS4340 0x13e51002 > + > +#define CORTINA_GPIO_GPIO_INTS 0x16D > + > +static int cortina_read_x(struct phy_device *phydev, int off, u16 regnum) > +{ > + return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr + off, > + MII_ADDR_C45 | regnum); > +} > + > +static int cortina_read(struct phy_device *phydev, u16 regnum) > +{ > + return cortina_read_x(phydev, 0, regnum); > +} > + > +static int cortina_config_aneg(struct phy_device *phydev) > +{ > + phydev->supported = SUPPORTED_10000baseT_Full; > + phydev->advertising = SUPPORTED_10000baseT_Full; > + > + return 0; > +} > + > +static int cortina_read_status(struct phy_device *phydev) > +{ > + int gpio_int_status; > + int ret = 0; I think there needs to be some explanation here. What exactly are you using to indicate link up? What does CORTINA_GPIO_GPIO_INTS mean? > + gpio_int_status = cortina_read(phydev, CORTINA_GPIO_GPIO_INTS); > + if (gpio_int_status < 0) { > + ret = gpio_int_status; > + goto err; > + } > + > + if (gpio_int_status & 0x8) { > + phydev->speed = SPEED_10000; > + phydev->duplex = DUPLEX_FULL; > + phydev->link = 1; > + } else { > + phydev->link = 0; > + } > + > +err: > + return ret; > +} > + > +static int cortina_soft_reset(struct phy_device *phydev) > +{ > + return 0; > +} > + > +static struct phy_driver cortina_driver[] = { > +{ > + .phy_id = PHY_ID_CS4340, > + .phy_id_mask = 0xffffffff, > + .name = "Cortina CS4340", > + .config_aneg = cortina_config_aneg, > + .read_status = cortina_read_status, > + .soft_reset = cortina_soft_reset, > +}, > +}; Having two } at the same indentation level seems odd. Please can you fix this. Thanks Andrew > + > +module_phy_driver(cortina_driver); > + > +static struct mdio_device_id __maybe_unused cortina_tbl[] = { > + { PHY_ID_CS4340, 0xffffffff}, > + {}, > +}; > + > +MODULE_DEVICE_TABLE(mdio, cortina_tbl); > -- > 1.9.1 >