leds: lp5523: restore legacy device attributes

git commit db6eaf8388a413a5ee1b4547ce78506b9c6456b0 (leds-lp5523: use generic firmware interface) causes an application conflict. This interface should be maintained for compatibility. Restored device attributes are 'engineN_mode', 'engineN_load' and 'engineN_leds'. (N = 1, 2 or 3) A 'selftest' attribute macro is replaced with LP55xx common macro. Those are accessed when a LED pattern is run by an application. Use a mutex in lp5523_update_program_memory() : This function is called when an user-application writes a 'engineN_load' file or pattern data is loaded from generic firmware interface. So, writing program memory should be protected. If an error occurs on accessing this area, just it returns as -EINVAL quickly. This error code is exact same as old driver function, lp5523_do_store_load() because it should be kept for an user-application compatibility. Even the driver is changed, we can use the application without re-compiling sources. Reported-by: Pali Rohár <pali.rohar@gmail.com> Signed-off-by: Milo Kim <milo.kim@ti.com> Signed-off-by: Bryan Wu <cooloney@gmail.com>

leds: lp5523: restore legacy device attributes
git commit db6eaf8388a413a5ee1b4547ce78506b9c6456b0 (leds-lp5523: use generic firmware interface) causes an application conflict. This interface should be maintained for compatibility. Restored device attributes are 'engineN_mode', 'engineN_load' and 'engineN_leds'. (N = 1, 2 or 3) A 'selftest' attribute macro is replaced with LP55xx common macro. Those are accessed when a LED pattern is run by an application. Use a mutex in lp5523_update_program_memory() : This function is called when an user-application writes a 'engineN_load' file or pattern data is loaded from generic firmware interface. So, writing program memory should be protected. If an error occurs on accessing this area, just it returns as -EINVAL quickly. This error code is exact same as old driver function, lp5523_do_store_load() because it should be kept for an user-application compatibility. Even the driver is changed, we can use the application without re-compiling sources. Reported-by: Pali Rohár <pali.rohar@gmail.com> Signed-off-by: Milo Kim <milo.kim@ti.com> Signed-off-by: Bryan Wu <cooloney@gmail.com>
Milo Kim · Bryan Wu
1 parent 224604389a
Showing 1 changed file with 223 additions and 4 deletions Side-by-side Diff
drivers/leds/leds-lp5523.c
@@ -74,6 +74,9 @@
 #define LP5523_PAGE_ENG1		0
 #define LP5523_PAGE_ENG2		1
 #define LP5523_PAGE_ENG3		2
+#define LP5523_PAGE_MUX1		3
+#define LP5523_PAGE_MUX2		4
+#define LP5523_PAGE_MUX3		5
  
 /* Program Memory Operations */
 #define LP5523_MODE_ENG1_M		0x30	/* Operation Mode Register */
@@ -98,6 +101,8 @@
 #define LP5523_RUN_ENG2			0x08
 #define LP5523_RUN_ENG3			0x02
  
+#define LED_ACTIVE(mux, led)		(!!(mux & (0x0001 << led)))
+
 enum lp5523_chip_id {
 	LP5523,
 	LP55231,
  
  
@@ -338,11 +343,21 @@
 		goto err;
  
 	update_size = i;
-	for (i = 0; i < update_size; i++)
-		lp55xx_write(chip, LP5523_REG_PROG_MEM + i, pattern[i]);
  
-	return 0;
+	mutex_lock(&chip->lock);
  
+	for (i = 0; i < update_size; i++) {
+		ret = lp55xx_write(chip, LP5523_REG_PROG_MEM + i, pattern[i]);
+		if (ret) {
+			mutex_unlock(&chip->lock);
+			return -EINVAL;
+		}
+	}
+
+	mutex_unlock(&chip->lock);
+
+	return size;
+
 err:
 	dev_err(&chip->cl->dev, "wrong pattern format\n");
 	return -EINVAL;
@@ -368,6 +383,192 @@
 	lp5523_update_program_memory(chip, fw->data, fw->size);
 }
  
+static ssize_t show_engine_mode(struct device *dev,
+				struct device_attribute *attr,
+				char *buf, int nr)
+{
+	struct lp55xx_led *led = i2c_get_clientdata(to_i2c_client(dev));
+	struct lp55xx_chip *chip = led->chip;
+	enum lp55xx_engine_mode mode = chip->engines[nr - 1].mode;
+
+	switch (mode) {
+	case LP55XX_ENGINE_RUN:
+		return sprintf(buf, "run\n");
+	case LP55XX_ENGINE_LOAD:
+		return sprintf(buf, "load\n");
+	case LP55XX_ENGINE_DISABLED:
+	default:
+		return sprintf(buf, "disabled\n");
+	}
+}
+show_mode(1)
+show_mode(2)
+show_mode(3)
+
+static ssize_t store_engine_mode(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t len, int nr)
+{
+	struct lp55xx_led *led = i2c_get_clientdata(to_i2c_client(dev));
+	struct lp55xx_chip *chip = led->chip;
+	struct lp55xx_engine *engine = &chip->engines[nr - 1];
+
+	mutex_lock(&chip->lock);
+
+	chip->engine_idx = nr;
+
+	if (!strncmp(buf, "run", 3)) {
+		lp5523_run_engine(chip, true);
+		engine->mode = LP55XX_ENGINE_RUN;
+	} else if (!strncmp(buf, "load", 4)) {
+		lp5523_stop_engine(chip);
+		lp5523_load_engine(chip);
+		engine->mode = LP55XX_ENGINE_LOAD;
+	} else if (!strncmp(buf, "disabled", 8)) {
+		lp5523_stop_engine(chip);
+		engine->mode = LP55XX_ENGINE_DISABLED;
+	}
+
+	mutex_unlock(&chip->lock);
+
+	return len;
+}
+store_mode(1)
+store_mode(2)
+store_mode(3)
+
+static int lp5523_mux_parse(const char *buf, u16 *mux, size_t len)
+{
+	u16 tmp_mux = 0;
+	int i;
+
+	len = min_t(int, len, LP5523_MAX_LEDS);
+
+	for (i = 0; i < len; i++) {
+		switch (buf[i]) {
+		case '1':
+			tmp_mux |= (1 << i);
+			break;
+		case '0':
+			break;
+		case '\n':
+			i = len;
+			break;
+		default:
+			return -1;
+		}
+	}
+	*mux = tmp_mux;
+
+	return 0;
+}
+
+static void lp5523_mux_to_array(u16 led_mux, char *array)
+{
+	int i, pos = 0;
+	for (i = 0; i < LP5523_MAX_LEDS; i++)
+		pos += sprintf(array + pos, "%x", LED_ACTIVE(led_mux, i));
+
+	array[pos] = '\0';
+}
+
+static ssize_t show_engine_leds(struct device *dev,
+			    struct device_attribute *attr,
+			    char *buf, int nr)
+{
+	struct lp55xx_led *led = i2c_get_clientdata(to_i2c_client(dev));
+	struct lp55xx_chip *chip = led->chip;
+	char mux[LP5523_MAX_LEDS + 1];
+
+	lp5523_mux_to_array(chip->engines[nr - 1].led_mux, mux);
+
+	return sprintf(buf, "%s\n", mux);
+}
+show_leds(1)
+show_leds(2)
+show_leds(3)
+
+static int lp5523_load_mux(struct lp55xx_chip *chip, u16 mux, int nr)
+{
+	struct lp55xx_engine *engine = &chip->engines[nr - 1];
+	int ret;
+	u8 mux_page[] = {
+		[LP55XX_ENGINE_1] = LP5523_PAGE_MUX1,
+		[LP55XX_ENGINE_2] = LP5523_PAGE_MUX2,
+		[LP55XX_ENGINE_3] = LP5523_PAGE_MUX3,
+	};
+
+	lp5523_load_engine(chip);
+
+	ret = lp55xx_write(chip, LP5523_REG_PROG_PAGE_SEL, mux_page[nr]);
+	if (ret)
+		return ret;
+
+	ret = lp55xx_write(chip, LP5523_REG_PROG_MEM , (u8)(mux >> 8));
+	if (ret)
+		return ret;
+
+	ret = lp55xx_write(chip, LP5523_REG_PROG_MEM + 1, (u8)(mux));
+	if (ret)
+		return ret;
+
+	engine->led_mux = mux;
+	return 0;
+}
+
+static ssize_t store_engine_leds(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf, size_t len, int nr)
+{
+	struct lp55xx_led *led = i2c_get_clientdata(to_i2c_client(dev));
+	struct lp55xx_chip *chip = led->chip;
+	struct lp55xx_engine *engine = &chip->engines[nr - 1];
+	u16 mux = 0;
+	ssize_t ret;
+
+	if (lp5523_mux_parse(buf, &mux, len))
+		return -EINVAL;
+
+	mutex_lock(&chip->lock);
+
+	chip->engine_idx = nr;
+	ret = -EINVAL;
+
+	if (engine->mode != LP55XX_ENGINE_LOAD)
+		goto leave;
+
+	if (lp5523_load_mux(chip, mux, nr))
+		goto leave;
+
+	ret = len;
+leave:
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+store_leds(1)
+store_leds(2)
+store_leds(3)
+
+static ssize_t store_engine_load(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf, size_t len, int nr)
+{
+	struct lp55xx_led *led = i2c_get_clientdata(to_i2c_client(dev));
+	struct lp55xx_chip *chip = led->chip;
+
+	mutex_lock(&chip->lock);
+
+	chip->engine_idx = nr;
+	lp5523_load_engine_and_select_page(chip);
+
+	mutex_unlock(&chip->lock);
+
+	return lp5523_update_program_memory(chip, buf, len);
+}
+store_load(1)
+store_load(2)
+store_load(3)
+
 static ssize_t lp5523_selftest(struct device *dev,
 			       struct device_attribute *attr,
 			       char *buf)
  
@@ -467,9 +668,27 @@
 	mutex_unlock(&chip->lock);
 }
  
-static DEVICE_ATTR(selftest, S_IRUGO, lp5523_selftest, NULL);
+static LP55XX_DEV_ATTR_RW(engine1_mode, show_engine1_mode, store_engine1_mode);
+static LP55XX_DEV_ATTR_RW(engine2_mode, show_engine2_mode, store_engine2_mode);
+static LP55XX_DEV_ATTR_RW(engine3_mode, show_engine3_mode, store_engine3_mode);
+static LP55XX_DEV_ATTR_RW(engine1_leds, show_engine1_leds, store_engine1_leds);
+static LP55XX_DEV_ATTR_RW(engine2_leds, show_engine2_leds, store_engine2_leds);
+static LP55XX_DEV_ATTR_RW(engine3_leds, show_engine3_leds, store_engine3_leds);
+static LP55XX_DEV_ATTR_WO(engine1_load, store_engine1_load);
+static LP55XX_DEV_ATTR_WO(engine2_load, store_engine2_load);
+static LP55XX_DEV_ATTR_WO(engine3_load, store_engine3_load);
+static LP55XX_DEV_ATTR_RO(selftest, lp5523_selftest);
  
 static struct attribute *lp5523_attributes[] = {
+	&dev_attr_engine1_mode.attr,
+	&dev_attr_engine2_mode.attr,
+	&dev_attr_engine3_mode.attr,
+	&dev_attr_engine1_load.attr,
+	&dev_attr_engine2_load.attr,
+	&dev_attr_engine3_load.attr,
+	&dev_attr_engine1_leds.attr,
+	&dev_attr_engine2_leds.attr,
+	&dev_attr_engine3_leds.attr,
 	&dev_attr_selftest.attr,
 	NULL,
 };
...	...	@@ -74,6 +74,9 @@
74	74	#define LP5523_PAGE_ENG1 0
75	75	#define LP5523_PAGE_ENG2 1
76	76	#define LP5523_PAGE_ENG3 2
	77	+#define LP5523_PAGE_MUX1 3
	78	+#define LP5523_PAGE_MUX2 4
	79	+#define LP5523_PAGE_MUX3 5
77	80
78	81	/* Program Memory Operations */
79	82	#define LP5523_MODE_ENG1_M 0x30 /* Operation Mode Register */
...	...	@@ -98,6 +101,8 @@
98	101	#define LP5523_RUN_ENG2 0x08
99	102	#define LP5523_RUN_ENG3 0x02
100	103
	104	+#define LED_ACTIVE(mux, led) (!!(mux & (0x0001 << led)))
	105	+
101	106	enum lp5523_chip_id {
102	107	LP5523,
103	108	LP55231,
104	109
105	110
...	...	@@ -338,11 +343,21 @@
338	343	goto err;
339	344
340	345	update_size = i;
341		- for (i = 0; i < update_size; i++)
342		- lp55xx_write(chip, LP5523_REG_PROG_MEM + i, pattern[i]);
343	346
344		- return 0;
	347	+ mutex_lock(&chip->lock);
345	348
	349	+ for (i = 0; i < update_size; i++) {
	350	+ ret = lp55xx_write(chip, LP5523_REG_PROG_MEM + i, pattern[i]);
	351	+ if (ret) {
	352	+ mutex_unlock(&chip->lock);
	353	+ return -EINVAL;
	354	+ }
	355	+ }
	356	+
	357	+ mutex_unlock(&chip->lock);
	358	+
	359	+ return size;
	360	+
346	361	err:
347	362	dev_err(&chip->cl->dev, "wrong pattern format\n");
348	363	return -EINVAL;
...	...	@@ -368,6 +383,192 @@
368	383	lp5523_update_program_memory(chip, fw->data, fw->size);
369	384	}
370	385
	386	+static ssize_t show_engine_mode(struct device *dev,
	387	+ struct device_attribute *attr,
	388	+ char *buf, int nr)
	389	+{
	390	+ struct lp55xx_led *led = i2c_get_clientdata(to_i2c_client(dev));
	391	+ struct lp55xx_chip *chip = led->chip;
	392	+ enum lp55xx_engine_mode mode = chip->engines[nr - 1].mode;
	393	+
	394	+ switch (mode) {
	395	+ case LP55XX_ENGINE_RUN:
	396	+ return sprintf(buf, "run\n");
	397	+ case LP55XX_ENGINE_LOAD:
	398	+ return sprintf(buf, "load\n");
	399	+ case LP55XX_ENGINE_DISABLED:
	400	+ default:
	401	+ return sprintf(buf, "disabled\n");
	402	+ }
	403	+}
	404	+show_mode(1)
	405	+show_mode(2)
	406	+show_mode(3)
	407	+
	408	+static ssize_t store_engine_mode(struct device *dev,
	409	+ struct device_attribute *attr,
	410	+ const char *buf, size_t len, int nr)
	411	+{
	412	+ struct lp55xx_led *led = i2c_get_clientdata(to_i2c_client(dev));
	413	+ struct lp55xx_chip *chip = led->chip;
	414	+ struct lp55xx_engine *engine = &chip->engines[nr - 1];
	415	+
	416	+ mutex_lock(&chip->lock);
	417	+
	418	+ chip->engine_idx = nr;
	419	+
	420	+ if (!strncmp(buf, "run", 3)) {
	421	+ lp5523_run_engine(chip, true);
	422	+ engine->mode = LP55XX_ENGINE_RUN;
	423	+ } else if (!strncmp(buf, "load", 4)) {
	424	+ lp5523_stop_engine(chip);
	425	+ lp5523_load_engine(chip);
	426	+ engine->mode = LP55XX_ENGINE_LOAD;
	427	+ } else if (!strncmp(buf, "disabled", 8)) {
	428	+ lp5523_stop_engine(chip);
	429	+ engine->mode = LP55XX_ENGINE_DISABLED;
	430	+ }
	431	+
	432	+ mutex_unlock(&chip->lock);
	433	+
	434	+ return len;
	435	+}
	436	+store_mode(1)
	437	+store_mode(2)
	438	+store_mode(3)
	439	+
	440	+static int lp5523_mux_parse(const char buf, u16 mux, size_t len)
	441	+{
	442	+ u16 tmp_mux = 0;
	443	+ int i;
	444	+
	445	+ len = min_t(int, len, LP5523_MAX_LEDS);
	446	+
	447	+ for (i = 0; i < len; i++) {
	448	+ switch (buf[i]) {
	449	+ case '1':
	450	+ tmp_mux \|= (1 << i);
	451	+ break;
	452	+ case '0':
	453	+ break;
	454	+ case '\n':
	455	+ i = len;
	456	+ break;
	457	+ default:
	458	+ return -1;
	459	+ }
	460	+ }
	461	+ *mux = tmp_mux;
	462	+
	463	+ return 0;
	464	+}
	465	+
	466	+static void lp5523_mux_to_array(u16 led_mux, char *array)
	467	+{
	468	+ int i, pos = 0;
	469	+ for (i = 0; i < LP5523_MAX_LEDS; i++)
	470	+ pos += sprintf(array + pos, "%x", LED_ACTIVE(led_mux, i));
	471	+
	472	+ array[pos] = '\0';
	473	+}
	474	+
	475	+static ssize_t show_engine_leds(struct device *dev,
	476	+ struct device_attribute *attr,
	477	+ char *buf, int nr)
	478	+{
	479	+ struct lp55xx_led *led = i2c_get_clientdata(to_i2c_client(dev));
	480	+ struct lp55xx_chip *chip = led->chip;
	481	+ char mux[LP5523_MAX_LEDS + 1];
	482	+
	483	+ lp5523_mux_to_array(chip->engines[nr - 1].led_mux, mux);
	484	+
	485	+ return sprintf(buf, "%s\n", mux);
	486	+}
	487	+show_leds(1)
	488	+show_leds(2)
	489	+show_leds(3)
	490	+
	491	+static int lp5523_load_mux(struct lp55xx_chip *chip, u16 mux, int nr)
	492	+{
	493	+ struct lp55xx_engine *engine = &chip->engines[nr - 1];
	494	+ int ret;
	495	+ u8 mux_page[] = {
	496	+ [LP55XX_ENGINE_1] = LP5523_PAGE_MUX1,
	497	+ [LP55XX_ENGINE_2] = LP5523_PAGE_MUX2,
	498	+ [LP55XX_ENGINE_3] = LP5523_PAGE_MUX3,
	499	+ };
	500	+
	501	+ lp5523_load_engine(chip);
	502	+
	503	+ ret = lp55xx_write(chip, LP5523_REG_PROG_PAGE_SEL, mux_page[nr]);
	504	+ if (ret)
	505	+ return ret;
	506	+
	507	+ ret = lp55xx_write(chip, LP5523_REG_PROG_MEM , (u8)(mux >> 8));
	508	+ if (ret)
	509	+ return ret;
	510	+
	511	+ ret = lp55xx_write(chip, LP5523_REG_PROG_MEM + 1, (u8)(mux));
	512	+ if (ret)
	513	+ return ret;
	514	+
	515	+ engine->led_mux = mux;
	516	+ return 0;
	517	+}
	518	+
	519	+static ssize_t store_engine_leds(struct device *dev,
	520	+ struct device_attribute *attr,
	521	+ const char *buf, size_t len, int nr)
	522	+{
	523	+ struct lp55xx_led *led = i2c_get_clientdata(to_i2c_client(dev));
	524	+ struct lp55xx_chip *chip = led->chip;
	525	+ struct lp55xx_engine *engine = &chip->engines[nr - 1];
	526	+ u16 mux = 0;
	527	+ ssize_t ret;
	528	+
	529	+ if (lp5523_mux_parse(buf, &mux, len))
	530	+ return -EINVAL;
	531	+
	532	+ mutex_lock(&chip->lock);
	533	+
	534	+ chip->engine_idx = nr;
	535	+ ret = -EINVAL;
	536	+
	537	+ if (engine->mode != LP55XX_ENGINE_LOAD)
	538	+ goto leave;
	539	+
	540	+ if (lp5523_load_mux(chip, mux, nr))
	541	+ goto leave;
	542	+
	543	+ ret = len;
	544	+leave:
	545	+ mutex_unlock(&chip->lock);
	546	+ return ret;
	547	+}
	548	+store_leds(1)
	549	+store_leds(2)
	550	+store_leds(3)
	551	+
	552	+static ssize_t store_engine_load(struct device *dev,
	553	+ struct device_attribute *attr,
	554	+ const char *buf, size_t len, int nr)
	555	+{
	556	+ struct lp55xx_led *led = i2c_get_clientdata(to_i2c_client(dev));
	557	+ struct lp55xx_chip *chip = led->chip;
	558	+
	559	+ mutex_lock(&chip->lock);
	560	+
	561	+ chip->engine_idx = nr;
	562	+ lp5523_load_engine_and_select_page(chip);
	563	+
	564	+ mutex_unlock(&chip->lock);
	565	+
	566	+ return lp5523_update_program_memory(chip, buf, len);
	567	+}
	568	+store_load(1)
	569	+store_load(2)
	570	+store_load(3)
	571	+
371	572	static ssize_t lp5523_selftest(struct device *dev,
372	573	struct device_attribute *attr,
373	574	char *buf)
374	575
...	...	@@ -467,9 +668,27 @@
467	668	mutex_unlock(&chip->lock);
468	669	}
469	670
470		-static DEVICE_ATTR(selftest, S_IRUGO, lp5523_selftest, NULL);
	671	+static LP55XX_DEV_ATTR_RW(engine1_mode, show_engine1_mode, store_engine1_mode);
	672	+static LP55XX_DEV_ATTR_RW(engine2_mode, show_engine2_mode, store_engine2_mode);
	673	+static LP55XX_DEV_ATTR_RW(engine3_mode, show_engine3_mode, store_engine3_mode);
	674	+static LP55XX_DEV_ATTR_RW(engine1_leds, show_engine1_leds, store_engine1_leds);
	675	+static LP55XX_DEV_ATTR_RW(engine2_leds, show_engine2_leds, store_engine2_leds);
	676	+static LP55XX_DEV_ATTR_RW(engine3_leds, show_engine3_leds, store_engine3_leds);
	677	+static LP55XX_DEV_ATTR_WO(engine1_load, store_engine1_load);
	678	+static LP55XX_DEV_ATTR_WO(engine2_load, store_engine2_load);
	679	+static LP55XX_DEV_ATTR_WO(engine3_load, store_engine3_load);
	680	+static LP55XX_DEV_ATTR_RO(selftest, lp5523_selftest);
471	681
472	682	static struct attribute *lp5523_attributes[] = {
	683	+ &dev_attr_engine1_mode.attr,
	684	+ &dev_attr_engine2_mode.attr,
	685	+ &dev_attr_engine3_mode.attr,
	686	+ &dev_attr_engine1_load.attr,
	687	+ &dev_attr_engine2_load.attr,
	688	+ &dev_attr_engine3_load.attr,
	689	+ &dev_attr_engine1_leds.attr,
	690	+ &dev_attr_engine2_leds.attr,
	691	+ &dev_attr_engine3_leds.attr,
473	692	&dev_attr_selftest.attr,
474	693	NULL,
475	694	};