Subject: Re: [PATCH] net: fec: fix conversion to gpiod API

From: Dmitry Torokhov <[email-masked]>
Date: Tue, 31 Jan 2023 19:04:19 -0800

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