Re: [PATCH v2] Input: silead - Do not try to directly access the GPIO when using ACPI pm

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Wed, 2017-03-08 at 10:08 +0100, Hans de Goede wrote:
> Hi,
> 
> On 07-03-17 14:55, Hans de Goede wrote:
> > Hi,
> 
> <more snip>

Thanks for looking into this!
My comments below.

> > Ok, since it seems clear that I'm not going to be able to change
> > your
> > mind on this, I will give your patches a try and see if they fix the
> > silead ts problems.
> 
> So I've cherry picked all the gpio related patches from your
> topic/uart/rpm
> branch into my wip branch and then ran some tests. 

Some of them are WIP, so, they might break something as well.

> I did not get around
> to actually test if the fix the silead issue (I believe they will) as
> I
> started testing on a cht device and looking if soc_button_array still
> works with your patches applied.
> 
> Unfortunately it no longer works, there are 2 problems:
> 
> 1) "Input: soc_button_array - Add GPIO ACPI mapping table" should
> also replace:
> 
> 	desc = gpiod_get_index(dev, info->name, info->acpi_index,
> GPIOD_ASIS);
> 
> with:
> 
> 	desc = gpiod_get(dev, info->name, GPIOD_ASIS);
> 
> At which point we can also drop the acpi_index field from the
> buttoninfo struct
> altogether.
> 

I was thinking about passing NULL as connection ID there as it's done
for surface3 button array driver. In current soc-button-array we have
file name passed for all of the pins, which is slightly informative.

> I think that "extcon: int3496: Add GPIO ACPI mapping table" will need
> a similar change (I haven't tested it yet).

The mapping table converts Linux index, which you pass via
gpiod_get_index(), and _CRS index pair (resource, index in a list).

If it doesn't work that way, there is another bug then.

> 2) acpi_gpio_count() does not seem to work right in combination with
> the
> new patches. It returns -ENOENT rather then the number of gpios
> specified
> in the table passed to devm_acpi_dev_add_driver_gpios. It seems to
> only
> check for gpios actually in the acpi-properties without looking at
> adev->driver_gpios, where as acpi_can_fallback_to_crs() does check for
> that and disallows fallback to counting the gpios in the _CRS causing
> acpi_gpio_count() to not find any gpios. I believe the right fix for
> this is to make acpi_gpio_count() also count the number of entries
> in the adev->driver_gpios table.
> 

Thanks for catching this, it sounds indeed as a bug.

> For now I've just removed the acpi_gpio_count() check from
> soc_button_array,
> with that removed and 1) fixed soc_button_array does work.
> 
> I will try to do some more testing later today, but all my cht work is
> a side project and I first need to finish some stuff for my actual
> main $dayjob project.

Understood.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy