netdev - Re: [PATCH v10 1/3] net: ethernet: mtk_eth_soc: fix return value and refactor MDIO ops

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]

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