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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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