[RFC PATCH 0/2] net: ethernet: Add support for gmii2rgmii converter

[Kedareswara rao Appana]

This patch series does the following
---> Add support for gmii2rgmii converter.
---> Add support for gmii2rgmii converter in the macb driver.

Earlier sent one RFC patch https://patchwork.kernel.org/patch/9186615/
which includes converter related stuff also in macb driver.
This patch series fixes this issue.

Kedareswara rao Appana (2):
net: ethernet: xilinx: Add gmii2rgmii converter support
net: macb: Add gmii2rgmii phy converter support

drivers/net/ethernet/cadence/macb.c | 21 ++++++-
drivers/net/ethernet/cadence/macb.h | 3 +
drivers/net/ethernet/xilinx/Kconfig | 7 ++
drivers/net/ethernet/xilinx/Makefile | 1 +
drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c | 76 +++++++++++++++++++++++
include/linux/xilinx_gmii2rgmii.h | 24 +++++++
6 files changed, 131 insertions(+), 1 deletions(-)
create mode 100644 drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c
create mode 100644 include/linux/xilinx_gmii2rgmii.h

[Kedareswara rao Appana]

This patch adds support for gmii2rgmii 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.
MDIO interface is used to set operating speed of Ethernet MAC

Signed-off-by: Kedareswara rao Appana <app...@xilinx.com>
---

drivers/net/ethernet/xilinx/Kconfig | 7 ++
drivers/net/ethernet/xilinx/Makefile | 1 +
drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c | 76 +++++++++++++++++++++++
include/linux/xilinx_gmii2rgmii.h | 24 +++++++

4 files changed, 108 insertions(+), 0 deletions(-)

create mode 100644 drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c
create mode 100644 include/linux/xilinx_gmii2rgmii.h

diff --git a/drivers/net/ethernet/xilinx/Kconfig b/drivers/net/ethernet/xilinx/Kconfig
index 4f5c024..d7df70a 100644
--- a/drivers/net/ethernet/xilinx/Kconfig
+++ b/drivers/net/ethernet/xilinx/Kconfig
@@ -39,4 +39,11 @@ config XILINX_LL_TEMAC
This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
core used in Xilinx Spartan and Virtex FPGAs

+config XILINX_GMII2RGMII
+ tristate "Xilinx GMII2RGMII converter driver"
+ ---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 # NET_VENDOR_XILINX
diff --git a/drivers/net/ethernet/xilinx/Makefile b/drivers/net/ethernet/xilinx/Makefile
index 214205e..bca0da0 100644
--- a/drivers/net/ethernet/xilinx/Makefile
+++ b/drivers/net/ethernet/xilinx/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_XILINX_LL_TEMAC) += ll_temac.o
obj-$(CONFIG_XILINX_EMACLITE) += xilinx_emaclite.o
xilinx_emac-objs := xilinx_axienet_main.o xilinx_axienet_mdio.o
obj-$(CONFIG_XILINX_AXI_EMAC) += xilinx_emac.o
+obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
diff --git a/drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c b/drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c
new file mode 100644
index 0000000..ca9f1ad
--- /dev/null
+++ b/drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c
@@ -0,0 +1,76 @@
+/* Xilinx GMII2RGMII Converter driver
+ *
+ * Copyright (C) 2016 Xilinx, Inc.
+ *
+ * Author: Kedareswara rao Appana <app...@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/types.h>
+#include <linux/netdevice.h>
+#include <linux/mii.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/of_mdio.h>
+#include <linux/xilinx_gmii2rgmii.h>
+
+static void xgmii2rgmii_fix_mac_speed(void *priv, unsigned int speed)
+{
+ struct gmii2rgmii *xphy = (struct xphy *)priv;
+ struct phy_device *gmii2rgmii_phydev = xphy->gmii2rgmii_phy_dev;
+ u16 gmii2rgmii_reg = 0;
+
+ switch (speed) {
+ case 1000:
+ gmii2rgmii_reg |= XILINX_GMII2RGMII_SPEED1000;
+ break;
+ case 100:
+ gmii2rgmii_reg |= XILINX_GMII2RGMII_SPEED100;
+ break;
+ default:
+ return;
+ }
+
+ xphy->mdio_write(xphy->mii_bus, gmii2rgmii_phydev->mdio.addr,
+ XILINX_GMII2RGMII_REG_NUM,
+ gmii2rgmii_reg);
+}
+
+int gmii2rgmii_phyprobe(struct gmii2rgmii *xphy)
+{
+ struct device_node *phy_node;
+ struct phy_device *phydev;
+ struct device_node *np = (struct device_node *)xphy->platform_data;
+
+ phy_node = of_parse_phandle(np, "gmii2rgmii-phy-handle", 0);
+ if (phy_node) {
+ phydev = of_phy_attach(xphy->dev, phy_node, 0, 0);
+ if (!phydev) {
+ netdev_err(xphy->dev,
+ "%s: no gmii to rgmii converter found\n",
+ xphy->dev->name);
+ return -1;
+ }
+ xphy->gmii2rgmii_phy_dev = phydev;
+ }
+ xphy->fix_mac_speed = xgmii2rgmii_fix_mac_speed;
+
+ return 0;
+}
+EXPORT_SYMBOL(gmii2rgmii_phyprobe);
+
+MODULE_DESCRIPTION("Xilinx GMII2RGMII converter driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/xilinx_gmii2rgmii.h b/include/linux/xilinx_gmii2rgmii.h
new file mode 100644
index 0000000..64e1659
--- /dev/null
+++ b/include/linux/xilinx_gmii2rgmii.h
@@ -0,0 +1,24 @@
+#ifndef _GMII2RGMII_H
+#define _GMII2RGMII_H
+
+#include <linux/of.h>
+#include <linux/phy.h>
+#include <linux/mii.h>
+
+#define XILINX_GMII2RGMII_FULLDPLX BMCR_FULLDPLX
+#define XILINX_GMII2RGMII_SPEED1000 BMCR_SPEED1000
+#define XILINX_GMII2RGMII_SPEED100 BMCR_SPEED100
+#define XILINX_GMII2RGMII_REG_NUM 0x10
+
+struct gmii2rgmii {
+ struct net_device *dev;
+ struct mii_bus *mii_bus;
+ struct phy_device *gmii2rgmii_phy_dev;
+ void *platform_data;
+ void (*mdio_write)(struct mii_bus *bus, int mii_id, int reg,
+ int val);
+ void (*fix_mac_speed)(void *priv, unsigned int speed);
+};
+
+extern int gmii2rgmii_phyprobe(struct gmii2rgmii *xphy);
+#endif
--
1.7.1

[Florian Fainelli]

Why not pass struct xphy pointer directly?

Is that property documented in a binding document?

> + if (phy_node) {

Should not there be an else clause which does not assign
xphy->fix_mac_speed in case this property lookup fails?

So the register semantics are fairly standard but not the register
location, have you considered writing a small PHY driver for this block?

> +
> +struct gmii2rgmii {
> + struct net_device *dev;
> + struct mii_bus *mii_bus;
> + struct phy_device *gmii2rgmii_phy_dev;
> + void *platform_data;
> + void (*mdio_write)(struct mii_bus *bus, int mii_id, int reg,
> + int val);
> + void (*fix_mac_speed)(void *priv, unsigned int speed);
> +};
> +
> +extern int gmii2rgmii_phyprobe(struct gmii2rgmii *xphy);
> +#endif
>


--

Florian

[Florian Fainelli]

Le 30/06/2016 23:20, Kedareswara rao Appana a écrit :

> This patch series does the following
> ---> Add support for gmii2rgmii converter.
> ---> Add support for gmii2rgmii converter in the macb driver.
>
> Earlier sent one RFC patch https://patchwork.kernel.org/patch/9186615/
> which includes converter related stuff also in macb driver.
> This patch series fixes this issue.

This still seems very adhoc and not completely explained. Can you
clarify how the gmmi2rgmii converter gets used?

Is the expectation that a MACB Ethernet adapter will be connected to a
RGMII PHY like this:

MACB <=> GMII2RGMII <=> RGMII PHY
MACB MDIO <===========> RGMII_PHY

and still have the ability to control both the PHY device's MDIO
registers and the GMII2RGMII converter and we need to make sure both
have matching settings, or is it something like this:

MACB <=> GMII2RGMII <=> RGMII PHY
MACB MDIO unconnected

and we lose the ability to query the PHY via MDIO?

As Nicolas pointed out, providing a binding documentation update may
help reviewers understand what is being accomplished here.

Thanks!

>
> Kedareswara rao Appana (2):
> net: ethernet: xilinx: Add gmii2rgmii converter support
> net: macb: Add gmii2rgmii phy converter support
>
> drivers/net/ethernet/cadence/macb.c | 21 ++++++-
> drivers/net/ethernet/cadence/macb.h | 3 +
> drivers/net/ethernet/xilinx/Kconfig | 7 ++
> drivers/net/ethernet/xilinx/Makefile | 1 +
> drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c | 76 +++++++++++++++++++++++
> include/linux/xilinx_gmii2rgmii.h | 24 +++++++
> 6 files changed, 131 insertions(+), 1 deletions(-)
> create mode 100644 drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c
> create mode 100644 include/linux/xilinx_gmii2rgmii.h
>

--
Florian

[Andrew Lunn]

On Fri, Jul 01, 2016 at 11:50:10AM +0530, Kedareswara rao Appana wrote:
> This patch series does the following
> ---> Add support for gmii2rgmii converter.

How generic is this gmii2rgmii IP block? Could it be used with any
GMII and RGMII device?

Should it be placed in drivers/net/phy, so making it easier to reuse?

Also, Russell Kings phylink might be a more natural structure for
this. It is hard to know when those patches will land, but it might be
worth looking at.

Andrew

[Appana Durga Kedareswara Rao]

Hi Florian,

Thanks for the review...

> Le 30/06/2016 23:20, Kedareswara rao Appana a écrit :
> > This patch series does the following
> > ---> Add support for gmii2rgmii converter.
> > ---> Add support for gmii2rgmii converter in the macb driver.
> >
> > Earlier sent one RFC patch https://patchwork.kernel.org/patch/9186615/
> > which includes converter related stuff also in macb driver.
> > This patch series fixes this issue.
>
> This still seems very adhoc and not completely explained. Can you clarify how
> the gmmi2rgmii converter gets used?

Sorry I should have been explained it clearly in the patch.
Will fix it in the v2.

This converter works like below

MACB <==> GMII2RGMII <==> RGMII_PHY

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

Using MACB 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.

MDIO interface is used to set operating speed (line speed)

The converter has only one register (0x10) that we need to program to set the operating speed.
The composition of this register is similar to the IEEE standard 802.3 MDIO control register 0x0.

Please let me know if you still need not clear with how this converter works.

IP data sheet is available here @ http://www.xilinx.com/support/documentation/ip_documentation/gmii_to_rgmii/v3_0/pg160-gmii-to-rgmii.pdf

>
> Is the expectation that a MACB Ethernet adapter will be connected to a RGMII
> PHY like this:
>
> MACB <=> GMII2RGMII <=> RGMII PHY
> MACB MDIO <===========> RGMII_PHY
>
> and still have the ability to control both the PHY device's MDIO registers and the
> GMII2RGMII converter and we need to make sure both have matching settings,
> or is it something like this:
>
> MACB <=> GMII2RGMII <=> RGMII PHY
> MACB MDIO unconnected
>
> and we lose the ability to query the PHY via MDIO?
>
> As Nicolas pointed out, providing a binding documentation update may help
> reviewers understand what is being accomplished here.

Sure will fix in v2.

Regards,
Kedar.

[Appana Durga Kedareswara Rao]

Hi Florian,

Thanks for the review.

> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/types.h>
> > +#include <linux/netdevice.h>
> > +#include <linux/mii.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_mdio.h>
> > +#include <linux/xilinx_gmii2rgmii.h>
> > +
> > +static void xgmii2rgmii_fix_mac_speed(void *priv, unsigned int speed)
> > +{
> > + struct gmii2rgmii *xphy = (struct xphy *)priv;
>
> Why not pass struct xphy pointer directly?

Ok will fix in v2...

>
> > + struct phy_device *gmii2rgmii_phydev = xphy->gmii2rgmii_phy_dev;
> > + u16 gmii2rgmii_reg = 0;
> > +
> > + switch (speed) {
> > + case 1000:
> > + gmii2rgmii_reg |= XILINX_GMII2RGMII_SPEED1000;
> > + break;
> > + case 100:
> > + gmii2rgmii_reg |= XILINX_GMII2RGMII_SPEED100;
> > + break;
> > + default:
> > + return;
> > + }
> > +
> > + xphy->mdio_write(xphy->mii_bus, gmii2rgmii_phydev->mdio.addr,
> > + XILINX_GMII2RGMII_REG_NUM,
> > + gmii2rgmii_reg);
> > +}
> > +
> > +int gmii2rgmii_phyprobe(struct gmii2rgmii *xphy) {

> > + struct device_node *phy_node;
> > + struct phy_device *phydev;
> > + struct device_node *np = (struct device_node *)xphy->platform_data;
> > +
> > + phy_node = of_parse_phandle(np, "gmii2rgmii-phy-handle", 0);
>
> Is that property documented in a binding document?

Will document. Will fix in v2...

>
> > + if (phy_node) {
>
> Should not there be an else clause which does not assign
> xphy->fix_mac_speed in case this property lookup fails?

Will fix in v2...

I tried but this PHY doesn't have any vendor / Device ID's
This converter won't suit to PHY framework as we need to programmed the
PHY Control register with the external phy negotiated speed as explained in the other
Mail thread...

Regards,
Kedar.

[Appana Durga Kedareswara Rao]

Hi Andrew,

> On Fri, Jul 01, 2016 at 11:50:10AM +0530, Kedareswara rao Appana wrote:
> > This patch series does the following
> > ---> Add support for gmii2rgmii converter.
>
> How generic is this gmii2rgmii IP block? Could it be used with any GMII and
> RGMII device?

This converter does GMII2RGMII conversion.
This can be used with any MAC which has shared MDIO with external PHY
And this Converter. This Converter IP is validated for MACB.
But it should work with any MAC which has shared MDIO bus (I mean single MDIO multiple PHYS)...

This converter works like below

MACB <==> GMII2RGMII <==> RGMII_PHY

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

Using MACB 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.

MDIO interface is used to set operating speed (line speed)

The converter has only one register (0x10) that we need to program to set the operating speed.
The composition of this register is similar to the IEEE standard 802.3 MDIO control register 0x0.

Please let me know if you still not clear about how this converter works.

>
> Should it be placed in drivers/net/phy, so making it easier to reuse?

Ok will move it drivers/net/phy folder in the next version...

>
> Also, Russell Kings phylink might be a more natural structure for this. It is hard to
> know when those patches will land, but it might be worth looking at.

Ok sure will take a look at this series once posted...

Regards,
Kedar.

>
> Andrew

[Kedareswara rao Appana]

This patch adds support for gmii2rgmii 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.

Signed-off-by: Kedareswara rao Appana <app...@xilinx.com>
---

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/ethernet/xilinx/Kconfig | 8 +++
drivers/net/ethernet/xilinx/Makefile | 1 +
drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c | 79 +++++++++++++++++++++++++
include/linux/xilinx_gmii2rgmii.h | 24 ++++++++
4 files changed, 112 insertions(+)

create mode 100644 drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c
create mode 100644 include/linux/xilinx_gmii2rgmii.h

diff --git a/drivers/net/ethernet/xilinx/Kconfig b/drivers/net/ethernet/xilinx/Kconfig
index 4f5c024..4b65174 100644
--- a/drivers/net/ethernet/xilinx/Kconfig
+++ b/drivers/net/ethernet/xilinx/Kconfig
@@ -39,4 +39,12 @@ config XILINX_LL_TEMAC

This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
core used in Xilinx Spartan and Virtex FPGAs

+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 # NET_VENDOR_XILINX
diff --git a/drivers/net/ethernet/xilinx/Makefile b/drivers/net/ethernet/xilinx/Makefile
index 214205e..bca0da0 100644
--- a/drivers/net/ethernet/xilinx/Makefile
+++ b/drivers/net/ethernet/xilinx/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_XILINX_LL_TEMAC) += ll_temac.o
obj-$(CONFIG_XILINX_EMACLITE) += xilinx_emaclite.o
xilinx_emac-objs := xilinx_axienet_main.o xilinx_axienet_mdio.o
obj-$(CONFIG_XILINX_AXI_EMAC) += xilinx_emac.o
+obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
diff --git a/drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c b/drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c

new file mode 100644
index 0000000..de3bd89
--- /dev/null
+++ b/drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c
@@ -0,0 +1,79 @@

+/* Xilinx GMII2RGMII Converter driver
+ *
+ * Copyright (C) 2016 Xilinx, Inc.
+ *
+ * Author: Kedareswara rao Appana <app...@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/types.h>
+#include <linux/netdevice.h>
+#include <linux/mii.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/of_mdio.h>
+#include <linux/xilinx_gmii2rgmii.h>
+

+static void xgmii2rgmii_fix_mac_speed(struct gmii2rgmii *xphy,
+ unsigned int speed)
+{

+ struct phy_device *gmii2rgmii_phydev = xphy->gmii2rgmii_phy_dev;
+ u16 gmii2rgmii_reg = 0;
+
+ switch (speed) {
+ case 1000:
+ gmii2rgmii_reg |= XILINX_GMII2RGMII_SPEED1000;
+ break;
+ case 100:
+ gmii2rgmii_reg |= XILINX_GMII2RGMII_SPEED100;
+ break;
+ default:
+ return;
+ }
+
+ xphy->mdio_write(xphy->mii_bus, gmii2rgmii_phydev->mdio.addr,
+ XILINX_GMII2RGMII_REG_NUM,
+ gmii2rgmii_reg);
+}
+
+int gmii2rgmii_phyprobe(struct gmii2rgmii *xphy)

+{

+ struct device_node *phy_node;
+ struct phy_device *phydev;
+ struct device_node *np = (struct device_node *)xphy->platform_data;
+
+ phy_node = of_parse_phandle(np, "gmii2rgmii-phy-handle", 0);

+ if (phy_node) {

+ phydev = of_phy_attach(xphy->dev, phy_node, 0, 0);
+ if (!phydev) {
+ netdev_err(xphy->dev,
+ "%s: no gmii to rgmii converter found\n",
+ xphy->dev->name);
+ return -1;
+ }
+ xphy->gmii2rgmii_phy_dev = phydev;
+ xphy->fix_mac_speed = xgmii2rgmii_fix_mac_speed;

+ } else {
+ xphy->gmii2rgmii_phy_dev = 0;
+ xphy->fix_mac_speed = 0;
+ }

+
+ return 0;
+}
+EXPORT_SYMBOL(gmii2rgmii_phyprobe);
+
+MODULE_DESCRIPTION("Xilinx GMII2RGMII converter driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/xilinx_gmii2rgmii.h b/include/linux/xilinx_gmii2rgmii.h
new file mode 100644

index 0000000..b328ee7

--- /dev/null
+++ b/include/linux/xilinx_gmii2rgmii.h
@@ -0,0 +1,24 @@
+#ifndef _GMII2RGMII_H
+#define _GMII2RGMII_H
+
+#include <linux/of.h>
+#include <linux/phy.h>
+#include <linux/mii.h>
+
+#define XILINX_GMII2RGMII_FULLDPLX BMCR_FULLDPLX
+#define XILINX_GMII2RGMII_SPEED1000 BMCR_SPEED1000
+#define XILINX_GMII2RGMII_SPEED100 BMCR_SPEED100
+#define XILINX_GMII2RGMII_REG_NUM 0x10

+
+struct gmii2rgmii {
+ struct net_device *dev;
+ struct mii_bus *mii_bus;
+ struct phy_device *gmii2rgmii_phy_dev;
+ void *platform_data;

+ int (*mdio_write)(struct mii_bus *bus, int mii_id, int reg,
+ u16 val);
+ void (*fix_mac_speed)(struct gmii2rgmii *xphy, unsigned int speed);
+};
+

+extern int gmii2rgmii_phyprobe(struct gmii2rgmii *xphy);
+#endif
--

2.1.2

[Kedareswara rao Appana]

This patch series does the following
---> Add support for gmii2rgmii converter.

---> Add support for gmii2rgmii converter in the macb driver.

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.

Kedareswara rao Appana (4):
Documentation: DT: net: Add Xilinx gmiitorgmii converter device tree
binding documentation

net: ethernet: xilinx: Add gmii2rgmii converter support

Documentation: DT: net: Update binding doc for gmiitorgmii conveter

net: macb: Add gmii2rgmii phy converter support

Documentation/devicetree/bindings/net/macb.txt | 4 ++
.../devicetree/bindings/net/xilinx_gmii2rgmii.txt | 31 +++++++++
drivers/net/ethernet/cadence/macb.c | 21 ++++++
drivers/net/ethernet/cadence/macb.h | 1 +

drivers/net/ethernet/xilinx/Kconfig | 8 +++
drivers/net/ethernet/xilinx/Makefile | 1 +
drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c | 79 ++++++++++++++++++++++

include/linux/xilinx_gmii2rgmii.h | 24 +++++++
8 files changed, 169 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt

create mode 100644 drivers/net/ethernet/xilinx/xilinx_gmii2rgmii.c
create mode 100644 include/linux/xilinx_gmii2rgmii.h

--
2.1.2

[Kedareswara rao Appana]

This patch adds support for gmii2rgmii phy converter in the macb driver.

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.

Signed-off-by: Kedareswara rao Appana <app...@xilinx.com>
---
Changes for v2:

---> Moved header file define xilinx_gmii2rgmii.h to macb.c file
as suggested by the Nicolas.
---> Updated the commit meassage as suggested by the Nicolas.
---> Fixed minor coding comments as suggested by the Nicolas.

drivers/net/ethernet/cadence/macb.c | 21 +++++++++++++++++++++
drivers/net/ethernet/cadence/macb.h | 1 +
2 files changed, 22 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index cb07d95..b900eb4 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -32,6 +32,7 @@
#include <linux/of_gpio.h>
#include <linux/of_mdio.h>
#include <linux/of_net.h>
+#include <linux/xilinx_gmii2rgmii.h>

#include "macb.h"

@@ -257,6 +258,14 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
return 0;
}

+static inline void macb_hw_fix_mac_speed(struct macb *bp,
+ struct phy_device *phydev)
+{
+ if (bp->converter_phy.fix_mac_speed)
+ bp->converter_phy.fix_mac_speed(&bp->converter_phy,
+ phydev->speed);
+}
+
/**
* macb_set_tx_clk() - Set a clock to a new frequency
* @clk Pointer to the clock to change
@@ -329,6 +338,7 @@ static void macb_handle_link_change(struct net_device *dev)
reg |= GEM_BIT(GBE);

macb_or_gem_writel(bp, NCFGR, reg);
+ macb_hw_fix_mac_speed(bp, phydev);

bp->speed = phydev->speed;
bp->duplex = phydev->duplex;
@@ -2886,6 +2896,7 @@ static int macb_probe(struct platform_device *pdev)
int (*init)(struct platform_device *) = macb_init;
struct device_node *np = pdev->dev.of_node;
struct device_node *phy_node;
+ struct device_node *child_node, *np1;
const struct macb_config *macb_config = NULL;
struct clk *pclk, *hclk = NULL, *tx_clk = NULL;
unsigned int queue_mask, num_queues;
@@ -3011,6 +3022,16 @@ static int macb_probe(struct platform_device *pdev)
goto err_out_free_netdev;

phydev = bp->phy_dev;
+ child_node = of_get_next_child(np, NULL);
+ for_each_child_of_node(child_node, np1) {
+ if (of_device_is_compatible(np1, "xlnx,gmiitorgmii")) {
+ bp->converter_phy.dev = dev;
+ bp->converter_phy.mii_bus = bp->mii_bus;
+ bp->converter_phy.mdio_write = macb_mdio_write;
+ bp->converter_phy.platform_data = bp->pdev->dev.of_node;
+ gmii2rgmii_phyprobe(&bp->converter_phy);
+ }
+ }

netif_carrier_off(dev);

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 8a13824..414005c 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -846,6 +846,7 @@ struct macb {
unsigned int jumbo_max_len;

u32 wol;
+ struct gmii2rgmii converter_phy;
};

static inline bool macb_is_gem(struct macb *bp)
--
2.1.2

[Kedareswara rao Appana]

This patch updates the macb device-tree binding doc for adding
Support for gmiitorgmii converter support.

Signed-off-by: Kedareswara rao Appana <app...@xilinx.com>
---
Changes for v2:

--> New patch.

Documentation/devicetree/bindings/net/macb.txt | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index b5a42df..0d497a7 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -23,6 +23,10 @@ Required properties:
Optional elements: 'tx_clk'
- clocks: Phandles to input clocks.

+Optional properties:
+- gmii2rgmii-phy-handle: phy-handle for gmiitorgmii converter.
+ See xilinx_gmii2rgmii.txt file in the same directory.
+
Optional properties for PHY child node:
- reset-gpios : Should specify the gpio for phy reset
- magic-packet : If present, indicates that the hardware supports waking
--
2.1.2

[Kedareswara rao Appana]

Device-tree binding documentation for xilinx gmiitorgmii converter.

Signed-off-by: Kedareswara rao Appana <app...@xilinx.com>
---
Changes for v2:
--> New patch.

.../devicetree/bindings/net/xilinx_gmii2rgmii.txt | 31 ++++++++++++++++++++++
1 file changed, 31 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..d11e259
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
@@ -0,0 +1,31 @@
+* XILINX GMIITORGMII Converter Driver
+
+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.
+
+Required properties:
+ - compatible : Should be "xlnx,gmiitorgmii"
+ - reg : The ID number for the phy, usually a small integer
+
+Example:
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ethernet-phy@0 {
+ ......
+ };
+ gmii_to_rgmii: gmii_to_rgmii@8 {
+ compatible = "xlnx,gmiitorgmii";
+ reg = <8>;
+ };
+ };
--
2.1.2

[Nicolas Ferre]

Here, header of the file seems needed.

> +#ifndef _GMII2RGMII_H
> +#define _GMII2RGMII_H
> +
> +#include <linux/of.h>
> +#include <linux/phy.h>
> +#include <linux/mii.h>
> +
> +#define XILINX_GMII2RGMII_FULLDPLX BMCR_FULLDPLX
> +#define XILINX_GMII2RGMII_SPEED1000 BMCR_SPEED1000
> +#define XILINX_GMII2RGMII_SPEED100 BMCR_SPEED100
> +#define XILINX_GMII2RGMII_REG_NUM 0x10
> +
> +struct gmii2rgmii {
> + struct net_device *dev;
> + struct mii_bus *mii_bus;
> + struct phy_device *gmii2rgmii_phy_dev;
> + void *platform_data;
> + int (*mdio_write)(struct mii_bus *bus, int mii_id, int reg,
> + u16 val);
> + void (*fix_mac_speed)(struct gmii2rgmii *xphy, unsigned int speed);
> +};
> +
> +extern int gmii2rgmii_phyprobe(struct gmii2rgmii *xphy);
> +#endif

I see a compilation issue here:

You should provide a way to have this function even if the
NET_VENDOR_XILINX config option is not selected (test to compile with
the sama5_defconfig and you'll see).

What about making this function void in case of !XILINX?

(so, NACK for the series as it is).
Bye,
--
Nicolas Ferre

[Appana Durga Kedareswara Rao]

Hi Nicolas,

Thanks for the review...

> > diff --git a/include/linux/xilinx_gmii2rgmii.h
> > b/include/linux/xilinx_gmii2rgmii.h
> > new file mode 100644
> > index 0000000..b328ee7
> > --- /dev/null
> > +++ b/include/linux/xilinx_gmii2rgmii.h
> > @@ -0,0 +1,24 @@
>
>
> Here, header of the file seems needed.

Sure will fix in the next version...

>
> > +#ifndef _GMII2RGMII_H
> > +#define _GMII2RGMII_H
> > +
> > +#include <linux/of.h>
> > +#include <linux/phy.h>
> > +#include <linux/mii.h>
> > +
> > +#define XILINX_GMII2RGMII_FULLDPLX BMCR_FULLDPLX
> > +#define XILINX_GMII2RGMII_SPEED1000 BMCR_SPEED1000
> > +#define XILINX_GMII2RGMII_SPEED100 BMCR_SPEED100
> > +#define XILINX_GMII2RGMII_REG_NUM 0x10
> > +
> > +struct gmii2rgmii {
> > + struct net_device *dev;
> > + struct mii_bus *mii_bus;
> > + struct phy_device *gmii2rgmii_phy_dev;
> > + void *platform_data;
> > + int (*mdio_write)(struct mii_bus *bus, int mii_id, int reg,
> > + u16 val);
> > + void (*fix_mac_speed)(struct gmii2rgmii *xphy, unsigned int speed);
> > +};
> > +

> > +extern int gmii2rgmii_phyprobe(struct gmii2rgmii *xphy); #endif

>
> I see a compilation issue here:
>
> You should provide a way to have this function even if the NET_VENDOR_XILINX
> config option is not selected (test to compile with the sama5_defconfig and
> you'll see).

Ok will fix in the next version...

>
> What about making this function void in case of !XILINX?

This is one way to get rid of compilation error. Changes will be look like below

#ifdef CONFIG_NET_VENDOR_XILINX
extern int gmii2rgmii_phyprobe(struct gmii2rgmii *xphy);
#else
extern void gmii2rgmii_phyprobe(struct gmii2rgmii *xphy);
void gmii2rgmii_phyprobe(struct gmii2rgmii *xphy)
{
}
#endif
For me the changes are looking odd...

Other possible ways
1) Put a config check around phyprobe api in the macb driver.
#ifdef CONFIG_XILINX_GMII2RGMII
gmii2rgmii_phyprobe(&bp->converter_phy);
#endif
2) Select NET_VENDOR_XILINX in the macb Kconfig
@ -22,6 +22,7 @@ config MACB
tristate "Cadence MACB/GEM support"
depends on HAS_DMA
select PHYLIB
+ select NET_VENDOR_XILINX

Please let me know which one you prefer will fix that and will post v3...

Regards,
Kedar.

[Nicolas Ferre]

You may need to have:
#if defined(CONFIG_NET_VENDOR_XILINX) && defined(CONFIG_XILINX_GMII2RGMII)

> extern int gmii2rgmii_phyprobe(struct gmii2rgmii *xphy);
> #else
> extern void gmii2rgmii_phyprobe(struct gmii2rgmii *xphy);

No need for the line above...

> void gmii2rgmii_phyprobe(struct gmii2rgmii *xphy)
> {
> }

On one single line, like:

static inline void gmii2rgmii_phyprobe(struct gmii2rgmii *xphy) { }

> #endif
> For me the changes are looking odd...

For me, it's okay...

>
> Other possible ways
> 1) Put a config check around phyprobe api in the macb driver.
> #ifdef CONFIG_XILINX_GMII2RGMII
> gmii2rgmii_phyprobe(&bp->converter_phy);
> #endif

Nope!

> 2) Select NET_VENDOR_XILINX in the macb Kconfig
> @ -22,6 +22,7 @@ config MACB
> tristate "Cadence MACB/GEM support"
> depends on HAS_DMA
> select PHYLIB
> + select NET_VENDOR_XILINX



> Please let me know which one you prefer will fix that and will post v3...

First one with my changes is the best. But maybe wait for more feedback...

Bye,
--
Nicolas Ferre

[Appana Durga Kedareswara Rao]

Hi Nicolas,

> >
> > #ifdef CONFIG_NET_VENDOR_XILINX
>
> You may need to have:
> #if defined(CONFIG_NET_VENDOR_XILINX) &&
> defined(CONFIG_XILINX_GMII2RGMII)
>
> > extern int gmii2rgmii_phyprobe(struct gmii2rgmii *xphy); #else extern
> > void gmii2rgmii_phyprobe(struct gmii2rgmii *xphy);
>
> No need for the line above...
>
> > void gmii2rgmii_phyprobe(struct gmii2rgmii *xphy) { }
>
> On one single line, like:
>
> static inline void gmii2rgmii_phyprobe(struct gmii2rgmii *xphy) { }
>
> > #endif
> > For me the changes are looking odd...
>
> For me, it's okay...

Ok will fix as you suggested...

>
> >
> > Other possible ways
> > 1) Put a config check around phyprobe api in the macb driver.
> > #ifdef CONFIG_XILINX_GMII2RGMII
> > gmii2rgmii_phyprobe(&bp->converter_phy);
> > #endif
>
> Nope!
>
> > 2) Select NET_VENDOR_XILINX in the macb Kconfig @ -22,6 +22,7 @@
> > config MACB
> > tristate "Cadence MACB/GEM support"
> > depends on HAS_DMA
> > select PHYLIB
> > + select NET_VENDOR_XILINX
>
>
>
> > Please let me know which one you prefer will fix that and will post v3...
>
> First one with my changes is the best. But maybe wait for more feedback...

Ok sure will wait and will post next version with your suggestion in case of no comments...

Regards,
Kedar.

>
> Bye,
> --
> Nicolas Ferre

[Andrew Lunn]

Hi Kedareswara

So looking at the device tree, you have the gmiitorgmii as an mdio
device. It will get probed as an mdio device, and from that you know
the address on the bus. However, your driver does not actually do
this. xilinx_gmii2rgmii.c is just a library of two functions, and does
not use any of this device tree information. You device tree binding
is completely bogus.

What i think is a much more logical structure, and fits the hardware,
which is what DT is all about, is to make your driver an mdio driver.
Also, have a phy-handle pointing to the PHY in the gmii_to_rgmii node.
You then no longer need the exported gmii2rgmii_phyprobe() function.

Next, you want gmiitorgmii driver to register a phy. The MAC driver
can then look this up using phy-handle:

mdio {
#address-cells = <1>;
#size-cells = <0>;

phy: ethernet-phy@0 {
reg = <0>;
};

gmii_to_rgmii: gmii-to-rgmii@8 {
compatible = "xlnx,gmiitorgmii";
reg = <8>;
phy-handle = <&phy>;
};
};

macb0: ethernet@fffc4000 {
compatible = "cdns,at32ap7000-macb";
reg = <0xfffc4000 0x4000>;
interrupts = <21>;
phy-mode = "rmii";
phy-handle = <&gmii_to_rgmii>
local-mac-address = [3a 0e 03 04 05 06];
clock-names = "pclk", "hclk", "tx_clk";
clocks = <&clkc 30>, <&clkc 30>, <&clkc 13>;
ethernet-phy@1 {
reg = <0x1>;
reset-gpios = <&pioE 6 1>;
};
};

This description is the same as the figure in the data sheet. You have
the gmii_to_rgmii which passes through to the real PHY.

The phy device that gmiitorgmii registers needs to pass through all
its operations to the real PHY. The exception is read_status()
function. You want to wrap this, so that after it completes and you
know the speed the PHY is using, you set the same speed in your
gmiitorgmii device.

Everything then becomes transparent. There is no need to change the
MAC driver, and you have a generic solution which will work with any
MAC and PHY.

Andrew

[Punnaiah Choudary Kalluri]

Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Monday, July 04, 2016 7:35 PM
> To: Appana Durga Kedareswara Rao <app...@xilinx.com>
> Cc: rob...@kernel.org; mark.r...@arm.com; Michal Simek
> <mic...@xilinx.com>; Soren Brinkmann <sor...@xilinx.com>; Punnaiah
> Choudary Kalluri <pun...@xilinx.com>; nicola...@atmel.com;
> f.fai...@gmail.com; Anirudha Sarangi <ani...@xilinx.com>; Harini
> Katakam <har...@xilinx.com>; net...@vger.kernel.org;
> devic...@vger.kernel.org; linux-ar...@lists.infradead.org; linux-
> ker...@vger.kernel.org; Appana Durga Kedareswara Rao
> <app...@xilinx.com>
> Subject: Re: [RFC PATCH v2 1/4] Documentation: DT: net: Add Xilinx

> gmiitorgmii converter device tree binding documentation
>

Thanks for your inputs initially we too thought the similar implementation
But the GMII2RGMII converter contains only one register and it is
not compatible to the standard ethernet MII interface. Also it doesn't have
a standard VID and PID registers So, during the mdio bus scan, this device will
not appear. When macb driver calls the gmii2rgmii phy-handle through
phy_connect_direct/of_phy_connect, these calls fail for the above reason.
So, we come up with the current implementation.

Please suggest if you have any other solutions in mind or if our understanding
is wrong for the current approach that you suggested.

Regards,
Punnaiah

[Andrew Lunn]

Hi Punnaiah

Use missed some subtlety in my description. I did not call the
GMII2RGMII a PHY device, i called it an MDIO device. These are
different things. Go look at the MDIO subsystem to figure out the
difference.

Andrew

[Punnaiah Choudary Kalluri]

> -----Original Message-----
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Wednesday, July 06, 2016 7:51 PM
> To: Punnaiah Choudary Kalluri <pun...@xilinx.com>
> Cc: Appana Durga Kedareswara Rao <app...@xilinx.com>;
> rob...@kernel.org; mark.r...@arm.com; Michal Simek
> <mic...@xilinx.com>; Soren Brinkmann <sor...@xilinx.com>;
> nicola...@atmel.com; f.fai...@gmail.com; Anirudha Sarangi
> <ani...@xilinx.com>; Harini Katakam <har...@xilinx.com>;

> net...@vger.kernel.org; devic...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; linux-...@vger.kernel.org

> Subject: Re: [RFC PATCH v2 1/4] Documentation: DT: net: Add Xilinx
> gmiitorgmii converter device tree binding documentation
>

Hi Andrew

Got it. Thanks.

Punnaiah

> Andrew

[Appana Durga Kedareswara Rao]

Hi Andrew,

Thanks for the inputs...

I tried to implement the driver as you suggested but there are few limitations in this
Implementation please correct me if my understanding is wrong...

The device-tree will look like below...

mdio {
#address-cells = <1>;
#size-cells = <0>;
phy: ethernet-phy@0 {
reg = <0>;
};
gmii_to_rgmii: gmii-to-rgmii@8 {
compatible = "xlnx,gmiitorgmii";
reg = <8>;
phy-handle = <&phy>;
};
};

The MAC driver does the below.
---> It creates a MDIO device for the gmii_to_rgmii node.
---> It creates a PHY device for the External PHY node.

The GMII2RGMII driver does the below.
1) It registers the gmii_to_rgmii node as a MDIO driver.
It contains the external phy as a phy-handle
2) Register a PHY Driver.
---> Register it as a phy driver (phy_register_driver):
For this implementation the converter won't have any VID or DID(PHY register 2 and 3)
---> Register phy using of_phy_connect call:
For this implementation needed a netdev pointer to pass it to the of_phy_connect call.
(Need to allocate a network device during probe and need to register it as netdev)

For the 2nd one implement I am facing above limitations...
Please correct me if my understanding is wrong...

Regards,
Kedar.

[Andrew Lunn]

Hi Appana

Here is roughly what i was thinking:

struct priv {
phy_device *master;
phy_device *slave;
struct phy_driver *slave_drv;
};

phy_status_clone(phy_device *master, phy_device *slave)
{
master->speed = slave->speed;
master->duplex = slave->duplex;
master->pause = slave->pause;
}

read_status(struct phy_device *phydev)
{
struct priv *priv = phydev->priv;

/* Get the status from the slave, and duplicate in into the
* master */
slave_drv->read_status(priv->slave);
phy_status_clone(priv->master, priv->slave);

/* Update the gmiitorgmii with the current link parameters */
update_link(master);
}

config_init(struct phy_device *phydev)
{
struct priv *priv = phydev->priv;

/* Configure the slave, and duplicate in into the master */
slave_drv->config_init(priv->slave);
phy_status_clone(priv->master, priv->slave);
}

struct phy_driver master_drv = {
.read_status = read_status,
.config_init = config_init,
.soft_reset = ...
.suspend = ...
};

probe(mdio_device *mdio)
{
struct priv *priv = devm_alloc();

/* Use the phy-handle property to find the slave phy */
node_phy = of_parse_phandle(mdio->of_node, "phy", 0);
priv->slave = of_phy_find_device(node_phy);

/* Create the master phy on the control address. Use the phy
ID from the slave. */
priv->master = phy_device_create(mdio->bus, mdio->addr,
phy->slave->phy_id,
phy->slave->is_c45,
phy->slave->c45_ids);

slave_dev_drv = phydev->mdio.dev.driver;
priv->slave_drv = to_phy_driver(slave_dev_drv);
priv->master->mdio.dev.driver = master_drv;
}

It would however be nice to only have one phydev structure, so you are
not copying status and settings backwards and forwards from one to the
other all the time, and need a wrapper for every function in
phy_driver. Studying the structures a bit, that might be possible. You
would then only need to wrap the read_status(), so that when the link
speed/duplex changes, you can configure the converter as appropriate.

Andrew

[Florian Fainelli]

The key here is really that except for the phy_driver::read_status
callback, we want to defer every operation to the slave (full MDIO
register range compatible) PHY.

> }
>
> It would however be nice to only have one phydev structure, so you are
> not copying status and settings backwards and forwards from one to the
> other all the time, and need a wrapper for every function in
> phy_driver. Studying the structures a bit, that might be possible. You
> would then only need to wrap the read_status(), so that when the link
> speed/duplex changes, you can configure the converter as appropriate.

Agreed.
--
Florian

[Appana Durga Kedareswara Rao]

Hi Florian,

Will fix it in the next version...

>
> > }
> >
> > It would however be nice to only have one phydev structure, so you are
> > not copying status and settings backwards and forwards from one to the
> > other all the time, and need a wrapper for every function in
> > phy_driver. Studying the structures a bit, that might be possible. You
> > would then only need to wrap the read_status(), so that when the link
> > speed/duplex changes, you can configure the converter as appropriate.
>
> Agreed.

Will implement as suggested by Andrew...

Regards,
Kedar.

> --
> Florian

[Kedareswara rao Appana]

Device-tree binding documentation for xilinx gmiitorgmii converter.

Signed-off-by: Kedareswara rao Appana <app...@xilinx.com>
---

Changes for v3:
--> None.

Changes for v2:
--> New patch.

.../devicetree/bindings/net/xilinx_gmii2rgmii.txt | 32 ++++++++++++++++++++++
1 file changed, 32 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..79fa504
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
@@ -0,0 +1,32 @@
+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.
+
+Required properties:

+ - compatible : Should be "xilinx,gmiitorgmii"

+ - reg : The ID number for the phy, usually a small integer
+
+Example:
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ethernet-phy@0 {
+ ......
+ };
+ gmii_to_rgmii: gmii_to_rgmii@8 {

+ compatible = "xilinx,gmiitorgmii";

+ reg = <8>;
+ };
+ };

--
2.1.2

[Rob Herring]

Perhaps xilinx,gmii-to-rgmii.

This needs some sort of version information in the compatible string.

> + - reg : The ID number for the phy, usually a small integer
> +
> +Example:
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + ethernet-phy@0 {
> + ......
> + };
> + gmii_to_rgmii: gmii_to_rgmii@8 {

Don't use underscores in node names.

[Appana Durga Kedareswara Rao]

Hi Rob,

Thanks for the review...

<Snip>

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

> > +
> > +Required properties:
> > + - compatible : Should be "xilinx,gmiitorgmii"
>
> Perhaps xilinx,gmii-to-rgmii.

Sure...

>
> This needs some sort of version information in the compatible string.

Sure will fix in the next version...

>

> > + - reg : The ID number for the phy, usually a small integer
> > +
> > +Example:
> > + mdio {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + ethernet-phy@0 {
> > + ......
> > + };
> > + gmii_to_rgmii: gmii_to_rgmii@8 {
>
> Don't use underscores in node names.

Ok will fix in the next version...

Regards,
Kedar.

[Kedareswara rao Appana]

Device-tree binding documentation for xilinx gmiitorgmii converter.

Signed-off-by: Kedareswara rao Appana <app...@xilinx.com>
---

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..453680d
--- /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 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

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

Signed-off-by: Kedareswara rao Appana <app...@xilinx.com>
---

Thanks a lot Andrew for your inputs.
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 | 125 ++++++++++++++++++++++++++++++++++++
3 files changed, 134 insertions(+)
create mode 100644 drivers/net/phy/xilinx_gmii2rgmii.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 47a6434..28150f8 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -307,6 +307,14 @@ config MDIO_XGENE
This module provides a driver for the MDIO busses found in the
APM X-Gene SoC's.

+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 534dfa7..5d38f5a 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -49,3 +49,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..8e4a0f3
--- /dev/null
+++ b/drivers/net/phy/xilinx_gmii2rgmii.c
@@ -0,0 +1,125 @@

+/* Xilinx GMII2RGMII Converter driver
+ *
+ * Copyright (C) 2016 Xilinx, Inc.
+ *
+ * Author: Kedareswara rao Appana <app...@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 BMCR_SPEED10 0x00
+
+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);
+
+ 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;
+ int ret;
+
+ 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");
+ ret = -ENODEV;
+ goto out;
+ }
+
+ priv->phy_dev = of_phy_find_device(phy_node);
+ if (!priv->phy_dev) {
+ ret = -EPROBE_DEFER;
+ dev_info(dev, "Couldn't find phydev\n");
+ goto out;
+ }
+
+ 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;
+out:
+ return ret;
+}
+
+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

[Michal Simek]

Indentation in this example is quite weird. You are mixing tabs and spaces.

Thanks,
Michal

[Appana Durga Kedareswara Rao]

Hi Michal,

> > +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 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>;
> > + };
>
> Indentation in this example is quite weird. You are mixing tabs and spaces.

Sure will fix in the next version...

Regards,
Kedar.

>
> Thanks,
> Michal

[Punnaiah Choudary Kalluri]

Hi Kedar,

> -----Original Message-----
> From: Kedareswara rao Appana [mailto:appana.d...@xilinx.com]
> Sent: Monday, August 08, 2016 12:45 PM
> To: rob...@kernel.org; mark.r...@arm.com; Michal Simek
> <mic...@xilinx.com>; Soren Brinkmann <sor...@xilinx.com>; Appana
> Durga Kedareswara Rao <app...@xilinx.com>; f.fai...@gmail.com;
> and...@lunn.ch; Punnaiah Choudary Kalluri <pun...@xilinx.com>;
> Anirudha Sarangi <ani...@xilinx.com>
> Cc: devic...@vger.kernel.org; linux-ar...@lists.infradead.org; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org
> Subject: [RFC PATCH v4 2/2] net: phy: Add gmiitorgmii converter support

>
> 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.
>
> Signed-off-by: Kedareswara rao Appana <app...@xilinx.com>
> ---
> Thanks a lot Andrew for your inputs.
> 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
>

> +
> +#define XILINX_GMII2RGMII_REG 0x10
> +#define BMCR_SPEED10 0x00

Move this macro to mii.h

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

Since its read and then modify, you should mask the speed field
With zero and then write new value.

Since there is no resource cleaning here, you could consider
Return from this function in above conditions and avoid goto here.

Regards,
Punnaiah

[Kedareswara rao Appana]

Device-tree binding documentation for xilinx gmiitorgmii converter.

Signed-off-by: Kedareswara rao Appana <app...@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.
+
+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

[Appana Durga Kedareswara Rao]

Hi Punnaiah,

Thanks for the review...

> > +
> > +#define XILINX_GMII2RGMII_REG 0x10
> > +#define BMCR_SPEED10 0x00
>
> Move this macro to mii.h

Sure will fix in the next version...

>

> > +
> > +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);
> > +
>
> Since its read and then modify, you should mask the speed field With zero and
> then write new value.
>

Sure will fix in the next version...

>

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

Sure will fix in the next version...

Regards,
Kedar.

> Regards,
> Punnaiah

[Florian Fainelli]

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

[Kedareswara rao Appana]

Device-tree binding documentation for xilinx gmiitorgmii converter.

Signed-off-by: Kedareswara rao Appana <app...@xilinx.com>
---

Changes for v6:
---> Removed mdio description as suggested by Florian.

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 | 35 ++++++++++++++++++++++
1 file changed, 35 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..038dda4
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
@@ -0,0 +1,35 @@

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

+This converter sits between the ethernet MAC and the external phy.
+MAC <==> GMII2RGMII <==> RGMII_PHY
+

+For more details about mdio please refer phy.txt file in the same directory.
+

+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

[Appana Durga Kedareswara Rao]

Hi Florian,

Thanks for the review...

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

[Rob Herring]

On Wed, Aug 10, 2016 at 11:20:07AM +0530, Kedareswara rao Appana wrote:
> Device-tree binding documentation for xilinx gmiitorgmii converter.
>
> Signed-off-by: Kedareswara rao Appana <app...@xilinx.com>
> ---
> Changes for v6:
> ---> Removed mdio description as suggested by Florian.
> 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 | 35 ++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt

Acked-by: Rob Herring <ro...@kernel.org>