linux-kernel - Re: [PATCH 1/4] dt-bindings: memory: additional compatible strings for Broadcom DPFE

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]

Message-ID: <25177e5c-880e-4c7b-8a72-2d908a970afb@broadcom.com>
Date:   Wed, 6 Dec 2023 09:36:39 -0800
From:   Florian Fainelli <florian.fainelli@...adcom.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Markus Mayer <mmayer@...adcom.com>,
        Rob Herring <robh+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>
Cc:     Linux ARM Kernel List <linux-arm-kernel@...ts.infradead.org>,
        Device Tree Mailing List <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/4] dt-bindings: memory: additional compatible strings
 for Broadcom DPFE

On 12/6/23 09:29, Krzysztof Kozlowski wrote:
> On 06/12/2023 17:32, Florian Fainelli wrote:
>>
>>
>> On 12/6/2023 3:09 AM, Krzysztof Kozlowski wrote:
>>> On 05/12/2023 19:47, Markus Mayer wrote:
>>>> Add versioned compatible strings for Broadcom DPFE. These take the form
>>>> brcm,dpfe-cpu-v<N> where <N> is a number from 1 to 4.
>>>>
>>>> These API version related compatible strings are more specific than the
>>>> catch-all "brcm,dpfe-cpu" and more generic than chip-specific compatible
>>>> strings.
>>>
>>> None of this explains: Why? I don't see any point in this and commit
>>> does not explain.
>>>
>>>>
>>>> Signed-off-by: Markus Mayer <mmayer@...adcom.com>
>>>> ---
>>>>    .../bindings/memory-controllers/brcm,dpfe-cpu.yaml        | 8 +++++++-
>>>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/memory-controllers/brcm,dpfe-cpu.yaml b/Documentation/devicetree/bindings/memory-controllers/brcm,dpfe-cpu.yaml
>>>> index 08cbdcddfead..6dffa7b62baf 100644
>>>> --- a/Documentation/devicetree/bindings/memory-controllers/brcm,dpfe-cpu.yaml
>>>> +++ b/Documentation/devicetree/bindings/memory-controllers/brcm,dpfe-cpu.yaml
>>>> @@ -16,6 +16,11 @@ properties:
>>>>          - enum:
>>>>              - brcm,bcm7271-dpfe-cpu
>>>>              - brcm,bcm7268-dpfe-cpu
>>>> +      - enum:
>>>> +          - brcm,dpfe-cpu-v1
>>>> +          - brcm,dpfe-cpu-v2
>>>> +          - brcm,dpfe-cpu-v3
>>>> +          - brcm,dpfe-cpu-v4
>>>
>>> No, that's just wrong. So you want to say bcm7271 is brcm,dpfe-cpu-v4?
>>
>> No as the example shows it "speaks" API v1.
> 
> Example is not a binding. It does not matter except of validating the
> binding. This is just incorrect.
> 
>>
>> I would be inclined to completely remove the chip specific compatible
>> strings from the binding because they are not sufficient or descriptive
>> enough to determine which API version is being spoken, since the
>> firmware is unfortunately allowed to change major APIs (and the
>> messaging format, because why not?) at a moments notice.
> 
> Then versions do not give you anything more.

The versions indicate exactly which API to be spoken to with the 
firmware. The firmware API was not properly designed, it should have had 
a way to indicate which API it has, regardless of the messaging format 
it implements, but for reasons unknown that is not how it was implemented.

Essentially we need to know right away and ahead of time which API to be 
used, otherwise that means doing runtime detection like what patch 4 
does which you do not want to see.
-- 
Florian


Download attachment "smime.p7s" of type "application/pkcs7-signature" (4221 bytes)