lkml.org 
[lkml]   [2021]   [Jul]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] swap led_brightness from enum to typedef
Hi!

> >>> vim +18 include/media/v4l2-flash-led-class.h
> >>>
> >>> 14
> >>> 15 struct led_classdev_flash;
> >>> 16 struct led_classdev;
> >>> 17 struct v4l2_flash;
> >>> > 18 led_brightness;
> >>> 19
> >>>
> >>> ---
>
> >
> > Another patch was sent into the list to correct this error.
>
> Hopefully Pavel (LED subsystem maintainer) will comment soon-ish.
>
> My comments:
>
> a. This patch would be the right thing to do if your large patch had already been
> applied (merged) somewhere, but AFAIK it hasn't been. So:
>
> b. IMO you should resend your entire patch set with this fix included.
> Send it as "v2" (version 2) and explain the changes in it since your
> original patch (which was v1). This v2 explanation should be below the
> "---" line in the patch. (See Documentation/process/submitting-patches.rst
> for more info -- or ask for more info/help.)

I still remember the old patch, so b. is not strictly neccessary here.

> c. For your follow-up patch to include/media/v4l2-flash-led-class.h, which was:
>
> -led_brightness;
> +typedef u8 led_brightness;
>
> I would just add this to include/media/v4l2-flash-led-class.h:
>
> #include <linux/leds.h>
>
> That way, in a few years, when the type of led_brightness changes again,
> someone won't have to remember to search for other typedefs of it and
> update them also. Or maybe they will do that after a bug happens and
> someone notices it.
>
> (Note that I am just trying to help. Pavel has more of a final
> say-so about this.)

And your comments are reasonable.

Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2021-07-19 14:06    [W:0.095 / U:0.660 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site