Re: [PATCH v5 4/6] media: i2c: lm3560: Convert to use OF bindings

Sakari Ailus
2026-05-04T06:36:17Z

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", &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