[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <98d3cefc-0198-f3b2-8ab6-c99ed50e5f57@arinc9.com>
Date: Thu, 25 May 2023 09:00:27 +0300
From: Arınç ÜNAL <arinc.unal@...nc9.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Sean Wang <sean.wang@...iatek.com>, Landen Chao
<Landen.Chao@...iatek.com>, DENG Qingfang <dqfext@...il.com>,
Daniel Golle <daniel@...rotopia.org>, Andrew Lunn <andrew@...n.ch>,
Florian Fainelli <f.fainelli@...il.com>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Russell King <linux@...linux.org.uk>,
Richard van Schagen <richard@...terhints.com>,
Richard van Schagen <vschagen@...com>,
Frank Wunderlich <frank-w@...lic-files.de>,
Bartel Eerdekens <bartel.eerdekens@...stell8.be>, erkin.bozoglu@...ont.com,
mithat.guner@...ont.com, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH net-next 03/30] net: dsa: mt7530: properly support
MT7531AE and MT7531BE
On 24.05.2023 17:48, Vladimir Oltean wrote:
> On Mon, May 22, 2023 at 03:15:05PM +0300, arinc9.unal@...il.com wrote:
>> From: Arınç ÜNAL <arinc.unal@...nc9.com>
>>
>> Introduce the p5_sgmii field to store the information for whether port 5
>> has got SGMII or not.
>>
>> 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@...nc9.com>
>> Acked-by: Daniel Golle <daniel@...rotopia.org>
>> ---
>
> Pretty busy patch, and after reading it, I'm not sure:
>
> - why? (this seems to be absent from the commit message)
> - how are MT7531AE and MT7531BE supported any more properly after this
> change, as the commit title claims
> - what is the overall effect, other than just refactoring. If that's
> all, what's written on the tin needs to be a better representation of
> what's inside.
>
> Pure refactoring is not a bad goal in itself, as long as we're on the
> same page that there is a justification which makes the new code better
> than the old one.
With this patch, instead of reading the MT7531_TOP_SIG_SR register
multiple times, the register will be read once and the value will be
stored on the p5_sgmii field. This saves unnecessary reads of the
register. I will mention this on the patch log.
This is already being done on the U-Boot driver.
https://github.com/u-boot/u-boot/blob/master/drivers/net/mtk_eth.c#L859
I think "improve MT7531AE and MT7531BE support" will be a better subject.
Arınç
Powered by blists - more mailing lists