Re: [PATCH RFC net-next v2 03/12] net: mdio: mdiobus_register: update validation test

From: Michael Walle <michael@walle.cc>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: Heiner Kallweit <hkallweit1@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Jose Abreu <Jose.Abreu@synopsys.com>,
	Sergey Shtylyov <s.shtylyov@omp.ru>, Wei Fang <wei.fang@nxp.com>,
	Shenwei Wang <shenwei.wang@nxp.com>,
	Clark Wang <xiaoning.wang@nxp.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Sean Wang <sean.wang@mediatek.com>,
	Landen Chao <Landen.Chao@mediatek.com>,
	DENG Qingfang <dqfext@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vladimir Oltean <olteanv@gmail.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, Andrew Lunn <andrew@lunn.ch>,
	Geert Uytterhoeven <geert+renesas@glider.be>
Subject: Re: [PATCH RFC net-next v2 03/12] net: mdio: mdiobus_register: update validation test
Date: Tue, 03 Jan 2023 11:21:08 +0100	[thread overview]
Message-ID: <37247c17e5e555dddbc37c3c63a2cadb@walle.cc> (raw)
In-Reply-To: <Y7P/45Owf2IezIpO@shell.armlinux.org.uk>

Hi Russell,

Am 2023-01-03 11:13, schrieb Russell King (Oracle):
> On Wed, Dec 28, 2022 at 12:07:19AM +0100, Michael Walle wrote:
>> +	if (!bus || !bus->name)
>> +		return -EINVAL;
>> +
>> +	/* An access method always needs both read and write operations */
>> +	if ((bus->read && !bus->write) ||
>> +	    (!bus->read && bus->write) ||
>> +	    (bus->read_c45 && !bus->write_c45) ||
>> +	    (!bus->read_c45 && bus->write_c45))
> 
> I wonder whether the following would be even more readable:
> 
> 	if (!bus->read != !bus->write || !bus->read_c45 != !bus->write_c45)

That's what Andrew had originally. But there was a comment from Sergey 
[1]
which I agree with. I had a hard time wrapping my head around that, so I
just listed all the possible bad cases.

I don't have a strong opinion, though.

> which essentially asserts that the boolean of !method for the read and
> write methods must match.

Maybe with that as a comment?

-michael

[1] 
https://lore.kernel.org/netdev/ae79823f-3697-feee-32e6-645c6f4b4e93@omp.ru/