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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 01/26/2023 05:56 AM, Rob Herring wrote:
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.

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

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature


[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux