[PATCH rtems-libbsd 1/3] freebsd/cgem: Add phy address to read it from device tree

Padmarao.Begari at microchip.com Padmarao.Begari at microchip.com
Thu Mar 2 05:31:57 UTC 2023


Hi Will,

Thanks for review comments.

> On Wed, 2023-03-01 at 08:10 -0600, Will wrote:
> 	
> On Tue, Feb 28, 2023 at 11:57 PM Padmarao Begari <
> padmarao.begari at microchip.com> wrote:
> > Read the phy address from the device tree and use it to
> > find the phy device if not found then search in the
> > range of 0 to 31.
> > ---
> >  freebsd/sys/dev/cadence/if_cgem.c | 41
> > ++++++++++++++++++++++++++++---
> >  1 file changed, 37 insertions(+), 4 deletions(-)
> > 
> > diff --git a/freebsd/sys/dev/cadence/if_cgem.c
> > b/freebsd/sys/dev/cadence/if_cgem.c
> > index 689c3611..2888a085 100644
> > --- a/freebsd/sys/dev/cadence/if_cgem.c
> > +++ b/freebsd/sys/dev/cadence/if_cgem.c
> > @@ -1217,6 +1217,27 @@ cgem_intr(void *arg)
> >         CGEM_UNLOCK(sc);
> >  }
> > 
> > +static int
> > +cgem_get_phyaddr(phandle_t node, int *phy_addr)
> > +{
> > +       phandle_t phy_node;
> > +       pcell_t phy_handle, phy_reg;
> > +
> > +       if (OF_getencprop(node, "phy-handle", (void *)&phy_handle,
> > +               sizeof(phy_handle)) <= 0)
> > +               return (ENXIO);
> > +
> > +       phy_node = OF_node_from_xref(phy_handle);
> > +
> > +       if (OF_getencprop(phy_node, "reg", (void *)&phy_reg,
> > +               sizeof(phy_reg)) <= 0)
> > +               return (ENXIO);
> > +
> > +       *phy_addr = phy_reg;
> > +
> > +       return (0);
> > +}
> > +
> > 
> 
> All changes should be gated behind #ifdef __rtems__, even if they're
> backports from upstream.
> 
Ok, will update
>  
> >  /* Reset hardware. */
> >  static void
> >  cgem_reset(struct cgem_softc *sc)
> > @@ -2003,6 +2024,7 @@ cgem_attach(device_t dev)
> >         pcell_t cell;
> >         int rid, err;
> >         u_char eaddr[ETHER_ADDR_LEN];
> > +       int phy_addr;
> > 
> 
> Same here.
>  
> >         sc->dev = dev;
> >         CGEM_LOCK_INIT(sc);
> > @@ -2091,10 +2113,21 @@ cgem_attach(device_t dev)
> >         cgem_reset(sc);
> >         CGEM_UNLOCK(sc);
> > 
> > -       /* Attach phy to mii bus. */
> > -       err = mii_attach(dev, &sc->miibus, ifp,
> > -                        cgem_ifmedia_upd, cgem_ifmedia_sts,
> > -                        BMSR_DEFCAPMASK, MII_PHY_ANY,
> > MII_OFFSET_ANY, 0);
> > +       err = cgem_get_phyaddr(node, &phy_addr);
> > +       if (err == 0) {
> > +               /* Attach phy to mii bus. */
> > +               err = mii_attach(dev, &sc->miibus, ifp,
> > +                                cgem_ifmedia_upd,
> > cgem_ifmedia_sts,
> > +                                BMSR_DEFCAPMASK, phy_addr,
> > MII_OFFSET_ANY, 0);
> > +       }
> > +
> > +       if (err) {
> > +               /* Attach phy to mii bus. */
> > +               err = mii_attach(dev, &sc->miibus, ifp,
> > +                                cgem_ifmedia_upd,
> > cgem_ifmedia_sts,
> > +                                BMSR_DEFCAPMASK, MII_PHY_ANY,
> > MII_OFFSET_ANY, 0);
> > +       }
> > +
> > 
> 
> Same here. Also, a simpler implementation would be to have
> cgem_get_phyaddr return the PHY address or MII_PHY_ANY on error and
> fold that all into the declaration. That would leave the declaration
> as:
> #ifdef __rtems__
>        int phy_addr = cgem_get_phyaddr(node);
> #endif
> 
> and the change here as:
>        /* Attach phy to mii bus. */
>        err = mii_attach(dev, &sc->miibus, ifp,
>                         cgem_ifmedia_upd, cgem_ifmedia_sts,
> #ifdef __rtems__
>                         BMSR_DEFCAPMASK, phy_addr, MII_OFFSET_ANY,
> 0);
> #else /* __rtems__ */
>                         BMSR_DEFCAPMASK, MII_PHY_ANY, MII_OFFSET_ANY,
> 0);
> #endif /* __rtems__ */
> 

Ok sure, will update

Regards
Padmarao
> >         if (err) {
> >                 device_printf(dev, "attaching PHYs failed\n");
> >                 cgem_detach(dev);


More information about the devel mailing list