Re: [net-next PATCH 2/6] net: pcs: Implement OF support for PCS driver

[Russell King (Oracle)]

On Wed, Mar 19, 2025 at 05:03:15PM +0100, Christian Marangi wrote:
> On Wed, Mar 19, 2025 at 03:17:27PM +0000, Russell King (Oracle) wrote:
> > On Wed, Mar 19, 2025 at 12:58:38AM +0100, Christian Marangi wrote:
> > > Implement the foundation of OF support for PCS driver.
> > > 
> > > To support this, implement a simple Provider API where a PCS driver can
> > > expose multiple PCS with an xlate .get function.
> > > 
> > > PCS driver will have to call of_pcs_add_provider() and pass the device
> > > node pointer and a xlate function to return the correct PCS for the
> > > requested interface and the passed #pcs-cells.
> > > 
> > > This will register the PCS in a global list of providers so that
> > > consumer can access it.
> > > 
> > > Consumer will then use of_pcs_get() to get the actual PCS by passing the
> > > device_node pointer, the index for #pcs-cells and the requested
> > > interface.
> > > 
> > > For simple implementation where #pcs-cells is 0 and the PCS driver
> > > expose a single PCS, the xlate function of_pcs_simple_get() is
> > > provided. In such case the passed interface is ignored and is expected
> > > that the PCS supports any interface mode supported by the MAC.
> > > 
> > > For advanced implementation a custom xlate function is required. Such
> > > function should return an error if the PCS is not supported for the
> > > requested interface type.
> > > 
> > > This is needed for the correct function of of_phylink_mac_select_pcs()
> > > later described.
> > > 
> > > PCS driver on removal should first call phylink_pcs_release() on every
> > > PCS the driver provides and then correctly delete as a provider with
> > > the usage of of_pcs_del_provider().
> > > 
> > > A generic function for .mac_select_pcs is provided for any MAC driver
> > > that will declare PCS in DT, of_phylink_mac_select_pcs().
> > > This function will parse "pcs-handle" property and will try every PCS
> > > declared in DT until one that supports the requested interface type is
> > > found. This works by leveraging the return value of the xlate function
> > > returned by of_pcs_get() and checking if it's an ERROR or NULL, in such
> > > case the next PCS in the phandle array is tested.
> > > 
> > > Some additional helper are provided for xlate functions,
> > > pcs_supports_interface() as a simple function to check if the requested
> > > interface is supported by the PCS and phylink_pcs_release() to release a
> > > PCS from a phylink instance.
> > > 
> > > Co-developed-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>
> > > Signed-off-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>
> > > Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx>
> > 
> > As a general comment, should we be developing stuff that is DT-centric
> > or fwnode-centric. We already have users of phylink using swnodes, and
> > it seems bad to design something today that is centred around just one
> > method of describing something.
> >
> 
> Honestly, at least for me testing scenario different than DT is quite
> difficult, we can make changes to also support swnodes but we won't have
> anything to test them. Given the bad situation PCS is currently I feel
> we should first focus on DT or at least model something workable first
> than put even more complex stuff on the table.

The problem I have is that once we invent DT specific interfaces, we
seem to endlessly persist with them even when we have corresponding
fwnode interfaces, even when it's easy to convert.

So, my opinion today is that we should avoid single-firmware specific
interfaces.

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