[RFC PATCH v5 0/3] net: phy: Add xilinx gmiitorgmii converter support

[RFC PATCH v5 0/3] net: phy: Add xilinx gmiitorgmii converter support

[Kedareswara rao Appana]

The Gigabit Media Independent Interface (GMII) to Reduced Gigabit Media
Independent Interface (RGMII) core provides the RGMII between RGMII-compliant
Ethernet physical media devices (PHY) and the Gigabit Ethernet controller.
This core can be used in all three modes of operation(10/100/1000 Mb/s).
The Management Data Input/Output (MDIO) interface is used to configure the
Speed of operation. This core can switch dynamically between the three
Different speed modes by configuring the conveter register through mdio write.

The conveter sits b/w the MAC and external phy like below

MACB <==> GMII2RGMII <==> RGMII_PHY

        MDIO   <========> GMII2RGMII
MCAB <=======>
               <========> RGMII

Using MAC MDIO bus we can access both the converter and the external PHY.
We need to program the line speed of the converter during run time based
On the external phy negotiated speed.

This patch series does the below
---> Add mask for Control register 10Mbps speed.
---> Add support for xilinx gmiitorgmii converter.

Kedareswara rao Appana (3):
  net: Add mask for Control register 10Mbps speed
  Documentation: DT: net: Add Xilinx gmiitorgmii converter device tree
    binding documentation
  net: phy: Add gmiitorgmii converter support

 .../devicetree/bindings/net/xilinx_gmii2rgmii.txt  |  38 +++++++
 drivers/net/phy/Kconfig                            |   8 ++
 drivers/net/phy/Makefile                           |   1 +
 drivers/net/phy/xilinx_gmii2rgmii.c                | 121 +++++++++++++++++++++
 include/uapi/linux/mii.h                           |   1 +
 5 files changed, 169 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
 create mode 100644 drivers/net/phy/xilinx_gmii2rgmii.c

-- 
2.1.2

[RFC PATCH v5 1/3] net: Add mask for Control register 10Mbps speed

[Kedareswara rao Appana]

This patch adds mask for the Control register
10Mbps speed.

Signed-off-by: Kedareswara rao Appana <appanad-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
Changes for v5:
---> New patch as suggested by Punnaiah.

 include/uapi/linux/mii.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/mii.h b/include/uapi/linux/mii.h
index 237fac4..15d8510 100644
--- a/include/uapi/linux/mii.h
+++ b/include/uapi/linux/mii.h
@@ -48,6 +48,7 @@
 #define BMCR_SPEED100		0x2000	/* Select 100Mbps              */
 #define BMCR_LOOPBACK		0x4000	/* TXD loopback bits           */
 #define BMCR_RESET		0x8000	/* Reset to default state      */
+#define BMCR_SPEED10		0x0000	/* Select 10Mbps               */
 
 /* Basic mode status register. */
 #define BMSR_ERCAP		0x0001	/* Ext-reg capability          */
-- 
2.1.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v5 1/3] net: Add mask for Control register 10Mbps speed

[Florian Fainelli]

On 08/09/2016 02:34 AM, Kedareswara rao Appana wrote:
> This patch adds mask for the Control register
> 10Mbps speed.
> 
> Signed-off-by: Kedareswara rao Appana <appanad-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>

Reviewed-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v5 2/3] Documentation: DT: net: Add Xilinx gmiitorgmii converter device tree binding documentation

[Kedareswara rao Appana]

Device-tree binding documentation for xilinx gmiitorgmii converter.

Signed-off-by: Kedareswara rao Appana <appanad-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
Changes for v5:
---> Fixed Indentation in the example as suggested by Michal.
Changes for v4:
--> Modified compatible as suggested by Rob.
--> Removed underscores from the converter node name as suggested by Rob.
Changes for v3:
--> None.
Changes for v2:
--> New patch.

 .../devicetree/bindings/net/xilinx_gmii2rgmii.txt  | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt

diff --git a/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
new file mode 100644
index 0000000..5f48793
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
@@ -0,0 +1,38 @@
+XILINX GMIITORGMII Converter Driver Device Tree Bindings
+--------------------------------------------------------
+
+The Gigabit Media Independent Interface (GMII) to Reduced Gigabit Media
+Independent Interface (RGMII) core provides the RGMII between RGMII-compliant
+Ethernet physical media devices (PHY) and the Gigabit Ethernet controller.
+This core can be used in all three modes of operation(10/100/1000 Mb/s).
+The Management Data Input/Output (MDIO) interface is used to configure the
+Speed of operation. This core can switch dynamically between the three
+Different speed modes by configuring the conveter register through mdio write.
+
+The MDIO is a bus to which the PHY devices are connected.  For each
+device that exists on this bus, a child node should be created.  See
+the definition of the PHY node in booting-without-of.txt for an example
+of how to define a PHY.
+
+This converter sits between the ethernet MAC and the external phy.
+MAC <==> GMII2RGMII <==> RGMII_PHY
+
+Required properties:
+- compatible	: Should be "xlnx,gmii-to-rgmii-1.0"
+- reg		: The ID number for the phy, usually a small integer
+- phy-handle	: Should point to the external phy device.
+		  See ethernet.txt file in the same directory.
+
+Example:
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		phy: ethernet-phy@0 {
+			......
+		};
+		gmiitorgmii: gmiitorgmii@8 {
+			compatible = "xlnx,gmii-to-rgmii-1.0";
+			reg = <8>;
+			phy-handle = <&phy>;
+		};
+	};
-- 
2.1.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v5 2/3] Documentation: DT: net: Add Xilinx gmiitorgmii converter device tree binding documentation

[Florian Fainelli]

On 08/09/2016 02:34 AM, Kedareswara rao Appana wrote:
> Device-tree binding documentation for xilinx gmiitorgmii converter.
> 
> Signed-off-by: Kedareswara rao Appana <appanad-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
> Changes for v5:
> ---> Fixed Indentation in the example as suggested by Michal.
> Changes for v4:
> --> Modified compatible as suggested by Rob.
> --> Removed underscores from the converter node name as suggested by Rob.
> Changes for v3:
> --> None.
> Changes for v2:
> --> New patch.
> 
>  .../devicetree/bindings/net/xilinx_gmii2rgmii.txt  | 38 ++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
> new file mode 100644
> index 0000000..5f48793
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
> @@ -0,0 +1,38 @@
> +XILINX GMIITORGMII Converter Driver Device Tree Bindings
> +--------------------------------------------------------
> +
> +The Gigabit Media Independent Interface (GMII) to Reduced Gigabit Media
> +Independent Interface (RGMII) core provides the RGMII between RGMII-compliant
> +Ethernet physical media devices (PHY) and the Gigabit Ethernet controller.
> +This core can be used in all three modes of operation(10/100/1000 Mb/s).
> +The Management Data Input/Output (MDIO) interface is used to configure the
> +Speed of operation. This core can switch dynamically between the three
> +Different speed modes by configuring the conveter register through mdio write.
> +
> +The MDIO is a bus to which the PHY devices are connected.  For each
> +device that exists on this bus, a child node should be created.  See
> +the definition of the PHY node in booting-without-of.txt for an example
> +of how to define a PHY.

I would skip this paragraph which does not really help with
understanding, and just refer to
Documentation/devicetree/bindings/net/phy.txt for examples.


> +
> +This converter sits between the ethernet MAC and the external phy.
> +MAC <==> GMII2RGMII <==> RGMII_PHY
> +
> +Required properties:
> +- compatible	: Should be "xlnx,gmii-to-rgmii-1.0"
> +- reg		: The ID number for the phy, usually a small integer

You would want specify that "reg" property needs to match the one of the
PHY (specified via phy-handle) you are converting to/from for this
"proxy" piece of hardware to work.

If these two have the same "reg" value, is not that going to lead to
duplicate MDIO devices created on the bus, this may work, based on
probing ordering, but seems unusual, you don't really need the "reg"
property here it seems?

> +- phy-handle	: Should point to the external phy device.
> +		  See ethernet.txt file in the same directory.
> +
> +Example:
> +	mdio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		phy: ethernet-phy@0 {
> +			......
> +		};
> +		gmiitorgmii: gmiitorgmii@8 {
> +			compatible = "xlnx,gmii-to-rgmii-1.0";
> +			reg = <8>;
> +			phy-handle = <&phy>;
> +		};
> +	};
> 


-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC PATCH v5 2/3] Documentation: DT: net: Add Xilinx gmiitorgmii converter device tree binding documentation

[Appana Durga Kedareswara Rao]

Hi Florian,

	Thanks for the review...

> 
> On 08/09/2016 02:34 AM, Kedareswara rao Appana wrote:
> > Device-tree binding documentation for xilinx gmiitorgmii converter.
> >
> > Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> > ---
> > Changes for v5:
> > ---> Fixed Indentation in the example as suggested by Michal.
> > Changes for v4:
> > --> Modified compatible as suggested by Rob.
> > --> Removed underscores from the converter node name as suggested by Rob.
> > Changes for v3:
> > --> None.
> > Changes for v2:
> > --> New patch.
> >
> >  .../devicetree/bindings/net/xilinx_gmii2rgmii.txt  | 38
> > ++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
> > b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
> > new file mode 100644
> > index 0000000..5f48793
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
> > @@ -0,0 +1,38 @@
> > +XILINX GMIITORGMII Converter Driver Device Tree Bindings
> > +--------------------------------------------------------
> > +
> > +The Gigabit Media Independent Interface (GMII) to Reduced Gigabit
> > +Media Independent Interface (RGMII) core provides the RGMII between
> > +RGMII-compliant Ethernet physical media devices (PHY) and the Gigabit
> Ethernet controller.
> > +This core can be used in all three modes of operation(10/100/1000 Mb/s).
> > +The Management Data Input/Output (MDIO) interface is used to
> > +configure the Speed of operation. This core can switch dynamically
> > +between the three Different speed modes by configuring the conveter
> register through mdio write.
> > +
> > +The MDIO is a bus to which the PHY devices are connected.  For each
> > +device that exists on this bus, a child node should be created.  See
> > +the definition of the PHY node in booting-without-of.txt for an
> > +example of how to define a PHY.
> 
> I would skip this paragraph which does not really help with understanding, and
> just refer to Documentation/devicetree/bindings/net/phy.txt for examples.

Sure will fix in the next version...

> 
> 
> > +
> > +This converter sits between the ethernet MAC and the external phy.
> > +MAC <==> GMII2RGMII <==> RGMII_PHY
> > +
> > +Required properties:
> > +- compatible	: Should be "xlnx,gmii-to-rgmii-1.0"
> > +- reg		: The ID number for the phy, usually a small integer
> 
> You would want specify that "reg" property needs to match the one of the PHY
> (specified via phy-handle) you are converting to/from for this "proxy" piece of
> hardware to work.
> 
> If these two have the same "reg" value, is not that going to lead to duplicate
> MDIO devices created on the bus, this may work, based on probing ordering, but
> seems unusual, you don't really need the "reg"
> property here it seems?

The converter Phy address is different from the external phy address.

Regards,
Kedar.


> 
> > +- phy-handle	: Should point to the external phy device.
> > +		  See ethernet.txt file in the same directory.
> > +
> > +Example:
> > +	mdio {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		phy: ethernet-phy@0 {
> > +			......
> > +		};
> > +		gmiitorgmii: gmiitorgmii@8 {
> > +			compatible = "xlnx,gmii-to-rgmii-1.0";
> > +			reg = <8>;
> > +			phy-handle = <&phy>;
> > +		};
> > +	};
> >
> 
> 
> --
> Florian

[RFC PATCH v5 3/3] net: phy: Add gmiitorgmii converter support

[Kedareswara rao Appana]

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

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
+       ---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
+
+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;
+	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;
+	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;
+}
+
+int xgmiitorgmii_probe(struct mdio_device *mdiodev)
+{
+	struct device *dev = &mdiodev->dev;
+	struct device_node *np = dev->of_node, *phy_node;
+	struct gmii2rgmii *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	phy_node = of_parse_phandle(np, "phy-handle", 0);
+	if (IS_ERR(phy_node)) {
+		dev_err(dev, "Couldn't parse phy-handle\n");
+		return -ENODEV;
+	}
+
+	priv->phy_dev = of_phy_find_device(phy_node);
+	if (!priv->phy_dev) {
+		dev_info(dev, "Couldn't find phydev\n");
+		return -EPROBE_DEFER;
+	}
+
+	priv->addr = mdiodev->addr;
+	priv->phy_drv = priv->phy_dev->drv;
+	memcpy(&priv->conv_phy_drv, priv->phy_dev->drv,
+	       sizeof(struct phy_driver));
+	priv->conv_phy_drv.read_status = xgmiitorgmii_read_status;
+	priv->phy_dev->priv = priv;
+	priv->phy_dev->drv = &priv->conv_phy_drv;
+
+	return 0;
+}
+
+static const struct of_device_id xgmiitorgmii_of_match[] = {
+	{ .compatible = "xlnx,gmii-to-rgmii-1.0" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, xgmiitorgmii_of_match);
+
+static struct mdio_driver xgmiitorgmii_driver = {
+	.probe	= xgmiitorgmii_probe,
+	.mdiodrv.driver = {
+		.name = "xgmiitorgmii",
+		.of_match_table = xgmiitorgmii_of_match,
+	},
+};
+
+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);
+
+MODULE_DESCRIPTION("Xilinx GMII2RGMII converter driver");
+MODULE_LICENSE("GPL");
-- 
2.1.2

Re: [RFC PATCH v5 3/3] net: phy: Add gmiitorgmii converter support

[Florian Fainelli]

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-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
> 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-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> + *
> + * 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC PATCH v5 3/3] net: phy: Add gmiitorgmii converter support

[Appana Durga Kedareswara Rao]

Hi Florian,

	Thanks for the review...

> >
> > This converter sits between the MAC and the external phy MAC <==>
> > GMII2RGMII <==> RGMII_PHY
> 
> This looks good, just a few things, see below:

Thanks...

> > +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.

Ok sure will fix in the next version...

> 
> > +       ---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

<snip>

> > +#define XILINX_GMII2RGMII_REG		0x10
> > +#define XILINX_GMII2RGMII_SPEED_MASK	0x2040
> 
> BMCR_SPEED1000 | BMCR_SPEED100 would be clearer here.

Sure will fix...

> 
> > +
> > +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 *.

Ok will remove...

> 
> > +	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.

Ok will fix...

> 
> > +	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.

Sure will fix in the next version...

Regards,
Kedar.
> --
> Florian

Re: [RFC PATCH v5 3/3] net: phy: Add gmiitorgmii converter support

[Andrew Lunn]

> Signed-off-by: Kedareswara rao Appana <appanad-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
> 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.
> +/* Xilinx GMII2RGMII Converter driver
> + *
> + * Copyright (C) 2016 Xilinx, Inc.
> + *
> + * Author: Kedareswara rao Appana <appanad-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>

Not cool

https://github.com/lunn/linux/commit/03d375489ceb56e171056f44d0fe9c34ca5a098e

Notice the

Not-Signed-off-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>

and the Copyright i added?

In various emails i gave you the basic idea how this should be done, a
framework of code, and then took your code which did not even compile
and made the core of it work. This is as much my code as your code, so
i expect my Copyright to be kept on this code. And since this is my
code, i need to give a Signed-off-by, which i was not yet ready to
give since the code was not complete or working.

Please put both my Copyright back and Not-Signed-Off-by back and post
the next version as RFC. Once i'm happy this code is O.K, i will give
you a Signed-off-by.

    Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC PATCH v5 3/3] net: phy: Add gmiitorgmii converter support

[Appana Durga Kedareswara Rao]

Hi Andrew,

> 
> > Signed-off-by: Kedareswara rao Appana <appanad-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> > ---
> > 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.
> > +/* Xilinx GMII2RGMII Converter driver
> > + *
> > + * Copyright (C) 2016 Xilinx, Inc.
> > + *
> > + * Author: Kedareswara rao Appana <appanad-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> 
> Not cool
> 
> https://github.com/lunn/linux/commit/03d375489ceb56e171056f44d0fe9c34ca
> 5a098e
> 
> Notice the
> 
> Not-Signed-off-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
> 
> and the Copyright i added?
> 
> In various emails i gave you the basic idea how this should be done, a framework
> of code, and then took your code which did not even compile and made the core
> of it work. This is as much my code as your code, so i expect my Copyright to be
> kept on this code. And since this is my code, i need to give a Signed-off-by,
> which i was not yet ready to give since the code was not complete or working.
> 
> Please put both my Copyright back and Not-Signed-Off-by back and post the
> next version as RFC. Once i'm happy this code is O.K, i will give you a Signed-off-
> by.

Sorry I should have been included your copyright.

I missed it...
I didn't included your SOF because.
AFAIK without explicit permission of the other person we shouldn't include SOF.
That's why I didn't included your SOF and thanked for your efforts after SOF.
Sorry if my understanding is wrong.
 
There are few comments for this patch series and I fixed it and sent the v6 version.
 V6 version of patch series already got merged on the net-next.

Sent the patch by updating the driver with your copy right 
"net: phy: Update copyright info".

Sorry for the noise will fix these type of issues next time onwards...

Regards,
Kedar.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v5 3/3] net: phy: Add gmiitorgmii converter support

[Andrew Lunn]

> > https://github.com/lunn/linux/commit/03d375489ceb56e171056f44d0fe9c34ca
> > 5a098e
> > 
> > Notice the
> > 
> > Not-Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > 
> > and the Copyright i added?
> > 
> > In various emails i gave you the basic idea how this should be done, a framework
> > of code, and then took your code which did not even compile and made the core
> > of it work. This is as much my code as your code, so i expect my Copyright to be
> > kept on this code. And since this is my code, i need to give a Signed-off-by,
> > which i was not yet ready to give since the code was not complete or working.
> > 
> > Please put both my Copyright back and Not-Signed-Off-by back and post the
> > next version as RFC. Once i'm happy this code is O.K, i will give you a Signed-off-
> > by.
> 
> Sorry I should have been included your copyright.
> 
> I missed it...
> I didn't included your SOF because.
> AFAIK without explicit permission of the other person we shouldn't include SOF.
> That's why I didn't included your SOF and thanked for your efforts after SOF.
> Sorry if my understanding is wrong.

Without a SOB you cannot submit the patch for inclusion at all. You
should however reproduce tags which have been given, i.e. you should
of kept my Not-Signed-Off-by: making it clear i have not yet given
permission for this code to go into mainline.

	   Andrew