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

[Rob Herring]

On Wed, Jan 25, 2023 at 3:41 PM William Zhang
<william.zhang@xxxxxxxxxxxx> 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