On 09/01/2023 20:13, William Zhang wrote:Ah you are right. Add minItems: 1 for reg-name then.
On 01/09/2023 12:56 AM, Krzysztof Kozlowski wrote:
On 09/01/2023 09:27, William Zhang wrote:I saw a few other vendors like these qcom ones:
Hi Krzysztof,
On 01/08/2023 06:51 AM, Krzysztof Kozlowski wrote:
On 06/01/2023 21:07, William Zhang wrote:will fix in v2.
The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
controller. Add a new compatible string and required fields for the new
driver. Also add myself and Kursad as the maintainers.
Signed-off-by: William Zhang <william.zhang@xxxxxxxxxxxx>
---
.../bindings/spi/brcm,bcm63xx-hsspi.yaml | 84 +++++++++++++++++--
1 file changed, 78 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
index 45f1417b1213..56e69d4a1faf 100644
--- a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
+++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
@@ -4,22 +4,51 @@
$id: http://devicetree.org/schemas/spi/brcm,bcm63xx-hsspi.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: Broadcom BCM6328 High Speed SPI controller
+title: Broadcom Broadband SoC High Speed SPI controller
maintainers:
+
Drop blank line.
"bca" is not model number, rather it is a group (broadband carrier
+ - William Zhang <william.zhang@xxxxxxxxxxxx>
+ - Kursad Oney <kursad.oney@xxxxxxxxxxxx>
- Jonas Gorski <jonas.gorski@xxxxxxxxx>
+description: |
+ Broadcom Broadband SoC supports High Speed SPI master controller since the
+ early MIPS based chips such as BCM6328 and BCM63268. This controller was
+ carried over to recent ARM based chips, such as BCM63138, BCM4908 and BCM6858.
+
+ It has a limitation that can not keep the chip select line active between
+ the SPI transfers within the same SPI message. This can terminate the
+ transaction to some SPI devices prematurely. The issue can be worked around by
+ either the controller's prepend mode or using the dummy chip select
+ workaround. This controller uses the compatible string brcm,bcm6328-hsspi.
+
+ The newer SoCs such as BCM6756, BCM4912 and BCM6855 include an updated SPI
+ controller that add the capability to allow the driver to control chip select
+ explicitly. This solves the issue in the old controller. This new controller
+ uses the compatible string brcm,bcmbca-hsspi.
+
properties:
compatible:
- const: brcm,bcm6328-hsspi
+ enum:
+ - brcm,bcm6328-hsspi
+ - brcm,bcmbca-hsspi
bca seems quite unspecific. Your description above mentions several
model numbers and "bca" is not listed as model. Compatibles cannot be
generic.
access) of chip that share the same spi host controller IP. Agree it is
not particularly specific but it differentiate from other broadcom spi
controller ip used by other groups. We just don't have a specific name
for this spi host controller but can we treat bcmbca as the ip name?
No, it is discouraged in such forms. Family or IP block compatibles
should be prepended with a specific compatible. There were many issues
when people insisted on generic or family compatibles...
Otherwise we will have to have a compatible string with chip model for
each SoC even they share the same IP. We already have more than ten of
SoCs and the list will increase. I don't see this is a good solution too.
You will have to do it anyway even with generic fallback, so I don't get
what is here to gain... I also don't get why Broadcom should be here
special, different than others. Why it is not a good solution for
Broadcom SoCs but it is for others?
qcom,spi-qup.yaml
- qcom,spi-qup-v1.1.1 # for 8660, 8960 and 8064
- qcom,spi-qup-v2.1.1 # for 8974 and later
- qcom,spi-qup-v2.2.1 # for 8974 v2 and later
qcom,spi-qup.yaml
const: qcom,geni-spi
IP block version numbers are allowed when there is clear mapping between
version and SoCs using it. This is the case for Qualcomm because there
is such clear mapping documented and available for Qualcomm engineers
and also some of us (although not public).
I guess when individual who only has one particular board/chip and is
not aware of the IP family, it is understandable to use the chip
specific compatible string.
Family of devices is not a versioned IP block.
But when company works on it, we have the
visibility and access to all the chips and ip blocks in the family and
IMHO it is very reasonable to use the IP family name for the same IP as
these examples shows.
No, because family of devices is not a versioned IP block. I wrote
before that families and wildcards are not allowed.
Are you saying these are not good example to
follow?
It's nothing related to your case.
What are the issues with generic or family compatibles?
Could
you please elaborate?
They stop matching and some point and cause ABI breaks. We had several
cases where engineer believed "I have here family of devices" and then
later it turned out that the family is different.
I'll drop the "minItems: 1" from the reg property then.
Do you mean it does not match the description?
reg:
- maxItems: 1
+ items:
+ - description: main registers
+ - description: miscellaneous control registers
+ minItems: 1
+
+ reg-names:
+ items:
+ - const: hsspi
+ - const: spim-ctrl
This does not match reg
No. reg can be 1 item but you state reg-names cannot. These are always
the same. If one is 1 item, second is as well.
Then it won't be correct, because it would mean two items are required
always.
I think I misunderstand what you said. You basically want the reg-name minItem/maxItem constraints for brcm,bcm6328-hsspi compatible as well so it is consistent for all the compatibles? I was confused and thought it is not needed as reg-name is not required for brcm,bcm6328-hsspi compatible.
I agree it is better to have the uniform DTS but the situation here isThe previous patch was just word to word conversion of the text file. I
clocks:
items:
- - description: spi master reference clock
- - description: spi master pll clock
+ - description: SPI master reference clock
+ - description: SPI master pll clock
Really? You just added it in previous patch, didn't you?
will update that patch to include this change.
Will do.
clock-names:
items:
@@ -29,12 +58,43 @@ properties:
interrupts:
maxItems: 1
+ brcm,use-cs-workaround:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ Enable dummy chip select workaround for SPI transfers that can not be
+ supported by the default controller's prepend mode, i.e. delay or cs
+ change needed between SPI transfers.
You need to describe what is the workaround.
Will remove quote and put it in patch 1.
+
required:
- compatible
- reg
- clocks
- clock-names
- - interrupts
+
+allOf:
+ - $ref: "spi-controller.yaml#"
No quotes. How this is related to this patch?
Don't quite understand your comment. What do I need to drop and what is
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - brcm,bcm6328-hsspi
+ then:
+ properties:
+ reg:
+ minItems: 1
Drop.
reg-names now do not match.
not matched?
You need to add constraints for reg-names, same way as for reg.
Disallowing the reg-names also could work, but there won't be benefit in
it. Better to have uniform DTS.
that the brcm,bcm6328-hsspi does not require reg name since there is
only one register needed and it was already used in many chip dts for
long time. If I enforce it to have the corresponding reg name, that
No one told you to enforce to have a reg-names.
could potentially break the compatibility of those old device if the
driver change to use reg name, right?
How compatibility is broken by some optional, unrelated property?
Best regards,
Krzysztof
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature