Re: [EXT] Re: [PATCH net-next 03/18] net: mvpp2: set the SMI PHY address when connecting to the PHY

[Andrew Lunn]

> This feature work only for Out-of-Band Auto-Negotiation in SGMII Mode.
> Current GoP(MAC) code configure SGMII In-band Auto-Negotiation performed by 
> the PCS layer
> without PHY polling.

Hi Stefan

So there is no need to configure the address then, leave PHY polling
turned off and we avoid all the issues.

   Andrew

RE: [EXT] Re: [PATCH net-next 03/18] net: mvpp2: set the SMI PHY address when connecting to the PHY

[Stefan Chulski]

> In order to safely read/write the PHY, you need to hold the PHY mutex.
> Unless the hardware is very smart, please don't enable this. Let the phylib 
> and
> the appropriate PHY driver do the work.
> 
>Andrew

Hi Andrew,

This feature work only for Out-of-Band Auto-Negotiation in SGMII Mode.
Current GoP(MAC) code configure SGMII In-band Auto-Negotiation performed by the 
PCS layer
without PHY polling.

Regards,
Stefan. 

Re: [EXT] Re: [PATCH net-next 03/18] net: mvpp2: set the SMI PHY address when connecting to the PHY

[Andrew Lunn]

> 2. Auto-Negotiation with PHY devices connected to the GMAC ports.
> The device uses a standard master Serial Management Interface for reading 
> from/writing to the PHY
> registers. In addition, the PHY polling unit performs Auto-Negotiation status 
> update with PHY devices attached
> to the Network ports via the Master SMI Interface.
> The device polls the Status register of each PHY in a round-robin manner.
> If the device detects a change in the link from down to up on 1 of the ports, 
> it performs a series of
> register reads from the PHY and updates the Auto-Negotiation results in the 
> device's registers. The
> Port MAC Status register is updated with these results only if 
> Auto-Negotiation is enabled.

Hi Stefan

That is what i was afraid off.

How clever is this phy polling hardware? e.g. Say somebody reads the
PHY temperature sensor:

commit 0b04680fdae464ee51409b8cb36005f6ef8bd689
Author: Andrew Lunn 
Date:   Fri Jan 20 01:37:49 2017 +0100

phy: marvell: Add support for temperature sensor

Some Marvell PHYs have an inbuilt temperature sensor. Add hwmon
support for this sensor.

There are two different variants. The simpler, older chips have a 5
degree accuracy. The newer devices have 1 degree accuracy.

Signed-off-by: Andrew Lunn 
Signed-off-by: David S. Miller 


This requires changing the PHY page to 0x1a. Any reads the polling
unit does at that time are going to get registers from page 0x1a, not
0x0.

And there are other examples where the page may change,
e.g. configuring WOL, LEDs. Cable test is not yet supported, but it is
on my todo list.

In order to safely read/write the PHY, you need to hold the PHY mutex.
Unless the hardware is very smart, please don't enable this. Let the
phylib and the appropriate PHY driver do the work.

   Andrew

RE: [EXT] Re: [PATCH net-next 03/18] net: mvpp2: set the SMI PHY address when connecting to the PHY

[Stefan Chulski]

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Friday, July 28, 2017 7:22 AM
> To: Antoine Tenart <antoine.ten...@free-electrons.com>
> Cc: da...@davemloft.net; ja...@lakedaemon.net; gregory.clement@free-
> electrons.com; sebastian.hesselba...@gmail.com; thomas.petazzoni@free-
> electrons.com; Nadav Haklai <nad...@marvell.com>; li...@armlinux.org.uk;
> m...@semihalf.com; Stefan Chulski <stef...@marvell.com>;
> netdev@vger.kernel.org; linux-arm-ker...@lists.infradead.org
> Subject: [EXT] Re: [PATCH net-next 03/18] net: mvpp2: set the SMI PHY address
> when connecting to the PHY
> 
> External Email
> 
> --
> On Thu, Jul 27, 2017 at 06:49:05PM -0700, Antoine Tenart wrote:
> > Hi Andrew,
> >
> > On Wed, Jul 26, 2017 at 06:08:06PM +0200, Andrew Lunn wrote:
> > > On Mon, Jul 24, 2017 at 03:48:33PM +0200, Antoine Tenart wrote:
> > > >
> > > > +   if (priv->hw_version != MVPP22)
> > > > +   return 0;
> > > > +
> > > > +   /* Set the SMI PHY address */
> > > > +   if (of_property_read_u32(port->phy_node, "reg", _addr)) {
> > > > +   netdev_err(port->dev, "cannot find the PHY address\n");
> > > > +   return -EINVAL;
> > > > +   }
> > > > +
> > > > +   writel(phy_addr, priv->iface_base +
> > > > +MVPP22_SMI_PHY_ADDR(port->gop_id));
> > > > return 0;
> > > >  }
> > >
> > > You could use phy_dev->mdiodev->addr, rather than parse the DT.
> >
> > OK.
> >
> > > Why does the MAC need to know this address? The phylib and PHY
> > > driver should be the only thing accessing the PHY, otherwise you are
> > > asking for trouble.
> >
> > This is part of the SMI/xSMI interface. I added into the mvpp2 driver
> > and not in the mvmdio one because the GoP port number must be known to
> > set this register (so that would be even less clean to do it).
> 
> Hi Antoine
> 
> It is still not clear to my why you need to program the address into the
> hardware. Is the hardware talking to the PHY?
> 
> Andrew

Hi Andrew,

This register configures SMI(Serial Management Interface) hardware unit, not 
PPv2(Packet Processor) hardware unit.
The SB incorporates the following SMI management interfaces:
MDC - Serial Management Interface Clock , MDIO - Serial Management Interface 
Data and complies with IEEE 802.3 Clause 22.

SMI interface used for:

1. PHY register read/write.
The device provides a mechanism for PHY registers read and write access.

2. Auto-Negotiation with PHY devices connected to the GMAC ports.
The device uses a standard master Serial Management Interface for reading 
from/writing to the PHY
registers. In addition, the PHY polling unit performs Auto-Negotiation status 
update with PHY devices attached
to the Network ports via the Master SMI Interface.
The device polls the Status register of each PHY in a round-robin manner.
If the device detects a change in the link from down to up on 1 of the ports, 
it performs a series of
register reads from the PHY and updates the Auto-Negotiation results in the 
device's registers. The
Port MAC Status register is updated with these results only if Auto-Negotiation 
is enabled.

So SMI interface should know GoP(MAC) id.

Regards,
Stefan.