On 30/10/2023 20:07, Cristian Ciocaltea wrote:

On 10/30/23 09:26, Krzysztof Kozlowski wrote:

quoted

On 29/10/2023 23:24, Cristian Ciocaltea wrote:

quoted

On 10/29/23 13:25, Krzysztof Kozlowski wrote:

quoted

On 29/10/2023 05:27, Cristian Ciocaltea wrote:

quoted

The Synopsys DesignWare MAC found on the StarFive JH7100 SoC requires
just the 'ahb' reset name, but the binding allows selecting it only in
conjunction with 'stmmaceth'.

Fix the issue by permitting exclusive usage of the 'ahb' reset name.

Signed-off-by: Cristian Ciocaltea <redacted>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 5c2769dc689a..a4d7172ea701 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

@@ -146,7 +146,7 @@ properties:
   reset-names:
     minItems: 1
     items:
-      - const: stmmaceth
+      - enum: [stmmaceth, ahb]

Also, this makes sense only with patch #4, so this should be squashed there.

I added this as a separate patch since it changes the generic schema
which is included by many other bindings.  JH7100 just happens to be the
first use-case requiring this update.  But I can squash the patch if
that's not a good enough reason to keep it separately.

If there is no single user of this, why changing this? I would even
argue that it is not correct from existing bindings point of view -
nothing allows and uses ahb as the only reset. Even the commit msg
mentions your hardware from patch 4.

Sorry, I'm not sure I follow. JH7100 is (or will be) the user of it and,
as a matter of fact, something similar has been done recently while
adding support for JH7110.

Every patch should stand on its own and at this point nothing uses it.
We apply this patch #1 and we add dead code, unused case.

In particular, commit [1] changed this binding before the JH7110
compatible was introduced in a subsequent patch. On a closer look that
commit made a statement which is not entirely correct:

"dwmac controller may require one (stmmaceth) or two (stmmaceth+ahb)
reset signals"

That's because stmmaceth is also optional in dwmac's driver, hence the
correct message would have been:

"[...] may require one (stmmaceth OR ahb) [...]"

Driver does not describe the hardware. The bindings do, so according to
that description all supported hardware required MAC reset (stmmaceth).
Otherwise please point me to any hardware which skips MAC reset and
requires AHB reset instead (not future hardware, but current).

Hence, I think it makes sense to keep this patch, after adding the above
details in the commit message.

[1] 843f603762a5 ("dt-bindings: net: snps,dwmac: Add 'ahb'
reset/reset-name")

Thanks,
Cristian

Best regards,
Krzysztof