openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH linux dev-5.10 0/7] leds: Support retaining state for the PCA955x
@ 2021-09-16 21:21 Eddie James
  2021-09-16 21:21 ` [PATCH linux dev-5.10 1/7] dt-bindings: leds: Add retain-state-shutdown boolean Eddie James
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Eddie James @ 2021-09-16 21:21 UTC (permalink / raw)
  To: joel; +Cc: openbmc

This series implements the ability to retain the state of the LEDs
controlled by the PCA955x across system reboots. This includes a
change to the LED core driver to respect the retain-state-shutdown
device tree property. It also cleans up the PCA955x driver, adds the
ability to query the hardware LED brightness, switches the I2C probe
function to probe_new, and uses more core functionality for parsing
the fwnode.

This series has been applied to linux-next.

Eddie James (7):
  dt-bindings: leds: Add retain-state-shutdown boolean
  leds: leds-core: Implement the retain-state-shutdown property
  leds: pca955x: Clean up code formatting
  leds: pca955x: Add brightness_get function
  leds: pca955x: Implement the default-state property
  leds: pca955x: Let the core process the fwnode
  leds: pca955x: Switch to i2c probe_new

 .../devicetree/bindings/leds/common.yaml      |   6 +
 drivers/leds/led-class.c                      |  10 +-
 drivers/leds/leds-pca955x.c                   | 232 +++++++++++++-----
 3 files changed, 182 insertions(+), 66 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH linux dev-5.10 1/7] dt-bindings: leds: Add retain-state-shutdown boolean
  2021-09-16 21:21 [PATCH linux dev-5.10 0/7] leds: Support retaining state for the PCA955x Eddie James
@ 2021-09-16 21:21 ` Eddie James
  2021-09-16 21:21 ` [PATCH linux dev-5.10 2/7] leds: leds-core: Implement the retain-state-shutdown property Eddie James
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Eddie James @ 2021-09-16 21:21 UTC (permalink / raw)
  To: joel; +Cc: openbmc

Document the retain-state-shutdown property that indicates that a LED
should not be turned off or changed during system shutdown.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
---
 Documentation/devicetree/bindings/leds/common.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
index f1211e7045f1..fd74d33291da 100644
--- a/Documentation/devicetree/bindings/leds/common.yaml
+++ b/Documentation/devicetree/bindings/leds/common.yaml
@@ -128,6 +128,12 @@ properties:
       as a panic indicator.
     type: boolean
 
+  retain-state-shutdown:
+    description:
+      This property specifies that the LED should not be turned off or changed
+      when the system shuts down.
+    type: boolean
+
   trigger-sources:
     description: |
       List of devices which should be used as a source triggering this LED
-- 
2.27.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH linux dev-5.10 2/7] leds: leds-core: Implement the retain-state-shutdown property
  2021-09-16 21:21 [PATCH linux dev-5.10 0/7] leds: Support retaining state for the PCA955x Eddie James
  2021-09-16 21:21 ` [PATCH linux dev-5.10 1/7] dt-bindings: leds: Add retain-state-shutdown boolean Eddie James
@ 2021-09-16 21:21 ` Eddie James
  2021-09-16 21:21 ` [PATCH linux dev-5.10 3/7] leds: pca955x: Clean up code formatting Eddie James
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Eddie James @ 2021-09-16 21:21 UTC (permalink / raw)
  To: joel; +Cc: openbmc

Read the retain-state-shutdown device tree property to set the
existing LED_RETAIN_AT_SHUTDOWN flag. Then check the flag when
unregistering, and if set, don't set the brightness to OFF. This
is useful for systems that want to keep the HW state of the LED
across reboots.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/led-class.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 4365c1cc4505..bc66f1035d06 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -351,10 +351,15 @@ int led_classdev_register_ext(struct device *parent,
 		if (ret < 0)
 			return ret;
 
-		if (init_data->fwnode)
+		if (init_data->fwnode) {
 			fwnode_property_read_string(init_data->fwnode,
 				"linux,default-trigger",
 				&led_cdev->default_trigger);
+
+			if (fwnode_property_present(init_data->fwnode,
+						    "retain-state-shutdown"))
+				led_cdev->flags |= LED_RETAIN_AT_SHUTDOWN;
+		}
 	} else {
 		proposed_name = led_cdev->name;
 	}
@@ -445,7 +450,8 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
 	/* Stop blinking */
 	led_stop_software_blink(led_cdev);
 
-	led_set_brightness(led_cdev, LED_OFF);
+	if (!(led_cdev->flags & LED_RETAIN_AT_SHUTDOWN))
+		led_set_brightness(led_cdev, LED_OFF);
 
 	flush_work(&led_cdev->set_brightness_work);
 
-- 
2.27.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH linux dev-5.10 3/7] leds: pca955x: Clean up code formatting
  2021-09-16 21:21 [PATCH linux dev-5.10 0/7] leds: Support retaining state for the PCA955x Eddie James
  2021-09-16 21:21 ` [PATCH linux dev-5.10 1/7] dt-bindings: leds: Add retain-state-shutdown boolean Eddie James
  2021-09-16 21:21 ` [PATCH linux dev-5.10 2/7] leds: leds-core: Implement the retain-state-shutdown property Eddie James
@ 2021-09-16 21:21 ` Eddie James
  2021-09-16 21:21 ` [PATCH linux dev-5.10 4/7] leds: pca955x: Add brightness_get function Eddie James
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Eddie James @ 2021-09-16 21:21 UTC (permalink / raw)
  To: joel; +Cc: openbmc

Format the code. Add some variables to help shorten lines.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/leds-pca955x.c | 63 ++++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 33 deletions(-)

diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index 7087ca4592fc..f0d841cb59fc 100644
--- a/drivers/leds/leds-pca955x.c
+++ b/drivers/leds/leds-pca955x.c
@@ -166,11 +166,10 @@ 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);
 	int ret;
 
-	ret = i2c_smbus_write_byte_data(client,
-		pca95xx_num_input_regs(pca955x->chipdef->bits) + 2*n,
-		val);
+	ret = i2c_smbus_write_byte_data(client, cmd, val);
 	if (ret < 0)
 		dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n",
 			__func__, n, val, ret);
@@ -187,11 +186,10 @@ 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);
 	int ret;
 
-	ret = i2c_smbus_write_byte_data(client,
-		pca95xx_num_input_regs(pca955x->chipdef->bits) + 1 + 2*n,
-		val);
+	ret = i2c_smbus_write_byte_data(client, cmd, val);
 	if (ret < 0)
 		dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n",
 			__func__, n, val, ret);
@@ -205,11 +203,10 @@ 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;
 	int ret;
 
-	ret = i2c_smbus_write_byte_data(client,
-		pca95xx_num_input_regs(pca955x->chipdef->bits) + 4 + n,
-		val);
+	ret = i2c_smbus_write_byte_data(client, cmd, val);
 	if (ret < 0)
 		dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n",
 			__func__, n, val, ret);
@@ -223,10 +220,10 @@ 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;
 	int ret;
 
-	ret = i2c_smbus_read_byte_data(client,
-		pca95xx_num_input_regs(pca955x->chipdef->bits) + 4 + n);
+	ret = i2c_smbus_read_byte_data(client, cmd);
 	if (ret < 0) {
 		dev_err(&client->dev, "%s: reg 0x%x, err %d\n",
 			__func__, n, ret);
@@ -371,6 +368,7 @@ static struct pca955x_platform_data *
 pca955x_get_pdata(struct i2c_client *client, struct pca955x_chipdef *chip)
 {
 	struct pca955x_platform_data *pdata;
+	struct pca955x_led *led;
 	struct fwnode_handle *child;
 	int count;
 
@@ -401,13 +399,13 @@ pca955x_get_pdata(struct i2c_client *client, struct pca955x_chipdef *chip)
 		if ((res != 0) && is_of_node(child))
 			name = to_of_node(child)->name;
 
-		snprintf(pdata->leds[reg].name, sizeof(pdata->leds[reg].name),
-			 "%s", name);
+		led = &pdata->leds[reg];
+		snprintf(led->name, sizeof(led->name), "%s", name);
 
-		pdata->leds[reg].type = PCA955X_TYPE_LED;
-		fwnode_property_read_u32(child, "type", &pdata->leds[reg].type);
+		led->type = PCA955X_TYPE_LED;
+		fwnode_property_read_u32(child, "type", &led->type);
 		fwnode_property_read_string(child, "linux,default-trigger",
-					&pdata->leds[reg].default_trigger);
+					    &led->default_trigger);
 	}
 
 	pdata->num_leds = chip->bits;
@@ -426,11 +424,12 @@ static const struct of_device_id of_pca955x_match[] = {
 MODULE_DEVICE_TABLE(of, of_pca955x_match);
 
 static int pca955x_probe(struct i2c_client *client,
-					const struct i2c_device_id *id)
+			 const struct i2c_device_id *id)
 {
 	struct pca955x *pca955x;
 	struct pca955x_led *pca955x_led;
 	struct pca955x_chipdef *chip;
+	struct led_classdev *led;
 	struct i2c_adapter *adapter;
 	int i, err;
 	struct pca955x_platform_data *pdata;
@@ -449,13 +448,13 @@ static int pca955x_probe(struct i2c_client *client,
 	if ((client->addr & ~((1 << chip->slv_addr_shift) - 1)) !=
 	    chip->slv_addr) {
 		dev_err(&client->dev, "invalid slave address %02x\n",
-				client->addr);
+			client->addr);
 		return -ENODEV;
 	}
 
 	dev_info(&client->dev, "leds-pca955x: Using %s %d-bit LED driver at "
-			"slave address 0x%02x\n",
-			client->name, chip->bits, client->addr);
+		 "slave address 0x%02x\n", client->name, chip->bits,
+		 client->addr);
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -EIO;
@@ -471,8 +470,8 @@ static int pca955x_probe(struct i2c_client *client,
 	if (!pca955x)
 		return -ENOMEM;
 
-	pca955x->leds = devm_kcalloc(&client->dev,
-			chip->bits, sizeof(*pca955x_led), GFP_KERNEL);
+	pca955x->leds = devm_kcalloc(&client->dev, chip->bits,
+				     sizeof(*pca955x_led), GFP_KERNEL);
 	if (!pca955x->leds)
 		return -ENOMEM;
 
@@ -501,27 +500,25 @@ static int pca955x_probe(struct i2c_client *client,
 			 */
 			if (pdata->leds[i].name[0] == '\0')
 				snprintf(pdata->leds[i].name,
-					sizeof(pdata->leds[i].name), "%d", i);
+					 sizeof(pdata->leds[i].name), "%d", i);
 
-			snprintf(pca955x_led->name,
-				sizeof(pca955x_led->name), "pca955x:%s",
-				pdata->leds[i].name);
+			snprintf(pca955x_led->name, sizeof(pca955x_led->name),
+				 "pca955x:%s", pdata->leds[i].name);
 
+			led = &pca955x_led->led_cdev;
 			if (pdata->leds[i].default_trigger)
-				pca955x_led->led_cdev.default_trigger =
+				led->default_trigger =
 					pdata->leds[i].default_trigger;
 
-			pca955x_led->led_cdev.name = pca955x_led->name;
-			pca955x_led->led_cdev.brightness_set_blocking =
-				pca955x_led_set;
+			led->name = pca955x_led->name;
+			led->brightness_set_blocking = pca955x_led_set;
 
-			err = devm_led_classdev_register(&client->dev,
-							&pca955x_led->led_cdev);
+			err = devm_led_classdev_register(&client->dev, led);
 			if (err)
 				return err;
 
 			/* Turn off LED */
-			err = pca955x_led_set(&pca955x_led->led_cdev, LED_OFF);
+			err = pca955x_led_set(led, LED_OFF);
 			if (err)
 				return err;
 		}
-- 
2.27.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH linux dev-5.10 4/7] leds: pca955x: Add brightness_get function
  2021-09-16 21:21 [PATCH linux dev-5.10 0/7] leds: Support retaining state for the PCA955x Eddie James
                   ` (2 preceding siblings ...)
  2021-09-16 21:21 ` [PATCH linux dev-5.10 3/7] leds: pca955x: Clean up code formatting Eddie James
@ 2021-09-16 21:21 ` Eddie James
  2021-09-16 21:21 ` [PATCH linux dev-5.10 5/7] leds: pca955x: Implement the default-state property Eddie James
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Eddie James @ 2021-09-16 21:21 UTC (permalink / raw)
  To: joel; +Cc: openbmc

Add a function to fetch the state of the hardware LED.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/leds-pca955x.c | 52 +++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index f0d841cb59fc..e47ba7c3b7c7 100644
--- a/drivers/leds/leds-pca955x.c
+++ b/drivers/leds/leds-pca955x.c
@@ -233,6 +233,57 @@ static int pca955x_read_ls(struct i2c_client *client, int n, u8 *val)
 	return 0;
 }
 
+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);
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(client, cmd);
+	if (ret < 0) {
+		dev_err(&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 = container_of(led_cdev,
+						       struct pca955x_led,
+						       led_cdev);
+	struct pca955x *pca955x = pca955x_led->pca955x;
+	u8 ls, pwm;
+	int ret;
+
+	ret = pca955x_read_ls(pca955x->client, pca955x_led->led_num / 4, &ls);
+	if (ret)
+		return ret;
+
+	ls = (ls >> ((pca955x_led->led_num % 4) << 1)) & 0x3;
+	switch (ls) {
+	case PCA955X_LS_LED_ON:
+		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->client, 1, &pwm);
+		if (ret)
+			return ret;
+		ret = 255 - pwm;
+		break;
+	}
+
+	return ret;
+}
+
 static int pca955x_led_set(struct led_classdev *led_cdev,
 			    enum led_brightness value)
 {
@@ -512,6 +563,7 @@ static int pca955x_probe(struct i2c_client *client,
 
 			led->name = pca955x_led->name;
 			led->brightness_set_blocking = pca955x_led_set;
+			led->brightness_get = pca955x_led_get;
 
 			err = devm_led_classdev_register(&client->dev, led);
 			if (err)
-- 
2.27.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH linux dev-5.10 5/7] leds: pca955x: Implement the default-state property
  2021-09-16 21:21 [PATCH linux dev-5.10 0/7] leds: Support retaining state for the PCA955x Eddie James
                   ` (3 preceding siblings ...)
  2021-09-16 21:21 ` [PATCH linux dev-5.10 4/7] leds: pca955x: Add brightness_get function Eddie James
@ 2021-09-16 21:21 ` Eddie James
  2021-09-16 21:21 ` [PATCH linux dev-5.10 6/7] leds: pca955x: Let the core process the fwnode Eddie James
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Eddie James @ 2021-09-16 21:21 UTC (permalink / raw)
  To: joel; +Cc: openbmc

In order to retain the LED state after a system reboot, check the
documented default-state device tree property during initialization.
Modify the behavior of the probe according to the property.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/leds-pca955x.c | 54 +++++++++++++++++++++++++++++++------
 1 file changed, 46 insertions(+), 8 deletions(-)

diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index e47ba7c3b7c7..fa1d77d86ef6 100644
--- a/drivers/leds/leds-pca955x.c
+++ b/drivers/leds/leds-pca955x.c
@@ -129,6 +129,7 @@ struct pca955x_led {
 	int			led_num;	/* 0 .. 15 potentially */
 	char			name[32];
 	u32			type;
+	int			default_state;
 	const char		*default_trigger;
 };
 
@@ -439,6 +440,7 @@ pca955x_get_pdata(struct i2c_client *client, struct pca955x_chipdef *chip)
 
 	device_for_each_child_node(&client->dev, child) {
 		const char *name;
+		const char *state;
 		u32 reg;
 		int res;
 
@@ -457,6 +459,18 @@ pca955x_get_pdata(struct i2c_client *client, struct pca955x_chipdef *chip)
 		fwnode_property_read_u32(child, "type", &led->type);
 		fwnode_property_read_string(child, "linux,default-trigger",
 					    &led->default_trigger);
+
+		if (!fwnode_property_read_string(child, "default-state",
+						 &state)) {
+			if (!strcmp(state, "keep"))
+				led->default_state = LEDS_GPIO_DEFSTATE_KEEP;
+			else if (!strcmp(state, "on"))
+				led->default_state = LEDS_GPIO_DEFSTATE_ON;
+			else
+				led->default_state = LEDS_GPIO_DEFSTATE_OFF;
+		} else {
+			led->default_state = LEDS_GPIO_DEFSTATE_OFF;
+		}
 	}
 
 	pdata->num_leds = chip->bits;
@@ -485,6 +499,7 @@ static int pca955x_probe(struct i2c_client *client,
 	int i, err;
 	struct pca955x_platform_data *pdata;
 	int ngpios = 0;
+	bool keep_pwm = false;
 
 	chip = &pca955x_chipdefs[id->driver_data];
 	adapter = client->adapter;
@@ -565,14 +580,35 @@ static int pca955x_probe(struct i2c_client *client,
 			led->brightness_set_blocking = pca955x_led_set;
 			led->brightness_get = pca955x_led_get;
 
+			if (pdata->leds[i].default_state ==
+			    LEDS_GPIO_DEFSTATE_OFF) {
+				err = pca955x_led_set(led, LED_OFF);
+				if (err)
+					return err;
+			} else if (pdata->leds[i].default_state ==
+				   LEDS_GPIO_DEFSTATE_ON) {
+				err = pca955x_led_set(led, LED_FULL);
+				if (err)
+					return err;
+			}
+
 			err = devm_led_classdev_register(&client->dev, led);
 			if (err)
 				return err;
 
-			/* Turn off LED */
-			err = pca955x_led_set(led, LED_OFF);
-			if (err)
-				return err;
+			/*
+			 * 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_GPIO_DEFSTATE_KEEP) {
+				if (led->brightness != LED_FULL &&
+				    led->brightness != LED_OFF &&
+				    led->brightness != LED_HALF)
+					keep_pwm = true;
+			}
 		}
 	}
 
@@ -581,10 +617,12 @@ static int pca955x_probe(struct i2c_client *client,
 	if (err)
 		return err;
 
-	/* PWM1 is used for variable brightness, default to OFF */
-	err = pca955x_write_pwm(client, 1, 0);
-	if (err)
-		return err;
+	if (!keep_pwm) {
+		/* PWM1 is used for variable brightness, default to OFF */
+		err = pca955x_write_pwm(client, 1, 0);
+		if (err)
+			return err;
+	}
 
 	/* Set to fast frequency so we do not see flashing */
 	err = pca955x_write_psc(client, 0, 0);
-- 
2.27.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH linux dev-5.10 6/7] leds: pca955x: Let the core process the fwnode
  2021-09-16 21:21 [PATCH linux dev-5.10 0/7] leds: Support retaining state for the PCA955x Eddie James
                   ` (4 preceding siblings ...)
  2021-09-16 21:21 ` [PATCH linux dev-5.10 5/7] leds: pca955x: Implement the default-state property Eddie James
@ 2021-09-16 21:21 ` Eddie James
  2021-09-16 21:21 ` [PATCH linux dev-5.10 7/7] leds: pca955x: Switch to i2c probe_new Eddie James
  2021-09-22  2:21 ` [PATCH linux dev-5.10 0/7] leds: Support retaining state for the PCA955x Joel Stanley
  7 siblings, 0 replies; 9+ messages in thread
From: Eddie James @ 2021-09-16 21:21 UTC (permalink / raw)
  To: joel; +Cc: openbmc

Much of the fwnode processing in the PCA955x driver is now in the
LEDs core driver, so pass the fwnode in the init data when
registering the LED device. In order to preserve the existing naming
scheme, check for an empty name and set it to the LED number.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/leds-pca955x.c | 58 +++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index fa1d77d86ef6..a6aa4b9abde8 100644
--- a/drivers/leds/leds-pca955x.c
+++ b/drivers/leds/leds-pca955x.c
@@ -127,10 +127,9 @@ struct pca955x_led {
 	struct pca955x	*pca955x;
 	struct led_classdev	led_cdev;
 	int			led_num;	/* 0 .. 15 potentially */
-	char			name[32];
 	u32			type;
 	int			default_state;
-	const char		*default_trigger;
+	struct fwnode_handle	*fwnode;
 };
 
 struct pca955x_platform_data {
@@ -439,7 +438,6 @@ pca955x_get_pdata(struct i2c_client *client, struct pca955x_chipdef *chip)
 		return ERR_PTR(-ENOMEM);
 
 	device_for_each_child_node(&client->dev, child) {
-		const char *name;
 		const char *state;
 		u32 reg;
 		int res;
@@ -448,17 +446,10 @@ pca955x_get_pdata(struct i2c_client *client, struct pca955x_chipdef *chip)
 		if ((res != 0) || (reg >= chip->bits))
 			continue;
 
-		res = fwnode_property_read_string(child, "label", &name);
-		if ((res != 0) && is_of_node(child))
-			name = to_of_node(child)->name;
-
 		led = &pdata->leds[reg];
-		snprintf(led->name, sizeof(led->name), "%s", name);
-
 		led->type = PCA955X_TYPE_LED;
+		led->fwnode = child;
 		fwnode_property_read_u32(child, "type", &led->type);
-		fwnode_property_read_string(child, "linux,default-trigger",
-					    &led->default_trigger);
 
 		if (!fwnode_property_read_string(child, "default-state",
 						 &state)) {
@@ -495,11 +486,14 @@ static int pca955x_probe(struct i2c_client *client,
 	struct pca955x_led *pca955x_led;
 	struct pca955x_chipdef *chip;
 	struct led_classdev *led;
+	struct led_init_data init_data;
 	struct i2c_adapter *adapter;
 	int i, err;
 	struct pca955x_platform_data *pdata;
 	int ngpios = 0;
+	bool set_default_label = false;
 	bool keep_pwm = false;
+	char default_label[8];
 
 	chip = &pca955x_chipdefs[id->driver_data];
 	adapter = client->adapter;
@@ -547,6 +541,9 @@ static int pca955x_probe(struct i2c_client *client,
 	pca955x->client = client;
 	pca955x->chipdef = chip;
 
+	init_data.devname_mandatory = false;
+	init_data.devicename = "pca955x";
+
 	for (i = 0; i < chip->bits; i++) {
 		pca955x_led = &pca955x->leds[i];
 		pca955x_led->led_num = i;
@@ -560,23 +557,7 @@ static int pca955x_probe(struct i2c_client *client,
 			ngpios++;
 			break;
 		case PCA955X_TYPE_LED:
-			/*
-			 * Platform data can specify LED names and
-			 * default triggers
-			 */
-			if (pdata->leds[i].name[0] == '\0')
-				snprintf(pdata->leds[i].name,
-					 sizeof(pdata->leds[i].name), "%d", i);
-
-			snprintf(pca955x_led->name, sizeof(pca955x_led->name),
-				 "pca955x:%s", pdata->leds[i].name);
-
 			led = &pca955x_led->led_cdev;
-			if (pdata->leds[i].default_trigger)
-				led->default_trigger =
-					pdata->leds[i].default_trigger;
-
-			led->name = pca955x_led->name;
 			led->brightness_set_blocking = pca955x_led_set;
 			led->brightness_get = pca955x_led_get;
 
@@ -592,7 +573,28 @@ static int pca955x_probe(struct i2c_client *client,
 					return err;
 			}
 
-			err = devm_led_classdev_register(&client->dev, led);
+			init_data.fwnode = pdata->leds[i].fwnode;
+
+			if (is_of_node(init_data.fwnode)) {
+				if (to_of_node(init_data.fwnode)->name[0] ==
+				    '\0')
+					set_default_label = true;
+				else
+					set_default_label = false;
+			} else {
+				set_default_label = true;
+			}
+
+			if (set_default_label) {
+				snprintf(default_label, sizeof(default_label),
+					 "%d", i);
+				init_data.default_label = default_label;
+			} else {
+				init_data.default_label = NULL;
+			}
+
+			err = devm_led_classdev_register_ext(&client->dev, led,
+							     &init_data);
 			if (err)
 				return err;
 
-- 
2.27.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH linux dev-5.10 7/7] leds: pca955x: Switch to i2c probe_new
  2021-09-16 21:21 [PATCH linux dev-5.10 0/7] leds: Support retaining state for the PCA955x Eddie James
                   ` (5 preceding siblings ...)
  2021-09-16 21:21 ` [PATCH linux dev-5.10 6/7] leds: pca955x: Let the core process the fwnode Eddie James
@ 2021-09-16 21:21 ` Eddie James
  2021-09-22  2:21 ` [PATCH linux dev-5.10 0/7] leds: Support retaining state for the PCA955x Joel Stanley
  7 siblings, 0 replies; 9+ messages in thread
From: Eddie James @ 2021-09-16 21:21 UTC (permalink / raw)
  To: joel; +Cc: openbmc

The deprecated i2c probe functionality doesn't work with OF
compatible strings, as it only checks for the i2c device id. Switch
to the new way of probing and grab the match data to select the
chip type.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/leds-pca955x.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index a6aa4b9abde8..a6b5699aeae4 100644
--- a/drivers/leds/leds-pca955x.c
+++ b/drivers/leds/leds-pca955x.c
@@ -479,8 +479,7 @@ static const struct of_device_id of_pca955x_match[] = {
 };
 MODULE_DEVICE_TABLE(of, of_pca955x_match);
 
-static int pca955x_probe(struct i2c_client *client,
-			 const struct i2c_device_id *id)
+static int pca955x_probe(struct i2c_client *client)
 {
 	struct pca955x *pca955x;
 	struct pca955x_led *pca955x_led;
@@ -494,8 +493,24 @@ static int pca955x_probe(struct i2c_client *client,
 	bool set_default_label = false;
 	bool keep_pwm = false;
 	char default_label[8];
+	enum pca955x_type chip_type;
+	const void *md = device_get_match_data(&client->dev);
 
-	chip = &pca955x_chipdefs[id->driver_data];
+	if (md) {
+		chip_type = (enum pca955x_type)md;
+	} else {
+		const struct i2c_device_id *id = i2c_match_id(pca955x_id,
+							      client);
+
+		if (id) {
+			chip_type = (enum pca955x_type)id->driver_data;
+		} else {
+			dev_err(&client->dev, "unknown chip\n");
+			return -ENODEV;
+		}
+	}
+
+	chip = &pca955x_chipdefs[chip_type];
 	adapter = client->adapter;
 	pdata = dev_get_platdata(&client->dev);
 	if (!pdata) {
@@ -670,7 +685,7 @@ static struct i2c_driver pca955x_driver = {
 		.name	= "leds-pca955x",
 		.of_match_table = of_pca955x_match,
 	},
-	.probe	= pca955x_probe,
+	.probe_new = pca955x_probe,
 	.id_table = pca955x_id,
 };
 
-- 
2.27.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH linux dev-5.10 0/7] leds: Support retaining state for the PCA955x
  2021-09-16 21:21 [PATCH linux dev-5.10 0/7] leds: Support retaining state for the PCA955x Eddie James
                   ` (6 preceding siblings ...)
  2021-09-16 21:21 ` [PATCH linux dev-5.10 7/7] leds: pca955x: Switch to i2c probe_new Eddie James
@ 2021-09-22  2:21 ` Joel Stanley
  7 siblings, 0 replies; 9+ messages in thread
From: Joel Stanley @ 2021-09-22  2:21 UTC (permalink / raw)
  To: Eddie James; +Cc: OpenBMC Maillist

On Thu, 16 Sept 2021 at 21:21, Eddie James <eajames@linux.ibm.com> wrote:
>
> This series implements the ability to retain the state of the LEDs
> controlled by the PCA955x across system reboots. This includes a
> change to the LED core driver to respect the retain-state-shutdown
> device tree property. It also cleans up the PCA955x driver, adds the
> ability to query the hardware LED brightness, switches the I2C probe
> function to probe_new, and uses more core functionality for parsing
> the fwnode.
>
> This series has been applied to linux-next.
>
> Eddie James (7):
>   dt-bindings: leds: Add retain-state-shutdown boolean
>   leds: leds-core: Implement the retain-state-shutdown property
>   leds: pca955x: Clean up code formatting
>   leds: pca955x: Add brightness_get function
>   leds: pca955x: Implement the default-state property
>   leds: pca955x: Let the core process the fwnode
>   leds: pca955x: Switch to i2c probe_new

Thanks, applied.

>
>  .../devicetree/bindings/leds/common.yaml      |   6 +
>  drivers/leds/led-class.c                      |  10 +-
>  drivers/leds/leds-pca955x.c                   | 232 +++++++++++++-----
>  3 files changed, 182 insertions(+), 66 deletions(-)
>
> --
> 2.27.0
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-09-22  2:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-16 21:21 [PATCH linux dev-5.10 0/7] leds: Support retaining state for the PCA955x Eddie James
2021-09-16 21:21 ` [PATCH linux dev-5.10 1/7] dt-bindings: leds: Add retain-state-shutdown boolean Eddie James
2021-09-16 21:21 ` [PATCH linux dev-5.10 2/7] leds: leds-core: Implement the retain-state-shutdown property Eddie James
2021-09-16 21:21 ` [PATCH linux dev-5.10 3/7] leds: pca955x: Clean up code formatting Eddie James
2021-09-16 21:21 ` [PATCH linux dev-5.10 4/7] leds: pca955x: Add brightness_get function Eddie James
2021-09-16 21:21 ` [PATCH linux dev-5.10 5/7] leds: pca955x: Implement the default-state property Eddie James
2021-09-16 21:21 ` [PATCH linux dev-5.10 6/7] leds: pca955x: Let the core process the fwnode Eddie James
2021-09-16 21:21 ` [PATCH linux dev-5.10 7/7] leds: pca955x: Switch to i2c probe_new Eddie James
2021-09-22  2:21 ` [PATCH linux dev-5.10 0/7] leds: Support retaining state for the PCA955x Joel Stanley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).