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

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