Am 2023-01-11 21:26, schrieb Rob Herring:
On Mon, Jan 09, 2023 at 01:30:11PM +0100, Michael Walle wrote:
Add the device tree bindings for the MaxLinear GPY2xx PHYs, which
essentially adds just one flag: maxlinear,use-broken-interrupts.
One might argue, that if interrupts are broken, just don't use
the interrupt property in the first place. But it needs to be more
nuanced. First, this interrupt line is also used to wake up systems by
WoL, which has nothing to do with the (broken) PHY interrupt handling.
I don't understand how this is useful. If the interrupt line is
asserted
after the 1st interrupt, how is it ever deasserted later on to be
useful.
Nobody said, that the interrupt line will stay asserted. The broken
behavior is that of the PHY doesn't respond *immediately* with a
deassertion of the interrupt line after the its internal status
register is cleared. Instead there is a random delay of up to 2ms.
There is already a workaround to avoid an interrupt storm by delaying
the ISR until the line is actually cleared. *But* if this line is
a shared one. The line is blocked by these 2ms and important
interrupts (like PTP timestaming) of other devices on this line
will get delayed. Therefore, the only viabale option is to disable
the interrupts handling in the broken PHY altogether. I.e. the line
will never be asserted by the broken PHY.
In any case, you could use 'wakeup-source' if that's the functionality
you need. Then just ignore the interrupt if 'wakeup-source' is not
present.
Right, that would work for the first case. But not if someone really
wants to use interrupts with the PHY, which is still a valid scenario
if it has a dedicated interrupt line.
Second and more importantly, there are devicetrees which have this
property set. Thus, within the driver we have to switch off interrupt
handling by default as a workaround. But OTOH, a systems designer who
knows the hardware and knows there are no shared interrupts for
example,
can use this new property as a hint to the driver that it can enable
the
interrupt nonetheless.
Pretty sure I said this already, but this schema has no effect. Add an
extra property to the example and see. No error despite your
'unevaluatedProperties: false'. Or drop 'interrupts-extended' and no
dependency error...
I know, I noticed this the first time I tested the schema. But then
I've looked at all the other PHY binding and not one has a compatible.
I presume if there is a compatible, the devicetrees also need a
compatible. So basically, "required: compatible" in the schema, right?
But that is where the PHY maintainers don't agree.
You won't get errors as there's no defined way to decide when to apply
this because it is based on node name or compatible unless you do a
custom select, but I don't see what you would key off of here...
Actually, in the previous version I've asked why the custom select
of ethernet-phy.yaml doesn't get applied here, when there is a
"allOf: $ref ethernet-phy.yaml".
-michael
The real answer here is add a compatible. But I'm tired of pointing
this
out to the networking maintainers every damn time. Ethernet PHYs are
not
special.
Rob
