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

[Kyle Switch]

On 8/17/26 21:12, Andrew Lunn wrote:
> 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@xxxxxxxxxxxxxx> 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.
Ans: The modifications in the next version V2 will retain the previous 
naming.
> > > > +       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.

Ans: In this application scenario, there is no firmware, and the eFuse

        may not be able to support so many registers.

> > 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.

Ans: Considering that the Motorcomm switch series currently all

        start with '92', and there will be driver work for other series 
in the future,

        the name was modified to 'yt92xx'. However, since it is 
uncertain whether

       there will be 'yt9xxx' in the future, the name has been reverted 
to the

       existing one for now.

>      Andrew