Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 4 Jul 2018 11:34:01 +0100
From:   Lee Jones <lee.jones@linaro.org>
To:     LKML <linux-kernel@vger.kernel.org>,
        DRI Development <dri-devel@lists.freedesktop.org>,
        Jani Nikula <jani.nikula@intel.com>,
        Daniel Vetter <daniel.vetter@intel.com>,
        Daniel Thompson <daniel.thompson@linaro.org>,
        Jingoo Han <jingoohan1@gmail.com>
Subject: Re: [PATCH] backlight: remove obsolete comment for ->state
Message-ID: <20180704103401.GB496@dell>
References: <20180503141517.28207-1-daniel.vetter@ffwll.ch>
 <20180704093447.GU20176@dell>
 <20180704093816.GV20176@dell>
 <20180704100506.GT3891@phenom.ffwll.local>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20180704100506.GT3891@phenom.ffwll.local>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 04 Jul 2018, Daniel Vetter wrote:
> On Wed, Jul 04, 2018 at 10:38:16AM +0100, Lee Jones wrote:
> > On Wed, 04 Jul 2018, Lee Jones wrote:
> > 
> > > > Jani spotted this when reviewing my earlier patch to remove the driver
> > > > internal usage of this field in
> > > > 
> > > > commit 3cf91adaa594e8933af1727942ac560e5c7bc70e
> > > > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > Date:   Wed Apr 25 19:42:52 2018 +0200
> > > > 
> > > >     backlight: Nuke BL_CORE_DRIVER1
> > > 
> > > FYI, sending patches like this is not a good idea.
> > > 
> > > I'll clean it up for you this time, but in future please send patches
> > > properly and place any additional comments you may have below the
> > > '---' line.
> > 
> > Ah, I see what you've tried to do.  This hurt my eyes! :)
> > 
> > It's more conventional to reference commits like:
> > 
> >   Commit 3cf91adaa594 ("backlight: Nuke BL_CORE_DRIVER1")
> > 
> > Again, I'll make the amendment to avoid further confusion.
> 
> So the first mail doesn't even bother to explain what's
> objectionable

In the first instance it looked as though you'd copied and pasted `git
log`, which if you'd done so would have been obvious to you and would
have required no further explanation.

Also bear in mind that I took your standing within the kernel
community into consideration, so speaking to you like a n00b or going
into unnecessary detail could have been considered superfluous at best
and condescending at worst.

> the 2nd mail still says "This hurts my eyes!".

It certainly did, yes.

Usually taken to meaning that it was hard to read in these scenarios.

> Over whitespace in the commit message.

Nothing to do with white space.  It was the format by which you chose
to reference a previous commit.  Instead it looked like a patch
formatting error.  I have received patches pasted from `git log`
before, and this looked just the same.

Once I'd realised what was going on, I followed up to explain and
provided some feedback on what to do differently in future.

> This kind of stuff is why graphics people really don't enjoy contributing
> to the kernel at large. A friendly request to resend with the color choice
> adjusted would go a lot further.

I apologise if my brevity hurt your feelings.  I have 290 emails to
get though post-paternity leave before I can start to think about
getting some real/paid work done.

> > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > Cc: Lee Jones <lee.jones@linaro.org>
> > > > Cc: Daniel Thompson <daniel.thompson@linaro.org>
> > > > Cc: Jingoo Han <jingoohan1@gmail.com>
> > > > ---
> > > >  include/linux/backlight.h | 1 -
> > > >  1 file changed, 1 deletion(-)
> > > > 
> > > > diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> > > > index 7fbf0539e14a..0b5897446dca 100644
> > > > --- a/include/linux/backlight.h
> > > > +++ b/include/linux/backlight.h
> > > > @@ -79,7 +79,6 @@ struct backlight_properties {
> > > >  	/* Backlight type */
> > > >  	enum backlight_type type;
> > > >  	/* Flags used to signal drivers of state changes */
> > > > -	/* Upper 4 bits are reserved for driver internal use */
> > > >  	unsigned int state;
> > > >  
> > > >  #define BL_CORE_SUSPENDED	(1 << 0)	/* backlight is suspended */
> > > 
> > 
> 

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog