[PATCH v1] net: sun8i-emac: Add support for fixed-link phy

[Andre Przywara]

On Tue, 16 Jan 2024 19:58:56 +0300
Maxim Kiselev <bigunclemax at gmail.com> wrote:

Hi Maxim,

> вт, 16 янв. 2024 г. в 03:18, Andre Przywara <andre.przywara at arm.com>:
> >
> > On Thu, 15 Jun 2023 00:44:06 +0300
> > Maxim Kiselev <bigunclemax at gmail.com> wrote:
> >
> > Hi Maxim,
> >  
> > > From: Maksim Kiselev <bigunclemax at gmail.com>
> > >
> > > Based on dt-specs fixed-link doesn't require phy-handle to be used.  
> >
> > Do you have such a board?  
> 
> Yes, I had a custom board with T113 SoC which has fixed phy eth.

So just to clarify: this board connected something like a switch directly
to the MAC, just using RMII/RGMII? And then you have a fixed-phy node in
the DT, detailing the speed and duplex parameters? Matching the fixed-phy
node description in ethernet-controller.yaml? And drivers/net/phy/fixed.c
takes care of parsing that?

> > And where is that written down? I don't see
> > it explicitly mentioned as optional in ethernet-controller.yaml or in
> > the DT spec PDF.  
> 
> Sorry for that commit description. I just found the similar commit, that
> fixes the same problem for zynq_gem
> 3888c8d1979289efe685fe29276aed4d4b685975 ("net: zynq_gem: Add support for
> fixed-link phy") and copied the description from there.
> 
> > The sun8i EMAC binding lists phy-handle as required,
> > so that would need to be relaxed there then.  
> 
> Oh, indeed. So it will require to send dt-binding changes to Linux first...
> 
> Therefore, I think we can live without fixed-phy support for sun8i EMAC.
> At least until some device with such a configuration appears on the
> market :)

Well, I am fine with that patch if it fixes a problem for you. It looks
like requiring the phy-handle property is too strict, and just needs to be
relaxed in the binding. If I see this correctly, the driver would still
accept every valid DT, by today's and future bindings?

If you could just confirm my above assumptions, and maybe send a v2 with an
amended commit message, I would be happy to merge that patch.

Cheers,
Andre

> > > Fix driver to only read phy related setting when phy-handle is found.  
> >
> > The patch itself looks fine, we already specify -1 as the default when
> > the PHY DT node does not contain a reg property, so that looks like it
> > would work.
> >
> > Cheers,
> > Andre
> >  
> > > Signed-off-by: Maksim Kiselev <bigunclemax at gmail.com>
> > > ---
> > >  drivers/net/sun8i_emac.c | 7 ++-----
> > >  1 file changed, 2 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> > > index 04c3274fbe..0e339d69e0 100644
> > > --- a/drivers/net/sun8i_emac.c
> > > +++ b/drivers/net/sun8i_emac.c
> > > @@ -834,11 +834,8 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev)
> > >       priv->use_internal_phy = false;
> > >
> > >       offset = fdtdec_lookup_phandle(gd->fdt_blob, node, "phy-handle");
> > > -     if (offset < 0) {
> > > -             debug("%s: Cannot find PHY address\n", __func__);
> > > -             return -EINVAL;
> > > -     }
> > > -     priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> > > +     if (offset >= 0)
> > > +             priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", -1);
> > >
> > >       pdata->phy_interface = dev_read_phy_mode(dev);
> > >       debug("phy interface %d\n", pdata->phy_interface);  
> >  
> 
> Best wishes,
> Maksim