linux-kernel - Re: [PATCH v3 2/4] spi: dt-binding: add coreqspi as a fallback for mpfs-qspi

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:   Fri, 5 Aug 2022 08:44:39 +0000
From:   <Conor.Dooley@...rochip.com>
To:     <krzysztof.kozlowski@...aro.org>, <Nagasuresh.Relli@...rochip.com>,
        <broonie@...nel.org>, <robh+dt@...nel.org>,
        <krzysztof.kozlowski+dt@...aro.org>
CC:     <linux-spi@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <Valentina.FernandezAlanis@...rochip.com>
Subject: Re: [PATCH v3 2/4] spi: dt-binding: add coreqspi as a fallback for
 mpfs-qspi

On 05/08/2022 09:12, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 05/08/2022 09:34, Conor.Dooley@...rochip.com wrote:
>> On 05/08/2022 07:49, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 05/08/2022 07:30, Naga Sureshkumar Relli wrote:
>>>> diff --git a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
>>>> index a47d4923b51b..84d32c1a4d60 100644
>>>> --- a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
>>>> +++ b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml
>>>> @@ -18,10 +18,12 @@ allOf:
>>>>
>>>>    properties:
>>>>      compatible:
>>>> -    enum:
>>>> -      - microchip,mpfs-spi
>>>> -      - microchip,mpfs-qspi
>>>> -      - microchip,coreqspi-rtl-v2 # FPGA QSPI
>>>> +   oneOf:
>>>> +    - items:
>>>> +        - const: microchip,mpfs-qspi
>>>> +        - const: microchip,coreqspi-rtl-v2
>>>
>>> Eh, this does not make sense after looking at your driver...
>>
>> What is wrong with explicitly binding the driver to both of the
>> compatible strings? The "hard" peripheral in the SoC part of the
>> FPGA is a superset of version 2 of the coreQSPI IP so the fallback
>> used in the binding here makes sense to me. coreQSPI can be
>> instantiated in the FPGA fabric and used there, so it needs a
>> compatible of its own.
>>
>> That brings me back to the original point question, why not
>> explicitly bind the driver to both of the compatible strings it
>> is known to work for?
> 
> There is nothing particularly bad with matching to both of compatibles.
> It is valid code. There are however questions/issues with that:
> 
> 1. It is redundant. I did not look too much at the driver, but none of
> the of_device_id entries have some driver data to differentiate,
> therefore - for the driver - the devices are identical. If they are
> identical and according to binding compatible, use less code and just
> one compatible.

Right. Then the binding is correct and the driver should only bind
against "microchip,coreqspi-rtl-v2".

> 
> 2. Otherwise, maybe the devices are not actually fully compatible?
> That's the second problem. If one writes binding like that and codes it
> in driver differently, it looks like it was not investigated really and
> I ask questions...
> 
> Best regards,
> Krzysztof