[PATCH V2 11/14] ARM: dts: stm32: Add missing ethernet PHY reset on AV96

[Marek Vasut]

On 4/1/20 10:57 AM, Patrick DELAUNAY wrote:
> Hi,

Hi,

>> From: Marek Vasut <marex at 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 at denx.de>
>>>> Sent: mardi 31 mars 2020 02:49
>>>>
>>>> Add PHY reset GPIO on AV96 ethernet PHY.
>>>>
>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>> Cc: Patrick Delaunay <patrick.delaunay at st.com>
>>>> Cc: Patrice Chotard <patrice.chotard at 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!

-- 
Best regards,
Marek Vasut