From 114fa138e41d8407da491da60e72e84fbb07889b Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Tue, 11 Jan 2022 18:54:30 +0100 Subject: [PATCH 1/8] dt-bindings: leds: common: fix unit address in max77693 example The max77693 LED device node should not take an unit address, because it is instantiated from a max77693 I2C parent device node. This also splits all examples to separate DTS examples because they are actually independent. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Rob Herring Reviewed-by: Javier Martinez Canillas Signed-off-by: Pavel Machek --- Documentation/devicetree/bindings/leds/common.yaml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml index 697102707703..328952d7acbb 100644 --- a/Documentation/devicetree/bindings/leds/common.yaml +++ b/Documentation/devicetree/bindings/leds/common.yaml @@ -185,9 +185,11 @@ examples: }; }; - led-controller@0 { + - | + #include + + led-controller { compatible = "maxim,max77693-led"; - reg = <0 0x100>; led { function = LED_FUNCTION_FLASH; @@ -199,6 +201,9 @@ examples: }; }; + - | + #include + i2c { #address-cells = <1>; #size-cells = <0>; From 2f1b6bb66900ed3b54bda885ac87dcd5a7e122b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Apitzsch?= Date: Tue, 8 Feb 2022 00:06:36 +0100 Subject: [PATCH 2/8] dt-bindings: vendor-prefixes: Add ocs prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the prefix for Orient Chip Technology. Signed-off-by: André Apitzsch Acked-by: Rob Herring Signed-off-by: Pavel Machek --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 294093d45a23..9ffffa9773ca 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -854,6 +854,8 @@ patternProperties: description: NXP Semiconductors "^oceanic,.*": description: Oceanic Systems (UK) Ltd. + "^ocs,.*": + description: Orient Chip Technology Co., Ltd. "^oct,.*": description: Octavo Systems LLC "^okaya,.*": From 77d62fccebd4dd2e4dbec2bbe581ef98e128561b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Apitzsch?= Date: Tue, 8 Feb 2022 00:06:38 +0100 Subject: [PATCH 3/8] leds: sgm3140: Add ocs,ocp8110 compatible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Orient-Chip's ocp8110 has the same pin configuration as the sgm3140. The data sheet can be found at: https://cdn.datasheetspdf.com/pdf-down/O/C/P/OCP8110-OrientChip.pdf Signed-off-by: André Apitzsch Signed-off-by: Pavel Machek --- drivers/leds/flash/leds-sgm3140.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/leds/flash/leds-sgm3140.c b/drivers/leds/flash/leds-sgm3140.c index f4f831570f11..d3a30ad94ac4 100644 --- a/drivers/leds/flash/leds-sgm3140.c +++ b/drivers/leds/flash/leds-sgm3140.c @@ -290,6 +290,7 @@ static int sgm3140_remove(struct platform_device *pdev) } static const struct of_device_id sgm3140_dt_match[] = { + { .compatible = "ocs,ocp8110" }, { .compatible = "sgmicro,sgm3140" }, { /* sentinel */ } }; From cf642faef74f453df14c2b8cef533dfd819f425e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sun, 6 Feb 2022 23:08:12 +0100 Subject: [PATCH 4/8] leds: lm3692x: Return 0 from remove callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The only difference between returning zero or a non-zero value is that for the non-zero case the i2c will print a generic error message ("remove failed (-ESOMETHING), will be ignored"). In this case however the driver itself already emitted a more helpful error message, so the additional error message isn't helpful at all. The long-term goal is to make the i2c remove callback return void, making all implementations return 0 is preparatory work for this change. Signed-off-by: Uwe Kleine-König Signed-off-by: Pavel Machek --- drivers/leds/leds-lm3692x.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c index afe6fb297855..87cd24ce3f95 100644 --- a/drivers/leds/leds-lm3692x.c +++ b/drivers/leds/leds-lm3692x.c @@ -494,11 +494,8 @@ static int lm3692x_probe(struct i2c_client *client, static int lm3692x_remove(struct i2c_client *client) { struct lm3692x_led *led = i2c_get_clientdata(client); - int ret; - ret = lm3692x_leds_disable(led); - if (ret) - return ret; + lm3692x_leds_disable(led); mutex_destroy(&led->lock); return 0; From a8f59497a430e8a93aa9a5b0fb0feefe061195ee Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 17 Jan 2022 12:21:08 +0100 Subject: [PATCH 5/8] leds: simatic-ipc-leds: Make simatic_ipc_led_mem_res static simatic_ipc_led_mem_res is not used outside of the driver, make it static. Cc: Henning Schild Reported-by: kernel test robot Signed-off-by: Hans de Goede Signed-off-by: Pavel Machek --- drivers/leds/simple/simatic-ipc-leds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/leds/simple/simatic-ipc-leds.c b/drivers/leds/simple/simatic-ipc-leds.c index ff2c96e73241..179110448659 100644 --- a/drivers/leds/simple/simatic-ipc-leds.c +++ b/drivers/leds/simple/simatic-ipc-leds.c @@ -39,7 +39,7 @@ static struct simatic_ipc_led simatic_ipc_leds_io[] = { }; /* the actual start will be discovered with PCI, 0 is a placeholder */ -struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, SZ_4K, KBUILD_MODNAME); +static struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, SZ_4K, KBUILD_MODNAME); static void *simatic_ipc_led_memory; From 8b43ef06ff893f733a0b66e9875518c0230f2715 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 17 Jan 2022 12:21:09 +0100 Subject: [PATCH 6/8] leds: simatic-ipc-leds: Don't directly deref ioremap_resource() returned ptr Sparse (rightly) currently gives the following warning: drivers/leds/simple/simatic-ipc-leds.c:155:40: sparse: sparse: incorrect type in assignment (different address spaces) expected void *static [toplevel] simatic_ipc_led_memory got void [noderef] __iomem * Fix this by changing the type of simatic_ipc_led_memory to void __iomem * and use readl()/writel() to access it. Cc: Henning Schild Reported-by: kernel test robot Signed-off-by: Hans de Goede Tested-by: Gerd Haeussler Signed-off-by: Pavel Machek --- drivers/leds/simple/simatic-ipc-leds.c | 32 +++++++++++++++----------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/drivers/leds/simple/simatic-ipc-leds.c b/drivers/leds/simple/simatic-ipc-leds.c index 179110448659..078d43f5ba38 100644 --- a/drivers/leds/simple/simatic-ipc-leds.c +++ b/drivers/leds/simple/simatic-ipc-leds.c @@ -41,7 +41,7 @@ static struct simatic_ipc_led simatic_ipc_leds_io[] = { /* the actual start will be discovered with PCI, 0 is a placeholder */ static struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, SZ_4K, KBUILD_MODNAME); -static void *simatic_ipc_led_memory; +static void __iomem *simatic_ipc_led_memory; static struct simatic_ipc_led simatic_ipc_leds_mem[] = { {0x500 + 0x1A0, "red:" LED_FUNCTION_STATUS "-1"}, @@ -92,21 +92,22 @@ static void simatic_ipc_led_set_mem(struct led_classdev *led_cd, enum led_brightness brightness) { struct simatic_ipc_led *led = cdev_to_led(led_cd); + void __iomem *reg = simatic_ipc_led_memory + led->value; + u32 val; - u32 *p; - - p = simatic_ipc_led_memory + led->value; - *p = (*p & ~1) | (brightness == LED_OFF); + val = readl(reg); + val = (val & ~1) | (brightness == LED_OFF); + writel(val, reg); } static enum led_brightness simatic_ipc_led_get_mem(struct led_classdev *led_cd) { struct simatic_ipc_led *led = cdev_to_led(led_cd); + void __iomem *reg = simatic_ipc_led_memory + led->value; + u32 val; - u32 *p; - - p = simatic_ipc_led_memory + led->value; - return (*p & 1) ? LED_OFF : led_cd->max_brightness; + val = readl(reg); + return (val & 1) ? LED_OFF : led_cd->max_brightness; } static int simatic_ipc_leds_probe(struct platform_device *pdev) @@ -116,8 +117,9 @@ static int simatic_ipc_leds_probe(struct platform_device *pdev) struct simatic_ipc_led *ipcled; struct led_classdev *cdev; struct resource *res; + void __iomem *reg; int err, type; - u32 *p; + u32 val; switch (plat->devmode) { case SIMATIC_IPC_DEVICE_227D: @@ -157,11 +159,13 @@ static int simatic_ipc_leds_probe(struct platform_device *pdev) return PTR_ERR(simatic_ipc_led_memory); /* initialize power/watchdog LED */ - p = simatic_ipc_led_memory + 0x500 + 0x1D8; /* PM_WDT_OUT */ - *p = (*p & ~1); - p = simatic_ipc_led_memory + 0x500 + 0x1C0; /* PM_BIOS_BOOT_N */ - *p = (*p | 1); + reg = simatic_ipc_led_memory + 0x500 + 0x1D8; /* PM_WDT_OUT */ + val = readl(reg); + writel(val & ~1, reg); + reg = simatic_ipc_led_memory + 0x500 + 0x1C0; /* PM_BIOS_BOOT_N */ + val = readl(reg); + writel(val | 1, reg); break; default: return -ENODEV; From ca386253ff6fb86128a7c61e1c38ca91de38048d Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Tue, 21 Sep 2021 14:09:35 +0930 Subject: [PATCH 7/8] leds: pca955x: Make the gpiochip always expose all pins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The devicetree binding allows specifying which pins are GPIO vs LED. Limiting the instantiated gpiochip to just these pins as the driver currently does requires an arbitrary mapping between pins and GPIOs, but such a mapping is not implemented by the driver. As a result, specifying GPIOs in such a way that they don't map 1-to-1 to pin indexes does not function as expected. Establishing such a mapping is more complex than not and even if we did, doing so leads to a slightly hairy userspace experience as the behaviour of the PCA955x gpiochip would depend on how the pins are assigned in the devicetree. Instead, always expose all pins via the gpiochip to provide a stable interface and track which pins are in use. Specifying a pin as `type = ;` in the devicetree becomes a no-op. I've assessed the impact of this change by looking through all of the affected devicetrees as of the tag leds-5.15-rc1: ``` $ git grep -l 'pca955[0123]' $(find . -name dts -type d) arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts arch/arm/boot/dts/aspeed-bmc-opp-mowgli.dts arch/arm/boot/dts/aspeed-bmc-opp-swift.dts arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts ``` These are all IBM-associated platforms. I've analysed both the devicetrees and schematics where necessary to determine whether any systems hit the hazard of the current broken behaviour. For the most part, the systems specify the pins as either all LEDs or all GPIOs, or at least do so in a way such that the broken behaviour isn't exposed. The main counter-point to this observation is the Everest system whose devicetree describes a large number of PCA955x devices and in some cases has pin assignments that hit the hazard. However, there does not seem to be any use of the affected GPIOs in the userspace associated with Everest. Regardless, any use of the hazardous GPIOs in Everest is already broken, so let's fix the interface and then fix any already broken userspace with it. Signed-off-by: Andrew Jeffery Acked-by: Linus Walleij Reviewed-by: Cédric Le Goater Signed-off-by: Pavel Machek --- drivers/leds/leds-pca955x.c | 63 +++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c index a6b5699aeae4..77c0f461ab95 100644 --- a/drivers/leds/leds-pca955x.c +++ b/drivers/leds/leds-pca955x.c @@ -37,6 +37,7 @@ * bits the chip supports. */ +#include #include #include #include @@ -118,6 +119,7 @@ struct pca955x { struct pca955x_led *leds; struct pca955x_chipdef *chipdef; struct i2c_client *client; + unsigned long active_pins; #ifdef CONFIG_LEDS_PCA955X_GPIO struct gpio_chip gpio; #endif @@ -360,12 +362,15 @@ static int pca955x_read_input(struct i2c_client *client, int n, u8 *val) static int pca955x_gpio_request_pin(struct gpio_chip *gc, unsigned int offset) { struct pca955x *pca955x = gpiochip_get_data(gc); - struct pca955x_led *led = &pca955x->leds[offset]; - if (led->type == PCA955X_TYPE_GPIO) - return 0; + return test_and_set_bit(offset, &pca955x->active_pins) ? -EBUSY : 0; +} - return -EBUSY; +static void pca955x_gpio_free_pin(struct gpio_chip *gc, unsigned int offset) +{ + struct pca955x *pca955x = gpiochip_get_data(gc); + + clear_bit(offset, &pca955x->active_pins); } static int pca955x_set_value(struct gpio_chip *gc, unsigned int offset, @@ -489,7 +494,6 @@ static int pca955x_probe(struct i2c_client *client) 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]; @@ -567,9 +571,7 @@ static int pca955x_probe(struct i2c_client *client) switch (pca955x_led->type) { case PCA955X_TYPE_NONE: - break; case PCA955X_TYPE_GPIO: - ngpios++; break; case PCA955X_TYPE_LED: led = &pca955x_led->led_cdev; @@ -613,6 +615,8 @@ static int pca955x_probe(struct i2c_client *client) if (err) return err; + set_bit(i, &pca955x->active_pins); + /* * For default-state == "keep", let the core update the * brightness from the hardware, then check the @@ -650,31 +654,30 @@ static int pca955x_probe(struct i2c_client *client) return err; #ifdef CONFIG_LEDS_PCA955X_GPIO - if (ngpios) { - pca955x->gpio.label = "gpio-pca955x"; - pca955x->gpio.direction_input = pca955x_gpio_direction_input; - pca955x->gpio.direction_output = pca955x_gpio_direction_output; - pca955x->gpio.set = pca955x_gpio_set_value; - pca955x->gpio.get = pca955x_gpio_get_value; - pca955x->gpio.request = pca955x_gpio_request_pin; - pca955x->gpio.can_sleep = 1; - pca955x->gpio.base = -1; - pca955x->gpio.ngpio = ngpios; - pca955x->gpio.parent = &client->dev; - pca955x->gpio.owner = THIS_MODULE; + pca955x->gpio.label = "gpio-pca955x"; + pca955x->gpio.direction_input = pca955x_gpio_direction_input; + pca955x->gpio.direction_output = pca955x_gpio_direction_output; + pca955x->gpio.set = pca955x_gpio_set_value; + pca955x->gpio.get = pca955x_gpio_get_value; + pca955x->gpio.request = pca955x_gpio_request_pin; + pca955x->gpio.free = pca955x_gpio_free_pin; + pca955x->gpio.can_sleep = 1; + pca955x->gpio.base = -1; + pca955x->gpio.ngpio = chip->bits; + pca955x->gpio.parent = &client->dev; + pca955x->gpio.owner = THIS_MODULE; - err = devm_gpiochip_add_data(&client->dev, &pca955x->gpio, - pca955x); - if (err) { - /* Use data->gpio.dev as a flag for freeing gpiochip */ - pca955x->gpio.parent = NULL; - dev_warn(&client->dev, "could not add gpiochip\n"); - return err; - } - dev_info(&client->dev, "gpios %i...%i\n", - pca955x->gpio.base, pca955x->gpio.base + - pca955x->gpio.ngpio - 1); + err = devm_gpiochip_add_data(&client->dev, &pca955x->gpio, + pca955x); + if (err) { + /* Use data->gpio.dev as a flag for freeing gpiochip */ + pca955x->gpio.parent = NULL; + dev_warn(&client->dev, "could not add gpiochip\n"); + return err; } + dev_info(&client->dev, "gpios %i...%i\n", + pca955x->gpio.base, pca955x->gpio.base + + pca955x->gpio.ngpio - 1); #endif return 0; From e26557a0aa68acfb705b51947b7c756401a1ab71 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Tue, 21 Sep 2021 14:09:36 +0930 Subject: [PATCH 8/8] leds: pca955x: Allow zero LEDs to be specified MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's valid to use the PCA955x devices just for GPIOs and not for LEDs. In this case, as PCA955X_TYPE_GPIO is now equivalent to PCA955X_TYPE_NONE, remove the test for whether we have any child nodes specified in the devicetree. A consequence of this is it's now possible to bind the driver to a PCA955x device when dynamically instantiated through the I2C subsystem's `new_device` interface. Signed-off-by: Andrew Jeffery Reviewed-by: Andy Shevchenko Reviewed-by: Linus Walleij Reviewed-by: Joel Stanley Reviewed-by: Cédric Le Goater Signed-off-by: Pavel Machek --- drivers/leds/leds-pca955x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c index 77c0f461ab95..81aaf21212d7 100644 --- a/drivers/leds/leds-pca955x.c +++ b/drivers/leds/leds-pca955x.c @@ -429,7 +429,7 @@ pca955x_get_pdata(struct i2c_client *client, struct pca955x_chipdef *chip) int count; count = device_get_child_node_count(&client->dev); - if (!count || count > chip->bits) + if (count > chip->bits) return ERR_PTR(-ENODEV); pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);