[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <C246CAC1457055469EF09E3A7AC4E11A4A5DCC2F@XAP-PVEXMBX01.xlnx.xilinx.com>
Date: Thu, 4 Aug 2016 10:34:30 +0000
From: Appana Durga Kedareswara Rao <appana.durga.rao@...inx.com>
To: Florian Fainelli <f.fainelli@...il.com>,
Andrew Lunn <andrew@...n.ch>
CC: "robh+dt@...nel.org" <robh+dt@...nel.org>,
"mark.rutland@....com" <mark.rutland@....com>,
Michal Simek <michals@...inx.com>,
Soren Brinkmann <sorenb@...inx.com>,
"nicolas.ferre@...el.com" <nicolas.ferre@...el.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Punnaiah Choudary Kalluri <punnaia@...inx.com>
Subject: RE: [RFC PATCH v2 1/4] Documentation: DT: net: Add Xilinx
gmiitorgmii converter device tree binding documentation
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
Powered by blists - more mailing lists