On Thu, Mar 23, 2017 at 09:46:14PM +0200, Andy Shevchenko wrote:
> The commit 10cf4899f8af ("gpiolib: tighten up ACPI legacy gpio lookups")
> prevents to getting same resource twice if the driver asks twice using same
s/same/different/
> connection ID.
>
> But the whole idea of fallback might bring some problems. Imagine the case
> when
> we have two versions of BIOS/hardware where in one _DSD is introduced along
> with GPIO resources, but the other one uses just plain GPIO resource for
> another purpose
>
> Case 1:
>
> Device (DEVX)
> {
> ...
> Name (_CRS, ResourceTemplate ()
> {
> GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
> "\\_SB.GPO0", 0, ResourceConsumer) {15}
> })
> Name (_DSD, Package ()
> {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package ()
> {
> Package () {"some-gpios", Package() {^DEVX, 0, 0, 0 }},
> }
> })
> }
>
> Case 2:
>
> Device (DEVX)
> {
> ...
> Name (_CRS, ResourceTemplate ()
> {
> GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
> "\\_SB.GPO0", 0, ResourceConsumer) {27}
> })
> }
>
> To prevent the possible misconfiguration tighten up even more ACPI GPIO
> lookups
> for case without connection ID provided.
I wonder if this will break Goodix. Irina, Bastien?
>
> In the past the issue had been triggered by "use mctrl_gpio helpers" series
> [1,2].
>
> Besides above, removal of the main logic of acpi_can_fallback_to_crs()
> eliminates a potential memory leak when the same device has been unbound and
> bound again.
Where? We'll reuse lookup table as ACPI device is still the same.
>
> [1] commit 4ef03d328769 ("tty/serial/8250: use mctrl_gpio helpers")
> [2] https://patchwork.kernel.org/patch/9283745/
>
> Cc: Dmitry Torokhov <dmitry.torok...@gmail.com>
> Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
> ---
> drivers/gpio/gpiolib-acpi.c | 36 +-----------------------------------
> 1 file changed, 1 insertion(+), 35 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 21e4930ca2db..e516b7a0cc50 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -1121,45 +1121,11 @@ int acpi_gpio_count(struct device *dev, const char
> *con_id)
> return count ? count : -ENOENT;
> }
>
> -struct acpi_crs_lookup {
> - struct list_head node;
> - struct acpi_device *adev;
> - const char *con_id;
> -};
> -
> -static DEFINE_MUTEX(acpi_crs_lookup_lock);
> -static LIST_HEAD(acpi_crs_lookup_list);
> -
> bool acpi_can_fallback_to_crs(struct acpi_device *adev, const char *con_id)
> {
> - struct acpi_crs_lookup *l, *lookup = NULL;
> -
> /* Never allow fallback if the device has properties */
> if (adev->data.properties || adev->driver_gpios)
> return false;
>
> - mutex_lock(&acpi_crs_lookup_lock);
> -
> - list_for_each_entry(l, &acpi_crs_lookup_list, node) {
> - if (l->adev == adev) {
> - lookup = l;
> - break;
> - }
> - }
> -
> - if (!lookup) {
> - lookup = kmalloc(sizeof(*lookup), GFP_KERNEL);
> - if (lookup) {
> - lookup->adev = adev;
> - lookup->con_id = kstrdup(con_id, GFP_KERNEL);
> - list_add_tail(&lookup->node, &acpi_crs_lookup_list);
> - }
> - }
> -
> - mutex_unlock(&acpi_crs_lookup_lock);
> -
> - return lookup &&
> - ((!lookup->con_id && !con_id) ||
> - (lookup->con_id && con_id &&
> - strcmp(lookup->con_id, con_id) == 0));
> + return con_id == NULL;
> }
> --
> 2.11.0
>
--
Dmitry
