On 23/02/2021 12:01, Andy Shevchenko wrote: >>>> + if (ares->type != ACPI_RESOURCE_TYPE_GPIO || >>>> + ares->data.gpio.connection_type != ACPI_RESOURCE_GPIO_TYPE_IO) >>>> + return 1; /* Deliberately positive so parsing continues */ >>> I don't like to lose control over ACPI_RESOURCE_TYPE_GPIO, i.e. >>> spreading it over kernel code (yes, I know about one existing TS >>> case). >>> Consider to provide a helper in analogue to acpi_gpio_get_irq_resource(). >> Sure, but I probably name it acpi_gpio_is_io_resource() - a function >> named "get" which returns a bool seems a bit funny to me. > But don't you need the resource itself? > > You may extract and check resource at the same time as > acpi_gpio_get_irq_resource() does. Oh! Reading comprehension fail; I didn't notice it was returning the pointer through agpio; you're right of course. > > ... > >>>> + struct int3472_discrete_device *int3472 = platform_get_drvdata(pdev); >>>> + if (int3472->gpios.dev_id) >>>> + gpiod_remove_lookup_table(&int3472->gpios); >>> gpiod_remove_lookup_table() is now NULL-aware. >>> But in any case I guess you don't need the above check. >> Sorry; forgot to call out that I didn't follow that suggestion; >> int3472->gpios is a _struct_ rather than a pointer, so &int3472->gpios >> won't be NULL, even if I haven't filled anything in to there yet because >> it failed before it got to that point. So, not sure that it quite works >> there. > I think if you initialize the ->list member you can remove without check. I'll give that a try - thanks >
