Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure
Hi Andrew,
On Tue, Aug 13, 2019 at 9:40 PM Andrew Lunn wrote:
>
> > > The kernel does have a few helper, spi_get_drvdata, pci_get_drvdata,
> > > hci_get_drvdata. So maybe had add phydev_get_drvdata(struct phy_device
> > > *phydev)?
> >
> > Maybe phydev_mdio_get_drvdata? Because the driver data member available is
> > phydev->mdio.dev.driver_data.
>
> I still prefer phydev_get_drvdata(). It fits with the X_get_drvdata()
> pattern, where X is the type of parameter passed to the call, spi,
> pci, hci.
>
> We can also add mdiodev_get_drvdata(mdiodev). A few DSA drivers could
> use that.
Sorry for the late reply. I just sent a v2 adding
mdiodev_get/set_drvdata helpers
and using them in gmii2rgmii driver.
I did not add a corresponding phydev helper because there is no "struct dev" in
"struct phy_device" and I dint know if there were any users to add the member
and then a helper for driver data. Also,
strutct phy_device { struct mdio_device { struct device }}
is already available and it seemed logical to use that field to
set/get driver data
for gmii2rgmii. Please let me know if v2 is okay.
Regards,
Harini
Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure
> > The kernel does have a few helper, spi_get_drvdata, pci_get_drvdata, > > hci_get_drvdata. So maybe had add phydev_get_drvdata(struct phy_device > > *phydev)? > > Maybe phydev_mdio_get_drvdata? Because the driver data member available is > phydev->mdio.dev.driver_data. I still prefer phydev_get_drvdata(). It fits with the X_get_drvdata() pattern, where X is the type of parameter passed to the call, spi, pci, hci. We can also add mdiodev_get_drvdata(mdiodev). A few DSA drivers could use that. Andrew
Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure
Hi Andrew, On Tue, Aug 13, 2019 at 6:54 PM Andrew Lunn wrote: > > On Tue, Aug 13, 2019 at 04:46:40PM +0530, Harini Katakam wrote: > > Hi Andrew, > > > > On Thu, Aug 1, 2019 at 9:36 AM Andrew Lunn wrote: > > > > > > On Wed, Jul 31, 2019 at 03:06:19PM +0530, Harini Katakam wrote: > > > > Use the priv field in mdio device structure instead of the one in > > > > phy device structure. The phy device priv field may be used by the > > > > external phy driver and should not be overwritten. > > > > > > Hi Harini > > > > > > I _think_ you could use dev_set_drvdata(>dev) in > > > xgmiitorgmii_probe() and > > > dev_get_drvdata(>mdiomdio.dev) in _read_status() > > > > Thanks for the review. This works if I do: > > dev_set_drvdata(>phy_dev->mdio.dev->dev) in probe > > and then > > dev_get_drvdata(>mdio.dev) in _read_status() > > > > i.e mdiodev in gmii2rgmii probe and priv->phy_dev->mdio are not the same. > > > > If this is acceptable, I can send a v2. > > Hi Harini > > I think this is better, making use of the central driver > infrastructure, rather than inventing something new. Ok sure. > > The kernel does have a few helper, spi_get_drvdata, pci_get_drvdata, > hci_get_drvdata. So maybe had add phydev_get_drvdata(struct phy_device > *phydev)? Maybe phydev_mdio_get_drvdata? Because the driver data member available is phydev->mdio.dev.driver_data. Regards, Harini
Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure
On Tue, Aug 13, 2019 at 04:46:40PM +0530, Harini Katakam wrote: > Hi Andrew, > > On Thu, Aug 1, 2019 at 9:36 AM Andrew Lunn wrote: > > > > On Wed, Jul 31, 2019 at 03:06:19PM +0530, Harini Katakam wrote: > > > Use the priv field in mdio device structure instead of the one in > > > phy device structure. The phy device priv field may be used by the > > > external phy driver and should not be overwritten. > > > > Hi Harini > > > > I _think_ you could use dev_set_drvdata(>dev) in > > xgmiitorgmii_probe() and > > dev_get_drvdata(>mdiomdio.dev) in _read_status() > > Thanks for the review. This works if I do: > dev_set_drvdata(>phy_dev->mdio.dev->dev) in probe > and then > dev_get_drvdata(>mdio.dev) in _read_status() > > i.e mdiodev in gmii2rgmii probe and priv->phy_dev->mdio are not the same. > > If this is acceptable, I can send a v2. Hi Harini I think this is better, making use of the central driver infrastructure, rather than inventing something new. The kernel does have a few helper, spi_get_drvdata, pci_get_drvdata, hci_get_drvdata. So maybe had add phydev_get_drvdata(struct phy_device *phydev)? Thanks Andrew
Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure
Hi Andrew, On Thu, Aug 1, 2019 at 9:36 AM Andrew Lunn wrote: > > On Wed, Jul 31, 2019 at 03:06:19PM +0530, Harini Katakam wrote: > > Use the priv field in mdio device structure instead of the one in > > phy device structure. The phy device priv field may be used by the > > external phy driver and should not be overwritten. > > Hi Harini > > I _think_ you could use dev_set_drvdata(>dev) in > xgmiitorgmii_probe() and > dev_get_drvdata(>mdiomdio.dev) in _read_status() Thanks for the review. This works if I do: dev_set_drvdata(>phy_dev->mdio.dev->dev) in probe and then dev_get_drvdata(>mdio.dev) in _read_status() i.e mdiodev in gmii2rgmii probe and priv->phy_dev->mdio are not the same. If this is acceptable, I can send a v2. Regards, Harini
Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure
On Wed, Jul 31, 2019 at 03:06:19PM +0530, Harini Katakam wrote: > Use the priv field in mdio device structure instead of the one in > phy device structure. The phy device priv field may be used by the > external phy driver and should not be overwritten. Hi Harini I _think_ you could use dev_set_drvdata(>dev) in xgmiitorgmii_probe() and dev_get_drvdata(>mdiomdio.dev) in _read_status() Andrew
[PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure
Use the priv field in mdio device structure instead of the one in
phy device structure. The phy device priv field may be used by the
external phy driver and should not be overwritten.
Signed-off-by: Harini Katakam
Reviewed-by: Radhey Shyam Pandey
---
drivers/net/phy/xilinx_gmii2rgmii.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c
b/drivers/net/phy/xilinx_gmii2rgmii.c
index 2d14493..ba31b5c3 100644
--- a/drivers/net/phy/xilinx_gmii2rgmii.c
+++ b/drivers/net/phy/xilinx_gmii2rgmii.c
@@ -29,7 +29,7 @@ struct gmii2rgmii {
static int xgmiitorgmii_read_status(struct phy_device *phydev)
{
- struct gmii2rgmii *priv = phydev->priv;
+ struct gmii2rgmii *priv = phydev->mdio.priv;
struct mii_bus *bus = priv->mdio->bus;
int addr = priv->mdio->addr;
u16 val = 0;
@@ -90,7 +90,7 @@ static int xgmiitorgmii_probe(struct mdio_device *mdiodev)
memcpy(>conv_phy_drv, priv->phy_dev->drv,
sizeof(struct phy_driver));
priv->conv_phy_drv.read_status = xgmiitorgmii_read_status;
- priv->phy_dev->priv = priv;
+ priv->phy_dev->mdio.priv = priv;
priv->phy_dev->drv = >conv_phy_drv;
return 0;
--
2.7.4
