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
