[PATCH net-next] net: axienet: Allow phytool access to PCS/PMA PHY

[PATCH net-next] net: axienet: Allow phytool access to PCS/PMA PHY

[Robert Hancock]

Allow phytool ioctl access to read/write registers in the internal
PCS/PMA PHY if it is enabled.

Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 .../net/ethernet/xilinx/xilinx_axienet_main.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 13cd799541aa..41f2c2255118 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1213,10 +1213,29 @@ static void axienet_poll_controller(struct net_device *ndev)
 static int axienet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 {
 	struct axienet_local *lp = netdev_priv(dev);
+	struct mii_ioctl_data *mii = if_mii(rq);
 
 	if (!netif_running(dev))
 		return -EINVAL;
 
+	if (lp->pcs_phy && lp->pcs_phy->addr == mii->phy_id) {
+		int ret;
+
+		switch (cmd) {
+		case SIOCGMIIREG:
+			ret = mdiobus_read(lp->pcs_phy->bus, mii->phy_id, mii->reg_num);
+			if (ret >= 0) {
+				mii->val_out = ret;
+				ret = 0;
+			}
+			return ret;
+
+		case SIOCSMIIREG:
+			return mdiobus_write(lp->pcs_phy->bus, mii->phy_id,
+					     mii->reg_num, mii->val_in);
+		}
+	}
+
 	return phylink_mii_ioctl(lp->phylink, rq, cmd);
 }
 
-- 
2.27.0

Re: [PATCH net-next] net: axienet: Allow phytool access to PCS/PMA PHY

[Russell King (Oracle)]

On Wed, Jun 30, 2021 at 11:40:22AM -0600, Robert Hancock wrote:
> Allow phytool ioctl access to read/write registers in the internal
> PCS/PMA PHY if it is enabled.

I wonder if this is something that should happen in phylink?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] net: axienet: Allow phytool access to PCS/PMA PHY

[Robert Hancock]

On Wed, 2021-06-30 at 18:46 +0100, Russell King (Oracle) wrote:
> On Wed, Jun 30, 2021 at 11:40:22AM -0600, Robert Hancock wrote:
> > Allow phytool ioctl access to read/write registers in the internal
> > PCS/PMA PHY if it is enabled.
> 
> I wonder if this is something that should happen in phylink?
> 

If there are other drivers which have a PCS which could be accessed with
phytool etc., it might make sense. Right now phylink core doesn't really have
any knowledge that the PCS PHY actually exists as something that can be
accessed via MDIO registers, it just talks to it indirectly through the
mac_config and mac_pcs_get_state callbacks in the driver which then call back
into the c22_pcs helper functions to actually talk to the PCS. 

I'm not sure phylink could generically assume that the PCS can be accessed over
MDIO however, as I believe that the Cadence MACB IP, for example, at least as
implemented in the Xilinx ZynqMP parts, exposes its PCS with some PHY-style
registers but they are just a portion of the device's register space and not
accessed via MDIO, so we'd want to support that kind of setup. I suppose it
could implement an emulated MDIO bus to access those registers for that
purpose?

-- 
Robert Hancock
Senior Hardware Designer, Calian Advanced Technologies
www.calian.com

Re: [PATCH net-next] net: axienet: Allow phytool access to PCS/PMA PHY

[Russell King (Oracle)]

On Wed, Jun 30, 2021 at 06:23:46PM +0000, Robert Hancock wrote:
> On Wed, 2021-06-30 at 18:46 +0100, Russell King (Oracle) wrote:
> > On Wed, Jun 30, 2021 at 11:40:22AM -0600, Robert Hancock wrote:
> > > Allow phytool ioctl access to read/write registers in the internal
> > > PCS/PMA PHY if it is enabled.
> > 
> > I wonder if this is something that should happen in phylink?
> > 
> 
> If there are other drivers which have a PCS which could be accessed with
> phytool etc., it might make sense. Right now phylink core doesn't really have
> any knowledge that the PCS PHY actually exists as something that can be
> accessed via MDIO registers, it just talks to it indirectly through the
> mac_config and mac_pcs_get_state callbacks in the driver which then call back
> into the c22_pcs helper functions to actually talk to the PCS. 

Phylink does know that a PCS exists. It has separate pcs_ops for it, and
slightly changes its behaviour when a PCS exists.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] net: axienet: Allow phytool access to PCS/PMA PHY

[Andrew Lunn]

On Wed, Jun 30, 2021 at 11:40:22AM -0600, Robert Hancock wrote:
> Allow phytool ioctl access to read/write registers in the internal
> PCS/PMA PHY if it is enabled.
> 
> Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> ---
>  .../net/ethernet/xilinx/xilinx_axienet_main.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 13cd799541aa..41f2c2255118 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -1213,10 +1213,29 @@ static void axienet_poll_controller(struct net_device *ndev)
>  static int axienet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
>  {
>  	struct axienet_local *lp = netdev_priv(dev);
> +	struct mii_ioctl_data *mii = if_mii(rq);
>  
>  	if (!netif_running(dev))
>  		return -EINVAL;
>  
> +	if (lp->pcs_phy && lp->pcs_phy->addr == mii->phy_id) {
> +		int ret;
> +
> +		switch (cmd) {
> +		case SIOCGMIIREG:
> +			ret = mdiobus_read(lp->pcs_phy->bus, mii->phy_id, mii->reg_num);
> +			if (ret >= 0) {
> +				mii->val_out = ret;
> +				ret = 0;
> +			}
> +			return ret;
> +
> +		case SIOCSMIIREG:
> +			return mdiobus_write(lp->pcs_phy->bus, mii->phy_id,
> +					     mii->reg_num, mii->val_in);
> +		}


I would prefer not to allow write. The kernel should be driving the
hardware, and if user space changes values, the kernel has no idea
about it, and can do the wrong things.

      Andrew