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] [day] [month] [year] [list]
Message-Id: <b694c12a-e0f7-47af-85c3-eb527963a7aa@app.fastmail.com>
Date:   Thu, 26 Jan 2023 21:32:21 +0100
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Andrew Lunn" <andrew@...n.ch>, "Arnd Bergmann" <arnd@...nel.org>
Cc:     "Wei Fang" <wei.fang@....com>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        "Shenwei Wang" <shenwei.wang@....com>,
        "Clark Wang" <xiaoning.wang@....com>,
        "NXP Linux Team" <linux-imx@....com>,
        "David S . Miller" <davem@...emloft.net>,
        "Eric Dumazet" <edumazet@...gle.com>,
        "Jakub Kicinski" <kuba@...nel.org>,
        "Paolo Abeni" <pabeni@...hat.com>, Netdev <netdev@...r.kernel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fec: convert to gpio descriptor

On Thu, Jan 26, 2023, at 19:02, Andrew Lunn wrote:
> On Thu, Jan 26, 2023 at 02:52:58PM +0100, Arnd Bergmann wrote:
>>  static int fec_reset_phy(struct platform_device *pdev)
>>  {
>> -	int err, phy_reset;
>> +	int err;
>> +	struct gpio_desc *phy_reset;
>>  	bool active_high = false;
>>  	int msec = 1, phy_post_delay = 0;
>>  	struct device_node *np = pdev->dev.of_node;
>
> Hi Arnd
>
> netdev drivers are supposed to use 'reverse Christmas tree'. It looks
> like this function is actually using 'Christmas tree' :-) Please could
> you keep with the current coding style.

My feeling is that the style in this file is just random,
but having 'err' as the last one fits with the usual style
and with what some of the other functions do, so I'll do that.

>> +			active_high ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
>> +	if (IS_ERR(phy_reset)) {
>> +		err = PTR_ERR(phy_reset);
>> +		if (err != -EPROBE_DEFER)
>> +			dev_err(&pdev->dev,
>> +				"failed to get phy-reset-gpios: %d\n", err);
>
> dev_err_probe() looks usable here.

Ah nice, I've never been able to use that one so far.

Will send a v2 with both suggestions.

    ARnd

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ