[PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

It's an UART controller that first appeared on BCM63138 SoC and then was
reused on other bcmbca familiy chipsets.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Extend "compatible" and rename YAML file accordingly

Krzysztof: since I reworked "compatible" I didn't want to carry on your
Reviewed in case there is sth wrong with the updated schema.

 .../bindings/serial/brcm,bcmbca-hs-uart.yaml  | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/brcm,bcmbca-hs-uart.yaml

diff --git a/Documentation/devicetree/bindings/serial/brcm,bcmbca-hs-uart.yaml b/Documentation/devicetree/bindings/serial/brcm,bcmbca-hs-uart.yaml
new file mode 100644
index 000000000000..64ef9eee7be2
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/brcm,bcmbca-hs-uart.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/serial/brcm,bcmbca-hs-uart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom Broadband SoC High Speed UART
+
+description:
+  High speed serial port controller that was designed to handle Bluetooth
+  devices communication. It supports sending custom frames that need to be
+  processed by a host system.
+
+maintainers:
+  - Rafał Miłecki <rafal@milecki.pl>
+
+allOf:
+  - $ref: serial.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - brcm,bcm4908-hs-uart
+          - brcm,bcm4912-hs-uart
+          - brcm,bcm6756-hs-uart
+          - brcm,bcm6813-hs-uart
+          - brcm,bcm6846-hs-uart
+          - brcm,bcm6855-hs-uart
+          - brcm,bcm6856-hs-uart
+          - brcm,bcm6858-hs-uart
+          - brcm,bcm6878-hs-uart
+          - brcm,bcm47622-hs-uart
+          - brcm,bcm63138-hs-uart
+          - brcm,bcm63146-hs-uart
+          - brcm,bcm63158-hs-uart
+          - brcm,bcm63178-hs-uart
+      - const: brcm,bcmbca-hs-uart
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    serial@fffec400 {
+        compatible = "brcm,bcm63138-hs-uart", "brcm,bcmbca-hs-uart";
+        reg = <0xfffec400 0x1e0>;
+        interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
+    };
-- 
2.35.3

[PATCH V2 2/2] ARM: dts: broadcom: Add BCM63138's high speed UART

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

It's designed for hardwiring Bluetooth devices to it.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Update "compatible"

 arch/arm/boot/dts/broadcom/bcm63138.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/broadcom/bcm63138.dtsi b/arch/arm/boot/dts/broadcom/bcm63138.dtsi
index 93281c47c9ba..4ef02283612b 100644
--- a/arch/arm/boot/dts/broadcom/bcm63138.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm63138.dtsi
@@ -232,6 +232,12 @@ nand_controller: nand-controller@2000 {
 			interrupt-names = "nand";
 		};
 
+		serial@4400 {
+			compatible = "brcm,bcm63138-hs-uart", "brcm,bcmbca-hs-uart";
+			reg = <0x4400 0x1e0>;
+			interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
 		bootlut: bootlut@8000 {
 			compatible = "brcm,bcm63138-bootlut";
 			reg = <0x8000 0x50>;
-- 
2.35.3

Re: [PATCH V2 2/2] ARM: dts: broadcom: Add BCM63138's high speed UART

[Florian Fainelli]

From: Florian Fainelli <f.fainelli@gmail.com>

On Wed, 22 Nov 2023 15:42:08 +0100, Rafał Miłecki <zajec5@gmail.com> wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> It's designed for hardwiring Bluetooth devices to it.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---

Applied to https://github.com/Broadcom/stblinux/commits/devicetree/next, thanks!
--
Florian

Re: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[Krzysztof Kozlowski]

On 22/11/2023 15:42, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> It's an UART controller that first appeared on BCM63138 SoC and then was
> reused on other bcmbca familiy chipsets.

If there is going to be a new version, typo: family

> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Extend "compatible" and rename YAML file accordingly
> 
> Krzysztof: since I reworked "compatible" I didn't want to carry on your
> Reviewed in case there is sth wrong with the updated schema.

Thanks for letting me know.

> +
> +maintainers:
> +  - Rafał Miłecki <rafal@milecki.pl>
> +
> +allOf:
> +  - $ref: serial.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - brcm,bcm4908-hs-uart
> +          - brcm,bcm4912-hs-uart
> +          - brcm,bcm6756-hs-uart
> +          - brcm,bcm6813-hs-uart
> +          - brcm,bcm6846-hs-uart
> +          - brcm,bcm6855-hs-uart
> +          - brcm,bcm6856-hs-uart
> +          - brcm,bcm6858-hs-uart
> +          - brcm,bcm6878-hs-uart
> +          - brcm,bcm47622-hs-uart
> +          - brcm,bcm63138-hs-uart
> +          - brcm,bcm63146-hs-uart
> +          - brcm,bcm63158-hs-uart
> +          - brcm,bcm63178-hs-uart
> +      - const: brcm,bcmbca-hs-uart

git grep did not find driver for this compatible. Is it in separate
patchset?

Best regards,
Krzysztof

Re: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[Rafał Miłecki]

On 22.11.2023 15:46, Krzysztof Kozlowski wrote:
> On 22/11/2023 15:42, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> It's an UART controller that first appeared on BCM63138 SoC and then was
>> reused on other bcmbca familiy chipsets.
> 
> If there is going to be a new version, typo: family
> 
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>> V2: Extend "compatible" and rename YAML file accordingly
>>
>> Krzysztof: since I reworked "compatible" I didn't want to carry on your
>> Reviewed in case there is sth wrong with the updated schema.
> 
> Thanks for letting me know.
> 
>> +
>> +maintainers:
>> +  - Rafał Miłecki <rafal@milecki.pl>
>> +
>> +allOf:
>> +  - $ref: serial.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    items:
>> +      - enum:
>> +          - brcm,bcm4908-hs-uart
>> +          - brcm,bcm4912-hs-uart
>> +          - brcm,bcm6756-hs-uart
>> +          - brcm,bcm6813-hs-uart
>> +          - brcm,bcm6846-hs-uart
>> +          - brcm,bcm6855-hs-uart
>> +          - brcm,bcm6856-hs-uart
>> +          - brcm,bcm6858-hs-uart
>> +          - brcm,bcm6878-hs-uart
>> +          - brcm,bcm47622-hs-uart
>> +          - brcm,bcm63138-hs-uart
>> +          - brcm,bcm63146-hs-uart
>> +          - brcm,bcm63158-hs-uart
>> +          - brcm,bcm63178-hs-uart
>> +      - const: brcm,bcmbca-hs-uart
> 
> git grep did not find driver for this compatible. Is it in separate
> patchset?

No. My project based on BCMBCA has been canceled and I don't work on it
full time anymore. I just wanted to fill empty bits I can afford
handling in my free time and complete hardware description in DTS.

I may still work on some BCMBCA drivers from time to time but as a side
project.

Re: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[Krzysztof Kozlowski]

On 22/11/2023 15:52, Rafał Miłecki wrote:
>>> +maintainers:
>>> +  - Rafał Miłecki <rafal@milecki.pl>
>>> +
>>> +allOf:
>>> +  - $ref: serial.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - brcm,bcm4908-hs-uart
>>> +          - brcm,bcm4912-hs-uart
>>> +          - brcm,bcm6756-hs-uart
>>> +          - brcm,bcm6813-hs-uart
>>> +          - brcm,bcm6846-hs-uart
>>> +          - brcm,bcm6855-hs-uart
>>> +          - brcm,bcm6856-hs-uart
>>> +          - brcm,bcm6858-hs-uart
>>> +          - brcm,bcm6878-hs-uart
>>> +          - brcm,bcm47622-hs-uart
>>> +          - brcm,bcm63138-hs-uart
>>> +          - brcm,bcm63146-hs-uart
>>> +          - brcm,bcm63158-hs-uart
>>> +          - brcm,bcm63178-hs-uart
>>> +      - const: brcm,bcmbca-hs-uart
>>
>> git grep did not find driver for this compatible. Is it in separate
>> patchset?
> 
> No. My project based on BCMBCA has been canceled and I don't work on it
> full time anymore. I just wanted to fill empty bits I can afford
> handling in my free time and complete hardware description in DTS.
> 
> I may still work on some BCMBCA drivers from time to time but as a side
> project.

This means we cannot use driver to verify whether the fallback is
actually suitable. Considering that existing UART bindings do not
fallback (brcm,bcm6345-uart, brcm,bcm7271-uart), I don't understand what
is the benefit here.

Best regards,
Krzysztof

Re: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[Rafał Miłecki]

On 22.11.2023 16:00, Krzysztof Kozlowski wrote:
> On 22/11/2023 15:52, Rafał Miłecki wrote:
>>>> +maintainers:
>>>> +  - Rafał Miłecki <rafal@milecki.pl>
>>>> +
>>>> +allOf:
>>>> +  - $ref: serial.yaml#
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    items:
>>>> +      - enum:
>>>> +          - brcm,bcm4908-hs-uart
>>>> +          - brcm,bcm4912-hs-uart
>>>> +          - brcm,bcm6756-hs-uart
>>>> +          - brcm,bcm6813-hs-uart
>>>> +          - brcm,bcm6846-hs-uart
>>>> +          - brcm,bcm6855-hs-uart
>>>> +          - brcm,bcm6856-hs-uart
>>>> +          - brcm,bcm6858-hs-uart
>>>> +          - brcm,bcm6878-hs-uart
>>>> +          - brcm,bcm47622-hs-uart
>>>> +          - brcm,bcm63138-hs-uart
>>>> +          - brcm,bcm63146-hs-uart
>>>> +          - brcm,bcm63158-hs-uart
>>>> +          - brcm,bcm63178-hs-uart
>>>> +      - const: brcm,bcmbca-hs-uart
>>>
>>> git grep did not find driver for this compatible. Is it in separate
>>> patchset?
>>
>> No. My project based on BCMBCA has been canceled and I don't work on it
>> full time anymore. I just wanted to fill empty bits I can afford
>> handling in my free time and complete hardware description in DTS.
>>
>> I may still work on some BCMBCA drivers from time to time but as a side
>> project.
> 
> This means we cannot use driver to verify whether the fallback is
> actually suitable. Considering that existing UART bindings do not
> fallback (brcm,bcm6345-uart, brcm,bcm7271-uart), I don't understand what
> is the benefit here.

I believed the rule for maintaining bindings and DTS files was to
describe hardware no matter what/if system needs it.

For example a year ago I added binding for BCMBCA SoC timer without
actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's
BCMBCA timers").

I'm not sure if we're going to agree on this, but personally I like
describing hardware as much as I can. So it's well documented /
understood and people may eventually write drivers for it. Maybe it's
partially because I come from Broadcom's world that isn't well known
for upstream efforts in general.

As for verifying this binding against actual driver I can definitely
understand your concerns. Hoping it may help I uploaded Broadcom's HS
UART driver extracted from the RAXE500-V1.0.8.70_2.0.36_gpl SDK/GPL
package: http://files.zajec.net/hs_uart/

Please note it's not much of a clean code and its design would not be
accepted upstream but hopefully you can glance at it to verify this
binding's compatibility.

Let me know if there is anything else (other than rewriting Broadcom's
downstream driver) I could do to get this binding accepted.

Re: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[Krzysztof Kozlowski]

On 22/11/2023 16:32, Rafał Miłecki wrote:
> On 22.11.2023 16:00, Krzysztof Kozlowski wrote:
>> On 22/11/2023 15:52, Rafał Miłecki wrote:
>>>>> +maintainers:
>>>>> +  - Rafał Miłecki <rafal@milecki.pl>
>>>>> +
>>>>> +allOf:
>>>>> +  - $ref: serial.yaml#
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    items:
>>>>> +      - enum:
>>>>> +          - brcm,bcm4908-hs-uart
>>>>> +          - brcm,bcm4912-hs-uart
>>>>> +          - brcm,bcm6756-hs-uart
>>>>> +          - brcm,bcm6813-hs-uart
>>>>> +          - brcm,bcm6846-hs-uart
>>>>> +          - brcm,bcm6855-hs-uart
>>>>> +          - brcm,bcm6856-hs-uart
>>>>> +          - brcm,bcm6858-hs-uart
>>>>> +          - brcm,bcm6878-hs-uart
>>>>> +          - brcm,bcm47622-hs-uart
>>>>> +          - brcm,bcm63138-hs-uart
>>>>> +          - brcm,bcm63146-hs-uart
>>>>> +          - brcm,bcm63158-hs-uart
>>>>> +          - brcm,bcm63178-hs-uart
>>>>> +      - const: brcm,bcmbca-hs-uart
>>>>
>>>> git grep did not find driver for this compatible. Is it in separate
>>>> patchset?
>>>
>>> No. My project based on BCMBCA has been canceled and I don't work on it
>>> full time anymore. I just wanted to fill empty bits I can afford
>>> handling in my free time and complete hardware description in DTS.
>>>
>>> I may still work on some BCMBCA drivers from time to time but as a side
>>> project.
>>
>> This means we cannot use driver to verify whether the fallback is
>> actually suitable. Considering that existing UART bindings do not
>> fallback (brcm,bcm6345-uart, brcm,bcm7271-uart), I don't understand what
>> is the benefit here.
> 
> I believed the rule for maintaining bindings and DTS files was to
> describe hardware no matter what/if system needs it.
> 
> For example a year ago I added binding for BCMBCA SoC timer without
> actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's
> BCMBCA timers").
> 
> I'm not sure if we're going to agree on this, but personally I like
> describing hardware as much as I can. So it's well documented /
> understood and people may eventually write drivers for it. Maybe it's
> partially because I come from Broadcom's world that isn't well known
> for upstream efforts in general.

The problem is that "brcm,bcmbca-hs-uart" is not describing hardware. It
is saying that all these devices have similar (compatible) programming
model, so the OS can use just one compatible. This goes away from pure
hardware description into interpretation.

Rob already commented on such non-SoC compatibles multiple times. I do
not see any reason here to not use specific compatible as fallback.

> 
> As for verifying this binding against actual driver I can definitely
> understand your concerns. Hoping it may help I uploaded Broadcom's HS
> UART driver extracted from the RAXE500-V1.0.8.70_2.0.36_gpl SDK/GPL
> package: http://files.zajec.net/hs_uart/
> 
> Please note it's not much of a clean code and its design would not be
> accepted upstream but hopefully you can glance at it to verify this
> binding's compatibility.
> 
> Let me know if there is anything else (other than rewriting Broadcom's
> downstream driver) I could do to get this binding accepted.



Best regards,
Krzysztof

Re: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[Rafał Miłecki]

On 22.11.2023 16:37, Krzysztof Kozlowski wrote:
> On 22/11/2023 16:32, Rafał Miłecki wrote:
>> On 22.11.2023 16:00, Krzysztof Kozlowski wrote:
>>> On 22/11/2023 15:52, Rafał Miłecki wrote:
>>>>>> +maintainers:
>>>>>> +  - Rafał Miłecki <rafal@milecki.pl>
>>>>>> +
>>>>>> +allOf:
>>>>>> +  - $ref: serial.yaml#
>>>>>> +
>>>>>> +properties:
>>>>>> +  compatible:
>>>>>> +    items:
>>>>>> +      - enum:
>>>>>> +          - brcm,bcm4908-hs-uart
>>>>>> +          - brcm,bcm4912-hs-uart
>>>>>> +          - brcm,bcm6756-hs-uart
>>>>>> +          - brcm,bcm6813-hs-uart
>>>>>> +          - brcm,bcm6846-hs-uart
>>>>>> +          - brcm,bcm6855-hs-uart
>>>>>> +          - brcm,bcm6856-hs-uart
>>>>>> +          - brcm,bcm6858-hs-uart
>>>>>> +          - brcm,bcm6878-hs-uart
>>>>>> +          - brcm,bcm47622-hs-uart
>>>>>> +          - brcm,bcm63138-hs-uart
>>>>>> +          - brcm,bcm63146-hs-uart
>>>>>> +          - brcm,bcm63158-hs-uart
>>>>>> +          - brcm,bcm63178-hs-uart
>>>>>> +      - const: brcm,bcmbca-hs-uart
>>>>>
>>>>> git grep did not find driver for this compatible. Is it in separate
>>>>> patchset?
>>>>
>>>> No. My project based on BCMBCA has been canceled and I don't work on it
>>>> full time anymore. I just wanted to fill empty bits I can afford
>>>> handling in my free time and complete hardware description in DTS.
>>>>
>>>> I may still work on some BCMBCA drivers from time to time but as a side
>>>> project.
>>>
>>> This means we cannot use driver to verify whether the fallback is
>>> actually suitable. Considering that existing UART bindings do not
>>> fallback (brcm,bcm6345-uart, brcm,bcm7271-uart), I don't understand what
>>> is the benefit here.
>>
>> I believed the rule for maintaining bindings and DTS files was to
>> describe hardware no matter what/if system needs it.
>>
>> For example a year ago I added binding for BCMBCA SoC timer without
>> actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's
>> BCMBCA timers").
>>
>> I'm not sure if we're going to agree on this, but personally I like
>> describing hardware as much as I can. So it's well documented /
>> understood and people may eventually write drivers for it. Maybe it's
>> partially because I come from Broadcom's world that isn't well known
>> for upstream efforts in general.
> 
> The problem is that "brcm,bcmbca-hs-uart" is not describing hardware. It
> is saying that all these devices have similar (compatible) programming
> model, so the OS can use just one compatible. This goes away from pure
> hardware description into interpretation.
> 
> Rob already commented on such non-SoC compatibles multiple times. I do
> not see any reason here to not use specific compatible as fallback.

Do I get it right we should rather have some base specific compatible
like: "brcm,bcm63138-hs-uart" and then if anything use fallback to it
like: "brcm,bcm4908-hs-uart", "brcm,bcm63138-hs-uart"; ?

Re: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[Krzysztof Kozlowski]

On 22/11/2023 16:49, Rafał Miłecki wrote:
>>> For example a year ago I added binding for BCMBCA SoC timer without
>>> actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's
>>> BCMBCA timers").
>>>
>>> I'm not sure if we're going to agree on this, but personally I like
>>> describing hardware as much as I can. So it's well documented /
>>> understood and people may eventually write drivers for it. Maybe it's
>>> partially because I come from Broadcom's world that isn't well known
>>> for upstream efforts in general.
>>
>> The problem is that "brcm,bcmbca-hs-uart" is not describing hardware. It
>> is saying that all these devices have similar (compatible) programming
>> model, so the OS can use just one compatible. This goes away from pure
>> hardware description into interpretation.
>>
>> Rob already commented on such non-SoC compatibles multiple times. I do
>> not see any reason here to not use specific compatible as fallback.
> 
> Do I get it right we should rather have some base specific compatible
> like: "brcm,bcm63138-hs-uart" and then if anything use fallback to it
> like: "brcm,bcm4908-hs-uart", "brcm,bcm63138-hs-uart"; ?

Yes, or the other way around, depends which is probably the oldest.

Best regards,
Krzysztof

Re: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[Rafał Miłecki]

On 22.11.2023 16:50, Krzysztof Kozlowski wrote:
> On 22/11/2023 16:49, Rafał Miłecki wrote:
>>>> For example a year ago I added binding for BCMBCA SoC timer without
>>>> actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's
>>>> BCMBCA timers").
>>>>
>>>> I'm not sure if we're going to agree on this, but personally I like
>>>> describing hardware as much as I can. So it's well documented /
>>>> understood and people may eventually write drivers for it. Maybe it's
>>>> partially because I come from Broadcom's world that isn't well known
>>>> for upstream efforts in general.
>>>
>>> The problem is that "brcm,bcmbca-hs-uart" is not describing hardware. It
>>> is saying that all these devices have similar (compatible) programming
>>> model, so the OS can use just one compatible. This goes away from pure
>>> hardware description into interpretation.
>>>
>>> Rob already commented on such non-SoC compatibles multiple times. I do
>>> not see any reason here to not use specific compatible as fallback.
>>
>> Do I get it right we should rather have some base specific compatible
>> like: "brcm,bcm63138-hs-uart" and then if anything use fallback to it
>> like: "brcm,bcm4908-hs-uart", "brcm,bcm63138-hs-uart"; ?
> 
> Yes, or the other way around, depends which is probably the oldest.

Thank you for helping me on that!

Re: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[William Zhang]

[-- Attachment #1: Type: text/plain, Size: 1991 bytes --]

Hi,

On 11/22/2023 07:52 AM, Rafał Miłecki wrote:
> On 22.11.2023 16:50, Krzysztof Kozlowski wrote:
>> On 22/11/2023 16:49, Rafał Miłecki wrote:
>>>>> For example a year ago I added binding for BCMBCA SoC timer without
>>>>> actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's
>>>>> BCMBCA timers").
>>>>>
>>>>> I'm not sure if we're going to agree on this, but personally I like
>>>>> describing hardware as much as I can. So it's well documented /
>>>>> understood and people may eventually write drivers for it. Maybe it's
>>>>> partially because I come from Broadcom's world that isn't well known
>>>>> for upstream efforts in general.
>>>>
>>>> The problem is that "brcm,bcmbca-hs-uart" is not describing 
>>>> hardware. It
>>>> is saying that all these devices have similar (compatible) programming
>>>> model, so the OS can use just one compatible. This goes away from pure
>>>> hardware description into interpretation.
>>>>
It is the same hardware IP block used in bcmbca SoCs.  To me, it 
perfectly describe the hardware IP block and it does not need fallback 
because there is no fallback.  We did that for SPI controller although 
it has two revisions of that IP block so we have brcm,bcmbca-hsspi-v1.0 
and 1.1

>>>> Rob already commented on such non-SoC compatibles multiple times. I do
>>>> not see any reason here to not use specific compatible as fallback.
>>>
Sorry I missed Rob's comments.  If we have any new rule or notes about 
this, I would like to check it out.

>>> Do I get it right we should rather have some base specific compatible
>>> like: "brcm,bcm63138-hs-uart" and then if anything use fallback to it
>>> like: "brcm,bcm4908-hs-uart", "brcm,bcm63138-hs-uart"; ?
>>
>> Yes, or the other way around, depends which is probably the oldest.
If we absolutely can not use bcmbca-hs-uart, I would suggest to use 
bcm63xx-hs-uart to be more soc specific and in fact the oldest SoC have 
such IP is 63138.

> 
> Thank you for helping me on that!

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[Krzysztof Kozlowski]

On 22/11/2023 19:39, William Zhang wrote:
> Hi,
> 
> On 11/22/2023 07:52 AM, Rafał Miłecki wrote:
>> On 22.11.2023 16:50, Krzysztof Kozlowski wrote:
>>> On 22/11/2023 16:49, Rafał Miłecki wrote:
>>>>>> For example a year ago I added binding for BCMBCA SoC timer without
>>>>>> actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's
>>>>>> BCMBCA timers").
>>>>>>
>>>>>> I'm not sure if we're going to agree on this, but personally I like
>>>>>> describing hardware as much as I can. So it's well documented /
>>>>>> understood and people may eventually write drivers for it. Maybe it's
>>>>>> partially because I come from Broadcom's world that isn't well known
>>>>>> for upstream efforts in general.
>>>>>
>>>>> The problem is that "brcm,bcmbca-hs-uart" is not describing 
>>>>> hardware. It
>>>>> is saying that all these devices have similar (compatible) programming
>>>>> model, so the OS can use just one compatible. This goes away from pure
>>>>> hardware description into interpretation.
>>>>>
> It is the same hardware IP block used in bcmbca SoCs.  To me, it 
> perfectly describe the hardware IP block and it does not need fallback 
> because there is no fallback.  We did that for SPI controller although 
> it has two revisions of that IP block so we have brcm,bcmbca-hsspi-v1.0 
> and 1.1
> 
>>>>> Rob already commented on such non-SoC compatibles multiple times. I do
>>>>> not see any reason here to not use specific compatible as fallback.
>>>>
> Sorry I missed Rob's comments.  If we have any new rule or notes about 
> this, I would like to check it out.
> 
>>>> Do I get it right we should rather have some base specific compatible
>>>> like: "brcm,bcm63138-hs-uart" and then if anything use fallback to it
>>>> like: "brcm,bcm4908-hs-uart", "brcm,bcm63138-hs-uart"; ?
>>>
>>> Yes, or the other way around, depends which is probably the oldest.
> If we absolutely can not use bcmbca-hs-uart, I would suggest to use 

We can, but I am surprised that you want without any driver. What's the
point of generic compatible?

> bcm63xx-hs-uart to be more soc specific and in fact the oldest SoC have 

What is xx? Wildcard? I mean... ehhh...

Best regards,
Krzysztof

Re: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[Krzysztof Kozlowski]

On 22/11/2023 19:46, Krzysztof Kozlowski wrote:
> On 22/11/2023 19:39, William Zhang wrote:
>> Hi,
>>
>> On 11/22/2023 07:52 AM, Rafał Miłecki wrote:
>>> On 22.11.2023 16:50, Krzysztof Kozlowski wrote:
>>>> On 22/11/2023 16:49, Rafał Miłecki wrote:
>>>>>>> For example a year ago I added binding for BCMBCA SoC timer without
>>>>>>> actual driver, see e112f2de151b ("dt-bindings: timer: Add Broadcom's
>>>>>>> BCMBCA timers").
>>>>>>>
>>>>>>> I'm not sure if we're going to agree on this, but personally I like
>>>>>>> describing hardware as much as I can. So it's well documented /
>>>>>>> understood and people may eventually write drivers for it. Maybe it's
>>>>>>> partially because I come from Broadcom's world that isn't well known
>>>>>>> for upstream efforts in general.
>>>>>>
>>>>>> The problem is that "brcm,bcmbca-hs-uart" is not describing 
>>>>>> hardware. It
>>>>>> is saying that all these devices have similar (compatible) programming
>>>>>> model, so the OS can use just one compatible. This goes away from pure
>>>>>> hardware description into interpretation.
>>>>>>
>> It is the same hardware IP block used in bcmbca SoCs.  To me, it 
>> perfectly describe the hardware IP block and it does not need fallback 
>> because there is no fallback.  We did that for SPI controller although 
>> it has two revisions of that IP block so we have brcm,bcmbca-hsspi-v1.0 
>> and 1.1
>>
>>>>>> Rob already commented on such non-SoC compatibles multiple times. I do
>>>>>> not see any reason here to not use specific compatible as fallback.
>>>>>
>> Sorry I missed Rob's comments.  If we have any new rule or notes about 
>> this, I would like to check it out.
>>
>>>>> Do I get it right we should rather have some base specific compatible
>>>>> like: "brcm,bcm63138-hs-uart" and then if anything use fallback to it
>>>>> like: "brcm,bcm4908-hs-uart", "brcm,bcm63138-hs-uart"; ?
>>>>
>>>> Yes, or the other way around, depends which is probably the oldest.
>> If we absolutely can not use bcmbca-hs-uart, I would suggest to use 
> 
> We can, but I am surprised that you want without any driver. What's the
> point of generic compatible?
> 
>> bcm63xx-hs-uart to be more soc specific and in fact the oldest SoC have 
> 
> What is xx? Wildcard? I mean... ehhh...

OK, it's not worth my time. Neither Rafał's.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

I can go to Embedded OSS every year and give the same speech every year
and still people will on:
1. insist on generic fallback compatible,
2. wildcards
3. families

I will keep this email and use it to justify the same, third speech next
year. Which won't be listened to, so I will go in 2025 fourth time. :)

Best regards,
Krzysztof

Re: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 2456 bytes --]

Howdy,

On 11/22/2023 10:56 AM, Krzysztof Kozlowski wrote:
>>
>> What is xx? Wildcard? I mean... ehhh...
> 
> OK, it's not worth my time. Neither Rafał's.

Let me start of that I do get your point, but I also get William's.

If you are going to use this email thread as proof that people should 
not use wildcards or families in compatible strings, this is very much 
worth everyone's time so we can actually detail better why that is an 
issue. So far it has been described as an inadequate description of the 
hardware which is somewhat fair, but subjective as well.

The point of the fallback is precisely to express that the various HW IP 
versions have a similar programming model and that unless specified 
otherwise by a more descriptive compatible string, the driver can make 
that assumption. This is entirely within the spirit of the DT spec:

"""
The compatible property value consists of one or more strings that 
define the specific programming model for
the device. This list of strings should be used by a client program for 
device driver selection. The property
value consists of a concatenated list of null terminated strings, from 
most specific to most general. They allow
a device to express its compatibility with a family of similar devices, 
potentially allowing a single device driver
to match against several devices
"""

we simply differ from the recommendation, which is a recommendation, 
meaning deviation is allowed, and even that is entirely subjective:

"""
The recommended format is "manufacturer,model", where manufacturer is a 
string describing the name
of the manufacturer (such as a stock ticker symbol), and model specifies 
the model number.
""

that kind of fits in. I suppose that if we like to paint ourselves in a 
corner that allows us to simplify the logistics of maintaining our 
platforms' DTS and drivers without bending the specification we should 
be allowed to. How does that make your job as a DT maintainer any harder?

I do not disagree that identifying the oldest SoC that featured the 
specific block is best, but it may not always be that simple or that 
descriptive either.

There are a lot more properties and compatibles that are IMHO bending 
the Device Tree to describe how they *intend* to get the HW configured 
by the client program, and fall backs are really not amongst them IMHO.

Anyway.

> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 

Thanks.
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

RE: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA family High Speed UART

[William Zhang]

[-- Attachment #1: Type: text/plain, Size: 3687 bytes --]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Wednesday, November 22, 2023 10:46 AM
> To: William Zhang <william.zhang@broadcom.com>; Rafał Miłecki
> <zajec5@gmail.com>; Florian Fainelli <florian.fainelli@broadcom.com>;
> Anand Gore <anand.gore@broadcom.com>; Kursad Oney
> <kursad.oney@broadcom.com>; Rob Herring <robh+dt@kernel.org>;
> Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Jiri Slaby
> <jirislaby@kernel.org>; Andre Przywara <andre.przywara@arm.com>;
> Alexandre TORGUE <alexandre.torgue@st.com>; Neil Armstrong
> <neil.armstrong@linaro.org>; linux-serial@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; bcm-
> kernel-feedback-list@broadcom.com; Rafał Miłecki <rafal@milecki.pl>
> Subject: Re: [PATCH V2 1/2] dt-bindings: serial: add Broadcom's BCMBCA
> family High Speed UART
>
> On 22/11/2023 19:39, William Zhang wrote:
> > Hi,
> >
> > On 11/22/2023 07:52 AM, Rafał Miłecki wrote:
> >> On 22.11.2023 16:50, Krzysztof Kozlowski wrote:
> >>> On 22/11/2023 16:49, Rafał Miłecki wrote:
> >>>>>> For example a year ago I added binding for BCMBCA SoC timer
> without
> >>>>>> actual driver, see e112f2de151b ("dt-bindings: timer: Add
> Broadcom's
> >>>>>> BCMBCA timers").
> >>>>>>
> >>>>>> I'm not sure if we're going to agree on this, but personally I like
> >>>>>> describing hardware as much as I can. So it's well documented /
> >>>>>> understood and people may eventually write drivers for it. Maybe
> it's
> >>>>>> partially because I come from Broadcom's world that isn't well
> known
> >>>>>> for upstream efforts in general.
> >>>>>
> >>>>> The problem is that "brcm,bcmbca-hs-uart" is not describing
> >>>>> hardware. It
> >>>>> is saying that all these devices have similar (compatible)
> >>>>> programming
> >>>>> model, so the OS can use just one compatible. This goes away from
> pure
> >>>>> hardware description into interpretation.
> >>>>>
> > It is the same hardware IP block used in bcmbca SoCs.  To me, it
> > perfectly describe the hardware IP block and it does not need fallback
> > because there is no fallback.  We did that for SPI controller although
> > it has two revisions of that IP block so we have brcm,bcmbca-hsspi-v1.0
> > and 1.1
> >
> >>>>> Rob already commented on such non-SoC compatibles multiple times.
> I do
> >>>>> not see any reason here to not use specific compatible as fallback.
> >>>>
> > Sorry I missed Rob's comments.  If we have any new rule or notes about
> > this, I would like to check it out.
> >
> >>>> Do I get it right we should rather have some base specific compatible
> >>>> like: "brcm,bcm63138-hs-uart" and then if anything use fallback to it
> >>>> like: "brcm,bcm4908-hs-uart", "brcm,bcm63138-hs-uart"; ?
> >>>
> >>> Yes, or the other way around, depends which is probably the oldest.
> > If we absolutely can not use bcmbca-hs-uart, I would suggest to use
>
> We can, but I am surprised that you want without any driver. What's the
> point of generic compatible?
>
I agree we should have the driver along with the dts. But it looks it
depends on
the bandwidth of Rafal.

> > bcm63xx-hs-uart to be more soc specific and in fact the oldest SoC have
>
> What is xx? Wildcard? I mean... ehhh...
>
Bcm63 series of SoC (DSL broadband chip, part of the bcmbca family) and it
is
the oldest chip for such IP.   bcm63xx has been used in many ip block's
compatible
string for long time in the upstream kernel.

> Best regards,
> Krzysztof

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]