Skip to contentSkip to conversation

leds: Clarify supported chips by LM355x driver

PATCH In review Andy Shevchenko 4 participants 14 messages Jan 26, 2018 – Feb 1, 2018 LED

lore
Share & tools
lore

Andy Shevchenko

author
#0 lore raw

PATCH 1/1 leds: Clarify supported chips by LM355x driver

LM3555 is supported by AS3645A.
Clarify this in Kconfig by referencing to AS3645A.

No functional change.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/leds/Kconfig | 2 ++
 1 file changed, 2 insertions(+)
drivers/leds/Kconfig +2 -0
index 3e763d2a0cb3..0961882fa57f 100644
@@ -601,6 +601,8 @@ config LEDS_LM355x
 	  This option enables support for LEDs connected to LM355x.
 	  LM355x includes Torch, Flash and Indicator functions.
 
+	  Note, LM3555 is driven by AS3645A.
+
 config LEDS_OT200
 	tristate "LED support for the Bachmann OT200"
 	depends on LEDS_CLASS && HAS_IOMEM && (X86_32 || COMPILE_TEST)
Signature · 1 line
2.15.1
#1 lore raw
Quoted 15 lines · Andy Shevchenko

LM3555 is supported by AS3645A.
Clarify this in Kconfig by referencing to AS3645A.

No functional change.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/leds/Kconfig | 2 ++
 1 file changed, 2 insertions(+)
drivers/leds/Kconfig +0 -0
index 3e763d2a0cb3..0961882fa57f 100644

drivers/leds/Kconfig config LEDS_LM355x · line 605

Quoted 5 lines
drivers/leds/Kconfig +2 -0
@@ -601,6 +601,8 @@ config LEDS_LM355x
 	  This option enables support for LEDs connected to LM355x.
 	  LM355x includes Torch, Flash and Indicator functions.
 
+	  Note, LM3555 is driven by AS3645A.
+

I took a look at the driver. It seems to me lm355x only handles lm3554 and lm3556? Perhaps Kconfig help should say that explicitely?

(I guess lm3555 is somehow different from the other two?)
Pavel

Quoted 3 lines
drivers/leds/Kconfig +0 -0
 config LEDS_OT200
 	tristate "LED support for the Bachmann OT200"
 	depends on LEDS_CLASS && HAS_IOMEM && (X86_32 || COMPILE_TEST)
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#2 lore raw
Quoted 6 lines · Pavel Machek
Quoted 4 lines · Andy Shevchenko

LM3555 is supported by AS3645A.
Clarify this in Kconfig by referencing to AS3645A.

No functional change.

Quoted 8 lines

drivers/leds/Kconfig config LEDS_LM355x · line 605

Quoted 5 lines

This option enables support for LEDs connected to LM355x.

drivers/leds/Kconfig +2 -0
 	  LM355x includes Torch, Flash and Indicator functions.
 
+	  Note, LM3555 is driven by AS3645A.
+

I took a look at the driver. It seems to me lm355x only handles lm3554 and lm3556?

Correct.

Quoted 1 line

Perhaps Kconfig help should say that explicitely?

It's in the title.

Quoted 1 line

(I guess lm3555 is somehow different from the other two?)

Correct.

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
Acked-by
#3 lore raw
Quoted 16 lines · Andy Shevchenko
Quoted 6 lines · Pavel Machek
Quoted 4 lines · Andy Shevchenko

LM3555 is supported by AS3645A.
Clarify this in Kconfig by referencing to AS3645A.

No functional change.

Quoted 8 lines

drivers/leds/Kconfig config LEDS_LM355x · line 605

Quoted 5 lines

This option enables support for LEDs connected to LM355x.

drivers/leds/Kconfig +2 -0
 	  LM355x includes Torch, Flash and Indicator functions.
 
+	  Note, LM3555 is driven by AS3645A.
+

I took a look at the driver. It seems to me lm355x only handles lm3554 and lm3556?

Quoted 5 lines

Correct.

Quoted 1 line

Perhaps Kconfig help should say that explicitely?

It's in the title.

Aha.

Acked-by Pavel Machek <[email protected]>

Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#4 lore raw
In reply to Pavel Machek
Quoted 10 lines · Pavel Machek
Quoted 9 lines · Andy Shevchenko

LM3555 is supported by AS3645A.
Clarify this in Kconfig by referencing to AS3645A.

No functional change.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/leds/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

Actually, what about this?

Signed-off-by Pavel Machek <[email protected]>
drivers/leds/Kconfig +4 -4
index 318a28f..2acf864 100644
@@ -69,7 +69,7 @@ config LEDS_APU
 	  module will be called leds-apu.
 
 config LEDS_AS3645A
-	tristate "AS3645A LED flash controller support"
+	tristate "AS3645A and LM3555 LED flash controller support"
 	depends on I2C && LEDS_CLASS_FLASH
 	depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
 	help

@@ -587,12 +587,12 @@ config LEDS_MAX8997
 	  MAXIM MAX8997 PMIC.
 
 config LEDS_LM355x
-	tristate "LED support for LM355x Chips, LM3554 and LM3556"
+	tristate "LED support for LM3554 and LM3556 chips"
 	depends on LEDS_CLASS && I2C
 	select REGMAP_I2C
 	help
-	  This option enables support for LEDs connected to LM355x.
-	  LM355x includes Torch, Flash and Indicator functions.
+	  This option enables support for LEDs connected to LM3554
+	  and lM3556. It includes Torch, Flash and Indicator functions.
 
 config LEDS_OT200
 	tristate "LED support for the Bachmann OT200"
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#5 lore raw

Hi Pavel,

Quoted 32 lines · Pavel Machek
Quoted 10 lines · Pavel Machek
Quoted 9 lines · Andy Shevchenko

LM3555 is supported by AS3645A.
Clarify this in Kconfig by referencing to AS3645A.

No functional change.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/leds/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

Actually, what about this?

Signed-off-by Pavel Machek <[email protected]>
drivers/leds/Kconfig +1 -1
index 318a28f..2acf864 100644
@@ -69,7 +69,7 @@ config LEDS_APU
 	  module will be called leds-apu.
 
 config LEDS_AS3645A
-	tristate "AS3645A LED flash controller support"
+	tristate "AS3645A and LM3555 LED flash controller support"
 	depends on I2C && LEDS_CLASS_FLASH
 	depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
 	help

@@ -587,12 +587,12 @@ config LEDS_MAX8997
 	  MAXIM MAX8997 PMIC.

drivers/leds/Kconfig config LEDS_MAX8997 · line 595

Quoted 10 lines
drivers/leds/Kconfig +3 -3
 config LEDS_LM355x
-	tristate "LED support for LM355x Chips, LM3554 and LM3556"
+	tristate "LED support for LM3554 and LM3556 chips"
 	depends on LEDS_CLASS && I2C
 	select REGMAP_I2C
 	help
-	  This option enables support for LEDs connected to LM355x.
-	  LM355x includes Torch, Flash and Indicator functions.
+	  This option enables support for LEDs connected to LM3554
+	  and lM3556. It includes Torch, Flash and Indicator functions.

s/l/L/

drivers/leds/Kconfig config LEDS_MAX8997 · line 598

Quoted 3 lines
drivers/leds/Kconfig +0 -0
 config LEDS_OT200
 	tristate "LED support for the Bachmann OT200"

Could you submit a patch to linux-leds, please?

Signature · 2 lines
Sakari Ailus
e-mail: [email protected]
author Reviewed-by
#6 lore raw
In reply to Pavel Machek
Quoted 3 lines · Pavel Machek

On Fri 2018-01-26 11:25:07, Pavel Machek wrote:

Actually, what about this?

Works for me:

Reviewed-by Andy Shevchenko <[email protected]>

Though one typo below.

drivers/leds/Kconfig config LEDS_APU · line 72

Quoted 13 lines
Signed-off-by Pavel Machek <[email protected]>
drivers/leds/Kconfig +1 -1
index 318a28f..2acf864 100644
@@ -69,7 +69,7 @@ config LEDS_APU
 	  module will be called leds-apu.
 
 config LEDS_AS3645A
-	tristate "AS3645A LED flash controller support"
+	tristate "AS3645A and LM3555 LED flash controller support"

controller -> controllers

Quoted 26 lines
drivers/leds/Kconfig +3 -3
 	depends on I2C && LEDS_CLASS_FLASH
 	depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
 	help

@@ -587,12 +587,12 @@ config LEDS_MAX8997
 	  MAXIM MAX8997 PMIC.
 
 config LEDS_LM355x
-	tristate "LED support for LM355x Chips, LM3554 and LM3556"
+	tristate "LED support for LM3554 and LM3556 chips"
 	depends on LEDS_CLASS && I2C
 	select REGMAP_I2C
 	help
-	  This option enables support for LEDs connected to LM355x.
-	  LM355x includes Torch, Flash and Indicator functions.
+	  This option enables support for LEDs connected to LM3554
+	  and lM3556. It includes Torch, Flash and Indicator

functions.

config LEDS_OT200
tristate "LED support for the Bachmann OT200"

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
#7 lore raw
In reply to Andy Shevchenko

Hi Andy,

Thanks for the patch.

Quoted 15 lines · Andy Shevchenko

LM3555 is supported by AS3645A.
Clarify this in Kconfig by referencing to AS3645A.

No functional change.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/leds/Kconfig | 2 ++
 1 file changed, 2 insertions(+)
drivers/leds/Kconfig +0 -0
index 3e763d2a0cb3..0961882fa57f 100644

drivers/leds/Kconfig config LEDS_LM355x · line 604

Quoted 4 lines
drivers/leds/Kconfig +1 -0
@@ -601,6 +601,8 @@ config LEDS_LM355x
 	  This option enables support for LEDs connected to LM355x.
 	  LM355x includes Torch, Flash and Indicator functions.
 
+	  Note, LM3555 is driven by AS3645A.

Could you please provide more details on this relationship? Looking at the datasheet it seems that both chips are standalone and controlled via I2C and/or via GPIO in case of LM3555.

Quoted 4 lines
drivers/leds/Kconfig +0 -0
 config LEDS_OT200
 	tristate "LED support for the Bachmann OT200"
 	depends on LEDS_CLASS && HAS_IOMEM && (X86_32 || COMPILE_TEST)
Signature · 2 lines
Best regards,
Jacek Anaszewski
author
#8 lore raw
Quoted 7 lines · Jacek Anaszewski

drivers/leds/Kconfig config LEDS_LM355x · line 604

Quoted 1 line · Andy Shevchenko
drivers/leds/Kconfig +1 -0
+	  Note, LM3555 is driven by AS3645A.

Could you please provide more details on this relationship? Looking at the datasheet it seems that both chips are standalone and controlled via I2C and/or via GPIO in case of LM3555.

The only difference I see looking in datasheets again that AS3645A supports dual-led mode, by using STROBE signal in case of parallel interface or thru additional bits in I2C mode.

Can you be more specific what exactly makes you feel that they are not compatible?

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
#9 lore raw
Quoted 9 lines · Andy Shevchenko
Quoted 7 lines · Jacek Anaszewski

drivers/leds/Kconfig config LEDS_LM355x · line 604

Quoted 1 line · Andy Shevchenko
drivers/leds/Kconfig +1 -0
+	  Note, LM3555 is driven by AS3645A.

Could you please provide more details on this relationship? Looking at the datasheet it seems that both chips are standalone and controlled via I2C and/or via GPIO in case of LM3555.

Quoted 6 lines

The only difference I see looking in datasheets again that AS3645A supports dual-led mode, by using STROBE signal in case of parallel interface or thru additional bits in I2C mode.

Can you be more specific what exactly makes you feel that they are not compatible?

The expression "LM3555 is driven by AS3645A" brings on mind rather some type of hardware dependency between chips than drivers similarity.

Now I understand that you meant that both devices can be driven by any of leds-lm355x or leds-as3645a driver, am I right?

If so, then I don't see the compatibility:

Excerpt from both drivers showing register addresses:

drivers/leds/leds-as3645a.c:

#define AS_DESIGN_INFO_REG 0x00
#define AS_VERSION_CONTROL_REG 0x01
#define AS_INDICATOR_AND_TIMER_REG 0x02
#define AS_CURRENT_SET_REG 0x03
#define AS_CONTROL_REG 0x04
#define AS_FAULT_INFO_REG 0x05
#define AS_BOOST_REG 0x0d
#define AS_PASSWORD_REG 0x0f

drivers/leds/leds-lm355x.c:

struct lm355x_reg_data {
        u8 regno; // this is register address
        u8 mask;
        u8 shift;
};
static struct lm355x_reg_data lm3554_regs[REG_MAX] = {
	[REG_FLAG] = {0xD0, 0xBF, 0},
	[REG_TORCH_CFG] = {0xE0, 0x80, 7},
	[REG_TORCH_CTRL] = {0xA0, 0x38, 3},
	[REG_STROBE_CFG] = {0xE0, 0x04, 2},
	[REG_FLASH_CTRL] = {0xB0, 0x78, 3},
	[REG_INDI_CFG] = {0xE0, 0x08, 3},
	[REG_INDI_CTRL] = {0xA0, 0xC0, 6},
	[REG_OPMODE] = {0xA0, 0x03, 0},
};
static struct lm355x_reg_data lm3556_regs[REG_MAX] = {
	[REG_FLAG] = {0x0B, 0xFF, 0},
	[REG_TORCH_CFG] = {0x0A, 0x10, 4},
	[REG_TORCH_CTRL] = {0x09, 0x70, 4},
	[REG_STROBE_CFG] = {0x0A, 0x20, 5},
	[REG_FLASH_CTRL] = {0x09, 0x0F, 0},
	[REG_INDI_CFG] = {0xFF, 0xFF, 0},
	[REG_INDI_CTRL] = {0x09, 0x70, 4},
	[REG_OPMODE] = {0x0A, 0x03, 0},
};

This is consistent with documentation:

[0] http://www.ti.com/lit/ds/symlink/lm3554.pdf
[1] http://www.ti.com/lit/ds/symlink/lm3556.pdf
[2] https://www.mouser.com/ds/2/588/AS3645_Datasheet_EN_v3-1214758.pdf

I suspected that I got something entirely wrong here, especially that both you and Pavel agreed.

But, given that it turned out that leds-lm355x.c doesn't support LM3555 and its documentation [3] indeed shows the affinity with AS3645, I suppose that this patch is a result of a premature conclusion drawn basing on the driver's description rather than its capabilities.

[3] http://www.ti.com/lit/ds/symlink/lm3555.pdf

Effectively, the only sensible change seems to be:

- tristate "LED support for LM355x Chips, LM3554 and LM3556"
+ tristate "LED support for LM3554 and LM3556 chips"
depends on LEDS_CLASS && I2C
select REGMAP_I2C
help

-	  This option enables support for LEDs connected to LM355x.
-	  LM355x includes Torch, Flash and Indicator functions.
+	  This option enables support for LEDs connected to LM3554
+	  and LM3556. It includes Torch, Flash and Indicator functions.
Signature · 2 lines
Best regards,
Jacek Anaszewski
author
#10 lore raw
Quoted 17 lines · Jacek Anaszewski
Quoted 8 lines · Andy Shevchenko
Quoted 6 lines · Jacek Anaszewski

drivers/leds/Kconfig config LEDS_LM355x · line 604

Quoted 1 line · Andy Shevchenko
drivers/leds/Kconfig +1 -0
+	  Note, LM3555 is driven by AS3645A.

Could you please provide more details on this relationship? Looking at the datasheet it seems that both chips are standalone and controlled via I2C and/or via GPIO in case of LM3555.

Quoted 7 lines

The only difference I see looking in datasheets again that AS3645A supports dual-led mode, by using STROBE signal in case of parallel interface or thru additional bits in I2C mode.

Can you be more specific what exactly makes you feel that they are not
compatible?

Quoted 6 lines

The expression "LM3555 is driven by AS3645A" brings on mind rather some type of hardware dependency between chips than drivers similarity.

Now I understand that you meant that both devices can be driven by any of leds-lm355x or leds-as3645a driver, am I right?

No. The chip in question is LM3555, and it's handled only in as3645a driver.

Quoted 22 lines

I suspected that I got something entirely wrong here, especially that both you and Pavel agreed.

But, given that it turned out that leds-lm355x.c doesn't support LM3555 and its documentation [3] indeed shows the affinity with AS3645, I suppose that this patch is a result of a premature conclusion drawn basing on the driver's description rather than its capabilities.

[3] http://www.ti.com/lit/ds/symlink/lm3555.pdf

Effectively, the only sensible change seems to be:

(quoted diff) +3 -3
-	tristate "LED support for LM355x Chips, LM3554 and LM3556"
+	tristate "LED support for LM3554 and LM3556 chips"
 	depends on LEDS_CLASS && I2C
 	select REGMAP_I2C
 	help
-	  This option enables support for LEDs connected to LM355x.
-	  LM355x includes Torch, Flash and Indicator functions.
+	  This option enables support for LEDs connected to LM3554
+	  and LM3556. It includes Torch, Flash and Indicator
Quoted 1 line

functions.

I like patch Pavel sent previously. It feels more comprehensive than this one.

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
#11 lore raw
Quoted 41 lines · Andy Shevchenko
Quoted 17 lines · Jacek Anaszewski
Quoted 8 lines · Andy Shevchenko
Quoted 6 lines · Jacek Anaszewski

drivers/leds/Kconfig config LEDS_LM355x · line 604

Quoted 1 line · Andy Shevchenko
drivers/leds/Kconfig +1 -0
+	  Note, LM3555 is driven by AS3645A.

Could you please provide more details on this relationship? Looking at the datasheet it seems that both chips are standalone and controlled via I2C and/or via GPIO in case of LM3555.

Quoted 7 lines

The only difference I see looking in datasheets again that AS3645A supports dual-led mode, by using STROBE signal in case of parallel interface or thru additional bits in I2C mode.

Can you be more specific what exactly makes you feel that they are not
compatible?

Quoted 6 lines

The expression "LM3555 is driven by AS3645A" brings on mind rather some type of hardware dependency between chips than drivers similarity.

Now I understand that you meant that both devices can be driven by any of leds-lm355x or leds-as3645a driver, am I right?

No. The chip in question is LM3555, and it's handled only in as3645a driver.

Quoted 13 lines

I suspected that I got something entirely wrong here, especially that both you and Pavel agreed.

But, given that it turned out that leds-lm355x.c doesn't support LM3555 and its documentation [3] indeed shows the affinity with AS3645, I suppose that this patch is a result of a premature conclusion drawn basing on the driver's description rather than its capabilities.

[3] http://www.ti.com/lit/ds/symlink/lm3555.pdf

Effectively, the only sensible change seems to be:

Quoted 13 lines
Quoted 10 lines
(quoted diff) +3 -3
-	tristate "LED support for LM355x Chips, LM3554 and LM3556"
+	tristate "LED support for LM3554 and LM3556 chips"
 	depends on LEDS_CLASS && I2C
 	select REGMAP_I2C
 	help
-	  This option enables support for LEDs connected to LM355x.
-	  LM355x includes Torch, Flash and Indicator functions.
+	  This option enables support for LEDs connected to LM3554
+	  and LM3556. It includes Torch, Flash and Indicator

functions.

I like patch Pavel sent previously. It feels more comprehensive than this one.

Right. The patch subject and description will have to be tweaked a bit then. I'll apply it once it is officially submitted.

Signature · 2 lines
Best regards,
Jacek Anaszewski
#12 lore raw

PATCH v2 1/1 leds: Clarify supported chips by LM355x driver

Reported-by Andy Shevchenko <[email protected]>

Clarify which controllers are supported by which driver.

Signed-off-by Pavel Machek <[email protected]>
drivers/leds/Kconfig +4 -4
index 318a28f..9a538de 100644
@@ -69,7 +69,7 @@ config LEDS_APU
 	  module will be called leds-apu.
 
 config LEDS_AS3645A
-	tristate "AS3645A LED flash controller support"
+	tristate "AS3645A and LM3555 LED flash controllers support"
 	depends on I2C && LEDS_CLASS_FLASH
 	depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
 	help

@@ -587,12 +587,12 @@ config LEDS_MAX8997
 	  MAXIM MAX8997 PMIC.
 
 config LEDS_LM355x
-	tristate "LED support for LM355x Chips, LM3554 and LM3556"
+	tristate "LED support for LM3554 and LM3556 chips"
 	depends on LEDS_CLASS && I2C
 	select REGMAP_I2C
 	help
-	  This option enables support for LEDs connected to LM355x.
-	  LM355x includes Torch, Flash and Indicator functions.
+	  This option enables support for LEDs connected to LM3554
+	  and LM3556. It includes Torch, Flash and Indicator functions.
 
 config LEDS_OT200
 	tristate "LED support for the Bachmann OT200"
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Applied
#13 lore raw

Hi Pavel,

Thanks for the update.

Quoted 26 lines · Pavel Machek
Reported-by Andy Shevchenko <[email protected]>

Clarify which controllers are supported by which driver.

Signed-off-by Pavel Machek <[email protected]>
drivers/leds/Kconfig +2 -2
index 318a28f..9a538de 100644
@@ -69,7 +69,7 @@ config LEDS_APU
 	  module will be called leds-apu.
 
 config LEDS_AS3645A
-	tristate "AS3645A LED flash controller support"
+	tristate "AS3645A and LM3555 LED flash controllers support"
 	depends on I2C && LEDS_CLASS_FLASH
 	depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
 	help

@@ -587,12 +587,12 @@ config LEDS_MAX8997
 	  MAXIM MAX8997 PMIC.
 
 config LEDS_LM355x
-	tristate "LED support for LM355x Chips, LM3554 and LM3556"
+	tristate "LED support for LM3554 and LM3556 chips"

drivers/leds/Kconfig config LEDS_MAX8997 · line 598

Quoted 10 lines
drivers/leds/Kconfig +2 -2
 	depends on LEDS_CLASS && I2C
 	select REGMAP_I2C
 	help
-	  This option enables support for LEDs connected to LM355x.
-	  LM355x includes Torch, Flash and Indicator functions.
+	  This option enables support for LEDs connected to LM3554
+	  and LM3556. It includes Torch, Flash and Indicator functions.
 
 config LEDS_OT200
 	tristate "LED support for the Bachmann OT200"

Applied to the for-4.17 branch.

Signature · 2 lines
Best regards,
Jacek Anaszewski