Re: [PATCH v9 2/9] Input: goodix - reset device at init

[Dmitry Torokhov]

On Tue, Oct 13, 2015 at 01:07:24PM +0300, mika.westerberg@xxxxxxxxxxxxxxx wrote:
> On Tue, Oct 13, 2015 at 08:54:12AM +0000, Tirdea, Irina wrote:
> > > > I did not use devm_gpiod_get_optional() in order to ignore more errors
> > > > than -ENOENT. This is needed because the ACPI gpio core will fall back
> > > > to indexed gpios if named gpios are not found. In the common case of
> > > > having 2 indexed gpio pins declared in the ACPI table, the first
> > > > devm_gpiod_get() will successfully get indexed gpio pin 0 and the
> > > > second devm_gpiod_get() will try to get the same gpio pin 0 and return
> > > > -EBUSY. Considering this, I thought it is better to just ignore all errors in
> > > > order not to break any platforms currently using this driver.
> > > 
> > > This seems like issue with ACPI gpio lookup implementation. If I am
> > > requesting named gpio and it is not present then I definitely do not
> > > need to be returned some random gpio. Doing so breaks all other drivers
> > > that use several names to retrieve GPIOs. We basically can't trust GPIO
> > > API on ACPI systems.
> > > 
> > 
> > I'm not sure there is a way to avoid fall back to indexed gpios when requesting
> > named gpios.
> > Adding Mika to this thread as he might help answer this.
> 
> Before ACPI 5.1 _DSD device properties were introduced all we had was an
> array of GPIOs returned by _CRS ACPI method. Ordering of those GPIOs
> could change from one vendor to another :-(
> 
> We can (and do) use acpi_dev_add_driver_gpios() to pass correct mappings
> where _DSD is not present based on the device ACPI ID for instance. Not
> all drivers do that, though.
> 
> I would like to get rid of the fallback completely at some point. We
> have had already problems with the API because then some ACPI only
> drivers did this:
> 
> 	reset_gpio = gpiod_get_index(dev, NULL, 0);
> 	power_gpio = gpiod_get_index(dev, NULL, 1);
> 
> which might not do what is expected on DT systems. That's why
> acpi_dev_add_driver_gpios() was added in the first place IIRC.

I understand why one might use acpi_dev_add_driver_gpios() to augment
data in ACPI, however here we have completely different issue: driver
that expects named gpios gets returned gpio that has nothing to do with
what it requested, because gpiolib acpi code always falls back to
unnamed gpio if it does not find named gpio. That can be acceptable if
driver uses the same con_id for all requests to gpiolib, but is not
working when driver supplies different con_ids.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/