netdev - Re: [PATCH 1/3] dt-bings: net: fsl,fec: update compatible item

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 18 Aug 2022 12:46:33 +0300
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Shawn Guo <shawnguo@...nel.org>
Cc:     Wei Fang <wei.fang@....com>, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        s.hauer@...gutronix.de, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel@...gutronix.de, festevam@...il.com, linux-imx@....com,
        peng.fan@....com, ping.bai@....com, sudeep.holla@....com,
        linux-arm-kernel@...ts.infradead.org, aisheng.dong@....com
Subject: Re: [PATCH 1/3] dt-bings: net: fsl,fec: update compatible item

On 18/08/2022 12:22, Shawn Guo wrote:
> On Thu, Aug 18, 2022 at 10:51:02AM +0300, Krzysztof Kozlowski wrote:
>> On 18/08/2022 04:33, Shawn Guo wrote:
>>> On Mon, Jul 04, 2022 at 11:12:09AM +0200, Krzysztof Kozlowski wrote:
>>>>> diff --git a/Documentation/devicetree/bindings/net/fsl,fec.yaml b/Documentation/devicetree/bindings/net/fsl,fec.yaml
>>>>> index daa2f79a294f..6642c246951b 100644
>>>>> --- a/Documentation/devicetree/bindings/net/fsl,fec.yaml
>>>>> +++ b/Documentation/devicetree/bindings/net/fsl,fec.yaml
>>>>> @@ -40,6 +40,10 @@ properties:
>>>>>            - enum:
>>>>>                - fsl,imx7d-fec
>>>>>            - const: fsl,imx6sx-fec
>>>>> +      - items:
>>>>> +          - enum:
>>>>> +              - fsl,imx8ulp-fec
>>>>> +          - const: fsl,imx6ul-fec
>>>>
>>>> This is wrong.  fsl,imx6ul-fec has to be followed by fsl,imx6q-fec. I
>>>> think someone made similar mistakes earlier so this is a mess.
>>>
>>> Hmm, not sure I follow this.  Supposing we want to have the following
>>> compatible for i.MX8ULP FEC, why do we have to have "fsl,imx6q-fec"
>>> here?
>>>
>>> 	fec: ethernet@...50000 {
>>> 		compatible = "fsl,imx8ulp-fec", "fsl,imx6ul-fec";
>>> 		...
>>> 	};
>>
>> Because a bit earlier this bindings is saying that fsl,imx6ul-fec must
>> be followed by fsl,imx6q-fec.
> 
> The FEC driver OF match table suggests that fsl,imx6ul-fec and fsl,imx6q-fec
> are not really compatible.
> 
> static const struct of_device_id fec_dt_ids[] = {
>         { .compatible = "fsl,imx25-fec", .data = &fec_devtype[IMX25_FEC], },
>         { .compatible = "fsl,imx27-fec", .data = &fec_devtype[IMX27_FEC], },
>         { .compatible = "fsl,imx28-fec", .data = &fec_devtype[IMX28_FEC], },
>         { .compatible = "fsl,imx6q-fec", .data = &fec_devtype[IMX6Q_FEC], },
>         { .compatible = "fsl,mvf600-fec", .data = &fec_devtype[MVF600_FEC], },
>         { .compatible = "fsl,imx6sx-fec", .data = &fec_devtype[IMX6SX_FEC], },
>         { .compatible = "fsl,imx6ul-fec", .data = &fec_devtype[IMX6UL_FEC], },

I don't see here any incompatibility. Binding driver with different
driver data is not a proof of incompatible devices. Additionally, the
binding describes the hardware, not the driver.

>         { .compatible = "fsl,imx8mq-fec", .data = &fec_devtype[IMX8MQ_FEC], },
>         { .compatible = "fsl,imx8qm-fec", .data = &fec_devtype[IMX8QM_FEC], },
>         { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, fec_dt_ids);
> 
> Should we fix the binding doc?

Maybe, I don't know. The binding describes the hardware, so based on it
the devices are compatible. Changing this, except ABI impact, would be
possible with proper reason, but not based on Linux driver code.


Best regards,
Krzysztof