linux-kernel - Re: [PATCH 1/5] regulator: of: Add regulator-initial-mode parse support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141009102717.GG3869@leverpostej>
Date:	Thu, 9 Oct 2014 11:27:17 +0100
From:	Mark Rutland <mark.rutland@....com>
To:	Javier Martinez Canillas <javier.martinez@...labora.co.uk>
Cc:	Mark Brown <broonie@...nel.org>,
	Doug Anderson <dianders@...omium.org>,
	Chanwoo Choi <cw00.choi@...sung.com>,
	Olof Johansson <olof@...om.net>,
	Chris Zhong <zyw@...k-chips.com>,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	Abhilash Kesavan <kesavan.abhilash@...il.com>,
	"linux-samsung-soc@...r.kernel.org" 
	<linux-samsung-soc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH 1/5] regulator: of: Add regulator-initial-mode parse
 support

On Wed, Oct 08, 2014 at 05:29:26PM +0100, Javier Martinez Canillas wrote:
> On 10/08/2014 05:12 PM, Mark Brown wrote:
> > On Wed, Oct 08, 2014 at 04:38:53PM +0200, Javier Martinez Canillas wrote:
> >> On 10/08/2014 04:25 PM, Mark Brown wrote:
> > 
> >> > That doesn't mean that the definition of those modes is something we can
> >> > sensibly provide in generic code, especially in a completely
> >> > undocumented fashion (perhaps you've done that later in the patch series
> >> > but bisection also applies to reviewability).
> > 
> >> As a general question, now that the convention is for DT binding docs to go
> >> in a separate patch, should the DT documentation be added before or after
> >> that code using these bindings is added?
> > 
> > It fairly obviously needs to go before so that reviewers can tell if the
> > code is actually implementing the binding.
> > 
> 
> 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.

> 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.

For small patches, this is obviously less of a concern.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/