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

[Krzysztof Kozlowski]

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@29950000 {
>>> 		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