Re: [PATCH] backlight: remove obsolete comment for ->state

[Daniel Vetter <daniel@ffwll.ch>]

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 and
the 2nd mail still says "This hurts my eyes!".

Over whitespace in the commit message.

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.

Thanks, Daniel

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch