| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <DGBI0L4E59ZY.24OGXR0XUDH4Z@disroot.org>
Date: Wed, 11 Feb 2026 00:07:50 +0530
From: "Kaustabh Chakraborty" <kauschluss@...root.org>
To: André Draszik <andre.draszik@...aro.org>, "Kaustabh
Chakraborty" <kauschluss@...root.org>, "Lee Jones" <lee@...nel.org>, "Pavel
Machek" <pavel@...nel.org>, "Rob Herring" <robh@...nel.org>, "Krzysztof
Kozlowski" <krzk+dt@...nel.org>, "Conor Dooley" <conor+dt@...nel.org>,
"MyungJoo Ham" <myungjoo.ham@...sung.com>, "Chanwoo Choi"
<cw00.choi@...sung.com>, "Sebastian Reichel" <sre@...nel.org>, "Krzysztof
Kozlowski" <krzk@...nel.org>, "Alexandre Belloni"
<alexandre.belloni@...tlin.com>, "Jonathan Corbet" <corbet@....net>, "Shuah
Khan" <skhan@...uxfoundation.org>
Cc: <linux-leds@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-pm@...r.kernel.org>,
<linux-samsung-soc@...r.kernel.org>, <linux-rtc@...r.kernel.org>,
<linux-doc@...r.kernel.org>
Subject: Re: [PATCH v2 08/12] leds: flash: add support for Samsung S2M
series PMIC flash LED device
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@...root.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@...root.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'
Powered by blists - more mailing lists