Re: [PATCH v3 net-next 2/2] net: dsa: Add support for Motorcomm YT922x

[Kyle Switch]

On 8/26/26 20:43, Andrew Lunn wrote:
> On Wed, Aug 26, 2026 at 05:08:02PM +0800, Kyle Switch wrote:
> > Motorcomm YT922x is a series of ethernet switches including:
> >
> >   - YT9224: 4 * 2.5G UTPs and 2 serdes interface
> >
> > This patch just add basic func for a working DSA switch.
> I asked you to break this patch up. I want to see lots of little
> patches, each with a good commit messages, each obviously correct.
>
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
>
> says you can have a patch series of up to 15 patches.
>
> >   static int yt921x_mbus_int_read(struct mii_bus *mbus, int port, int reg)
> >   {
> >   	struct yt921x_priv *priv = mbus->priv;
> > +	int max_ports;
> >   	u16 val;
> >   	int res;
> >   
> > -	if (port >= YT921X_PORT_NUM)
> > +	max_ports = priv->series_info->ports;
> > +	if (port >= max_ports)
> >   		return U16_MAX;
> What did i say about this in my last review? This would make a good,
> simple patch, changing all YT921X_PORT_NUM to priv->series_info->ports.
>
> You first need a simple patch which adds struct yt92xx_series and
> yt92xx_series_table[] with very minimal contents, only mode and name,
> for the existing device. And add the code to do the lookup and
> associate it to priv.
>
> Then you can have a patch which adds .ports, and changes all
> YT921X_PORT_NUM to priv->series_info->ports.
>
> Then you can add .num_lag_ids and change all those references.
>
> Then you can add .ageing_time_FOO and change all those references.
>
> Slowly make the existing code more generic by adding to struct
> yt92xx_series.
>
>      Andrew

Ans: We intend to split this patch into smaller functional pieces and submit
them as separate commits.
I would also like to seek your advice on a design question.
For the yt92xx_series_info table, the maveall reference design invokes
chip_detect() inside probe() to perform the matching lookup.
However, in the existing yt921x DSA driver, chip_detect() is called within
dsa_setup() instead.

We are considering two possible approaches:
1. Keep the code as-is and relocate the initialization of certain
dsa_switch structure parameters into dsa_setup().
2. Refactor by moving chip_detect() into probe() to align with
the reference design.

Which of these two approaches would you lean toward,
and what are your considerations?

> ---
> pw-bot: cr