RE: [PATCH 6/6] net: mvpp2: convert FDT access to ofnode API

["Peng Fan (OSS)" <peng.fan@oss.nxp.com>]

Hi Simon,

Thanks for giving a look.

> Subject: Re: [PATCH 6/6] net: mvpp2: convert FDT access to ofnode API
> 
> Hi Peng,
> 
> On 2026-05-26T07:25:09, Peng Fan (OSS) <peng.fan@oss.nxp.com>
> wrote:
> > net: mvpp2: convert FDT access to ofnode API
> >
> > Convert mvpp2 driver from legacy fdtdec/fdt_* APIs to the ofnode-
> based
> > interfaces.
> >
> > Replace usage of dev_of_offset(), fdtdec_lookup_phandle(),
> > fdtdec_get_int(), fdt_parent_offset(), and related helpers with their
> > ofnode equivalents, including dev_ofnode(), ofnode_parse_phandle(),
> > ofnode_read_s32_default(), ofnode_get_parent(), and
> > ofnode_for_each_subnode().
> >
> > Remove direct dependencies on gd->fdt_blob.
> >
> > Main changes:
> > - Use ofnode_valid() instead of integer checks for node presence
> > - Switch fixed-link detection to ofnode_find_subnode()
> > - Replace uclass_get_device_by_of_offset() with
> >   uclass_get_device_by_ofnode()
> > - Update subnode iteration and device binding to use ofnode
> >
> > [...]
> >
> > drivers/net/mvpp2.c | 54
> > ++++++++++++++++++++++++++---------------------------
> >  1 file changed, 27 insertions(+), 27 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> > diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c @@ -5436,15
> > +5436,14 @@ static struct driver mvpp2_driver = {
> >   */
> >  static int mvpp2_base_bind(struct udevice *parent)  {
> > -     const void *blob = gd->fdt_blob;
> > -     int node = dev_of_offset(parent);
> > +     ofnode node = dev_ofnode(parent);
> 
> 'node' is now no longer used by the look of it. Do you see a warning?

I use "./tools/buildman/buildman x250 ", I may miss something.
indeed no user, I will remove it. 

> 
> > diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
> > @@ -5478,8 +5477,9 @@ static int mvpp2_base_bind(struct udevice
> *parent)
> >               sprintf(name, 'mvpp2-%d', id);
> >
> >               /* Create child device UCLASS_ETH and bind it */
> > -             device_bind(parent, &mvpp2_driver, name, plat,
> > -                         offset_to_ofnode(subnode), &dev);
> > +             ret = device_bind(parent, &mvpp2_driver, name, plat,
> subnode, &dev);
> > +             if (ret)
> > +                     return ret;
> 
> This is a behavioural change - the old code ignored bind failures and
> continued, the new code aborts and leaks the already-allocated plat
> and name. Probably the right thing, but please mention it in the
> commit message (the "No functional changes" claim is not quite
> accurate) and free plat/name on the error path.

Will update commit message in v2.

> 
> > diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
> > @@ -4731,33 +4731,34 @@ static int mvpp2_port_init(struct
> udevice *dev, struct mvpp2_port *port)
> > -             parent = fdt_parent_offset(gd->fdt_blob, phy_node);
> > -             ret = uclass_get_device_by_of_offset(UCLASS_MDIO, parent,
> > -                                                  &port->mdio_dev);
> > +             parent = ofnode_get_parent(phy_node);
> > +             if (!ofnode_valid(parent))
> > +                     return -ENODEV;
> 
> We should be careful returning -ENODEV as it has a special meaning
> with DM. This is a small behavioural change versus the old code, which
> passed a negative offset straight through. Worth noting in the commit
> message, or drop the check since the call below will return an error

I will drop the check in v2.

Thanks,
Peng.

> anyway.
> 
> Regards,
> Simon