MIME-Version: 1.0
References: <20230124221218.341511-1-william.zhang@broadcom.com>
 <20230124221218.341511-3-william.zhang@broadcom.com> <abedd2e8-3c7e-f347-06af-99f2e5a2412b@linaro.org>
 <ee4727e1-5705-edb0-c724-2ae4d4d1a8e2@broadcom.com> <20230125205123.GA2864330-robh@kernel.org>
 <1489564a-59d3-6d38-fad7-02119bfedbeb@broadcom.com>
In-Reply-To: <1489564a-59d3-6d38-fad7-02119bfedbeb@broadcom.com>
From:   Rob Herring <robh@kernel.org>
Date:   Thu, 26 Jan 2023 07:56:54 -0600
Message-ID: <CAL_JsqL3CYCdamv15-kzvMgoYVpftJ0DoyB5L=LGVi-54GXP5Q@mail.gmail.com>
Subject: Re: [PATCH v2 02/14] dt-bindings: spi: Add bcmbca-hsspi controller support
To:     William Zhang <william.zhang@broadcom.com>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Linux SPI List <linux-spi@vger.kernel.org>,
        Broadcom Kernel List <bcm-kernel-feedback-list@broadcom.com>,
        tomer.yacoby@broadcom.com, kursad.oney@broadcom.com,
        dregan@mail.com, f.fainelli@gmail.com, anand.gore@broadcom.com,
        jonas.gorski@gmail.com, dan.beygelman@broadcom.com,
        joel.peshkin@broadcom.com,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Mark Brown <broonie@kernel.org>, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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