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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2d35127c-d4ef-6644-289a-5c10bcbbbf84@kernel.org>
Date:   Sun, 13 Mar 2022 17:10:02 +0100
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Michael Walle <michael@...le.cc>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Rob Herring <robh+dt@...nel.org>, Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2 1/3] dt-bindings: net: mscc-miim: add lan966x
 compatible

On 13/03/2022 11:47, Michael Walle wrote:
> Hi Krzysztof,
> 
> 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@...le.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".

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.

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


Best regards,
Krzysztof