| Message ID | 20200331004851.282583-12-marex@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Patrick Delaunay |
| Headers | show |
| Series | ARM: stm32: Fix Avenger96 | expand |
Hi Marek On 3/31/20 2:48 AM, Marek Vasut wrote: > Add PHY reset GPIO on AV96 ethernet PHY. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Patrick Delaunay <patrick.delaunay@st.com> > Cc: Patrice Chotard <patrice.chotard@st.com> Reviewed-by: Patrice Chotard <patrice.chotard@st.com> Thanks > --- > V2: No change > --- > arch/arm/dts/stm32mp157a-avenger96.dts | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/dts/stm32mp157a-avenger96.dts b/arch/arm/dts/stm32mp157a-avenger96.dts > index fb8f0021b3..7a4b6e6a2c 100644 > --- a/arch/arm/dts/stm32mp157a-avenger96.dts > +++ b/arch/arm/dts/stm32mp157a-avenger96.dts > @@ -101,6 +101,7 @@ > phy-mode = "rgmii"; > max-speed = <1000>; > phy-handle = <&phy0>; > + phy-reset-gpios = <&gpioz 2 GPIO_ACTIVE_LOW>; > > mdio0 { > #address-cells = <1>;
Hi Marek, > From: Marek Vasut <marex@denx.de> > Sent: mardi 31 mars 2020 02:49 > > Add PHY reset GPIO on AV96 ethernet PHY. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Patrick Delaunay <patrick.delaunay@st.com> > Cc: Patrice Chotard <patrice.chotard@st.com> > --- > V2: No change > --- > arch/arm/dts/stm32mp157a-avenger96.dts | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/dts/stm32mp157a-avenger96.dts > b/arch/arm/dts/stm32mp157a-avenger96.dts > index fb8f0021b3..7a4b6e6a2c 100644 > --- a/arch/arm/dts/stm32mp157a-avenger96.dts > +++ b/arch/arm/dts/stm32mp157a-avenger96.dts > @@ -101,6 +101,7 @@ > phy-mode = "rgmii"; > max-speed = <1000>; > phy-handle = <&phy0>; > + phy-reset-gpios = <&gpioz 2 GPIO_ACTIVE_LOW>; This obsolete binding is (and won't be) supported for STM32MP15x variant of dwc_eth_qos driver. I am currenty upstream the support in U-boot of the support of the final binding aligned with Kernel one. "net: dwc_eth_qos: implement reset-gpios for stm32" http://patchwork.ozlabs.org/patch/1257377/ => phy-handle and "reset-gpios" in sub node PHY > mdio0 { > #address-cells = <1>; > -- > 2.25.1 Regards Patrick
On 3/31/20 4:06 PM, Patrick DELAUNAY wrote: > Hi Marek, > >> From: Marek Vasut <marex@denx.de> >> Sent: mardi 31 mars 2020 02:49 >> >> Add PHY reset GPIO on AV96 ethernet PHY. >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> Cc: Patrick Delaunay <patrick.delaunay@st.com> >> Cc: Patrice Chotard <patrice.chotard@st.com> >> --- >> V2: No change >> --- >> arch/arm/dts/stm32mp157a-avenger96.dts | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/arm/dts/stm32mp157a-avenger96.dts >> b/arch/arm/dts/stm32mp157a-avenger96.dts >> index fb8f0021b3..7a4b6e6a2c 100644 >> --- a/arch/arm/dts/stm32mp157a-avenger96.dts >> +++ b/arch/arm/dts/stm32mp157a-avenger96.dts >> @@ -101,6 +101,7 @@ >> phy-mode = "rgmii"; >> max-speed = <1000>; >> phy-handle = <&phy0>; >> + phy-reset-gpios = <&gpioz 2 GPIO_ACTIVE_LOW>; > > This obsolete binding is (and won't be) supported for STM32MP15x variant of dwc_eth_qos driver. > > I am currenty upstream the support in U-boot of the support of the final binding > aligned with Kernel one. > > "net: dwc_eth_qos: implement reset-gpios for stm32" > http://patchwork.ozlabs.org/patch/1257377/ That's probably gonna go in the next cycle, right? So you likely want to grab this for this cycle to fix the ethernet here and then in next cycle rework the bindings. btw you also want to look at this series: [PATCH 1/6] net: dwc_eth_qos: Fully rewrite RX descriptor field 3
Hi, > From: Marek Vasut <marex@denx.de> > Sent: mardi 31 mars 2020 19:07 > > On 3/31/20 4:06 PM, Patrick DELAUNAY wrote: > > Hi Marek, > > > >> From: Marek Vasut <marex@denx.de> > >> Sent: mardi 31 mars 2020 02:49 > >> > >> Add PHY reset GPIO on AV96 ethernet PHY. > >> > >> Signed-off-by: Marek Vasut <marex@denx.de> > >> Cc: Patrick Delaunay <patrick.delaunay@st.com> > >> Cc: Patrice Chotard <patrice.chotard@st.com> > >> --- > >> V2: No change > >> --- > >> arch/arm/dts/stm32mp157a-avenger96.dts | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/arch/arm/dts/stm32mp157a-avenger96.dts > >> b/arch/arm/dts/stm32mp157a-avenger96.dts > >> index fb8f0021b3..7a4b6e6a2c 100644 > >> --- a/arch/arm/dts/stm32mp157a-avenger96.dts > >> +++ b/arch/arm/dts/stm32mp157a-avenger96.dts > >> @@ -101,6 +101,7 @@ > >> phy-mode = "rgmii"; > >> max-speed = <1000>; > >> phy-handle = <&phy0>; > >> + phy-reset-gpios = <&gpioz 2 GPIO_ACTIVE_LOW>; > > > > This obsolete binding is (and won't be) supported for STM32MP15x variant of > dwc_eth_qos driver. > > > > I am currenty upstream the support in U-boot of the support of the > > final binding aligned with Kernel one. > > > > "net: dwc_eth_qos: implement reset-gpios for stm32" > > http://patchwork.ozlabs.org/patch/1257377/ > > That's probably gonna go in the next cycle, right? So you likely want to grab this > for this cycle to fix the ethernet here and then in next cycle rework the bindings. Yes in the next cycle probably. But the ethernet can't be not fixed with this patch, at least in U-boot.... as the reset by gpio is not executed in driver: it is not supported. Only present in eqos_probe_resources_tegra186 but not in eqos_probe_resources_stm32. Or you have perhaps a local patch on dwc_eth_qos.c I think I can drop this unneeded patch for the release... Or merge it, as is no effect. And after binding rework, the AV96 board DT should be updated (in kernel also, I think, as the old binding should be refused). > btw you also want to look at this series: > [PATCH 1/6] net: dwc_eth_qos: Fully rewrite RX descriptor field 3 Yes I see the serie.... I want to review and test it since 2 weeks. And particularly check if I can remove the CONFIG_SYS_NONCACHED_MEMORY in stm32mp1.h with this serie. I am not happy with this configuration (it is more a workaround for cache management issue in this driver that a real solution). But I am at home since 3 weeks with COVID-19, and my test capacity is reduced. I will review this serie this week. Thanks for reminder. Patrick
On 4/1/20 10:57 AM, Patrick DELAUNAY wrote: > Hi, Hi, >> From: Marek Vasut <marex@denx.de> >> Sent: mardi 31 mars 2020 19:07 >> >> On 3/31/20 4:06 PM, Patrick DELAUNAY wrote: >>> Hi Marek, >>> >>>> From: Marek Vasut <marex@denx.de> >>>> Sent: mardi 31 mars 2020 02:49 >>>> >>>> Add PHY reset GPIO on AV96 ethernet PHY. >>>> >>>> Signed-off-by: Marek Vasut <marex@denx.de> >>>> Cc: Patrick Delaunay <patrick.delaunay@st.com> >>>> Cc: Patrice Chotard <patrice.chotard@st.com> >>>> --- >>>> V2: No change >>>> --- >>>> arch/arm/dts/stm32mp157a-avenger96.dts | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/arch/arm/dts/stm32mp157a-avenger96.dts >>>> b/arch/arm/dts/stm32mp157a-avenger96.dts >>>> index fb8f0021b3..7a4b6e6a2c 100644 >>>> --- a/arch/arm/dts/stm32mp157a-avenger96.dts >>>> +++ b/arch/arm/dts/stm32mp157a-avenger96.dts >>>> @@ -101,6 +101,7 @@ >>>> phy-mode = "rgmii"; >>>> max-speed = <1000>; >>>> phy-handle = <&phy0>; >>>> + phy-reset-gpios = <&gpioz 2 GPIO_ACTIVE_LOW>; >>> >>> This obsolete binding is (and won't be) supported for STM32MP15x variant of >> dwc_eth_qos driver. >>> >>> I am currenty upstream the support in U-boot of the support of the >>> final binding aligned with Kernel one. >>> >>> "net: dwc_eth_qos: implement reset-gpios for stm32" >>> http://patchwork.ozlabs.org/patch/1257377/ >> >> That's probably gonna go in the next cycle, right? So you likely want to grab this >> for this cycle to fix the ethernet here and then in next cycle rework the bindings. > > Yes in the next cycle probably. > > But the ethernet can't be not fixed with this patch, at least in U-boot.... > as the reset by gpio is not executed in driver: it is not supported. > > Only present in eqos_probe_resources_tegra186 > but not in eqos_probe_resources_stm32. > > Or you have perhaps a local patch on dwc_eth_qos.c Oh doh, I don't. > I think I can drop this unneeded patch for the release... > Or merge it, as is no effect. > > And after binding rework, the AV96 board DT should be updated > (in kernel also, I think, as the old binding should be refused). Agreed. Drop it then. >> btw you also want to look at this series: >> [PATCH 1/6] net: dwc_eth_qos: Fully rewrite RX descriptor field 3 > > Yes I see the serie.... > > I want to review and test it since 2 weeks. > > And particularly check if I can remove the CONFIG_SYS_NONCACHED_MEMORY > in stm32mp1.h with this serie. We should consider that, yes, it looks somewhat crappy. Basically, your problem is that the descriptor is shorter than your cacheline, that means you cannot write into the descriptor right away after the MAC indicates the descriptor is complete, because you would evict the entire cache line and likely corrupt the neighboring descriptors too. What you would have to do is - invalidate the cacheline which contains the currently finished desc - read the desc status - IFF and only IFF this is the last desc in the cacheline, flag all the descriptors as ready for the DMA engine to use again, and only then flush the entire cacheline And you probably also want to make sure that the DWMAC4 "last usable descriptor" pointer does not point at these currently updated descriptors, so that the DWMAC DMA engine won't access them while you are flushing the cache line. This is certainly doable, one just has to be careful about the cache intricacies. > I am not happy with this configuration (it is more a workaround > for cache management issue in this driver that a real solution). > > But I am at home since 3 weeks with COVID-19, and my test > capacity is reduced. Get better soon!
diff --git a/arch/arm/dts/stm32mp157a-avenger96.dts b/arch/arm/dts/stm32mp157a-avenger96.dts index fb8f0021b3..7a4b6e6a2c 100644 --- a/arch/arm/dts/stm32mp157a-avenger96.dts +++ b/arch/arm/dts/stm32mp157a-avenger96.dts @@ -101,6 +101,7 @@ phy-mode = "rgmii"; max-speed = <1000>; phy-handle = <&phy0>; + phy-reset-gpios = <&gpioz 2 GPIO_ACTIVE_LOW>; mdio0 { #address-cells = <1>;
Add PHY reset GPIO on AV96 ethernet PHY. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Patrick Delaunay <patrick.delaunay@st.com> Cc: Patrice Chotard <patrice.chotard@st.com> --- V2: No change --- arch/arm/dts/stm32mp157a-avenger96.dts | 1 + 1 file changed, 1 insertion(+)