netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix GMII2RGMII private field
@ 2019-07-31  9:36 Harini Katakam
  2019-07-31  9:36 ` [PATCH 1/2] include: mdio: Add private field to mdio structure Harini Katakam
  2019-07-31  9:36 ` [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure Harini Katakam
  0 siblings, 2 replies; 9+ messages in thread
From: Harini Katakam @ 2019-07-31  9:36 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, davem
  Cc: michal.simek, netdev, linux-arm-kernel, linux-kernel,
	harinikatakamlinux, harini.katakam, radhey.shyam.pandey

Fix the usage of external phy's priv field by gmii2rgmii driver.

Based on net-next.

Harini Katakam (2):
  include: mdio: Add private field to mdio structure
  net: gmii2rgmii: Switch priv field in mdio device structure

 drivers/net/phy/xilinx_gmii2rgmii.c | 4 ++--
 include/linux/mdio.h                | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] include: mdio: Add private field to mdio structure
  2019-07-31  9:36 [PATCH 0/2] Fix GMII2RGMII private field Harini Katakam
@ 2019-07-31  9:36 ` Harini Katakam
  2019-07-31  9:36 ` [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure Harini Katakam
  1 sibling, 0 replies; 9+ messages in thread
From: Harini Katakam @ 2019-07-31  9:36 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, davem
  Cc: michal.simek, netdev, linux-arm-kernel, linux-kernel,
	harinikatakamlinux, harini.katakam, radhey.shyam.pandey

Add a priv pointer to mdio structure to be used by mdio devices if
required.

This priv field will be used by gmii2rgmii driver. As this IP has
no capability to read status on the MDIO bus, the driver currently
snoops the same and needs the instance information is some private
field. Since phy device "priv" can be used by external phy drivers,
it is not appropriate. Hence this addition to mdio device. This is
a temporary solution before the IP can be improved. The need for
this priv field can be re-evaluated later based on other mdio devices.

Signed-off-by: Harini Katakam <harini.katakam@xilinx.com>
Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
 include/linux/mdio.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index e8242ad8..3399de7 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -40,6 +40,9 @@ struct mdio_device {
 	struct reset_control *reset_ctrl;
 	unsigned int reset_assert_delay;
 	unsigned int reset_deassert_delay;
+
+	/* private data pointer for use by MDIO devices */
+	void *priv;
 };
 #define to_mdio_device(d) container_of(d, struct mdio_device, dev)
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure
  2019-07-31  9:36 [PATCH 0/2] Fix GMII2RGMII private field Harini Katakam
  2019-07-31  9:36 ` [PATCH 1/2] include: mdio: Add private field to mdio structure Harini Katakam
@ 2019-07-31  9:36 ` Harini Katakam
  2019-08-01  4:06   ` Andrew Lunn
  1 sibling, 1 reply; 9+ messages in thread
From: Harini Katakam @ 2019-07-31  9:36 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, davem
  Cc: michal.simek, netdev, linux-arm-kernel, linux-kernel,
	harinikatakamlinux, harini.katakam, radhey.shyam.pandey

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 <harini.katakam@xilinx.com>
Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
 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(&priv->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 = &priv->conv_phy_drv;
 
 	return 0;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure
  2019-07-31  9:36 ` [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure Harini Katakam
@ 2019-08-01  4:06   ` Andrew Lunn
  2019-08-13 11:16     ` Harini Katakam
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2019-08-01  4:06 UTC (permalink / raw)
  To: Harini Katakam
  Cc: f.fainelli, hkallweit1, davem, michal.simek, netdev,
	linux-arm-kernel, linux-kernel, harinikatakamlinux,
	radhey.shyam.pandey

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(&mdiodev->dev) in xgmiitorgmii_probe() and
dev_get_drvdata(&phydev->mdiomdio.dev) in _read_status()

       Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure
  2019-08-01  4:06   ` Andrew Lunn
@ 2019-08-13 11:16     ` Harini Katakam
  2019-08-13 13:23       ` Andrew Lunn
  0 siblings, 1 reply; 9+ messages in thread
From: Harini Katakam @ 2019-08-13 11:16 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Harini Katakam, Florian Fainelli, Heiner Kallweit, David Miller,
	Michal Simek, netdev, linux-arm-kernel, linux-kernel,
	radhey.shyam.pandey

Hi Andrew,

On Thu, Aug 1, 2019 at 9:36 AM Andrew Lunn <andrew@lunn.ch> 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(&mdiodev->dev) in xgmiitorgmii_probe() and
> dev_get_drvdata(&phydev->mdiomdio.dev) in _read_status()

Thanks for the review. This works if I do:
dev_set_drvdata(&priv->phy_dev->mdio.dev->dev) in probe
and then
dev_get_drvdata(&phydev->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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure
  2019-08-13 11:16     ` Harini Katakam
@ 2019-08-13 13:23       ` Andrew Lunn
  2019-08-13 15:13         ` Harini Katakam
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2019-08-13 13:23 UTC (permalink / raw)
  To: Harini Katakam
  Cc: Harini Katakam, Florian Fainelli, Heiner Kallweit, David Miller,
	Michal Simek, netdev, linux-arm-kernel, linux-kernel,
	radhey.shyam.pandey

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 <andrew@lunn.ch> 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(&mdiodev->dev) in xgmiitorgmii_probe() and
> > dev_get_drvdata(&phydev->mdiomdio.dev) in _read_status()
> 
> Thanks for the review. This works if I do:
> dev_set_drvdata(&priv->phy_dev->mdio.dev->dev) in probe
> and then
> dev_get_drvdata(&phydev->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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure
  2019-08-13 13:23       ` Andrew Lunn
@ 2019-08-13 15:13         ` Harini Katakam
  2019-08-13 15:38           ` Andrew Lunn
  0 siblings, 1 reply; 9+ messages in thread
From: Harini Katakam @ 2019-08-13 15:13 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Harini Katakam, Florian Fainelli, Heiner Kallweit, David Miller,
	Michal Simek, netdev, linux-arm-kernel, linux-kernel,
	radhey.shyam.pandey

Hi Andrew,

On Tue, Aug 13, 2019 at 6:54 PM Andrew Lunn <andrew@lunn.ch> 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 <andrew@lunn.ch> 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(&mdiodev->dev) in xgmiitorgmii_probe() and
> > > dev_get_drvdata(&phydev->mdiomdio.dev) in _read_status()
> >
> > Thanks for the review. This works if I do:
> > dev_set_drvdata(&priv->phy_dev->mdio.dev->dev) in probe
> > and then
> > dev_get_drvdata(&phydev->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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure
  2019-08-13 15:13         ` Harini Katakam
@ 2019-08-13 15:38           ` Andrew Lunn
  2019-09-04 14:11             ` Harini Katakam
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2019-08-13 15:38 UTC (permalink / raw)
  To: Harini Katakam
  Cc: Harini Katakam, Florian Fainelli, Heiner Kallweit, David Miller,
	Michal Simek, netdev, linux-arm-kernel, linux-kernel,
	radhey.shyam.pandey

> > 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure
  2019-08-13 15:38           ` Andrew Lunn
@ 2019-09-04 14:11             ` Harini Katakam
  0 siblings, 0 replies; 9+ messages in thread
From: Harini Katakam @ 2019-09-04 14:11 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Harini Katakam, Florian Fainelli, Heiner Kallweit, David Miller,
	Michal Simek, netdev, linux-arm-kernel, linux-kernel,
	radhey.shyam.pandey

Hi Andrew,

On Tue, Aug 13, 2019 at 9:40 PM Andrew Lunn <andrew@lunn.ch> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-09-04 14:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-31  9:36 [PATCH 0/2] Fix GMII2RGMII private field Harini Katakam
2019-07-31  9:36 ` [PATCH 1/2] include: mdio: Add private field to mdio structure Harini Katakam
2019-07-31  9:36 ` [PATCH 2/2] net: gmii2rgmii: Switch priv field in mdio device structure Harini Katakam
2019-08-01  4:06   ` Andrew Lunn
2019-08-13 11:16     ` Harini Katakam
2019-08-13 13:23       ` Andrew Lunn
2019-08-13 15:13         ` Harini Katakam
2019-08-13 15:38           ` Andrew Lunn
2019-09-04 14:11             ` Harini Katakam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).