[PATCH v2 1/3] dt-bindings: hwlock: Adding brcmstb-hwspinlock support

Kamal Dasu posted 3 patches 4 months, 1 week ago
There is a newer version of this series
[PATCH v2 1/3] dt-bindings: hwlock: Adding brcmstb-hwspinlock support
Posted by Kamal Dasu 4 months, 1 week ago
Adding brcmstb-hwspinlock bindings.

Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
---
 .../hwlock/brcm,brcmstb-hwspinlock.yaml       | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml

diff --git a/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
new file mode 100644
index 000000000000..f45399b4fe0b
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
@@ -0,0 +1,36 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwlock/brcm,brcmstb-hwspinlock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom settop Hardware Spinlock
+
+maintainers:
+  - Kamal Dasu <kamal.dasu@broadcom.com>
+
+properties:
+  compatible:
+    const: brcm,brcmstb-hwspinlock
+
+  "#hwlock-cells":
+    const: 1
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - "#hwlock-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    hwlock@8404038 {
+        compatible = "brcm,brcmstb-hwspinlock";
+        reg = <0x8404038 0x40>;
+        #hwlock-cells = <1>;
+    };
+
-- 
2.34.1
Re: [PATCH v2 1/3] dt-bindings: hwlock: Adding brcmstb-hwspinlock support
Posted by Rob Herring 4 months ago
On Wed, Oct 01, 2025 at 02:16:39PM -0400, Kamal Dasu wrote:
> Adding brcmstb-hwspinlock bindings.

That's obvious from the diff. Tell us something about the h/w and 
convince me we don't need per SoC compatible which is standard practice.

> 
> Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
> ---
>  .../hwlock/brcm,brcmstb-hwspinlock.yaml       | 36 +++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
> new file mode 100644
> index 000000000000..f45399b4fe0b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
> @@ -0,0 +1,36 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwlock/brcm,brcmstb-hwspinlock.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom settop Hardware Spinlock
> +
> +maintainers:
> +  - Kamal Dasu <kamal.dasu@broadcom.com>
> +
> +properties:
> +  compatible:
> +    const: brcm,brcmstb-hwspinlock

hwspinlock is the name of the h/w block? Use the name of the h/w, not 
linux subsystem names.

> +
> +  "#hwlock-cells":
> +    const: 1
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#hwlock-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    hwlock@8404038 {
> +        compatible = "brcm,brcmstb-hwspinlock";
> +        reg = <0x8404038 0x40>;

h/w blocks rarely start at an offset like that. Is this part of some 
other h/w block? If so, then just add '#hwlock-cells' to *that* node.

> +        #hwlock-cells = <1>;
> +    };
> +
> -- 
> 2.34.1
>
Re: [PATCH v2 1/3] dt-bindings: hwlock: Adding brcmstb-hwspinlock support
Posted by Florian Fainelli 4 months ago

On 10/8/2025 8:56 AM, Rob Herring wrote:
> On Wed, Oct 01, 2025 at 02:16:39PM -0400, Kamal Dasu wrote:
>> Adding brcmstb-hwspinlock bindings.
> 
> That's obvious from the diff. Tell us something about the h/w and
> convince me we don't need per SoC compatible which is standard practice.
> 
>>
>> Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
>> ---
>>   .../hwlock/brcm,brcmstb-hwspinlock.yaml       | 36 +++++++++++++++++++
>>   1 file changed, 36 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
>> new file mode 100644
>> index 000000000000..f45399b4fe0b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
>> @@ -0,0 +1,36 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/hwlock/brcm,brcmstb-hwspinlock.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Broadcom settop Hardware Spinlock
>> +
>> +maintainers:
>> +  - Kamal Dasu <kamal.dasu@broadcom.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: brcm,brcmstb-hwspinlock
> 
> hwspinlock is the name of the h/w block? Use the name of the h/w, not
> linux subsystem names.
> 
>> +
>> +  "#hwlock-cells":
>> +    const: 1
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - "#hwlock-cells"
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    hwlock@8404038 {
>> +        compatible = "brcm,brcmstb-hwspinlock";
>> +        reg = <0x8404038 0x40>;
> 
> h/w blocks rarely start at an offset like that. Is this part of some
> other h/w block? If so, then just add '#hwlock-cells' to *that* node.

We've answered that in the previous review:

The block is part of a "sundry" IP which has lots of controls that did 
not belong anywhere else, for better or for worse (pin/mux controls, SoC 
identification, drive strength, reset controls, and other misc bits).
-- 
Florian
Re: [PATCH v2 1/3] dt-bindings: hwlock: Adding brcmstb-hwspinlock support
Posted by Krzysztof Kozlowski 4 months ago
On 09/10/2025 01:39, Florian Fainelli wrote:
> 
> 
> On 10/8/2025 8:56 AM, Rob Herring wrote:
>> On Wed, Oct 01, 2025 at 02:16:39PM -0400, Kamal Dasu wrote:
>>> Adding brcmstb-hwspinlock bindings.
>>
>> That's obvious from the diff. Tell us something about the h/w and
>> convince me we don't need per SoC compatible which is standard practice.
>>
>>>
>>> Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
>>> ---
>>>   .../hwlock/brcm,brcmstb-hwspinlock.yaml       | 36 +++++++++++++++++++
>>>   1 file changed, 36 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
>>> new file mode 100644
>>> index 000000000000..f45399b4fe0b
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
>>> @@ -0,0 +1,36 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/hwlock/brcm,brcmstb-hwspinlock.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Broadcom settop Hardware Spinlock
>>> +
>>> +maintainers:
>>> +  - Kamal Dasu <kamal.dasu@broadcom.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: brcm,brcmstb-hwspinlock
>>
>> hwspinlock is the name of the h/w block? Use the name of the h/w, not
>> linux subsystem names.
>>
>>> +
>>> +  "#hwlock-cells":
>>> +    const: 1
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - "#hwlock-cells"
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    hwlock@8404038 {
>>> +        compatible = "brcm,brcmstb-hwspinlock";
>>> +        reg = <0x8404038 0x40>;
>>
>> h/w blocks rarely start at an offset like that. Is this part of some
>> other h/w block? If so, then just add '#hwlock-cells' to *that* node.
> 
> We've answered that in the previous review:
> 
> The block is part of a "sundry" IP which has lots of controls that did 
> not belong anywhere else, for better or for worse (pin/mux controls, SoC 
> identification, drive strength, reset controls, and other misc bits).


And every time above explanation will not reach description or commit
msg we will ask the same.



Best regards,
Krzysztof
Re: [PATCH v2 1/3] dt-bindings: hwlock: Adding brcmstb-hwspinlock support
Posted by Kamal Dasu 4 months ago
Ok I will send a v3 with the explanation in the commit message for
both the patches.

On Wed, Oct 8, 2025 at 8:40 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 09/10/2025 01:39, Florian Fainelli wrote:
> >
> >
> > On 10/8/2025 8:56 AM, Rob Herring wrote:
> >> On Wed, Oct 01, 2025 at 02:16:39PM -0400, Kamal Dasu wrote:
> >>> Adding brcmstb-hwspinlock bindings.
> >>
> >> That's obvious from the diff. Tell us something about the h/w and
> >> convince me we don't need per SoC compatible which is standard practice.
> >>
> >>>
> >>> Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
> >>> ---
> >>>   .../hwlock/brcm,brcmstb-hwspinlock.yaml       | 36 +++++++++++++++++++
> >>>   1 file changed, 36 insertions(+)
> >>>   create mode 100644 Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
> >>> new file mode 100644
> >>> index 000000000000..f45399b4fe0b
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
> >>> @@ -0,0 +1,36 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/hwlock/brcm,brcmstb-hwspinlock.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Broadcom settop Hardware Spinlock
> >>> +
> >>> +maintainers:
> >>> +  - Kamal Dasu <kamal.dasu@broadcom.com>
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    const: brcm,brcmstb-hwspinlock
> >>
> >> hwspinlock is the name of the h/w block? Use the name of the h/w, not
> >> linux subsystem names.
> >>
> >>> +
> >>> +  "#hwlock-cells":
> >>> +    const: 1
> >>> +
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +required:
> >>> +  - compatible
> >>> +  - reg
> >>> +  - "#hwlock-cells"
> >>> +
> >>> +additionalProperties: false
> >>> +
> >>> +examples:
> >>> +  - |
> >>> +    hwlock@8404038 {
> >>> +        compatible = "brcm,brcmstb-hwspinlock";
> >>> +        reg = <0x8404038 0x40>;
> >>
> >> h/w blocks rarely start at an offset like that. Is this part of some
> >> other h/w block? If so, then just add '#hwlock-cells' to *that* node.
> >
> > We've answered that in the previous review:
> >
> > The block is part of a "sundry" IP which has lots of controls that did
> > not belong anywhere else, for better or for worse (pin/mux controls, SoC
> > identification, drive strength, reset controls, and other misc bits).
>
>
> And every time above explanation will not reach description or commit
> msg we will ask the same.
>
>
>
> Best regards,
> Krzysztof