[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <248d0898-2eca-4ba8-9669-cd3d1cd8ad1b@kernel.org>
Date: Thu, 9 Oct 2025 09:40:06 +0900
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Florian Fainelli <florian.fainelli@...adcom.com>,
Rob Herring <robh@...nel.org>, Kamal Dasu <kamal.dasu@...adcom.com>
Cc: peng.fan@....nxp.com, andersson@...nel.org,
baolin.wang@...ux.alibaba.com, krzk+dt@...nel.org, conor+dt@...nel.org,
bcm-kernel-feedback-list@...adcom.com, linux-remoteproc@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] dt-bindings: hwlock: Adding brcmstb-hwspinlock
support
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@...adcom.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@...adcom.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@...4038 {
>>> + 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
Powered by blists - more mailing lists