Re: [PATCH 2/3] leds: as3645a: Add LED flash class driver
Hi Jacek, On Wed, Aug 16, 2017 at 10:27:31PM +0200, Jacek Anaszewski wrote: > Hi Sakari, > > Thanks for the patch. Thanks for the review! > > I have few more remarks regarding LED class device naming and > pm handling below. > > On 08/16/2017 02:55 PM, Sakari Ailus wrote: > > From: Sakari Ailus> > > > Add a LED flash class driver for the as3654a flash controller. A V4L2 flash > > driver for it already exists (drivers/media/i2c/as3645a.c), and this driver > > is based on that. > > > > Signed-off-by: Sakari Ailus > > --- > > MAINTAINERS | 6 + > > drivers/leds/Kconfig| 8 + > > drivers/leds/Makefile | 1 + > > drivers/leds/leds-as3645a.c | 785 > > > > 4 files changed, 800 insertions(+) > > create mode 100644 drivers/leds/leds-as3645a.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 931abca006b7..8f40ba2e5303 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -2124,6 +2124,12 @@ F: arch/arm64/ > > F: Documentation/arm64/ > > > > AS3645A LED FLASH CONTROLLER DRIVER > > +M: Sakari Ailus > > +L: linux-l...@vger.kernel.org > > +S: Maintained > > +F: drivers/leds/leds-as3645a.c > > + > > +AS3645A LED FLASH CONTROLLER DRIVER > > M: Laurent Pinchart > > L: linux-media@vger.kernel.org > > T: git git://linuxtv.org/media_tree.git > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > > index 594b24d410c3..bad3a4098104 100644 > > --- a/drivers/leds/Kconfig > > +++ b/drivers/leds/Kconfig > > @@ -58,6 +58,14 @@ config LEDS_AAT1290 > > help > > This option enables support for the LEDs on the AAT1290. > > > > +config LEDS_AS3645A > > + tristate "AS3645A LED flash controller support" > > + depends on I2C && LEDS_CLASS_FLASH > > + help > > + Enable LED flash class support for AS3645A LED flash > > + controller. V4L2 flash API is provided as well if > > + CONFIG_V4L2_FLASH_API is enabled. > > + > > config LEDS_BCM6328 > > tristate "LED Support for Broadcom BCM6328" > > depends on LEDS_CLASS > > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > > index 909dae62ba05..7d7b26552923 100644 > > --- a/drivers/leds/Makefile > > +++ b/drivers/leds/Makefile > > @@ -8,6 +8,7 @@ obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o > > # LED Platform Drivers > > obj-$(CONFIG_LEDS_88PM860X)+= leds-88pm860x.o > > obj-$(CONFIG_LEDS_AAT1290) += leds-aat1290.o > > +obj-$(CONFIG_LEDS_AS3645A) += leds-as3645a.o > > obj-$(CONFIG_LEDS_BCM6328) += leds-bcm6328.o > > obj-$(CONFIG_LEDS_BCM6358) += leds-bcm6358.o > > obj-$(CONFIG_LEDS_BD2802) += leds-bd2802.o > > diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c > > new file mode 100644 > > index ..2335510a08e1 > > --- /dev/null > > +++ b/drivers/leds/leds-as3645a.c > > @@ -0,0 +1,785 @@ > > +/* > > + * drivers/leds/leds-as3645a.c - AS3645A and LM3555 flash controllers > > driver > > + * > > + * Copyright (C) 2008-2011 Nokia Corporation > > + * Copyright (c) 2011, 2017 Intel Corporation. > > + * > > + * Based on drivers/media/i2c/as3645a.c. > > + * > > + * Contact: Sakari Ailus > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License > > + * version 2 as published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope that it will be useful, but > > + * WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + * General Public License for more details. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > + > > +#define AS_TIMER_US_TO_CODE(t) (((t) / 1000 - 100) / > > 50) > > +#define AS_TIMER_CODE_TO_US(c) ((50 * (c) + 100) * > > 1000) > > + > > +/* Register definitions */ > > + > > +/* Read-only Design info register: Reset state: 0001 */ > > +#define AS_DESIGN_INFO_REG 0x00 > > +#define AS_DESIGN_INFO_FACTORY(x) (((x) >> 4)) > > +#define AS_DESIGN_INFO_MODEL(x)((x) & 0x0f) > > + > > +/* Read-only Version control register: Reset state: > > + * for first engineering samples > > + */ > > +#define AS_VERSION_CONTROL_REG 0x01 > > +#define AS_VERSION_CONTROL_RFU(x) (((x) >> 4)) > > +#define AS_VERSION_CONTROL_VERSION(x) ((x) & 0x0f) > > + > > +/* Read / Write(Indicator and timer register): Reset state: > > */ > > +#define AS_INDICATOR_AND_TIMER_REG 0x02 > > +#define AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT
Re: [PATCH 2/3] leds: as3645a: Add LED flash class driver
Hi Sakari, Thanks for the patch. I have few more remarks regarding LED class device naming and pm handling below. On 08/16/2017 02:55 PM, Sakari Ailus wrote: > From: Sakari Ailus> > Add a LED flash class driver for the as3654a flash controller. A V4L2 flash > driver for it already exists (drivers/media/i2c/as3645a.c), and this driver > is based on that. > > Signed-off-by: Sakari Ailus > --- > MAINTAINERS | 6 + > drivers/leds/Kconfig| 8 + > drivers/leds/Makefile | 1 + > drivers/leds/leds-as3645a.c | 785 > > 4 files changed, 800 insertions(+) > create mode 100644 drivers/leds/leds-as3645a.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 931abca006b7..8f40ba2e5303 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2124,6 +2124,12 @@ F: arch/arm64/ > F: Documentation/arm64/ > > AS3645A LED FLASH CONTROLLER DRIVER > +M: Sakari Ailus > +L: linux-l...@vger.kernel.org > +S: Maintained > +F: drivers/leds/leds-as3645a.c > + > +AS3645A LED FLASH CONTROLLER DRIVER > M: Laurent Pinchart > L: linux-media@vger.kernel.org > T: git git://linuxtv.org/media_tree.git > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 594b24d410c3..bad3a4098104 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -58,6 +58,14 @@ config LEDS_AAT1290 > help >This option enables support for the LEDs on the AAT1290. > > +config LEDS_AS3645A > + tristate "AS3645A LED flash controller support" > + depends on I2C && LEDS_CLASS_FLASH > + help > + Enable LED flash class support for AS3645A LED flash > + controller. V4L2 flash API is provided as well if > + CONFIG_V4L2_FLASH_API is enabled. > + > config LEDS_BCM6328 > tristate "LED Support for Broadcom BCM6328" > depends on LEDS_CLASS > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index 909dae62ba05..7d7b26552923 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -8,6 +8,7 @@ obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o > # LED Platform Drivers > obj-$(CONFIG_LEDS_88PM860X) += leds-88pm860x.o > obj-$(CONFIG_LEDS_AAT1290) += leds-aat1290.o > +obj-$(CONFIG_LEDS_AS3645A) += leds-as3645a.o > obj-$(CONFIG_LEDS_BCM6328) += leds-bcm6328.o > obj-$(CONFIG_LEDS_BCM6358) += leds-bcm6358.o > obj-$(CONFIG_LEDS_BD2802)+= leds-bd2802.o > diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c > new file mode 100644 > index ..2335510a08e1 > --- /dev/null > +++ b/drivers/leds/leds-as3645a.c > @@ -0,0 +1,785 @@ > +/* > + * drivers/leds/leds-as3645a.c - AS3645A and LM3555 flash controllers driver > + * > + * Copyright (C) 2008-2011 Nokia Corporation > + * Copyright (c) 2011, 2017 Intel Corporation. > + * > + * Based on drivers/media/i2c/as3645a.c. > + * > + * Contact: Sakari Ailus > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#define AS_TIMER_US_TO_CODE(t) (((t) / 1000 - 100) / > 50) > +#define AS_TIMER_CODE_TO_US(c) ((50 * (c) + 100) * > 1000) > + > +/* Register definitions */ > + > +/* Read-only Design info register: Reset state: 0001 */ > +#define AS_DESIGN_INFO_REG 0x00 > +#define AS_DESIGN_INFO_FACTORY(x)(((x) >> 4)) > +#define AS_DESIGN_INFO_MODEL(x) ((x) & 0x0f) > + > +/* Read-only Version control register: Reset state: > + * for first engineering samples > + */ > +#define AS_VERSION_CONTROL_REG 0x01 > +#define AS_VERSION_CONTROL_RFU(x)(((x) >> 4)) > +#define AS_VERSION_CONTROL_VERSION(x)((x) & 0x0f) > + > +/* Read / Write (Indicator and timer register): Reset state: > */ > +#define AS_INDICATOR_AND_TIMER_REG 0x02 > +#define AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT 0 > +#define AS_INDICATOR_AND_TIMER_VREF_SHIFT4 > +#define AS_INDICATOR_AND_TIMER_INDICATOR_SHIFT 6 > + > +/* Read / Write (Current set register): Reset state: 0110 1001 */ > +#define AS_CURRENT_SET_REG 0x03 > +#define AS_CURRENT_ASSIST_LIGHT_SHIFT0 > +#define
Re: [PATCH 2/3] leds: as3645a: Add LED flash class driver
Hello Sakari, I haven't looked at the driver, but just have a comment about the I2C subsystem. On Wed, Aug 16, 2017 at 2:55 PM, Sakari Ailuswrote: [snip] > + > +static const struct of_device_id as3645a_of_table[] = { > + { .compatible = "ams,as3645a" }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, as3645a_of_table); > + > +SIMPLE_DEV_PM_OPS(as3645a_pm_ops, as3645a_resume, as3645a_suspend); > + > +static struct i2c_driver as3645a_i2c_driver = { > + .driver = { > + .of_match_table = as3645a_of_table, > + .name = AS_NAME, > + .pm = _pm_ops, > + }, > + .probe_new = as3645a_probe, > + .remove = as3645a_remove, > +}; > + > +module_i2c_driver(as3645a_i2c_driver); > + The I2C core is still broken w.r.t reporting a proper MODALIAS for OF registered devices, it will report a MODALIAS=i2c:as3645a in this case. So if you build this as a module, autoload won't work. In theory this will be fixed soon, but for now you should add a i2c_device_id table and export it with MODULE_DEVICE_TABLE(i2c,...) if you care about module autoloading. Best regards, Javier
[PATCH 2/3] leds: as3645a: Add LED flash class driver
From: Sakari AilusAdd a LED flash class driver for the as3654a flash controller. A V4L2 flash driver for it already exists (drivers/media/i2c/as3645a.c), and this driver is based on that. Signed-off-by: Sakari Ailus --- MAINTAINERS | 6 + drivers/leds/Kconfig| 8 + drivers/leds/Makefile | 1 + drivers/leds/leds-as3645a.c | 785 4 files changed, 800 insertions(+) create mode 100644 drivers/leds/leds-as3645a.c diff --git a/MAINTAINERS b/MAINTAINERS index 931abca006b7..8f40ba2e5303 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2124,6 +2124,12 @@ F: arch/arm64/ F: Documentation/arm64/ AS3645A LED FLASH CONTROLLER DRIVER +M: Sakari Ailus +L: linux-l...@vger.kernel.org +S: Maintained +F: drivers/leds/leds-as3645a.c + +AS3645A LED FLASH CONTROLLER DRIVER M: Laurent Pinchart L: linux-media@vger.kernel.org T: git git://linuxtv.org/media_tree.git diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 594b24d410c3..bad3a4098104 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -58,6 +58,14 @@ config LEDS_AAT1290 help This option enables support for the LEDs on the AAT1290. +config LEDS_AS3645A + tristate "AS3645A LED flash controller support" + depends on I2C && LEDS_CLASS_FLASH + help + Enable LED flash class support for AS3645A LED flash + controller. V4L2 flash API is provided as well if + CONFIG_V4L2_FLASH_API is enabled. + config LEDS_BCM6328 tristate "LED Support for Broadcom BCM6328" depends on LEDS_CLASS diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 909dae62ba05..7d7b26552923 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o # LED Platform Drivers obj-$(CONFIG_LEDS_88PM860X)+= leds-88pm860x.o obj-$(CONFIG_LEDS_AAT1290) += leds-aat1290.o +obj-$(CONFIG_LEDS_AS3645A) += leds-as3645a.o obj-$(CONFIG_LEDS_BCM6328) += leds-bcm6328.o obj-$(CONFIG_LEDS_BCM6358) += leds-bcm6358.o obj-$(CONFIG_LEDS_BD2802) += leds-bd2802.o diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c new file mode 100644 index ..2335510a08e1 --- /dev/null +++ b/drivers/leds/leds-as3645a.c @@ -0,0 +1,785 @@ +/* + * drivers/leds/leds-as3645a.c - AS3645A and LM3555 flash controllers driver + * + * Copyright (C) 2008-2011 Nokia Corporation + * Copyright (c) 2011, 2017 Intel Corporation. + * + * Based on drivers/media/i2c/as3645a.c. + * + * Contact: Sakari Ailus + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#define AS_TIMER_US_TO_CODE(t) (((t) / 1000 - 100) / 50) +#define AS_TIMER_CODE_TO_US(c) ((50 * (c) + 100) * 1000) + +/* Register definitions */ + +/* Read-only Design info register: Reset state: 0001 */ +#define AS_DESIGN_INFO_REG 0x00 +#define AS_DESIGN_INFO_FACTORY(x) (((x) >> 4)) +#define AS_DESIGN_INFO_MODEL(x)((x) & 0x0f) + +/* Read-only Version control register: Reset state: + * for first engineering samples + */ +#define AS_VERSION_CONTROL_REG 0x01 +#define AS_VERSION_CONTROL_RFU(x) (((x) >> 4)) +#define AS_VERSION_CONTROL_VERSION(x) ((x) & 0x0f) + +/* Read / Write(Indicator and timer register): Reset state: */ +#define AS_INDICATOR_AND_TIMER_REG 0x02 +#define AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT 0 +#define AS_INDICATOR_AND_TIMER_VREF_SHIFT 4 +#define AS_INDICATOR_AND_TIMER_INDICATOR_SHIFT 6 + +/* Read / Write(Current set register): Reset state: 0110 1001 */ +#define AS_CURRENT_SET_REG 0x03 +#define AS_CURRENT_ASSIST_LIGHT_SHIFT 0 +#define AS_CURRENT_LED_DET_ON (1 << 3) +#define AS_CURRENT_FLASH_CURRENT_SHIFT 4 + +/* Read / Write(Control register): Reset state: 1011 0100 */ +#define AS_CONTROL_REG 0x04 +#define AS_CONTROL_MODE_SETTING_SHIFT 0 +#define AS_CONTROL_STROBE_ON (1 << 2) +#define AS_CONTROL_OUT_ON (1 << 3) +#define