RE: [RFC PATCH 0/4] Add ACPI support for pinctrl configuration

[Tirdea, Irina]

> -----Original Message-----
> From: Mark Rutland [mailto:mark.rutland@xxxxxxx]
> Sent: 05 April, 2016 1:52
> To: Tirdea, Irina
> Cc: Rafael J. Wysocki; Len Brown; Mika Westerberg; Linus Walleij; linux-gpio@xxxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; Rob
> Herring; Heikki Krogerus; Andy Shevchenko; Purdila, Octavian; Ciocan, Cristina; devicetree@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; charles.garcia-tobin@xxxxxxx
> Subject: Re: [RFC PATCH 0/4] Add ACPI support for pinctrl configuration
> 
> Hi,

Hi Mark,

> 
> On Thu, Mar 31, 2016 at 02:44:41PM +0300, Irina Tirdea wrote:
> > This is a proposal for adding ACPI support for pin controller
> > configuration.
> >
> > It has been developed to enable the MinnowBoard and IoT community
> > by providing an easy way to specify pin multiplexing and
> > pin configuration.
> >
> > This proposal is based on using _DSD properties to specify device
> > states and configuration nodes and it follows closely the device
> > tree model. Device states are defined using the Device Properties
> > format and the configuration nodes are defined using the
> > Hierarchical Properties Extension format. The generic properties
> > for the configuration nodes are the same as the ones for device
> > tree, while pincontroller drivers can also define custom ones.
> 
> From a look of the Documentation addition, and of the current uses of
> pinctrl-names in device tree bindings, one reason for requiring multiple
> pinctrl states is power management. Given that, I'm somewhat concerned by this,
> as it goes against the usual ACPI model of abstracting this sort of thing
> behind power control methods.
> 

Right, there is an overlap of the pinctrl "sleep" state with the ACPI power
management model. 

However, the main reason for implementing this is setting initial pin multiplexing
and configuration. This is normally done by BIOS, but there is currently no way of
changing the default configuration (except for a BIOS update). This is a problem
when using boards like MinnowBoard, where it is expected to get the board and
to be able to add various devices to its exported GPIO pins. We need a way to
change default pin multiplexing to enable the functionality required by a specific
device. In some cases we also need to set the bias to get things like interrupts working.

Another use case for pinctrl states is using custom reset pin configurations that need
to be controlled from the driver.

Since we have the pinctrl infrastructure and the all Intel ACPI pinctrl drivers already
support it, this seems the most straightforward way to implement it.

> To the best of my knowledge, that goes against the ASWG's expectations on how
> _DSD will be used (per [1]). Charles, please correct me if that document is no
> longer representative.
> 
> Additionally, pinctrl has some cross-cutting concerns (e.g. mutually exclusive
> device usage due to a shared pin), and I can imagine that may interact poorly
> with any AML or firmware assumptions about the state of the world, as there's
> no mechanism present to notify those of changes to pins.
> 

This is a problem with any modification we would make to the ACPI tables. 
It is true that pinctrl configuration could create conflicts when changing
pin multiplexing, but that could be avoided by using only the pinctrl states that
make sense for the entire ACPI configuration.

> I think that this is a class of problem which needs to be raised with the ASWG,
> and solved in an ACPI-native fashion, rather than simply copying properties
> from DT using _DSD. If nothing else, the restrictions on FW and AML would need
> to be specified.

I agree this should be at least mentioned in the Documentation. I'll update it accordingly.


Thanks,
Irina

> 
> Thanks,
> Mark.
> 
> [1] https://lists.acpica.org/pipermail/dsd/2015-September/000019.html