From: Krzysztof Kozlowski <krzk@kernel.org>
To: Holger Assmann <h.assmann@pengutronix.de>,
lgirdwood@gmail.com, broonie@kernel.org, robh@kernel.org,
krzk+dt@kernel.org, conor+dt@kernel.org, yibin.gong@nxp.com
Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Pengutronix Kernel Team <kernel@pengutronix.de>
Subject: Re: [PATCH 2/2] dt-bindings: regulator: pca9450: add restart handler priority
Date: Mon, 22 Apr 2024 19:57:49 +0200 [thread overview]
Message-ID: <10811cbe-359f-42fe-b318-490309f95942@kernel.org> (raw)
In-Reply-To: <f70d53d2-0494-461d-9cbb-086077324e03@pengutronix.de>
On 22/04/2024 10:24, Holger Assmann wrote:
> Hello Krzysztof,
>
> also thanks for the feedback on this one.
>
> Am 19.04.24 um 15:39 schrieb Krzysztof Kozlowski:
>>
>> Please use subject prefixes matching the subsystem. You can get them for
>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>> your patch is touching.
>
> Short note: I did that prior submitting, but I did it directly for the
> yaml-file and not for the directory - Those do not look the same
> regarding their prefix scheme.
No, if you run it per file or per directory, prefix is the same. Starts with regulator, doesn't it?
>
> I will change it for my v2 and use a subject like for the directory.
regulator: dt-bindings: nxp,pca9450: add foo bar
>
>
>>
>>> ---
>>> .../devicetree/bindings/regulator/nxp,pca9450-regulator.yaml | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>>> index 3d469b8e97748..7cc2d6636cf52 100644
>>> --- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>>> +++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>>> @@ -35,6 +35,9 @@ properties:
>>> interrupts:
>>> maxItems: 1
>>>
>>> + priority:
>>> + $ref: /schemas/power/reset/restart-handler.yaml#
>>
>> You defined object, which is not explained in commit msg. This code does
>> not look correct or it does not implement what you said.
>>
>> Please look at existing code - do you see anything like this? No, there
>> is no such code and this should raise question.
>
> I am a bit lost on that one to be honest.
>
> The only other instances where a "priority" for restart handling is
> described are "gpio-poweroff.yaml" and "syscon-reboot.yaml". These files
> are dedicated documentation for the reset bindings, so I tried to
> transfer the respective entry over for my commit.
Where do you see such syntax?
>
> Do you suggest I should replace
>
> + priority:
> + $ref: /schemas/power/reset/restart-handler.yaml#
I don't understand what you want to express with such syntax. That's why I suggested you to look for existing examples. There is no such code as above, so you are the first one to add it. And then the obvious question: why are you doing things quite different than others? That's pretty often a hint that something is odd.
>
> with
>
> +allOf:
> + - $ref: /schemas/power/reset/restart-handler.yaml#
>
> in order to properly include the context for the restart handling?
> Running dt_binding_check does not indicate an issue with any of those two.
Again, what do you want to achieve?
>
>
>>
>> You probably want to annotate that device is a restart handler?
>
> You mean by adding to the "description" part of the file?
No. What do you want to achieve?
Best regards,
Krzysztof
prev parent reply other threads:[~2024-04-22 17:57 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-19 8:31 [PATCH 0/2] regulator: pca9450: enable restart handler for I2C Holger Assmann
2024-04-19 8:31 ` [PATCH 1/2] regulator: pca9450: enable restart handler for I2C operation Holger Assmann
2024-04-19 13:41 ` Krzysztof Kozlowski
2024-04-22 8:23 ` Holger Assmann
2024-04-19 8:31 ` [PATCH 2/2] dt-bindings: regulator: pca9450: add restart handler priority Holger Assmann
2024-04-19 13:39 ` Krzysztof Kozlowski
2024-04-22 8:24 ` Holger Assmann
2024-04-22 17:57 ` Krzysztof Kozlowski [this message]
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=10811cbe-359f-42fe-b318-490309f95942@kernel.org \
--to=krzk@kernel.org \
--cc=broonie@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=h.assmann@pengutronix.de \
--cc=kernel@pengutronix.de \
--cc=krzk+dt@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=robh@kernel.org \
--cc=yibin.gong@nxp.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).