linux-kernel - Re: [PATCH v2 02/14] dt-bindings: spi: Add bcmbca-hsspi controller support

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, 26 Jan 2023 12:04:06 -0800
From:   William Zhang <william.zhang@...adcom.com>
To:     Rob Herring <robh@...nel.org>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Linux SPI List <linux-spi@...r.kernel.org>,
        Broadcom Kernel List <bcm-kernel-feedback-list@...adcom.com>,
        tomer.yacoby@...adcom.com, kursad.oney@...adcom.com,
        dregan@...l.com, f.fainelli@...il.com, anand.gore@...adcom.com,
        jonas.gorski@...il.com, dan.beygelman@...adcom.com,
        joel.peshkin@...adcom.com,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Mark Brown <broonie@...nel.org>, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 02/14] dt-bindings: spi: Add bcmbca-hsspi controller
 support



On 01/26/2023 05:56 AM, Rob Herring wrote:
> On Wed, Jan 25, 2023 at 3:41 PM William Zhang
> <william.zhang@...adcom.com> wrote:
>> On 01/25/2023 12:51 PM, Rob Herring wrote:
>>> On Wed, Jan 25, 2023 at 11:23:52AM -0800, William Zhang wrote:
>>>> On 01/24/2023 11:35 PM, Krzysztof Kozlowski wrote:
>>>>> On 24/01/2023 23:12, William Zhang wrote:
>>>>>> The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
>>>>>> controller. Add new compatible strings to differentiate the old and new
>>>>>> controller while keeping MIPS based chip with the old compatible. Update
>>>>>> property requirements for these two revisions of the controller.  Also
>>>>>> add myself and Kursad as the maintainers.
> 
> [...]
> 
>>>>>>     properties:
>>>>>>       compatible:
>>>>>> -    const: brcm,bcm6328-hsspi
>>>>>> +    oneOf:
>>>>>> +      - const: brcm,bcm6328-hsspi
>>>>>> +      - items:
>>>>>> +          - enum:
>>>>>> +              - brcm,bcm47622-hsspi
>>>>>> +              - brcm,bcm4908-hsspi
>>>>>> +              - brcm,bcm63138-hsspi
>>>>>> +              - brcm,bcm63146-hsspi
>>>>>> +              - brcm,bcm63148-hsspi
>>>>>> +              - brcm,bcm63158-hsspi
>>>>>> +              - brcm,bcm63178-hsspi
>>>>>> +              - brcm,bcm6846-hsspi
>>>>>> +              - brcm,bcm6856-hsspi
>>>>>> +              - brcm,bcm6858-hsspi
>>>>>> +              - brcm,bcm6878-hsspi
>>>>>> +          - const: brcm,bcmbca-hsspi-v1.0
>>>>>> +          - const: brcm,bcmbca-hsspi
>>>>>
>>>>> Why do you need "brcm,bcmbca-hsspi"? Nothing binds to it, so it's
>>>>> useless and very generic.
>>>>>
>>>> This was from Florian's suggestion and Broadcom's convention. See [1] and
>>>> you are okay with that [2].  I added the rev compatible and you were not
>>>> objecting it finally if I understand you correctly.
>>>
>>> Can you have a driver that only understands what 'brcm,bcmbca-hsspi' is
>>> work on all h/w that includes the compatible string? It doesn't seem
>>> like it since v1.1 is a completely new driver. Therefore
>>> 'brcm,bcmbca-hsspi' is pretty much useless.
>>>
>> 'brcm,bcmbca-hsspi' should be added to the binding table of
>> spi-bcm63xx-hsspi.c driver.   This is the initial driver that works for
>> v1.0 controller.  For v1.1 controller, yes it can fallback and work with
>> 1.0 driver spi-bcm63xx-hsspi.c simply not using the new feature in
>> v1.1(chip select signal control through software) and keeping using the
>> prepend mode or dummy cs workaround supported in 1.0 driver.
> 
> If v1.1 is compatible with v1.0, then say that:
> 
> soc-compat, "brcm,bcmbca-hsspi-v1.1", "brcm,bcmbca-hsspi-v1.0"
> 
> IOW, 'brcm,bcmbca-hsspi' is redundant with 'brcm,bcmbca-hsspi-v1.0'.
> They have the same meaning. So pick which one you want to use. Not
> both.
> 
Agree brcm,bcmbca-hsspi fallback is redundant to brcm,bcmbca-hsspi-v1.0. 
I added it to conform Broadcom convention.  But I understand your and 
Krzystof's concern so I'll drop the brcm,bcmbca-hsspi in v3 to get 
things going.

> Also, if that is the case, you shouldn't be introducing a whole new
> driver for v1.1.
> 
Technically I can combine the new feature to the existing driver v1.0 
but I prefer to start the a new driver so it will be much cleaner and 
simpler to follow without all the workarounds and complex logic.

> Rob
> 

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