There are two special pins needed to init the LEDs. We used to have them
at the end of the gpiod_lookup table to give to "leds-gpio". A cleaner
way is to have a dedicated table for the special pins.
Signed-off-by: Henning Schild <[email protected]>
---
drivers/leds/simple/simatic-ipc-leds-gpio.c | 26 ++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio.c b/drivers/leds/simple/simatic-ipc-leds-gpio.c
index e8d329b5a68c..ba0f24638af5 100644
--- a/drivers/leds/simple/simatic-ipc-leds-gpio.c
+++ b/drivers/leds/simple/simatic-ipc-leds-gpio.c
@@ -16,6 +16,7 @@
#include <linux/platform_data/x86/simatic-ipc-base.h>
static struct gpiod_lookup_table *simatic_ipc_led_gpio_table;
+static struct gpiod_lookup_table *simatic_ipc_led_gpio_table_extra;
static struct gpiod_lookup_table simatic_ipc_led_gpio_table_127e = {
.dev_id = "leds-gpio",
@@ -26,6 +27,12 @@ static struct gpiod_lookup_table simatic_ipc_led_gpio_table_127e = {
GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 58, NULL, 3, GPIO_ACTIVE_LOW),
GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 60, NULL, 4, GPIO_ACTIVE_LOW),
GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 51, NULL, 5, GPIO_ACTIVE_LOW),
+ },
+};
+
+static struct gpiod_lookup_table simatic_ipc_led_gpio_table_127e_extra = {
+ .dev_id = NULL,
+ .table = {
GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 56, NULL, 6, GPIO_ACTIVE_LOW),
GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 59, NULL, 7, GPIO_ACTIVE_HIGH),
},
@@ -40,9 +47,15 @@ static struct gpiod_lookup_table simatic_ipc_led_gpio_table_227g = {
GPIO_LOOKUP_IDX("gpio-f7188x-2", 3, NULL, 3, GPIO_ACTIVE_LOW),
GPIO_LOOKUP_IDX("gpio-f7188x-2", 4, NULL, 4, GPIO_ACTIVE_LOW),
GPIO_LOOKUP_IDX("gpio-f7188x-2", 5, NULL, 5, GPIO_ACTIVE_LOW),
+ },
+};
+
+static struct gpiod_lookup_table simatic_ipc_led_gpio_table_227g_extra = {
+ .dev_id = NULL,
+ .table = {
GPIO_LOOKUP_IDX("gpio-f7188x-3", 6, NULL, 6, GPIO_ACTIVE_HIGH),
GPIO_LOOKUP_IDX("gpio-f7188x-3", 7, NULL, 7, GPIO_ACTIVE_HIGH),
- }
+ },
};
static const struct gpio_led simatic_ipc_gpio_leds[] = {
@@ -64,6 +77,7 @@ static struct platform_device *simatic_leds_pdev;
static int simatic_ipc_leds_gpio_remove(struct platform_device *pdev)
{
gpiod_remove_lookup_table(simatic_ipc_led_gpio_table);
+ gpiod_remove_lookup_table(simatic_ipc_led_gpio_table_extra);
platform_device_unregister(simatic_leds_pdev);
return 0;
@@ -72,6 +86,7 @@ static int simatic_ipc_leds_gpio_remove(struct platform_device *pdev)
static int simatic_ipc_leds_gpio_probe(struct platform_device *pdev)
{
const struct simatic_ipc_platform *plat = pdev->dev.platform_data;
+ struct device *dev = &pdev->dev;
struct gpio_desc *gpiod;
int err;
@@ -80,12 +95,14 @@ static int simatic_ipc_leds_gpio_probe(struct platform_device *pdev)
if (!IS_ENABLED(CONFIG_PINCTRL_BROXTON))
return -ENODEV;
simatic_ipc_led_gpio_table = &simatic_ipc_led_gpio_table_127e;
+ simatic_ipc_led_gpio_table_extra = &simatic_ipc_led_gpio_table_127e_extra;
break;
case SIMATIC_IPC_DEVICE_227G:
if (!IS_ENABLED(CONFIG_GPIO_F7188X))
return -ENODEV;
request_module("gpio-f7188x");
simatic_ipc_led_gpio_table = &simatic_ipc_led_gpio_table_227g;
+ simatic_ipc_led_gpio_table_extra = &simatic_ipc_led_gpio_table_227g_extra;
break;
default:
return -ENODEV;
@@ -101,8 +118,11 @@ static int simatic_ipc_leds_gpio_probe(struct platform_device *pdev)
goto out;
}
+ simatic_ipc_led_gpio_table_extra->dev_id = dev_name(dev);
+ gpiod_add_lookup_table(simatic_ipc_led_gpio_table_extra);
+
/* PM_BIOS_BOOT_N */
- gpiod = gpiod_get_index(&simatic_leds_pdev->dev, NULL, 6, GPIOD_OUT_LOW);
+ gpiod = gpiod_get_index(dev, NULL, 6, GPIOD_OUT_LOW);
if (IS_ERR(gpiod)) {
err = PTR_ERR(gpiod);
goto out;
@@ -110,7 +130,7 @@ static int simatic_ipc_leds_gpio_probe(struct platform_device *pdev)
gpiod_put(gpiod);
/* PM_WDT_OUT */
- gpiod = gpiod_get_index(&simatic_leds_pdev->dev, NULL, 7, GPIOD_OUT_LOW);
+ gpiod = gpiod_get_index(dev, NULL, 7, GPIOD_OUT_LOW);
if (IS_ERR(gpiod)) {
err = PTR_ERR(gpiod);
goto out;
--
2.39.2
On Wed, Mar 01, 2023 at 06:02:13PM +0100, Henning Schild wrote:
> There are two special pins needed to init the LEDs. We used to have them
> at the end of the gpiod_lookup table to give to "leds-gpio". A cleaner
> way is to have a dedicated table for the special pins.
...
> + .dev_id = NULL,
Technically speaking this is redundant. Maybe you wanted to be more explicit
with a comment like
.dev_id = NULL, /* Filled during initialization */
...
> + .dev_id = NULL,
Ditto.
--
With Best Regards,
Andy Shevchenko
Am Wed, 1 Mar 2023 19:21:53 +0200
schrieb Andy Shevchenko <[email protected]>:
> On Wed, Mar 01, 2023 at 06:02:13PM +0100, Henning Schild wrote:
> > There are two special pins needed to init the LEDs. We used to have
> > them at the end of the gpiod_lookup table to give to "leds-gpio". A
> > cleaner way is to have a dedicated table for the special pins.
>
> ...
>
> > + .dev_id = NULL,
>
> Technically speaking this is redundant. Maybe you wanted to be more
> explicit with a comment like
>
> .dev_id = NULL, /* Filled during
> initialization */
>
> ...
>
> > + .dev_id = NULL,
>
> Ditto.
Sorry you already pointed that out earlier, it slipped back in with
the rewrite.
Henning