Re: [PATCH net-next v1 3/4] dt-bindings: net: phy: add MaxLinear GPY2xx bindings

From: Michael Walle
Date: Tue Dec 06 2022 - 04:44:14 EST


Am 2022-12-06 09:38, schrieb Krzysztof Kozlowski:

Just omit the interrupt property if you don't want interrupts and
add it if you do.

How does that work together with "the device tree describes
the hardware and not the configuration". The interrupt line
is there, its just broken sometimes and thus it's disabled
by default for these PHY revisions/firmwares. With this
flag the user can say, "hey on this hardware it is not
relevant because we don't have shared interrupts or because
I know what I'm doing".

Yeah, that's a good question. In your case broken interrupts could be
understood the same as "not connected", so property not present. When
things are broken, you do not describe them fully in DTS for the
completeness of hardware description, right?

I'd agree here, but in this case it's different. First, it isn't
obvious in the first place that things are broken and boards in
the field wouldn't/couldn't get that update. I'd really expect
an erratum from MaxLinear here. And secondly, (which I
just noticed right now, sorry), is that the interrupt line
is also used for wake-on-lan, which can also be used even for
the "broken" PHYs.

To work around this, the basic idea was to just disable the
normal interrupts and fall back to polling mode, as the PHY
driver just use it for link detection and don't offer any
advanced features like PTP (for now). But still get the system
integrator a knob to opt-in to the old behavior on new device
trees.

Specifically you can't do the following: Have the same device
tree and still being able to use it with a future PHY firmware
update/revision. Because according to your suggestion, this
won't have the interrupt property set. With this flag you can
have the following cases:
(1) the interrupt information is there and can be used in the
future by non-broken PHY revisions,
(2) broken PHYs will ignore the interrupt line
(3) except the system designer opts-in with this flag (because
maybe this is the only PHY on the interrupt line etc).

I am not sure if I understand the case. You want to have a DTS with
interrupts and "maxlinear,use-broken-interrupts", where the latter will
be ignored by some future firmware?

Yes, that's correct.

Isn't then the property not really correct? Broken for one firmware
on the same device, working for other firmware on the same device?

Arguable, but you can interpret "use broken-interrupts" as no-op
if there are no broken interrupts.

I would assume that in such cases you (or bootloader or overlay)
should patch the DTS...

I think this would turn the opt-in into an opt-out and we'd rely
on the bootloader to workaround the erratum. Which isn't what we
want here.

-michael