Re: [PATCH v3 09/13] leds: flash: add support for Samsung S2M series PMIC flash LED device

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

On 2026-03-10 11:38 +00:00, Lee Jones wrote:
> On Wed, 25 Feb 2026, 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 | 429 ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 442 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..536a529889a9c
>> --- /dev/null
>> +++ b/drivers/leds/flash/leds-s2m-flash.c
>> @@ -0,0 +1,429 @@
>> +// 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 {
>
> This is not the fled, change this to _led and call pointers to it 'led'.
>
>> +	struct device *dev;
>> +	struct regmap *regmap;
>> +	struct led_classdev_flash cdev;
>
> This is the 'fled'.  Please change all pointer to it to 'fled'.
>
>> +	struct v4l2_flash *v4l2_flash;
>> +	/*
>> +	 * The mutex object prevents the concurrent access of flash control
>> +	 * registers by the LED and V4L2 subsystems.
>> +	 */
>> +	struct mutex lock;
>> +	const struct s2m_fled_spec *spec;
>
> This should not be in here.
>
>> +	unsigned int reg_enable;
>> +	u8 channel;
>> +	u8 flash_brightness;
>> +	u8 flash_timeout;
>> +};
>> +
>> +struct s2m_fled_spec {
>> +	u8 nr_channels;
>> +	u32 torch_max_brightness;
>> +	u32 flash_min_current_ua;
>> +	u32 flash_max_current_ua;
>> +	u32 flash_min_timeout_us;
>> +	u32 flash_max_timeout_us;
>> +	int (*torch_brightness_set_blocking)(struct led_classdev *led_cdev,
>> +					     enum led_brightness brightness);
>
> I'm really not a fan of pointers to functions at the driver level.
>
> If there are differences between devices, which I cannot see here, store
> an identifier and use that to choose which call-back function is the
> most appropriate.
>
>> +	const struct led_flash_ops *flash_ops;
>
> Why do we need to store these?
>
> The same principles apply to the function pointer above.

Yes, I have done exactly that. I have moved these values in
s2m_fled_init_channel() and renamed it s2mu005_fled_init_channel(). This
function will be chosen as a result of evaluation of a switch-block.

>> +};
>> +
>> +static struct led_classdev_flash *to_cdev_flash(struct led_classdev *cdev)
>> +{
>> +	return container_of(cdev, struct led_classdev_flash, led_cdev);
>> +}
>> +
>> +static struct s2m_fled *to_led_priv(struct led_classdev_flash *cdev)
>
> There is nothing private about s2m_{f}led.
>
> 'led' is the most common nomenclature.  So: led->channel, etc.
>
> Remove 'priv' throughout.
>
>> +{
>> +	return container_of(cdev, struct s2m_fled, cdev);
>
> 	return container_of(fled, struct s2m_led, fled);
>
>> +}
>> +
>> +static int s2m_fled_flash_brightness_set(struct led_classdev_flash *cdev,
>> +					 u32 brightness)
>> +{
>> +	struct s2m_fled *priv = to_led_priv(cdev);
>> +	struct led_flash_setting *setting = &cdev->brightness;
>> +
>> +	priv->flash_brightness = (brightness - setting->min) / setting->step;
>> +
>> +	return 0;
>> +}
>> +
>> +static int s2m_fled_flash_timeout_set(struct led_classdev_flash *cdev,
>> +				      u32 timeout)
>> +{
>> +	struct s2m_fled *priv = to_led_priv(cdev);
>> +	struct led_flash_setting *setting = &cdev->timeout;
>> +
>> +	priv->flash_timeout = (timeout - setting->min) / setting->step;
>> +
>> +	return 0;
>> +}
>> +
>> +#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
>> +static int s2m_fled_flash_external_strobe_set(struct v4l2_flash *v4l2_flash,
>> +					      bool enable)
>> +{
>> +	struct s2m_fled *priv = to_led_priv(v4l2_flash->fled_cdev);
>> +
>> +	mutex_lock(&priv->lock);
>> +
>> +	priv->cdev.ops->strobe_set(&priv->cdev, enable);
>> +
>> +	mutex_unlock(&priv->lock);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct v4l2_flash_ops s2m_fled_v4l2_flash_ops = {
>> +	.external_strobe_set = s2m_fled_flash_external_strobe_set,
>> +};
>> +#else
>> +static const struct v4l2_flash_ops s2m_fled_v4l2_flash_ops;
>> +#endif
>> +
>> +static void s2m_fled_v4l2_flash_release(void *v4l2_flash)
>> +{
>> +	v4l2_flash_release(v4l2_flash);
>> +}
>> +
>> +static int s2mu005_fled_torch_brightness_set(struct led_classdev *cdev,
>> +					     enum led_brightness value)
>> +{
>> +	struct s2m_fled *priv = to_led_priv(to_cdev_flash(cdev));
>> +	struct regmap *regmap = priv->regmap;
>> +	int ret;
>> +
>> +	mutex_lock(&priv->lock);
>> +
>> +	if (value == LED_OFF) {
>
> These defines are deprecated.
>
> From include/linux/leds.h:
>
> /* This is obsolete/useless. We now support variable maximum brightness. */
> enum led_brightness {
>         LED_OFF         = 0,
>         LED_ON          = 1,
>         LED_HALF        = 127,
>         LED_FULL        = 255,
> };
>

Let me know what am I supposed to use then. The
brightness_set_blocking() function is defined as such:

	int (*brightness_set_blocking)(struct led_classdev *led_cdev,
				       enum led_brightness brightness);

Which has enum led_brightness as one of its params.

Do I just ignore the 'obsolete' param for now and replace ` == LED_OFF`
with a logical NOT?

>> +		ret = regmap_clear_bits(regmap, priv->reg_enable,
>> +					S2MU005_FLED_TORCH_EN(priv->channel));
>> +		if (ret < 0)
>> +			dev_err(priv->dev, "failed to disable torch LED\n");
>> +		goto unlock;
>> +	}
>> +
>> +	ret = regmap_update_bits(regmap, S2MU005_REG_FLED_CH_CTRL1(priv->channel),
> j> +				 S2MU005_FLED_TORCH_IOUT,
>> +				 FIELD_PREP(S2MU005_FLED_TORCH_IOUT, value - 1));
>> +	if (ret < 0) {
>> +		dev_err(priv->dev, "failed to set torch current\n");
>> +		goto unlock;
>> +	}
>> +
>> +	ret = regmap_set_bits(regmap, priv->reg_enable,
>> +			      S2MU005_FLED_TORCH_EN(priv->channel));
>> +	if (ret < 0) {
>> +		dev_err(priv->dev, "failed to enable torch LED\n");
>> +		goto unlock;
>> +	}
>> +
>> +unlock:
>> +	mutex_unlock(&priv->lock);
>> +
>> +	return ret;
>> +}
>> +
>> +static int s2mu005_fled_flash_strobe_set(struct led_classdev_flash *cdev,
>> +					 bool state)
>> +{
>> +	struct s2m_fled *priv = to_led_priv(cdev);
>> +	struct regmap *regmap = priv->regmap;
>> +	int ret;
>> +
>> +	mutex_lock(&priv->lock);
>> +
>> +	ret = regmap_clear_bits(regmap, priv->reg_enable,
>> +				S2MU005_FLED_FLASH_EN(priv->channel));
>> +	if (ret < 0) {
>> +		dev_err(priv->dev, "failed to disable flash LED\n");
>> +		goto unlock;
>> +	}
>> +
>> +	if (!state)
>> +		goto unlock;
>> +
>> +	ret = regmap_update_bits(regmap, S2MU005_REG_FLED_CH_CTRL0(priv->channel),
>> +				 S2MU005_FLED_FLASH_IOUT,
>> +				 FIELD_PREP(S2MU005_FLED_FLASH_IOUT,
>> +					    priv->flash_brightness));
>> +	if (ret < 0) {
>> +		dev_err(priv->dev, "failed to set flash brightness\n");
>> +		goto unlock;
>> +	}
>> +
>> +	ret = regmap_update_bits(regmap, S2MU005_REG_FLED_CH_CTRL3(priv->channel),
>> +				 S2MU005_FLED_FLASH_TIMEOUT,
>> +				 FIELD_PREP(S2MU005_FLED_FLASH_TIMEOUT,
>> +					    priv->flash_timeout));
>> +	if (ret < 0) {
>> +		dev_err(priv->dev, "failed to set flash timeout\n");
>> +		goto unlock;
>> +	}
>> +
>> +	ret = regmap_set_bits(regmap, priv->reg_enable,
>> +			      S2MU005_FLED_FLASH_EN(priv->channel));
>> +	if (ret < 0) {
>> +		dev_err(priv->dev, "failed to enable flash LED\n");
>> +		goto unlock;
>> +	}
>> +
>> +unlock:
>> +	mutex_unlock(&priv->lock);
>> +
>> +	return 0;
>> +}
>> +
>> +static int s2mu005_fled_flash_strobe_get(struct led_classdev_flash *cdev,
>> +					 bool *state)
>> +{
>> +	struct s2m_fled *priv = to_led_priv(cdev);
>> +	struct regmap *regmap = priv->regmap;
>
> Since this is only used once, I don't see anything to gain from
> utilising a local variable here.
>
>> +	u8 channel = priv->channel;
>
> Same here.
>
>> +	u32 val;
>> +	int ret;
>> +
>> +	mutex_lock(&priv->lock);
>> +
>> +	ret = regmap_read(regmap, S2MU005_REG_FLED_STATUS, &val);
>> +	if (ret < 0) {
>> +		dev_err(priv->dev, "failed to fetch LED status");
>> +		goto unlock;
>> +	}
>> +
>> +	*state = !!(val & S2MU005_FLED_FLASH_STATUS(channel));
>> +
>> +unlock:
>> +	mutex_unlock(&priv->lock);
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct led_flash_ops s2mu005_fled_flash_ops = {
>> +	.flash_brightness_set = s2m_fled_flash_brightness_set,
>> +	.timeout_set = s2m_fled_flash_timeout_set,
>> +	.strobe_set = s2mu005_fled_flash_strobe_set,
>> +	.strobe_get = s2mu005_fled_flash_strobe_get,
>> +};
>> +
>> +static int s2mu005_fled_init(struct s2m_fled *priv)
>> +{
>> +	unsigned int val;
>> +	int ret;
>> +
>> +	/* Enable the LED channels. */
>> +	ret = regmap_set_bits(priv->regmap, S2MU005_REG_FLED_CTRL1,
>> +			      S2MU005_FLED_CH_EN);
>
> Feel free to use up to 100-char to eradicate these wraps.
>
>> +	if (ret < 0)
>> +		return dev_err_probe(priv->dev, ret, "failed to enable LED channels\n");
>> +
>> +	/*
>> +	 * Get the LED enable register address. Revision EVT0 has the
>> +	 * register at CTRL4, while EVT1 and higher have it at CTRL6.
>> +	 */
>> +	ret = regmap_read(priv->regmap, S2MU005_REG_ID, &val);
>> +	if (ret < 0)
>> +		return dev_err_probe(priv->dev, ret, "failed to read revision\n");
>> +
>> +	if (FIELD_GET(S2MU005_ID_MASK, val) == 0)
>
> A comment above to explain what this means please.

A comment is right above the regmap read. I will shift it below. (Seems
also like its a bit incorrect, will fix that too.)

>
> And/or define the zero to something obvious.
>
>> +		priv->reg_enable = S2MU005_REG_FLED_CTRL4;
>> +	else
>> +		priv->reg_enable = S2MU005_REG_FLED_CTRL6;
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct s2m_fled_spec s2mu005_fled_spec = {
>> +	.nr_channels = 2,
>> +	.torch_max_brightness = 16,
>> +	.flash_min_current_ua = 25000,
>> +	.flash_max_current_ua = 375000, /* 400000 causes flickering */
>> +	.flash_min_timeout_us = 62000,
>> +	.flash_max_timeout_us = 992000,
>> +	.torch_brightness_set_blocking = s2mu005_fled_torch_brightness_set,
>> +	.flash_ops = &s2mu005_fled_flash_ops,
>> +};
>> +
>> +static int s2m_fled_init_channel(struct s2m_fled *priv,
>> +				 struct fwnode_handle *fwnp)
>
> Unwrap all of these that fit into 100-chars.
>
>> +{
>> +	struct led_classdev *led = &priv->cdev.led_cdev;
>> +	struct led_init_data init_data = {};
>> +	struct v4l2_flash_config v4l2_cfg = {};
>> +	int ret;
>> +
>> +	led->max_brightness = priv->spec->torch_max_brightness;
>> +	led->brightness_set_blocking = priv->spec->torch_brightness_set_blocking;
>> +	led->flags |= LED_DEV_CAP_FLASH;
>> +
>> +	priv->cdev.timeout.min = priv->spec->flash_min_timeout_us;
>> +	priv->cdev.timeout.step = priv->spec->flash_min_timeout_us;
>> +	priv->cdev.timeout.max = priv->spec->flash_max_timeout_us;
>> +	priv->cdev.timeout.val = priv->spec->flash_max_timeout_us;
>> +
>> +	priv->cdev.brightness.min = priv->spec->flash_min_current_ua;
>> +	priv->cdev.brightness.step = priv->spec->flash_min_current_ua;
>> +	priv->cdev.brightness.max = priv->spec->flash_max_current_ua;
>> +	priv->cdev.brightness.val = priv->spec->flash_max_current_ua;
>
> Moving things around in priv/data tells me that something isn't right.
>
> I think the spec should be removed from what is currently called priv.
>
>> +	s2m_fled_flash_timeout_set(&priv->cdev, priv->cdev.timeout.val);
>> +	s2m_fled_flash_brightness_set(&priv->cdev, priv->cdev.brightness.val);
>> +
>> +	priv->cdev.ops = priv->spec->flash_ops;
>> +
>> +	init_data.fwnode = fwnp;
>> +	ret = devm_led_classdev_flash_register_ext(priv->dev, &priv->cdev,
>> +						   &init_data);
>> +	if (ret < 0)
>> +		return dev_err_probe(priv->dev, ret, "failed to create LED flash device\n");
>> +
>> +	v4l2_cfg.intensity.min = priv->spec->flash_min_current_ua;
>> +	v4l2_cfg.intensity.step = priv->spec->flash_min_current_ua;
>> +	v4l2_cfg.intensity.max = priv->spec->flash_max_current_ua;
>> +	v4l2_cfg.intensity.val = priv->spec->flash_max_current_ua;
>> +
>> +	v4l2_cfg.has_external_strobe = true;
>> +
>> +	priv->v4l2_flash = v4l2_flash_init(priv->dev, fwnp, &priv->cdev,
>> +					   &s2m_fled_v4l2_flash_ops, &v4l2_cfg);
>> +	if (IS_ERR(priv->v4l2_flash)) {
>> +		v4l2_flash_release(priv->v4l2_flash);
>> +		return dev_err_probe(priv->dev, PTR_ERR(priv->v4l2_flash),
>> +				     "failed to create V4L2 flash device\n");
>> +	}
>> +
>> +	ret = devm_add_action_or_reset(priv->dev, (void *)s2m_fled_v4l2_flash_release,
>> +				       priv->v4l2_flash);
>> +	if (ret < 0)
>> +		return dev_err_probe(priv->dev, ret, "failed to add cleanup action\n");
>
> v4l2_flash_release()?

Why would we call flash_release here? it should be called when the
driver is unloaded.

Regarding that, I have added an action which does exactly that. The
actual function is wrapped in s2m_fled_v4l2_flash_release() so as to not
make CFI unhappy.

>
>> +	return 0;
>> +}
>> +
>> +static int s2m_fled_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct sec_pmic_dev *pmic_drvdata = dev_get_drvdata(dev->parent);
>
> s/pmic_drvdata/ddata/
>
>> +	struct s2m_fled *priv;
>> +	bool channel_initialized[MAX_CHANNELS] = { false };
>> +	int ret;
>> +
>> +	priv = devm_kzalloc(dev, sizeof(*priv) * MAX_CHANNELS, GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	platform_set_drvdata(pdev, priv);
>
> Where is this used?

>> +	priv->dev = dev;
>
> This is already available to you in what is currently called:
>
>   priv->cdev->led_cdev->dev

I am assuming priv->dev is to be removed so as to save space.

>
> Also, isn't this an array?
>

Yeah so, about that. It allocates n-many s2m_fled (now s2m_led) for
n-many LEDs. it does all allocations in the first struct here, and then
for subsequent LEDs it copies them over (that's the convoluted logic
below). Not the cleanest solution, maybe there's a better way to solve
it? [a solution discussed below]

>> +	priv->regmap = pmic_drvdata->regmap_pmic;
>> +
>> +	switch (platform_get_device_id(pdev)->driver_data) {
>> +	case S2MU005:
>> +		priv->spec = &s2mu005_fled_spec;
>> +		ret = s2mu005_fled_init(priv);
>> +		if (ret)
>> +			return ret;
>> +		break;
>> +	default:
>> +		return dev_err_probe(dev, -ENODEV,
>> +				     "device type %d is not supported by driver\n",
>> +				     pmic_drvdata->device_type);
>> +	}
>> +
>> +	if (priv->spec->nr_channels > MAX_CHANNELS)
>> +		return dev_err_probe(dev, -EINVAL,
>> +				     "number of channels specified (%u) exceeds the limit (%u)\n",
>> +				     priv->spec->nr_channels, MAX_CHANNELS);
>> +
>> +	device_for_each_child_node_scoped(dev, child) {
>> +		u32 reg;
>> +
>> +		if (fwnode_property_read_u32(child, "reg", &reg))
>> +			continue;
>> +
>> +		if (reg >= priv->spec->nr_channels) {
>> +			dev_warn(dev, "channel %d is non-existent\n", reg);
>> +			continue;
>> +		}
>> +
>> +		if (channel_initialized[reg]) {
>
> Do you really need a whole variable just for this?
>
> If this a risk?  If it really is, why not check (priv[reg].dev)?

Your above comment suggests an alternative approach to get dev from
priv. Assuming you implied that priv->dev be removed, doesn't this come
in conflict?

The same thing however can be done with regmap, so that's a possibility.
I will move over the `priv->regmap = pmic_drvdata->regmap_pmic` from
above and add it only if the channel is initialized.
As a bonus this also removes "setting pointers to the first element and
copying them over" procedure, that would be a cleaner solution as well.

>> +			dev_warn(dev, "duplicate node for channel %d\n", reg);
>> +			continue;
>> +		}
>> +
>> +		priv[reg].dev = priv->dev;
>
> What on earth is going on here?
>
>> +		priv[reg].regmap = priv->regmap;
>> +		priv[reg].spec = priv->spec;
>> +		priv[reg].reg_enable = priv->reg_enable;
>
> And why do you need ${reg} copies of the same data?
>
> Sounds like a waste of resources, no?
>
>> +		priv[reg].channel = (u8)reg;
>> +
>> +		ret = devm_mutex_init(dev, &priv[reg].lock);
>
> Why not just look the whole device?  Rather than per channel?
>
> To channels have no shared resources?

No, they are independent as far as I can tell.

>> +		if (ret)
>> +			return dev_err_probe(dev, ret, "failed to create mutex lock\n");
>
> You can drop the ' lock' part.
>
>> +
>> +		ret = s2m_fled_init_channel(priv + reg, child);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		channel_initialized[reg] = true;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct platform_device_id s2m_fled_id_table[] = {
>> +	{ "s2mu005-flash", S2MU005 },
>> +	{ /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(platform, s2m_fled_id_table);
>> +
>> +/*
>> + * Device is instantiated through parent MFD device and device matching
>> + * is done through platform_device_id.
>> + *
>> + * However if device's DT node contains proper compatible and driver is
>> + * built as a module, then the *module* matching will be done through DT
>> + * aliases. This requires of_device_id table. In the same time this will
>> + * not change the actual *device* matching so do not add .of_match_table.
>> + */
>
> All of this is already implied.
>
> No need to have this comment on all MFD sub-devices.
>
>> +static const struct of_device_id s2m_fled_of_match_table[] = {
>> +	{
>> +		.compatible = "samsung,s2mu005-flash",
>> +		.data = (void *)S2MU005,
>
> These usually fit on a single line.
>
>> +	}, {
>> +		/* sentinel */
>> +	},
>> +};
>> +MODULE_DEVICE_TABLE(of, s2m_fled_of_match_table);
>> +
>> +static struct platform_driver s2m_fled_driver = {
>> +	.driver = {
>> +		.name = "s2m-flash",
>> +	},
>> +	.probe = s2m_fled_probe,
>> +	.id_table = s2m_fled_id_table,
>> +};
>> +module_platform_driver(s2m_fled_driver);
>> +
>> +MODULE_DESCRIPTION("Flash/Torch LED Driver For Samsung S2M Series PMICs");
>> +MODULE_AUTHOR("Kaustabh Chakraborty <kauschluss@disroot.org>");
>> +MODULE_LICENSE("GPL");
>> 
>> -- 
>> 2.52.0
>>