Re: [PATCH net-next v1] net:dsa:yt922x: Add support for Motorcomm YT922x

[Andrew Lunn <andrew@lunn.ch>]

On Mon, Aug 17, 2026 at 06:40:02PM +0800, Kyle Switch wrote:
> 
> On 8/17/26 05:17, David Yang wrote:
> > On Fri, Aug 14, 2026 at 9:50 AM Kyle Switch <kyle.switch@motor-comm.com> wrote:
> > 
> > > @@ -148,10 +148,15 @@ static const struct yt921x_info yt921x_infos[] = {
> > >                  YT921X_PORT_MASK_INT0_n(8),
> > >                  YT921X_PORT_MASK_EXT0 | YT921X_PORT_MASK_EXT1,
> > >          },
> > > +       {
> > > +               "YT9224", YT9224_MAJOR, 0, 0,
> > > +               0x1f1,
> > > +               0x0,
> > > +       },
> > Why plain magic numbers here, despite what the above entries shows?
> Ans: will fixed done in patch v2.
> > 
> > >          {}
> > >   };
> > > 
> > > -#define YT921X_NAME    "yt921x"
> > > +#define YT92XX_NAME    "yt92xx"
> > This changed the module name which may affect end users. While I have
> > no objection to this, subsystem maintainers may have different
> > opinions.
> Ans: This is used to indicate support for both YT921X and YT922X.

It is pretty common for newer generations of devices to be mostly
backwards compatible, with older generations. The driver does not
change name in Linux it causes too much pointless churn. Please just
get the names correct in the struct phy_driver.
> > > +       res = yt921x_intif_read(priv, addr, 0x0, &data);
> > > +       if (res)
> > > +               return res;
> > > +       data |= 1 << 15;
> > > +       res = yt921x_intif_write(priv, addr, 0x0, data);
> > > +       if (res)
> > > +               return res;
> > Numerous magic numbers. On YT9215, same finetunes are not required for
> > a usable port. If you cannot name it, drop it if it is not mandatory.
> 
> Ans: Some patches are mandatory

If they are mandatory, why are they needed at all? Why are they not in
the firmware. Seems like a poor hardware/firmware design to me.

> while others are for performance optimization.

For these, it would be good to fully document them, because board
designers might want to optimze them in other ways.

> > > -static void yt921x_mdio_remove(struct mdio_device *mdiodev)
> > > +static void yt92xx_mdio_remove(struct mdio_device *mdiodev)
> > Unnecessary renaming.

And taking this to its logical conclusion...

In a few years time you want to rename to yt9xxx.  And a few years
after that, rename to ytxxxx. 

    Andrew