lkml.org 
[lkml]   [2016]   [Aug]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH v5 3/3] net: phy: Add gmiitorgmii converter support
From
Date
On 08/09/2016 02:34 AM, Kedareswara rao Appana wrote:
> This patch adds support for gmiitorgmii converter.
>
> The GMII to RGMII IP core provides the Reduced Gigabit Media
> Independent Interface (RGMII) between Ethernet physical media
> Devices and the Gigabit Ethernet controller. This core can
> Switch dynamically between the three different speed modes of
> Operation by configuring the converter register through mdio write.
>
> MDIO interface is used to set operating speed of Ethernet MAC.
>
> This converter sits between the MAC and the external phy
> MAC <==> GMII2RGMII <==> RGMII_PHY

This looks good, just a few things, see below:

>
> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> ---
> Thanks a lot Andrew for your inputs.
> Changes for v5:
> --> Fixed return values in the probe as suggested by punnaiah.
> --> Added a mask for the converter speed as suggested by punnaiah.
> Changes for v4:
> --> Updated phydev speed for all 3 speeds as suggested by zhuyj.
> Changes for v3:
> --> Updated the driver as suggested by Andrew.
> Changes for v2:
> --> Passed struct xphy pointer directly to the fix_mac_speed
> API as suggested by the Florian.
> --> Added checks for the phy-node fail case as suggested
> by the Florian
>
> drivers/net/phy/Kconfig | 8 +++
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/xilinx_gmii2rgmii.c | 121 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 130 insertions(+)
> create mode 100644 drivers/net/phy/xilinx_gmii2rgmii.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 1b534ea..c79f347 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -312,6 +312,14 @@ config MICROSEMI_PHY
> ---help---
> Currently supports the VSC8531 and VSC8541 PHYs
>
> +config XILINX_GMII2RGMII
> + tristate "Xilinx GMII2RGMII converter driver"
> + default y

Don't force that, or at least make the default based on the potential
users/drivers here.

> + ---help---
> + This driver support xilinx GMII to RGMII IP core it provides
> + the Reduced Gigabit Media Independent Interface(RGMII) between
> + Ethernet physical media devices and the Gigabit Ethernet controller.
> +
> endif # PHYLIB
>
> config MICREL_KS8995MA
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index a713bd4..73d65ce 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -50,3 +50,4 @@ obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o
> obj-$(CONFIG_INTEL_XWAY_PHY) += intel-xway.o
> obj-$(CONFIG_MDIO_HISI_FEMAC) += mdio-hisi-femac.o
> obj-$(CONFIG_MDIO_XGENE) += mdio-xgene.o
> +obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
> diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c
> new file mode 100644
> index 0000000..1456e27
> --- /dev/null
> +++ b/drivers/net/phy/xilinx_gmii2rgmii.c
> @@ -0,0 +1,121 @@
> +/* Xilinx GMII2RGMII Converter driver
> + *
> + * Copyright (C) 2016 Xilinx, Inc.
> + *
> + * Author: Kedareswara rao Appana <appanad@xilinx.com>
> + *
> + * Description:
> + * This driver is developed for Xilinx GMII2RGMII Converter
> + *
> + * 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/kernel.h>
> +#include <linux/mii.h>
> +#include <linux/mdio.h>
> +#include <linux/phy.h>
> +#include <linux/of_mdio.h>
> +
> +#define XILINX_GMII2RGMII_REG 0x10
> +#define XILINX_GMII2RGMII_SPEED_MASK 0x2040

BMCR_SPEED1000 | BMCR_SPEED100 would be clearer here.

> +
> +struct gmii2rgmii {
> + struct phy_device *phy_dev;
> + struct phy_driver *phy_drv;
> + struct phy_driver conv_phy_drv;
> + int addr;
> +};
> +
> +static int xgmiitorgmii_read_status(struct phy_device *phydev)
> +{
> + struct gmii2rgmii *priv = (struct gmii2rgmii *)phydev->priv;

Casting is not required here, priv is void *.

> + u16 val = 0;
> +
> + priv->phy_drv->read_status(phydev);
> +
> + val = mdiobus_read(phydev->mdio.bus, priv->addr, XILINX_GMII2RGMII_REG);
> + val &= XILINX_GMII2RGMII_SPEED_MASK;
> +
> + switch (phydev->speed) {
> + case SPEED_1000:
> + val |= BMCR_SPEED1000;

Is the fall through really intentional here? See genphy_setup_forced()
for instance.

> + case SPEED_100:
> + val |= BMCR_SPEED100;
> + case SPEED_10:
> + val |= BMCR_SPEED10;
> + }
> +
> + mdiobus_write(phydev->mdio.bus, priv->addr, XILINX_GMII2RGMII_REG, val);
> +
> + return 0;
> +}
[snip]

> +static int __init xgmiitorgmii_init(void)
> +{
> + return mdio_driver_register(&xgmiitorgmii_driver);
> +}
> +module_init(xgmiitorgmii_init);
> +
> +static void __exit xgmiitorgmii_cleanup(void)
> +{
> + mdio_driver_unregister(&xgmiitorgmii_driver);
> +}
> +module_exit(xgmiitorgmii_cleanup);

mdio_module_driver() does eliminate a bit of this boilerplate code.
--
Florian

\
 
 \ /
  Last update: 2016-08-10 00:01    [W:0.086 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site