Message-ID: <c07651cd-27b4-5ba4-8116-398522327d27@arinc9.com>
Date:   Tue, 14 Mar 2023 21:21:40 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH net-next v13 11/16] net: dsa: mt7530: use external PCS
 driver
Content-Language: en-US
To:     Daniel Golle <daniel@makrotopia.org>
Cc:     netdev@vger.kernel.org, linux-mediatek@lists.infradead.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Russell King <linux@armlinux.org.uk>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Lorenzo Bianconi <lorenzo@kernel.org>,
        Mark Lee <Mark-MC.Lee@mediatek.com>,
        John Crispin <john@phrozen.org>, Felix Fietkau <nbd@nbd.name>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        DENG Qingfang <dqfext@gmail.com>,
        Landen Chao <Landen.Chao@mediatek.com>,
        Sean Wang <sean.wang@mediatek.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        "David S. Miller" <davem@davemloft.net>,
        Vladimir Oltean <olteanv@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Vladimir Oltean <vladimir.oltean@nxp.com>,
        =?UTF-8?Q?Bj=c3=b8rn_Mork?= <bjorn@mork.no>,
        Frank Wunderlich <frank-w@public-files.de>,
        Alexander Couzens <lynxis@fe80.eu>
References: <cover.1678357225.git.daniel@makrotopia.org>
 <2ac2ee40d3b0e705461b50613fda6a7edfdbc4b3.1678357225.git.daniel@makrotopia.org>
 <e99cc7d1-554d-5d4d-e69a-a38ded02bb08@arinc9.com>
 <ZBCyqdfaeF/q8oZr@makrotopia.org>
From:   =?UTF-8?B?QXLEsW7DpyDDnE5BTA==?= <arinc.unal@arinc9.com>
In-Reply-To: <ZBCyqdfaeF/q8oZr@makrotopia.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 14/03/2023 20:45, Daniel Golle wrote:
> On Tue, Mar 14, 2023 at 08:16:35PM +0300, Arınç ÜNAL wrote:
>> On 9.03.2023 13:57, Daniel Golle wrote:
>>> [...]
>>> +static int mt7530_regmap_read(void *context, unsigned int reg, unsigned int *val)
>>> +{
>>> +	struct mt7530_priv *priv = context;
>>> +
>>> +	*val = mt7530_read(priv, reg);
>>> +	return 0;
>>> +};
>>> +
>>> +static int mt7530_regmap_write(void *context, unsigned int reg, unsigned int val)
>>> +{
>>> +	struct mt7530_priv *priv = context;
>>> +
>>> +	mt7530_write(priv, reg, val);
>>> +	return 0;
>>> +};
>>> +
>>> +static int mt7530_regmap_update_bits(void *context, unsigned int reg,
>>> +				     unsigned int mask, unsigned int val)
>>> +{
>>> +	struct mt7530_priv *priv = context;
>>> +
>>> +	mt7530_rmw(priv, reg, mask, val);
>>> +	return 0;
>>> +};
>>> +
>>> +static const struct regmap_bus mt7531_regmap_bus = {
>>> +	.reg_write = mt7530_regmap_write,
>>> +	.reg_read = mt7530_regmap_read,
>>> +	.reg_update_bits = mt7530_regmap_update_bits,
>>
>> These new functions can be used for both switches, mt7530 and mt7531,
>> correct?
> 
> In theory, yes, they could be used on all switch ICs supported by the
> mt7530.c driver. In practise they are used on MT7531 only because MT7530
> and MT7621 don't have any SGMII/SerDes ports, but only MT7531 does.
> 
> 
>> If so, I believe they are supposed to be called mt753x since
>> 88bdef8be9f6 ("net: dsa: mt7530: Extend device data ready for adding a new
>> hardware").
>>
>> mt753x: functions that can be used for mt7530 and mt7531 switches.
>> mt7530: functions specific to mt7530 switch.
>> mt7531: functions specific to mt7531 switch.
> 
> Good catch, so mt7530_* is for sure not accurate. I used that naming
> due to existing function names mt7530_read, mt7530_write and mt7530_rmw.

I was going to send a mail to netdev mailing list to ask for opinions 
whether we should rename the mt7530 DSA driver to mt753x and rename 
these functions you mentioned to mt753x so it's crystal clear what code 
is for what hardware. Now that we glossed over it here, I guess I can 
ask it here instead.

> 
> Given the situation I've explained above I think that mt753x_* would
> be the best and I will change that for v14.
> 
> Thank you for reviewing!

Sounds good, cheers.

Arınç