From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: "Théo Lebrun" <theo.lebrun@bootlin.com>,
"Rob Herring" <robh@kernel.org>,
"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
"Conor Dooley" <conor+dt@kernel.org>,
"Michael Turquette" <mturquette@baylibre.com>,
"Stephen Boyd" <sboyd@kernel.org>,
"Philipp Zabel" <p.zabel@pengutronix.de>,
"Linus Walleij" <linus.walleij@linaro.org>
Cc: linux-mips@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
linux-gpio@vger.kernel.org,
Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>,
Gregory CLEMENT <gregory.clement@bootlin.com>,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
Tawfik Bayouk <tawfik.bayouk@mobileye.com>
Subject: Re: [PATCH 03/11] dt-bindings: reset: mobileye,eyeq5-reset: add EyeQ6L and EyeQ6H
Date: Thu, 11 Apr 2024 17:05:22 +0200 [thread overview]
Message-ID: <a6f91997-bbaf-48fd-9b8d-2bb671cd026b@linaro.org> (raw)
In-Reply-To: <D0HCMDMWTO61.1F860N5I5SKS3@bootlin.com>
On 11/04/2024 16:04, Théo Lebrun wrote:
> Hello,
>
> On Thu Apr 11, 2024 at 8:14 AM CEST, Krzysztof Kozlowski wrote:
>> On 10/04/2024 19:12, Théo Lebrun wrote:
>>> Add bindings for EyeQ6L and EyeQ6H reset controllers.
>>>
>>> Some controllers host a single domain, meaning a single cell is enough.
>>> We do not enforce reg-names for such nodes.
>>>
>>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>>> ---
>>> .../bindings/reset/mobileye,eyeq5-reset.yaml | 88 ++++++++++++++++++----
>>> MAINTAINERS | 1 +
>>> 2 files changed, 74 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
>>> index 062b4518347b..799bcf15bed9 100644
>>> --- a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
>>> +++ b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
>>> @@ -4,11 +4,13 @@
>>> $id: http://devicetree.org/schemas/reset/mobileye,eyeq5-reset.yaml#
>>> $schema: http://devicetree.org/meta-schemas/core.yaml#
>>>
>>> -title: Mobileye EyeQ5 reset controller
>>> +title: Mobileye EyeQ reset controller
>>>
>>> description:
>>> - The EyeQ5 reset driver handles three reset domains. Its registers live in a
>>> - shared region called OLB.
>>> + EyeQ reset controller handles one or more reset domains. They live in shared
>>> + regions called OLB. EyeQ5 and EyeQ6L host one OLB each, each with one reset
>>> + instance. EyeQ6H hosts 7 OLB regions; three of those (west, east,
>>> + accelerator) host reset controllers. West and east are duplicates.
>>>
>>> maintainers:
>>> - Grégory Clement <gregory.clement@bootlin.com>
>>> @@ -17,27 +19,83 @@ maintainers:
>>>
>>> properties:
>>> compatible:
>>> - const: mobileye,eyeq5-reset
>>> + enum:
>>> + - mobileye,eyeq5-reset
>>> + - mobileye,eyeq6l-reset
>>> + - mobileye,eyeq6h-we-reset
>>> + - mobileye,eyeq6h-acc-reset
>>>
>>> - reg:
>>> - maxItems: 3
>>> + reg: true
>>
>> Same mistakes. Please open existing bindings with multiple variants,
>> e.g. some Qualcomm, and take a look how it is done there.
>
> Thanks for the pointer to good example, that is useful! So if we take
> one random binding matching
> Documentation/devicetree/bindings/clock/qcom,*.yaml and that contains
> the "reg-names" string, we see:
>
> reg:
> items:
> - description: LPASS qdsp6ss register
> - description: LPASS top-cc register
>
> reg-names:
> items:
> - const: qdsp6ss
> - const: top_cc
>
> I don't understand one thing; this doesn't tell you:
>
> You can provide 2 MMIO blocks, which must be qdsp6ss and top_cc.
No, it tells you exactly this, with difference: s/can/must/
>
> But it tells you:
>
> Block zero must be qdsp6ss.
> Block one must be top_cc.
>
> If we do that I do not get the point of reg-names; we put more
> information in our devicetree that is in any case imposed.
Same old argument. Order is not flexible. Order is fixed.
Why do you need names? I don't need, it's purely your optional choice.
Maybe you find it more readable, up to you.
>
> This is why I went with a different approach looking like:
>
> reg:
> minItems: 2
> maxItems: 2
> reg-names:
> minItems: 2
> maxItems: 2
> items:
> enum: [ d0, d1 ]
No, order is fixed.
>
> I know this is not perfect, but at least you don't enforce an order for
> no reason. If "items: const..." approach should be taken, then I'll
> remove reg-names which bring no benefit.
You can remove it, you can keep it, whatever makes code more readable,
but order is fixed.
Best regards,
Krzysztof
next prev parent reply other threads:[~2024-04-11 15:05 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-10 17:12 [PATCH 00/11] Add Mobileye EyeQ system controller support (clk, reset, pinctrl) Théo Lebrun
2024-04-10 17:12 ` [PATCH 01/11] dt-bindings: soc: mobileye: add EyeQ5 OLB system controller Théo Lebrun
2024-04-10 18:52 ` Rob Herring
2024-04-11 9:19 ` Théo Lebrun
2024-04-11 3:29 ` Stephen Boyd
2024-04-10 17:12 ` [PATCH 02/11] dt-bindings: clock: mobileye,eyeq5-clk: add EyeQ6L and EyeQ6H Théo Lebrun
2024-04-11 6:14 ` Krzysztof Kozlowski
2024-04-11 13:49 ` Théo Lebrun
2024-04-11 15:02 ` Krzysztof Kozlowski
2024-04-10 17:12 ` [PATCH 03/11] dt-bindings: reset: mobileye,eyeq5-reset: " Théo Lebrun
2024-04-11 6:14 ` Krzysztof Kozlowski
2024-04-11 14:04 ` Théo Lebrun
2024-04-11 15:05 ` Krzysztof Kozlowski [this message]
2024-04-10 17:12 ` [PATCH 04/11] clk: divider: Introduce CLK_DIVIDER_EVEN_INTEGERS flag Théo Lebrun
2024-04-11 3:06 ` Stephen Boyd
2024-04-11 10:14 ` Théo Lebrun
2024-04-12 5:19 ` Stephen Boyd
2024-04-10 17:12 ` [PATCH 05/11] clk: eyeq: add driver Théo Lebrun
2024-04-11 3:22 ` Stephen Boyd
2024-04-11 10:46 ` Théo Lebrun
2024-04-12 5:46 ` Stephen Boyd
2024-04-17 10:18 ` Théo Lebrun
2024-04-10 17:12 ` [PATCH 06/11] reset: eyeq: add platform driver Théo Lebrun
2024-04-10 17:12 ` [PATCH 07/11] pinctrl: eyeq5: " Théo Lebrun
2024-04-10 17:12 ` [PATCH 08/11] MIPS: mobileye: eyeq5: add OLB syscon node Théo Lebrun
2024-04-11 6:15 ` Krzysztof Kozlowski
2024-04-11 14:34 ` Théo Lebrun
2024-04-11 15:07 ` Krzysztof Kozlowski
2024-04-17 7:53 ` Théo Lebrun
2024-04-10 17:12 ` [PATCH 09/11] MIPS: mobileye: eyeq5: use OLB clocks controller node Théo Lebrun
2024-04-10 17:12 ` [PATCH 10/11] MIPS: mobileye: eyeq5: add OLB reset " Théo Lebrun
2024-04-10 17:12 ` [PATCH 11/11] MIPS: mobileye: eyeq5: add pinctrl node & pinmux function nodes Théo Lebrun
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a6f91997-bbaf-48fd-9b8d-2bb671cd026b@linaro.org \
--to=krzysztof.kozlowski@linaro.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=gregory.clement@bootlin.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linus.walleij@linaro.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=p.zabel@pengutronix.de \
--cc=robh@kernel.org \
--cc=sboyd@kernel.org \
--cc=tawfik.bayouk@mobileye.com \
--cc=theo.lebrun@bootlin.com \
--cc=thomas.petazzoni@bootlin.com \
--cc=vladimir.kondratiev@mobileye.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).