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 07:56:54 -0600
From:   Rob Herring <robh@...nel.org>
To:     William Zhang <william.zhang@...adcom.com>
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 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.

Also, if that is the case, you shouldn't be introducing a whole new
driver for v1.1.

Rob