Re: [PATCH v2 4/4] leds: simatic-ipc-leds-gpio: add GPIO version of Siemens driver

[Andy Shevchenko]

On Wed, May 11, 2022 at 6:40 PM Henning Schild
<henning.schild@xxxxxxxxxxx> wrote:
>
> On Apollo Lake the pinctrl drivers will now come up without ACPI. Use
> that instead of open coding it.
> Create a new driver for that which can later be filled with more GPIO
> based models, and which has different dependencies.

...

> +static struct gpiod_lookup_table simatic_ipc_led_gpio_table = {
> +       .dev_id = "leds-gpio",
> +       .table = {
> +               GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 51, NULL, 0, GPIO_ACTIVE_LOW),
> +               GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 52, NULL, 1, GPIO_ACTIVE_LOW),
> +               GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 53, NULL, 2, GPIO_ACTIVE_LOW),
> +               GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 57, NULL, 3, GPIO_ACTIVE_LOW),
> +               GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 58, NULL, 4, GPIO_ACTIVE_LOW),
> +               GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 60, NULL, 5, GPIO_ACTIVE_LOW),
> +               GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 56, NULL, 6, GPIO_ACTIVE_LOW),
> +               GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 59, NULL, 7, GPIO_ACTIVE_HIGH),

> +       }

Keeping a comma is good here.

> +};

...

> +       /* PM_BIOS_BOOT_N */
> +       gpiod = gpiod_get_index(&simatic_leds_pdev->dev, NULL, 6, 0);

This is basically GPIOD_ASIS...

> +       if (IS_ERR(gpiod)) {
> +               err = PTR_ERR(gpiod);
> +               goto out;
> +       }

> +       gpiod_set_value(gpiod, 0);

...but you may apply GPIOD_OUTPUT_LOW there and drop this call.

> +       gpiod_put(gpiod);

Ditto for the rest GPIO requests.

...

> +static struct platform_driver simatic_ipc_led_gpio_driver = {
> +       .probe = simatic_ipc_leds_gpio_probe,
> +       .remove = simatic_ipc_leds_gpio_remove,
> +       .driver = {
> +               .name = KBUILD_MODNAME,
> +       }
> +};

> +

No need to have this blank line.

> +module_platform_driver(simatic_ipc_led_gpio_driver);

-- 
With Best Regards,
Andy Shevchenko