* [PATCH v5 1/6] dt-bindings: leds: Document TI LM3560 Synchronous Boost Flash Driver
2026-05-03 16:44 [PATCH v5 0/6] media: lm3560: convert to use OF bindings Svyatoslav Ryhel
@ 2026-05-03 16:44 ` Svyatoslav Ryhel
2026-05-03 16:44 ` [PATCH v5 2/6] media: i2c: lm3560: Fix v4l2 subdev registration Svyatoslav Ryhel
` (5 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-03 16:44 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sakari Ailus, Mauro Carvalho Chehab,
Svyatoslav Ryhel
Cc: linux-leds, devicetree, linux-kernel, linux-media
Document TI LM3559 and LM3560 Synchronous Boost Flash Driver used for
camera flash LEDs.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
.../devicetree/bindings/leds/ti,lm3560.yaml | 163 ++++++++++++++++++
1 file changed, 163 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/ti,lm3560.yaml
diff --git a/Documentation/devicetree/bindings/leds/ti,lm3560.yaml b/Documentation/devicetree/bindings/leds/ti,lm3560.yaml
new file mode 100644
index 000000000000..e79de4a57f83
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/ti,lm3560.yaml
@@ -0,0 +1,163 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/ti,lm3560.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI LM3560 Synchronous Boost Flash Driver
+
+maintainers:
+ - Svyatoslav Ryhel <clamor95@gmail.com>
+
+description:
+ The LM3560 is a 2-MHz fixed frequency synchronous boost converter with two
+ 1000-mA constant current drivers for high-current white LEDs. The dual high-
+ side current sources allow for grounded cathode LED operation and can be
+ tied together for providing flash currents at up to 2 A through a single LED.
+ An adaptive regulation method ensures the current for each LED remains in
+ regulation and maximizes efficiency.
+
+properties:
+ compatible:
+ enum:
+ - ti,lm3559
+ - ti,lm3560
+
+ reg:
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ enable-gpios:
+ description: GPIO connected to the HWEN pin.
+ maxItems: 1
+
+ vin-supply:
+ description: Supply connected to the IN line.
+
+ flash-max-timeout-us:
+ minimum: 32000
+ maximum: 1024000
+ default: 32000
+
+ ti,peak-current-microamp:
+ description:
+ The LM3560 features 4 selectable current limits 1.6A, 2.3A, 3A, and 3.6A
+ (in case of LM3559 - 1.4A, 2.1A, 2.7A, and 3.2A). When the current limit
+ is reached, the LM3559/LM3560 stops switching for the remainder of the
+ switching cycle.
+
+patternProperties:
+ '^led@[01]$':
+ type: object
+ $ref: /schemas/leds/common.yaml#
+ description: LED control bank nodes.
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ description: Control bank selection (0 = bank A, 1 = bank B).
+ maximum: 1
+
+ required:
+ - reg
+ - flash-max-microamp
+ - led-max-microamp
+
+allOf:
+ - $ref: /schemas/leds/common.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: ti,lm3559
+ then:
+ properties:
+ ti,peak-current-microamp:
+ enum: [1400000, 2100000, 2700000, 3200000]
+ default: 1400000
+ patternProperties:
+ '^led@[01]$':
+ properties:
+ flash-max-microamp:
+ minimum: 62500
+ maximum: 1000000
+ led-max-microamp:
+ minimum: 31250
+ maximum: 250000
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: ti,lm3560
+ then:
+ properties:
+ ti,peak-current-microamp:
+ enum: [1600000, 2300000, 3000000, 3600000]
+ default: 1600000
+ patternProperties:
+ '^led@[01]$':
+ properties:
+ flash-max-microamp:
+ minimum: 56250
+ maximum: 900000
+ led-max-microamp:
+ minimum: 28125
+ maximum: 225000
+
+required:
+ - compatible
+ - reg
+ - '#address-cells'
+ - '#size-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/leds/common.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led-controller@53 {
+ compatible = "ti,lm3560";
+ reg = <0x53>;
+
+ enable-gpios = <&gpio 28 GPIO_ACTIVE_HIGH>;
+ vin-supply = <&vdd_3v3_sys>;
+
+ flash-max-timeout-us = <1024000>;
+ ti,peak-current-microamp = <1600000>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led@0 {
+ reg = <0>;
+
+ function = LED_FUNCTION_FLASH;
+ color = <LED_COLOR_ID_WHITE>;
+
+ flash-max-microamp = <562500>;
+ led-max-microamp = <156250>;
+ };
+
+ led@1 {
+ reg = <1>;
+
+ function = LED_FUNCTION_FLASH;
+ color = <LED_COLOR_ID_YELLOW>;
+
+ flash-max-microamp = <562500>;
+ led-max-microamp = <156250>;
+ };
+ };
+ };
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 2/6] media: i2c: lm3560: Fix v4l2 subdev registration
2026-05-03 16:44 [PATCH v5 0/6] media: lm3560: convert to use OF bindings Svyatoslav Ryhel
2026-05-03 16:44 ` [PATCH v5 1/6] dt-bindings: leds: Document TI LM3560 Synchronous Boost Flash Driver Svyatoslav Ryhel
@ 2026-05-03 16:44 ` Svyatoslav Ryhel
2026-05-03 16:44 ` [PATCH v5 3/6] media: i2c: lm3560: Optimize mutex lock usage Svyatoslav Ryhel
` (4 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-03 16:44 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sakari Ailus, Mauro Carvalho Chehab,
Svyatoslav Ryhel
Cc: linux-leds, devicetree, linux-kernel, linux-media
The existing driver does not call media subdev registration, making it
invisible to the media framework. Since the LM3560 supports two
independent LEDs, register each LED as a separate media entity.
Because registering LEDs before device initialization may cause access
attempts before the hardware is ready, lm3560_init_device has been moved
before the subdevice initializations.
An additional helper, lm3560_subdev_cleanup, was added to release LED0 if
the initialization of LED1 fails, and to deregister both LEDs in the
remove function.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/media/i2c/lm3560.c | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
index f4cc844f4e3c..edfb07587cab 100644
--- a/drivers/media/i2c/lm3560.c
+++ b/drivers/media/i2c/lm3560.c
@@ -364,8 +364,15 @@ static int lm3560_subdev_init(struct lm3560_flash *flash,
goto err_out;
flash->subdev_led[led_no].entity.function = MEDIA_ENT_F_FLASH;
- return rval;
+ rval = v4l2_async_register_subdev(&flash->subdev_led[led_no]);
+ if (rval < 0) {
+ dev_err(flash->dev, "failed to register V4L2 subdev");
+ goto error_out_media;
+ }
+ return rval;
+error_out_media:
+ media_entity_cleanup(&flash->subdev_led[led_no].entity);
err_out:
v4l2_ctrl_handler_free(&flash->ctrls_led[led_no]);
return rval;
@@ -391,6 +398,14 @@ static int lm3560_init_device(struct lm3560_flash *flash)
return rval;
}
+static void lm3560_subdev_cleanup(struct lm3560_flash *flash,
+ enum lm3560_led_id led_no)
+{
+ v4l2_async_unregister_subdev(&flash->subdev_led[led_no]);
+ v4l2_ctrl_handler_free(&flash->ctrls_led[led_no]);
+ media_entity_cleanup(&flash->subdev_led[led_no].entity);
+}
+
static int lm3560_probe(struct i2c_client *client)
{
struct lm3560_flash *flash;
@@ -425,17 +440,19 @@ static int lm3560_probe(struct i2c_client *client)
flash->dev = &client->dev;
mutex_init(&flash->lock);
- rval = lm3560_subdev_init(flash, LM3560_LED0, "lm3560-led0");
+ rval = lm3560_init_device(flash);
if (rval < 0)
return rval;
- rval = lm3560_subdev_init(flash, LM3560_LED1, "lm3560-led1");
+ rval = lm3560_subdev_init(flash, LM3560_LED0, "lm3560-led0");
if (rval < 0)
return rval;
- rval = lm3560_init_device(flash);
- if (rval < 0)
+ rval = lm3560_subdev_init(flash, LM3560_LED1, "lm3560-led1");
+ if (rval < 0) {
+ lm3560_subdev_cleanup(flash, LM3560_LED0);
return rval;
+ }
i2c_set_clientdata(client, flash);
@@ -447,11 +464,8 @@ static void lm3560_remove(struct i2c_client *client)
struct lm3560_flash *flash = i2c_get_clientdata(client);
unsigned int i;
- for (i = LM3560_LED0; i < LM3560_LED_MAX; i++) {
- v4l2_device_unregister_subdev(&flash->subdev_led[i]);
- v4l2_ctrl_handler_free(&flash->ctrls_led[i]);
- media_entity_cleanup(&flash->subdev_led[i].entity);
- }
+ for (i = LM3560_LED0; i < LM3560_LED_MAX; i++)
+ lm3560_subdev_cleanup(flash, i);
}
static const struct i2c_device_id lm3560_id_table[] = {
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 3/6] media: i2c: lm3560: Optimize mutex lock usage
2026-05-03 16:44 [PATCH v5 0/6] media: lm3560: convert to use OF bindings Svyatoslav Ryhel
2026-05-03 16:44 ` [PATCH v5 1/6] dt-bindings: leds: Document TI LM3560 Synchronous Boost Flash Driver Svyatoslav Ryhel
2026-05-03 16:44 ` [PATCH v5 2/6] media: i2c: lm3560: Fix v4l2 subdev registration Svyatoslav Ryhel
@ 2026-05-03 16:44 ` Svyatoslav Ryhel
2026-05-04 6:26 ` Sakari Ailus
2026-05-03 16:44 ` [PATCH v5 4/6] media: i2c: lm3560: Convert to use OF bindings Svyatoslav Ryhel
` (3 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-03 16:44 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sakari Ailus, Mauro Carvalho Chehab,
Svyatoslav Ryhel
Cc: linux-leds, devicetree, linux-kernel, linux-media
Pass the device's own mutex lock to the control handler so that the media
framework can handle control access instead of managing it manually. The
lock must be common to both sub-devices since they share same hardware,
so the individual sub-device locks will not work here.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/media/i2c/lm3560.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
index edfb07587cab..5b568ed9536b 100644
--- a/drivers/media/i2c/lm3560.c
+++ b/drivers/media/i2c/lm3560.c
@@ -162,14 +162,12 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
struct lm3560_flash *flash = to_lm3560_flash(ctrl, led_no);
int rval = -EINVAL;
- mutex_lock(&flash->lock);
-
if (ctrl->id == V4L2_CID_FLASH_FAULT) {
s32 fault = 0;
unsigned int reg_val;
rval = regmap_read(flash->regmap, REG_FLAG, ®_val);
if (rval < 0)
- goto out;
+ return rval;
if (reg_val & FAULT_SHORT_CIRCUIT)
fault |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
if (reg_val & FAULT_OVERTEMP)
@@ -179,8 +177,6 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
ctrl->cur.val = fault;
}
-out:
- mutex_unlock(&flash->lock);
return rval;
}
@@ -190,8 +186,6 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
u8 tout_bits;
int rval = -EINVAL;
- mutex_lock(&flash->lock);
-
switch (ctrl->id) {
case V4L2_CID_FLASH_LED_MODE:
flash->led_mode = ctrl->val;
@@ -202,14 +196,12 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
case V4L2_CID_FLASH_STROBE_SOURCE:
rval = regmap_update_bits(flash->regmap,
REG_CONFIG1, 0x04, (ctrl->val) << 2);
- if (rval < 0)
- goto err_out;
break;
case V4L2_CID_FLASH_STROBE:
if (flash->led_mode != V4L2_FLASH_LED_MODE_FLASH) {
rval = -EBUSY;
- goto err_out;
+ break;
}
flash->led_mode = V4L2_FLASH_LED_MODE_FLASH;
rval = lm3560_mode_ctrl(flash);
@@ -218,7 +210,7 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
case V4L2_CID_FLASH_STROBE_STOP:
if (flash->led_mode != V4L2_FLASH_LED_MODE_FLASH) {
rval = -EBUSY;
- goto err_out;
+ break;
}
flash->led_mode = V4L2_FLASH_LED_MODE_NONE;
rval = lm3560_mode_ctrl(flash);
@@ -239,8 +231,6 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
break;
}
-err_out:
- mutex_unlock(&flash->lock);
return rval;
}
@@ -328,6 +318,8 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
if (fault != NULL)
fault->flags |= V4L2_CTRL_FLAG_VOLATILE;
+ hdl->lock = &flash->lock;
+
if (hdl->error)
return hdl->error;
@@ -363,6 +355,7 @@ static int lm3560_subdev_init(struct lm3560_flash *flash,
if (rval < 0)
goto err_out;
flash->subdev_led[led_no].entity.function = MEDIA_ENT_F_FLASH;
+ flash->subdev_led[led_no].state_lock = &flash->lock;
rval = v4l2_async_register_subdev(&flash->subdev_led[led_no]);
if (rval < 0) {
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 3/6] media: i2c: lm3560: Optimize mutex lock usage
2026-05-03 16:44 ` [PATCH v5 3/6] media: i2c: lm3560: Optimize mutex lock usage Svyatoslav Ryhel
@ 2026-05-04 6:26 ` Sakari Ailus
2026-05-04 7:37 ` Svyatoslav Ryhel
0 siblings, 1 reply; 20+ messages in thread
From: Sakari Ailus @ 2026-05-04 6:26 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Mauro Carvalho Chehab, linux-leds, devicetree,
linux-kernel, linux-media
Hi Svyatoslav,
On Sun, May 03, 2026 at 07:44:42PM +0300, Svyatoslav Ryhel wrote:
> Pass the device's own mutex lock to the control handler so that the media
> framework can handle control access instead of managing it manually. The
> lock must be common to both sub-devices since they share same hardware,
> so the individual sub-device locks will not work here.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
> drivers/media/i2c/lm3560.c | 19 ++++++-------------
> 1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
> index edfb07587cab..5b568ed9536b 100644
> --- a/drivers/media/i2c/lm3560.c
> +++ b/drivers/media/i2c/lm3560.c
> @@ -162,14 +162,12 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> struct lm3560_flash *flash = to_lm3560_flash(ctrl, led_no);
> int rval = -EINVAL;
>
> - mutex_lock(&flash->lock);
> -
> if (ctrl->id == V4L2_CID_FLASH_FAULT) {
> s32 fault = 0;
> unsigned int reg_val;
> rval = regmap_read(flash->regmap, REG_FLAG, ®_val);
> if (rval < 0)
> - goto out;
> + return rval;
> if (reg_val & FAULT_SHORT_CIRCUIT)
> fault |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
> if (reg_val & FAULT_OVERTEMP)
> @@ -179,8 +177,6 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> ctrl->cur.val = fault;
> }
>
> -out:
> - mutex_unlock(&flash->lock);
> return rval;
> }
>
> @@ -190,8 +186,6 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> u8 tout_bits;
> int rval = -EINVAL;
>
> - mutex_lock(&flash->lock);
> -
> switch (ctrl->id) {
> case V4L2_CID_FLASH_LED_MODE:
> flash->led_mode = ctrl->val;
> @@ -202,14 +196,12 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> case V4L2_CID_FLASH_STROBE_SOURCE:
> rval = regmap_update_bits(flash->regmap,
> REG_CONFIG1, 0x04, (ctrl->val) << 2);
> - if (rval < 0)
> - goto err_out;
> break;
>
> case V4L2_CID_FLASH_STROBE:
> if (flash->led_mode != V4L2_FLASH_LED_MODE_FLASH) {
> rval = -EBUSY;
> - goto err_out;
> + break;
> }
> flash->led_mode = V4L2_FLASH_LED_MODE_FLASH;
> rval = lm3560_mode_ctrl(flash);
> @@ -218,7 +210,7 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> case V4L2_CID_FLASH_STROBE_STOP:
> if (flash->led_mode != V4L2_FLASH_LED_MODE_FLASH) {
> rval = -EBUSY;
> - goto err_out;
> + break;
> }
> flash->led_mode = V4L2_FLASH_LED_MODE_NONE;
> rval = lm3560_mode_ctrl(flash);
> @@ -239,8 +231,6 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> break;
> }
>
> -err_out:
> - mutex_unlock(&flash->lock);
> return rval;
> }
>
> @@ -328,6 +318,8 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
> if (fault != NULL)
> fault->flags |= V4L2_CTRL_FLAG_VOLATILE;
>
> + hdl->lock = &flash->lock;
> +
> if (hdl->error)
> return hdl->error;
>
> @@ -363,6 +355,7 @@ static int lm3560_subdev_init(struct lm3560_flash *flash,
> if (rval < 0)
> goto err_out;
> flash->subdev_led[led_no].entity.function = MEDIA_ENT_F_FLASH;
> + flash->subdev_led[led_no].state_lock = &flash->lock;
I must have missed it earlier but you can use the control handler's mutex
here. As a result, I believe you can drop the driver's own mutex
altogether.
>
> rval = v4l2_async_register_subdev(&flash->subdev_led[led_no]);
> if (rval < 0) {
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 3/6] media: i2c: lm3560: Optimize mutex lock usage
2026-05-04 6:26 ` Sakari Ailus
@ 2026-05-04 7:37 ` Svyatoslav Ryhel
2026-05-04 7:56 ` Sakari Ailus
0 siblings, 1 reply; 20+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-04 7:37 UTC (permalink / raw)
To: Sakari Ailus
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Mauro Carvalho Chehab, linux-leds, devicetree,
linux-kernel, linux-media
пн, 4 трав. 2026 р. о 09:26 Sakari Ailus <sakari.ailus@linux.intel.com> пише:
>
> Hi Svyatoslav,
>
> On Sun, May 03, 2026 at 07:44:42PM +0300, Svyatoslav Ryhel wrote:
> > Pass the device's own mutex lock to the control handler so that the media
> > framework can handle control access instead of managing it manually. The
> > lock must be common to both sub-devices since they share same hardware,
> > so the individual sub-device locks will not work here.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> > drivers/media/i2c/lm3560.c | 19 ++++++-------------
> > 1 file changed, 6 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
> > index edfb07587cab..5b568ed9536b 100644
> > --- a/drivers/media/i2c/lm3560.c
> > +++ b/drivers/media/i2c/lm3560.c
> > @@ -162,14 +162,12 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > struct lm3560_flash *flash = to_lm3560_flash(ctrl, led_no);
> > int rval = -EINVAL;
> >
> > - mutex_lock(&flash->lock);
> > -
> > if (ctrl->id == V4L2_CID_FLASH_FAULT) {
> > s32 fault = 0;
> > unsigned int reg_val;
> > rval = regmap_read(flash->regmap, REG_FLAG, ®_val);
> > if (rval < 0)
> > - goto out;
> > + return rval;
> > if (reg_val & FAULT_SHORT_CIRCUIT)
> > fault |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
> > if (reg_val & FAULT_OVERTEMP)
> > @@ -179,8 +177,6 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > ctrl->cur.val = fault;
> > }
> >
> > -out:
> > - mutex_unlock(&flash->lock);
> > return rval;
> > }
> >
> > @@ -190,8 +186,6 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > u8 tout_bits;
> > int rval = -EINVAL;
> >
> > - mutex_lock(&flash->lock);
> > -
> > switch (ctrl->id) {
> > case V4L2_CID_FLASH_LED_MODE:
> > flash->led_mode = ctrl->val;
> > @@ -202,14 +196,12 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > case V4L2_CID_FLASH_STROBE_SOURCE:
> > rval = regmap_update_bits(flash->regmap,
> > REG_CONFIG1, 0x04, (ctrl->val) << 2);
> > - if (rval < 0)
> > - goto err_out;
> > break;
> >
> > case V4L2_CID_FLASH_STROBE:
> > if (flash->led_mode != V4L2_FLASH_LED_MODE_FLASH) {
> > rval = -EBUSY;
> > - goto err_out;
> > + break;
> > }
> > flash->led_mode = V4L2_FLASH_LED_MODE_FLASH;
> > rval = lm3560_mode_ctrl(flash);
> > @@ -218,7 +210,7 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > case V4L2_CID_FLASH_STROBE_STOP:
> > if (flash->led_mode != V4L2_FLASH_LED_MODE_FLASH) {
> > rval = -EBUSY;
> > - goto err_out;
> > + break;
> > }
> > flash->led_mode = V4L2_FLASH_LED_MODE_NONE;
> > rval = lm3560_mode_ctrl(flash);
> > @@ -239,8 +231,6 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > break;
> > }
> >
> > -err_out:
> > - mutex_unlock(&flash->lock);
> > return rval;
> > }
> >
> > @@ -328,6 +318,8 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
> > if (fault != NULL)
> > fault->flags |= V4L2_CTRL_FLAG_VOLATILE;
> >
> > + hdl->lock = &flash->lock;
> > +
> > if (hdl->error)
> > return hdl->error;
> >
> > @@ -363,6 +355,7 @@ static int lm3560_subdev_init(struct lm3560_flash *flash,
> > if (rval < 0)
> > goto err_out;
> > flash->subdev_led[led_no].entity.function = MEDIA_ENT_F_FLASH;
> > + flash->subdev_led[led_no].state_lock = &flash->lock;
>
> I must have missed it earlier but you can use the control handler's mutex
> here. As a result, I believe you can drop the driver's own mutex
> altogether.
>
Control handler mutexes are per device, but both devices share the
same hardware so those mutexes will not prevent simultaneous access
from both devices. For this reason driver's own mutex is used.
> >
> > rval = v4l2_async_register_subdev(&flash->subdev_led[led_no]);
> > if (rval < 0) {
>
> --
> Kind regards,
>
> Sakari Ailus
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 3/6] media: i2c: lm3560: Optimize mutex lock usage
2026-05-04 7:37 ` Svyatoslav Ryhel
@ 2026-05-04 7:56 ` Sakari Ailus
0 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2026-05-04 7:56 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Mauro Carvalho Chehab, linux-leds, devicetree,
linux-kernel, linux-media
Hi Svyatoslav,
On Mon, May 04, 2026 at 10:37:40AM +0300, Svyatoslav Ryhel wrote:
> пн, 4 трав. 2026 р. о 09:26 Sakari Ailus <sakari.ailus@linux.intel.com> пише:
> >
> > Hi Svyatoslav,
> >
> > On Sun, May 03, 2026 at 07:44:42PM +0300, Svyatoslav Ryhel wrote:
> > > Pass the device's own mutex lock to the control handler so that the media
> > > framework can handle control access instead of managing it manually. The
> > > lock must be common to both sub-devices since they share same hardware,
> > > so the individual sub-device locks will not work here.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > > drivers/media/i2c/lm3560.c | 19 ++++++-------------
> > > 1 file changed, 6 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
> > > index edfb07587cab..5b568ed9536b 100644
> > > --- a/drivers/media/i2c/lm3560.c
> > > +++ b/drivers/media/i2c/lm3560.c
> > > @@ -162,14 +162,12 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > > struct lm3560_flash *flash = to_lm3560_flash(ctrl, led_no);
> > > int rval = -EINVAL;
> > >
> > > - mutex_lock(&flash->lock);
> > > -
> > > if (ctrl->id == V4L2_CID_FLASH_FAULT) {
> > > s32 fault = 0;
> > > unsigned int reg_val;
> > > rval = regmap_read(flash->regmap, REG_FLAG, ®_val);
> > > if (rval < 0)
> > > - goto out;
> > > + return rval;
> > > if (reg_val & FAULT_SHORT_CIRCUIT)
> > > fault |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
> > > if (reg_val & FAULT_OVERTEMP)
> > > @@ -179,8 +177,6 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > > ctrl->cur.val = fault;
> > > }
> > >
> > > -out:
> > > - mutex_unlock(&flash->lock);
> > > return rval;
> > > }
> > >
> > > @@ -190,8 +186,6 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > > u8 tout_bits;
> > > int rval = -EINVAL;
> > >
> > > - mutex_lock(&flash->lock);
> > > -
> > > switch (ctrl->id) {
> > > case V4L2_CID_FLASH_LED_MODE:
> > > flash->led_mode = ctrl->val;
> > > @@ -202,14 +196,12 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > > case V4L2_CID_FLASH_STROBE_SOURCE:
> > > rval = regmap_update_bits(flash->regmap,
> > > REG_CONFIG1, 0x04, (ctrl->val) << 2);
> > > - if (rval < 0)
> > > - goto err_out;
> > > break;
> > >
> > > case V4L2_CID_FLASH_STROBE:
> > > if (flash->led_mode != V4L2_FLASH_LED_MODE_FLASH) {
> > > rval = -EBUSY;
> > > - goto err_out;
> > > + break;
> > > }
> > > flash->led_mode = V4L2_FLASH_LED_MODE_FLASH;
> > > rval = lm3560_mode_ctrl(flash);
> > > @@ -218,7 +210,7 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > > case V4L2_CID_FLASH_STROBE_STOP:
> > > if (flash->led_mode != V4L2_FLASH_LED_MODE_FLASH) {
> > > rval = -EBUSY;
> > > - goto err_out;
> > > + break;
> > > }
> > > flash->led_mode = V4L2_FLASH_LED_MODE_NONE;
> > > rval = lm3560_mode_ctrl(flash);
> > > @@ -239,8 +231,6 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > > break;
> > > }
> > >
> > > -err_out:
> > > - mutex_unlock(&flash->lock);
> > > return rval;
> > > }
> > >
> > > @@ -328,6 +318,8 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
> > > if (fault != NULL)
> > > fault->flags |= V4L2_CTRL_FLAG_VOLATILE;
> > >
> > > + hdl->lock = &flash->lock;
> > > +
> > > if (hdl->error)
> > > return hdl->error;
> > >
> > > @@ -363,6 +355,7 @@ static int lm3560_subdev_init(struct lm3560_flash *flash,
> > > if (rval < 0)
> > > goto err_out;
> > > flash->subdev_led[led_no].entity.function = MEDIA_ENT_F_FLASH;
> > > + flash->subdev_led[led_no].state_lock = &flash->lock;
> >
> > I must have missed it earlier but you can use the control handler's mutex
> > here. As a result, I believe you can drop the driver's own mutex
> > altogether.
> >
>
> Control handler mutexes are per device, but both devices share the
> same hardware so those mutexes will not prevent simultaneous access
> from both devices. For this reason driver's own mutex is used.
Right. You could still use one for the other handler.
Feel free to keep it as-is, too.
>
> > >
> > > rval = v4l2_async_register_subdev(&flash->subdev_led[led_no]);
> > > if (rval < 0) {
> >
--
Sakari Ailus
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 4/6] media: i2c: lm3560: Convert to use OF bindings
2026-05-03 16:44 [PATCH v5 0/6] media: lm3560: convert to use OF bindings Svyatoslav Ryhel
` (2 preceding siblings ...)
2026-05-03 16:44 ` [PATCH v5 3/6] media: i2c: lm3560: Optimize mutex lock usage Svyatoslav Ryhel
@ 2026-05-03 16:44 ` Svyatoslav Ryhel
2026-05-04 6:36 ` Sakari Ailus
2026-05-03 16:44 ` [PATCH v5 5/6] media: i2c: lm3560: Add support for PM features Svyatoslav Ryhel
` (2 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-03 16:44 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sakari Ailus, Mauro Carvalho Chehab,
Svyatoslav Ryhel
Cc: linux-leds, devicetree, linux-kernel, linux-media
Since there are no users of this driver via platform data, remove the
platform data support and switch to using Device Tree bindings.
Converting to Device Tree assumes dynamic and independent registration of
LEDs. To monitor the configured LEDs, a bitmap has been added. This makes
LED cleanup more robust and less context dependent.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/media/i2c/lm3560.c | 143 ++++++++++++++++++++++++++-----------
include/media/i2c/lm3560.h | 15 ----
2 files changed, 102 insertions(+), 56 deletions(-)
diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
index 5b568ed9536b..ce4b09d1f208 100644
--- a/drivers/media/i2c/lm3560.c
+++ b/drivers/media/i2c/lm3560.c
@@ -9,11 +9,15 @@
* Ldd-Mlp <ldd-mlp@list.ti.com>
*/
+#include <linux/bitmap.h>
#include <linux/delay.h>
#include <linux/module.h>
#include <linux/i2c.h>
#include <linux/slab.h>
+#include <linux/mod_devicetable.h>
#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/videodev2.h>
#include <media/i2c/lm3560.h>
@@ -43,22 +47,33 @@ enum led_enable {
* struct lm3560_flash
*
* @dev: pointer to &struct device
- * @pdata: platform data
* @regmap: reg. map for i2c
* @lock: muxtex for serial access.
* @led_mode: V4L2 LED mode
* @ctrls_led: V4L2 controls
* @subdev_led: V4L2 subdev
+ * @led_id: LED status holder
+ * @peak: peak current
+ * @max_flash_timeout: flash timeout
+ * @max_flash_brt: flash mode led brightness
+ * @max_torch_brt: torch mode led brightness
*/
struct lm3560_flash {
struct device *dev;
- struct lm3560_platform_data *pdata;
struct regmap *regmap;
struct mutex lock;
enum v4l2_flash_led_mode led_mode;
struct v4l2_ctrl_handler ctrls_led[LM3560_LED_MAX];
struct v4l2_subdev subdev_led[LM3560_LED_MAX];
+
+ DECLARE_BITMAP(led_id, LM3560_LED_MAX);
+
+ enum lm3560_peak_current peak;
+ u32 max_flash_timeout;
+
+ u32 max_flash_brt[LM3560_LED_MAX];
+ u32 max_torch_brt[LM3560_LED_MAX];
};
#define to_lm3560_flash(_ctrl, _no) \
@@ -269,8 +284,8 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
enum lm3560_led_id led_no)
{
struct v4l2_ctrl *fault;
- u32 max_flash_brt = flash->pdata->max_flash_brt[led_no];
- u32 max_torch_brt = flash->pdata->max_torch_brt[led_no];
+ u32 max_flash_brt = flash->max_flash_brt[led_no];
+ u32 max_torch_brt = flash->max_torch_brt[led_no];
struct v4l2_ctrl_handler *hdl = &flash->ctrls_led[led_no];
const struct v4l2_ctrl_ops *ops = &lm3560_led_ctrl_ops[led_no];
@@ -295,9 +310,9 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
/* flash strobe timeout */
v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_TIMEOUT,
LM3560_FLASH_TOUT_MIN,
- flash->pdata->max_flash_timeout,
+ flash->max_flash_timeout,
LM3560_FLASH_TOUT_STEP,
- flash->pdata->max_flash_timeout);
+ flash->max_flash_timeout);
/* flash brt */
v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_INTENSITY,
@@ -339,15 +354,18 @@ static const struct regmap_config lm3560_regmap = {
};
static int lm3560_subdev_init(struct lm3560_flash *flash,
- enum lm3560_led_id led_no, char *led_name)
+ enum lm3560_led_id led_no,
+ struct device_node *np)
{
struct i2c_client *client = to_i2c_client(flash->dev);
int rval;
v4l2_i2c_subdev_init(&flash->subdev_led[led_no], client, &lm3560_ops);
flash->subdev_led[led_no].flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
- strscpy(flash->subdev_led[led_no].name, led_name,
- sizeof(flash->subdev_led[led_no].name));
+ snprintf(flash->subdev_led[led_no].name,
+ sizeof(flash->subdev_led[led_no].name),
+ "lm3560-led%d", led_no);
+ flash->subdev_led[led_no].fwnode = of_fwnode_handle(np);
rval = lm3560_init_controls(flash, led_no);
if (rval)
goto err_out;
@@ -378,7 +396,7 @@ static int lm3560_init_device(struct lm3560_flash *flash)
/* set peak current */
rval = regmap_update_bits(flash->regmap,
- REG_FLASH_TOUT, 0x60, flash->pdata->peak);
+ REG_FLASH_TOUT, 0x60, flash->peak);
if (rval < 0)
return rval;
/* output disable */
@@ -391,18 +409,22 @@ static int lm3560_init_device(struct lm3560_flash *flash)
return rval;
}
-static void lm3560_subdev_cleanup(struct lm3560_flash *flash,
- enum lm3560_led_id led_no)
+static void lm3560_subdev_cleanup(struct lm3560_flash *flash)
{
- v4l2_async_unregister_subdev(&flash->subdev_led[led_no]);
- v4l2_ctrl_handler_free(&flash->ctrls_led[led_no]);
- media_entity_cleanup(&flash->subdev_led[led_no].entity);
+ int led_no;
+
+ for_each_set_bit(led_no, flash->led_id, LM3560_LED_MAX) {
+ v4l2_async_unregister_subdev(&flash->subdev_led[led_no]);
+ v4l2_ctrl_handler_free(&flash->ctrls_led[led_no]);
+ media_entity_cleanup(&flash->subdev_led[led_no].entity);
+ }
}
static int lm3560_probe(struct i2c_client *client)
{
struct lm3560_flash *flash;
- struct lm3560_platform_data *pdata = dev_get_platdata(&client->dev);
+ struct device_node *node;
+ u32 peak_ua;
int rval;
flash = devm_kzalloc(&client->dev, sizeof(*flash), GFP_KERNEL);
@@ -415,36 +437,69 @@ static int lm3560_probe(struct i2c_client *client)
return rval;
}
- /* if there is no platform data, use chip default value */
- if (pdata == NULL) {
- pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
- if (pdata == NULL)
- return -ENODEV;
- pdata->peak = LM3560_PEAK_3600mA;
- pdata->max_flash_timeout = LM3560_FLASH_TOUT_MAX;
- /* led 1 */
- pdata->max_flash_brt[LM3560_LED0] = LM3560_FLASH_BRT_MAX;
- pdata->max_torch_brt[LM3560_LED0] = LM3560_TORCH_BRT_MAX;
- /* led 2 */
- pdata->max_flash_brt[LM3560_LED1] = LM3560_FLASH_BRT_MAX;
- pdata->max_torch_brt[LM3560_LED1] = LM3560_TORCH_BRT_MAX;
- }
- flash->pdata = pdata;
flash->dev = &client->dev;
mutex_init(&flash->lock);
+ bitmap_zero(flash->led_id, LM3560_LED_MAX);
+
+ flash->peak = LM3560_PEAK_1600mA;
+ rval = device_property_read_u32(flash->dev,
+ "ti,peak-current-microamp", &peak_ua);
+ if (!rval) {
+ switch (peak_ua) {
+ case 1600000:
+ flash->peak = LM3560_PEAK_1600mA;
+ break;
+ case 2300000:
+ flash->peak = LM3560_PEAK_2300mA;
+ break;
+ case 3000000:
+ flash->peak = LM3560_PEAK_3000mA;
+ break;
+ case 3600000:
+ flash->peak = LM3560_PEAK_3600mA;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ flash->max_flash_timeout = LM3560_FLASH_TOUT_MIN * 1000;
+ device_property_read_u32(flash->dev, "flash-max-timeout-us",
+ &flash->max_flash_timeout);
+ flash->max_flash_timeout /= 1000;
+
rval = lm3560_init_device(flash);
if (rval < 0)
return rval;
- rval = lm3560_subdev_init(flash, LM3560_LED0, "lm3560-led0");
- if (rval < 0)
- return rval;
+ for_each_available_child_of_node(dev_of_node(flash->dev), node) {
+ u32 reg;
- rval = lm3560_subdev_init(flash, LM3560_LED1, "lm3560-led1");
- if (rval < 0) {
- lm3560_subdev_cleanup(flash, LM3560_LED0);
- return rval;
+ rval = of_property_read_u32(node, "reg", ®);
+ if (rval < 0)
+ /* We care only about nodes with reg property */
+ continue;
+
+ if (reg == LM3560_LED0 || reg == LM3560_LED1) {
+ flash->max_flash_brt[reg] = LM3560_FLASH_BRT_MIN;
+ of_property_read_u32(node, "flash-max-microamp",
+ &flash->max_flash_brt[reg]);
+
+ flash->max_torch_brt[reg] = LM3560_TORCH_BRT_MIN;
+ of_property_read_u32(node, "led-max-microamp",
+ &flash->max_torch_brt[reg]);
+
+ rval = lm3560_subdev_init(flash, reg, node);
+ if (rval < 0) {
+ lm3560_subdev_cleanup(flash);
+ return dev_err_probe(flash->dev, rval,
+ "failed to register led%d\n",
+ reg);
+ }
+
+ set_bit(reg, flash->led_id);
+ }
}
i2c_set_clientdata(client, flash);
@@ -455,12 +510,17 @@ static int lm3560_probe(struct i2c_client *client)
static void lm3560_remove(struct i2c_client *client)
{
struct lm3560_flash *flash = i2c_get_clientdata(client);
- unsigned int i;
- for (i = LM3560_LED0; i < LM3560_LED_MAX; i++)
- lm3560_subdev_cleanup(flash, i);
+ lm3560_subdev_cleanup(flash);
}
+static const struct of_device_id lm3560_of_match[] = {
+ { .compatible = "ti,lm3559" },
+ { .compatible = "ti,lm3560" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, lm3560_of_match);
+
static const struct i2c_device_id lm3560_id_table[] = {
{ LM3559_NAME },
{ LM3560_NAME },
@@ -473,6 +533,7 @@ static struct i2c_driver lm3560_i2c_driver = {
.driver = {
.name = LM3560_NAME,
.pm = NULL,
+ .of_match_table = lm3560_of_match,
},
.probe = lm3560_probe,
.remove = lm3560_remove,
diff --git a/include/media/i2c/lm3560.h b/include/media/i2c/lm3560.h
index 770d8c72c94a..b56c1ff8fd49 100644
--- a/include/media/i2c/lm3560.h
+++ b/include/media/i2c/lm3560.h
@@ -66,19 +66,4 @@ enum lm3560_peak_current {
LM3560_PEAK_3600mA = 0x60
};
-/* struct lm3560_platform_data
- *
- * @peak : peak current
- * @max_flash_timeout: flash timeout
- * @max_flash_brt: flash mode led brightness
- * @max_torch_brt: torch mode led brightness
- */
-struct lm3560_platform_data {
- enum lm3560_peak_current peak;
-
- u32 max_flash_timeout;
- u32 max_flash_brt[LM3560_LED_MAX];
- u32 max_torch_brt[LM3560_LED_MAX];
-};
-
#endif /* __LM3560_H__ */
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 4/6] media: i2c: lm3560: Convert to use OF bindings
2026-05-03 16:44 ` [PATCH v5 4/6] media: i2c: lm3560: Convert to use OF bindings Svyatoslav Ryhel
@ 2026-05-04 6:36 ` Sakari Ailus
2026-05-04 7:40 ` Svyatoslav Ryhel
0 siblings, 1 reply; 20+ messages in thread
From: Sakari Ailus @ 2026-05-04 6:36 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Mauro Carvalho Chehab, linux-leds, devicetree,
linux-kernel, linux-media
Hi Svyatoslav,
On Sun, May 03, 2026 at 07:44:43PM +0300, Svyatoslav Ryhel wrote:
> Since there are no users of this driver via platform data, remove the
> platform data support and switch to using Device Tree bindings.
>
> Converting to Device Tree assumes dynamic and independent registration of
> LEDs. To monitor the configured LEDs, a bitmap has been added. This makes
> LED cleanup more robust and less context dependent.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
> drivers/media/i2c/lm3560.c | 143 ++++++++++++++++++++++++++-----------
> include/media/i2c/lm3560.h | 15 ----
> 2 files changed, 102 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
> index 5b568ed9536b..ce4b09d1f208 100644
> --- a/drivers/media/i2c/lm3560.c
> +++ b/drivers/media/i2c/lm3560.c
> @@ -9,11 +9,15 @@
> * Ldd-Mlp <ldd-mlp@list.ti.com>
> */
>
> +#include <linux/bitmap.h>
> #include <linux/delay.h>
> #include <linux/module.h>
> #include <linux/i2c.h>
> #include <linux/slab.h>
> +#include <linux/mod_devicetable.h>
> #include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/property.h>
> #include <linux/regmap.h>
> #include <linux/videodev2.h>
> #include <media/i2c/lm3560.h>
> @@ -43,22 +47,33 @@ enum led_enable {
> * struct lm3560_flash
> *
> * @dev: pointer to &struct device
> - * @pdata: platform data
> * @regmap: reg. map for i2c
> * @lock: muxtex for serial access.
> * @led_mode: V4L2 LED mode
> * @ctrls_led: V4L2 controls
> * @subdev_led: V4L2 subdev
> + * @led_id: LED status holder
> + * @peak: peak current
> + * @max_flash_timeout: flash timeout
> + * @max_flash_brt: flash mode led brightness
> + * @max_torch_brt: torch mode led brightness
> */
> struct lm3560_flash {
> struct device *dev;
> - struct lm3560_platform_data *pdata;
> struct regmap *regmap;
> struct mutex lock;
>
> enum v4l2_flash_led_mode led_mode;
> struct v4l2_ctrl_handler ctrls_led[LM3560_LED_MAX];
> struct v4l2_subdev subdev_led[LM3560_LED_MAX];
> +
> + DECLARE_BITMAP(led_id, LM3560_LED_MAX);
> +
> + enum lm3560_peak_current peak;
> + u32 max_flash_timeout;
> +
> + u32 max_flash_brt[LM3560_LED_MAX];
> + u32 max_torch_brt[LM3560_LED_MAX];
> };
>
> #define to_lm3560_flash(_ctrl, _no) \
> @@ -269,8 +284,8 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
> enum lm3560_led_id led_no)
> {
> struct v4l2_ctrl *fault;
> - u32 max_flash_brt = flash->pdata->max_flash_brt[led_no];
> - u32 max_torch_brt = flash->pdata->max_torch_brt[led_no];
> + u32 max_flash_brt = flash->max_flash_brt[led_no];
> + u32 max_torch_brt = flash->max_torch_brt[led_no];
> struct v4l2_ctrl_handler *hdl = &flash->ctrls_led[led_no];
> const struct v4l2_ctrl_ops *ops = &lm3560_led_ctrl_ops[led_no];
>
> @@ -295,9 +310,9 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
> /* flash strobe timeout */
> v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_TIMEOUT,
> LM3560_FLASH_TOUT_MIN,
> - flash->pdata->max_flash_timeout,
> + flash->max_flash_timeout,
> LM3560_FLASH_TOUT_STEP,
> - flash->pdata->max_flash_timeout);
> + flash->max_flash_timeout);
>
> /* flash brt */
> v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_INTENSITY,
> @@ -339,15 +354,18 @@ static const struct regmap_config lm3560_regmap = {
> };
>
> static int lm3560_subdev_init(struct lm3560_flash *flash,
> - enum lm3560_led_id led_no, char *led_name)
> + enum lm3560_led_id led_no,
> + struct device_node *np)
> {
> struct i2c_client *client = to_i2c_client(flash->dev);
> int rval;
>
> v4l2_i2c_subdev_init(&flash->subdev_led[led_no], client, &lm3560_ops);
> flash->subdev_led[led_no].flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> - strscpy(flash->subdev_led[led_no].name, led_name,
> - sizeof(flash->subdev_led[led_no].name));
> + snprintf(flash->subdev_led[led_no].name,
> + sizeof(flash->subdev_led[led_no].name),
> + "lm3560-led%d", led_no);
> + flash->subdev_led[led_no].fwnode = of_fwnode_handle(np);
> rval = lm3560_init_controls(flash, led_no);
> if (rval)
> goto err_out;
> @@ -378,7 +396,7 @@ static int lm3560_init_device(struct lm3560_flash *flash)
>
> /* set peak current */
> rval = regmap_update_bits(flash->regmap,
> - REG_FLASH_TOUT, 0x60, flash->pdata->peak);
> + REG_FLASH_TOUT, 0x60, flash->peak);
> if (rval < 0)
> return rval;
> /* output disable */
> @@ -391,18 +409,22 @@ static int lm3560_init_device(struct lm3560_flash *flash)
> return rval;
> }
>
> -static void lm3560_subdev_cleanup(struct lm3560_flash *flash,
> - enum lm3560_led_id led_no)
> +static void lm3560_subdev_cleanup(struct lm3560_flash *flash)
> {
> - v4l2_async_unregister_subdev(&flash->subdev_led[led_no]);
> - v4l2_ctrl_handler_free(&flash->ctrls_led[led_no]);
> - media_entity_cleanup(&flash->subdev_led[led_no].entity);
> + int led_no;
> +
> + for_each_set_bit(led_no, flash->led_id, LM3560_LED_MAX) {
> + v4l2_async_unregister_subdev(&flash->subdev_led[led_no]);
> + v4l2_ctrl_handler_free(&flash->ctrls_led[led_no]);
> + media_entity_cleanup(&flash->subdev_led[led_no].entity);
> + }
> }
>
> static int lm3560_probe(struct i2c_client *client)
> {
> struct lm3560_flash *flash;
> - struct lm3560_platform_data *pdata = dev_get_platdata(&client->dev);
> + struct device_node *node;
> + u32 peak_ua;
> int rval;
>
> flash = devm_kzalloc(&client->dev, sizeof(*flash), GFP_KERNEL);
> @@ -415,36 +437,69 @@ static int lm3560_probe(struct i2c_client *client)
> return rval;
> }
>
> - /* if there is no platform data, use chip default value */
> - if (pdata == NULL) {
> - pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
> - if (pdata == NULL)
> - return -ENODEV;
> - pdata->peak = LM3560_PEAK_3600mA;
> - pdata->max_flash_timeout = LM3560_FLASH_TOUT_MAX;
> - /* led 1 */
> - pdata->max_flash_brt[LM3560_LED0] = LM3560_FLASH_BRT_MAX;
> - pdata->max_torch_brt[LM3560_LED0] = LM3560_TORCH_BRT_MAX;
> - /* led 2 */
> - pdata->max_flash_brt[LM3560_LED1] = LM3560_FLASH_BRT_MAX;
> - pdata->max_torch_brt[LM3560_LED1] = LM3560_TORCH_BRT_MAX;
> - }
> - flash->pdata = pdata;
> flash->dev = &client->dev;
> mutex_init(&flash->lock);
>
> + bitmap_zero(flash->led_id, LM3560_LED_MAX);
> +
> + flash->peak = LM3560_PEAK_1600mA;
> + rval = device_property_read_u32(flash->dev,
> + "ti,peak-current-microamp", &peak_ua);
> + if (!rval) {
> + switch (peak_ua) {
> + case 1600000:
> + flash->peak = LM3560_PEAK_1600mA;
> + break;
> + case 2300000:
> + flash->peak = LM3560_PEAK_2300mA;
> + break;
> + case 3000000:
> + flash->peak = LM3560_PEAK_3000mA;
> + break;
> + case 3600000:
> + flash->peak = LM3560_PEAK_3600mA;
> + break;
> + default:
> + return -EINVAL;
> + }
> + }
> +
> + flash->max_flash_timeout = LM3560_FLASH_TOUT_MIN * 1000;
> + device_property_read_u32(flash->dev, "flash-max-timeout-us",
> + &flash->max_flash_timeout);
> + flash->max_flash_timeout /= 1000;
> +
> rval = lm3560_init_device(flash);
> if (rval < 0)
> return rval;
>
> - rval = lm3560_subdev_init(flash, LM3560_LED0, "lm3560-led0");
> - if (rval < 0)
> - return rval;
> + for_each_available_child_of_node(dev_of_node(flash->dev), node) {
device_for_each_child_node(), please.
> + u32 reg;
>
> - rval = lm3560_subdev_init(flash, LM3560_LED1, "lm3560-led1");
> - if (rval < 0) {
> - lm3560_subdev_cleanup(flash, LM3560_LED0);
> - return rval;
> + rval = of_property_read_u32(node, "reg", ®);
device_property_read_u32() here and elsewhere.
> + if (rval < 0)
> + /* We care only about nodes with reg property */
> + continue;
> +
> + if (reg == LM3560_LED0 || reg == LM3560_LED1) {
> + flash->max_flash_brt[reg] = LM3560_FLASH_BRT_MIN;
> + of_property_read_u32(node, "flash-max-microamp",
> + &flash->max_flash_brt[reg]);
> +
> + flash->max_torch_brt[reg] = LM3560_TORCH_BRT_MIN;
> + of_property_read_u32(node, "led-max-microamp",
> + &flash->max_torch_brt[reg]);
> +
> + rval = lm3560_subdev_init(flash, reg, node);
> + if (rval < 0) {
> + lm3560_subdev_cleanup(flash);
> + return dev_err_probe(flash->dev, rval,
> + "failed to register led%d\n",
> + reg);
> + }
> +
> + set_bit(reg, flash->led_id);
> + }
> }
>
> i2c_set_clientdata(client, flash);
> @@ -455,12 +510,17 @@ static int lm3560_probe(struct i2c_client *client)
> static void lm3560_remove(struct i2c_client *client)
> {
> struct lm3560_flash *flash = i2c_get_clientdata(client);
> - unsigned int i;
>
> - for (i = LM3560_LED0; i < LM3560_LED_MAX; i++)
> - lm3560_subdev_cleanup(flash, i);
> + lm3560_subdev_cleanup(flash);
> }
>
> +static const struct of_device_id lm3560_of_match[] = {
> + { .compatible = "ti,lm3559" },
> + { .compatible = "ti,lm3560" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, lm3560_of_match);
> +
> static const struct i2c_device_id lm3560_id_table[] = {
> { LM3559_NAME },
> { LM3560_NAME },
> @@ -473,6 +533,7 @@ static struct i2c_driver lm3560_i2c_driver = {
> .driver = {
> .name = LM3560_NAME,
> .pm = NULL,
> + .of_match_table = lm3560_of_match,
> },
> .probe = lm3560_probe,
> .remove = lm3560_remove,
> diff --git a/include/media/i2c/lm3560.h b/include/media/i2c/lm3560.h
> index 770d8c72c94a..b56c1ff8fd49 100644
> --- a/include/media/i2c/lm3560.h
> +++ b/include/media/i2c/lm3560.h
> @@ -66,19 +66,4 @@ enum lm3560_peak_current {
> LM3560_PEAK_3600mA = 0x60
> };
>
> -/* struct lm3560_platform_data
> - *
> - * @peak : peak current
> - * @max_flash_timeout: flash timeout
> - * @max_flash_brt: flash mode led brightness
> - * @max_torch_brt: torch mode led brightness
> - */
> -struct lm3560_platform_data {
> - enum lm3560_peak_current peak;
> -
> - u32 max_flash_timeout;
> - u32 max_flash_brt[LM3560_LED_MAX];
> - u32 max_torch_brt[LM3560_LED_MAX];
> -};
> -
> #endif /* __LM3560_H__ */
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 4/6] media: i2c: lm3560: Convert to use OF bindings
2026-05-04 6:36 ` Sakari Ailus
@ 2026-05-04 7:40 ` Svyatoslav Ryhel
2026-05-04 8:08 ` Sakari Ailus
0 siblings, 1 reply; 20+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-04 7:40 UTC (permalink / raw)
To: Sakari Ailus
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Mauro Carvalho Chehab, linux-leds, devicetree,
linux-kernel, linux-media
пн, 4 трав. 2026 р. о 09:36 Sakari Ailus <sakari.ailus@linux.intel.com> пише:
>
> Hi Svyatoslav,
>
> On Sun, May 03, 2026 at 07:44:43PM +0300, Svyatoslav Ryhel wrote:
> > Since there are no users of this driver via platform data, remove the
> > platform data support and switch to using Device Tree bindings.
> >
> > Converting to Device Tree assumes dynamic and independent registration of
> > LEDs. To monitor the configured LEDs, a bitmap has been added. This makes
> > LED cleanup more robust and less context dependent.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> > drivers/media/i2c/lm3560.c | 143 ++++++++++++++++++++++++++-----------
> > include/media/i2c/lm3560.h | 15 ----
> > 2 files changed, 102 insertions(+), 56 deletions(-)
> >
> > diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
> > index 5b568ed9536b..ce4b09d1f208 100644
> > --- a/drivers/media/i2c/lm3560.c
> > +++ b/drivers/media/i2c/lm3560.c
> > @@ -9,11 +9,15 @@
> > * Ldd-Mlp <ldd-mlp@list.ti.com>
> > */
> >
> > +#include <linux/bitmap.h>
> > #include <linux/delay.h>
> > #include <linux/module.h>
> > #include <linux/i2c.h>
> > #include <linux/slab.h>
> > +#include <linux/mod_devicetable.h>
> > #include <linux/mutex.h>
> > +#include <linux/of.h>
> > +#include <linux/property.h>
> > #include <linux/regmap.h>
> > #include <linux/videodev2.h>
> > #include <media/i2c/lm3560.h>
> > @@ -43,22 +47,33 @@ enum led_enable {
> > * struct lm3560_flash
> > *
> > * @dev: pointer to &struct device
> > - * @pdata: platform data
> > * @regmap: reg. map for i2c
> > * @lock: muxtex for serial access.
> > * @led_mode: V4L2 LED mode
> > * @ctrls_led: V4L2 controls
> > * @subdev_led: V4L2 subdev
> > + * @led_id: LED status holder
> > + * @peak: peak current
> > + * @max_flash_timeout: flash timeout
> > + * @max_flash_brt: flash mode led brightness
> > + * @max_torch_brt: torch mode led brightness
> > */
> > struct lm3560_flash {
> > struct device *dev;
> > - struct lm3560_platform_data *pdata;
> > struct regmap *regmap;
> > struct mutex lock;
> >
> > enum v4l2_flash_led_mode led_mode;
> > struct v4l2_ctrl_handler ctrls_led[LM3560_LED_MAX];
> > struct v4l2_subdev subdev_led[LM3560_LED_MAX];
> > +
> > + DECLARE_BITMAP(led_id, LM3560_LED_MAX);
> > +
> > + enum lm3560_peak_current peak;
> > + u32 max_flash_timeout;
> > +
> > + u32 max_flash_brt[LM3560_LED_MAX];
> > + u32 max_torch_brt[LM3560_LED_MAX];
> > };
> >
> > #define to_lm3560_flash(_ctrl, _no) \
> > @@ -269,8 +284,8 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
> > enum lm3560_led_id led_no)
> > {
> > struct v4l2_ctrl *fault;
> > - u32 max_flash_brt = flash->pdata->max_flash_brt[led_no];
> > - u32 max_torch_brt = flash->pdata->max_torch_brt[led_no];
> > + u32 max_flash_brt = flash->max_flash_brt[led_no];
> > + u32 max_torch_brt = flash->max_torch_brt[led_no];
> > struct v4l2_ctrl_handler *hdl = &flash->ctrls_led[led_no];
> > const struct v4l2_ctrl_ops *ops = &lm3560_led_ctrl_ops[led_no];
> >
> > @@ -295,9 +310,9 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
> > /* flash strobe timeout */
> > v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_TIMEOUT,
> > LM3560_FLASH_TOUT_MIN,
> > - flash->pdata->max_flash_timeout,
> > + flash->max_flash_timeout,
> > LM3560_FLASH_TOUT_STEP,
> > - flash->pdata->max_flash_timeout);
> > + flash->max_flash_timeout);
> >
> > /* flash brt */
> > v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_INTENSITY,
> > @@ -339,15 +354,18 @@ static const struct regmap_config lm3560_regmap = {
> > };
> >
> > static int lm3560_subdev_init(struct lm3560_flash *flash,
> > - enum lm3560_led_id led_no, char *led_name)
> > + enum lm3560_led_id led_no,
> > + struct device_node *np)
> > {
> > struct i2c_client *client = to_i2c_client(flash->dev);
> > int rval;
> >
> > v4l2_i2c_subdev_init(&flash->subdev_led[led_no], client, &lm3560_ops);
> > flash->subdev_led[led_no].flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > - strscpy(flash->subdev_led[led_no].name, led_name,
> > - sizeof(flash->subdev_led[led_no].name));
> > + snprintf(flash->subdev_led[led_no].name,
> > + sizeof(flash->subdev_led[led_no].name),
> > + "lm3560-led%d", led_no);
> > + flash->subdev_led[led_no].fwnode = of_fwnode_handle(np);
> > rval = lm3560_init_controls(flash, led_no);
> > if (rval)
> > goto err_out;
> > @@ -378,7 +396,7 @@ static int lm3560_init_device(struct lm3560_flash *flash)
> >
> > /* set peak current */
> > rval = regmap_update_bits(flash->regmap,
> > - REG_FLASH_TOUT, 0x60, flash->pdata->peak);
> > + REG_FLASH_TOUT, 0x60, flash->peak);
> > if (rval < 0)
> > return rval;
> > /* output disable */
> > @@ -391,18 +409,22 @@ static int lm3560_init_device(struct lm3560_flash *flash)
> > return rval;
> > }
> >
> > -static void lm3560_subdev_cleanup(struct lm3560_flash *flash,
> > - enum lm3560_led_id led_no)
> > +static void lm3560_subdev_cleanup(struct lm3560_flash *flash)
> > {
> > - v4l2_async_unregister_subdev(&flash->subdev_led[led_no]);
> > - v4l2_ctrl_handler_free(&flash->ctrls_led[led_no]);
> > - media_entity_cleanup(&flash->subdev_led[led_no].entity);
> > + int led_no;
> > +
> > + for_each_set_bit(led_no, flash->led_id, LM3560_LED_MAX) {
> > + v4l2_async_unregister_subdev(&flash->subdev_led[led_no]);
> > + v4l2_ctrl_handler_free(&flash->ctrls_led[led_no]);
> > + media_entity_cleanup(&flash->subdev_led[led_no].entity);
> > + }
> > }
> >
> > static int lm3560_probe(struct i2c_client *client)
> > {
> > struct lm3560_flash *flash;
> > - struct lm3560_platform_data *pdata = dev_get_platdata(&client->dev);
> > + struct device_node *node;
> > + u32 peak_ua;
> > int rval;
> >
> > flash = devm_kzalloc(&client->dev, sizeof(*flash), GFP_KERNEL);
> > @@ -415,36 +437,69 @@ static int lm3560_probe(struct i2c_client *client)
> > return rval;
> > }
> >
> > - /* if there is no platform data, use chip default value */
> > - if (pdata == NULL) {
> > - pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
> > - if (pdata == NULL)
> > - return -ENODEV;
> > - pdata->peak = LM3560_PEAK_3600mA;
> > - pdata->max_flash_timeout = LM3560_FLASH_TOUT_MAX;
> > - /* led 1 */
> > - pdata->max_flash_brt[LM3560_LED0] = LM3560_FLASH_BRT_MAX;
> > - pdata->max_torch_brt[LM3560_LED0] = LM3560_TORCH_BRT_MAX;
> > - /* led 2 */
> > - pdata->max_flash_brt[LM3560_LED1] = LM3560_FLASH_BRT_MAX;
> > - pdata->max_torch_brt[LM3560_LED1] = LM3560_TORCH_BRT_MAX;
> > - }
> > - flash->pdata = pdata;
> > flash->dev = &client->dev;
> > mutex_init(&flash->lock);
> >
> > + bitmap_zero(flash->led_id, LM3560_LED_MAX);
> > +
> > + flash->peak = LM3560_PEAK_1600mA;
> > + rval = device_property_read_u32(flash->dev,
> > + "ti,peak-current-microamp", &peak_ua);
> > + if (!rval) {
> > + switch (peak_ua) {
> > + case 1600000:
> > + flash->peak = LM3560_PEAK_1600mA;
> > + break;
> > + case 2300000:
> > + flash->peak = LM3560_PEAK_2300mA;
> > + break;
> > + case 3000000:
> > + flash->peak = LM3560_PEAK_3000mA;
> > + break;
> > + case 3600000:
> > + flash->peak = LM3560_PEAK_3600mA;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + flash->max_flash_timeout = LM3560_FLASH_TOUT_MIN * 1000;
> > + device_property_read_u32(flash->dev, "flash-max-timeout-us",
> > + &flash->max_flash_timeout);
> > + flash->max_flash_timeout /= 1000;
> > +
> > rval = lm3560_init_device(flash);
> > if (rval < 0)
> > return rval;
> >
> > - rval = lm3560_subdev_init(flash, LM3560_LED0, "lm3560-led0");
> > - if (rval < 0)
> > - return rval;
> > + for_each_available_child_of_node(dev_of_node(flash->dev), node) {
>
> device_for_each_child_node(), please.
>
> > + u32 reg;
> >
> > - rval = lm3560_subdev_init(flash, LM3560_LED1, "lm3560-led1");
> > - if (rval < 0) {
> > - lm3560_subdev_cleanup(flash, LM3560_LED0);
> > - return rval;
> > + rval = of_property_read_u32(node, "reg", ®);
>
> device_property_read_u32() here and elsewhere.
>
I have switched to OF equivalent deliberately because if I use
device_for_each_child_node I found no sane way to pass fwnode to
subdev_led since device_for_each_child_node releases fwnode handle it
uses and I cannot assign it to subdev_led.fwnode since it will result
in NULL pointer on loop exit
> > + if (rval < 0)
> > + /* We care only about nodes with reg property */
> > + continue;
> > +
> > + if (reg == LM3560_LED0 || reg == LM3560_LED1) {
> > + flash->max_flash_brt[reg] = LM3560_FLASH_BRT_MIN;
> > + of_property_read_u32(node, "flash-max-microamp",
> > + &flash->max_flash_brt[reg]);
> > +
> > + flash->max_torch_brt[reg] = LM3560_TORCH_BRT_MIN;
> > + of_property_read_u32(node, "led-max-microamp",
> > + &flash->max_torch_brt[reg]);
> > +
> > + rval = lm3560_subdev_init(flash, reg, node);
> > + if (rval < 0) {
> > + lm3560_subdev_cleanup(flash);
> > + return dev_err_probe(flash->dev, rval,
> > + "failed to register led%d\n",
> > + reg);
> > + }
> > +
> > + set_bit(reg, flash->led_id);
> > + }
> > }
> >
> > i2c_set_clientdata(client, flash);
> > @@ -455,12 +510,17 @@ static int lm3560_probe(struct i2c_client *client)
> > static void lm3560_remove(struct i2c_client *client)
> > {
> > struct lm3560_flash *flash = i2c_get_clientdata(client);
> > - unsigned int i;
> >
> > - for (i = LM3560_LED0; i < LM3560_LED_MAX; i++)
> > - lm3560_subdev_cleanup(flash, i);
> > + lm3560_subdev_cleanup(flash);
> > }
> >
> > +static const struct of_device_id lm3560_of_match[] = {
> > + { .compatible = "ti,lm3559" },
> > + { .compatible = "ti,lm3560" },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, lm3560_of_match);
> > +
> > static const struct i2c_device_id lm3560_id_table[] = {
> > { LM3559_NAME },
> > { LM3560_NAME },
> > @@ -473,6 +533,7 @@ static struct i2c_driver lm3560_i2c_driver = {
> > .driver = {
> > .name = LM3560_NAME,
> > .pm = NULL,
> > + .of_match_table = lm3560_of_match,
> > },
> > .probe = lm3560_probe,
> > .remove = lm3560_remove,
> > diff --git a/include/media/i2c/lm3560.h b/include/media/i2c/lm3560.h
> > index 770d8c72c94a..b56c1ff8fd49 100644
> > --- a/include/media/i2c/lm3560.h
> > +++ b/include/media/i2c/lm3560.h
> > @@ -66,19 +66,4 @@ enum lm3560_peak_current {
> > LM3560_PEAK_3600mA = 0x60
> > };
> >
> > -/* struct lm3560_platform_data
> > - *
> > - * @peak : peak current
> > - * @max_flash_timeout: flash timeout
> > - * @max_flash_brt: flash mode led brightness
> > - * @max_torch_brt: torch mode led brightness
> > - */
> > -struct lm3560_platform_data {
> > - enum lm3560_peak_current peak;
> > -
> > - u32 max_flash_timeout;
> > - u32 max_flash_brt[LM3560_LED_MAX];
> > - u32 max_torch_brt[LM3560_LED_MAX];
> > -};
> > -
> > #endif /* __LM3560_H__ */
>
> --
> Kind regards,
>
> Sakari Ailus
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 4/6] media: i2c: lm3560: Convert to use OF bindings
2026-05-04 7:40 ` Svyatoslav Ryhel
@ 2026-05-04 8:08 ` Sakari Ailus
2026-05-04 8:42 ` Svyatoslav Ryhel
0 siblings, 1 reply; 20+ messages in thread
From: Sakari Ailus @ 2026-05-04 8:08 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Mauro Carvalho Chehab, linux-leds, devicetree,
linux-kernel, linux-media
On Mon, May 04, 2026 at 10:40:11AM +0300, Svyatoslav Ryhel wrote:
> пн, 4 трав. 2026 р. о 09:36 Sakari Ailus <sakari.ailus@linux.intel.com> пише:
> >
> > Hi Svyatoslav,
> >
> > On Sun, May 03, 2026 at 07:44:43PM +0300, Svyatoslav Ryhel wrote:
> > > Since there are no users of this driver via platform data, remove the
> > > platform data support and switch to using Device Tree bindings.
> > >
> > > Converting to Device Tree assumes dynamic and independent registration of
> > > LEDs. To monitor the configured LEDs, a bitmap has been added. This makes
> > > LED cleanup more robust and less context dependent.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > > drivers/media/i2c/lm3560.c | 143 ++++++++++++++++++++++++++-----------
> > > include/media/i2c/lm3560.h | 15 ----
> > > 2 files changed, 102 insertions(+), 56 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
> > > index 5b568ed9536b..ce4b09d1f208 100644
> > > --- a/drivers/media/i2c/lm3560.c
> > > +++ b/drivers/media/i2c/lm3560.c
> > > @@ -9,11 +9,15 @@
> > > * Ldd-Mlp <ldd-mlp@list.ti.com>
> > > */
> > >
> > > +#include <linux/bitmap.h>
> > > #include <linux/delay.h>
> > > #include <linux/module.h>
> > > #include <linux/i2c.h>
> > > #include <linux/slab.h>
> > > +#include <linux/mod_devicetable.h>
> > > #include <linux/mutex.h>
> > > +#include <linux/of.h>
> > > +#include <linux/property.h>
> > > #include <linux/regmap.h>
> > > #include <linux/videodev2.h>
> > > #include <media/i2c/lm3560.h>
> > > @@ -43,22 +47,33 @@ enum led_enable {
> > > * struct lm3560_flash
> > > *
> > > * @dev: pointer to &struct device
> > > - * @pdata: platform data
> > > * @regmap: reg. map for i2c
> > > * @lock: muxtex for serial access.
> > > * @led_mode: V4L2 LED mode
> > > * @ctrls_led: V4L2 controls
> > > * @subdev_led: V4L2 subdev
> > > + * @led_id: LED status holder
> > > + * @peak: peak current
> > > + * @max_flash_timeout: flash timeout
> > > + * @max_flash_brt: flash mode led brightness
> > > + * @max_torch_brt: torch mode led brightness
> > > */
> > > struct lm3560_flash {
> > > struct device *dev;
> > > - struct lm3560_platform_data *pdata;
> > > struct regmap *regmap;
> > > struct mutex lock;
> > >
> > > enum v4l2_flash_led_mode led_mode;
> > > struct v4l2_ctrl_handler ctrls_led[LM3560_LED_MAX];
> > > struct v4l2_subdev subdev_led[LM3560_LED_MAX];
> > > +
> > > + DECLARE_BITMAP(led_id, LM3560_LED_MAX);
> > > +
> > > + enum lm3560_peak_current peak;
> > > + u32 max_flash_timeout;
> > > +
> > > + u32 max_flash_brt[LM3560_LED_MAX];
> > > + u32 max_torch_brt[LM3560_LED_MAX];
> > > };
> > >
> > > #define to_lm3560_flash(_ctrl, _no) \
> > > @@ -269,8 +284,8 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
> > > enum lm3560_led_id led_no)
> > > {
> > > struct v4l2_ctrl *fault;
> > > - u32 max_flash_brt = flash->pdata->max_flash_brt[led_no];
> > > - u32 max_torch_brt = flash->pdata->max_torch_brt[led_no];
> > > + u32 max_flash_brt = flash->max_flash_brt[led_no];
> > > + u32 max_torch_brt = flash->max_torch_brt[led_no];
> > > struct v4l2_ctrl_handler *hdl = &flash->ctrls_led[led_no];
> > > const struct v4l2_ctrl_ops *ops = &lm3560_led_ctrl_ops[led_no];
> > >
> > > @@ -295,9 +310,9 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
> > > /* flash strobe timeout */
> > > v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_TIMEOUT,
> > > LM3560_FLASH_TOUT_MIN,
> > > - flash->pdata->max_flash_timeout,
> > > + flash->max_flash_timeout,
> > > LM3560_FLASH_TOUT_STEP,
> > > - flash->pdata->max_flash_timeout);
> > > + flash->max_flash_timeout);
> > >
> > > /* flash brt */
> > > v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_INTENSITY,
> > > @@ -339,15 +354,18 @@ static const struct regmap_config lm3560_regmap = {
> > > };
> > >
> > > static int lm3560_subdev_init(struct lm3560_flash *flash,
> > > - enum lm3560_led_id led_no, char *led_name)
> > > + enum lm3560_led_id led_no,
> > > + struct device_node *np)
> > > {
> > > struct i2c_client *client = to_i2c_client(flash->dev);
> > > int rval;
> > >
> > > v4l2_i2c_subdev_init(&flash->subdev_led[led_no], client, &lm3560_ops);
> > > flash->subdev_led[led_no].flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > - strscpy(flash->subdev_led[led_no].name, led_name,
> > > - sizeof(flash->subdev_led[led_no].name));
> > > + snprintf(flash->subdev_led[led_no].name,
> > > + sizeof(flash->subdev_led[led_no].name),
> > > + "lm3560-led%d", led_no);
> > > + flash->subdev_led[led_no].fwnode = of_fwnode_handle(np);
> > > rval = lm3560_init_controls(flash, led_no);
> > > if (rval)
> > > goto err_out;
> > > @@ -378,7 +396,7 @@ static int lm3560_init_device(struct lm3560_flash *flash)
> > >
> > > /* set peak current */
> > > rval = regmap_update_bits(flash->regmap,
> > > - REG_FLASH_TOUT, 0x60, flash->pdata->peak);
> > > + REG_FLASH_TOUT, 0x60, flash->peak);
> > > if (rval < 0)
> > > return rval;
> > > /* output disable */
> > > @@ -391,18 +409,22 @@ static int lm3560_init_device(struct lm3560_flash *flash)
> > > return rval;
> > > }
> > >
> > > -static void lm3560_subdev_cleanup(struct lm3560_flash *flash,
> > > - enum lm3560_led_id led_no)
> > > +static void lm3560_subdev_cleanup(struct lm3560_flash *flash)
> > > {
> > > - v4l2_async_unregister_subdev(&flash->subdev_led[led_no]);
> > > - v4l2_ctrl_handler_free(&flash->ctrls_led[led_no]);
> > > - media_entity_cleanup(&flash->subdev_led[led_no].entity);
> > > + int led_no;
> > > +
> > > + for_each_set_bit(led_no, flash->led_id, LM3560_LED_MAX) {
> > > + v4l2_async_unregister_subdev(&flash->subdev_led[led_no]);
> > > + v4l2_ctrl_handler_free(&flash->ctrls_led[led_no]);
> > > + media_entity_cleanup(&flash->subdev_led[led_no].entity);
> > > + }
> > > }
> > >
> > > static int lm3560_probe(struct i2c_client *client)
> > > {
> > > struct lm3560_flash *flash;
> > > - struct lm3560_platform_data *pdata = dev_get_platdata(&client->dev);
> > > + struct device_node *node;
> > > + u32 peak_ua;
> > > int rval;
> > >
> > > flash = devm_kzalloc(&client->dev, sizeof(*flash), GFP_KERNEL);
> > > @@ -415,36 +437,69 @@ static int lm3560_probe(struct i2c_client *client)
> > > return rval;
> > > }
> > >
> > > - /* if there is no platform data, use chip default value */
> > > - if (pdata == NULL) {
> > > - pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
> > > - if (pdata == NULL)
> > > - return -ENODEV;
> > > - pdata->peak = LM3560_PEAK_3600mA;
> > > - pdata->max_flash_timeout = LM3560_FLASH_TOUT_MAX;
> > > - /* led 1 */
> > > - pdata->max_flash_brt[LM3560_LED0] = LM3560_FLASH_BRT_MAX;
> > > - pdata->max_torch_brt[LM3560_LED0] = LM3560_TORCH_BRT_MAX;
> > > - /* led 2 */
> > > - pdata->max_flash_brt[LM3560_LED1] = LM3560_FLASH_BRT_MAX;
> > > - pdata->max_torch_brt[LM3560_LED1] = LM3560_TORCH_BRT_MAX;
> > > - }
> > > - flash->pdata = pdata;
> > > flash->dev = &client->dev;
> > > mutex_init(&flash->lock);
> > >
> > > + bitmap_zero(flash->led_id, LM3560_LED_MAX);
> > > +
> > > + flash->peak = LM3560_PEAK_1600mA;
> > > + rval = device_property_read_u32(flash->dev,
> > > + "ti,peak-current-microamp", &peak_ua);
> > > + if (!rval) {
> > > + switch (peak_ua) {
> > > + case 1600000:
> > > + flash->peak = LM3560_PEAK_1600mA;
> > > + break;
> > > + case 2300000:
> > > + flash->peak = LM3560_PEAK_2300mA;
> > > + break;
> > > + case 3000000:
> > > + flash->peak = LM3560_PEAK_3000mA;
> > > + break;
> > > + case 3600000:
> > > + flash->peak = LM3560_PEAK_3600mA;
> > > + break;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > + }
> > > +
> > > + flash->max_flash_timeout = LM3560_FLASH_TOUT_MIN * 1000;
> > > + device_property_read_u32(flash->dev, "flash-max-timeout-us",
> > > + &flash->max_flash_timeout);
> > > + flash->max_flash_timeout /= 1000;
> > > +
> > > rval = lm3560_init_device(flash);
> > > if (rval < 0)
> > > return rval;
> > >
> > > - rval = lm3560_subdev_init(flash, LM3560_LED0, "lm3560-led0");
> > > - if (rval < 0)
> > > - return rval;
> > > + for_each_available_child_of_node(dev_of_node(flash->dev), node) {
> >
> > device_for_each_child_node(), please.
> >
> > > + u32 reg;
> > >
> > > - rval = lm3560_subdev_init(flash, LM3560_LED1, "lm3560-led1");
> > > - if (rval < 0) {
> > > - lm3560_subdev_cleanup(flash, LM3560_LED0);
> > > - return rval;
> > > + rval = of_property_read_u32(node, "reg", ®);
> >
> > device_property_read_u32() here and elsewhere.
> >
>
> I have switched to OF equivalent deliberately because if I use
> device_for_each_child_node I found no sane way to pass fwnode to
> subdev_led since device_for_each_child_node releases fwnode handle it
> uses and I cannot assign it to subdev_led.fwnode since it will result
> in NULL pointer on loop exit
device_for_each_child_node() is indeed meant the be functionally equivalent
to for_each_available_child_of_node().
If you need to hold a reference to the child node -- as I think you do --
you'll need to call fwnode_handle_get() on it, as you'd call of_node_get()
on OF API.
>
> > > + if (rval < 0)
> > > + /* We care only about nodes with reg property */
> > > + continue;
> > > +
> > > + if (reg == LM3560_LED0 || reg == LM3560_LED1) {
> > > + flash->max_flash_brt[reg] = LM3560_FLASH_BRT_MIN;
> > > + of_property_read_u32(node, "flash-max-microamp",
> > > + &flash->max_flash_brt[reg]);
> > > +
> > > + flash->max_torch_brt[reg] = LM3560_TORCH_BRT_MIN;
> > > + of_property_read_u32(node, "led-max-microamp",
> > > + &flash->max_torch_brt[reg]);
> > > +
> > > + rval = lm3560_subdev_init(flash, reg, node);
> > > + if (rval < 0) {
> > > + lm3560_subdev_cleanup(flash);
> > > + return dev_err_probe(flash->dev, rval,
> > > + "failed to register led%d\n",
> > > + reg);
> > > + }
> > > +
> > > + set_bit(reg, flash->led_id);
> > > + }
> > > }
> > >
> > > i2c_set_clientdata(client, flash);
> > > @@ -455,12 +510,17 @@ static int lm3560_probe(struct i2c_client *client)
> > > static void lm3560_remove(struct i2c_client *client)
> > > {
> > > struct lm3560_flash *flash = i2c_get_clientdata(client);
> > > - unsigned int i;
> > >
> > > - for (i = LM3560_LED0; i < LM3560_LED_MAX; i++)
> > > - lm3560_subdev_cleanup(flash, i);
> > > + lm3560_subdev_cleanup(flash);
> > > }
> > >
> > > +static const struct of_device_id lm3560_of_match[] = {
> > > + { .compatible = "ti,lm3559" },
> > > + { .compatible = "ti,lm3560" },
> > > + { }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, lm3560_of_match);
> > > +
> > > static const struct i2c_device_id lm3560_id_table[] = {
> > > { LM3559_NAME },
> > > { LM3560_NAME },
> > > @@ -473,6 +533,7 @@ static struct i2c_driver lm3560_i2c_driver = {
> > > .driver = {
> > > .name = LM3560_NAME,
> > > .pm = NULL,
> > > + .of_match_table = lm3560_of_match,
> > > },
> > > .probe = lm3560_probe,
> > > .remove = lm3560_remove,
> > > diff --git a/include/media/i2c/lm3560.h b/include/media/i2c/lm3560.h
> > > index 770d8c72c94a..b56c1ff8fd49 100644
> > > --- a/include/media/i2c/lm3560.h
> > > +++ b/include/media/i2c/lm3560.h
> > > @@ -66,19 +66,4 @@ enum lm3560_peak_current {
> > > LM3560_PEAK_3600mA = 0x60
> > > };
> > >
> > > -/* struct lm3560_platform_data
> > > - *
> > > - * @peak : peak current
> > > - * @max_flash_timeout: flash timeout
> > > - * @max_flash_brt: flash mode led brightness
> > > - * @max_torch_brt: torch mode led brightness
> > > - */
> > > -struct lm3560_platform_data {
> > > - enum lm3560_peak_current peak;
> > > -
> > > - u32 max_flash_timeout;
> > > - u32 max_flash_brt[LM3560_LED_MAX];
> > > - u32 max_torch_brt[LM3560_LED_MAX];
> > > -};
> > > -
> > > #endif /* __LM3560_H__ */
> >
> > --
> > Kind regards,
> >
> > Sakari Ailus
--
Sakari Ailus
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 4/6] media: i2c: lm3560: Convert to use OF bindings
2026-05-04 8:08 ` Sakari Ailus
@ 2026-05-04 8:42 ` Svyatoslav Ryhel
0 siblings, 0 replies; 20+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-04 8:42 UTC (permalink / raw)
To: Sakari Ailus
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Mauro Carvalho Chehab, linux-leds, devicetree,
linux-kernel, linux-media
пн, 4 трав. 2026 р. о 11:08 Sakari Ailus <sakari.ailus@linux.intel.com> пише:
>
> On Mon, May 04, 2026 at 10:40:11AM +0300, Svyatoslav Ryhel wrote:
> > пн, 4 трав. 2026 р. о 09:36 Sakari Ailus <sakari.ailus@linux.intel.com> пише:
> > >
> > > Hi Svyatoslav,
> > >
> > > On Sun, May 03, 2026 at 07:44:43PM +0300, Svyatoslav Ryhel wrote:
> > > > Since there are no users of this driver via platform data, remove the
> > > > platform data support and switch to using Device Tree bindings.
> > > >
> > > > Converting to Device Tree assumes dynamic and independent registration of
> > > > LEDs. To monitor the configured LEDs, a bitmap has been added. This makes
> > > > LED cleanup more robust and less context dependent.
> > > >
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > ---
> > > > drivers/media/i2c/lm3560.c | 143 ++++++++++++++++++++++++++-----------
> > > > include/media/i2c/lm3560.h | 15 ----
> > > > 2 files changed, 102 insertions(+), 56 deletions(-)
> > > >
> > > > diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
> > > > index 5b568ed9536b..ce4b09d1f208 100644
> > > > --- a/drivers/media/i2c/lm3560.c
> > > > +++ b/drivers/media/i2c/lm3560.c
> > > > @@ -9,11 +9,15 @@
> > > > * Ldd-Mlp <ldd-mlp@list.ti.com>
> > > > */
> > > >
> > > > +#include <linux/bitmap.h>
> > > > #include <linux/delay.h>
> > > > #include <linux/module.h>
> > > > #include <linux/i2c.h>
> > > > #include <linux/slab.h>
> > > > +#include <linux/mod_devicetable.h>
> > > > #include <linux/mutex.h>
> > > > +#include <linux/of.h>
> > > > +#include <linux/property.h>
> > > > #include <linux/regmap.h>
> > > > #include <linux/videodev2.h>
> > > > #include <media/i2c/lm3560.h>
> > > > @@ -43,22 +47,33 @@ enum led_enable {
> > > > * struct lm3560_flash
> > > > *
> > > > * @dev: pointer to &struct device
> > > > - * @pdata: platform data
> > > > * @regmap: reg. map for i2c
> > > > * @lock: muxtex for serial access.
> > > > * @led_mode: V4L2 LED mode
> > > > * @ctrls_led: V4L2 controls
> > > > * @subdev_led: V4L2 subdev
> > > > + * @led_id: LED status holder
> > > > + * @peak: peak current
> > > > + * @max_flash_timeout: flash timeout
> > > > + * @max_flash_brt: flash mode led brightness
> > > > + * @max_torch_brt: torch mode led brightness
> > > > */
> > > > struct lm3560_flash {
> > > > struct device *dev;
> > > > - struct lm3560_platform_data *pdata;
> > > > struct regmap *regmap;
> > > > struct mutex lock;
> > > >
> > > > enum v4l2_flash_led_mode led_mode;
> > > > struct v4l2_ctrl_handler ctrls_led[LM3560_LED_MAX];
> > > > struct v4l2_subdev subdev_led[LM3560_LED_MAX];
> > > > +
> > > > + DECLARE_BITMAP(led_id, LM3560_LED_MAX);
> > > > +
> > > > + enum lm3560_peak_current peak;
> > > > + u32 max_flash_timeout;
> > > > +
> > > > + u32 max_flash_brt[LM3560_LED_MAX];
> > > > + u32 max_torch_brt[LM3560_LED_MAX];
> > > > };
> > > >
> > > > #define to_lm3560_flash(_ctrl, _no) \
> > > > @@ -269,8 +284,8 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
> > > > enum lm3560_led_id led_no)
> > > > {
> > > > struct v4l2_ctrl *fault;
> > > > - u32 max_flash_brt = flash->pdata->max_flash_brt[led_no];
> > > > - u32 max_torch_brt = flash->pdata->max_torch_brt[led_no];
> > > > + u32 max_flash_brt = flash->max_flash_brt[led_no];
> > > > + u32 max_torch_brt = flash->max_torch_brt[led_no];
> > > > struct v4l2_ctrl_handler *hdl = &flash->ctrls_led[led_no];
> > > > const struct v4l2_ctrl_ops *ops = &lm3560_led_ctrl_ops[led_no];
> > > >
> > > > @@ -295,9 +310,9 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
> > > > /* flash strobe timeout */
> > > > v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_TIMEOUT,
> > > > LM3560_FLASH_TOUT_MIN,
> > > > - flash->pdata->max_flash_timeout,
> > > > + flash->max_flash_timeout,
> > > > LM3560_FLASH_TOUT_STEP,
> > > > - flash->pdata->max_flash_timeout);
> > > > + flash->max_flash_timeout);
> > > >
> > > > /* flash brt */
> > > > v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_INTENSITY,
> > > > @@ -339,15 +354,18 @@ static const struct regmap_config lm3560_regmap = {
> > > > };
> > > >
> > > > static int lm3560_subdev_init(struct lm3560_flash *flash,
> > > > - enum lm3560_led_id led_no, char *led_name)
> > > > + enum lm3560_led_id led_no,
> > > > + struct device_node *np)
> > > > {
> > > > struct i2c_client *client = to_i2c_client(flash->dev);
> > > > int rval;
> > > >
> > > > v4l2_i2c_subdev_init(&flash->subdev_led[led_no], client, &lm3560_ops);
> > > > flash->subdev_led[led_no].flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > > - strscpy(flash->subdev_led[led_no].name, led_name,
> > > > - sizeof(flash->subdev_led[led_no].name));
> > > > + snprintf(flash->subdev_led[led_no].name,
> > > > + sizeof(flash->subdev_led[led_no].name),
> > > > + "lm3560-led%d", led_no);
> > > > + flash->subdev_led[led_no].fwnode = of_fwnode_handle(np);
> > > > rval = lm3560_init_controls(flash, led_no);
> > > > if (rval)
> > > > goto err_out;
> > > > @@ -378,7 +396,7 @@ static int lm3560_init_device(struct lm3560_flash *flash)
> > > >
> > > > /* set peak current */
> > > > rval = regmap_update_bits(flash->regmap,
> > > > - REG_FLASH_TOUT, 0x60, flash->pdata->peak);
> > > > + REG_FLASH_TOUT, 0x60, flash->peak);
> > > > if (rval < 0)
> > > > return rval;
> > > > /* output disable */
> > > > @@ -391,18 +409,22 @@ static int lm3560_init_device(struct lm3560_flash *flash)
> > > > return rval;
> > > > }
> > > >
> > > > -static void lm3560_subdev_cleanup(struct lm3560_flash *flash,
> > > > - enum lm3560_led_id led_no)
> > > > +static void lm3560_subdev_cleanup(struct lm3560_flash *flash)
> > > > {
> > > > - v4l2_async_unregister_subdev(&flash->subdev_led[led_no]);
> > > > - v4l2_ctrl_handler_free(&flash->ctrls_led[led_no]);
> > > > - media_entity_cleanup(&flash->subdev_led[led_no].entity);
> > > > + int led_no;
> > > > +
> > > > + for_each_set_bit(led_no, flash->led_id, LM3560_LED_MAX) {
> > > > + v4l2_async_unregister_subdev(&flash->subdev_led[led_no]);
> > > > + v4l2_ctrl_handler_free(&flash->ctrls_led[led_no]);
> > > > + media_entity_cleanup(&flash->subdev_led[led_no].entity);
> > > > + }
> > > > }
> > > >
> > > > static int lm3560_probe(struct i2c_client *client)
> > > > {
> > > > struct lm3560_flash *flash;
> > > > - struct lm3560_platform_data *pdata = dev_get_platdata(&client->dev);
> > > > + struct device_node *node;
> > > > + u32 peak_ua;
> > > > int rval;
> > > >
> > > > flash = devm_kzalloc(&client->dev, sizeof(*flash), GFP_KERNEL);
> > > > @@ -415,36 +437,69 @@ static int lm3560_probe(struct i2c_client *client)
> > > > return rval;
> > > > }
> > > >
> > > > - /* if there is no platform data, use chip default value */
> > > > - if (pdata == NULL) {
> > > > - pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
> > > > - if (pdata == NULL)
> > > > - return -ENODEV;
> > > > - pdata->peak = LM3560_PEAK_3600mA;
> > > > - pdata->max_flash_timeout = LM3560_FLASH_TOUT_MAX;
> > > > - /* led 1 */
> > > > - pdata->max_flash_brt[LM3560_LED0] = LM3560_FLASH_BRT_MAX;
> > > > - pdata->max_torch_brt[LM3560_LED0] = LM3560_TORCH_BRT_MAX;
> > > > - /* led 2 */
> > > > - pdata->max_flash_brt[LM3560_LED1] = LM3560_FLASH_BRT_MAX;
> > > > - pdata->max_torch_brt[LM3560_LED1] = LM3560_TORCH_BRT_MAX;
> > > > - }
> > > > - flash->pdata = pdata;
> > > > flash->dev = &client->dev;
> > > > mutex_init(&flash->lock);
> > > >
> > > > + bitmap_zero(flash->led_id, LM3560_LED_MAX);
> > > > +
> > > > + flash->peak = LM3560_PEAK_1600mA;
> > > > + rval = device_property_read_u32(flash->dev,
> > > > + "ti,peak-current-microamp", &peak_ua);
> > > > + if (!rval) {
> > > > + switch (peak_ua) {
> > > > + case 1600000:
> > > > + flash->peak = LM3560_PEAK_1600mA;
> > > > + break;
> > > > + case 2300000:
> > > > + flash->peak = LM3560_PEAK_2300mA;
> > > > + break;
> > > > + case 3000000:
> > > > + flash->peak = LM3560_PEAK_3000mA;
> > > > + break;
> > > > + case 3600000:
> > > > + flash->peak = LM3560_PEAK_3600mA;
> > > > + break;
> > > > + default:
> > > > + return -EINVAL;
> > > > + }
> > > > + }
> > > > +
> > > > + flash->max_flash_timeout = LM3560_FLASH_TOUT_MIN * 1000;
> > > > + device_property_read_u32(flash->dev, "flash-max-timeout-us",
> > > > + &flash->max_flash_timeout);
> > > > + flash->max_flash_timeout /= 1000;
> > > > +
> > > > rval = lm3560_init_device(flash);
> > > > if (rval < 0)
> > > > return rval;
> > > >
> > > > - rval = lm3560_subdev_init(flash, LM3560_LED0, "lm3560-led0");
> > > > - if (rval < 0)
> > > > - return rval;
> > > > + for_each_available_child_of_node(dev_of_node(flash->dev), node) {
> > >
> > > device_for_each_child_node(), please.
> > >
> > > > + u32 reg;
> > > >
> > > > - rval = lm3560_subdev_init(flash, LM3560_LED1, "lm3560-led1");
> > > > - if (rval < 0) {
> > > > - lm3560_subdev_cleanup(flash, LM3560_LED0);
> > > > - return rval;
> > > > + rval = of_property_read_u32(node, "reg", ®);
> > >
> > > device_property_read_u32() here and elsewhere.
> > >
> >
> > I have switched to OF equivalent deliberately because if I use
> > device_for_each_child_node I found no sane way to pass fwnode to
> > subdev_led since device_for_each_child_node releases fwnode handle it
> > uses and I cannot assign it to subdev_led.fwnode since it will result
> > in NULL pointer on loop exit
>
> device_for_each_child_node() is indeed meant the be functionally equivalent
> to for_each_available_child_of_node().
>
> If you need to hold a reference to the child node -- as I think you do --
> you'll need to call fwnode_handle_get() on it, as you'd call of_node_get()
> on OF API.
>
ATM fwnode of led is passed with of_fwnode_handle from of_node, but it
seems that your suggestion is reasonable, thank you.
> >
> > > > + if (rval < 0)
> > > > + /* We care only about nodes with reg property */
> > > > + continue;
> > > > +
> > > > + if (reg == LM3560_LED0 || reg == LM3560_LED1) {
> > > > + flash->max_flash_brt[reg] = LM3560_FLASH_BRT_MIN;
> > > > + of_property_read_u32(node, "flash-max-microamp",
> > > > + &flash->max_flash_brt[reg]);
> > > > +
> > > > + flash->max_torch_brt[reg] = LM3560_TORCH_BRT_MIN;
> > > > + of_property_read_u32(node, "led-max-microamp",
> > > > + &flash->max_torch_brt[reg]);
> > > > +
> > > > + rval = lm3560_subdev_init(flash, reg, node);
> > > > + if (rval < 0) {
> > > > + lm3560_subdev_cleanup(flash);
> > > > + return dev_err_probe(flash->dev, rval,
> > > > + "failed to register led%d\n",
> > > > + reg);
> > > > + }
> > > > +
> > > > + set_bit(reg, flash->led_id);
> > > > + }
> > > > }
> > > >
> > > > i2c_set_clientdata(client, flash);
> > > > @@ -455,12 +510,17 @@ static int lm3560_probe(struct i2c_client *client)
> > > > static void lm3560_remove(struct i2c_client *client)
> > > > {
> > > > struct lm3560_flash *flash = i2c_get_clientdata(client);
> > > > - unsigned int i;
> > > >
> > > > - for (i = LM3560_LED0; i < LM3560_LED_MAX; i++)
> > > > - lm3560_subdev_cleanup(flash, i);
> > > > + lm3560_subdev_cleanup(flash);
> > > > }
> > > >
> > > > +static const struct of_device_id lm3560_of_match[] = {
> > > > + { .compatible = "ti,lm3559" },
> > > > + { .compatible = "ti,lm3560" },
> > > > + { }
> > > > +};
> > > > +MODULE_DEVICE_TABLE(of, lm3560_of_match);
> > > > +
> > > > static const struct i2c_device_id lm3560_id_table[] = {
> > > > { LM3559_NAME },
> > > > { LM3560_NAME },
> > > > @@ -473,6 +533,7 @@ static struct i2c_driver lm3560_i2c_driver = {
> > > > .driver = {
> > > > .name = LM3560_NAME,
> > > > .pm = NULL,
> > > > + .of_match_table = lm3560_of_match,
> > > > },
> > > > .probe = lm3560_probe,
> > > > .remove = lm3560_remove,
> > > > diff --git a/include/media/i2c/lm3560.h b/include/media/i2c/lm3560.h
> > > > index 770d8c72c94a..b56c1ff8fd49 100644
> > > > --- a/include/media/i2c/lm3560.h
> > > > +++ b/include/media/i2c/lm3560.h
> > > > @@ -66,19 +66,4 @@ enum lm3560_peak_current {
> > > > LM3560_PEAK_3600mA = 0x60
> > > > };
> > > >
> > > > -/* struct lm3560_platform_data
> > > > - *
> > > > - * @peak : peak current
> > > > - * @max_flash_timeout: flash timeout
> > > > - * @max_flash_brt: flash mode led brightness
> > > > - * @max_torch_brt: torch mode led brightness
> > > > - */
> > > > -struct lm3560_platform_data {
> > > > - enum lm3560_peak_current peak;
> > > > -
> > > > - u32 max_flash_timeout;
> > > > - u32 max_flash_brt[LM3560_LED_MAX];
> > > > - u32 max_torch_brt[LM3560_LED_MAX];
> > > > -};
> > > > -
> > > > #endif /* __LM3560_H__ */
> > >
> > > --
> > > Kind regards,
> > >
> > > Sakari Ailus
>
> --
> Sakari Ailus
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 5/6] media: i2c: lm3560: Add support for PM features
2026-05-03 16:44 [PATCH v5 0/6] media: lm3560: convert to use OF bindings Svyatoslav Ryhel
` (3 preceding siblings ...)
2026-05-03 16:44 ` [PATCH v5 4/6] media: i2c: lm3560: Convert to use OF bindings Svyatoslav Ryhel
@ 2026-05-03 16:44 ` Svyatoslav Ryhel
2026-05-04 6:37 ` Sakari Ailus
2026-05-03 16:44 ` [PATCH v5 6/6] media: i2c: lm3560: Add proper support for LM3559 Svyatoslav Ryhel
2026-05-04 5:35 ` [PATCH v5 0/6] media: lm3560: convert to use OF bindings Svyatoslav Ryhel
6 siblings, 1 reply; 20+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-03 16:44 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sakari Ailus, Mauro Carvalho Chehab,
Svyatoslav Ryhel
Cc: linux-leds, devicetree, linux-kernel, linux-media
Add support for power management features to better control the LM3560
within the media framework. To achieve the desired PM support, the HWEN
GPIO and VIN power supply were added and configured into power on/off
sequences. Added PM operations along with the PM configuration setup.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/media/i2c/lm3560.c | 117 ++++++++++++++++++++++++++++++++++---
1 file changed, 110 insertions(+), 7 deletions(-)
diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
index ce4b09d1f208..15741ea5684f 100644
--- a/drivers/media/i2c/lm3560.c
+++ b/drivers/media/i2c/lm3560.c
@@ -12,13 +12,16 @@
#include <linux/bitmap.h>
#include <linux/delay.h>
#include <linux/module.h>
+#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/slab.h>
#include <linux/mod_devicetable.h>
#include <linux/mutex.h>
#include <linux/of.h>
+#include <linux/pm_runtime.h>
#include <linux/property.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <linux/videodev2.h>
#include <media/i2c/lm3560.h>
#include <media/v4l2-ctrls.h>
@@ -49,6 +52,8 @@ enum led_enable {
* @dev: pointer to &struct device
* @regmap: reg. map for i2c
* @lock: muxtex for serial access.
+ * @hwen_gpio: line connected to HWEN pin
+ * @vin_supply: line connected to IN supply (2.5V - 5.5V)
* @led_mode: V4L2 LED mode
* @ctrls_led: V4L2 controls
* @subdev_led: V4L2 subdev
@@ -63,6 +68,9 @@ struct lm3560_flash {
struct regmap *regmap;
struct mutex lock;
+ struct gpio_desc *hwen_gpio;
+ struct regulator *vin_supply;
+
enum v4l2_flash_led_mode led_mode;
struct v4l2_ctrl_handler ctrls_led[LM3560_LED_MAX];
struct v4l2_subdev subdev_led[LM3560_LED_MAX];
@@ -177,12 +185,17 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
struct lm3560_flash *flash = to_lm3560_flash(ctrl, led_no);
int rval = -EINVAL;
+ if (!pm_runtime_get_if_in_use(flash->dev))
+ return 0;
+
if (ctrl->id == V4L2_CID_FLASH_FAULT) {
s32 fault = 0;
unsigned int reg_val;
rval = regmap_read(flash->regmap, REG_FLAG, ®_val);
- if (rval < 0)
+ if (rval < 0) {
+ pm_runtime_put(flash->dev);
return rval;
+ }
if (reg_val & FAULT_SHORT_CIRCUIT)
fault |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
if (reg_val & FAULT_OVERTEMP)
@@ -192,6 +205,8 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
ctrl->cur.val = fault;
}
+ pm_runtime_put(flash->dev);
+
return rval;
}
@@ -201,6 +216,9 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
u8 tout_bits;
int rval = -EINVAL;
+ if (!pm_runtime_get_if_in_use(flash->dev))
+ return 0;
+
switch (ctrl->id) {
case V4L2_CID_FLASH_LED_MODE:
flash->led_mode = ctrl->val;
@@ -246,6 +264,8 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
break;
}
+ pm_runtime_put(flash->dev);
+
return rval;
}
@@ -409,6 +429,38 @@ static int lm3560_init_device(struct lm3560_flash *flash)
return rval;
}
+static int __maybe_unused lm3560_power_off(struct device *dev)
+{
+ struct lm3560_flash *flash = dev_get_drvdata(dev);
+
+ gpiod_set_value_cansleep(flash->hwen_gpio, 0);
+ regulator_disable(flash->vin_supply);
+
+ return 0;
+}
+
+static int __maybe_unused lm3560_power_on(struct device *dev)
+{
+ struct lm3560_flash *flash = dev_get_drvdata(dev);
+ int rval;
+
+ rval = regulator_enable(flash->vin_supply);
+ if (rval < 0) {
+ dev_err(flash->dev, "failed to enable vin power supply\n");
+ return rval;
+ }
+
+ gpiod_set_value_cansleep(flash->hwen_gpio, 1);
+
+ rval = lm3560_init_device(flash);
+ if (rval < 0) {
+ lm3560_power_off(dev);
+ return rval;
+ }
+
+ return 0;
+}
+
static void lm3560_subdev_cleanup(struct lm3560_flash *flash)
{
int led_no;
@@ -442,6 +494,17 @@ static int lm3560_probe(struct i2c_client *client)
bitmap_zero(flash->led_id, LM3560_LED_MAX);
+ flash->hwen_gpio = devm_gpiod_get_optional(flash->dev, "enable",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(flash->hwen_gpio))
+ return dev_err_probe(flash->dev, PTR_ERR(flash->hwen_gpio),
+ "failed to get hwen gpio\n");
+
+ flash->vin_supply = devm_regulator_get(flash->dev, "vin");
+ if (IS_ERR(flash->vin_supply))
+ return dev_err_probe(flash->dev, PTR_ERR(flash->vin_supply),
+ "failed to get vin-supply\n");
+
flash->peak = LM3560_PEAK_1600mA;
rval = device_property_read_u32(flash->dev,
"ti,peak-current-microamp", &peak_ua);
@@ -469,9 +532,19 @@ static int lm3560_probe(struct i2c_client *client)
&flash->max_flash_timeout);
flash->max_flash_timeout /= 1000;
+ rval = regulator_enable(flash->vin_supply);
+ if (rval < 0)
+ return dev_err_probe(flash->dev, rval,
+ "failed to enable vin power supply\n");
+
+ gpiod_set_value_cansleep(flash->hwen_gpio, 1);
+
rval = lm3560_init_device(flash);
if (rval < 0)
- return rval;
+ goto error_disable;
+
+ pm_runtime_set_active(flash->dev);
+ pm_runtime_enable(flash->dev);
for_each_available_child_of_node(dev_of_node(flash->dev), node) {
u32 reg;
@@ -492,10 +565,10 @@ static int lm3560_probe(struct i2c_client *client)
rval = lm3560_subdev_init(flash, reg, node);
if (rval < 0) {
- lm3560_subdev_cleanup(flash);
- return dev_err_probe(flash->dev, rval,
- "failed to register led%d\n",
- reg);
+ dev_err(flash->dev,
+ "failed to register led%d: %d\n",
+ reg, rval);
+ goto error_clean;
}
set_bit(reg, flash->led_id);
@@ -504,7 +577,23 @@ static int lm3560_probe(struct i2c_client *client)
i2c_set_clientdata(client, flash);
+ pm_runtime_set_autosuspend_delay(flash->dev, 1000);
+ pm_runtime_use_autosuspend(flash->dev);
+ pm_runtime_idle(flash->dev);
+
return 0;
+
+error_clean:
+ pm_runtime_disable(flash->dev);
+ pm_runtime_set_suspended(flash->dev);
+
+ lm3560_subdev_cleanup(flash);
+
+error_disable:
+ gpiod_set_value_cansleep(flash->hwen_gpio, 0);
+ regulator_disable(flash->vin_supply);
+
+ return rval;
}
static void lm3560_remove(struct i2c_client *client)
@@ -512,8 +601,22 @@ static void lm3560_remove(struct i2c_client *client)
struct lm3560_flash *flash = i2c_get_clientdata(client);
lm3560_subdev_cleanup(flash);
+
+ /*
+ * Disable runtime PM. In case runtime PM is disabled in the kernel,
+ * make sure to turn power off manually.
+ */
+ pm_runtime_disable(&client->dev);
+ if (!pm_runtime_status_suspended(&client->dev)) {
+ lm3560_power_off(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
+ }
}
+static const struct dev_pm_ops lm3560_pm_ops = {
+ SET_RUNTIME_PM_OPS(lm3560_power_off, lm3560_power_on, NULL)
+};
+
static const struct of_device_id lm3560_of_match[] = {
{ .compatible = "ti,lm3559" },
{ .compatible = "ti,lm3560" },
@@ -532,7 +635,7 @@ MODULE_DEVICE_TABLE(i2c, lm3560_id_table);
static struct i2c_driver lm3560_i2c_driver = {
.driver = {
.name = LM3560_NAME,
- .pm = NULL,
+ .pm = pm_ptr(&lm3560_pm_ops),
.of_match_table = lm3560_of_match,
},
.probe = lm3560_probe,
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 5/6] media: i2c: lm3560: Add support for PM features
2026-05-03 16:44 ` [PATCH v5 5/6] media: i2c: lm3560: Add support for PM features Svyatoslav Ryhel
@ 2026-05-04 6:37 ` Sakari Ailus
2026-05-04 7:40 ` Svyatoslav Ryhel
0 siblings, 1 reply; 20+ messages in thread
From: Sakari Ailus @ 2026-05-04 6:37 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Mauro Carvalho Chehab, linux-leds, devicetree,
linux-kernel, linux-media
Hi Svyatoslav,
On Sun, May 03, 2026 at 07:44:44PM +0300, Svyatoslav Ryhel wrote:
> Add support for power management features to better control the LM3560
> within the media framework. To achieve the desired PM support, the HWEN
> GPIO and VIN power supply were added and configured into power on/off
> sequences. Added PM operations along with the PM configuration setup.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
> drivers/media/i2c/lm3560.c | 117 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 110 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
> index ce4b09d1f208..15741ea5684f 100644
> --- a/drivers/media/i2c/lm3560.c
> +++ b/drivers/media/i2c/lm3560.c
> @@ -12,13 +12,16 @@
> #include <linux/bitmap.h>
> #include <linux/delay.h>
> #include <linux/module.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> #include <linux/slab.h>
> #include <linux/mod_devicetable.h>
> #include <linux/mutex.h>
> #include <linux/of.h>
> +#include <linux/pm_runtime.h>
> #include <linux/property.h>
> #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> #include <linux/videodev2.h>
> #include <media/i2c/lm3560.h>
> #include <media/v4l2-ctrls.h>
> @@ -49,6 +52,8 @@ enum led_enable {
> * @dev: pointer to &struct device
> * @regmap: reg. map for i2c
> * @lock: muxtex for serial access.
> + * @hwen_gpio: line connected to HWEN pin
> + * @vin_supply: line connected to IN supply (2.5V - 5.5V)
> * @led_mode: V4L2 LED mode
> * @ctrls_led: V4L2 controls
> * @subdev_led: V4L2 subdev
> @@ -63,6 +68,9 @@ struct lm3560_flash {
> struct regmap *regmap;
> struct mutex lock;
>
> + struct gpio_desc *hwen_gpio;
> + struct regulator *vin_supply;
> +
> enum v4l2_flash_led_mode led_mode;
> struct v4l2_ctrl_handler ctrls_led[LM3560_LED_MAX];
> struct v4l2_subdev subdev_led[LM3560_LED_MAX];
> @@ -177,12 +185,17 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> struct lm3560_flash *flash = to_lm3560_flash(ctrl, led_no);
> int rval = -EINVAL;
>
> + if (!pm_runtime_get_if_in_use(flash->dev))
> + return 0;
> +
> if (ctrl->id == V4L2_CID_FLASH_FAULT) {
> s32 fault = 0;
> unsigned int reg_val;
> rval = regmap_read(flash->regmap, REG_FLAG, ®_val);
> - if (rval < 0)
> + if (rval < 0) {
> + pm_runtime_put(flash->dev);
> return rval;
> + }
> if (reg_val & FAULT_SHORT_CIRCUIT)
> fault |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
> if (reg_val & FAULT_OVERTEMP)
> @@ -192,6 +205,8 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> ctrl->cur.val = fault;
> }
>
> + pm_runtime_put(flash->dev);
> +
> return rval;
> }
>
> @@ -201,6 +216,9 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> u8 tout_bits;
> int rval = -EINVAL;
>
> + if (!pm_runtime_get_if_in_use(flash->dev))
This should be pm_runtime_get_if_active().
> + return 0;
> +
> switch (ctrl->id) {
> case V4L2_CID_FLASH_LED_MODE:
> flash->led_mode = ctrl->val;
> @@ -246,6 +264,8 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> break;
> }
>
> + pm_runtime_put(flash->dev);
> +
> return rval;
> }
>
> @@ -409,6 +429,38 @@ static int lm3560_init_device(struct lm3560_flash *flash)
> return rval;
> }
>
> +static int __maybe_unused lm3560_power_off(struct device *dev)
> +{
> + struct lm3560_flash *flash = dev_get_drvdata(dev);
> +
> + gpiod_set_value_cansleep(flash->hwen_gpio, 0);
> + regulator_disable(flash->vin_supply);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused lm3560_power_on(struct device *dev)
> +{
> + struct lm3560_flash *flash = dev_get_drvdata(dev);
> + int rval;
> +
> + rval = regulator_enable(flash->vin_supply);
> + if (rval < 0) {
> + dev_err(flash->dev, "failed to enable vin power supply\n");
> + return rval;
> + }
> +
> + gpiod_set_value_cansleep(flash->hwen_gpio, 1);
> +
> + rval = lm3560_init_device(flash);
> + if (rval < 0) {
> + lm3560_power_off(dev);
> + return rval;
> + }
> +
> + return 0;
> +}
> +
> static void lm3560_subdev_cleanup(struct lm3560_flash *flash)
> {
> int led_no;
> @@ -442,6 +494,17 @@ static int lm3560_probe(struct i2c_client *client)
>
> bitmap_zero(flash->led_id, LM3560_LED_MAX);
>
> + flash->hwen_gpio = devm_gpiod_get_optional(flash->dev, "enable",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(flash->hwen_gpio))
> + return dev_err_probe(flash->dev, PTR_ERR(flash->hwen_gpio),
> + "failed to get hwen gpio\n");
> +
> + flash->vin_supply = devm_regulator_get(flash->dev, "vin");
> + if (IS_ERR(flash->vin_supply))
> + return dev_err_probe(flash->dev, PTR_ERR(flash->vin_supply),
> + "failed to get vin-supply\n");
> +
> flash->peak = LM3560_PEAK_1600mA;
> rval = device_property_read_u32(flash->dev,
> "ti,peak-current-microamp", &peak_ua);
> @@ -469,9 +532,19 @@ static int lm3560_probe(struct i2c_client *client)
> &flash->max_flash_timeout);
> flash->max_flash_timeout /= 1000;
>
> + rval = regulator_enable(flash->vin_supply);
> + if (rval < 0)
> + return dev_err_probe(flash->dev, rval,
> + "failed to enable vin power supply\n");
> +
> + gpiod_set_value_cansleep(flash->hwen_gpio, 1);
> +
> rval = lm3560_init_device(flash);
> if (rval < 0)
> - return rval;
> + goto error_disable;
> +
> + pm_runtime_set_active(flash->dev);
> + pm_runtime_enable(flash->dev);
>
> for_each_available_child_of_node(dev_of_node(flash->dev), node) {
> u32 reg;
> @@ -492,10 +565,10 @@ static int lm3560_probe(struct i2c_client *client)
>
> rval = lm3560_subdev_init(flash, reg, node);
> if (rval < 0) {
> - lm3560_subdev_cleanup(flash);
> - return dev_err_probe(flash->dev, rval,
> - "failed to register led%d\n",
> - reg);
> + dev_err(flash->dev,
> + "failed to register led%d: %d\n",
> + reg, rval);
> + goto error_clean;
> }
>
> set_bit(reg, flash->led_id);
> @@ -504,7 +577,23 @@ static int lm3560_probe(struct i2c_client *client)
>
> i2c_set_clientdata(client, flash);
>
> + pm_runtime_set_autosuspend_delay(flash->dev, 1000);
> + pm_runtime_use_autosuspend(flash->dev);
> + pm_runtime_idle(flash->dev);
> +
> return 0;
> +
> +error_clean:
> + pm_runtime_disable(flash->dev);
> + pm_runtime_set_suspended(flash->dev);
> +
> + lm3560_subdev_cleanup(flash);
> +
> +error_disable:
> + gpiod_set_value_cansleep(flash->hwen_gpio, 0);
> + regulator_disable(flash->vin_supply);
> +
> + return rval;
> }
>
> static void lm3560_remove(struct i2c_client *client)
> @@ -512,8 +601,22 @@ static void lm3560_remove(struct i2c_client *client)
> struct lm3560_flash *flash = i2c_get_clientdata(client);
>
> lm3560_subdev_cleanup(flash);
> +
> + /*
> + * Disable runtime PM. In case runtime PM is disabled in the kernel,
> + * make sure to turn power off manually.
> + */
> + pm_runtime_disable(&client->dev);
> + if (!pm_runtime_status_suspended(&client->dev)) {
> + lm3560_power_off(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> + }
> }
>
> +static const struct dev_pm_ops lm3560_pm_ops = {
> + SET_RUNTIME_PM_OPS(lm3560_power_off, lm3560_power_on, NULL)
> +};
> +
> static const struct of_device_id lm3560_of_match[] = {
> { .compatible = "ti,lm3559" },
> { .compatible = "ti,lm3560" },
> @@ -532,7 +635,7 @@ MODULE_DEVICE_TABLE(i2c, lm3560_id_table);
> static struct i2c_driver lm3560_i2c_driver = {
> .driver = {
> .name = LM3560_NAME,
> - .pm = NULL,
> + .pm = pm_ptr(&lm3560_pm_ops),
> .of_match_table = lm3560_of_match,
> },
> .probe = lm3560_probe,
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 5/6] media: i2c: lm3560: Add support for PM features
2026-05-04 6:37 ` Sakari Ailus
@ 2026-05-04 7:40 ` Svyatoslav Ryhel
2026-05-04 9:37 ` Svyatoslav Ryhel
0 siblings, 1 reply; 20+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-04 7:40 UTC (permalink / raw)
To: Sakari Ailus
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Mauro Carvalho Chehab, linux-leds, devicetree,
linux-kernel, linux-media
пн, 4 трав. 2026 р. о 09:37 Sakari Ailus <sakari.ailus@linux.intel.com> пише:
>
> Hi Svyatoslav,
>
> On Sun, May 03, 2026 at 07:44:44PM +0300, Svyatoslav Ryhel wrote:
> > Add support for power management features to better control the LM3560
> > within the media framework. To achieve the desired PM support, the HWEN
> > GPIO and VIN power supply were added and configured into power on/off
> > sequences. Added PM operations along with the PM configuration setup.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> > drivers/media/i2c/lm3560.c | 117 ++++++++++++++++++++++++++++++++++---
> > 1 file changed, 110 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
> > index ce4b09d1f208..15741ea5684f 100644
> > --- a/drivers/media/i2c/lm3560.c
> > +++ b/drivers/media/i2c/lm3560.c
> > @@ -12,13 +12,16 @@
> > #include <linux/bitmap.h>
> > #include <linux/delay.h>
> > #include <linux/module.h>
> > +#include <linux/gpio/consumer.h>
> > #include <linux/i2c.h>
> > #include <linux/slab.h>
> > #include <linux/mod_devicetable.h>
> > #include <linux/mutex.h>
> > #include <linux/of.h>
> > +#include <linux/pm_runtime.h>
> > #include <linux/property.h>
> > #include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> > #include <linux/videodev2.h>
> > #include <media/i2c/lm3560.h>
> > #include <media/v4l2-ctrls.h>
> > @@ -49,6 +52,8 @@ enum led_enable {
> > * @dev: pointer to &struct device
> > * @regmap: reg. map for i2c
> > * @lock: muxtex for serial access.
> > + * @hwen_gpio: line connected to HWEN pin
> > + * @vin_supply: line connected to IN supply (2.5V - 5.5V)
> > * @led_mode: V4L2 LED mode
> > * @ctrls_led: V4L2 controls
> > * @subdev_led: V4L2 subdev
> > @@ -63,6 +68,9 @@ struct lm3560_flash {
> > struct regmap *regmap;
> > struct mutex lock;
> >
> > + struct gpio_desc *hwen_gpio;
> > + struct regulator *vin_supply;
> > +
> > enum v4l2_flash_led_mode led_mode;
> > struct v4l2_ctrl_handler ctrls_led[LM3560_LED_MAX];
> > struct v4l2_subdev subdev_led[LM3560_LED_MAX];
> > @@ -177,12 +185,17 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > struct lm3560_flash *flash = to_lm3560_flash(ctrl, led_no);
> > int rval = -EINVAL;
> >
> > + if (!pm_runtime_get_if_in_use(flash->dev))
> > + return 0;
> > +
> > if (ctrl->id == V4L2_CID_FLASH_FAULT) {
> > s32 fault = 0;
> > unsigned int reg_val;
> > rval = regmap_read(flash->regmap, REG_FLAG, ®_val);
> > - if (rval < 0)
> > + if (rval < 0) {
> > + pm_runtime_put(flash->dev);
> > return rval;
> > + }
> > if (reg_val & FAULT_SHORT_CIRCUIT)
> > fault |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
> > if (reg_val & FAULT_OVERTEMP)
> > @@ -192,6 +205,8 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > ctrl->cur.val = fault;
> > }
> >
> > + pm_runtime_put(flash->dev);
> > +
> > return rval;
> > }
> >
> > @@ -201,6 +216,9 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > u8 tout_bits;
> > int rval = -EINVAL;
> >
> > + if (!pm_runtime_get_if_in_use(flash->dev))
>
> This should be pm_runtime_get_if_active().
>
Noted
> > + return 0;
> > +
> > switch (ctrl->id) {
> > case V4L2_CID_FLASH_LED_MODE:
> > flash->led_mode = ctrl->val;
> > @@ -246,6 +264,8 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > break;
> > }
> >
> > + pm_runtime_put(flash->dev);
> > +
> > return rval;
> > }
> >
> > @@ -409,6 +429,38 @@ static int lm3560_init_device(struct lm3560_flash *flash)
> > return rval;
> > }
> >
> > +static int __maybe_unused lm3560_power_off(struct device *dev)
> > +{
> > + struct lm3560_flash *flash = dev_get_drvdata(dev);
> > +
> > + gpiod_set_value_cansleep(flash->hwen_gpio, 0);
> > + regulator_disable(flash->vin_supply);
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused lm3560_power_on(struct device *dev)
> > +{
> > + struct lm3560_flash *flash = dev_get_drvdata(dev);
> > + int rval;
> > +
> > + rval = regulator_enable(flash->vin_supply);
> > + if (rval < 0) {
> > + dev_err(flash->dev, "failed to enable vin power supply\n");
> > + return rval;
> > + }
> > +
> > + gpiod_set_value_cansleep(flash->hwen_gpio, 1);
> > +
> > + rval = lm3560_init_device(flash);
> > + if (rval < 0) {
> > + lm3560_power_off(dev);
> > + return rval;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static void lm3560_subdev_cleanup(struct lm3560_flash *flash)
> > {
> > int led_no;
> > @@ -442,6 +494,17 @@ static int lm3560_probe(struct i2c_client *client)
> >
> > bitmap_zero(flash->led_id, LM3560_LED_MAX);
> >
> > + flash->hwen_gpio = devm_gpiod_get_optional(flash->dev, "enable",
> > + GPIOD_OUT_LOW);
> > + if (IS_ERR(flash->hwen_gpio))
> > + return dev_err_probe(flash->dev, PTR_ERR(flash->hwen_gpio),
> > + "failed to get hwen gpio\n");
> > +
> > + flash->vin_supply = devm_regulator_get(flash->dev, "vin");
> > + if (IS_ERR(flash->vin_supply))
> > + return dev_err_probe(flash->dev, PTR_ERR(flash->vin_supply),
> > + "failed to get vin-supply\n");
> > +
> > flash->peak = LM3560_PEAK_1600mA;
> > rval = device_property_read_u32(flash->dev,
> > "ti,peak-current-microamp", &peak_ua);
> > @@ -469,9 +532,19 @@ static int lm3560_probe(struct i2c_client *client)
> > &flash->max_flash_timeout);
> > flash->max_flash_timeout /= 1000;
> >
> > + rval = regulator_enable(flash->vin_supply);
> > + if (rval < 0)
> > + return dev_err_probe(flash->dev, rval,
> > + "failed to enable vin power supply\n");
> > +
> > + gpiod_set_value_cansleep(flash->hwen_gpio, 1);
> > +
> > rval = lm3560_init_device(flash);
> > if (rval < 0)
> > - return rval;
> > + goto error_disable;
> > +
> > + pm_runtime_set_active(flash->dev);
> > + pm_runtime_enable(flash->dev);
> >
> > for_each_available_child_of_node(dev_of_node(flash->dev), node) {
> > u32 reg;
> > @@ -492,10 +565,10 @@ static int lm3560_probe(struct i2c_client *client)
> >
> > rval = lm3560_subdev_init(flash, reg, node);
> > if (rval < 0) {
> > - lm3560_subdev_cleanup(flash);
> > - return dev_err_probe(flash->dev, rval,
> > - "failed to register led%d\n",
> > - reg);
> > + dev_err(flash->dev,
> > + "failed to register led%d: %d\n",
> > + reg, rval);
> > + goto error_clean;
> > }
> >
> > set_bit(reg, flash->led_id);
> > @@ -504,7 +577,23 @@ static int lm3560_probe(struct i2c_client *client)
> >
> > i2c_set_clientdata(client, flash);
> >
> > + pm_runtime_set_autosuspend_delay(flash->dev, 1000);
> > + pm_runtime_use_autosuspend(flash->dev);
> > + pm_runtime_idle(flash->dev);
> > +
> > return 0;
> > +
> > +error_clean:
> > + pm_runtime_disable(flash->dev);
> > + pm_runtime_set_suspended(flash->dev);
> > +
> > + lm3560_subdev_cleanup(flash);
> > +
> > +error_disable:
> > + gpiod_set_value_cansleep(flash->hwen_gpio, 0);
> > + regulator_disable(flash->vin_supply);
> > +
> > + return rval;
> > }
> >
> > static void lm3560_remove(struct i2c_client *client)
> > @@ -512,8 +601,22 @@ static void lm3560_remove(struct i2c_client *client)
> > struct lm3560_flash *flash = i2c_get_clientdata(client);
> >
> > lm3560_subdev_cleanup(flash);
> > +
> > + /*
> > + * Disable runtime PM. In case runtime PM is disabled in the kernel,
> > + * make sure to turn power off manually.
> > + */
> > + pm_runtime_disable(&client->dev);
> > + if (!pm_runtime_status_suspended(&client->dev)) {
> > + lm3560_power_off(&client->dev);
> > + pm_runtime_set_suspended(&client->dev);
> > + }
> > }
> >
> > +static const struct dev_pm_ops lm3560_pm_ops = {
> > + SET_RUNTIME_PM_OPS(lm3560_power_off, lm3560_power_on, NULL)
> > +};
> > +
> > static const struct of_device_id lm3560_of_match[] = {
> > { .compatible = "ti,lm3559" },
> > { .compatible = "ti,lm3560" },
> > @@ -532,7 +635,7 @@ MODULE_DEVICE_TABLE(i2c, lm3560_id_table);
> > static struct i2c_driver lm3560_i2c_driver = {
> > .driver = {
> > .name = LM3560_NAME,
> > - .pm = NULL,
> > + .pm = pm_ptr(&lm3560_pm_ops),
> > .of_match_table = lm3560_of_match,
> > },
> > .probe = lm3560_probe,
>
> --
> Kind regards,
>
> Sakari Ailus
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 5/6] media: i2c: lm3560: Add support for PM features
2026-05-04 7:40 ` Svyatoslav Ryhel
@ 2026-05-04 9:37 ` Svyatoslav Ryhel
2026-05-04 10:14 ` Sakari Ailus
0 siblings, 1 reply; 20+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-04 9:37 UTC (permalink / raw)
To: Sakari Ailus
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Mauro Carvalho Chehab, linux-leds, devicetree,
linux-kernel, linux-media
пн, 4 трав. 2026 р. о 10:40 Svyatoslav Ryhel <clamor95@gmail.com> пише:
>
> пн, 4 трав. 2026 р. о 09:37 Sakari Ailus <sakari.ailus@linux.intel.com> пише:
> >
> > Hi Svyatoslav,
> >
> > On Sun, May 03, 2026 at 07:44:44PM +0300, Svyatoslav Ryhel wrote:
> > > Add support for power management features to better control the LM3560
> > > within the media framework. To achieve the desired PM support, the HWEN
> > > GPIO and VIN power supply were added and configured into power on/off
> > > sequences. Added PM operations along with the PM configuration setup.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > > drivers/media/i2c/lm3560.c | 117 ++++++++++++++++++++++++++++++++++---
> > > 1 file changed, 110 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
> > > index ce4b09d1f208..15741ea5684f 100644
> > > --- a/drivers/media/i2c/lm3560.c
> > > +++ b/drivers/media/i2c/lm3560.c
> > > @@ -12,13 +12,16 @@
> > > #include <linux/bitmap.h>
> > > #include <linux/delay.h>
> > > #include <linux/module.h>
> > > +#include <linux/gpio/consumer.h>
> > > #include <linux/i2c.h>
> > > #include <linux/slab.h>
> > > #include <linux/mod_devicetable.h>
> > > #include <linux/mutex.h>
> > > #include <linux/of.h>
> > > +#include <linux/pm_runtime.h>
> > > #include <linux/property.h>
> > > #include <linux/regmap.h>
> > > +#include <linux/regulator/consumer.h>
> > > #include <linux/videodev2.h>
> > > #include <media/i2c/lm3560.h>
> > > #include <media/v4l2-ctrls.h>
> > > @@ -49,6 +52,8 @@ enum led_enable {
> > > * @dev: pointer to &struct device
> > > * @regmap: reg. map for i2c
> > > * @lock: muxtex for serial access.
> > > + * @hwen_gpio: line connected to HWEN pin
> > > + * @vin_supply: line connected to IN supply (2.5V - 5.5V)
> > > * @led_mode: V4L2 LED mode
> > > * @ctrls_led: V4L2 controls
> > > * @subdev_led: V4L2 subdev
> > > @@ -63,6 +68,9 @@ struct lm3560_flash {
> > > struct regmap *regmap;
> > > struct mutex lock;
> > >
> > > + struct gpio_desc *hwen_gpio;
> > > + struct regulator *vin_supply;
> > > +
> > > enum v4l2_flash_led_mode led_mode;
> > > struct v4l2_ctrl_handler ctrls_led[LM3560_LED_MAX];
> > > struct v4l2_subdev subdev_led[LM3560_LED_MAX];
> > > @@ -177,12 +185,17 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > > struct lm3560_flash *flash = to_lm3560_flash(ctrl, led_no);
> > > int rval = -EINVAL;
> > >
> > > + if (!pm_runtime_get_if_in_use(flash->dev))
> > > + return 0;
> > > +
> > > if (ctrl->id == V4L2_CID_FLASH_FAULT) {
> > > s32 fault = 0;
> > > unsigned int reg_val;
> > > rval = regmap_read(flash->regmap, REG_FLAG, ®_val);
> > > - if (rval < 0)
> > > + if (rval < 0) {
> > > + pm_runtime_put(flash->dev);
> > > return rval;
> > > + }
> > > if (reg_val & FAULT_SHORT_CIRCUIT)
> > > fault |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
> > > if (reg_val & FAULT_OVERTEMP)
> > > @@ -192,6 +205,8 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > > ctrl->cur.val = fault;
> > > }
> > >
> > > + pm_runtime_put(flash->dev);
> > > +
> > > return rval;
> > > }
> > >
> > > @@ -201,6 +216,9 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > > u8 tout_bits;
> > > int rval = -EINVAL;
> > >
> > > + if (!pm_runtime_get_if_in_use(flash->dev))
> >
> > This should be pm_runtime_get_if_active().
> >
>
For both lm3560_set_ctrl and lm3560_get_ctrl or only for lm3560_set_ctrl?
> Noted
>
> > > + return 0;
> > > +
> > > switch (ctrl->id) {
> > > case V4L2_CID_FLASH_LED_MODE:
> > > flash->led_mode = ctrl->val;
> > > @@ -246,6 +264,8 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > > break;
> > > }
> > >
> > > + pm_runtime_put(flash->dev);
> > > +
> > > return rval;
> > > }
> > >
> > > @@ -409,6 +429,38 @@ static int lm3560_init_device(struct lm3560_flash *flash)
> > > return rval;
> > > }
> > >
> > > +static int __maybe_unused lm3560_power_off(struct device *dev)
> > > +{
> > > + struct lm3560_flash *flash = dev_get_drvdata(dev);
> > > +
> > > + gpiod_set_value_cansleep(flash->hwen_gpio, 0);
> > > + regulator_disable(flash->vin_supply);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int __maybe_unused lm3560_power_on(struct device *dev)
> > > +{
> > > + struct lm3560_flash *flash = dev_get_drvdata(dev);
> > > + int rval;
> > > +
> > > + rval = regulator_enable(flash->vin_supply);
> > > + if (rval < 0) {
> > > + dev_err(flash->dev, "failed to enable vin power supply\n");
> > > + return rval;
> > > + }
> > > +
> > > + gpiod_set_value_cansleep(flash->hwen_gpio, 1);
> > > +
> > > + rval = lm3560_init_device(flash);
> > > + if (rval < 0) {
> > > + lm3560_power_off(dev);
> > > + return rval;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static void lm3560_subdev_cleanup(struct lm3560_flash *flash)
> > > {
> > > int led_no;
> > > @@ -442,6 +494,17 @@ static int lm3560_probe(struct i2c_client *client)
> > >
> > > bitmap_zero(flash->led_id, LM3560_LED_MAX);
> > >
> > > + flash->hwen_gpio = devm_gpiod_get_optional(flash->dev, "enable",
> > > + GPIOD_OUT_LOW);
> > > + if (IS_ERR(flash->hwen_gpio))
> > > + return dev_err_probe(flash->dev, PTR_ERR(flash->hwen_gpio),
> > > + "failed to get hwen gpio\n");
> > > +
> > > + flash->vin_supply = devm_regulator_get(flash->dev, "vin");
> > > + if (IS_ERR(flash->vin_supply))
> > > + return dev_err_probe(flash->dev, PTR_ERR(flash->vin_supply),
> > > + "failed to get vin-supply\n");
> > > +
> > > flash->peak = LM3560_PEAK_1600mA;
> > > rval = device_property_read_u32(flash->dev,
> > > "ti,peak-current-microamp", &peak_ua);
> > > @@ -469,9 +532,19 @@ static int lm3560_probe(struct i2c_client *client)
> > > &flash->max_flash_timeout);
> > > flash->max_flash_timeout /= 1000;
> > >
> > > + rval = regulator_enable(flash->vin_supply);
> > > + if (rval < 0)
> > > + return dev_err_probe(flash->dev, rval,
> > > + "failed to enable vin power supply\n");
> > > +
> > > + gpiod_set_value_cansleep(flash->hwen_gpio, 1);
> > > +
> > > rval = lm3560_init_device(flash);
> > > if (rval < 0)
> > > - return rval;
> > > + goto error_disable;
> > > +
> > > + pm_runtime_set_active(flash->dev);
> > > + pm_runtime_enable(flash->dev);
> > >
> > > for_each_available_child_of_node(dev_of_node(flash->dev), node) {
> > > u32 reg;
> > > @@ -492,10 +565,10 @@ static int lm3560_probe(struct i2c_client *client)
> > >
> > > rval = lm3560_subdev_init(flash, reg, node);
> > > if (rval < 0) {
> > > - lm3560_subdev_cleanup(flash);
> > > - return dev_err_probe(flash->dev, rval,
> > > - "failed to register led%d\n",
> > > - reg);
> > > + dev_err(flash->dev,
> > > + "failed to register led%d: %d\n",
> > > + reg, rval);
> > > + goto error_clean;
> > > }
> > >
> > > set_bit(reg, flash->led_id);
> > > @@ -504,7 +577,23 @@ static int lm3560_probe(struct i2c_client *client)
> > >
> > > i2c_set_clientdata(client, flash);
> > >
> > > + pm_runtime_set_autosuspend_delay(flash->dev, 1000);
> > > + pm_runtime_use_autosuspend(flash->dev);
> > > + pm_runtime_idle(flash->dev);
> > > +
> > > return 0;
> > > +
> > > +error_clean:
> > > + pm_runtime_disable(flash->dev);
> > > + pm_runtime_set_suspended(flash->dev);
> > > +
> > > + lm3560_subdev_cleanup(flash);
> > > +
> > > +error_disable:
> > > + gpiod_set_value_cansleep(flash->hwen_gpio, 0);
> > > + regulator_disable(flash->vin_supply);
> > > +
> > > + return rval;
> > > }
> > >
> > > static void lm3560_remove(struct i2c_client *client)
> > > @@ -512,8 +601,22 @@ static void lm3560_remove(struct i2c_client *client)
> > > struct lm3560_flash *flash = i2c_get_clientdata(client);
> > >
> > > lm3560_subdev_cleanup(flash);
> > > +
> > > + /*
> > > + * Disable runtime PM. In case runtime PM is disabled in the kernel,
> > > + * make sure to turn power off manually.
> > > + */
> > > + pm_runtime_disable(&client->dev);
> > > + if (!pm_runtime_status_suspended(&client->dev)) {
> > > + lm3560_power_off(&client->dev);
> > > + pm_runtime_set_suspended(&client->dev);
> > > + }
> > > }
> > >
> > > +static const struct dev_pm_ops lm3560_pm_ops = {
> > > + SET_RUNTIME_PM_OPS(lm3560_power_off, lm3560_power_on, NULL)
> > > +};
> > > +
> > > static const struct of_device_id lm3560_of_match[] = {
> > > { .compatible = "ti,lm3559" },
> > > { .compatible = "ti,lm3560" },
> > > @@ -532,7 +635,7 @@ MODULE_DEVICE_TABLE(i2c, lm3560_id_table);
> > > static struct i2c_driver lm3560_i2c_driver = {
> > > .driver = {
> > > .name = LM3560_NAME,
> > > - .pm = NULL,
> > > + .pm = pm_ptr(&lm3560_pm_ops),
> > > .of_match_table = lm3560_of_match,
> > > },
> > > .probe = lm3560_probe,
> >
> > --
> > Kind regards,
> >
> > Sakari Ailus
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 5/6] media: i2c: lm3560: Add support for PM features
2026-05-04 9:37 ` Svyatoslav Ryhel
@ 2026-05-04 10:14 ` Sakari Ailus
0 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2026-05-04 10:14 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Mauro Carvalho Chehab, linux-leds, devicetree,
linux-kernel, linux-media
On Mon, May 04, 2026 at 12:37:40PM +0300, Svyatoslav Ryhel wrote:
> пн, 4 трав. 2026 р. о 10:40 Svyatoslav Ryhel <clamor95@gmail.com> пише:
> >
> > пн, 4 трав. 2026 р. о 09:37 Sakari Ailus <sakari.ailus@linux.intel.com> пише:
> > >
> > > Hi Svyatoslav,
> > >
> > > On Sun, May 03, 2026 at 07:44:44PM +0300, Svyatoslav Ryhel wrote:
> > > > Add support for power management features to better control the LM3560
> > > > within the media framework. To achieve the desired PM support, the HWEN
> > > > GPIO and VIN power supply were added and configured into power on/off
> > > > sequences. Added PM operations along with the PM configuration setup.
> > > >
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > ---
> > > > drivers/media/i2c/lm3560.c | 117 ++++++++++++++++++++++++++++++++++---
> > > > 1 file changed, 110 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
> > > > index ce4b09d1f208..15741ea5684f 100644
> > > > --- a/drivers/media/i2c/lm3560.c
> > > > +++ b/drivers/media/i2c/lm3560.c
> > > > @@ -12,13 +12,16 @@
> > > > #include <linux/bitmap.h>
> > > > #include <linux/delay.h>
> > > > #include <linux/module.h>
> > > > +#include <linux/gpio/consumer.h>
> > > > #include <linux/i2c.h>
> > > > #include <linux/slab.h>
> > > > #include <linux/mod_devicetable.h>
> > > > #include <linux/mutex.h>
> > > > #include <linux/of.h>
> > > > +#include <linux/pm_runtime.h>
> > > > #include <linux/property.h>
> > > > #include <linux/regmap.h>
> > > > +#include <linux/regulator/consumer.h>
> > > > #include <linux/videodev2.h>
> > > > #include <media/i2c/lm3560.h>
> > > > #include <media/v4l2-ctrls.h>
> > > > @@ -49,6 +52,8 @@ enum led_enable {
> > > > * @dev: pointer to &struct device
> > > > * @regmap: reg. map for i2c
> > > > * @lock: muxtex for serial access.
> > > > + * @hwen_gpio: line connected to HWEN pin
> > > > + * @vin_supply: line connected to IN supply (2.5V - 5.5V)
> > > > * @led_mode: V4L2 LED mode
> > > > * @ctrls_led: V4L2 controls
> > > > * @subdev_led: V4L2 subdev
> > > > @@ -63,6 +68,9 @@ struct lm3560_flash {
> > > > struct regmap *regmap;
> > > > struct mutex lock;
> > > >
> > > > + struct gpio_desc *hwen_gpio;
> > > > + struct regulator *vin_supply;
> > > > +
> > > > enum v4l2_flash_led_mode led_mode;
> > > > struct v4l2_ctrl_handler ctrls_led[LM3560_LED_MAX];
> > > > struct v4l2_subdev subdev_led[LM3560_LED_MAX];
> > > > @@ -177,12 +185,17 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > > > struct lm3560_flash *flash = to_lm3560_flash(ctrl, led_no);
> > > > int rval = -EINVAL;
> > > >
> > > > + if (!pm_runtime_get_if_in_use(flash->dev))
> > > > + return 0;
> > > > +
> > > > if (ctrl->id == V4L2_CID_FLASH_FAULT) {
> > > > s32 fault = 0;
> > > > unsigned int reg_val;
> > > > rval = regmap_read(flash->regmap, REG_FLAG, ®_val);
> > > > - if (rval < 0)
> > > > + if (rval < 0) {
> > > > + pm_runtime_put(flash->dev);
> > > > return rval;
> > > > + }
> > > > if (reg_val & FAULT_SHORT_CIRCUIT)
> > > > fault |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
> > > > if (reg_val & FAULT_OVERTEMP)
> > > > @@ -192,6 +205,8 @@ static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > > > ctrl->cur.val = fault;
> > > > }
> > > >
> > > > + pm_runtime_put(flash->dev);
> > > > +
> > > > return rval;
> > > > }
> > > >
> > > > @@ -201,6 +216,9 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > > > u8 tout_bits;
> > > > int rval = -EINVAL;
> > > >
> > > > + if (!pm_runtime_get_if_in_use(flash->dev))
> > >
> > > This should be pm_runtime_get_if_active().
> > >
> >
>
> For both lm3560_set_ctrl and lm3560_get_ctrl or only for lm3560_set_ctrl?
For both.
It doesn't matter much at this point as the usage_count is incremented for
each file handle.
>
> > Noted
> >
> > > > + return 0;
> > > > +
> > > > switch (ctrl->id) {
> > > > case V4L2_CID_FLASH_LED_MODE:
> > > > flash->led_mode = ctrl->val;
> > > > @@ -246,6 +264,8 @@ static int lm3560_set_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
> > > > break;
> > > > }
> > > >
> > > > + pm_runtime_put(flash->dev);
> > > > +
> > > > return rval;
> > > > }
> > > >
> > > > @@ -409,6 +429,38 @@ static int lm3560_init_device(struct lm3560_flash *flash)
> > > > return rval;
> > > > }
> > > >
> > > > +static int __maybe_unused lm3560_power_off(struct device *dev)
> > > > +{
> > > > + struct lm3560_flash *flash = dev_get_drvdata(dev);
> > > > +
> > > > + gpiod_set_value_cansleep(flash->hwen_gpio, 0);
> > > > + regulator_disable(flash->vin_supply);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int __maybe_unused lm3560_power_on(struct device *dev)
> > > > +{
> > > > + struct lm3560_flash *flash = dev_get_drvdata(dev);
> > > > + int rval;
> > > > +
> > > > + rval = regulator_enable(flash->vin_supply);
> > > > + if (rval < 0) {
> > > > + dev_err(flash->dev, "failed to enable vin power supply\n");
> > > > + return rval;
> > > > + }
> > > > +
> > > > + gpiod_set_value_cansleep(flash->hwen_gpio, 1);
> > > > +
> > > > + rval = lm3560_init_device(flash);
> > > > + if (rval < 0) {
> > > > + lm3560_power_off(dev);
> > > > + return rval;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > static void lm3560_subdev_cleanup(struct lm3560_flash *flash)
> > > > {
> > > > int led_no;
> > > > @@ -442,6 +494,17 @@ static int lm3560_probe(struct i2c_client *client)
> > > >
> > > > bitmap_zero(flash->led_id, LM3560_LED_MAX);
> > > >
> > > > + flash->hwen_gpio = devm_gpiod_get_optional(flash->dev, "enable",
> > > > + GPIOD_OUT_LOW);
> > > > + if (IS_ERR(flash->hwen_gpio))
> > > > + return dev_err_probe(flash->dev, PTR_ERR(flash->hwen_gpio),
> > > > + "failed to get hwen gpio\n");
> > > > +
> > > > + flash->vin_supply = devm_regulator_get(flash->dev, "vin");
> > > > + if (IS_ERR(flash->vin_supply))
> > > > + return dev_err_probe(flash->dev, PTR_ERR(flash->vin_supply),
> > > > + "failed to get vin-supply\n");
> > > > +
> > > > flash->peak = LM3560_PEAK_1600mA;
> > > > rval = device_property_read_u32(flash->dev,
> > > > "ti,peak-current-microamp", &peak_ua);
> > > > @@ -469,9 +532,19 @@ static int lm3560_probe(struct i2c_client *client)
> > > > &flash->max_flash_timeout);
> > > > flash->max_flash_timeout /= 1000;
> > > >
> > > > + rval = regulator_enable(flash->vin_supply);
> > > > + if (rval < 0)
> > > > + return dev_err_probe(flash->dev, rval,
> > > > + "failed to enable vin power supply\n");
> > > > +
> > > > + gpiod_set_value_cansleep(flash->hwen_gpio, 1);
> > > > +
> > > > rval = lm3560_init_device(flash);
> > > > if (rval < 0)
> > > > - return rval;
> > > > + goto error_disable;
> > > > +
> > > > + pm_runtime_set_active(flash->dev);
> > > > + pm_runtime_enable(flash->dev);
> > > >
> > > > for_each_available_child_of_node(dev_of_node(flash->dev), node) {
> > > > u32 reg;
> > > > @@ -492,10 +565,10 @@ static int lm3560_probe(struct i2c_client *client)
> > > >
> > > > rval = lm3560_subdev_init(flash, reg, node);
> > > > if (rval < 0) {
> > > > - lm3560_subdev_cleanup(flash);
> > > > - return dev_err_probe(flash->dev, rval,
> > > > - "failed to register led%d\n",
> > > > - reg);
> > > > + dev_err(flash->dev,
> > > > + "failed to register led%d: %d\n",
> > > > + reg, rval);
> > > > + goto error_clean;
> > > > }
> > > >
> > > > set_bit(reg, flash->led_id);
> > > > @@ -504,7 +577,23 @@ static int lm3560_probe(struct i2c_client *client)
> > > >
> > > > i2c_set_clientdata(client, flash);
> > > >
> > > > + pm_runtime_set_autosuspend_delay(flash->dev, 1000);
> > > > + pm_runtime_use_autosuspend(flash->dev);
> > > > + pm_runtime_idle(flash->dev);
> > > > +
> > > > return 0;
> > > > +
> > > > +error_clean:
> > > > + pm_runtime_disable(flash->dev);
> > > > + pm_runtime_set_suspended(flash->dev);
> > > > +
> > > > + lm3560_subdev_cleanup(flash);
> > > > +
> > > > +error_disable:
> > > > + gpiod_set_value_cansleep(flash->hwen_gpio, 0);
> > > > + regulator_disable(flash->vin_supply);
> > > > +
> > > > + return rval;
> > > > }
> > > >
> > > > static void lm3560_remove(struct i2c_client *client)
> > > > @@ -512,8 +601,22 @@ static void lm3560_remove(struct i2c_client *client)
> > > > struct lm3560_flash *flash = i2c_get_clientdata(client);
> > > >
> > > > lm3560_subdev_cleanup(flash);
> > > > +
> > > > + /*
> > > > + * Disable runtime PM. In case runtime PM is disabled in the kernel,
> > > > + * make sure to turn power off manually.
> > > > + */
> > > > + pm_runtime_disable(&client->dev);
> > > > + if (!pm_runtime_status_suspended(&client->dev)) {
> > > > + lm3560_power_off(&client->dev);
> > > > + pm_runtime_set_suspended(&client->dev);
> > > > + }
> > > > }
> > > >
> > > > +static const struct dev_pm_ops lm3560_pm_ops = {
> > > > + SET_RUNTIME_PM_OPS(lm3560_power_off, lm3560_power_on, NULL)
> > > > +};
> > > > +
> > > > static const struct of_device_id lm3560_of_match[] = {
> > > > { .compatible = "ti,lm3559" },
> > > > { .compatible = "ti,lm3560" },
> > > > @@ -532,7 +635,7 @@ MODULE_DEVICE_TABLE(i2c, lm3560_id_table);
> > > > static struct i2c_driver lm3560_i2c_driver = {
> > > > .driver = {
> > > > .name = LM3560_NAME,
> > > > - .pm = NULL,
> > > > + .pm = pm_ptr(&lm3560_pm_ops),
> > > > .of_match_table = lm3560_of_match,
> > > > },
> > > > .probe = lm3560_probe,
> > >
> > > --
> > > Kind regards,
> > >
> > > Sakari Ailus
--
Sakari Ailus
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 6/6] media: i2c: lm3560: Add proper support for LM3559
2026-05-03 16:44 [PATCH v5 0/6] media: lm3560: convert to use OF bindings Svyatoslav Ryhel
` (4 preceding siblings ...)
2026-05-03 16:44 ` [PATCH v5 5/6] media: i2c: lm3560: Add support for PM features Svyatoslav Ryhel
@ 2026-05-03 16:44 ` Svyatoslav Ryhel
2026-05-04 5:35 ` [PATCH v5 0/6] media: lm3560: convert to use OF bindings Svyatoslav Ryhel
6 siblings, 0 replies; 20+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-03 16:44 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sakari Ailus, Mauro Carvalho Chehab,
Svyatoslav Ryhel
Cc: linux-leds, devicetree, linux-kernel, linux-media
The LM3559 is very similar to the LM3560, but it operates at much lower
currents. This may result in incorrect current selection if LM3560 ranges
are applied to the LM3559. Implement driver data matching to use
device-specific current configurations.
Since the driver no longer supports platform data and device configuration
is performed more granularly, move the remaining enums from the header
into the driver file and remove the header.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/media/i2c/lm3560.c | 120 ++++++++++++++++++++++++++++++-------
include/media/i2c/lm3560.h | 69 ---------------------
2 files changed, 100 insertions(+), 89 deletions(-)
delete mode 100644 include/media/i2c/lm3560.h
diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
index 15741ea5684f..da12fbd7dc59 100644
--- a/drivers/media/i2c/lm3560.c
+++ b/drivers/media/i2c/lm3560.c
@@ -23,9 +23,9 @@
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/videodev2.h>
-#include <media/i2c/lm3560.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
+#include <media/v4l2-subdev.h>
/* registers definitions */
#define REG_ENABLE 0x10
@@ -40,12 +40,44 @@
#define FAULT_OVERTEMP (1<<1)
#define FAULT_SHORT_CIRCUIT (1<<2)
+#define LM3560_FLASH_TOUT_MIN 32
+#define LM3560_FLASH_TOUT_STEP 32
+#define LM3560_FLASH_TOUT_MAX 1024
+#define LM3560_FLASH_TOUT_ms_TO_REG(a) \
+ ((a) < LM3560_FLASH_TOUT_MIN ? 0 : \
+ (((a) - LM3560_FLASH_TOUT_MIN) / LM3560_FLASH_TOUT_STEP))
+#define LM3560_FLASH_TOUT_REG_TO_ms(a) \
+ ((a) * LM3560_FLASH_TOUT_STEP + LM3560_FLASH_TOUT_MIN)
+
+enum lm3560_led_id {
+ LM3560_LED0 = 0,
+ LM3560_LED1,
+ LM3560_LED_MAX
+};
+
+enum lm3560_peak_current {
+ LM3560_PEAK_1600mA = 0x00,
+ LM3560_PEAK_2300mA = 0x20,
+ LM3560_PEAK_3000mA = 0x40,
+ LM3560_PEAK_3600mA = 0x60
+};
+
enum led_enable {
MODE_SHDN = 0x0,
MODE_TORCH = 0x2,
MODE_FLASH = 0x3,
};
+struct lm3560_flash_config {
+ u32 flash_brt_min_ua;
+ u32 flash_brt_max_ua;
+ u32 flash_brt_step_ua;
+
+ u32 torch_brt_min_ua;
+ u32 torch_brt_max_ua;
+ u32 torch_brt_step_ua;
+};
+
/**
* struct lm3560_flash
*
@@ -62,6 +94,7 @@ enum led_enable {
* @max_flash_timeout: flash timeout
* @max_flash_brt: flash mode led brightness
* @max_torch_brt: torch mode led brightness
+ * @config: device specific current configuration
*/
struct lm3560_flash {
struct device *dev;
@@ -82,6 +115,8 @@ struct lm3560_flash {
u32 max_flash_brt[LM3560_LED_MAX];
u32 max_torch_brt[LM3560_LED_MAX];
+
+ const struct lm3560_flash_config *config;
};
#define to_lm3560_flash(_ctrl, _no) \
@@ -137,15 +172,20 @@ static int lm3560_enable_ctrl(struct lm3560_flash *flash,
static int lm3560_torch_brt_ctrl(struct lm3560_flash *flash,
enum lm3560_led_id led_no, unsigned int brt)
{
+ const struct lm3560_flash_config *config = flash->config;
int rval;
- u8 br_bits;
+ u32 br_bits;
- if (brt < LM3560_TORCH_BRT_MIN)
+ if (brt < config->torch_brt_min_ua)
return lm3560_enable_ctrl(flash, led_no, false);
else
rval = lm3560_enable_ctrl(flash, led_no, true);
- br_bits = LM3560_TORCH_BRT_uA_TO_REG(brt);
+ br_bits = clamp(brt, config->torch_brt_min_ua,
+ config->torch_brt_max_ua);
+ br_bits = (br_bits - config->torch_brt_min_ua) /
+ config->torch_brt_step_ua;
+
if (led_no == LM3560_LED0)
rval = regmap_update_bits(flash->regmap,
REG_TORCH_BR, 0x07, br_bits);
@@ -160,15 +200,20 @@ static int lm3560_torch_brt_ctrl(struct lm3560_flash *flash,
static int lm3560_flash_brt_ctrl(struct lm3560_flash *flash,
enum lm3560_led_id led_no, unsigned int brt)
{
+ const struct lm3560_flash_config *config = flash->config;
int rval;
- u8 br_bits;
+ u32 br_bits;
- if (brt < LM3560_FLASH_BRT_MIN)
+ if (brt < config->flash_brt_min_ua)
return lm3560_enable_ctrl(flash, led_no, false);
else
rval = lm3560_enable_ctrl(flash, led_no, true);
- br_bits = LM3560_FLASH_BRT_uA_TO_REG(brt);
+ br_bits = clamp(brt, config->flash_brt_min_ua,
+ config->flash_brt_max_ua);
+ br_bits = (br_bits - config->flash_brt_min_ua) /
+ config->flash_brt_step_ua;
+
if (led_no == LM3560_LED0)
rval = regmap_update_bits(flash->regmap,
REG_FLASH_BR, 0x0f, br_bits);
@@ -308,6 +353,7 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
u32 max_torch_brt = flash->max_torch_brt[led_no];
struct v4l2_ctrl_handler *hdl = &flash->ctrls_led[led_no];
const struct v4l2_ctrl_ops *ops = &lm3560_led_ctrl_ops[led_no];
+ const struct lm3560_flash_config *config = flash->config;
v4l2_ctrl_handler_init(hdl, 8);
@@ -336,13 +382,13 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
/* flash brt */
v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_INTENSITY,
- LM3560_FLASH_BRT_MIN, max_flash_brt,
- LM3560_FLASH_BRT_STEP, max_flash_brt);
+ config->flash_brt_min_ua, max_flash_brt,
+ config->flash_brt_step_ua, max_flash_brt);
/* torch brt */
v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_TORCH_INTENSITY,
- LM3560_TORCH_BRT_MIN, max_torch_brt,
- LM3560_TORCH_BRT_STEP, max_torch_brt);
+ config->torch_brt_min_ua, max_torch_brt,
+ config->torch_brt_step_ua, max_torch_brt);
/* fault */
fault = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_FAULT, 0,
@@ -483,6 +529,10 @@ static int lm3560_probe(struct i2c_client *client)
if (flash == NULL)
return -ENOMEM;
+ flash->config = device_get_match_data(&client->dev);
+ if (!flash->config)
+ return -ENODEV;
+
flash->regmap = devm_regmap_init_i2c(client, &lm3560_regmap);
if (IS_ERR(flash->regmap)) {
rval = PTR_ERR(flash->regmap);
@@ -509,16 +559,26 @@ static int lm3560_probe(struct i2c_client *client)
rval = device_property_read_u32(flash->dev,
"ti,peak-current-microamp", &peak_ua);
if (!rval) {
+ /*
+ * LM3559 has lower peak current limit, but
+ * bit configuration matches LM3560.
+ * Correct current restrictions are enforced
+ * by the LM3560 schema.
+ */
switch (peak_ua) {
+ case 1400000:
case 1600000:
flash->peak = LM3560_PEAK_1600mA;
break;
+ case 2100000:
case 2300000:
flash->peak = LM3560_PEAK_2300mA;
break;
+ case 2700000:
case 3000000:
flash->peak = LM3560_PEAK_3000mA;
break;
+ case 3200000:
case 3600000:
flash->peak = LM3560_PEAK_3600mA;
break;
@@ -547,6 +607,7 @@ static int lm3560_probe(struct i2c_client *client)
pm_runtime_enable(flash->dev);
for_each_available_child_of_node(dev_of_node(flash->dev), node) {
+ const struct lm3560_flash_config *config = flash->config;
u32 reg;
rval = of_property_read_u32(node, "reg", ®);
@@ -555,11 +616,11 @@ static int lm3560_probe(struct i2c_client *client)
continue;
if (reg == LM3560_LED0 || reg == LM3560_LED1) {
- flash->max_flash_brt[reg] = LM3560_FLASH_BRT_MIN;
+ flash->max_flash_brt[reg] = config->flash_brt_min_ua;
of_property_read_u32(node, "flash-max-microamp",
&flash->max_flash_brt[reg]);
- flash->max_torch_brt[reg] = LM3560_TORCH_BRT_MIN;
+ flash->max_torch_brt[reg] = config->torch_brt_min_ua;
of_property_read_u32(node, "led-max-microamp",
&flash->max_torch_brt[reg]);
@@ -617,24 +678,43 @@ static const struct dev_pm_ops lm3560_pm_ops = {
SET_RUNTIME_PM_OPS(lm3560_power_off, lm3560_power_on, NULL)
};
+static const struct lm3560_flash_config lm3559_config = {
+ .flash_brt_min_ua = 56250,
+ .flash_brt_max_ua = 900000,
+ .flash_brt_step_ua = 56250,
+
+ .torch_brt_min_ua = 28125,
+ .torch_brt_max_ua = 225000,
+ .torch_brt_step_ua = 28125,
+};
+
+static const struct lm3560_flash_config lm3560_config = {
+ .flash_brt_min_ua = 62500,
+ .flash_brt_max_ua = 1000000,
+ .flash_brt_step_ua = 62500,
+
+ .torch_brt_min_ua = 31250,
+ .torch_brt_max_ua = 250000,
+ .torch_brt_step_ua = 31250,
+};
+
static const struct of_device_id lm3560_of_match[] = {
- { .compatible = "ti,lm3559" },
- { .compatible = "ti,lm3560" },
+ { .compatible = "ti,lm3559", .data = &lm3559_config },
+ { .compatible = "ti,lm3560", .data = &lm3560_config },
{ }
};
MODULE_DEVICE_TABLE(of, lm3560_of_match);
static const struct i2c_device_id lm3560_id_table[] = {
- { LM3559_NAME },
- { LM3560_NAME },
- {}
+ { "lm3559", (kernel_ulong_t)&lm3559_config },
+ { "lm3560", (kernel_ulong_t)&lm3560_config },
+ { }
};
-
MODULE_DEVICE_TABLE(i2c, lm3560_id_table);
static struct i2c_driver lm3560_i2c_driver = {
.driver = {
- .name = LM3560_NAME,
+ .name = "lm3560",
.pm = pm_ptr(&lm3560_pm_ops),
.of_match_table = lm3560_of_match,
},
diff --git a/include/media/i2c/lm3560.h b/include/media/i2c/lm3560.h
deleted file mode 100644
index b56c1ff8fd49..000000000000
--- a/include/media/i2c/lm3560.h
+++ /dev/null
@@ -1,69 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * include/media/i2c/lm3560.h
- *
- * Copyright (C) 2013 Texas Instruments
- *
- * Contact: Daniel Jeong <gshark.jeong@gmail.com>
- * Ldd-Mlp <ldd-mlp@list.ti.com>
- */
-
-#ifndef __LM3560_H__
-#define __LM3560_H__
-
-#include <media/v4l2-subdev.h>
-
-#define LM3559_NAME "lm3559"
-#define LM3560_NAME "lm3560"
-#define LM3560_I2C_ADDR (0x53)
-
-/* FLASH Brightness
- * min 62500uA, step 62500uA, max 1000000uA
- */
-#define LM3560_FLASH_BRT_MIN 62500
-#define LM3560_FLASH_BRT_STEP 62500
-#define LM3560_FLASH_BRT_MAX 1000000
-#define LM3560_FLASH_BRT_uA_TO_REG(a) \
- ((a) < LM3560_FLASH_BRT_MIN ? 0 : \
- (((a) - LM3560_FLASH_BRT_MIN) / LM3560_FLASH_BRT_STEP))
-#define LM3560_FLASH_BRT_REG_TO_uA(a) \
- ((a) * LM3560_FLASH_BRT_STEP + LM3560_FLASH_BRT_MIN)
-
-/* FLASH TIMEOUT DURATION
- * min 32ms, step 32ms, max 1024ms
- */
-#define LM3560_FLASH_TOUT_MIN 32
-#define LM3560_FLASH_TOUT_STEP 32
-#define LM3560_FLASH_TOUT_MAX 1024
-#define LM3560_FLASH_TOUT_ms_TO_REG(a) \
- ((a) < LM3560_FLASH_TOUT_MIN ? 0 : \
- (((a) - LM3560_FLASH_TOUT_MIN) / LM3560_FLASH_TOUT_STEP))
-#define LM3560_FLASH_TOUT_REG_TO_ms(a) \
- ((a) * LM3560_FLASH_TOUT_STEP + LM3560_FLASH_TOUT_MIN)
-
-/* TORCH BRT
- * min 31250uA, step 31250uA, max 250000uA
- */
-#define LM3560_TORCH_BRT_MIN 31250
-#define LM3560_TORCH_BRT_STEP 31250
-#define LM3560_TORCH_BRT_MAX 250000
-#define LM3560_TORCH_BRT_uA_TO_REG(a) \
- ((a) < LM3560_TORCH_BRT_MIN ? 0 : \
- (((a) - LM3560_TORCH_BRT_MIN) / LM3560_TORCH_BRT_STEP))
-#define LM3560_TORCH_BRT_REG_TO_uA(a) \
- ((a) * LM3560_TORCH_BRT_STEP + LM3560_TORCH_BRT_MIN)
-
-enum lm3560_led_id {
- LM3560_LED0 = 0,
- LM3560_LED1,
- LM3560_LED_MAX
-};
-
-enum lm3560_peak_current {
- LM3560_PEAK_1600mA = 0x00,
- LM3560_PEAK_2300mA = 0x20,
- LM3560_PEAK_3000mA = 0x40,
- LM3560_PEAK_3600mA = 0x60
-};
-
-#endif /* __LM3560_H__ */
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 0/6] media: lm3560: convert to use OF bindings
2026-05-03 16:44 [PATCH v5 0/6] media: lm3560: convert to use OF bindings Svyatoslav Ryhel
` (5 preceding siblings ...)
2026-05-03 16:44 ` [PATCH v5 6/6] media: i2c: lm3560: Add proper support for LM3559 Svyatoslav Ryhel
@ 2026-05-04 5:35 ` Svyatoslav Ryhel
2026-05-04 6:25 ` Sakari Ailus
6 siblings, 1 reply; 20+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-04 5:35 UTC (permalink / raw)
To: Sakari Ailus, Svyatoslav Ryhel
Cc: linux-leds, devicetree, linux-kernel, linux-media
нд, 3 трав. 2026 р. о 19:44 Svyatoslav Ryhel <clamor95@gmail.com> пише:
>
> Add missing HWEN input pin and IN supply. Fix v4l2 subdev registration.
> Remove platform data and switch to OF device tree bindings.
>
> ---
> Changes in v5:
> - schema adjusted to take into account lm3559
> - device_for_each_child_node > for_each_available_child_of_node
> - lm3559 and lm3560 configuration was diverged with data match
> - removed redundant header
>
> Changes in v4:
> - fixed current being off by 10 in schema
> - label property from schema replaced with modern equivalents
> - lm3560_init_device moved before subdev registration
> - v4l2_device_unregister_subdev > v4l2_async_unregister_subdev
> - added subdevice cleanup if second led registration fails
> - added check if "reg" property exists for LED nodes
> - added missing fwnode_handle_put if device loop fails
> - added bitmap to monitor configured LED id
> - added pm_ptr() macro for PM operations pointer
>
> Changes in v3:
> - added note regarding lm3559 in the schema commit
> - lm3560 power on/off functions converted to be part of PM,
> dropped redundant wrappers
>
> Changes in v2:
> - vendor properties swapped with generic LED properties
> - added mutex lock usage optimization
> - power supply and enable gpio commits squashed into PM
> configuration since they are both required in making
> proper on/off sequence.
> ---
>
> Svyatoslav Ryhel (6):
> dt-bindings: leds: Document TI LM3560 Synchronous Boost Flash Driver
> media: i2c: lm3560: Fix v4l2 subdev registration
> media: i2c: lm3560: Optimize mutex lock usage
> media: i2c: lm3560: Convert to use OF bindings
> media: i2c: lm3560: Add support for PM features
> media: i2c: lm3560: Add proper support for LM3559
>
> .../devicetree/bindings/leds/ti,lm3560.yaml | 163 ++++++++
> drivers/media/i2c/lm3560.c | 385 +++++++++++++++---
> include/media/i2c/lm3560.h | 84 ----
> 3 files changed, 481 insertions(+), 151 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/leds/ti,lm3560.yaml
> delete mode 100644 include/media/i2c/lm3560.h
>
> --
> 2.51.0
>
Hello Sakari!
During preparation of this patchset 2 important issues were not
tracked and discovered only after sending.
1. In "dt-bindings: leds: Document TI LM3560 Synchronous Boost Flash Driver"
flash-max-microamp and led-max-microamp ranges of lm3559 and lm3560
pattern properties were swapped.
2. In "media: i2c: lm3560: Convert to use OF bindings"
In the lm3560_probe struct device_node *node should be removed and
for_each_available_child_of_node should be replaced with
for_each_available_child_of_node_scoped
I am sorry for this inconvenience. If you find it suitable to adjust
these commits on apply feel free to do so, alternatively I can fix
them in the next iteration.
With best regards,
Svyatoslav R.
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 0/6] media: lm3560: convert to use OF bindings
2026-05-04 5:35 ` [PATCH v5 0/6] media: lm3560: convert to use OF bindings Svyatoslav Ryhel
@ 2026-05-04 6:25 ` Sakari Ailus
0 siblings, 0 replies; 20+ messages in thread
From: Sakari Ailus @ 2026-05-04 6:25 UTC (permalink / raw)
To: Svyatoslav Ryhel; +Cc: linux-leds, devicetree, linux-kernel, linux-media
Hi Svyatoslav,
On Mon, May 04, 2026 at 08:35:44AM +0300, Svyatoslav Ryhel wrote:
> нд, 3 трав. 2026 р. о 19:44 Svyatoslav Ryhel <clamor95@gmail.com> пише:
> >
> > Add missing HWEN input pin and IN supply. Fix v4l2 subdev registration.
> > Remove platform data and switch to OF device tree bindings.
> >
> > ---
> > Changes in v5:
> > - schema adjusted to take into account lm3559
> > - device_for_each_child_node > for_each_available_child_of_node
> > - lm3559 and lm3560 configuration was diverged with data match
> > - removed redundant header
> >
> > Changes in v4:
> > - fixed current being off by 10 in schema
> > - label property from schema replaced with modern equivalents
> > - lm3560_init_device moved before subdev registration
> > - v4l2_device_unregister_subdev > v4l2_async_unregister_subdev
> > - added subdevice cleanup if second led registration fails
> > - added check if "reg" property exists for LED nodes
> > - added missing fwnode_handle_put if device loop fails
> > - added bitmap to monitor configured LED id
> > - added pm_ptr() macro for PM operations pointer
> >
> > Changes in v3:
> > - added note regarding lm3559 in the schema commit
> > - lm3560 power on/off functions converted to be part of PM,
> > dropped redundant wrappers
> >
> > Changes in v2:
> > - vendor properties swapped with generic LED properties
> > - added mutex lock usage optimization
> > - power supply and enable gpio commits squashed into PM
> > configuration since they are both required in making
> > proper on/off sequence.
> > ---
> >
> > Svyatoslav Ryhel (6):
> > dt-bindings: leds: Document TI LM3560 Synchronous Boost Flash Driver
> > media: i2c: lm3560: Fix v4l2 subdev registration
> > media: i2c: lm3560: Optimize mutex lock usage
> > media: i2c: lm3560: Convert to use OF bindings
> > media: i2c: lm3560: Add support for PM features
> > media: i2c: lm3560: Add proper support for LM3559
> >
> > .../devicetree/bindings/leds/ti,lm3560.yaml | 163 ++++++++
> > drivers/media/i2c/lm3560.c | 385 +++++++++++++++---
> > include/media/i2c/lm3560.h | 84 ----
> > 3 files changed, 481 insertions(+), 151 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/leds/ti,lm3560.yaml
> > delete mode 100644 include/media/i2c/lm3560.h
> >
> > --
> > 2.51.0
> >
>
> Hello Sakari!
>
> During preparation of this patchset 2 important issues were not
> tracked and discovered only after sending.
>
> 1. In "dt-bindings: leds: Document TI LM3560 Synchronous Boost Flash Driver"
>
> flash-max-microamp and led-max-microamp ranges of lm3559 and lm3560
> pattern properties were swapped.
>
> 2. In "media: i2c: lm3560: Convert to use OF bindings"
>
> In the lm3560_probe struct device_node *node should be removed and
> for_each_available_child_of_node should be replaced with
> for_each_available_child_of_node_scoped
>
> I am sorry for this inconvenience. If you find it suitable to adjust
> these commits on apply feel free to do so, alternatively I can fix
> them in the next iteration.
Ack, I have a few comments on the set as well; please address the issues in
v6.
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 20+ messages in thread