[PATCH v1 0/3] leds: aat1290: Add support for AAT1270

[PATCH v1 0/3] leds: aat1290: Add support for AAT1270

[Svyatoslav Ryhel]

Document AAT1270 and FLINH pin, adjust driver to suport AAT1270.

Svyatoslav Ryhel (3):
  dt-bindings: leds: skyworks,aat1290: Document AAT1270 Current
    Regulator for Flash LEDs
  dt-bindings: leds: skyworks,aat1290: Document FLINH pin
  leds: aat1290: Add support for AAT1270 Current Regulator for Flash
    LEDs

 .../bindings/leds/skyworks,aat1290.yaml       |  8 ++-
 drivers/leds/flash/leds-aat1290.c             | 66 +++++++++++++------
 2 files changed, 54 insertions(+), 20 deletions(-)

-- 
2.53.0

[PATCH v1 1/3] dt-bindings: leds: skyworks,aat1290: Document AAT1270 Current Regulator for Flash LEDs

[Svyatoslav Ryhel]

The AAT1270 Current Regulator for Flash LEDs has a similar configuration
to the AAT1290, but it uses a different programming model and requires a
separate compatible.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml b/Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml
index 65576dfdca110..a65d92522c9cf 100644
--- a/Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml
+++ b/Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml
@@ -19,7 +19,9 @@ description: |
 
 properties:
   compatible:
-    const: skyworks,aat1290
+    enum:
+      - skyworks,aat1290
+      - skyworks,aat1270
 
   enset-gpios:
     maxItems: 1
-- 
2.53.0

Re: [PATCH v1 1/3] dt-bindings: leds: skyworks,aat1290: Document AAT1270 Current Regulator for Flash LEDs

[sashiko-bot]

> The AAT1270 Current Regulator for Flash LEDs has a similar configuration
> to the AAT1290, but it uses a different programming model and requires a
> separate compatible.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007085457.98762-1-clamor95@gmail.com?part=1

[PATCH v1 2/3] dt-bindings: leds: skyworks,aat1290: Document FLINH pin

[Svyatoslav Ryhel]

Both the AAT1270 and AAT1290 feature a FLINH pin used for mandatory Flash
inhibition.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml b/Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml
index a65d92522c9cf..596b5e284f44a 100644
--- a/Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml
+++ b/Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml
@@ -31,6 +31,10 @@ properties:
     maxItems: 1
     description: FL_EN pin
 
+  flinh-gpios:
+    maxItems: 1
+    description: FLINH pin
+
   led:
     $ref: common.yaml#
     unevaluatedProperties: false
-- 
2.53.0

Re: [PATCH v1 2/3] dt-bindings: leds: skyworks,aat1290: Document FLINH pin

[sashiko-bot]

> Both the AAT1270 and AAT1290 feature a FLINH pin used for mandatory Flash
> inhibition.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007085457.98762-1-clamor95@gmail.com?part=2

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

[Svyatoslav Ryhel]

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);
 			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);
 		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);
-- 
2.53.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] leds: aat1270: silently ignores flash timeout while exposing sysfs attribute
--

commit 74189882b25371dce0e983ebf15bb883544c3cfd
Author: Svyatoslav Ryhel <clamor95@gmail.com>

leds: aat1290: Add support for AAT1270 Current Regulator for Flash LEDs

This commit adds support for the AAT1270 flash LED controller. It
differentiates it from the AAT1290 by introducing a chip description structure
that flags whether the Advanced Simple Serial Control (AS2C) interface is
supported, and gates AS2C-specific features accordingly.

> 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
[ ... ]
> @@ -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);

[Severity: Medium]
Can this regression silently ignore the flash timeout for the AAT1270 chip
while still exposing the sysfs attribute?

The patch bypasses programming the hardware flash safety timer here for chips
without AS2C support (like the AAT1270). However, the driver unconditionally
populates the fled_cdev->timeout bounds for all chips in
aat1290_init_flash_timeout():

drivers/leds/flash/leds-aat1290.c:aat1290_init_flash_timeout() {
	...
	/* Init flash timeout setting */
	setting = &fled_cdev->timeout;
	setting->min = cfg->max_flash_tm / AAT1290_FLASH_TM_NUM_LEVELS;
	setting->max = cfg->max_flash_tm;
	setting->step = setting->min;
	setting->val = setting->max;
}

As a result, the LED subsystem will expose a writable flash_timeout sysfs
attribute to userspace. Userspace can configure a timeout value, believing
it provides safety bounds, but the driver will silently discard it during
the actual flash strobe for the AAT1270.

Could this violate the API contract and mislead userspace into overestimating
thermal safety margins?

>  		gpiod_direction_output(led->gpio_fl_en, 1);
>  	} else {
>  		gpiod_direction_output(led->gpio_fl_en, 0);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007085457.98762-1-clamor95@gmail.com?part=3

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

[Jacek Anaszewski]

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.

It seems that it is not possible to set brightness level and flash
timeout. Those values are controlled by the values of RSET and CT
elements attached to the controller. Those values should be conveyed
via Device Tree, and the LED device API should reflect that accordingly.

The question is whether it is worth a hassle to add support for this
older chipset version. Could you let as know on which device it is
present.


> 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);
>   			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);
>   		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

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

[Svyatoslav Ryhel]

ср, 7 жовт. 2026 р. о 22:04 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.
>
> It seems that it is not possible to set brightness level and flash
> timeout. Those values are controlled by the values of RSET and CT
> elements attached to the controller.

This is not entirely true. S2C allows setting brightness level
perfectly fine, I have tested it on my LG Optimus 2X P990 (star) that
has this LED flash driver. Flash timeout is fixed, that is true, but
both AAT1270 and AAT1290 feature mandatory flash inhibit logic which
is not implemented by this driver and can be used to set emergency
flash disable.

> Those values should be conveyed
> via Device Tree, and the LED device API should reflect that accordingly.
>

They are calculated same way as AAT1290 does and set in the device
tree. Being fixed does not mean they should be removed. Device tree
describes hardware and those values are part of hw description.

> The question is whether it is worth a hassle to add support for this
> older chipset version. Could you let as know on which device it is
> present.
>

LG Optimus 2X P990 (star). It is a Tegra smartphone, it is not yet
supported by mainline Linux kernel, but it will be in near future. I
am currently upstreaming missing parts for better support.

>
> > 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);
> >                       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);
> >               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
>

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

[Jacek Anaszewski]

On 10/8/26 10:15 AM, Svyatoslav Ryhel wrote:
> ср, 7 жовт. 2026 р. о 22:04 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.
>>
>> It seems that it is not possible to set brightness level and flash
>> timeout. Those values are controlled by the values of RSET and CT
>> elements attached to the controller.
> 
> This is not entirely true. S2C allows setting brightness level
> perfectly fine, I have tested it on my LG Optimus 2X P990 (star) that
> has this LED flash driver. Flash timeout is fixed, that is true, but
> both AAT1270 and AAT1290 feature mandatory flash inhibit logic which
> is not implemented by this driver and can be used to set emergency
> flash disable.
OK, let me try to perform more thorough review, after consulting data
sheet, in the coming days.

>> Those values should be conveyed
>> via Device Tree, and the LED device API should reflect that accordingly.
>>
> 
> They are calculated same way as AAT1290 does and set in the device
> tree. Being fixed does not mean they should be removed. Device tree
> describes hardware and those values are part of hw description.
> 
>> The question is whether it is worth a hassle to add support for this
>> older chipset version. Could you let as know on which device it is
>> present.
>>
> 
> LG Optimus 2X P990 (star). It is a Tegra smartphone, it is not yet
> supported by mainline Linux kernel, but it will be in near future. I
> am currently upstreaming missing parts for better support.

Ack.

-- 
Best regards,
Jacek Anaszewski