diff mbox

[RFC,v2,1/4] Documentation: DT: net: Add Xilinx gmiitorgmii converter device tree binding documentation

Message ID 1467623084-15471-2-git-send-email-appanad@xilinx.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Appana Durga Kedareswara rao July 4, 2016, 9:04 a.m. UTC
Device-tree binding documentation for xilinx gmiitorgmii converter.

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

Comments

Andrew Lunn July 4, 2016, 2:04 p.m. UTC | #1
On Mon, Jul 04, 2016 at 02:34:41PM +0530, Kedareswara rao Appana wrote:
> Device-tree binding documentation for xilinx gmiitorgmii converter.
> 
> Signed-off-by: Kedareswara rao Appana <appanad@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>;
> +                };
> +        };

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 July 6, 2016, 2:12 p.m. UTC | #2
Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Monday, July 04, 2016 7:35 PM
> To: Appana Durga Kedareswara Rao <appanad@xilinx.com>
> Cc: robh+dt@kernel.org; mark.rutland@arm.com; Michal Simek
> <michals@xilinx.com>; Soren Brinkmann <sorenb@xilinx.com>; Punnaiah
> Choudary Kalluri <punnaia@xilinx.com>; nicolas.ferre@atmel.com;
> f.fainelli@gmail.com; Anirudha Sarangi <anirudh@xilinx.com>; Harini
> Katakam <harinik@xilinx.com>; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Appana Durga Kedareswara Rao
> <appanad@xilinx.com>
> Subject: Re: [RFC PATCH v2 1/4] Documentation: DT: net: Add Xilinx
> gmiitorgmii converter device tree binding documentation
> 
> On Mon, Jul 04, 2016 at 02:34:41PM +0530, Kedareswara rao Appana wrote:
> > Device-tree binding documentation for xilinx gmiitorgmii converter.
> >
> > Signed-off-by: Kedareswara rao Appana <appanad@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>;
> > +                };
> > +        };
> 
> 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>;
>                 };
>        };


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

> 
>         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
Andrew Lunn July 6, 2016, 2:21 p.m. UTC | #3
> > 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>;
> >                 };
> >        };
> 
> 
> 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.

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 July 6, 2016, 2:51 p.m. UTC | #4
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Wednesday, July 06, 2016 7:51 PM
> To: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
> Cc: Appana Durga Kedareswara Rao <appanad@xilinx.com>;
> robh+dt@kernel.org; mark.rutland@arm.com; Michal Simek
> <michals@xilinx.com>; Soren Brinkmann <sorenb@xilinx.com>;
> nicolas.ferre@atmel.com; f.fainelli@gmail.com; Anirudha Sarangi
> <anirudh@xilinx.com>; Harini Katakam <harinik@xilinx.com>;
> netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH v2 1/4] Documentation: DT: net: Add Xilinx
> gmiitorgmii converter device tree binding documentation
> 
> > > 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>;
> > >                 };
> > >        };
> >
> >
> > 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.
> 
> 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.
> 
Hi Andrew

Got it. Thanks.

Punnaiah

> 	Andrew
Appana Durga Kedareswara rao July 26, 2016, 3:09 p.m. UTC | #5
Hi Andrew,

	Thanks for the inputs...

> >
> > > > 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>;
> > > >                 };
> > > >        };
> > >
> > >
> > > 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.

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 July 27, 2016, 8:05 a.m. UTC | #6
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 Aug. 4, 2016, 3:42 a.m. UTC | #7
On 27/07/2016 01:05, Andrew Lunn wrote:
> 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;

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 Aug. 4, 2016, 10:34 a.m. UTC | #8
Hi Florian,

> 
> On 27/07/2016 01:05, Andrew Lunn wrote:
> > 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;
> 
> 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.

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
diff mbox

Patch

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