Message-ID: <6c82b403962aaf1450eb5014c9908328@walle.cc>
Date:   Tue, 06 Dec 2022 10:44:04 +0100
From:   Michael Walle <michael@...le.cc>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Rob Herring <robh@...nel.org>, Xu Liang <lxu@...linear.com>,
        Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH net-next v1 3/4] dt-bindings: net: phy: add MaxLinear
 GPY2xx bindings

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

Powered by blists - more mailing lists