Re: [PATCH v1 1/3] gpiolib: acpi: Respect bias settings for GpioInt() resource

[Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>]

On Wed, Oct 21, 2020 at 07:38:44PM +0300, Andy Shevchenko wrote:
> On Wed, Oct 21, 2020 at 12:58:54PM +0300, Mika Westerberg wrote:
> > On Wed, Oct 14, 2020 at 04:31:52PM +0300, Andy Shevchenko wrote:
> > > In some cases the GpioInt() resource is coming with bias settings
> > > which may affect system functioning. Respect bias settings for
> > > GpioInt() resource by calling acpi_gpio_update_gpiod_*flags() API
> > > in acpi_dev_gpio_irq_get().
> > > 
> > > While at it, refactor to configure flags first and, only when succeeded,
> > > map the IRQ descriptor.
> 
> ...
> 
> > > -			irq = gpiod_to_irq(desc);
> > > -			if (irq < 0)
> > > -				return irq;
> > > +			acpi_gpio_update_gpiod_flags(&dflags, &info);
> > > +			acpi_gpio_update_gpiod_lookup_flags(&lflags, &info);
> > >  
> > >  			snprintf(label, sizeof(label), "GpioInt() %d", index);
> > > -			ret = gpiod_configure_flags(desc, label, lflags, info.flags);
> > > +			ret = gpiod_configure_flags(desc, label, lflags, dflags);
> > >  			if (ret < 0)
> > >  				return ret;
> > >  
> > > +			irq = gpiod_to_irq(desc);
> > > +			if (irq < 0)
> > > +				return irq;
> > 
> > Should the above be undone if the conversion here fails?
> 
> But wouldn't it be not good if we changed direction, for example, and then
> change it back? (IRQ requires input, which is safer, right?)
> 
> This makes me think what gpiod_to_irq() may do for physical state of the pin.
> On the brief search it seems there is no side effect on the pin with that
> function, so, perhaps the original order has that in mind to not shuffle with
> line if mapping can't be established. But if setting flags fail, we may got
> into the state which is not equal to the initial one, right?
> 
> So, in either case I see no good way to roll back the physical pin state
> changes. But I can return ordering of the calls in next version.
> 
> What do you think?

If there is no good way rolling back to the previous state then I think
this ordering is as good as the original, so up to you :-)