Re: [PATCH 1/2 V3] dt: add Atmel Captouch bindings

Rob Herring
2016-05-20T22:12:55Z

On Fri, May 20, 2016 at 11:16 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, May 20, 2016 at 07:56:51AM -0500, Rob Herring wrote:
>> On Wed, May 18, 2016 at 4:59 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Wed, May 18, 2016 at 11:44:04AM -0500, Rob Herring wrote:
>> >> On Mon, May 16, 2016 at 01:54:53PM -0700, Grant Grundler wrote:
>> >> > From: Daniel Hung-yu Wu <hywu@google.com>
>> >> >
>> >> > Add binding for Atmel Capacitive Touch Button device.
>> >> >
>> >> > Signed-off-by: Daniel Hung-yu Wu <hywu@google.com>
>> >> > Signed-off-by: Grant Grundler <grundler@chromium.org>
>> >> > ---
>> >> > .../devicetree/bindings/input/atmel,captouch.txt | 36 ++++++++++++++++++++++
>> >> > 1 file changed, 34 insertions(+)
>> >> > create mode 100644 Documentation/devicetree/bindings/input/atmel,captouch.txt
>> >>
>> >> Acked-by: Rob Herring <robh@kernel.org>
>> >
>> > Folded into the driver patch and applied.
>>
>> Folded why? Please don't do that. You should be committing what is
>> posted as is for the most part. We specifically ask that binding
>> changes are kept separate commits. It also messes up the ability to
>
> I know that you ask for binding docs to be posted separately (I guess
> so that devicetree list is not overrun with driver code mails),

And because we're really only reviewing the binding, so putting my ack
on the driver is not really correct.

> but
> logically driver patch and binding doc patch are a single change and
> should be committed together, so that when I am researching the history
> I can easily see what was introduced and when. You do not require header
> changes to be submitted separately form .c files, do you?

Yes, for include/dt-bindings we ask that they are part of the binding
doc, not the driver even though both use it. You can also certainly
have bindings without drivers though generally we require them. I
would not if they had a driver in BSD or u-boot for example.

You can already easily see when things are introduced because they
will be next to each other in the git history.

>> correlate git commits to patchworks or mail searches.
>
> The fact that it was applied can be found in mail archives.

Yes, with extra effort reading the history you can, but not with a
script. There's a patchwork script to add commit hashes to patchwork
which works all based on the subject.

Regardless of one commit or two, you simply shouldn't be changing what
you commit. Either commit what was posted or require the author to
combine things and repost. That's our job as patch monkeys.

Rob