From: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Mark Brown <broonie@kernel.org>,
Doug Anderson <dianders@chromium.org>,
Chanwoo Choi <cw00.choi@samsung.com>,
Olof Johansson <olof@lixom.net>, Chris Zhong <zyw@rock-chips.com>,
Krzysztof Kozlowski <k.kozlowski@samsung.com>,
Abhilash Kesavan <kesavan.abhilash@gmail.com>,
"linux-samsung-soc@vger.kernel.org"
<linux-samsung-soc@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH 1/5] regulator: of: Add regulator-initial-mode parse support
Date: Thu, 09 Oct 2014 17:19:47 +0200 [thread overview]
Message-ID: <5436A793.6090204@collabora.co.uk> (raw)
In-Reply-To: <20141009102717.GG3869@leverpostej>
Hello Mark,
On 10/09/2014 12:27 PM, Mark Rutland wrote:
>>
>> Well, is not fairly obvious to me. One can also say the opposite, why the
>> kernel is documenting a DT binding that is not (currently) implemented?
>
> Checkpatch will complain regarding undocumented bindings, so from a
> pragmatic point of view the binding must come first.
>
> Personally, when I read a patch series I do an initial pass in-order,
> and having the binding first makes things clearer. I might have some
> questions regarding the binding that the driver answers later, and it makes it
> easier to spot undocumented properties or conventions used by the
> driver. Doing so the other way around usually leaves me with more
> questions at the end.
>
Thanks a lot for the explanation, it certainly makes sense then to have
the DT binding before. I'll propose a patch to add that information to
Documentation/devicetree/bindings/submitting-patches.txt so people
(like me) who didn't find it obvious can know what the convention is.
>> That's why what makes the most sense for me is what the old convention did,
>> add the DT binding docs in the same patch that implements the binding.
>
> Having a separate patch for the binding is very helpful for those of us
> doing review. For one thing it helps us to find the binding document,
> which can be important when a driver is thousands of lines long. For
> another it means that we can be clear that our Acked-by, Reviewed-by,
> etc apply to the binding and not necessarily the rest of the code.
>
Agreed.
> For small patches, this is obviously less of a concern.
>
> Thanks,
> Mark.
>
Best regards,
Javier
next prev parent reply other threads:[~2014-10-09 15:19 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-08 13:44 [PATCH 0/5] regulator: Add support for initial operating modes Javier Martinez Canillas
2014-10-08 13:44 ` Javier Martinez Canillas
2014-10-08 13:44 ` [PATCH 1/5] regulator: of: Add regulator-initial-mode parse support Javier Martinez Canillas
2014-10-08 14:25 ` Mark Brown
2014-10-08 14:38 ` Javier Martinez Canillas
2014-10-08 15:12 ` Mark Brown
2014-10-08 16:29 ` Javier Martinez Canillas
2014-10-09 10:27 ` Mark Rutland
2014-10-09 15:19 ` Javier Martinez Canillas [this message]
2014-10-10 13:17 ` Mark Rutland
2014-10-08 13:44 ` [PATCH 2/5] regulator: dt-bindings: Add DT include for constants Javier Martinez Canillas
2014-10-08 13:44 ` [PATCH 3/5] regulator: dt-bindings: Add regulator-initial-mode support Javier Martinez Canillas
2014-10-08 14:34 ` Mark Brown
2014-10-08 16:00 ` Javier Martinez Canillas
2014-10-09 8:45 ` Krzysztof Kozlowski
2014-10-09 15:04 ` Javier Martinez Canillas
2014-10-09 19:01 ` Mark Brown
[not found] ` <20141009190107.GY4609-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2014-10-09 21:56 ` Javier Martinez Canillas
2014-10-09 21:56 ` Javier Martinez Canillas
2014-10-08 13:44 ` [PATCH 4/5] regulator: max77802: Add regulator operating mode set support Javier Martinez Canillas
2014-10-08 14:36 ` Mark Brown
2014-10-08 16:01 ` Javier Martinez Canillas
2014-10-08 13:44 ` [PATCH 5/5] ARM: dts: Add initial regulator mode on exynos Peach boards Javier Martinez Canillas
2014-10-08 14:56 ` Mark Brown
2014-10-08 16:25 ` Javier Martinez Canillas
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=5436A793.6090204@collabora.co.uk \
--to=javier.martinez@collabora.co.uk \
--cc=broonie@kernel.org \
--cc=cw00.choi@samsung.com \
--cc=devicetree@vger.kernel.org \
--cc=dianders@chromium.org \
--cc=k.kozlowski@samsung.com \
--cc=kesavan.abhilash@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=olof@lixom.net \
--cc=zyw@rock-chips.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.