Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1451105 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH v2 1/4] Documentation: DT: net: Add Xilinx  gmiitorgmii converter device tree binding documentation Andrew Lunn <andrew@lunn.ch> - 2016-07-27 10:10 +0200
    Re: [RFC PATCH v2 1/4] Documentation: DT: net: Add Xilinx gmiitorgmii  converter device tree binding documentation Florian Fainelli <f.fainelli@gmail.com> - 2016-08-04 05:50 +0200

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

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

[toc] | [next] | [standalone]

#1456142 — 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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web