Messages in this thread | | | Subject | Re: [PATCH v5 4/4] backlight: add led-backlight driver | From | Jean-Jacques Hiblot <> | Date | Tue, 10 Sep 2019 13:38:18 +0200 |
| |
On 10/09/2019 13:26, Daniel Thompson wrote: > >> endmenu >> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile >> index 63c507c07437..2a67642966a5 100644 >> --- a/drivers/video/backlight/Makefile >> +++ b/drivers/video/backlight/Makefile >> @@ -57,3 +57,4 @@ obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o >> obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o >> obj-$(CONFIG_BACKLIGHT_ARCXCNN) += arcxcnn_bl.o >> obj-$(CONFIG_BACKLIGHT_RAVE_SP) += rave-sp-backlight.o >> +obj-$(CONFIG_BACKLIGHT_LED) += led_bl.o >> diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c >> new file mode 100644 >> index 000000000000..a72456e11fb9 >> --- /dev/null >> +++ b/drivers/video/backlight/led_bl.c >> @@ -0,0 +1,264 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2015-2019 Texas Instruments Incorporated - http://www.ti.com/ >> + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> >> + * >> + * Based on pwm_bl.c >> + */ >> + >> +#include <linux/backlight.h> >> +#include <linux/gpio/consumer.h> > Maybe this is a nitpick but it is one I have now raised three times and > I don't recall any response, what symbols from this header are used in > this file? > > AFAICT everything defined in this header includes the string "gpio" and > that string doesn't appear anywhere in the file (except this line). > > >> +#include <linux/leds.h> >> +#include <linux/module.h> >> +#include <linux/platform_device.h> >> +#include <linux/slab.h> > Come to think of it, are you sure you need this include? devm_kzalloc() > doesn't comes from this file. > > >> +#define BKL_FULL_BRIGHTNESS 255 > This is unused. Please remove. > > > Other than that, looks good!
Thanks for the quick review. I forgot to cleanup the headers. I'll fix that now
JJ
> > > Daniel.
| |