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@xxxxxxxxxxxxxxx> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
