Re: [PATCH v2 08/12] leds: flash: add support for Samsung S2M series PMIC flash LED device

["Kaustabh Chakraborty" <kauschluss@disroot.org>]

On 2026-02-10 10:03 +00:00, André Draszik wrote:
> On Thu, 2026-02-05 at 21:46 +0530, Kaustabh Chakraborty wrote:
>> On 2026-02-04 16:55 +00:00, André Draszik wrote:
>> > Hi,
>> > 
>> > On Mon, 2026-01-26 at 00:37 +0530, Kaustabh Chakraborty wrote:
>> > > Add support for flash LEDs found in certain Samsung S2M series PMICs.
>> > > The device has two channels for LEDs, typically for the back and front
>> > > cameras in mobile devices. Both channels can be independently
>> > > controlled, and can be operated in torch or flash modes.
>> > > 
>> > > The driver includes initial support for the S2MU005 PMIC flash LEDs.
>> > > 
>> > > Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
>> > > ---
>> > >  drivers/leds/flash/Kconfig          |  12 ++
>> > >  drivers/leds/flash/Makefile         |   1 +
>> > >  drivers/leds/flash/leds-s2m-flash.c | 410 ++++++++++++++++++++++++++++++++++++
>> > >  3 files changed, 423 insertions(+)
>> > > 
>> > > diff --git a/drivers/leds/flash/Kconfig b/drivers/leds/flash/Kconfig
>> > > index 5e08102a67841..be62e05277429 100644
>> > > --- a/drivers/leds/flash/Kconfig
>> > > +++ b/drivers/leds/flash/Kconfig
>> > > @@ -114,6 +114,18 @@ config LEDS_RT8515
>> > >  	  To compile this driver as a module, choose M here: the module
>> > >  	  will be called leds-rt8515.
>> > >  
>> > > +config LEDS_S2M_FLASH
>> > > +	tristate "Samsung S2M series PMICs flash/torch LED support"
>> > > +	depends on LEDS_CLASS
>> > > +	depends on MFD_SEC_CORE
>> > > +	depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
>> > > +	select REGMAP_IRQ
>> > > +	help
>> > > +	  This option enables support for the flash/torch LEDs found in
>> > > +	  certain Samsung S2M series PMICs, such as the S2MU005. It has
>> > > +	  a LED channel dedicated for every physical LED. The LEDs can
>> > > +	  be controlled in flash and torch modes.
>> > > +
>> > >  config LEDS_SGM3140
>> > >  	tristate "LED support for the SGM3140"
>> > >  	depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
>> > > diff --git a/drivers/leds/flash/Makefile b/drivers/leds/flash/Makefile
>> > > index 712fb737a428e..44e6c1b4beb37 100644
>> > > --- a/drivers/leds/flash/Makefile
>> > > +++ b/drivers/leds/flash/Makefile
>> > > @@ -10,6 +10,7 @@ obj-$(CONFIG_LEDS_MAX77693)	+= leds-max77693.o
>> > >  obj-$(CONFIG_LEDS_QCOM_FLASH)	+= leds-qcom-flash.o
>> > >  obj-$(CONFIG_LEDS_RT4505)	+= leds-rt4505.o
>> > >  obj-$(CONFIG_LEDS_RT8515)	+= leds-rt8515.o
>> > > +obj-$(CONFIG_LEDS_S2M_FLASH)	+= leds-s2m-flash.o
>> > >  obj-$(CONFIG_LEDS_SGM3140)	+= leds-sgm3140.o
>> > >  obj-$(CONFIG_LEDS_SY7802)	+= leds-sy7802.o
>> > >  obj-$(CONFIG_LEDS_TPS6131X)	+= leds-tps6131x.o
>> > > diff --git a/drivers/leds/flash/leds-s2m-flash.c b/drivers/leds/flash/leds-s2m-flash.c
>> > > new file mode 100644
>> > > index 0000000000000..1be2745c475bf
>> > > --- /dev/null
>> > > +++ b/drivers/leds/flash/leds-s2m-flash.c
>> > > @@ -0,0 +1,410 @@
>> > > +// SPDX-License-Identifier: GPL-2.0
>> > > +/*
>> > > + * Flash and Torch LED Driver for Samsung S2M series PMICs.
>> > > + *
>> > > + * Copyright (c) 2015 Samsung Electronics Co., Ltd
>> > > + * Copyright (c) 2025 Kaustabh Chakraborty <kauschluss@disroot.org>
>> > > + */
>> > > +
>> > > +#include <linux/container_of.h>
>> > > +#include <linux/led-class-flash.h>
>> > > +#include <linux/mfd/samsung/core.h>
>> > > +#include <linux/mfd/samsung/s2mu005.h>
>> > > +#include <linux/module.h>
>> > > +#include <linux/of.h>
>> > > +#include <linux/platform_device.h>
>> > > +#include <linux/regmap.h>
>> > > +#include <media/v4l2-flash-led-class.h>
>> > > +
>> > > +#define MAX_CHANNELS	2
>> > > +
>> > > +struct s2m_fled {
>> > > +	struct device *dev;
>> > > +	struct regmap *regmap;
>> > > +	struct led_classdev_flash cdev;
>> > > +	struct v4l2_flash *v4l2_flash;
>> > > +	struct mutex lock;
>> > 
>> > Please add a (brief) comment describing what the mutex protects.
>> 
>> The mutex object prevents the concurrent access of flash control
>> registers by the LED and V4L2 subsystems. -- will add this.
>> 
>> > > +
>> > > +	/*
>> > > +	 * Get the LED enable register address. Revision EVT0 has the
>> > > +	 * register at CTRL4, while EVT1 and higher have it at CTRL6.
>> > > +	 */
>> > > +	if (priv->pmic_revision == 0)
>> > > +		reg_enable = S2MU005_REG_FLED_CTRL4;
>> > > +	else
>> > > +		reg_enable = S2MU005_REG_FLED_CTRL6;
>> > 
>> > You could REG_FIELD() and friends for this and everywhere else with
>> > similar if / else.
>> > 
>> 
>> REG_FIELD(), from what I understood, is for selecting a bit field inside
>> a single register. However this code chooses between two separate
>> registers. I believe your interpretation was incorrect? Please clarify.
>
> The first argument to REG_FIELD is the register itself, so reg fields can
> be used to describe this difference. See e.g. drivers/leds/rgb/leds-mt6370-rgb.c
> Of course, you could have a member variable instead to hold the register
> index if all bits are the same in both revisions. Either way would avoid
> having to constantly check the revision during runtime.

Wow, this is a great way of abstraction, thanks. I will try to implement
this in all drivers, let's see.

>
> Cheers,
> Andre'