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

Add the device tree bindings for the MaxLinear GPY2xx PHYs.

Signed-off-by: Michael Walle <michael@walle.cc>
---

Is the filename ok? I was unsure because that flag is only for the GPY215
for now. But it might also apply to others. Also there is no compatible
string, so..

 .../bindings/net/maxlinear,gpy2xx.yaml        | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml

Am 2022-12-05 22:29, schrieb Rob Herring:
> On Fri, Dec 02, 2022 at 04:12:03PM +0100, Michael Walle wrote:
>> Add the device tree bindings for the MaxLinear GPY2xx PHYs.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>> 
>> Is the filename ok? I was unsure because that flag is only for the 
>> GPY215
>> for now. But it might also apply to others. Also there is no 
>> compatible
>> string, so..
>> 
>>  .../bindings/net/maxlinear,gpy2xx.yaml        | 47 
>> +++++++++++++++++++
>>  1 file changed, 47 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml 
>> b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
>> new file mode 100644
>> index 000000000000..d71fa9de2b64
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
>> @@ -0,0 +1,47 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/maxlinear,gpy2xx.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: MaxLinear GPY2xx PHY
>> +
>> +maintainers:
>> +  - Andrew Lunn <andrew@lunn.ch>
>> +  - Michael Walle <michael@walle.cc>
>> +
>> +allOf:
>> +  - $ref: ethernet-phy.yaml#
>> +
>> +properties:
>> +  maxlinear,use-broken-interrupts:
>> +    description: |
>> +      Interrupts are broken on some GPY2xx PHYs in that they keep the
>> +      interrupt line asserted even after the interrupt status 
>> register is
>> +      cleared. Thus it is blocking the interrupt line which is 
>> usually bad
>> +      for shared lines. By default interrupts are disabled for this 
>> PHY and
>> +      polling mode is used. If one can live with the consequences, 
>> this
>> +      property can be used to enable interrupt handling.
> 
> 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".

>> +
>> +      Affected PHYs (as far as known) are GPY215B and GPY215C.
>> +    type: boolean
>> +
>> +dependencies:
>> +  maxlinear,use-broken-interrupts: [ interrupts ]
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    ethernet {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        ethernet-phy@0 {
>> +            reg = <0>;
>> +            interrupts-extended = <&intc 0>;
>> +            maxlinear,use-broken-interrupts;
> 
> This is never actually checked by be schema because there is nothing to
> match on. If you want custom properties, then you need a compatible.

This seems to be a problem for any phy bindings then.

-michael

Am 2022-12-05 22:53, schrieb Michael Walle:
> Am 2022-12-05 22:29, schrieb Rob Herring:
>> On Fri, Dec 02, 2022 at 04:12:03PM +0100, Michael Walle wrote:
>>> Add the device tree bindings for the MaxLinear GPY2xx PHYs.
>>> 
>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>> ---
>>> 
>>> Is the filename ok? I was unsure because that flag is only for the 
>>> GPY215
>>> for now. But it might also apply to others. Also there is no 
>>> compatible
>>> string, so..
>>> 
>>>  .../bindings/net/maxlinear,gpy2xx.yaml        | 47 
>>> +++++++++++++++++++
>>>  1 file changed, 47 insertions(+)
>>>  create mode 100644 
>>> Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
>>> 
>>> diff --git 
>>> a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml 
>>> b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
>>> new file mode 100644
>>> index 000000000000..d71fa9de2b64
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
>>> @@ -0,0 +1,47 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/net/maxlinear,gpy2xx.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: MaxLinear GPY2xx PHY
>>> +
>>> +maintainers:
>>> +  - Andrew Lunn <andrew@lunn.ch>
>>> +  - Michael Walle <michael@walle.cc>
>>> +
>>> +allOf:
>>> +  - $ref: ethernet-phy.yaml#
>>> +
>>> +properties:
>>> +  maxlinear,use-broken-interrupts:
>>> +    description: |
>>> +      Interrupts are broken on some GPY2xx PHYs in that they keep 
>>> the
>>> +      interrupt line asserted even after the interrupt status 
>>> register is
>>> +      cleared. Thus it is blocking the interrupt line which is 
>>> usually bad
>>> +      for shared lines. By default interrupts are disabled for this 
>>> PHY and
>>> +      polling mode is used. If one can live with the consequences, 
>>> this
>>> +      property can be used to enable interrupt handling.
>> 
>> 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".

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).

-michael

On 06/12/2022 09:29, Michael Walle wrote:
> Am 2022-12-05 22:53, schrieb Michael Walle:
>> Am 2022-12-05 22:29, schrieb Rob Herring:
>>> On Fri, Dec 02, 2022 at 04:12:03PM +0100, Michael Walle wrote:
>>>> Add the device tree bindings for the MaxLinear GPY2xx PHYs.
>>>>
>>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>>> ---
>>>>
>>>> Is the filename ok? I was unsure because that flag is only for the 
>>>> GPY215
>>>> for now. But it might also apply to others. Also there is no 
>>>> compatible
>>>> string, so..
>>>>
>>>>  .../bindings/net/maxlinear,gpy2xx.yaml        | 47 
>>>> +++++++++++++++++++
>>>>  1 file changed, 47 insertions(+)
>>>>  create mode 100644 
>>>> Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
>>>>
>>>> diff --git 
>>>> a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml 
>>>> b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
>>>> new file mode 100644
>>>> index 000000000000..d71fa9de2b64
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
>>>> @@ -0,0 +1,47 @@
>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/net/maxlinear,gpy2xx.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: MaxLinear GPY2xx PHY
>>>> +
>>>> +maintainers:
>>>> +  - Andrew Lunn <andrew@lunn.ch>
>>>> +  - Michael Walle <michael@walle.cc>
>>>> +
>>>> +allOf:
>>>> +  - $ref: ethernet-phy.yaml#
>>>> +
>>>> +properties:
>>>> +  maxlinear,use-broken-interrupts:
>>>> +    description: |
>>>> +      Interrupts are broken on some GPY2xx PHYs in that they keep 
>>>> the
>>>> +      interrupt line asserted even after the interrupt status 
>>>> register is
>>>> +      cleared. Thus it is blocking the interrupt line which is 
>>>> usually bad
>>>> +      for shared lines. By default interrupts are disabled for this 
>>>> PHY and
>>>> +      polling mode is used. If one can live with the consequences, 
>>>> this
>>>> +      property can be used to enable interrupt handling.
>>>
>>> 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?

> 
> 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? Isn't then the property not really
correct? Broken for one firmware on the same device, working for other
firmware on the same device?

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



Best regards,
Krzysztof

>> +
>> +      Affected PHYs (as far as known) are GPY215B and GPY215C.
>> +    type: boolean
>> +
>> +dependencies:
>> +  maxlinear,use-broken-interrupts: [ interrupts ]

Btw. I'd presume that the tools will also allow interrupts-extended, but 
that
doesn't seem to be the case. Do I need some kind of anyOf here?

>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    ethernet {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        ethernet-phy@0 {
>> +            reg = <0>;
>> +            interrupts-extended = <&intc 0>;
>> +            maxlinear,use-broken-interrupts;
> 
> This is never actually checked by be schema because there is nothing to
> match on. If you want custom properties, then you need a compatible.

I can add an unwanted compatible here, or skip the example altogether. 
But
what puzzles me is that this schema pulls in the ethernet-phy.yaml. The 
latter
then has a custom select statement on the $nodename and even a comment:

# The dt-schema tools will generate a select statement first by using
# the compatible, and second by using the node name if any. In our
# case, the node name is the one we want to match on, while the
# compatible is optional.

Why doesn't that work?

-michael