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
>
