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
