linux-kernel - Re: [PATCH] net: fec: fix conversion to gpiod API

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: <Y9nWs+JXPHbE7SPo@google.com>
Date:   Tue, 31 Jan 2023 19:04:19 -0800
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Wei Fang <wei.fang@....com>, Jakub Kicinski <kuba@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        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>,
        Paolo Abeni <pabeni@...hat.com>,
        Marc Kleine-Budde <mkl@...gutronix.de>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: fec: fix conversion to gpiod API

Hi Andrew,

On Wed, Feb 01, 2023 at 02:31:12AM +0100, Andrew Lunn wrote:
> On Tue, Jan 31, 2023 at 02:48:15PM -0800, Dmitry Torokhov wrote:
> > The reset line is optional, so we should be using
> > devm_gpiod_get_optional() and not abort probing if it is not available.
> 
> > Also, gpiolib already handles phy-reset-active-high, continuing handling
> > it directly in the driver when using gpiod API results in flipped logic.
> 
> Please could you split this part into a separate patch. There is some
> history here, but i cannot remember which driver it actually applies
> to. It might be the FEC, it could be some other Ethernet driver.
> 
> For whatever driver it was, the initial support for GPIOs totally
> ignored the polarity value in DT. The API at the time meant you needed
> to take extra steps to get the polarity, and that was skipped. So it
> was hard coded. But developers copy/pasted DT statement from other DT
> files, putting in the opposite polarity to the hard coded
> value. Nobody noticed until somebody needed the opposite polarity to
> the hard coded implementation to make their board work. And then the
> problem was noticed. The simple solution to actually use the polarity
> in DT would break all the boards which had the wrong value. So a new
> property was added.
> 
> So i would like this change in a separate patch, so if it causes
> regressions, it can be reverted.

The quirk for the gpiod API to take into account "phy-reset-active-high"
for FEC driver is already in mainline:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpio/gpiolib-of.c?id=b02c85c9458cdd15e2c43413d7d2541a468cde57

This is limited only to devices matching compatibles handled by FEC
driver and is being considered automatically as soon as gpiod API is
used.


> 
> > While at this convert phy properties parsing from OF to generic device
> > properties to avoid #ifdef-ery.
> 
> We also need to be careful here. If you read fsl,fec.yaml, there are a
> number of deprecated properties. These need to keep working for OF,
> but we clearly don't want them exposed to ACPI or anything else. So if
> you use generic device properties, please ensure they are only for OF.

OK, if this is a concern I'll drop this from the patch and keep of_*
APIs since we need to keep #ifdef CONFIG_OF anyway.

Thanks.

-- 
Dmitry