Re: [PATCH net-next 04/24] net: dsa: mt7530: properly support MT7531AE and MT7531BE

[Daniel Golle]

On Wed, Apr 26, 2023 at 11:12:09AM +0300, Arınç ÜNAL wrote:
> On 25.04.2023 18:04, Daniel Golle wrote:
> > On Tue, Apr 25, 2023 at 11:29:13AM +0300, arinc9.unal@xxxxxxxxx wrote:
> > > From: Arınç ÜNAL <arinc.unal@xxxxxxxxxx>
> > > 
> > > Introduce the p5_sgmii pointer to store the information for whether port 5
> > > has got SGMII or not.
> > 
> > The p5_sgmii your are introducing to struct mt7530_priv is a boolean
> > variable, and not a pointer.
> 
> I must've meant to say field.

Being just a single boolean variable also 'field' would not be the right
word here. We use 'field' as in 'bitfield', ie. usually disguised integer
types in which each bit has an assigned meaning.

> 
> > 
> > > 
> > > Move the comment about MT7531AE and MT7531BE to mt7531_setup(), where the
> > > switch is identified.
> > > 
> > > Get rid of mt7531_dual_sgmii_supported() now that priv->p5_sgmii stores the
> > > information. Address the code where mt7531_dual_sgmii_supported() is used.
> > > 
> > > Get rid of mt7531_is_rgmii_port() which just prints the opposite of
> > > priv->p5_sgmii.
> > > 
> > > Remove P5_INTF_SEL_GMAC5_SGMII. The p5_interface_select enum is supposed to
> > > represent the mode that port 5 is being used in, not the hardware
> > > information of port 5. Set p5_intf_sel to P5_INTF_SEL_GMAC5 instead, if
> > > port 5 is not dsa_is_unused_port().
> > > 
> > > Signed-off-by: Arınç ÜNAL <arinc.unal@xxxxxxxxxx>
> > > ---
> > 
> > Other than the comment above this change makes sense and looks good to
> > me, so once you correct the commit message, you may add my Acked-by.
> 
> Will do, thanks.
> 
> Arınç