Date:   Wed, 1 Feb 2023 02:31:12 +0100
From:   Andrew Lunn <andrew@lunn.ch>
To:     Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc:     Wei Fang <wei.fang@nxp.com>, Jakub Kicinski <kuba@kernel.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Shenwei Wang <shenwei.wang@nxp.com>,
        Clark Wang <xiaoning.wang@nxp.com>,
        NXP Linux Team <linux-imx@nxp.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Marc Kleine-Budde <mkl@pengutronix.de>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: fec: fix conversion to gpiod API
Message-ID: <Y9nA4Mmi5hv5OzBh@lunn.ch>
References: <Y9mar1COtT5z4mvT@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y9mar1COtT5z4mvT@google.com>
Precedence: bulk

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.

> 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.

    Andrew