From 8168906bbb3ba678583422de29e6349407a94bb5 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thomas=20Wei=C3=9Fschuh?= Date: Tue, 4 Feb 2025 07:52:50 +0100 Subject: [PATCH 01/16] leds: st1202: Check for error code from devm_mutex_init() call MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Even if it's not critical, the avoidance of checking the error code from devm_mutex_init() call today diminishes the point of using devm variant of it. Tomorrow it may even leak something. Add the missed check. Fixes: 259230378c65 ("leds: Add LED1202 I2C driver") Signed-off-by: Thomas Weißschuh Link: https://lore.kernel.org/r/20250204-must_check-devm_mutex_init-v2-1-7b6271c4b7e6@weissschuh.net Signed-off-by: Lee Jones --- drivers/leds/leds-st1202.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c index b691c4886993..657c62cb24fa 100644 --- a/drivers/leds/leds-st1202.c +++ b/drivers/leds/leds-st1202.c @@ -356,7 +356,9 @@ static int st1202_probe(struct i2c_client *client) if (!chip) return -ENOMEM; - devm_mutex_init(&client->dev, &chip->lock); + ret = devm_mutex_init(&client->dev, &chip->lock); + if (ret < 0) + return ret; chip->client = client; ret = st1202_dt_init(chip); -- 2.50.1 From 346e704278151149e9b4c6807686ee667b911e77 Mon Sep 17 00:00:00 2001 From: Frank Li Date: Tue, 7 Jan 2025 11:16:26 -0500 Subject: [PATCH 02/16] dt-bindings: leds: Convert leds-tlc591xx.txt to yaml format Convert binding doc leds-tlc591xx.txt to yaml format to fix below DTB_CHECK warning. arch/arm64/boot/dts/freescale/imx8mp-aristainetos3-proton2s.dtb: /soc@0/bus@30800000/i2c@30a30000/tlc59108@40: failed to match any schema with compatible: ['ti,tlc59108'] Additional change: - ref to common.yaml for child nodes. - limit child's reg to 0 - 7 for ti,tlc59108. - fix typo 'linux,default_trigger' in example. - change child node name's prefix to led-. - change nodename to led-controller. - fix properties order in example. Signed-off-by: Frank Li Reviewed-by: Rob Herring (Arm) Link: https://lore.kernel.org/r/20250107161628.121685-1-Frank.Li@nxp.com Signed-off-by: Lee Jones --- .../bindings/leds/leds-tlc591xx.txt | 40 --------- .../devicetree/bindings/leds/ti,tlc59116.yaml | 90 +++++++++++++++++++ 2 files changed, 90 insertions(+), 40 deletions(-) delete mode 100644 Documentation/devicetree/bindings/leds/leds-tlc591xx.txt create mode 100644 Documentation/devicetree/bindings/leds/ti,tlc59116.yaml diff --git a/Documentation/devicetree/bindings/leds/leds-tlc591xx.txt b/Documentation/devicetree/bindings/leds/leds-tlc591xx.txt deleted file mode 100644 index 3bbbf7024411..000000000000 --- a/Documentation/devicetree/bindings/leds/leds-tlc591xx.txt +++ /dev/null @@ -1,40 +0,0 @@ -LEDs connected to tlc59116 or tlc59108 - -Required properties -- compatible: should be "ti,tlc59116" or "ti,tlc59108" -- #address-cells: must be 1 -- #size-cells: must be 0 -- reg: typically 0x68 - -Each led is represented as a sub-node of the ti,tlc59116. -See Documentation/devicetree/bindings/leds/common.txt - -LED sub-node properties: -- reg: number of LED line, 0 to 15 or 0 to 7 -- label: (optional) name of LED -- linux,default-trigger : (optional) - -Examples: - -tlc59116@68 { - #address-cells = <1>; - #size-cells = <0>; - compatible = "ti,tlc59116"; - reg = <0x68>; - - wan@0 { - label = "wrt1900ac:amber:wan"; - reg = <0x0>; - }; - - 2g@2 { - label = "wrt1900ac:white:2g"; - reg = <0x2>; - }; - - alive@9 { - label = "wrt1900ac:green:alive"; - reg = <0x9>; - linux,default_trigger = "heartbeat"; - }; -}; diff --git a/Documentation/devicetree/bindings/leds/ti,tlc59116.yaml b/Documentation/devicetree/bindings/leds/ti,tlc59116.yaml new file mode 100644 index 000000000000..ce9713793908 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/ti,tlc59116.yaml @@ -0,0 +1,90 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/ti,tlc59116.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: LEDs connected to tlc59116 or tlc59108 + +maintainers: + - Andrew Lunn + +properties: + compatible: + enum: + - ti,tlc59108 + - ti,tlc59116 + + reg: + maxItems: 1 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + +patternProperties: + "^led@[0-9a-f]$": + type: object + $ref: common.yaml# + properties: + reg: + items: + minimum: 0 + maximum: 15 + + unevaluatedProperties: false + +required: + - compatible + - reg + - "#address-cells" + - "#size-cells" + +allOf: + - if: + properties: + compatible: + contains: + const: ti,tlc59108 + then: + patternProperties: + "^led@[0-9a-f]$": + properties: + reg: + items: + maximum: 7 + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + led-controller@68 { + compatible = "ti,tlc59116"; + reg = <0x68>; + #address-cells = <1>; + #size-cells = <0>; + + led@0 { + reg = <0x0>; + label = "wrt1900ac:amber:wan"; + }; + + led@2 { + reg = <0x2>; + label = "wrt1900ac:white:2g"; + }; + + led@9 { + reg = <0x9>; + label = "wrt1900ac:green:alive"; + linux,default-trigger = "heartbeat"; + }; + }; + }; + -- 2.50.1 From ca3362a841b67a23d23c22ac16d18acec6cc75ec Mon Sep 17 00:00:00 2001 From: Eddie James Date: Wed, 12 Feb 2025 08:30:35 -0600 Subject: [PATCH 03/16] leds: pca955x: Refactor with helper functions and renaming Add helper functions to clean up the code, and rename a few oddly named functions and variables. Signed-off-by: Eddie James Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20250212143038.1416501-2-eajames@linux.ibm.com Signed-off-by: Lee Jones --- drivers/leds/leds-pca955x.c | 52 ++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c index 94a9f8a54b35..4a7111a24ab4 100644 --- a/drivers/leds/leds-pca955x.c +++ b/drivers/leds/leds-pca955x.c @@ -124,13 +124,15 @@ struct pca955x_led { struct fwnode_handle *fwnode; }; +#define led_to_pca955x(l) container_of(l, struct pca955x_led, led_cdev) + struct pca955x_platform_data { struct pca955x_led *leds; int num_leds; }; /* 8 bits per input register */ -static inline int pca95xx_num_input_regs(int bits) +static inline int pca955x_num_input_regs(int bits) { return (bits + 7) / 8; } @@ -145,6 +147,11 @@ static inline u8 pca955x_ledsel(u8 oldval, int led_num, int state) ((state & 0x3) << (led_num << 1)); } +static inline int pca955x_ledstate(u8 ls, int led_num) +{ + return (ls >> (led_num << 1)) & 0x3; +} + /* * Write to frequency prescaler register, used to program the * period of the PWM output. period = (PSCx + 1) / 38 @@ -152,7 +159,7 @@ static inline u8 pca955x_ledsel(u8 oldval, int led_num, int state) static int pca955x_write_psc(struct i2c_client *client, int n, u8 val) { struct pca955x *pca955x = i2c_get_clientdata(client); - u8 cmd = pca95xx_num_input_regs(pca955x->chipdef->bits) + (2 * n); + u8 cmd = pca955x_num_input_regs(pca955x->chipdef->bits) + (2 * n); int ret; ret = i2c_smbus_write_byte_data(client, cmd, val); @@ -172,7 +179,7 @@ static int pca955x_write_psc(struct i2c_client *client, int n, u8 val) static int pca955x_write_pwm(struct i2c_client *client, int n, u8 val) { struct pca955x *pca955x = i2c_get_clientdata(client); - u8 cmd = pca95xx_num_input_regs(pca955x->chipdef->bits) + 1 + (2 * n); + u8 cmd = pca955x_num_input_regs(pca955x->chipdef->bits) + 1 + (2 * n); int ret; ret = i2c_smbus_write_byte_data(client, cmd, val); @@ -189,7 +196,7 @@ static int pca955x_write_pwm(struct i2c_client *client, int n, u8 val) static int pca955x_write_ls(struct i2c_client *client, int n, u8 val) { struct pca955x *pca955x = i2c_get_clientdata(client); - u8 cmd = pca95xx_num_input_regs(pca955x->chipdef->bits) + 4 + n; + u8 cmd = pca955x_num_input_regs(pca955x->chipdef->bits) + 4 + n; int ret; ret = i2c_smbus_write_byte_data(client, cmd, val); @@ -206,7 +213,7 @@ static int pca955x_write_ls(struct i2c_client *client, int n, u8 val) static int pca955x_read_ls(struct i2c_client *client, int n, u8 *val) { struct pca955x *pca955x = i2c_get_clientdata(client); - u8 cmd = pca95xx_num_input_regs(pca955x->chipdef->bits) + 4 + n; + u8 cmd = pca955x_num_input_regs(pca955x->chipdef->bits) + 4 + n; int ret; ret = i2c_smbus_read_byte_data(client, cmd); @@ -222,7 +229,7 @@ static int pca955x_read_ls(struct i2c_client *client, int n, u8 *val) static int pca955x_read_pwm(struct i2c_client *client, int n, u8 *val) { struct pca955x *pca955x = i2c_get_clientdata(client); - u8 cmd = pca95xx_num_input_regs(pca955x->chipdef->bits) + 1 + (2 * n); + u8 cmd = pca955x_num_input_regs(pca955x->chipdef->bits) + 1 + (2 * n); int ret; ret = i2c_smbus_read_byte_data(client, cmd); @@ -237,9 +244,7 @@ static int pca955x_read_pwm(struct i2c_client *client, int n, u8 *val) static enum led_brightness pca955x_led_get(struct led_classdev *led_cdev) { - struct pca955x_led *pca955x_led = container_of(led_cdev, - struct pca955x_led, - led_cdev); + struct pca955x_led *pca955x_led = led_to_pca955x(led_cdev); struct pca955x *pca955x = pca955x_led->pca955x; u8 ls, pwm; int ret; @@ -248,8 +253,7 @@ static enum led_brightness pca955x_led_get(struct led_classdev *led_cdev) if (ret) return ret; - ls = (ls >> ((pca955x_led->led_num % 4) << 1)) & 0x3; - switch (ls) { + switch (pca955x_ledstate(ls, pca955x_led->led_num % 4)) { case PCA955X_LS_LED_ON: ret = LED_FULL; break; @@ -273,34 +277,28 @@ static enum led_brightness pca955x_led_get(struct led_classdev *led_cdev) static int pca955x_led_set(struct led_classdev *led_cdev, enum led_brightness value) { - struct pca955x_led *pca955x_led; - struct pca955x *pca955x; + struct pca955x_led *pca955x_led = led_to_pca955x(led_cdev); + struct pca955x *pca955x = pca955x_led->pca955x; + int reg = pca955x_led->led_num / 4; + int bit = pca955x_led->led_num % 4; u8 ls; - int chip_ls; /* which LSx to use (0-3 potentially) */ - int ls_led; /* which set of bits within LSx to use (0-3) */ int ret; - pca955x_led = container_of(led_cdev, struct pca955x_led, led_cdev); - pca955x = pca955x_led->pca955x; - - chip_ls = pca955x_led->led_num / 4; - ls_led = pca955x_led->led_num % 4; - mutex_lock(&pca955x->lock); - ret = pca955x_read_ls(pca955x->client, chip_ls, &ls); + ret = pca955x_read_ls(pca955x->client, reg, &ls); if (ret) goto out; switch (value) { case LED_FULL: - ls = pca955x_ledsel(ls, ls_led, PCA955X_LS_LED_ON); + ls = pca955x_ledsel(ls, bit, PCA955X_LS_LED_ON); break; case LED_OFF: - ls = pca955x_ledsel(ls, ls_led, PCA955X_LS_LED_OFF); + ls = pca955x_ledsel(ls, bit, PCA955X_LS_LED_OFF); break; case LED_HALF: - ls = pca955x_ledsel(ls, ls_led, PCA955X_LS_BLINK0); + ls = pca955x_ledsel(ls, bit, PCA955X_LS_BLINK0); break; default: /* @@ -313,11 +311,11 @@ static int pca955x_led_set(struct led_classdev *led_cdev, ret = pca955x_write_pwm(pca955x->client, 1, 255 - value); if (ret) goto out; - ls = pca955x_ledsel(ls, ls_led, PCA955X_LS_BLINK1); + ls = pca955x_ledsel(ls, bit, PCA955X_LS_BLINK1); break; } - ret = pca955x_write_ls(pca955x->client, chip_ls, ls); + ret = pca955x_write_ls(pca955x->client, reg, ls); out: mutex_unlock(&pca955x->lock); -- 2.50.1 From 1ddab1e2de10a37b66b235cff30e5b0a0beb8809 Mon Sep 17 00:00:00 2001 From: Eddie James Date: Wed, 12 Feb 2025 08:30:36 -0600 Subject: [PATCH 04/16] leds: pca955x: Use pointers to driver data rather than I2C client As a minor clean up item, pass the driver data pointer instead of the I2C client to the reader and writer helper functions. Now the PCA driver data doesn't have to be looked up again in the I2C client data Signed-off-by: Eddie James Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20250212143038.1416501-3-eajames@linux.ibm.com Signed-off-by: Lee Jones --- drivers/leds/leds-pca955x.c | 61 ++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 34 deletions(-) diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c index 4a7111a24ab4..4990f8aff6d1 100644 --- a/drivers/leds/leds-pca955x.c +++ b/drivers/leds/leds-pca955x.c @@ -156,16 +156,15 @@ static inline int pca955x_ledstate(u8 ls, int led_num) * Write to frequency prescaler register, used to program the * period of the PWM output. period = (PSCx + 1) / 38 */ -static int pca955x_write_psc(struct i2c_client *client, int n, u8 val) +static int pca955x_write_psc(struct pca955x *pca955x, int n, u8 val) { - struct pca955x *pca955x = i2c_get_clientdata(client); u8 cmd = pca955x_num_input_regs(pca955x->chipdef->bits) + (2 * n); int ret; - ret = i2c_smbus_write_byte_data(client, cmd, val); + ret = i2c_smbus_write_byte_data(pca955x->client, cmd, val); if (ret < 0) - dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n", - __func__, n, val, ret); + dev_err(&pca955x->client->dev, "%s: reg 0x%x, val 0x%x, err %d\n", __func__, n, + val, ret); return ret; } @@ -176,16 +175,15 @@ static int pca955x_write_psc(struct i2c_client *client, int n, u8 val) * * Duty cycle is (256 - PWMx) / 256 */ -static int pca955x_write_pwm(struct i2c_client *client, int n, u8 val) +static int pca955x_write_pwm(struct pca955x *pca955x, int n, u8 val) { - struct pca955x *pca955x = i2c_get_clientdata(client); u8 cmd = pca955x_num_input_regs(pca955x->chipdef->bits) + 1 + (2 * n); int ret; - ret = i2c_smbus_write_byte_data(client, cmd, val); + ret = i2c_smbus_write_byte_data(pca955x->client, cmd, val); if (ret < 0) - dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n", - __func__, n, val, ret); + dev_err(&pca955x->client->dev, "%s: reg 0x%x, val 0x%x, err %d\n", __func__, n, + val, ret); return ret; } @@ -193,16 +191,15 @@ static int pca955x_write_pwm(struct i2c_client *client, int n, u8 val) * Write to LED selector register, which determines the source that * drives the LED output. */ -static int pca955x_write_ls(struct i2c_client *client, int n, u8 val) +static int pca955x_write_ls(struct pca955x *pca955x, int n, u8 val) { - struct pca955x *pca955x = i2c_get_clientdata(client); u8 cmd = pca955x_num_input_regs(pca955x->chipdef->bits) + 4 + n; int ret; - ret = i2c_smbus_write_byte_data(client, cmd, val); + ret = i2c_smbus_write_byte_data(pca955x->client, cmd, val); if (ret < 0) - dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n", - __func__, n, val, ret); + dev_err(&pca955x->client->dev, "%s: reg 0x%x, val 0x%x, err %d\n", __func__, n, + val, ret); return ret; } @@ -210,32 +207,28 @@ static int pca955x_write_ls(struct i2c_client *client, int n, u8 val) * Read the LED selector register, which determines the source that * drives the LED output. */ -static int pca955x_read_ls(struct i2c_client *client, int n, u8 *val) +static int pca955x_read_ls(struct pca955x *pca955x, int n, u8 *val) { - struct pca955x *pca955x = i2c_get_clientdata(client); u8 cmd = pca955x_num_input_regs(pca955x->chipdef->bits) + 4 + n; int ret; - ret = i2c_smbus_read_byte_data(client, cmd); + ret = i2c_smbus_read_byte_data(pca955x->client, cmd); if (ret < 0) { - dev_err(&client->dev, "%s: reg 0x%x, err %d\n", - __func__, n, ret); + dev_err(&pca955x->client->dev, "%s: reg 0x%x, err %d\n", __func__, n, ret); return ret; } *val = (u8)ret; return 0; } -static int pca955x_read_pwm(struct i2c_client *client, int n, u8 *val) +static int pca955x_read_pwm(struct pca955x *pca955x, int n, u8 *val) { - struct pca955x *pca955x = i2c_get_clientdata(client); u8 cmd = pca955x_num_input_regs(pca955x->chipdef->bits) + 1 + (2 * n); int ret; - ret = i2c_smbus_read_byte_data(client, cmd); + ret = i2c_smbus_read_byte_data(pca955x->client, cmd); if (ret < 0) { - dev_err(&client->dev, "%s: reg 0x%x, err %d\n", - __func__, n, ret); + dev_err(&pca955x->client->dev, "%s: reg 0x%x, err %d\n", __func__, n, ret); return ret; } *val = (u8)ret; @@ -249,7 +242,7 @@ static enum led_brightness pca955x_led_get(struct led_classdev *led_cdev) u8 ls, pwm; int ret; - ret = pca955x_read_ls(pca955x->client, pca955x_led->led_num / 4, &ls); + ret = pca955x_read_ls(pca955x, pca955x_led->led_num / 4, &ls); if (ret) return ret; @@ -264,7 +257,7 @@ static enum led_brightness pca955x_led_get(struct led_classdev *led_cdev) ret = LED_HALF; break; case PCA955X_LS_BLINK1: - ret = pca955x_read_pwm(pca955x->client, 1, &pwm); + ret = pca955x_read_pwm(pca955x, 1, &pwm); if (ret) return ret; ret = 255 - pwm; @@ -286,7 +279,7 @@ static int pca955x_led_set(struct led_classdev *led_cdev, mutex_lock(&pca955x->lock); - ret = pca955x_read_ls(pca955x->client, reg, &ls); + ret = pca955x_read_ls(pca955x, reg, &ls); if (ret) goto out; @@ -308,14 +301,14 @@ static int pca955x_led_set(struct led_classdev *led_cdev, * OFF, HALF, or FULL. But, this is probably better than * just turning off for all other values. */ - ret = pca955x_write_pwm(pca955x->client, 1, 255 - value); + ret = pca955x_write_pwm(pca955x, 1, 255 - value); if (ret) goto out; ls = pca955x_ledsel(ls, bit, PCA955X_LS_BLINK1); break; } - ret = pca955x_write_ls(pca955x->client, reg, ls); + ret = pca955x_write_ls(pca955x, reg, ls); out: mutex_unlock(&pca955x->lock); @@ -579,22 +572,22 @@ static int pca955x_probe(struct i2c_client *client) } /* PWM0 is used for half brightness or 50% duty cycle */ - err = pca955x_write_pwm(client, 0, 255 - LED_HALF); + err = pca955x_write_pwm(pca955x, 0, 255 - LED_HALF); if (err) return err; if (!keep_pwm) { /* PWM1 is used for variable brightness, default to OFF */ - err = pca955x_write_pwm(client, 1, 0); + err = pca955x_write_pwm(pca955x, 1, 0); if (err) return err; } /* Set to fast frequency so we do not see flashing */ - err = pca955x_write_psc(client, 0, 0); + err = pca955x_write_psc(pca955x, 0, 0); if (err) return err; - err = pca955x_write_psc(client, 1, 0); + err = pca955x_write_psc(pca955x, 1, 0); if (err) return err; -- 2.50.1 From 14ef0738a31dcecfbba38626884b7d9a60b75cd0 Mon Sep 17 00:00:00 2001 From: Eddie James Date: Wed, 12 Feb 2025 08:30:37 -0600 Subject: [PATCH 05/16] leds: pca955x: Optimize probe LED selection Previously, the probe function might do up to 32 reads and writes to the same 4 registers to program the LED selection. Reduce this to a maximum of 5 operations by accumulating the changes to the LED selection and comparing with the previous value to write the selection if different. Signed-off-by: Eddie James Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20250212143038.1416501-4-eajames@linux.ibm.com Signed-off-by: Lee Jones --- drivers/leds/leds-pca955x.c | 38 +++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c index 4990f8aff6d1..eb268e6ee2a8 100644 --- a/drivers/leds/leds-pca955x.c +++ b/drivers/leds/leds-pca955x.c @@ -446,7 +446,9 @@ static int pca955x_probe(struct i2c_client *client) struct led_classdev *led; struct led_init_data init_data; struct i2c_adapter *adapter; - int i, err; + int i, bit, err, nls, reg; + u8 ls1[4]; + u8 ls2[4]; struct pca955x_platform_data *pdata; bool set_default_label = false; bool keep_pwm = false; @@ -504,6 +506,17 @@ static int pca955x_probe(struct i2c_client *client) init_data.devname_mandatory = false; init_data.devicename = "pca955x"; + nls = pca955x_num_led_regs(chip->bits); + /* Use auto-increment feature to read all the LED selectors at once. */ + err = i2c_smbus_read_i2c_block_data(client, + 0x10 | (pca955x_num_input_regs(chip->bits) + 4), nls, + ls1); + if (err < 0) + return err; + + for (i = 0; i < nls; i++) + ls2[i] = ls1[i]; + for (i = 0; i < chip->bits; i++) { pca955x_led = &pca955x->leds[i]; pca955x_led->led_num = i; @@ -515,19 +528,16 @@ static int pca955x_probe(struct i2c_client *client) case PCA955X_TYPE_GPIO: break; case PCA955X_TYPE_LED: + bit = i % 4; + reg = i / 4; led = &pca955x_led->led_cdev; led->brightness_set_blocking = pca955x_led_set; led->brightness_get = pca955x_led_get; - if (pdata->leds[i].default_state == LEDS_DEFSTATE_OFF) { - err = pca955x_led_set(led, LED_OFF); - if (err) - return err; - } else if (pdata->leds[i].default_state == LEDS_DEFSTATE_ON) { - err = pca955x_led_set(led, LED_FULL); - if (err) - return err; - } + if (pdata->leds[i].default_state == LEDS_DEFSTATE_OFF) + ls2[reg] = pca955x_ledsel(ls2[reg], bit, PCA955X_LS_LED_OFF); + else if (pdata->leds[i].default_state == LEDS_DEFSTATE_ON) + ls2[reg] = pca955x_ledsel(ls2[reg], bit, PCA955X_LS_LED_ON); init_data.fwnode = pdata->leds[i].fwnode; @@ -571,6 +581,14 @@ static int pca955x_probe(struct i2c_client *client) } } + for (i = 0; i < nls; i++) { + if (ls1[i] != ls2[i]) { + err = pca955x_write_ls(pca955x, i, ls2[i]); + if (err) + return err; + } + } + /* PWM0 is used for half brightness or 50% duty cycle */ err = pca955x_write_pwm(pca955x, 0, 255 - LED_HALF); if (err) -- 2.50.1 From 575f10dc64a251fc69a3187f4058f272eab94bfe Mon Sep 17 00:00:00 2001 From: Eddie James Date: Wed, 12 Feb 2025 08:30:38 -0600 Subject: [PATCH 06/16] leds: pca955x: Add HW blink support Support blinking using the PCA955x chip. Use PWM0 for blinking instead of LED_HALF brightness. Since there is only one frequency and brightness register for any blinking LED, track the blink state of each LED and only support one HW blinking frequency. If another frequency is requested, fallback to software blinking. In addition, blinked LEDs can only use full brightness in order to maintain 50% duty cycle, which is required for the specified blink rate. Signed-off-by: Eddie James Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20250212143038.1416501-5-eajames@linux.ibm.com Signed-off-by: Lee Jones --- drivers/leds/leds-pca955x.c | 224 +++++++++++++++++++++++++++--------- 1 file changed, 172 insertions(+), 52 deletions(-) diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c index eb268e6ee2a8..156649e1e1c4 100644 --- a/drivers/leds/leds-pca955x.c +++ b/drivers/leds/leds-pca955x.c @@ -62,6 +62,8 @@ #define PCA955X_GPIO_HIGH LED_OFF #define PCA955X_GPIO_LOW LED_FULL +#define PCA955X_BLINK_DEFAULT_MS 1000 + enum pca955x_type { pca9550, pca9551, @@ -74,6 +76,7 @@ struct pca955x_chipdef { int bits; u8 slv_addr; /* 7-bit slave address mask */ int slv_addr_shift; /* Number of bits to ignore */ + int blink_div; /* PSC divider */ }; static const struct pca955x_chipdef pca955x_chipdefs[] = { @@ -81,26 +84,31 @@ static const struct pca955x_chipdef pca955x_chipdefs[] = { .bits = 2, .slv_addr = /* 110000x */ 0x60, .slv_addr_shift = 1, + .blink_div = 44, }, [pca9551] = { .bits = 8, .slv_addr = /* 1100xxx */ 0x60, .slv_addr_shift = 3, + .blink_div = 38, }, [pca9552] = { .bits = 16, .slv_addr = /* 1100xxx */ 0x60, .slv_addr_shift = 3, + .blink_div = 44, }, [ibm_pca9552] = { .bits = 16, .slv_addr = /* 0110xxx */ 0x30, .slv_addr_shift = 3, + .blink_div = 44, }, [pca9553] = { .bits = 4, .slv_addr = /* 110001x */ 0x62, .slv_addr_shift = 1, + .blink_div = 44, }, }; @@ -109,7 +117,9 @@ struct pca955x { struct pca955x_led *leds; const struct pca955x_chipdef *chipdef; struct i2c_client *client; + unsigned long active_blink; unsigned long active_pins; + unsigned long blink_period; #ifdef CONFIG_LEDS_PCA955X_GPIO struct gpio_chip gpio; #endif @@ -154,7 +164,8 @@ static inline int pca955x_ledstate(u8 ls, int led_num) /* * Write to frequency prescaler register, used to program the - * period of the PWM output. period = (PSCx + 1) / 38 + * period of the PWM output. period = (PSCx + 1) / coeff + * Where for pca9551 chips coeff = 38 and for all other chips coeff = 44 */ static int pca955x_write_psc(struct pca955x *pca955x, int n, u8 val) { @@ -235,6 +246,21 @@ static int pca955x_read_pwm(struct pca955x *pca955x, int n, u8 *val) return 0; } +static int pca955x_read_psc(struct pca955x *pca955x, int n, u8 *val) +{ + int ret; + u8 cmd; + + cmd = pca955x_num_input_regs(pca955x->chipdef->bits) + (2 * n); + ret = i2c_smbus_read_byte_data(pca955x->client, cmd); + if (ret < 0) { + dev_err(&pca955x->client->dev, "%s: reg 0x%x, err %d\n", __func__, n, ret); + return ret; + } + *val = (u8)ret; + return 0; +} + static enum led_brightness pca955x_led_get(struct led_classdev *led_cdev) { struct pca955x_led *pca955x_led = led_to_pca955x(led_cdev); @@ -248,14 +274,12 @@ static enum led_brightness pca955x_led_get(struct led_classdev *led_cdev) switch (pca955x_ledstate(ls, pca955x_led->led_num % 4)) { case PCA955X_LS_LED_ON: + case PCA955X_LS_BLINK0: ret = LED_FULL; break; case PCA955X_LS_LED_OFF: ret = LED_OFF; break; - case PCA955X_LS_BLINK0: - ret = LED_HALF; - break; case PCA955X_LS_BLINK1: ret = pca955x_read_pwm(pca955x, 1, &pwm); if (ret) @@ -283,29 +307,36 @@ static int pca955x_led_set(struct led_classdev *led_cdev, if (ret) goto out; - switch (value) { - case LED_FULL: - ls = pca955x_ledsel(ls, bit, PCA955X_LS_LED_ON); - break; - case LED_OFF: - ls = pca955x_ledsel(ls, bit, PCA955X_LS_LED_OFF); - break; - case LED_HALF: - ls = pca955x_ledsel(ls, bit, PCA955X_LS_BLINK0); - break; - default: - /* - * Use PWM1 for all other values. This has the unwanted - * side effect of making all LEDs on the chip share the - * same brightness level if set to a value other than - * OFF, HALF, or FULL. But, this is probably better than - * just turning off for all other values. - */ - ret = pca955x_write_pwm(pca955x, 1, 255 - value); - if (ret) + if (test_bit(pca955x_led->led_num, &pca955x->active_blink)) { + if (value == LED_OFF) { + clear_bit(pca955x_led->led_num, &pca955x->active_blink); + ls = pca955x_ledsel(ls, bit, PCA955X_LS_LED_OFF); + } else { + /* No variable brightness for blinking LEDs */ goto out; - ls = pca955x_ledsel(ls, bit, PCA955X_LS_BLINK1); - break; + } + } else { + switch (value) { + case LED_FULL: + ls = pca955x_ledsel(ls, bit, PCA955X_LS_LED_ON); + break; + case LED_OFF: + ls = pca955x_ledsel(ls, bit, PCA955X_LS_LED_OFF); + break; + default: + /* + * Use PWM1 for all other values. This has the unwanted + * side effect of making all LEDs on the chip share the + * same brightness level if set to a value other than + * OFF or FULL. But, this is probably better than just + * turning off for all other values. + */ + ret = pca955x_write_pwm(pca955x, 1, 255 - value); + if (ret) + goto out; + ls = pca955x_ledsel(ls, bit, PCA955X_LS_BLINK1); + break; + } } ret = pca955x_write_ls(pca955x, reg, ls); @@ -316,6 +347,104 @@ out: return ret; } +static u8 pca955x_period_to_psc(struct pca955x *pca955x, unsigned long period) +{ + /* psc register value = (blink period * coeff) - 1 */ + period *= pca955x->chipdef->blink_div; + period /= MSEC_PER_SEC; + period -= 1; + + return period; +} + +static unsigned long pca955x_psc_to_period(struct pca955x *pca955x, u8 psc) +{ + unsigned long period = psc; + + /* blink period = (psc register value + 1) / coeff */ + period += 1; + period *= MSEC_PER_SEC; + period /= pca955x->chipdef->blink_div; + + return period; +} + +static int pca955x_led_blink(struct led_classdev *led_cdev, + unsigned long *delay_on, unsigned long *delay_off) +{ + struct pca955x_led *pca955x_led = led_to_pca955x(led_cdev); + struct pca955x *pca955x = pca955x_led->pca955x; + unsigned long period = *delay_on + *delay_off; + int ret = 0; + + mutex_lock(&pca955x->lock); + + if (period) { + if (*delay_on != *delay_off) { + ret = -EINVAL; + goto out; + } + + if (period < pca955x_psc_to_period(pca955x, 0) || + period > pca955x_psc_to_period(pca955x, 0xff)) { + ret = -EINVAL; + goto out; + } + } else { + period = pca955x->active_blink ? pca955x->blink_period : + PCA955X_BLINK_DEFAULT_MS; + } + + if (!pca955x->active_blink || + pca955x->active_blink == BIT(pca955x_led->led_num) || + pca955x->blink_period == period) { + u8 psc = pca955x_period_to_psc(pca955x, period); + + if (!test_and_set_bit(pca955x_led->led_num, + &pca955x->active_blink)) { + u8 ls; + int reg = pca955x_led->led_num / 4; + int bit = pca955x_led->led_num % 4; + + ret = pca955x_read_ls(pca955x, reg, &ls); + if (ret) + goto out; + + ls = pca955x_ledsel(ls, bit, PCA955X_LS_BLINK0); + ret = pca955x_write_ls(pca955x, reg, ls); + if (ret) + goto out; + + /* + * Force 50% duty cycle to maintain the specified + * blink rate. + */ + ret = pca955x_write_pwm(pca955x, 0, 128); + if (ret) + goto out; + } + + if (pca955x->blink_period != period) { + pca955x->blink_period = period; + ret = pca955x_write_psc(pca955x, 0, psc); + if (ret) + goto out; + } + + period = pca955x_psc_to_period(pca955x, psc); + period /= 2; + *delay_on = period; + *delay_off = period; + } else { + ret = -EBUSY; + } + +out: + mutex_unlock(&pca955x->lock); + + return ret; +} + #ifdef CONFIG_LEDS_PCA955X_GPIO /* * Read the INPUT register, which contains the state of LEDs. @@ -450,8 +579,9 @@ static int pca955x_probe(struct i2c_client *client) u8 ls1[4]; u8 ls2[4]; struct pca955x_platform_data *pdata; + u8 psc0; + bool keep_psc0 = false; bool set_default_label = false; - bool keep_pwm = false; char default_label[8]; chip = i2c_get_match_data(client); @@ -502,6 +632,7 @@ static int pca955x_probe(struct i2c_client *client) mutex_init(&pca955x->lock); pca955x->client = client; pca955x->chipdef = chip; + pca955x->blink_period = PCA955X_BLINK_DEFAULT_MS; init_data.devname_mandatory = false; init_data.devicename = "pca955x"; @@ -533,11 +664,16 @@ static int pca955x_probe(struct i2c_client *client) led = &pca955x_led->led_cdev; led->brightness_set_blocking = pca955x_led_set; led->brightness_get = pca955x_led_get; + led->blink_set = pca955x_led_blink; if (pdata->leds[i].default_state == LEDS_DEFSTATE_OFF) ls2[reg] = pca955x_ledsel(ls2[reg], bit, PCA955X_LS_LED_OFF); else if (pdata->leds[i].default_state == LEDS_DEFSTATE_ON) ls2[reg] = pca955x_ledsel(ls2[reg], bit, PCA955X_LS_LED_ON); + else if (pca955x_ledstate(ls2[reg], bit) == PCA955X_LS_BLINK0) { + keep_psc0 = true; + set_bit(i, &pca955x->active_blink); + } init_data.fwnode = pdata->leds[i].fwnode; @@ -565,19 +701,6 @@ static int pca955x_probe(struct i2c_client *client) return err; set_bit(i, &pca955x->active_pins); - - /* - * For default-state == "keep", let the core update the - * brightness from the hardware, then check the - * brightness to see if it's using PWM1. If so, PWM1 - * should not be written below. - */ - if (pdata->leds[i].default_state == LEDS_DEFSTATE_KEEP) { - if (led->brightness != LED_FULL && - led->brightness != LED_OFF && - led->brightness != LED_HALF) - keep_pwm = true; - } } } @@ -589,22 +712,19 @@ static int pca955x_probe(struct i2c_client *client) } } - /* PWM0 is used for half brightness or 50% duty cycle */ - err = pca955x_write_pwm(pca955x, 0, 255 - LED_HALF); - if (err) - return err; - - if (!keep_pwm) { - /* PWM1 is used for variable brightness, default to OFF */ - err = pca955x_write_pwm(pca955x, 1, 0); - if (err) - return err; + if (keep_psc0) { + err = pca955x_read_psc(pca955x, 0, &psc0); + } else { + psc0 = pca955x_period_to_psc(pca955x, pca955x->blink_period); + err = pca955x_write_psc(pca955x, 0, psc0); } - /* Set to fast frequency so we do not see flashing */ - err = pca955x_write_psc(pca955x, 0, 0); if (err) return err; + + pca955x->blink_period = pca955x_psc_to_period(pca955x, psc0); + + /* Set PWM1 to fast frequency so we do not see flashing */ err = pca955x_write_psc(pca955x, 1, 0); if (err) return err; -- 2.50.1 From 9ec336ba05f6786814696deef637ab2b9f6d0f10 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Barnab=C3=A1s=20Cz=C3=A9m=C3=A1n?= Date: Thu, 13 Feb 2025 20:54:47 +0100 Subject: [PATCH 07/16] dt-bindings: leds: qcom-lpg: Document PM8937 PWM compatible MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The PM8937 PWM modules are compatible with the PM8916 PWM modules, document the PM8937 PWM compatible as fallback for the PM8916 PWM. Signed-off-by: Barnabás Czémán Reviewed-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20250213-pm8937-pwm-v2-1-49ea59801a33@mainlining.org Signed-off-by: Lee Jones --- Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml index 8b82c45d1a48..841a0229c472 100644 --- a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml @@ -39,6 +39,10 @@ properties: - enum: - qcom,pm8550-pwm - const: qcom,pm8350c-pwm + - items: + - enum: + - qcom,pm8937-pwm + - const: qcom,pm8916-pwm "#pwm-cells": const: 2 -- 2.50.1 From 2f372a5dce6885f1d2647f7add01756bee0fef49 Mon Sep 17 00:00:00 2001 From: Pei Xiao Date: Wed, 19 Feb 2025 10:42:02 +0800 Subject: [PATCH 08/16] leds: st1202: Refactor st1202_led_set() to use !! operator for boolean conversion st1202_led_set function now uses the !! operator to convert the enum led_brightness() value to a boolean active state, which is then passed to the st1202_channel_set() function. This change maintains the existing functionality. cocci warnings: drivers/leds/leds-st1202.c:194:66-71: WARNING: conversion to bool not needed here. Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202502181845.xESVrC61-lkp@intel.com/ Signed-off-by: Pei Xiao Link: https://lore.kernel.org/r/tencent_3DF7518D407679C99C4CCCB1B8E64638700A@qq.com Signed-off-by: Lee Jones --- drivers/leds/leds-st1202.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c index 657c62cb24fa..360e9db78dc1 100644 --- a/drivers/leds/leds-st1202.c +++ b/drivers/leds/leds-st1202.c @@ -189,9 +189,8 @@ static int st1202_channel_set(struct st1202_chip *chip, int led_num, bool active static int st1202_led_set(struct led_classdev *ldev, enum led_brightness value) { struct st1202_led *led = cdev_to_st1202_led(ldev); - struct st1202_chip *chip = led->chip; - return st1202_channel_set(chip, led->led_num, value == LED_OFF ? false : true); + return st1202_channel_set(led->chip, led->led_num, !!value); } static int st1202_led_pattern_clear(struct led_classdev *ldev) -- 2.50.1 From 012825dbd5aa7454b93f7a17bd99efee114023ba Mon Sep 17 00:00:00 2001 From: Eddie James Date: Fri, 21 Feb 2025 09:51:44 -0600 Subject: [PATCH 09/16] Revert "leds-pca955x: Remove the unused function pca95xx_num_led_regs()" This reverts commit 38bcb51f81af17a6d40fc135e565fc1fb8aa8e9d. This function is needed by the hardware blink support just introduced. Also rename the function to pca955x_num_led_regs() to match the rest of the functions in the driver. Signed-off-by: Eddie James Link: https://lore.kernel.org/r/20250221155144.2109806-1-eajames@linux.ibm.com Signed-off-by: Lee Jones --- drivers/leds/leds-pca955x.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c index 156649e1e1c4..e9cfde9fe4b1 100644 --- a/drivers/leds/leds-pca955x.c +++ b/drivers/leds/leds-pca955x.c @@ -147,6 +147,12 @@ static inline int pca955x_num_input_regs(int bits) return (bits + 7) / 8; } +/* 4 bits per LED selector register */ +static inline int pca955x_num_led_regs(int bits) +{ + return (bits + 3) / 4; +} + /* * Return an LED selector register value based on an existing one, with * the appropriate 2-bit state value set for the given LED number (0-3). -- 2.50.1 From 2c70953b6f535f7698ccbf22c1f5ba26cb6c2816 Mon Sep 17 00:00:00 2001 From: Remi Pommarel Date: Thu, 20 Feb 2025 12:23:17 +0100 Subject: [PATCH 10/16] leds: Fix LED_OFF brightness race While commit fa15d8c69238 ("leds: Fix set_brightness_delayed() race") successfully forces led_set_brightness() to be called with LED_OFF at least once when switching from blinking to LED on state so that hw-blinking can be disabled, another race remains. Indeed in led_set_brightness(LED_OFF) followed by led_set_brightness(any) scenario the following CPU scheduling can happen: CPU0 CPU1 ---- ---- set_brightness_delayed() { test_and_clear_bit(BRIGHTNESS_OFF) led_set_brightness(LED_OFF) { set_bit(BRIGHTNESS_OFF) queue_work() } led_set_brightness(any) { set_bit(BRIGHTNESS) queue_work() //already queued } test_and_clear_bit(BRIGHTNESS) /* LED set with brightness any */ } /* From previous CPU1 queue_work() */ set_brightness_delayed() { test_and_clear_bit(BRIGHTNESS_OFF) /* LED turned off */ test_and_clear_bit(BRIGHTNESS) /* Clear from previous run, LED remains off */ In that case the led_set_brightness(LED_OFF)/led_set_brightness(any) sequence will be effectively executed in reverse order and LED will remain off. With the introduction of commit 32360bf6a5d4 ("leds: Introduce ordered workqueue for LEDs events instead of system_wq") the race is easier to trigger as sysfs brightness configuration does not wait for set_brightness_delayed() work to finish (flush_work() removal). Use delayed_set_value to optionnally re-configure brightness after a LED_OFF. That way a LED state could be configured more that once but final state will always be as expected. Ensure that delayed_set_value modification is seen before set_bit() using smp_mb__before_atomic(). Fixes: fa15d8c69238 ("leds: Fix set_brightness_delayed() race") Signed-off-by: Remi Pommarel Reviewed-by: Hans de Goede Link: https://lore.kernel.org/r/19c81177059dab7b656c42063958011a8e4d1a66.1740050412.git.repk@triplefau.lt Signed-off-by: Lee Jones --- drivers/leds/led-core.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c index f6c46d2e5276..e3d8ddcff567 100644 --- a/drivers/leds/led-core.c +++ b/drivers/leds/led-core.c @@ -159,8 +159,19 @@ static void set_brightness_delayed(struct work_struct *ws) * before this work item runs once. To make sure this works properly * handle LED_SET_BRIGHTNESS_OFF first. */ - if (test_and_clear_bit(LED_SET_BRIGHTNESS_OFF, &led_cdev->work_flags)) + if (test_and_clear_bit(LED_SET_BRIGHTNESS_OFF, &led_cdev->work_flags)) { set_brightness_delayed_set_brightness(led_cdev, LED_OFF); + /* + * The consecutives led_set_brightness(LED_OFF), + * led_set_brightness(LED_FULL) could have been executed out of + * order (LED_FULL first), if the work_flags has been set + * between LED_SET_BRIGHTNESS_OFF and LED_SET_BRIGHTNESS of this + * work. To avoid ending with the LED turned off, turn the LED + * on again. + */ + if (led_cdev->delayed_set_value != LED_OFF) + set_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags); + } if (test_and_clear_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags)) set_brightness_delayed_set_brightness(led_cdev, led_cdev->delayed_set_value); @@ -331,10 +342,13 @@ void led_set_brightness_nopm(struct led_classdev *led_cdev, unsigned int value) * change is done immediately afterwards (before the work runs), * it uses a separate work_flag. */ - if (value) { - led_cdev->delayed_set_value = value; + led_cdev->delayed_set_value = value; + /* Ensure delayed_set_value is seen before work_flags modification */ + smp_mb__before_atomic(); + + if (value) set_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags); - } else { + else { clear_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags); clear_bit(LED_SET_BLINK, &led_cdev->work_flags); set_bit(LED_SET_BRIGHTNESS_OFF, &led_cdev->work_flags); -- 2.50.1 From 7a3350495d9ae8ae5b178d603449d18fa7150560 Mon Sep 17 00:00:00 2001 From: Anjelique Melendez Date: Wed, 12 Feb 2025 16:35:33 -0800 Subject: [PATCH 11/16] leds: rgb: leds-qcom-lpg: Add support for 6-bit PWM resolution Currently, driver only allows for PWM modules to use 9-bit resolution. However, PWM modules can support 6-bit and 9-bit resolution. Add support for 6-bit resolution. Suggested-by: Zejiong Huang Signed-off-by: Anjelique Melendez Link: https://lore.kernel.org/r/20250213003533.1684131-1-anjelique.melendez@oss.qualcomm.com Signed-off-by: Lee Jones --- drivers/leds/rgb/leds-qcom-lpg.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c index f3c9ef2bfa57..4e5c56ded1f0 100644 --- a/drivers/leds/rgb/leds-qcom-lpg.c +++ b/drivers/leds/rgb/leds-qcom-lpg.c @@ -24,6 +24,7 @@ #define LPG_PATTERN_CONFIG_REG 0x40 #define LPG_SIZE_CLK_REG 0x41 #define PWM_CLK_SELECT_MASK GENMASK(1, 0) +#define PWM_SIZE_SELECT_MASK BIT(2) #define PWM_CLK_SELECT_HI_RES_MASK GENMASK(2, 0) #define PWM_SIZE_HI_RES_MASK GENMASK(6, 4) #define LPG_PREDIV_CLK_REG 0x42 @@ -412,8 +413,8 @@ static int lpg_lut_sync(struct lpg *lpg, unsigned int mask) static const unsigned int lpg_clk_rates[] = {0, 1024, 32768, 19200000}; static const unsigned int lpg_clk_rates_hi_res[] = {0, 1024, 32768, 19200000, 76800000}; static const unsigned int lpg_pre_divs[] = {1, 3, 5, 6}; -static const unsigned int lpg_pwm_resolution[] = {9}; -static const unsigned int lpg_pwm_resolution_hi_res[] = {8, 9, 10, 11, 12, 13, 14, 15}; +static const unsigned int lpg_pwm_resolution[] = {6, 9}; +static const unsigned int lpg_pwm_resolution_hi_res[] = {8, 9, 10, 11, 12, 13, 14, 15}; static int lpg_calc_freq(struct lpg_channel *chan, uint64_t period) { @@ -436,12 +437,12 @@ static int lpg_calc_freq(struct lpg_channel *chan, uint64_t period) * period = -------------------------- * refclk * - * Resolution = 2^9 bits for PWM or + * Resolution = 2^{6 or 9} bits for PWM or * 2^{8, 9, 10, 11, 12, 13, 14, 15} bits for high resolution PWM * pre_div = {1, 3, 5, 6} and * M = [0..7]. * - * This allows for periods between 27uS and 384s for PWM channels and periods between + * This allows for periods between 3uS and 384s for PWM channels and periods between * 3uS and 24576s for high resolution PWMs. * The PWM framework wants a period of equal or lower length than requested, * reject anything below minimum period. @@ -558,7 +559,7 @@ static void lpg_apply_freq(struct lpg_channel *chan) val |= GENMASK(5, 4); break; case LPG_SUBTYPE_PWM: - val |= BIT(2); + val |= FIELD_PREP(PWM_SIZE_SELECT_MASK, chan->pwm_resolution_sel); break; case LPG_SUBTYPE_HI_RES_PWM: val |= FIELD_PREP(PWM_SIZE_HI_RES_MASK, chan->pwm_resolution_sel); @@ -1276,7 +1277,7 @@ static int lpg_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, resolution = lpg_pwm_resolution_hi_res[FIELD_GET(PWM_SIZE_HI_RES_MASK, val)]; } else { refclk = lpg_clk_rates[FIELD_GET(PWM_CLK_SELECT_MASK, val)]; - resolution = 9; + resolution = lpg_pwm_resolution[FIELD_GET(PWM_SIZE_SELECT_MASK, val)]; } if (refclk) { -- 2.50.1 From 6d91124e7edc109f114b1afe6d00d85d0d0ac174 Mon Sep 17 00:00:00 2001 From: Yuanjun Gong Date: Sun, 23 Feb 2025 20:14:59 +0800 Subject: [PATCH 12/16] leds: pwm-multicolor: Add check for fwnode_property_read_u32 Add a check to the return value of fwnode_property_read_u32() in case it fails. Signed-off-by: Yuanjun Gong Link: https://lore.kernel.org/r/20250223121459.2889484-1-ruc_gongyuanjun@163.com Signed-off-by: Lee Jones --- drivers/leds/rgb/leds-pwm-multicolor.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c b/drivers/leds/rgb/leds-pwm-multicolor.c index f80a06cc31f8..1c7705bafdfc 100644 --- a/drivers/leds/rgb/leds-pwm-multicolor.c +++ b/drivers/leds/rgb/leds-pwm-multicolor.c @@ -141,8 +141,11 @@ static int led_pwm_mc_probe(struct platform_device *pdev) /* init the multicolor's LED class device */ cdev = &priv->mc_cdev.led_cdev; - fwnode_property_read_u32(mcnode, "max-brightness", + ret = fwnode_property_read_u32(mcnode, "max-brightness", &cdev->max_brightness); + if (ret) + goto release_mcnode; + cdev->flags = LED_CORE_SUSPENDRESUME; cdev->brightness_set_blocking = led_pwm_mc_set; -- 2.50.1 From a17d9e736ddd78323e77d3066c1e86371a99023c Mon Sep 17 00:00:00 2001 From: Manuel Fombuena Date: Wed, 26 Feb 2025 17:06:40 +0000 Subject: [PATCH 13/16] leds: leds-st1202: Initialize hardware before DT node child operations Arguably, there are more chances of errors occurring during the initialization of the hardware, so this should complete successfully before the devicetree node's children are initialized. st1202_dt_init() fills the led_classdev struct. st1202_setup() initializes the hardware. Specifically, resets the chip, enables its phase-shift delay feature, enables the device and disables all the LEDs channels. All that writing to registers, with no input from st1202_dt_init(). Real-world testing corroborates that calling st1202_setup() before st1202_dt_init() doesn't cause any issue during initialization. Switch the order of st1202_dt_init() and st1202_setup() to ensure the hardware is correctly initialized before the led_classdev struct is filled. Signed-off-by: Manuel Fombuena Link: https://lore.kernel.org/r/CWLP123MB54731877A8DC54EDD33F0229C5C22@CWLP123MB5473.GBRP123.PROD.OUTLOOK.COM Signed-off-by: Lee Jones --- drivers/leds/leds-st1202.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c index 360e9db78dc1..3ddf2786a745 100644 --- a/drivers/leds/leds-st1202.c +++ b/drivers/leds/leds-st1202.c @@ -360,11 +360,11 @@ static int st1202_probe(struct i2c_client *client) return ret; chip->client = client; - ret = st1202_dt_init(chip); + ret = st1202_setup(chip); if (ret < 0) return ret; - ret = st1202_setup(chip); + ret = st1202_dt_init(chip); if (ret < 0) return ret; -- 2.50.1 From 5d0e4816a9e7ee663e3f32e96b45d79aa900c398 Mon Sep 17 00:00:00 2001 From: Manuel Fombuena Date: Wed, 26 Feb 2025 17:08:44 +0000 Subject: [PATCH 14/16] leds: leds-st1202: Spacing and proofreading editing Minor edits regarding use of spacing and proofreading. There is a minor inconsistency in the use of spacing as margin in one of the comments providing details about the datasheet. There is also a typo that comes from the datasheet itself. Change spacing on comment and correct typo. Signed-off-by: Manuel Fombuena Link: https://lore.kernel.org/r/CWLP123MB547333EFFFBFFA840225BC02C5C22@CWLP123MB5473.GBRP123.PROD.OUTLOOK.COM Signed-off-by: Lee Jones --- drivers/leds/leds-st1202.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c index 3ddf2786a745..eb75c2d36d47 100644 --- a/drivers/leds/leds-st1202.c +++ b/drivers/leds/leds-st1202.c @@ -99,9 +99,9 @@ static int st1202_pwm_pattern_write(struct st1202_chip *chip, int led_num, value_h = (u8)(value >> 8); /* - * Datasheet: Register address low = 1Eh + 2*(xh) + 18h*(yh), - * where x is the channel number (led number) in hexadecimal (x = 00h .. 0Bh) - * and y is the pattern number in hexadecimal (y = 00h .. 07h) + * Datasheet: Register address low = 1Eh + 2*(xh) + 18h*(yh), + * where x is the channel number (led number) in hexadecimal (x = 00h .. 0Bh) + * and y is the pattern number in hexadecimal (y = 00h .. 07h) */ ret = st1202_write_reg(chip, (ST1202_PATTERN_PWM + (led_num * 2) + 0x18 * pattern), value_l); @@ -298,8 +298,8 @@ static int st1202_setup(struct st1202_chip *chip) guard(mutex)(&chip->lock); /* - * Once the supply voltage is applied, the LED1202 executes some internal checks, - * afterwords it stops the oscillator and puts the internal LDO in quiescent mode. + * Once the supply voltage is applied, the LED1202 executes some internal checks. + * Afterwards, it stops the oscillator and puts the internal LDO in quiescent mode. * To start the device, EN bit must be set inside the “Device Enable” register at * address 01h. As soon as EN is set, the LED1202 loads the adjustment parameters * from the internal non-volatile memory and performs an auto-calibration procedure -- 2.50.1 From be2f92844d0f8cb059cb6958c6d9582d381ca68e Mon Sep 17 00:00:00 2001 From: Manuel Fombuena Date: Wed, 26 Feb 2025 17:12:50 +0000 Subject: [PATCH 15/16] leds: Kconfig: leds-st1202: Add select for required LEDS_TRIGGER_PATTERN leds-st1202 requires the LED Pattern Trigger (LEDS_TRIGGER_PATTERN), which is not selected when LED Trigger support is (LEDS_TRIGGERS). To reproduce this: - make menuconfig KCONFIG_CONFIG= - select LEDS_ST1202 dependencies OF, I2C and LEDS_CLASS. - select LEDS_ST1202 - LEDS_TRIGGERS is selected but LEDS_TRIGGER_PATTERN isn't. The absence of LEDS_TRIGGER_PATTERN explicitly required can lead to builds in which LEDS_ST1202 is selected while LEDS_TRIGGER_PATTERN isn't. The direct result of that would be that /sys/class/leds//hw_pattern wouldn't be available and there would be no way of interacting with the driver and hardware from user space. Add select LEDS_TRIGGER_PATTERN to Kconfig to meet the requirement and indirectly document it as well. Signed-off-by: Manuel Fombuena Link: https://lore.kernel.org/r/CWLP123MB5473F4DF3A668F7DD057A280C5C22@CWLP123MB5473.GBRP123.PROD.OUTLOOK.COM Signed-off-by: Lee Jones --- drivers/leds/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 2b27d043921c..8859e8fe292a 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -971,6 +971,7 @@ config LEDS_ST1202 depends on I2C depends on OF select LEDS_TRIGGERS + select LEDS_TRIGGER_PATTERN help Say Y to enable support for LEDs connected to LED1202 LED driver chips accessed via the I2C bus. -- 2.50.1 From 835a0c10d33b54607f49edffbbeaea4c4cdcc49c Mon Sep 17 00:00:00 2001 From: Lee Jones Date: Fri, 7 Mar 2025 10:01:10 -0800 Subject: [PATCH 16/16] leds: Rename simple directory to simatic The drivers contained in this directory are not simplistic. Signed-off-by: Lee Jones --- drivers/leds/Kconfig | 4 ++-- drivers/leds/Makefile | 4 ++-- drivers/leds/{simple => simatic}/Kconfig | 0 drivers/leds/{simple => simatic}/Makefile | 0 .../{simple => simatic}/simatic-ipc-leds-gpio-apollolake.c | 0 drivers/leds/{simple => simatic}/simatic-ipc-leds-gpio-core.c | 0 .../{simple => simatic}/simatic-ipc-leds-gpio-elkhartlake.c | 0 .../leds/{simple => simatic}/simatic-ipc-leds-gpio-f7188x.c | 0 drivers/leds/{simple => simatic}/simatic-ipc-leds-gpio.h | 0 drivers/leds/{simple => simatic}/simatic-ipc-leds.c | 0 10 files changed, 4 insertions(+), 4 deletions(-) rename drivers/leds/{simple => simatic}/Kconfig (100%) rename drivers/leds/{simple => simatic}/Makefile (100%) rename drivers/leds/{simple => simatic}/simatic-ipc-leds-gpio-apollolake.c (100%) rename drivers/leds/{simple => simatic}/simatic-ipc-leds-gpio-core.c (100%) rename drivers/leds/{simple => simatic}/simatic-ipc-leds-gpio-elkhartlake.c (100%) rename drivers/leds/{simple => simatic}/simatic-ipc-leds-gpio-f7188x.c (100%) rename drivers/leds/{simple => simatic}/simatic-ipc-leds-gpio.h (100%) rename drivers/leds/{simple => simatic}/simatic-ipc-leds.c (100%) diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 8859e8fe292a..4f30e89e7e86 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -1015,7 +1015,7 @@ source "drivers/leds/rgb/Kconfig" comment "LED Triggers" source "drivers/leds/trigger/Kconfig" -comment "Simple LED drivers" -source "drivers/leds/simple/Kconfig" +comment "Simatic LED drivers" +source "drivers/leds/simatic/Kconfig" endif # NEW_LEDS diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 6ad52e219ec6..fd4e2873478c 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -121,5 +121,5 @@ obj-$(CONFIG_LEDS_TRIGGERS) += trigger/ # LED Blink obj-y += blink/ -# Simple LED drivers -obj-y += simple/ +# Simatic LED drivers +obj-y += simatic/ diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simatic/Kconfig similarity index 100% rename from drivers/leds/simple/Kconfig rename to drivers/leds/simatic/Kconfig diff --git a/drivers/leds/simple/Makefile b/drivers/leds/simatic/Makefile similarity index 100% rename from drivers/leds/simple/Makefile rename to drivers/leds/simatic/Makefile diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio-apollolake.c b/drivers/leds/simatic/simatic-ipc-leds-gpio-apollolake.c similarity index 100% rename from drivers/leds/simple/simatic-ipc-leds-gpio-apollolake.c rename to drivers/leds/simatic/simatic-ipc-leds-gpio-apollolake.c diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio-core.c b/drivers/leds/simatic/simatic-ipc-leds-gpio-core.c similarity index 100% rename from drivers/leds/simple/simatic-ipc-leds-gpio-core.c rename to drivers/leds/simatic/simatic-ipc-leds-gpio-core.c diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio-elkhartlake.c b/drivers/leds/simatic/simatic-ipc-leds-gpio-elkhartlake.c similarity index 100% rename from drivers/leds/simple/simatic-ipc-leds-gpio-elkhartlake.c rename to drivers/leds/simatic/simatic-ipc-leds-gpio-elkhartlake.c diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.c b/drivers/leds/simatic/simatic-ipc-leds-gpio-f7188x.c similarity index 100% rename from drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.c rename to drivers/leds/simatic/simatic-ipc-leds-gpio-f7188x.c diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio.h b/drivers/leds/simatic/simatic-ipc-leds-gpio.h similarity index 100% rename from drivers/leds/simple/simatic-ipc-leds-gpio.h rename to drivers/leds/simatic/simatic-ipc-leds-gpio.h diff --git a/drivers/leds/simple/simatic-ipc-leds.c b/drivers/leds/simatic/simatic-ipc-leds.c similarity index 100% rename from drivers/leds/simple/simatic-ipc-leds.c rename to drivers/leds/simatic/simatic-ipc-leds.c -- 2.50.1