linux-kernel - Re: [PATCH net-next v2 1/7] net: dsa: mt7530: always trap frames to active CPU port on MT7530

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6d6169bc-ce33-46e2-8226-0fef9f92949b@arinc9.com>
Date: Sat, 6 Jan 2024 17:33:56 +0300
From: Arınç ÜNAL <arinc.unal@...nc9.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Daniel Golle <daniel@...rotopia.org>,
 Landen Chao <Landen.Chao@...iatek.com>, DENG Qingfang <dqfext@...il.com>,
 Sean Wang <sean.wang@...iatek.com>, 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>,
 Frank Wunderlich <frank-w@...lic-files.de>,
 Bartel Eerdekens <bartel.eerdekens@...stell8.be>, mithat.guner@...ont.com,
 erkin.bozoglu@...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 v2 1/7] net: dsa: mt7530: always trap frames to
 active CPU port on MT7530

On 4.01.2024 18:22, Vladimir Oltean wrote:
> On Wed, Dec 27, 2023 at 07:43:41AM +0300, Arınç ÜNAL wrote:
>> @@ -3075,6 +3071,38 @@ static int mt753x_set_mac_eee(struct dsa_switch *ds, int port,
>>   	return 0;
>>   }
>>   
>> +static void
>> +mt753x_conduit_state_change(struct dsa_switch *ds,
>> +			    const struct net_device *conduit,
>> +			    bool operational)
>> +{
>> +	struct dsa_port *cpu_dp = conduit->dsa_ptr;
>> +	struct mt7530_priv *priv = ds->priv;
>> +	u8 mask;
>> +	int val = 0;
> 
> Longest line first.
> 
>> +
>> +	/* Set the CPU port to trap frames to for MT7530. Trapped frames will be
>> +	 * forwarded to the numerically smallest CPU port which the DSA conduit
>> +	 * interface its affine to is up.
> 
> "first CPU port whose conduit interface is up"
> 
>> +	 */
>> +	if (priv->id != ID_MT7530 && priv->id != ID_MT7621)
>> +		return;
>> +
>> +	mask = BIT(cpu_dp->index);
>> +
>> +	if (operational)
>> +		priv->active_cpu_ports |= mask;
>> +	else
>> +		priv->active_cpu_ports &= ~mask;
>> +
>> +	if (priv->active_cpu_ports)
>> +		val =
>> +		    CPU_EN |
>> +		    CPU_PORT(__ffs((unsigned long)priv->active_cpu_ports));
> 
> I don't think the type cast is necessary (implicit type promotion takes place).
> 
> Also, it is customary to put {} for multi-line "if" blocks, even if they are
> made up of a single expression.
> 
> But without the type cast, it could look like this.
> 
> 	if (priv->active_cpu_ports)
> 		val = CPU_EN | CPU_PORT(__ffs(priv->active_cpu_ports));

Will do. Thanks for dealing with my rookie mistakes. :)

Arınç