[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <971c966a-5335-310b-0a1a-3f1b7ee2f841@gmail.com>
Date: Wed, 3 Aug 2016 20:42:08 -0700
From: Florian Fainelli <f.fainelli@...il.com>
To: Andrew Lunn <andrew@...n.ch>,
Appana Durga Kedareswara Rao <appana.durga.rao@...inx.com>
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
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
Powered by blists - more mailing lists