Re: [PATCH RFC net-next 11/12] net: dsa: sja1105: convert to phylink_generic_validate()

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Wed, Nov 24, 2021 at 07:53:40PM +0000, Vladimir Oltean wrote:
> On Wed, Nov 24, 2021 at 05:53:19PM +0000, Russell King (Oracle) wrote:
> > Populate the supported interfaces and MAC capabilities for the SJA1105
> > DSA switch and remove the old validate implementation to allow DSA to
> > use phylink_generic_validate() for this switch driver.
> > 
> > This switch only supports a static model of configuration, so we
> > restrict the interface modes to the configured setting, and pass the
> > MAC capabilities. As it is unclear which interface modes support 1G
> > speeds, we keep the setting of MAC_1000FD conditional on the configured
> > interface mode.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > ---
> 
> Please use this patch for sja1105. Thanks.

Your patch is combining two changes into one patch. Specifically, the
there are two logical changes in your patch:

1) changing the existing behaviour of the validate() function by
allowing switching between PHY_INTERFACE_MODE_SGMII and
PHY_INTERFACE_MODE_2500BASEX, which was not permitted before with the
sja1105_phy_mode_mismatch() check.

2) converting to supported_interfaces / mac_capabilities way of defining
what is supported.

Combining the two changes makes the patch harder to review, and it
becomes less obvious that it is actually correct. I would recommend
changing the existing behaviour prior to the conversion, but ultimately
that is your decision.

For more information please see the "Separate your changes" section in
Documentation/process/submitting-patches.rst

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!