[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YdHWNIcr9AzFG7oN@lunn.ch>
Date: Sun, 2 Jan 2022 17:43:32 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Daniel Golle <daniel@...rotopia.org>
Cc: linux-mediatek@...ts.infradead.org, netdev@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Felix Fietkau <nbd@....name>, John Crispin <john@...ozen.org>,
Sean Wang <sean.wang@...iatek.com>,
Mark Lee <Mark-MC.Lee@...iatek.com>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Matthias Brugger <matthias.bgg@...il.com>,
Russell King <linux@...linux.org.uk>,
Michael Lee <igvtee@...il.com>,
Heiner Kallweit <hkallweit1@...il.com>
Subject: Re: [PATCH v10 1/3] net: ethernet: mtk_eth_soc: fix return value and
refactor MDIO ops
.> +static int _mtk_mdio_write(struct mtk_eth *eth, u32 phy_addr, u32 phy_reg,
> + u32 write_data)
> {
> if (mtk_mdio_busy_wait(eth))
> - return -1;
> -
> - write_data &= 0xffff;
> + return -EBUSY;
-ETIMEDOUT would be more normal.
I would probably also change mtk_mdio_busy_wait() so that it either
returned 0, or -ETIMEDOUT. That is the general pattern in Linux,
return 0 on success, or a negative error code. Returning -1 is an
invitation for trouble.
The code would then become
ret = mtk_mdio_busy_wait(eth);
if (ret < 0)
return ret;
which is a very common pattern in Linux.
Andrew
Powered by blists - more mailing lists