Re: [PATCH v3 7/8] net: add MV88E61xx DSA driver

[Tim Harvey <tharvey@gateworks.com>]

On Tue, Jun 21, 2022 at 12:21 AM Vladimir Oltean
<vladimir.oltean@nxp.com> wrote:
>
> On Mon, Jun 20, 2022 at 04:37:45PM -0700, Tim Harvey wrote:
> > On Mon, Jun 20, 2022 at 4:58 AM Vladimir Oltean
> > <vladimir.oltean@nxp.com> wrote:
> > >
> > > On Mon, May 23, 2022 at 11:25:48AM -0700, Tim Harvey wrote:
> > > > +/* bind and probe the switch mdios */
> > > > +static int mv88e61xx_dsa_probe_mdio(struct udevice *dev)
> > > > +{
> > > > +     struct udevice *pdev;
> > > > +     ofnode node, mdios;
> > > > +     const char *name;
> > > > +     int ret;
> > > > +
> > > > +     /* bind phy ports of mdios child node to mv88e61xx_mdio device */
> > > > +     mdios = dev_read_subnode(dev, "mdios");
> > > > +     if (ofnode_valid(mdios)) {
> > > > +             ofnode_for_each_subnode(node, mdios) {
> > > > +                     name = ofnode_get_name(node);
> > > > +                     ret = device_bind_driver_to_node(dev,
> > > > +                                                      "mv88e61xx_mdio",
> > > > +                                                      name, node, &pdev);
> > > > +                     if (ret) {
> > > > +                             dev_err(dev, "failed to bind %s: %d\n", name, ret);
> > > > +                             continue;
> > > > +                     }
> > > > +
> > > > +                     /* need to probe it as there is no compatible to do so */
> > > > +                     ret = uclass_get_device_by_ofnode(UCLASS_MDIO, node, &pdev);
> > > > +                     if (ret) {
> > > > +                             dev_err(dev, "failed to probe %s: %d\n", name, ret);
> > > > +                             continue;
> > > > +                     }
> > >
> > > What do you do with this pdev once you get it? Are you missing a device_probe() call?
> > > Also, why "pdev" and not "dev"? What does the "p" stand for?
> >
> > struct udevice *dev is passed into the function so I use pdev to
> > iterate over the ports in the mdios node so 'pdev' means 'port' here.
>
> Yes, but those under the mdios node aren't ports, they're MDIO
> controllers, hence my comment.

how about devp (dev pointer) or subdev or mdio?

Tim

>
> > I do not need to do anything with pdev but I must use
> > uclass_get_device_by_ofnode() to probe it and that function requires
> > it. I don't need to call device_probe because
> > uclass_get_device_by_ofnode does it for me
>
> Ok, I didn't notice they all call uclass_get_device_tail() which calls
> device_probe().