On Tue, Mar 02, 2021 at 05:11:48PM +0100, Marek Vasut wrote: > On 3/2/21 4:24 PM, Tom Rini wrote: > > On Tue, Mar 02, 2021 at 03:52:39PM +0100, Marek Vasut wrote: > > > On 3/2/21 10:16 AM, Michael Nazzareno Trimarchi wrote: > > > > Hi Marek > > > > > > > > On Tue, Mar 2, 2021 at 9:40 AM Marek Vasut <ma...@denx.de> wrote: > > > > > > > > > > On 3/2/21 9:21 AM, Michael Nazzareno Trimarchi wrote: > > > > > > Hi Marek > > > > > > > > > > > > On Tue, Mar 2, 2021 at 9:13 AM Marek Vasut <ma...@denx.de> wrote: > > > > > > > > > > > > > > On 3/2/21 8:53 AM, Jagan Teki wrote: > > > > > > > > On Sat, Feb 27, 2021 at 6:21 PM Marek Vasut <ma...@denx.de> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On 2/27/21 8:54 AM, Jagan Teki wrote: > > > > > > > > > > Hi Marek, > > > > > > > > > > > > > > > > > > > > On Sat, Feb 27, 2021 at 3:44 AM Marek Vasut <ma...@denx.de> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > Add support for ethernet on the imx8mn-ddr4-evk. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Marek Vasut <ma...@denx.de> > > > > > > > > > > > Cc: Fabio Estevam <feste...@gmail.com> > > > > > > > > > > > Cc: Peng Fan <peng....@nxp.com> > > > > > > > > > > > Cc: Stefano Babic <sba...@denx.de> > > > > > > > > > > > --- > > > > > > > > > > > arch/arm/dts/imx8mn-evk.dtsi | 1 + > > > > > > > > > > > board/freescale/imx8mn_evk/imx8mn_evk.c | 41 > > > > > > > > > > > +++++++++++++++++++++++-- > > > > > > > > > > > configs/imx8mn_ddr4_evk_defconfig | 8 +++++ > > > > > > > > > > > 3 files changed, 47 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/arm/dts/imx8mn-evk.dtsi > > > > > > > > > > > b/arch/arm/dts/imx8mn-evk.dtsi > > > > > > > > > > > index 76d042a4cf0..416fadb22b1 100644 > > > > > > > > > > > --- a/arch/arm/dts/imx8mn-evk.dtsi > > > > > > > > > > > +++ b/arch/arm/dts/imx8mn-evk.dtsi > > > > > > > > > > > @@ -53,6 +53,7 @@ > > > > > > > > > > > pinctrl-0 = <&pinctrl_fec1>; > > > > > > > > > > > phy-mode = "rgmii-id"; > > > > > > > > > > > phy-handle = <ðphy0>; > > > > > > > > > > > + phy-reset-gpios = <&gpio4 22 GPIO_ACTIVE_LOW>; > > > > > > > > > > > > > > > > > > > > I believe this is phy reset-gpio to be part of mdio child > > > > > > > > > > node. any > > > > > > > > > > idea why it added in fec1 node? > > > > > > > > > > > > > > > > > > Try $ git grep phy-reset drivers/net/ , that should clarify > > > > > > > > > it all. > > > > > > > > > In short, FEC in U-Boot still uses legacy bindings. > > > > > > > > > > > > > > > > True. I think it's better to add this property on -u-boot.dtsi > > > > > > > > which > > > > > > > > helps to maintain Linux bindings. > > > > > > > > > > > > > > The reset GPIO is not specific to U-Boot in any way, so no. > > > > > > > Instead, the Linux DT bindings should be updated too, but that's a > > > > > > > separate topic. > > > > > > > > > > > > Deprecated optional properties: > > > > > > To avoid these, create a phy node according to phy.txt > > > > > > in the same > > > > > > directory, and point the fec's "phy-handle" property to > > > > > > it. Then use > > > > > > the phy's reset binding, again described by phy.txt. > > > > > > - phy-reset-gpios : Should specify the gpio for phy reset > > > > > > - phy-reset-duration : Reset duration in milliseconds. Should > > > > > > present > > > > > > only if property "phy-reset-gpios" is available. Missing the > > > > > > property > > > > > > will have the duration be 1 millisecond. Numbers greater than > > > > > > 1000 are > > > > > > > > > > > > Jagan refer maybe that property is deprecated > > > > > > > > > > See above, U-Boot does not support the per-PHY reset bindings with > > > > > FEC. > > > > > However, the PHY reset GPIO binding is not U-Boot specific, so it > > > > > should > > > > > not be hidden in -u-boot.dtsi . > > > > > > > > Yes, I understand. What Jagan is proposing is the following (that you > > > > can disagree) it's > > > > just move deprecated part in uboot.dtsi so we can keep align dtsi from > > > > linux without > > > > having this problem. > > > > > > What should happen is that this same thing should be fixed in Linux, i.e. > > > the missing bindings should be added there, and then once a DT sync > > > happens, > > > whoever does the sync should also then update the FEC driver to handle > > > per-PHY resets. But that is out of scope of this patch. > > > > > > What is for certain is that if the DT sync removes the reset GPIO from DT, > > > ethernet will no longer work in U-Boot and it will show up during testing. > > > If there is some extra property hidden in -u-boot.dtsi, this problem will > > > not trigger test failure then. > > > > This seems like a case where the "deprecated" property needs to be > > populated in the upstream dts file, even if the Linux Kernel doesn't > > need it strictly because we do and we might not need / want to redesign > > our driver to meet what Linux expects yet again. > > We really should fix the FEC driver, except that its way out of scope of > this patch. And before someone points out that should be simple to do, there > is a real unpleasant bonus with FEC and PHY reset bindings, in the ordering > in which FEC generates clock for the PHY during reset, for which Linux grew > a rather convoluted logic in the FEC driver and the PHY subsystem, see the > LAN8710 PHY discussions in Linux which seem to pop up every now and then. So > implementing the whole PHY reset handling in FEC driver, to behave just like > Linux, isn't as trivial as it seems at first.
It's out of scope for this patch, yes. And given what you note here I'm not even sure it's really required / a great idea for U-Boot. I added Rob to the list here because this is another case of "Is DT ABI or Linux driver description?". -- Tom
signature.asc
Description: PGP signature
