mbox series

[0/4] ksz9477 dsa switch driver improvements

Message ID 20200905140325.108846-1-pbarker@konsulko.com
Headers show
Series ksz9477 dsa switch driver improvements | expand

Message

Paul Barker Sept. 5, 2020, 2:03 p.m. UTC
These changes were made while debugging the ksz9477 driver for use on a
custom board which uses the ksz9893 switch supported by this driver. The
patches have been runtime tested on top of Linux 5.8.4, I couldn't
runtime test them on top of 5.9-rc3 due to unrelated issues. They have
been build tested on top of 5.9-rc3.

Paul Barker (4):
  net: dsa: microchip: Make switch detection more informative
  net: dsa: microchip: Add debug print for XMII port mode
  net: dsa: microchip: Disable RGMII in-band status on KSZ9893
  net: dsa: microchip: Implement recommended reset timing

 drivers/net/dsa/microchip/ksz9477.c    | 11 +++++++++++
 drivers/net/dsa/microchip/ksz_common.c |  3 ++-
 2 files changed, 13 insertions(+), 1 deletion(-)

Comments

Andrew Lunn Sept. 5, 2020, 3:34 p.m. UTC | #1
On Sat, Sep 05, 2020 at 03:03:21PM +0100, Paul Barker wrote:
> These changes were made while debugging the ksz9477 driver for use on a
> custom board which uses the ksz9893 switch supported by this driver. The
> patches have been runtime tested on top of Linux 5.8.4, I couldn't
> runtime test them on top of 5.9-rc3 due to unrelated issues. They have
> been build tested on top of 5.9-rc3.

Hi Paul

Please rebase onto net-next. Take a look at:

https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html

	Andrew
Paul Barker Sept. 5, 2020, 3:53 p.m. UTC | #2
On Sat, 5 Sep 2020 at 16:32, Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Sat, Sep 05, 2020 at 03:03:24PM +0100, Paul Barker wrote:
> > We can't assume that the link partner supports the in-band status
> > reporting which is enabled by default on the KSZ9893 when using RGMII
> > for the upstream port.
>
> What do you mean by RGMII inband status reporting? SGMII/1000BaseX has
> in band signalling, but RGMII?
>
>    Andrew

I'm referencing page 56 of the KSZ9893 datasheet
(http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9893R-Data-Sheet-DS00002420D.pdf).
The datasheet says "The RGMII port will not function properly if IBS
is enabled in the switch, but it is not receiving in-band status from
a connected PHY." Since we can't guarantee all possible link partners
will support this it should be disabled. In particular, the IMX6 SoC
we're using with this switch doesn't support this on its Ethernet
port.

I don't really know much about how this is implemented or how widely
it's supported.

Thanks,
Florian Fainelli Sept. 5, 2020, 4:04 p.m. UTC | #3
On 9/5/2020 8:53 AM, Paul Barker wrote:
> On Sat, 5 Sep 2020 at 16:32, Andrew Lunn <andrew@lunn.ch> wrote:
>>
>> On Sat, Sep 05, 2020 at 03:03:24PM +0100, Paul Barker wrote:
>>> We can't assume that the link partner supports the in-band status
>>> reporting which is enabled by default on the KSZ9893 when using RGMII
>>> for the upstream port.
>>
>> What do you mean by RGMII inband status reporting? SGMII/1000BaseX has
>> in band signalling, but RGMII?
>>
>>     Andrew
> 
> I'm referencing page 56 of the KSZ9893 datasheet
> (http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9893R-Data-Sheet-DS00002420D.pdf).
> The datasheet says "The RGMII port will not function properly if IBS
> is enabled in the switch, but it is not receiving in-band status from
> a connected PHY." Since we can't guarantee all possible link partners
> will support this it should be disabled. In particular, the IMX6 SoC
> we're using with this switch doesn't support this on its Ethernet
> port.

The RGMII 2.0 specification, pages 7 and 8 has more details:

http://web.archive.org/web/20160303171328/http://www.hp.com/rnd/pdfs/RGMIIv2_0_final_hp.pdf

and section 3.4.1 indicates that this is optional anyway for link 
status/speed/duplex.

It comes down to putting a appropriate data word on RXD[7:0] to signal 
link status, speed and speed, if the link partner does not provide the 
inter-frame word, then the receiver cannot reconstruct that information, 
or it will incorrectly decode it.

> 
> I don't really know much about how this is implemented or how widely
> it's supported.

It is supported by the Broadcom GENET adapter and probably a few others, 
however for the same reasons, I have not seen it being widely used. You 
would save two reads of BMSR to determine the link status which is nice, 
but link parameter changes are disruptive anyway.