Re: [PATCH net-next v2 1/3] dt-bindings: net: mscc-miim: add lan966x compatible

Michael Walle
2022-03-13T16:30:42Z

[adding Horatiu and Kavyasree from Microchip]

Am 2022-03-13 17:10, schrieb Krzysztof Kozlowski:
> On 13/03/2022 11:47, Michael Walle wrote:
>> Am 2022-03-13 10:47, schrieb Krzysztof Kozlowski:
>>> On 13/03/2022 01:25, Michael Walle wrote:
>>>> The MDIO controller has support to release the internal PHYs from
>>>> reset
>>>> by specifying a second memory resource. This is different between
>>>> the
>>>> currently supported SparX-5 and the LAN966x. Add a new compatible to
>>>> distiguish between these two.
>
> Typo here, BTW.
>
>>>>
>>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>>> ---
>>>> Documentation/devicetree/bindings/net/mscc-miim.txt | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/mscc-miim.txt
>>>> b/Documentation/devicetree/bindings/net/mscc-miim.txt
>>>> index 7104679cf59d..a9efff252ca6 100644
>>>> --- a/Documentation/devicetree/bindings/net/mscc-miim.txt
>>>> +++ b/Documentation/devicetree/bindings/net/mscc-miim.txt
>>>> @@ -2,7 +2,7 @@ Microsemi MII Management Controller (MIIM) / MDIO
>>>> =================================================
>>>>
>>>> Properties:
>>>> -- compatible: must be "mscc,ocelot-miim"
>>>> +- compatible: must be "mscc,ocelot-miim" or "mscc,lan966x-miim"
>>>
>>> No wildcards, use one, specific compatible.
>>
>> I'm in a kind of dilemma here, have a look yourself:
>> grep -r "lan966[28x]-" Documentation
>>
>> Should I deviate from the common "name" now? To make things
>> worse, there was a similar request by Arnd [1]. But the
>> solution feels like cheating ("lan966x" -> "lan966") ;)
>
> The previous 966x cases were added by one person from Microchip, so he
> actually might know something. But do you know whether lan966x will
> cover all current and future designs from Microchip? E.g. lan9669 (if
> ever made) will be the same? Avoiding wildcard is the easiest, just
> choose one implementation, e.g. "lan9662".

So if Microchip would review/ack this it would be ok? I don't really
have a strong opinion, I just want to avoid any inconsistencies. If no
one from Microchip will answer, I'll use microchip,lan9668-miim.

> Different topic is that all current lan966[28] are from Microchip and
> you still add Microsemi, even though it was acquired by Microchip.
> That's an inconsistency which should be rather fixed.

Agreed, that was an oversight by me.

>> On a side note, I understand that there should be no wildcards,
>> because the compatible should target one specific implementation,
>> right? But then the codename "ocelot" represents a whole series of
>> chips. Therefore, names for whole families shouldn't be used neither,
>> right?
>
> You're not adding "ocelot" now, so it is separate topic. However a
> compatible like "mscc,ocelot" feels wrong, unless it is used as a
> fallback (see: git grep 'apple,').

Sure, it was just a question for my understanding, not to make a
point for a discussion.

-michael