Date:   Mon, 7 Nov 2022 13:32:41 +0100
From:   Oleksij Rempel <o.rempel@...gutronix.de>
To:     piergiorgio.beruto@...il.com
Cc:     Andrew Lunn <andrew@...n.ch>, netdev@...r.kernel.org
Subject: Re: Adding IEEE802.3cg Clause 148 PLCA support to Linux

Hi Piergiorgio,

please CC me for the next rounds of T1* related topics :)


On Sun, Nov 06, 2022 at 08:05:51PM +0100, Andrew Lunn wrote:
> On Sun, Nov 06, 2022 at 07:11:32PM +0100, piergiorgio.beruto@...il.com wrote:
> > > I suggest you define new ethtool netlink messages. I don't think PHY tunables would make a good interface, since you have > multiple values which need configuring, and you also have some status information.
> 
> Your email client is messing up emails. I follow the netique rules, my
> lines are wrapped at around 75 characters. This is recommended
> practice for all Linux kernel mailing lists. Your mailer has destroyed
> this. Please also wrap your own text at about 75 characters.
> 
> > That sounds fair to me, thanks for your advice.
> > 
> > > So you probably want a message to set the configuration, and another to get the current configuration. For the set, you 
> > > probably want an attribute per configuration value, and allow a subset of attributes to be included in the message. The get 
> > > configuration should by default return all the attributes, but not enforce this, since some vendor will implement it wrong 
> > > and miss something out.
> > 
> > Yes, that sounds about right. If you have any hint on where in the code to start looking at, I'll start from there.
> 
> ethtool --cable-test packs a number of optional attributes into a
> netlink message. It then gets passed to phylib. You could use that as
> an example. The way cable tests results are passed back later is
> pretty unusual, so don't copy that code!
> 
> > > What I don't see in the Open Alliance spec is anything about interrupts. It would be interesting to see if any vendor triggers 
> > > an interrupt when PST changes. A PHY which has this should probably send a linkstate message to userspace reporting the 
> > > state change. For PHYs without interrupts, phylib will poll the read_status method once per second. You probably want to 
> > > check the PST bit during that poll. If EN is true, but PST is false, is the link considered down?
> > 
> 
> > This is actually an interesting point. First of all, yes, vendors do have IRQs for the PST. At least, the products I'm working on do, including the already released NCN26010.
> 
> Each PHY driver is going to need its own code for enabling the
> interrupt, handling etc, since none of this is standardized. This is
> one reason why you provide helpers, but don't force there use.
> 
> > My thinking is that the PST should be taken into account to evaluate the status of the link. On a multi-drop network with no autoneg and no link training the link status would not make much sense anyway, just like the connected status of an UDP socket wouldn't make sense.
> 
> So the read_status() call should evaluate the PST bit, along with
> EN. Again, a helper to do that would be useful.
> 
> The user API is the most important bit of this work. Linux considers
> the uAPI an stable ABI. Once you have defined it, it cannot change in
> ways which break backwards compatibility. So the initial reviews of
> code you present will concentrate on the uAPI. Once that is good,
> reviews will then swap to all the implementation details in phylib and
> the drivers.
> 
>     Andrew

Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Powered by blists - more mailing lists