[PATCH 1/5] leds: lp8860: Use a single regmap table

[PATCH 1/5] leds: lp8860: Use a single regmap table

[Andrew Davis]

Instead of a regmap table each for the normal registers and the EEPROM
registers, make one table and use an access table to prevent read/write
to/from the registers between the two ranges. Slightly simplifies the
code.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/leds/leds-lp8860.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index 0962c00c215a1..87d298b6be7d0 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -89,14 +89,12 @@
  * @client: Pointer to the I2C client
  * @led_dev: led class device pointer
  * @regmap: Devices register map
- * @eeprom_regmap: EEPROM register map
  */
 struct lp8860_led {
 	struct mutex lock;
 	struct i2c_client *client;
 	struct led_classdev led_dev;
 	struct regmap *regmap;
-	struct regmap *eeprom_regmap;
 };
 
 static const struct reg_sequence lp8860_eeprom_disp_regs[] = {
@@ -231,7 +229,7 @@ static int lp8860_init(struct lp8860_led *led)
 	}
 
 	reg_count = ARRAY_SIZE(lp8860_eeprom_disp_regs);
-	ret = regmap_multi_reg_write(led->eeprom_regmap, lp8860_eeprom_disp_regs, reg_count);
+	ret = regmap_multi_reg_write(led->regmap, lp8860_eeprom_disp_regs, reg_count);
 	if (ret) {
 		dev_err(&led->client->dev, "Failed writing EEPROM\n");
 		goto out;
@@ -255,17 +253,21 @@ static int lp8860_init(struct lp8860_led *led)
 	return ret;
 }
 
-static const struct regmap_config lp8860_regmap_config = {
-	.reg_bits = 8,
-	.val_bits = 8,
+static const struct regmap_range lp8860_reg_ranges[] = {
+	regmap_reg_range(LP8860_DISP_CL1_BRT_MSB, LP8860_EEPROM_UNLOCK),
+	regmap_reg_range(LP8860_EEPROM_REG_0, LP8860_EEPROM_REG_24),
+};
 
-	.max_register = LP8860_EEPROM_UNLOCK,
+static const struct regmap_access_table lp8860_reg_table = {
+	.yes_ranges = lp8860_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(lp8860_reg_ranges),
 };
 
-static const struct regmap_config lp8860_eeprom_regmap_config = {
+static const struct regmap_config lp8860_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
-
+	.rd_table = &lp8860_reg_table,
+	.wr_table = &lp8860_reg_table,
 	.max_register = LP8860_EEPROM_REG_24,
 };
 
@@ -319,14 +321,6 @@ static int lp8860_probe(struct i2c_client *client)
 		return ret;
 	}
 
-	led->eeprom_regmap = devm_regmap_init_i2c(client, &lp8860_eeprom_regmap_config);
-	if (IS_ERR(led->eeprom_regmap)) {
-		ret = PTR_ERR(led->eeprom_regmap);
-		dev_err(&client->dev, "Failed to allocate register map: %d\n",
-			ret);
-		return ret;
-	}
-
 	ret = lp8860_init(led);
 	if (ret)
 		return ret;
-- 
2.39.2

[PATCH 2/5] leds: lp8860: Return directly from lp8860_init

[Andrew Davis]

No need to use goto to jump to a label that also just returns,
return directly in the if statements.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/leds/leds-lp8860.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index 87d298b6be7d0..71dcd55f0808f 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -216,41 +216,38 @@ static int lp8860_init(struct lp8860_led *led)
 
 	ret = lp8860_fault_check(led);
 	if (ret)
-		goto out;
+		return ret;
 
 	ret = regmap_read(led->regmap, LP8860_STATUS, &read_buf);
 	if (ret)
-		goto out;
+		return ret;
 
 	ret = lp8860_unlock_eeprom(led);
 	if (ret) {
 		dev_err(&led->client->dev, "Failed unlocking EEPROM\n");
-		goto out;
+		return ret;
 	}
 
 	reg_count = ARRAY_SIZE(lp8860_eeprom_disp_regs);
 	ret = regmap_multi_reg_write(led->regmap, lp8860_eeprom_disp_regs, reg_count);
 	if (ret) {
 		dev_err(&led->client->dev, "Failed writing EEPROM\n");
-		goto out;
+		return ret;
 	}
 
 	ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_LOCK_EEPROM);
 	if (ret)
-		goto out;
+		return ret;
 
 	ret = regmap_write(led->regmap,
 			LP8860_EEPROM_CNTRL,
 			LP8860_PROGRAM_EEPROM);
 	if (ret) {
 		dev_err(&led->client->dev, "Failed programming EEPROM\n");
-		goto out;
+		return ret;
 	}
 
-	return ret;
-
-out:
-	return ret;
+	return 0;
 }
 
 static const struct regmap_range lp8860_reg_ranges[] = {
-- 
2.39.2

[PATCH 3/5] leds: lp8860: Hold lock for all of EEPROM programming

[Andrew Davis]

The lock is taken while unlocking the EEPROM but then released, it should
instead be held for the whole EEPROM programming process. To do this
merge in the lp8860_unlock_eeprom() function to the only call site in
the lp8860_init() function. This way we hold the lock for all steps.
While here, rename this function to lp8860_program_eeprom() to better
represent what it really does.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/leds/leds-lp8860.c | 47 ++++++++++++++------------------------
 1 file changed, 17 insertions(+), 30 deletions(-)

diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index 71dcd55f0808f..16129ae94d65f 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -125,32 +125,6 @@ static const struct reg_sequence lp8860_eeprom_disp_regs[] = {
 	{ LP8860_EEPROM_REG_24, 0x3E },
 };
 
-static int lp8860_unlock_eeprom(struct lp8860_led *led)
-{
-	int ret;
-
-	guard(mutex)(&led->lock);
-
-	ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_1);
-	if (ret) {
-		dev_err(&led->client->dev, "EEPROM Unlock failed\n");
-		return ret;
-	}
-
-	ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_2);
-	if (ret) {
-		dev_err(&led->client->dev, "EEPROM Unlock failed\n");
-		return ret;
-	}
-	ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_3);
-	if (ret) {
-		dev_err(&led->client->dev, "EEPROM Unlock failed\n");
-		return ret;
-	}
-
-	return ret;
-}
-
 static int lp8860_fault_check(struct lp8860_led *led)
 {
 	int ret, fault;
@@ -209,11 +183,13 @@ static int lp8860_brightness_set(struct led_classdev *led_cdev,
 	return 0;
 }
 
-static int lp8860_init(struct lp8860_led *led)
+static int lp8860_program_eeprom(struct lp8860_led *led)
 {
 	unsigned int read_buf;
 	int ret, reg_count;
 
+	guard(mutex)(&led->lock);
+
 	ret = lp8860_fault_check(led);
 	if (ret)
 		return ret;
@@ -222,9 +198,20 @@ static int lp8860_init(struct lp8860_led *led)
 	if (ret)
 		return ret;
 
-	ret = lp8860_unlock_eeprom(led);
+	ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_1);
+	if (ret) {
+		dev_err(&led->client->dev, "EEPROM Unlock failed\n");
+		return ret;
+	}
+
+	ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_2);
+	if (ret) {
+		dev_err(&led->client->dev, "EEPROM Unlock failed\n");
+		return ret;
+	}
+	ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_3);
 	if (ret) {
-		dev_err(&led->client->dev, "Failed unlocking EEPROM\n");
+		dev_err(&led->client->dev, "EEPROM Unlock failed\n");
 		return ret;
 	}
 
@@ -318,7 +305,7 @@ static int lp8860_probe(struct i2c_client *client)
 		return ret;
 	}
 
-	ret = lp8860_init(led);
+	ret = lp8860_program_eeprom(led);
 	if (ret)
 		return ret;
 
-- 
2.39.2

[PATCH 4/5] leds: lp8860: Remove unused read of STATUS register

[Andrew Davis]

This register is read but the contents are never checked, remove
the read until we add status checking. While here add an error
message should the preceding fault check fail.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/leds/leds-lp8860.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index 16129ae94d65f..6d1c9434e6d17 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -185,18 +185,15 @@ static int lp8860_brightness_set(struct led_classdev *led_cdev,
 
 static int lp8860_program_eeprom(struct lp8860_led *led)
 {
-	unsigned int read_buf;
 	int ret, reg_count;
 
 	guard(mutex)(&led->lock);
 
 	ret = lp8860_fault_check(led);
-	if (ret)
-		return ret;
-
-	ret = regmap_read(led->regmap, LP8860_STATUS, &read_buf);
-	if (ret)
+	if (ret) {
+		dev_err(&led->client->dev, "Cannot read/clear faults\n");
 		return ret;
+	}
 
 	ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_1);
 	if (ret) {
-- 
2.39.2

[PATCH 5/5] leds: lp8860: Do not always program EEPROM on probe

[Andrew Davis]

The EEPROM has limited writes and the contents might have factory set
values that should not be changed. The values currently written by this
driver are just one example of values, but might not be correct for many
use-cases. Do not overwrite the EEPROM with these example values every
probe.

At some point it would be better to populate the content of the EEPROM
based on a configuration provided by the user and check that the values
in EEPROM are not already the same to avoid unneeded write cycles.

That configuration would depend on how the device is used on the board to
which it is attached, for that Device Tree might be the right way. Until a
method can be devised, gate the EEPROM writing behind a module param.

Reported-by: David Owens <daowens01@gmail.com>
Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/leds/leds-lp8860.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index 6d1c9434e6d17..7a436861c4b71 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -97,6 +97,16 @@ struct lp8860_led {
 	struct regmap *regmap;
 };
 
+static bool program_eeprom;
+module_param(program_eeprom, bool, 0644);
+MODULE_PARM_DESC(program_eeprom, "Program the configuration EEPROM on device startup");
+
+/*
+ * EEPROM bits are intended to be set/programmed before normal operation only
+ * once during silicon production, but can be reprogrammed for evaluation purposes
+ * up to 1000 cycles. To program this EEPROM using this driver, update the below
+ * table and set the module param "program_eeprom" to 1
+ */
 static const struct reg_sequence lp8860_eeprom_disp_regs[] = {
 	{ LP8860_EEPROM_REG_0, 0xed },
 	{ LP8860_EEPROM_REG_1, 0xdf },
@@ -302,9 +312,11 @@ static int lp8860_probe(struct i2c_client *client)
 		return ret;
 	}
 
-	ret = lp8860_program_eeprom(led);
-	if (ret)
-		return ret;
+	if (program_eeprom) {
+		ret = lp8860_program_eeprom(led);
+		if (ret)
+			return ret;
+	}
 
 	init_data.fwnode = of_fwnode_handle(child_node);
 	init_data.devicename = LP8860_NAME;
-- 
2.39.2

Re: [PATCH 1/5] leds: lp8860: Use a single regmap table

[Lee Jones]

On Thu, 05 Mar 2026 14:37:02 -0600, Andrew Davis wrote:
> Instead of a regmap table each for the normal registers and the EEPROM
> registers, make one table and use an access table to prevent read/write
> to/from the registers between the two ranges. Slightly simplifies the
> code.
> 
> 

Applied, thanks!

[1/5] leds: lp8860: Use a single regmap table
      commit: cfbe3ef10db19c2a39640204be160952754e430c
[2/5] leds: lp8860: Return directly from lp8860_init
      commit: c6cde1446602bf990ce8ad81181434f436a16037
[3/5] leds: lp8860: Hold lock for all of EEPROM programming
      commit: 34b7e5d805a33263f4297b42922bae4c70c34026
[4/5] leds: lp8860: Remove unused read of STATUS register
      commit: e4bacd32bcf0fd0a3dc78363d5ce6aa3cce63563
[5/5] leds: lp8860: Do not always program EEPROM on probe
      commit: e5e4ccc8896f9391f718817a19603c08bf8d4cc4

--
Lee Jones [李琼斯]