Re: [PATCH v1 3/3] leds: aat1290: Add support for AAT1270 Current Regulator for Flash LEDs

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

Hi Svyatoslav,

On 10/7/26 10:54 AM, Svyatoslav Ryhel wrote:
> The AAT1270 is quite similar to the AAT1290, but it does not support the
> Advanced Simple Serial Control (AS2C) used in the AAT1290. The AAT1270
> uses an earlier version of AS2C, namely Simple Serial Control, which lacks
> address-sending capabilities.
> 
> Differentiate the chips by using a description structure with a flag to
> indicate if AS2C is supported, placing all AS2C-only features under a
> conditional check.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>   drivers/leds/flash/leds-aat1290.c | 66 ++++++++++++++++++++++---------
>   1 file changed, 47 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/leds/flash/leds-aat1290.c b/drivers/leds/flash/leds-aat1290.c
> index 49251cfd33509..773cdcc73a9e1 100644
> --- a/drivers/leds/flash/leds-aat1290.c
> +++ b/drivers/leds/flash/leds-aat1290.c
> @@ -43,9 +43,6 @@
>   #define AAT1290_FLASH_TM_NUM_LEVELS	16
>   #define AAT1290_MM_CURRENT_SCALE_SIZE	15
>   
> -#define AAT1290_NAME			"aat1290"
> -
> -
>   struct aat1290_led_config_data {
>   	/* maximum LED current in movie mode */
>   	u32 max_mm_current;
> @@ -59,6 +56,13 @@ struct aat1290_led_config_data {
>   	enum led_brightness max_brightness;
>   };
>   
> +struct aat1290_led_chip_info {
> +	/* led controller name */
> +	const char *name;
> +	/* Advanced Simple Serial Control support */
> +	bool as2c;
> +};
> +
>   struct aat1290_led {
>   	/* platform device data */
>   	struct platform_device *pdev;
> @@ -74,6 +78,10 @@ struct aat1290_led {
>   	struct gpio_desc *gpio_fl_en;
>   	/* EN|SET pin  */
>   	struct gpio_desc *gpio_en_set;
> +
> +	/* led controller description */
> +	const struct aat1290_led_chip_info *info;
> +
>   	/* movie mode current scale */
>   	int *mm_current_scale;
>   	/* device mode */
> @@ -101,15 +109,17 @@ static void aat1290_as2cwire_write(struct aat1290_led *led, int addr, int value)
>   
>   	udelay(AAT1290_FLEN_OFF_DELAY_TIME_US);
>   
> -	/* write address */
> -	for (i = 0; i < addr; ++i) {
> -		udelay(AAT1290_EN_SET_TICK_TIME_US);
> -		gpiod_direction_output(led->gpio_en_set, 0);
> -		udelay(AAT1290_EN_SET_TICK_TIME_US);
> -		gpiod_direction_output(led->gpio_en_set, 1);
> -	}
> +	if (led->info->as2c) {
> +		/* write address */
> +		for (i = 0; i < addr; ++i) {
> +			udelay(AAT1290_EN_SET_TICK_TIME_US);
> +			gpiod_direction_output(led->gpio_en_set, 0);
> +			udelay(AAT1290_EN_SET_TICK_TIME_US);
> +			gpiod_direction_output(led->gpio_en_set, 1);
> +		}
>   
> -	usleep_range(AAT1290_LATCH_TIME_MIN_US, AAT1290_LATCH_TIME_MAX_US);
> +		usleep_range(AAT1290_LATCH_TIME_MIN_US, AAT1290_LATCH_TIME_MAX_US);
> +	}
>   
>   	/* write data */
>   	for (i = 0; i < value; ++i) {
> @@ -150,16 +160,18 @@ static int aat1290_led_brightness_set(struct led_classdev *led_cdev,
>   		led->movie_mode = false;
>   	} else {
>   		if (!led->movie_mode) {
> -			aat1290_as2cwire_write(led,
> -				AAT1290_MM_CURRENT_RATIO_ADDR,
> -				AAT1290_MM_TO_FL_1_92);
> +			if (led->info->as2c)
> +				aat1290_as2cwire_write(led,
> +					AAT1290_MM_CURRENT_RATIO_ADDR,
> +					AAT1290_MM_TO_FL_1_92);

In AAT1270 the Maximum Movie to Flash Mode Current Ratio is constant
and set to 1/7.3, whereas for AAT1290 we alter it to 1/1.92 here.

This is taken into account while calculating max_mm_current in
init_mm_current_scale(), where AAT1290_MAX_MM_CURRENT definition is
used, which is based on AAT1290_MM_TO_FL_RATIO (1000 / 1920).

Obviously it would have to be changed to 1000 / 7300 for AAT1270,
for the calculations between brightness levels and microamperes for
V4L2 Flash interface to work properly for this controller.

>   			led->movie_mode = true;
>   		}
>   
>   		aat1290_as2cwire_write(led, AAT1290_MOVIE_MODE_CURRENT_ADDR,
>   				AAT1290_MAX_MM_CURR_PERCENT_0 - brightness);
> -		aat1290_as2cwire_write(led, AAT1290_MOVIE_MODE_CONFIG_ADDR,
> -				AAT1290_MOVIE_MODE_ON);
> +		if (led->info->as2c)
> +			aat1290_as2cwire_write(led, AAT1290_MOVIE_MODE_CONFIG_ADDR,
> +					       AAT1290_MOVIE_MODE_ON);
>   	}
>   
>   	mutex_unlock(&led->lock);
> @@ -178,7 +190,8 @@ static int aat1290_led_flash_strobe_set(struct led_classdev_flash *fled_cdev,
>   	mutex_lock(&led->lock);
>   
>   	if (state) {
> -		aat1290_set_flash_safety_timer(led, timeout->val);
> +		if (led->info->as2c)
> +			aat1290_set_flash_safety_timer(led, timeout->val);

AAT1290 has the capability of setting flash safety timer timeout, not
like AAT1270, for which this is controlled solely by the discrete
elements, basing on the formula T = 21.5s/µF · CT. This needs to be
reflected both in the DT bindings and in the
aat1290_init_flash_timeout(), where the minimum value should be also
fixed for AAT1270.


>   		gpiod_direction_output(led->gpio_fl_en, 1);
>   	} else {
>   		gpiod_direction_output(led->gpio_fl_en, 0);
> @@ -475,6 +488,10 @@ static int aat1290_led_probe(struct platform_device *pdev)
>   	fled_cdev->ops = &flash_ops;
>   	led_cdev = &fled_cdev->led_cdev;
>   
> +	led->info = device_get_match_data(dev);
> +	if (!led->info)
> +		return -ENODEV;
> +
>   	ret = aat1290_led_get_configuration(led, &led_cfg, &sub_node);
>   	if (ret < 0)
>   		return ret;
> @@ -489,7 +506,7 @@ static int aat1290_led_probe(struct platform_device *pdev)
>   	aat1290_init_flash_timeout(led, &led_cfg);
>   
>   	init_data.fwnode = of_fwnode_handle(sub_node);
> -	init_data.devicename = AAT1290_NAME;
> +	init_data.devicename = led->info->name;
>   
>   	/* Register LED Flash class device */
>   	ret = led_classdev_flash_register_ext(&pdev->dev, fled_cdev,
> @@ -528,8 +545,19 @@ static void aat1290_led_remove(struct platform_device *pdev)
>   	mutex_destroy(&led->lock);
>   }
>   
> +static const struct aat1290_led_chip_info aat1290_info = {
> +	.name = "aat1290",
> +	.as2c = true,
> +};
> +
> +static const struct aat1290_led_chip_info aat1270_info = {
> +	.name = "aat1270",
> +	.as2c = false,
> +};
> +
>   static const struct of_device_id aat1290_led_dt_match[] = {
> -	{ .compatible = "skyworks,aat1290" },
> +	{ .compatible = "skyworks,aat1290", .data = &aat1290_info },
> +	{ .compatible = "skyworks,aat1270", .data = &aat1270_info },
>   	{},
>   };
>   MODULE_DEVICE_TABLE(of, aat1290_led_dt_match);

-- 
Best regards,
Jacek Anaszewski