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.beg...@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); _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel