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

[Rafael J. Wysocki]

On Tue, Apr 12, 2016 at 2:15 PM, Mark Brown  wrote:
> On Thu, Apr 07, 2016 at 11:24:00PM +0200, Rafael J. Wysocki wrote:
>> On Wednesday, April 06, 2016 11:39:27 AM Mark Rutland wrote:
>
>> > The ACPI model implies FW-driven pinctrl management, so if we're going to 
>> > put
>> > the OS in direct control of pinctrl, we have to make clear what 
>> > expectation FW
>> > and OS can have.
>
>> Well, orthodox ACPI people from the Windows camp would definitely agree with 
>> you,
>> but I'm not one of them, so let me play a devil's advocate for a while. :-)
>
>> IMO, exposing anything in the ACPI tables without explicitly saying "but you
>> should not touch that", eg by defining an operation region covering it or 
>> similar,
>> is equivalent to giving the OS a license to play with that thing as it 
>> wishes.
>
>> Therefore I would regard exposing pin control information in cases when the
>> OS is not allowed to use it to its fit as a firmware bug.
>
> This is something where it seems like it would be very easy for people
> to end up relying on current OS behaviour even if that only happened
> through accident rather than design.  You might assume that such
> behaviour is a bug but that may not be obvious to the firmware author if
> the OS behaviour makes sense to them.

If the OS started to support that interface, it would need to
guarantee consistent behavior with respect to it going forward.

It would be an interface for interacting with firmware, so it is not
much different from an interface to user space as far as the stability
rules go.  Which may be a reason for refusing to support it in the
first place if the OS is not willing to make such a guarantee.

> There may also be some expectation on the part of firmware that the OS
> will do some management of the pins even for devices that it's not
> actively working with (put them in an idle mode for example).
>
>> Now, the question in this patch series is how to expose things and not when 
>> to
>> do that.  It adds support for a specific method of exposing information to 
>> the
>> OS, but should it be concerned about the possible consequences of 
>> inappropriate
>> use of that method by firmware?
>
> The other issue here is that if (as Octavian mentioned) there are
> ongoing discussions within ASWG on producing an actual spec for this it
> doesn't seem great that we'd just go and do something completely
> different rather than trying to make sure that the spec works well.  Or
> if there isn't any spec work then writing one there for other OSs to
> pick up if they like.  This seems like it'd make us much better
> community players.

Agreed.  I actually asked Octavian to look at the other proposal and
see if/how that could be used to address the use case in question
and/or possibly extended to cover it.

Thanks,
Rafael

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

[Rafael J. Wysocki]

On Tue, Apr 12, 2016 at 2:15 PM, Mark Brown  wrote:
> On Thu, Apr 07, 2016 at 11:24:00PM +0200, Rafael J. Wysocki wrote:
>> On Wednesday, April 06, 2016 11:39:27 AM Mark Rutland wrote:
>
>> > The ACPI model implies FW-driven pinctrl management, so if we're going to 
>> > put
>> > the OS in direct control of pinctrl, we have to make clear what 
>> > expectation FW
>> > and OS can have.
>
>> Well, orthodox ACPI people from the Windows camp would definitely agree with 
>> you,
>> but I'm not one of them, so let me play a devil's advocate for a while. :-)
>
>> IMO, exposing anything in the ACPI tables without explicitly saying "but you
>> should not touch that", eg by defining an operation region covering it or 
>> similar,
>> is equivalent to giving the OS a license to play with that thing as it 
>> wishes.
>
>> Therefore I would regard exposing pin control information in cases when the
>> OS is not allowed to use it to its fit as a firmware bug.
>
> This is something where it seems like it would be very easy for people
> to end up relying on current OS behaviour even if that only happened
> through accident rather than design.  You might assume that such
> behaviour is a bug but that may not be obvious to the firmware author if
> the OS behaviour makes sense to them.

If the OS started to support that interface, it would need to
guarantee consistent behavior with respect to it going forward.

It would be an interface for interacting with firmware, so it is not
much different from an interface to user space as far as the stability
rules go.  Which may be a reason for refusing to support it in the
first place if the OS is not willing to make such a guarantee.

> There may also be some expectation on the part of firmware that the OS
> will do some management of the pins even for devices that it's not
> actively working with (put them in an idle mode for example).
>
>> Now, the question in this patch series is how to expose things and not when 
>> to
>> do that.  It adds support for a specific method of exposing information to 
>> the
>> OS, but should it be concerned about the possible consequences of 
>> inappropriate
>> use of that method by firmware?
>
> The other issue here is that if (as Octavian mentioned) there are
> ongoing discussions within ASWG on producing an actual spec for this it
> doesn't seem great that we'd just go and do something completely
> different rather than trying to make sure that the spec works well.  Or
> if there isn't any spec work then writing one there for other OSs to
> pick up if they like.  This seems like it'd make us much better
> community players.

Agreed.  I actually asked Octavian to look at the other proposal and
see if/how that could be used to address the use case in question
and/or possibly extended to cover it.

Thanks,
Rafael

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

[Mark Brown]

On Thu, Apr 07, 2016 at 11:24:00PM +0200, Rafael J. Wysocki wrote:
> On Wednesday, April 06, 2016 11:39:27 AM Mark Rutland wrote:

> > The ACPI model implies FW-driven pinctrl management, so if we're going to 
> > put
> > the OS in direct control of pinctrl, we have to make clear what expectation 
> > FW
> > and OS can have.

> Well, orthodox ACPI people from the Windows camp would definitely agree with 
> you,
> but I'm not one of them, so let me play a devil's advocate for a while. :-)

> IMO, exposing anything in the ACPI tables without explicitly saying "but you
> should not touch that", eg by defining an operation region covering it or 
> similar,
> is equivalent to giving the OS a license to play with that thing as it wishes.

> Therefore I would regard exposing pin control information in cases when the
> OS is not allowed to use it to its fit as a firmware bug.

This is something where it seems like it would be very easy for people
to end up relying on current OS behaviour even if that only happened
through accident rather than design.  You might assume that such
behaviour is a bug but that may not be obvious to the firmware author if
the OS behaviour makes sense to them.

There may also be some expectation on the part of firmware that the OS
will do some management of the pins even for devices that it's not
actively working with (put them in an idle mode for example).

> Now, the question in this patch series is how to expose things and not when to
> do that.  It adds support for a specific method of exposing information to the
> OS, but should it be concerned about the possible consequences of 
> inappropriate
> use of that method by firmware?

The other issue here is that if (as Octavian mentioned) there are
ongoing discussions within ASWG on producing an actual spec for this it
doesn't seem great that we'd just go and do something completely
different rather than trying to make sure that the spec works well.  Or
if there isn't any spec work then writing one there for other OSs to
pick up if they like.  This seems like it'd make us much better
community players.


signature.asc
Description: PGP signature

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

[Mark Brown]

On Thu, Apr 07, 2016 at 11:24:00PM +0200, Rafael J. Wysocki wrote:
> On Wednesday, April 06, 2016 11:39:27 AM Mark Rutland wrote:

> > The ACPI model implies FW-driven pinctrl management, so if we're going to 
> > put
> > the OS in direct control of pinctrl, we have to make clear what expectation 
> > FW
> > and OS can have.

> Well, orthodox ACPI people from the Windows camp would definitely agree with 
> you,
> but I'm not one of them, so let me play a devil's advocate for a while. :-)

> IMO, exposing anything in the ACPI tables without explicitly saying "but you
> should not touch that", eg by defining an operation region covering it or 
> similar,
> is equivalent to giving the OS a license to play with that thing as it wishes.

> Therefore I would regard exposing pin control information in cases when the
> OS is not allowed to use it to its fit as a firmware bug.

This is something where it seems like it would be very easy for people
to end up relying on current OS behaviour even if that only happened
through accident rather than design.  You might assume that such
behaviour is a bug but that may not be obvious to the firmware author if
the OS behaviour makes sense to them.

There may also be some expectation on the part of firmware that the OS
will do some management of the pins even for devices that it's not
actively working with (put them in an idle mode for example).

> Now, the question in this patch series is how to expose things and not when to
> do that.  It adds support for a specific method of exposing information to the
> OS, but should it be concerned about the possible consequences of 
> inappropriate
> use of that method by firmware?

The other issue here is that if (as Octavian mentioned) there are
ongoing discussions within ASWG on producing an actual spec for this it
doesn't seem great that we'd just go and do something completely
different rather than trying to make sure that the spec works well.  Or
if there isn't any spec work then writing one there for other OSs to
pick up if they like.  This seems like it'd make us much better
community players.


signature.asc
Description: PGP signature

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

[Rafael J. Wysocki]

On Wednesday, April 06, 2016 11:39:27 AM Mark Rutland wrote:
> On Thu, Apr 07, 2016 at 03:11:53PM +0300, Octavian Purdila wrote:
> > On Wed, Apr 6, 2016 at 3:01 AM, Mark Rutland  wrote:
> > > On Tue, Apr 05, 2016 at 11:09:34PM +0300, Octavian Purdila wrote:
> > >> On Tue, Apr 5, 2016 at 9:16 PM, Mark Rutland  
> > >> wrote:
> > >> > * The firmware is to some extent expected to manage pinctrl today (for 
> > >> > power
> > >> >   management of devices it does know about), and hence a pinctrl 
> > >> > device is
> > >> >   potentially under shared management of ACPI and the OS.
> > >> >
> > >> > * The ACPI specification says nothing about this style of pinctrl 
> > >> > management,
> > >> >   so it is unclear what the expectations are:
> > >>
> > >> Does it say anything at all about pinctrl management?
> > >
> > > To the best of my knowledge the ACPI spec does not explicitly mention 
> > > pinctrl.
> > > In another reply it was mentioned that there is work in progress for 
> > > pinmuxing,
> > > so evidently it is on the ASWG radar and within the scope of ACPI.
> > >
> > > I was trying to point out that it is _implicitly_ firmware's 
> > > responsibility to
> > > do any pinctrl today, due to the ACPI model for power management, and the 
> > > lack
> > > of an existing ACPI mechanisms to provide pinctrl data.
> > >
> > > In practice, firmware configures pinctrl at boot, and may modify pinctrl 
> > > as
> > > part of the runtime power management firmware is put in charge of.
> > >
> > 
> > AFAIK the firmware only uses the pinctrl at boot to set the initial
> > values and the OS owns it after boot. The only interaction I know of
> > after boot are GPIO signaled events, but those are executed under the
> > control of the OS.
> > 
> > I don't understand the part about firmware being put in charge of
> > runtime power management. Do you mean that the firmware directly
> > accesses the pinctrl registers? Doesn't this contradict the ACPI goal
> > of having the OS control power management? Or do you mean accessing
> > pinctrl registers via _PSx and PowerResource._On/_Off?
> 
> I mostly mean the latter. Even if the OS is in charge of _when_ those happen,
> that only solves mutual exclusion over the pinctrl registers. That does not
> handle expectations regarding the current _state_ of the pinctrl configuation,
> or the configurations the OS/FW can permit and/or require (e.g. there's no
> refcounting between OS and FW for the state of shared pins).
> 
> It may also be that pinctrl gets altered in the background (e.g. in SMM),
> outside of the OS's control also, but that's probably a rare/extreme case.
> 
> > > The lack of any statement about pinctrl would mean that there is 
> > > effectively no
> > > reasonable limitation on what firmware might do with pinctrl, and we 
> > > cannot
> > > assume specific behaviour from the firmware.
> > 
> > There is noting specified in the spec about other controllers, why is
> > pinctrl special in this regard?
> 
> Because there are clear demonstrable cases why FW would want to touch pinctrl
> today, that may clash with the pinctrl model you are importing from DT (where
> the OS is assumed to have complete ownership and control over pinctrl).
> 
> The ACPI model implies FW-driven pinctrl management, so if we're going to put
> the OS in direct control of pinctrl, we have to make clear what expectation FW
> and OS can have.

Well, orthodox ACPI people from the Windows camp would definitely agree with 
you,
but I'm not one of them, so let me play a devil's advocate for a while. :-)

IMO, exposing anything in the ACPI tables without explicitly saying "but you
should not touch that", eg by defining an operation region covering it or 
similar,
is equivalent to giving the OS a license to play with that thing as it wishes.

Therefore I would regard exposing pin control information in cases when the
OS is not allowed to use it to its fit as a firmware bug.

Now, the question in this patch series is how to expose things and not when to
do that.  It adds support for a specific method of exposing information to the
OS, but should it be concerned about the possible consequences of inappropriate
use of that method by firmware?

Thanks,
Rafael

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

[Rafael J. Wysocki]

On Wednesday, April 06, 2016 11:39:27 AM Mark Rutland wrote:
> On Thu, Apr 07, 2016 at 03:11:53PM +0300, Octavian Purdila wrote:
> > On Wed, Apr 6, 2016 at 3:01 AM, Mark Rutland  wrote:
> > > On Tue, Apr 05, 2016 at 11:09:34PM +0300, Octavian Purdila wrote:
> > >> On Tue, Apr 5, 2016 at 9:16 PM, Mark Rutland  
> > >> wrote:
> > >> > * The firmware is to some extent expected to manage pinctrl today (for 
> > >> > power
> > >> >   management of devices it does know about), and hence a pinctrl 
> > >> > device is
> > >> >   potentially under shared management of ACPI and the OS.
> > >> >
> > >> > * The ACPI specification says nothing about this style of pinctrl 
> > >> > management,
> > >> >   so it is unclear what the expectations are:
> > >>
> > >> Does it say anything at all about pinctrl management?
> > >
> > > To the best of my knowledge the ACPI spec does not explicitly mention 
> > > pinctrl.
> > > In another reply it was mentioned that there is work in progress for 
> > > pinmuxing,
> > > so evidently it is on the ASWG radar and within the scope of ACPI.
> > >
> > > I was trying to point out that it is _implicitly_ firmware's 
> > > responsibility to
> > > do any pinctrl today, due to the ACPI model for power management, and the 
> > > lack
> > > of an existing ACPI mechanisms to provide pinctrl data.
> > >
> > > In practice, firmware configures pinctrl at boot, and may modify pinctrl 
> > > as
> > > part of the runtime power management firmware is put in charge of.
> > >
> > 
> > AFAIK the firmware only uses the pinctrl at boot to set the initial
> > values and the OS owns it after boot. The only interaction I know of
> > after boot are GPIO signaled events, but those are executed under the
> > control of the OS.
> > 
> > I don't understand the part about firmware being put in charge of
> > runtime power management. Do you mean that the firmware directly
> > accesses the pinctrl registers? Doesn't this contradict the ACPI goal
> > of having the OS control power management? Or do you mean accessing
> > pinctrl registers via _PSx and PowerResource._On/_Off?
> 
> I mostly mean the latter. Even if the OS is in charge of _when_ those happen,
> that only solves mutual exclusion over the pinctrl registers. That does not
> handle expectations regarding the current _state_ of the pinctrl configuation,
> or the configurations the OS/FW can permit and/or require (e.g. there's no
> refcounting between OS and FW for the state of shared pins).
> 
> It may also be that pinctrl gets altered in the background (e.g. in SMM),
> outside of the OS's control also, but that's probably a rare/extreme case.
> 
> > > The lack of any statement about pinctrl would mean that there is 
> > > effectively no
> > > reasonable limitation on what firmware might do with pinctrl, and we 
> > > cannot
> > > assume specific behaviour from the firmware.
> > 
> > There is noting specified in the spec about other controllers, why is
> > pinctrl special in this regard?
> 
> Because there are clear demonstrable cases why FW would want to touch pinctrl
> today, that may clash with the pinctrl model you are importing from DT (where
> the OS is assumed to have complete ownership and control over pinctrl).
> 
> The ACPI model implies FW-driven pinctrl management, so if we're going to put
> the OS in direct control of pinctrl, we have to make clear what expectation FW
> and OS can have.

Well, orthodox ACPI people from the Windows camp would definitely agree with 
you,
but I'm not one of them, so let me play a devil's advocate for a while. :-)

IMO, exposing anything in the ACPI tables without explicitly saying "but you
should not touch that", eg by defining an operation region covering it or 
similar,
is equivalent to giving the OS a license to play with that thing as it wishes.

Therefore I would regard exposing pin control information in cases when the
OS is not allowed to use it to its fit as a firmware bug.

Now, the question in this patch series is how to expose things and not when to
do that.  It adds support for a specific method of exposing information to the
OS, but should it be concerned about the possible consequences of inappropriate
use of that method by firmware?

Thanks,
Rafael

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

[Linus Walleij]

On Thu, Apr 7, 2016 at 4:17 PM, Octavian Purdila
 wrote:
> On Wed, Apr 6, 2016 at 1:49 PM, Graeme Gregory  wrote:

>> Why has there been no attempt in ASWG to make these sort of features a
>> 1st class citizen of ACPI so they can interact correctly with the other
>> features?
>
> IMO having an ASWG pinctrl specification and using _DSD for Linux are
> orthogonal approaches. The latter is very specific to Linux and we
> want it so that we can support the Linux pinctrl model as best as
> possible and I doubt it is a common denominator for all OSes that ACPI
> supports.

Sob, I tried so hard to implement pin control referring to generic
concepts that everyone should be able to reuse. E.g. stressing SI
units to be used all over the place, neutral scientific terms for all
config options and what not.

The multiplexing with groups and functions is a simple (aehm,
OK, super-complicated) matter of mapping disjunct sets onto each
other, so that comes from logic.

Not even the GPIO ranges that map GPIOs to pin ranges are very
Linux-specific: it is a property of the hardware that sometimes
GPIO is "behind" the pin, in a distinct hardware unit. (See pinctrl.txt
for the GPIO pitfalls.)

If I was to rewrite pin control and GPIO from scratch I would make it
one subsystem instead of two. This is more of an architectural
choice. GPIO would still be a stand-alone part of it.

Yours,
Linus Walleij

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

[Linus Walleij]

On Thu, Apr 7, 2016 at 4:17 PM, Octavian Purdila
 wrote:
> On Wed, Apr 6, 2016 at 1:49 PM, Graeme Gregory  wrote:

>> Why has there been no attempt in ASWG to make these sort of features a
>> 1st class citizen of ACPI so they can interact correctly with the other
>> features?
>
> IMO having an ASWG pinctrl specification and using _DSD for Linux are
> orthogonal approaches. The latter is very specific to Linux and we
> want it so that we can support the Linux pinctrl model as best as
> possible and I doubt it is a common denominator for all OSes that ACPI
> supports.

Sob, I tried so hard to implement pin control referring to generic
concepts that everyone should be able to reuse. E.g. stressing SI
units to be used all over the place, neutral scientific terms for all
config options and what not.

The multiplexing with groups and functions is a simple (aehm,
OK, super-complicated) matter of mapping disjunct sets onto each
other, so that comes from logic.

Not even the GPIO ranges that map GPIOs to pin ranges are very
Linux-specific: it is a property of the hardware that sometimes
GPIO is "behind" the pin, in a distinct hardware unit. (See pinctrl.txt
for the GPIO pitfalls.)

If I was to rewrite pin control and GPIO from scratch I would make it
one subsystem instead of two. This is more of an architectural
choice. GPIO would still be a stand-alone part of it.

Yours,
Linus Walleij

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

[Mark Rutland]

On Thu, Apr 07, 2016 at 03:11:53PM +0300, Octavian Purdila wrote:
> On Wed, Apr 6, 2016 at 3:01 AM, Mark Rutland  wrote:
> > On Tue, Apr 05, 2016 at 11:09:34PM +0300, Octavian Purdila wrote:
> >> On Tue, Apr 5, 2016 at 9:16 PM, Mark Rutland  wrote:
> >> > * The firmware is to some extent expected to manage pinctrl today (for 
> >> > power
> >> >   management of devices it does know about), and hence a pinctrl device 
> >> > is
> >> >   potentially under shared management of ACPI and the OS.
> >> >
> >> > * The ACPI specification says nothing about this style of pinctrl 
> >> > management,
> >> >   so it is unclear what the expectations are:
> >>
> >> Does it say anything at all about pinctrl management?
> >
> > To the best of my knowledge the ACPI spec does not explicitly mention 
> > pinctrl.
> > In another reply it was mentioned that there is work in progress for 
> > pinmuxing,
> > so evidently it is on the ASWG radar and within the scope of ACPI.
> >
> > I was trying to point out that it is _implicitly_ firmware's responsibility 
> > to
> > do any pinctrl today, due to the ACPI model for power management, and the 
> > lack
> > of an existing ACPI mechanisms to provide pinctrl data.
> >
> > In practice, firmware configures pinctrl at boot, and may modify pinctrl as
> > part of the runtime power management firmware is put in charge of.
> >
> 
> AFAIK the firmware only uses the pinctrl at boot to set the initial
> values and the OS owns it after boot. The only interaction I know of
> after boot are GPIO signaled events, but those are executed under the
> control of the OS.
> 
> I don't understand the part about firmware being put in charge of
> runtime power management. Do you mean that the firmware directly
> accesses the pinctrl registers? Doesn't this contradict the ACPI goal
> of having the OS control power management? Or do you mean accessing
> pinctrl registers via _PSx and PowerResource._On/_Off?

I mostly mean the latter. Even if the OS is in charge of _when_ those happen,
that only solves mutual exclusion over the pinctrl registers. That does not
handle expectations regarding the current _state_ of the pinctrl configuation,
or the configurations the OS/FW can permit and/or require (e.g. there's no
refcounting between OS and FW for the state of shared pins).

It may also be that pinctrl gets altered in the background (e.g. in SMM),
outside of the OS's control also, but that's probably a rare/extreme case.

> > The lack of any statement about pinctrl would mean that there is 
> > effectively no
> > reasonable limitation on what firmware might do with pinctrl, and we cannot
> > assume specific behaviour from the firmware.
> 
> There is noting specified in the spec about other controllers, why is
> pinctrl special in this regard?

Because there are clear demonstrable cases why FW would want to touch pinctrl
today, that may clash with the pinctrl model you are importing from DT (where
the OS is assumed to have complete ownership and control over pinctrl).

The ACPI model implies FW-driven pinctrl management, so if we're going to put
the OS in direct control of pinctrl, we have to make clear what expectation FW
and OS can have.

> > [...]
> >
> >> Since our focus is for open-ended configurations and for hardware that
> >> it is not know to firmware we only considered the case where the pins
> >> are not touched after the system boots.
> >>
> >> Now I wonder what are the cases were the firmware changes the pin
> >> configuration after boot.
> >
> > Device power management and suspend/resume seem like the obvious cases.
> 
> I assume you are suggesting something like the following: we have a
> device that is powered via a GPIO and associated ACPI _PS3/_PS0
> methods are poking the pinctrl register to drive 0 or 1 to power on or
> off the device.

Potentially. In that case it's not clear what the FW is expected to do, and
what the OS is expected to do.

For things like system suspend/resume or hibernate, it's not clear what state
the FW is expected to save/restore, and what state might arbitrarily change.

> If that is the case, we can easily break that today, by changing that
> particular GPIO value via, e.g., sysfs.

Sure, but that is not part of the usual, automatic management of the system.

There are plenty of ways a user with sufficient privilege can bring easily down
a system. They are irrelevant.

Thanks,
Mark.

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

[Mark Rutland]

On Thu, Apr 07, 2016 at 03:11:53PM +0300, Octavian Purdila wrote:
> On Wed, Apr 6, 2016 at 3:01 AM, Mark Rutland  wrote:
> > On Tue, Apr 05, 2016 at 11:09:34PM +0300, Octavian Purdila wrote:
> >> On Tue, Apr 5, 2016 at 9:16 PM, Mark Rutland  wrote:
> >> > * The firmware is to some extent expected to manage pinctrl today (for 
> >> > power
> >> >   management of devices it does know about), and hence a pinctrl device 
> >> > is
> >> >   potentially under shared management of ACPI and the OS.
> >> >
> >> > * The ACPI specification says nothing about this style of pinctrl 
> >> > management,
> >> >   so it is unclear what the expectations are:
> >>
> >> Does it say anything at all about pinctrl management?
> >
> > To the best of my knowledge the ACPI spec does not explicitly mention 
> > pinctrl.
> > In another reply it was mentioned that there is work in progress for 
> > pinmuxing,
> > so evidently it is on the ASWG radar and within the scope of ACPI.
> >
> > I was trying to point out that it is _implicitly_ firmware's responsibility 
> > to
> > do any pinctrl today, due to the ACPI model for power management, and the 
> > lack
> > of an existing ACPI mechanisms to provide pinctrl data.
> >
> > In practice, firmware configures pinctrl at boot, and may modify pinctrl as
> > part of the runtime power management firmware is put in charge of.
> >
> 
> AFAIK the firmware only uses the pinctrl at boot to set the initial
> values and the OS owns it after boot. The only interaction I know of
> after boot are GPIO signaled events, but those are executed under the
> control of the OS.
> 
> I don't understand the part about firmware being put in charge of
> runtime power management. Do you mean that the firmware directly
> accesses the pinctrl registers? Doesn't this contradict the ACPI goal
> of having the OS control power management? Or do you mean accessing
> pinctrl registers via _PSx and PowerResource._On/_Off?

I mostly mean the latter. Even if the OS is in charge of _when_ those happen,
that only solves mutual exclusion over the pinctrl registers. That does not
handle expectations regarding the current _state_ of the pinctrl configuation,
or the configurations the OS/FW can permit and/or require (e.g. there's no
refcounting between OS and FW for the state of shared pins).

It may also be that pinctrl gets altered in the background (e.g. in SMM),
outside of the OS's control also, but that's probably a rare/extreme case.

> > The lack of any statement about pinctrl would mean that there is 
> > effectively no
> > reasonable limitation on what firmware might do with pinctrl, and we cannot
> > assume specific behaviour from the firmware.
> 
> There is noting specified in the spec about other controllers, why is
> pinctrl special in this regard?

Because there are clear demonstrable cases why FW would want to touch pinctrl
today, that may clash with the pinctrl model you are importing from DT (where
the OS is assumed to have complete ownership and control over pinctrl).

The ACPI model implies FW-driven pinctrl management, so if we're going to put
the OS in direct control of pinctrl, we have to make clear what expectation FW
and OS can have.

> > [...]
> >
> >> Since our focus is for open-ended configurations and for hardware that
> >> it is not know to firmware we only considered the case where the pins
> >> are not touched after the system boots.
> >>
> >> Now I wonder what are the cases were the firmware changes the pin
> >> configuration after boot.
> >
> > Device power management and suspend/resume seem like the obvious cases.
> 
> I assume you are suggesting something like the following: we have a
> device that is powered via a GPIO and associated ACPI _PS3/_PS0
> methods are poking the pinctrl register to drive 0 or 1 to power on or
> off the device.

Potentially. In that case it's not clear what the FW is expected to do, and
what the OS is expected to do.

For things like system suspend/resume or hibernate, it's not clear what state
the FW is expected to save/restore, and what state might arbitrarily change.

> If that is the case, we can easily break that today, by changing that
> particular GPIO value via, e.g., sysfs.

Sure, but that is not part of the usual, automatic management of the system.

There are plenty of ways a user with sufficient privilege can bring easily down
a system. They are irrelevant.

Thanks,
Mark.

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

[Octavian Purdila]

On Wed, Apr 6, 2016 at 1:49 PM, Graeme Gregory  wrote:

>>
>> I have to echo Mark's concern: wholesale importation of portions of current
>> DT bindings simply because it's expedient is one of the things I've been 
>> hoping
>> to avoid.  These patches seem to be just that.
>>
>> And while the latest version mentioned above describes a bit of a review
>> process to handle this case, I don't recall the kernel community at large
>> agreeing to it, nor to it having been implemented.  If I missed that part,
>> my apologies; please let me know where it was decided.  If I haven't, then
>> perhaps this needs to be the first test case of that process.
>>
>
> My concern over this is similar to Mark/Als this looks like a quick hack
> to "solve" an issue that has been worked on in Linux for at least 10
> years now. And thats how to describe a non descoverable card that is
> plugged into random busses on a SoC.
>
> The issue I see with a quick hack of DT pinctrl into _DSD is that this
> does not take into account the power model of ACPI. As far as I can see
> the core code has no manner to tell a pin/function allocated through
> _DSD actually has effects on the power of other devices. So the core
> could end up powering down devices when it should not. This is very
> relevent to your intended use of IoT devices where power control is key.
>

It is not clear to me how this could happen - core powering down
devices when it should not, unless pins are shared.  And if I am not
missing something, pinctrl is designed to handle this case by only by
switching to idle/suspend/default states from the controller drivers
in the suspend/resume routine. ACPI _Sx calls also happens at the same
time so there does not seem to be a contention point.

Perhaps I am missing something and an example of such issue would make
it more clear.

And, as our primary use case is to set the initial muxing, we can
easily remove the suspend/resume functionality if this turns out to be
the issue.

> Why has there been no attempt in ASWG to make these sort of features a
> 1st class citizen of ACPI so they can interact correctly with the other
> features?
>

IMO having an ASWG pinctrl specification and using _DSD for Linux are
orthogonal approaches. The latter is very specific to Linux and we
want it so that we can support the Linux pinctrl model as best as
possible and I doubt it is a common denominator for all OSes that ACPI
supports.

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

[Octavian Purdila]

On Wed, Apr 6, 2016 at 1:49 PM, Graeme Gregory  wrote:

>>
>> I have to echo Mark's concern: wholesale importation of portions of current
>> DT bindings simply because it's expedient is one of the things I've been 
>> hoping
>> to avoid.  These patches seem to be just that.
>>
>> And while the latest version mentioned above describes a bit of a review
>> process to handle this case, I don't recall the kernel community at large
>> agreeing to it, nor to it having been implemented.  If I missed that part,
>> my apologies; please let me know where it was decided.  If I haven't, then
>> perhaps this needs to be the first test case of that process.
>>
>
> My concern over this is similar to Mark/Als this looks like a quick hack
> to "solve" an issue that has been worked on in Linux for at least 10
> years now. And thats how to describe a non descoverable card that is
> plugged into random busses on a SoC.
>
> The issue I see with a quick hack of DT pinctrl into _DSD is that this
> does not take into account the power model of ACPI. As far as I can see
> the core code has no manner to tell a pin/function allocated through
> _DSD actually has effects on the power of other devices. So the core
> could end up powering down devices when it should not. This is very
> relevent to your intended use of IoT devices where power control is key.
>

It is not clear to me how this could happen - core powering down
devices when it should not, unless pins are shared.  And if I am not
missing something, pinctrl is designed to handle this case by only by
switching to idle/suspend/default states from the controller drivers
in the suspend/resume routine. ACPI _Sx calls also happens at the same
time so there does not seem to be a contention point.

Perhaps I am missing something and an example of such issue would make
it more clear.

And, as our primary use case is to set the initial muxing, we can
easily remove the suspend/resume functionality if this turns out to be
the issue.

> Why has there been no attempt in ASWG to make these sort of features a
> 1st class citizen of ACPI so they can interact correctly with the other
> features?
>

IMO having an ASWG pinctrl specification and using _DSD for Linux are
orthogonal approaches. The latter is very specific to Linux and we
want it so that we can support the Linux pinctrl model as best as
possible and I doubt it is a common denominator for all OSes that ACPI
supports.

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

[Octavian Purdila]

On Wed, Apr 6, 2016 at 3:01 AM, Mark Rutland  wrote:
> On Tue, Apr 05, 2016 at 11:09:34PM +0300, Octavian Purdila wrote:
>> On Tue, Apr 5, 2016 at 9:16 PM, Mark Rutland  wrote:
>> > * The firmware is to some extent expected to manage pinctrl today (for 
>> > power
>> >   management of devices it does know about), and hence a pinctrl device is
>> >   potentially under shared management of ACPI and the OS.
>> >
>> > * The ACPI specification says nothing about this style of pinctrl 
>> > management,
>> >   so it is unclear what the expectations are:
>>
>> Does it say anything at all about pinctrl management?
>
> To the best of my knowledge the ACPI spec does not explicitly mention pinctrl.
> In another reply it was mentioned that there is work in progress for 
> pinmuxing,
> so evidently it is on the ASWG radar and within the scope of ACPI.
>
> I was trying to point out that it is _implicitly_ firmware's responsibility to
> do any pinctrl today, due to the ACPI model for power management, and the lack
> of an existing ACPI mechanisms to provide pinctrl data.
>
> In practice, firmware configures pinctrl at boot, and may modify pinctrl as
> part of the runtime power management firmware is put in charge of.
>

AFAIK the firmware only uses the pinctrl at boot to set the initial
values and the OS owns it after boot. The only interaction I know of
after boot are GPIO signaled events, but those are executed under the
control of the OS.

I don't understand the part about firmware being put in charge of
runtime power management. Do you mean that the firmware directly
accesses the pinctrl registers? Doesn't this contradict the ACPI goal
of having the OS control power management? Or do you mean accessing
pinctrl registers via _PSx and PowerResource._On/_Off?

> The lack of any statement about pinctrl would mean that there is effectively 
> no
> reasonable limitation on what firmware might do with pinctrl, and we cannot
> assume specific behaviour from the firmware.
>

There is noting specified in the spec about other controllers, why is
pinctrl special in this regard?

> [...]
>
>> Since our focus is for open-ended configurations and for hardware that
>> it is not know to firmware we only considered the case where the pins
>> are not touched after the system boots.
>>
>> Now I wonder what are the cases were the firmware changes the pin
>> configuration after boot.
>
> Device power management and suspend/resume seem like the obvious cases.

I assume you are suggesting something like the following: we have a
device that is powered via a GPIO and associated ACPI _PS3/_PS0
methods are poking the pinctrl register to drive 0 or 1 to power on or
off the device.

If that is the case, we can easily break that today, by changing that
particular GPIO value via, e.g., sysfs.

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

[Octavian Purdila]

On Wed, Apr 6, 2016 at 3:01 AM, Mark Rutland  wrote:
> On Tue, Apr 05, 2016 at 11:09:34PM +0300, Octavian Purdila wrote:
>> On Tue, Apr 5, 2016 at 9:16 PM, Mark Rutland  wrote:
>> > * The firmware is to some extent expected to manage pinctrl today (for 
>> > power
>> >   management of devices it does know about), and hence a pinctrl device is
>> >   potentially under shared management of ACPI and the OS.
>> >
>> > * The ACPI specification says nothing about this style of pinctrl 
>> > management,
>> >   so it is unclear what the expectations are:
>>
>> Does it say anything at all about pinctrl management?
>
> To the best of my knowledge the ACPI spec does not explicitly mention pinctrl.
> In another reply it was mentioned that there is work in progress for 
> pinmuxing,
> so evidently it is on the ASWG radar and within the scope of ACPI.
>
> I was trying to point out that it is _implicitly_ firmware's responsibility to
> do any pinctrl today, due to the ACPI model for power management, and the lack
> of an existing ACPI mechanisms to provide pinctrl data.
>
> In practice, firmware configures pinctrl at boot, and may modify pinctrl as
> part of the runtime power management firmware is put in charge of.
>

AFAIK the firmware only uses the pinctrl at boot to set the initial
values and the OS owns it after boot. The only interaction I know of
after boot are GPIO signaled events, but those are executed under the
control of the OS.

I don't understand the part about firmware being put in charge of
runtime power management. Do you mean that the firmware directly
accesses the pinctrl registers? Doesn't this contradict the ACPI goal
of having the OS control power management? Or do you mean accessing
pinctrl registers via _PSx and PowerResource._On/_Off?

> The lack of any statement about pinctrl would mean that there is effectively 
> no
> reasonable limitation on what firmware might do with pinctrl, and we cannot
> assume specific behaviour from the firmware.
>

There is noting specified in the spec about other controllers, why is
pinctrl special in this regard?

> [...]
>
>> Since our focus is for open-ended configurations and for hardware that
>> it is not know to firmware we only considered the case where the pins
>> are not touched after the system boots.
>>
>> Now I wonder what are the cases were the firmware changes the pin
>> configuration after boot.
>
> Device power management and suspend/resume seem like the obvious cases.

I assume you are suggesting something like the following: we have a
device that is powered via a GPIO and associated ACPI _PS3/_PS0
methods are poking the pinctrl register to drive 0 or 1 to power on or
off the device.

If that is the case, we can easily break that today, by changing that
particular GPIO value via, e.g., sysfs.

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

[Graeme Gregory]

On Tue, Apr 05, 2016 at 06:00:31PM -0600, Al Stone wrote:
> On 04/05/2016 02:56 AM, Charles Garcia-Tobin wrote:
> > 
> > 
> > On 04/04/2016 23:52, "Mark Rutland"  wrote:
> > 
> >> Hi,
> >>
> >> 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.
> >>
> >> 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.
> > 
> > It is though latest version was posted by Rafael a bit later:
> > https://lists.acpica.org/pipermail/dsd/2015-December/27.html
> > 
> > 
> > In addition the core rules requiring that existing ACPI paradigms are not
> > subverted through DSD (basically the concern you express) are also
> > documented in the main DSD documentation itself:
> > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UU
> > ID.pdf (section 2.3)
> 
> I have to echo Mark's concern: wholesale importation of portions of current
> DT bindings simply because it's expedient is one of the things I've been 
> hoping
> to avoid.  These patches seem to be just that.
> 
> And while the latest version mentioned above describes a bit of a review
> process to handle this case, I don't recall the kernel community at large
> agreeing to it, nor to it having been implemented.  If I missed that part,
> my apologies; please let me know where it was decided.  If I haven't, then
> perhaps this needs to be the first test case of that process.
> 

My concern over this is similar to Mark/Als this looks like a quick hack
to "solve" an issue that has been worked on in Linux for at least 10
years now. And thats how to describe a non descoverable card that is
plugged into random busses on a SoC.

The issue I see with a quick hack of DT pinctrl into _DSD is that this
does not take into account the power model of ACPI. As far as I can see
the core code has no manner to tell a pin/function allocated through
_DSD actually has effects on the power of other devices. So the core
could end up powering down devices when it should not. This is very
relevent to your intended use of IoT devices where power control is key.

Why has there been no attempt in ASWG to make these sort of features a
1st class citizen of ACPI so they can interact correctly with the other
features?

Thanks

Graeme

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

[Graeme Gregory]

On Tue, Apr 05, 2016 at 06:00:31PM -0600, Al Stone wrote:
> On 04/05/2016 02:56 AM, Charles Garcia-Tobin wrote:
> > 
> > 
> > On 04/04/2016 23:52, "Mark Rutland"  wrote:
> > 
> >> Hi,
> >>
> >> 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.
> >>
> >> 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.
> > 
> > It is though latest version was posted by Rafael a bit later:
> > https://lists.acpica.org/pipermail/dsd/2015-December/27.html
> > 
> > 
> > In addition the core rules requiring that existing ACPI paradigms are not
> > subverted through DSD (basically the concern you express) are also
> > documented in the main DSD documentation itself:
> > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UU
> > ID.pdf (section 2.3)
> 
> I have to echo Mark's concern: wholesale importation of portions of current
> DT bindings simply because it's expedient is one of the things I've been 
> hoping
> to avoid.  These patches seem to be just that.
> 
> And while the latest version mentioned above describes a bit of a review
> process to handle this case, I don't recall the kernel community at large
> agreeing to it, nor to it having been implemented.  If I missed that part,
> my apologies; please let me know where it was decided.  If I haven't, then
> perhaps this needs to be the first test case of that process.
> 

My concern over this is similar to Mark/Als this looks like a quick hack
to "solve" an issue that has been worked on in Linux for at least 10
years now. And thats how to describe a non descoverable card that is
plugged into random busses on a SoC.

The issue I see with a quick hack of DT pinctrl into _DSD is that this
does not take into account the power model of ACPI. As far as I can see
the core code has no manner to tell a pin/function allocated through
_DSD actually has effects on the power of other devices. So the core
could end up powering down devices when it should not. This is very
relevent to your intended use of IoT devices where power control is key.

Why has there been no attempt in ASWG to make these sort of features a
1st class citizen of ACPI so they can interact correctly with the other
features?

Thanks

Graeme

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

[Lorenzo Pieralisi]

On Tue, Apr 05, 2016 at 10:37:14PM +0300, Octavian Purdila wrote:
> On Tue, Apr 5, 2016 at 7:59 PM, Mark Brown  wrote:

[...]

> Lets look at this from a different perspective. The proposal is not
> about importing the DT model into ACPI but importing the Linux pinctrl
> model into ACPI. That will allow us to use the Linux pinctrl drivers
> to their full potential.

Yes we understood that, and in the process you are bypassing the eg ACPI
power management model completely, but since all you are after is using
the Linux pinctrl kernel driver with ACPI _today_ without going through
the ASWG (and without booting with a device tree instead of ACPI) and
define a specification that has a chance to co-exist with the ACPI power
management model this proposal is the end result, it is a shortcut fraught
with problems.

> That doesn't stop the development of other, more OS independent, ACPI
> models for pinmuxing. Which we can also support.
> 
> I know that there are some discussions for pinmux configuration in the
> ASWG, but it does not match the Linux pinctrl model. So we will end up
> with a pinctrl driver that offers groups, functions and pin names and
> a totally different ACPI description that we can't map to the pinctrl
> driver.

If you know that there are some discussions please take place in those
discussions and work towards a solution that takes into account
other parts of ACPI specifications that can be affected, it may
take longer to get you there but that's true for everyone who
wants to contribute to ACPI specifications I am afraid.

Thank you,
Lorenzo

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

[Lorenzo Pieralisi]

On Tue, Apr 05, 2016 at 10:37:14PM +0300, Octavian Purdila wrote:
> On Tue, Apr 5, 2016 at 7:59 PM, Mark Brown  wrote:

[...]

> Lets look at this from a different perspective. The proposal is not
> about importing the DT model into ACPI but importing the Linux pinctrl
> model into ACPI. That will allow us to use the Linux pinctrl drivers
> to their full potential.

Yes we understood that, and in the process you are bypassing the eg ACPI
power management model completely, but since all you are after is using
the Linux pinctrl kernel driver with ACPI _today_ without going through
the ASWG (and without booting with a device tree instead of ACPI) and
define a specification that has a chance to co-exist with the ACPI power
management model this proposal is the end result, it is a shortcut fraught
with problems.

> That doesn't stop the development of other, more OS independent, ACPI
> models for pinmuxing. Which we can also support.
> 
> I know that there are some discussions for pinmux configuration in the
> ASWG, but it does not match the Linux pinctrl model. So we will end up
> with a pinctrl driver that offers groups, functions and pin names and
> a totally different ACPI description that we can't map to the pinctrl
> driver.

If you know that there are some discussions please take place in those
discussions and work towards a solution that takes into account
other parts of ACPI specifications that can be affected, it may
take longer to get you there but that's true for everyone who
wants to contribute to ACPI specifications I am afraid.

Thank you,
Lorenzo

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

[Mark Rutland]

On Tue, Apr 05, 2016 at 11:09:34PM +0300, Octavian Purdila wrote:
> On Tue, Apr 5, 2016 at 9:16 PM, Mark Rutland  wrote:
> > * The firmware is to some extent expected to manage pinctrl today (for power
> >   management of devices it does know about), and hence a pinctrl device is
> >   potentially under shared management of ACPI and the OS.
> >
> > * The ACPI specification says nothing about this style of pinctrl 
> > management,
> >   so it is unclear what the expectations are:
> 
> Does it say anything at all about pinctrl management?

To the best of my knowledge the ACPI spec does not explicitly mention pinctrl.
In another reply it was mentioned that there is work in progress for pinmuxing,
so evidently it is on the ASWG radar and within the scope of ACPI.

I was trying to point out that it is _implicitly_ firmware's responsibility to
do any pinctrl today, due to the ACPI model for power management, and the lack
of an existing ACPI mechanisms to provide pinctrl data.

In practice, firmware configures pinctrl at boot, and may modify pinctrl as
part of the runtime power management firmware is put in charge of.

The lack of any statement about pinctrl would mean that there is effectively no
reasonable limitation on what firmware might do with pinctrl, and we cannot
assume specific behaviour from the firmware.

[...]

> Since our focus is for open-ended configurations and for hardware that
> it is not know to firmware we only considered the case where the pins
> are not touched after the system boots.
> 
> Now I wonder what are the cases were the firmware changes the pin
> configuration after boot.

Device power management and suspend/resume seem like the obvious cases.

Thanks,
Mark

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

[Mark Rutland]

On Tue, Apr 05, 2016 at 11:09:34PM +0300, Octavian Purdila wrote:
> On Tue, Apr 5, 2016 at 9:16 PM, Mark Rutland  wrote:
> > * The firmware is to some extent expected to manage pinctrl today (for power
> >   management of devices it does know about), and hence a pinctrl device is
> >   potentially under shared management of ACPI and the OS.
> >
> > * The ACPI specification says nothing about this style of pinctrl 
> > management,
> >   so it is unclear what the expectations are:
> 
> Does it say anything at all about pinctrl management?

To the best of my knowledge the ACPI spec does not explicitly mention pinctrl.
In another reply it was mentioned that there is work in progress for pinmuxing,
so evidently it is on the ASWG radar and within the scope of ACPI.

I was trying to point out that it is _implicitly_ firmware's responsibility to
do any pinctrl today, due to the ACPI model for power management, and the lack
of an existing ACPI mechanisms to provide pinctrl data.

In practice, firmware configures pinctrl at boot, and may modify pinctrl as
part of the runtime power management firmware is put in charge of.

The lack of any statement about pinctrl would mean that there is effectively no
reasonable limitation on what firmware might do with pinctrl, and we cannot
assume specific behaviour from the firmware.

[...]

> Since our focus is for open-ended configurations and for hardware that
> it is not know to firmware we only considered the case where the pins
> are not touched after the system boots.
> 
> Now I wonder what are the cases were the firmware changes the pin
> configuration after boot.

Device power management and suspend/resume seem like the obvious cases.

Thanks,
Mark

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

[Al Stone]

On 04/05/2016 02:56 AM, Charles Garcia-Tobin wrote:
> 
> 
> On 04/04/2016 23:52, "Mark Rutland"  wrote:
> 
>> Hi,
>>
>> 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.
>>
>> 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.
> 
> It is though latest version was posted by Rafael a bit later:
> https://lists.acpica.org/pipermail/dsd/2015-December/27.html
> 
> 
> In addition the core rules requiring that existing ACPI paradigms are not
> subverted through DSD (basically the concern you express) are also
> documented in the main DSD documentation itself:
> http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UU
> ID.pdf (section 2.3)

I have to echo Mark's concern: wholesale importation of portions of current
DT bindings simply because it's expedient is one of the things I've been hoping
to avoid.  These patches seem to be just that.

And while the latest version mentioned above describes a bit of a review
process to handle this case, I don't recall the kernel community at large
agreeing to it, nor to it having been implemented.  If I missed that part,
my apologies; please let me know where it was decided.  If I haven't, then
perhaps this needs to be the first test case of that process.

-- 
ciao,
al
---
Al Stone
Software Engineer
Red Hat, Inc.
a...@redhat.com
---

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

[Al Stone]

On 04/05/2016 02:56 AM, Charles Garcia-Tobin wrote:
> 
> 
> On 04/04/2016 23:52, "Mark Rutland"  wrote:
> 
>> Hi,
>>
>> 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.
>>
>> 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.
> 
> It is though latest version was posted by Rafael a bit later:
> https://lists.acpica.org/pipermail/dsd/2015-December/27.html
> 
> 
> In addition the core rules requiring that existing ACPI paradigms are not
> subverted through DSD (basically the concern you express) are also
> documented in the main DSD documentation itself:
> http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UU
> ID.pdf (section 2.3)

I have to echo Mark's concern: wholesale importation of portions of current
DT bindings simply because it's expedient is one of the things I've been hoping
to avoid.  These patches seem to be just that.

And while the latest version mentioned above describes a bit of a review
process to handle this case, I don't recall the kernel community at large
agreeing to it, nor to it having been implemented.  If I missed that part,
my apologies; please let me know where it was decided.  If I haven't, then
perhaps this needs to be the first test case of that process.

-- 
ciao,
al
---
Al Stone
Software Engineer
Red Hat, Inc.
a...@redhat.com
---

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

[Al Stone]

On 04/05/2016 01:37 PM, Octavian Purdila wrote:
> On Tue, Apr 5, 2016 at 7:59 PM, Mark Brown  wrote:
>> On Tue, Apr 05, 2016 at 10:43:11AM +0200, Linus Walleij wrote:
>>[snip...]
> 
> I know that there are some discussions for pinmux configuration in the
> ASWG, but it does not match the Linux pinctrl model. So we will end up
> with a pinctrl driver that offers groups, functions and pin names and
> a totally different ACPI description that we can't map to the pinctrl
> driver.

Um, as a member of the ASWG, this is the first I've heard of this discussion.
And I do attend the meetings very regularly.  Please send me an issue number
off-line because I'm not finding it in the current issue list.

Secondly, if this is being discussed within the ASWG, please refer to the UEFI
member agreements about disclosing information prior to it being approved by
the forum; there are limits to such disclosures, and a public mailing list may
be well past those limits.

-- 
ciao,
al
---
Al Stone
Software Engineer
Red Hat, Inc.
a...@redhat.com
---

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

[Al Stone]

On 04/05/2016 01:37 PM, Octavian Purdila wrote:
> On Tue, Apr 5, 2016 at 7:59 PM, Mark Brown  wrote:
>> On Tue, Apr 05, 2016 at 10:43:11AM +0200, Linus Walleij wrote:
>>[snip...]
> 
> I know that there are some discussions for pinmux configuration in the
> ASWG, but it does not match the Linux pinctrl model. So we will end up
> with a pinctrl driver that offers groups, functions and pin names and
> a totally different ACPI description that we can't map to the pinctrl
> driver.

Um, as a member of the ASWG, this is the first I've heard of this discussion.
And I do attend the meetings very regularly.  Please send me an issue number
off-line because I'm not finding it in the current issue list.

Secondly, if this is being discussed within the ASWG, please refer to the UEFI
member agreements about disclosing information prior to it being approved by
the forum; there are limits to such disclosures, and a public mailing list may
be well past those limits.

-- 
ciao,
al
---
Al Stone
Software Engineer
Red Hat, Inc.
a...@redhat.com
---

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

[Mark Brown]

On Tue, Apr 05, 2016 at 10:37:14PM +0300, Octavian Purdila wrote:
> On Tue, Apr 5, 2016 at 7:59 PM, Mark Brown  wrote:
> > On Tue, Apr 05, 2016 at 10:43:11AM +0200, Linus Walleij wrote:

> >> And these products are only update-able
> >> with hairy BIOS patches that need to be applied
> >> using $SPECIAL_TOOL  that "normal users" do not want to
> >> concern themselves with, as this is not an "apt-get upgrade"
> >> kind of thing.

> > This bit has been addressed in UEFI - there's now a mechanism for the OS
> > to supply firmware updates to the BIOS via runtime sevices calls without
> > needing to go to the hairy tools.

> AFAIK there is still no standard way of updating just the ACPI tables.
> You need various tools to "stitch" a full firmware image that can be
> consumed by the update mechanism.

Right, but in terms of end users that doesn't matter - the users that
Linus is concerned with here aren't going to know what ACPI is.

> > There's also been facilities for some
> > time which allow ACPI fragments to be loaded at runtime without patching
> > the firmware (though they're not used so often at present).

> And I just sent a separate patch series for "ACPI overlays", similar
> with the dynamic DT, with the primary goal of supporting open-ended
> hardware configurations.

> But to be able to effectively use this we need a way to change pinmux 
> settings.

I'll need to find that series I guess...

Note that we currently don't have any actual mechanism to load a DT
overlay in mainline.  One of the issues there is making sure that the
external interfaces we're offering are actually stable, another is
making sure (for safety) that the overlays are only updating bits of the
device tree that corrspond to the bit of the hardware that the overlay
is for.

> > Given the rush to shoehorn existing DT into ACPI at some point it's
> > looking like it would be much more sensible for x86 to just do what
> > arm64 has done and support both DT and ACPI in parallel and let people
> > (either system integrators or end users) choose the most appropriate
> > interface for their application.

> Lets look at this from a different perspective. The proposal is not
> about importing the DT model into ACPI but importing the Linux pinctrl

It's not like this is the only change that has been sent to reuse some
DT element rather directly in ACPI recently...

> model into ACPI. That will allow us to use the Linux pinctrl drivers
> to their full potential.

> That doesn't stop the development of other, more OS independent, ACPI
> models for pinmuxing. Which we can also support.

That's not really a great answer.  Regardless of how good a solution
this is for handling pinmuxing ACPI it needs to be joined up with the
rest of ACPI, we need some clear instructions for both firmware and OS
authors about how this works in a wider ACPI context and interacts with
other features, preferably in the ACPI specs where people can find them.

It could be something really simple like just saying that if we've got
these pinctrl definitions then the firmware is not allowed to do any pin
management, or it could be something more involved like saying that the
OS has to notify the OS before using them.

> I know that there are some discussions for pinmux configuration in the
> ASWG, but it does not match the Linux pinctrl model. So we will end up
> with a pinctrl driver that offers groups, functions and pin names and
> a totally different ACPI description that we can't map to the pinctrl
> driver.

If there's an ASWG spec in progress that explicitly overlaps it seems
even more important that this is joined up - I'm not aware of what's
going on there.


signature.asc
Description: PGP signature

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

[Mark Brown]

On Tue, Apr 05, 2016 at 10:37:14PM +0300, Octavian Purdila wrote:
> On Tue, Apr 5, 2016 at 7:59 PM, Mark Brown  wrote:
> > On Tue, Apr 05, 2016 at 10:43:11AM +0200, Linus Walleij wrote:

> >> And these products are only update-able
> >> with hairy BIOS patches that need to be applied
> >> using $SPECIAL_TOOL  that "normal users" do not want to
> >> concern themselves with, as this is not an "apt-get upgrade"
> >> kind of thing.

> > This bit has been addressed in UEFI - there's now a mechanism for the OS
> > to supply firmware updates to the BIOS via runtime sevices calls without
> > needing to go to the hairy tools.

> AFAIK there is still no standard way of updating just the ACPI tables.
> You need various tools to "stitch" a full firmware image that can be
> consumed by the update mechanism.

Right, but in terms of end users that doesn't matter - the users that
Linus is concerned with here aren't going to know what ACPI is.

> > There's also been facilities for some
> > time which allow ACPI fragments to be loaded at runtime without patching
> > the firmware (though they're not used so often at present).

> And I just sent a separate patch series for "ACPI overlays", similar
> with the dynamic DT, with the primary goal of supporting open-ended
> hardware configurations.

> But to be able to effectively use this we need a way to change pinmux 
> settings.

I'll need to find that series I guess...

Note that we currently don't have any actual mechanism to load a DT
overlay in mainline.  One of the issues there is making sure that the
external interfaces we're offering are actually stable, another is
making sure (for safety) that the overlays are only updating bits of the
device tree that corrspond to the bit of the hardware that the overlay
is for.

> > Given the rush to shoehorn existing DT into ACPI at some point it's
> > looking like it would be much more sensible for x86 to just do what
> > arm64 has done and support both DT and ACPI in parallel and let people
> > (either system integrators or end users) choose the most appropriate
> > interface for their application.

> Lets look at this from a different perspective. The proposal is not
> about importing the DT model into ACPI but importing the Linux pinctrl

It's not like this is the only change that has been sent to reuse some
DT element rather directly in ACPI recently...

> model into ACPI. That will allow us to use the Linux pinctrl drivers
> to their full potential.

> That doesn't stop the development of other, more OS independent, ACPI
> models for pinmuxing. Which we can also support.

That's not really a great answer.  Regardless of how good a solution
this is for handling pinmuxing ACPI it needs to be joined up with the
rest of ACPI, we need some clear instructions for both firmware and OS
authors about how this works in a wider ACPI context and interacts with
other features, preferably in the ACPI specs where people can find them.

It could be something really simple like just saying that if we've got
these pinctrl definitions then the firmware is not allowed to do any pin
management, or it could be something more involved like saying that the
OS has to notify the OS before using them.

> I know that there are some discussions for pinmux configuration in the
> ASWG, but it does not match the Linux pinctrl model. So we will end up
> with a pinctrl driver that offers groups, functions and pin names and
> a totally different ACPI description that we can't map to the pinctrl
> driver.

If there's an ASWG spec in progress that explicitly overlaps it seems
even more important that this is joined up - I'm not aware of what's
going on there.


signature.asc
Description: PGP signature

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

[Octavian Purdila]

On Tue, Apr 5, 2016 at 9:16 PM, Mark Rutland  wrote:

>>
>> 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.
>
> In the absence of a BIOS update, how is it expected that the relevant pinctrl
> settings for a given device will be known? Does the device provide ACPI
> fragments like an SSDT? Does it simply identify itself in some manner, and
> leave the rest to the kernel? Is this entirely user-driven?
>

See this patch set I just proposed: https://lkml.org/lkml/2016/3/31/334

>> 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.
>
> To be clear, I'm not stating that having pinctrl under the OS is necessarily
> wrong, and I can see why the firmware may not have all the relevant knowledge
> in advance. I can certainly see why having the OS in control can be 
> preferable.
>
> My concern is that there is a conflict with the ACPI model, and potential
> fragility, given that:
>
> * The firmware does not have the relevant information in advance for a given
>   device that may be connected (i.e. how devices may change the pinctrl setup
>   is unknown).
>
> * The firmware is to some extent expected to manage pinctrl today (for power
>   management of devices it does know about), and hence a pinctrl device is
>   potentially under shared management of ACPI and the OS.
>
> * The ACPI specification says nothing about this style of pinctrl management,
>   so it is unclear what the expectations are:
>

Does it say anything at all about pinctrl management?

>   - Is a given pinctrl device under shared ownership by the firmware and
> kernel, or is a given device entirely under the control of just one?
>
>   - How shared access to the pinctrl device is mediated, e.g. is any locking 
> or
> signalling mechanism required to ensure that firmware and kernel do not
> access the device simultaneously in a manner that causes problems.
>
>   - Is the firmware permitted to perform power management of devices for which
> the kernel handles pinctrl? What states can either expect, and when is 
> such
> management permitted? e.g. must the OS ensure that a device is in its
> default state? Can it only call power management calls from particular
> states? What is the restored state?
>
>   - What the expectations are w.r.t. ownership of pins, e.g. must the firmware
> never change the state of certain pins? Must it save/restore their state 
> in
> system-wide power-management scenarios like suspend or hibernate?
>
> I think this needs to be raised with the ASWG, and some level of model needs 
> to
> be defined for this, to cater for the issues raised above.
>
> That might be very simple, e.g. pins are never shared, the pinctrl management
> device must permit concurrent accesses by FW and kernel, the kernel is
> responsible for all management of those pins after system reset.
>
> In the absence of that, this is liable to become fragmented and fragile, and 
> is
> practically impossible to rectify post-hoc.
>

All very good points.

Since our focus is for open-ended configurations and for hardware that
it is not know to firmware we only considered the case where the pins
are not touched after the system boots.

Now I wonder what are the cases were the firmware changes the pin
configuration after boot.

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

[Octavian Purdila]

On Tue, Apr 5, 2016 at 9:16 PM, Mark Rutland  wrote:

>>
>> 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.
>
> In the absence of a BIOS update, how is it expected that the relevant pinctrl
> settings for a given device will be known? Does the device provide ACPI
> fragments like an SSDT? Does it simply identify itself in some manner, and
> leave the rest to the kernel? Is this entirely user-driven?
>

See this patch set I just proposed: https://lkml.org/lkml/2016/3/31/334

>> 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.
>
> To be clear, I'm not stating that having pinctrl under the OS is necessarily
> wrong, and I can see why the firmware may not have all the relevant knowledge
> in advance. I can certainly see why having the OS in control can be 
> preferable.
>
> My concern is that there is a conflict with the ACPI model, and potential
> fragility, given that:
>
> * The firmware does not have the relevant information in advance for a given
>   device that may be connected (i.e. how devices may change the pinctrl setup
>   is unknown).
>
> * The firmware is to some extent expected to manage pinctrl today (for power
>   management of devices it does know about), and hence a pinctrl device is
>   potentially under shared management of ACPI and the OS.
>
> * The ACPI specification says nothing about this style of pinctrl management,
>   so it is unclear what the expectations are:
>

Does it say anything at all about pinctrl management?

>   - Is a given pinctrl device under shared ownership by the firmware and
> kernel, or is a given device entirely under the control of just one?
>
>   - How shared access to the pinctrl device is mediated, e.g. is any locking 
> or
> signalling mechanism required to ensure that firmware and kernel do not
> access the device simultaneously in a manner that causes problems.
>
>   - Is the firmware permitted to perform power management of devices for which
> the kernel handles pinctrl? What states can either expect, and when is 
> such
> management permitted? e.g. must the OS ensure that a device is in its
> default state? Can it only call power management calls from particular
> states? What is the restored state?
>
>   - What the expectations are w.r.t. ownership of pins, e.g. must the firmware
> never change the state of certain pins? Must it save/restore their state 
> in
> system-wide power-management scenarios like suspend or hibernate?
>
> I think this needs to be raised with the ASWG, and some level of model needs 
> to
> be defined for this, to cater for the issues raised above.
>
> That might be very simple, e.g. pins are never shared, the pinctrl management
> device must permit concurrent accesses by FW and kernel, the kernel is
> responsible for all management of those pins after system reset.
>
> In the absence of that, this is liable to become fragmented and fragile, and 
> is
> practically impossible to rectify post-hoc.
>

All very good points.

Since our focus is for open-ended configurations and for hardware that
it is not know to firmware we only considered the case where the pins
are not touched after the system boots.

Now I wonder what are the cases were the firmware changes the pin
configuration after boot.

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

[Octavian Purdila]

On Tue, Apr 5, 2016 at 7:59 PM, Mark Brown  wrote:
> On Tue, Apr 05, 2016 at 10:43:11AM +0200, Linus Walleij wrote:
>
>> And these products are only update-able
>> with hairy BIOS patches that need to be applied
>> using $SPECIAL_TOOL  that "normal users" do not want to
>> concern themselves with, as this is not an "apt-get upgrade"
>> kind of thing.
>
> This bit has been addressed in UEFI - there's now a mechanism for the OS
> to supply firmware updates to the BIOS via runtime sevices calls without
> needing to go to the hairy tools.

AFAIK there is still no standard way of updating just the ACPI tables.
You need various tools to "stitch" a full firmware image that can be
consumed by the update mechanism.

> There's also been facilities for some
> time which allow ACPI fragments to be loaded at runtime without patching
> the firmware (though they're not used so often at present).

And I just sent a separate patch series for "ACPI overlays", similar
with the dynamic DT, with the primary goal of supporting open-ended
hardware configurations.

But to be able to effectively use this we need a way to change pinmux settings.

>> And I think that is what is happening, it's just that so much
>> prestige is involved that no-one wants to officially admit it.
>
>> I was once poking fun at this development model accusing
>> firmware engineers of having a God complex when they claim
>> to be able to engineer in these complex use cases during
>
> That's not really a fair characterization of the situation.  The goal
> that ACPI has been trying to meet is allowing new hardware to work
> without needing software updates so people's installation experience
> isn't miserable.  The Windows monoculture that firmware developers have
> been targetting has hurt the achievement of that but it's the idea.
> It's a good model for some classes of device but not for all.
>
>> product firmware development. Now I just feel sad about this
>> situation and want things to "just work" for them. I think these
>> patches are good.
>
> See Mark Rutland's reply - there's a whole model behind how ACPI
> abstracts the system that needs to be taken into account, just stuffing
> the existing DT in through a mechanism intended for simple vendor
> specific key/value properties isn't guaranteed to give something that's
> coherent and sensible.  DT design decisions will obviously not have
> considered ACPI and exposing the full combination of the two system
> models to system integrators seems likely to lead to unfortunate
> corners, especially with things that happen to work right now but cause
> problems later on.
>
> Given the rush to shoehorn existing DT into ACPI at some point it's
> looking like it would be much more sensible for x86 to just do what
> arm64 has done and support both DT and ACPI in parallel and let people
> (either system integrators or end users) choose the most appropriate
> interface for their application.

Lets look at this from a different perspective. The proposal is not
about importing the DT model into ACPI but importing the Linux pinctrl
model into ACPI. That will allow us to use the Linux pinctrl drivers
to their full potential.

That doesn't stop the development of other, more OS independent, ACPI
models for pinmuxing. Which we can also support.

I know that there are some discussions for pinmux configuration in the
ASWG, but it does not match the Linux pinctrl model. So we will end up
with a pinctrl driver that offers groups, functions and pin names and
a totally different ACPI description that we can't map to the pinctrl
driver.

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

[Octavian Purdila]

On Tue, Apr 5, 2016 at 7:59 PM, Mark Brown  wrote:
> On Tue, Apr 05, 2016 at 10:43:11AM +0200, Linus Walleij wrote:
>
>> And these products are only update-able
>> with hairy BIOS patches that need to be applied
>> using $SPECIAL_TOOL  that "normal users" do not want to
>> concern themselves with, as this is not an "apt-get upgrade"
>> kind of thing.
>
> This bit has been addressed in UEFI - there's now a mechanism for the OS
> to supply firmware updates to the BIOS via runtime sevices calls without
> needing to go to the hairy tools.

AFAIK there is still no standard way of updating just the ACPI tables.
You need various tools to "stitch" a full firmware image that can be
consumed by the update mechanism.

> There's also been facilities for some
> time which allow ACPI fragments to be loaded at runtime without patching
> the firmware (though they're not used so often at present).

And I just sent a separate patch series for "ACPI overlays", similar
with the dynamic DT, with the primary goal of supporting open-ended
hardware configurations.

But to be able to effectively use this we need a way to change pinmux settings.

>> And I think that is what is happening, it's just that so much
>> prestige is involved that no-one wants to officially admit it.
>
>> I was once poking fun at this development model accusing
>> firmware engineers of having a God complex when they claim
>> to be able to engineer in these complex use cases during
>
> That's not really a fair characterization of the situation.  The goal
> that ACPI has been trying to meet is allowing new hardware to work
> without needing software updates so people's installation experience
> isn't miserable.  The Windows monoculture that firmware developers have
> been targetting has hurt the achievement of that but it's the idea.
> It's a good model for some classes of device but not for all.
>
>> product firmware development. Now I just feel sad about this
>> situation and want things to "just work" for them. I think these
>> patches are good.
>
> See Mark Rutland's reply - there's a whole model behind how ACPI
> abstracts the system that needs to be taken into account, just stuffing
> the existing DT in through a mechanism intended for simple vendor
> specific key/value properties isn't guaranteed to give something that's
> coherent and sensible.  DT design decisions will obviously not have
> considered ACPI and exposing the full combination of the two system
> models to system integrators seems likely to lead to unfortunate
> corners, especially with things that happen to work right now but cause
> problems later on.
>
> Given the rush to shoehorn existing DT into ACPI at some point it's
> looking like it would be much more sensible for x86 to just do what
> arm64 has done and support both DT and ACPI in parallel and let people
> (either system integrators or end users) choose the most appropriate
> interface for their application.

Lets look at this from a different perspective. The proposal is not
about importing the DT model into ACPI but importing the Linux pinctrl
model into ACPI. That will allow us to use the Linux pinctrl drivers
to their full potential.

That doesn't stop the development of other, more OS independent, ACPI
models for pinmuxing. Which we can also support.

I know that there are some discussions for pinmux configuration in the
ASWG, but it does not match the Linux pinctrl model. So we will end up
with a pinctrl driver that offers groups, functions and pin names and
a totally different ACPI description that we can't map to the pinctrl
driver.

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

[Mark Rutland]

On Tue, Apr 05, 2016 at 03:33:43PM +, Tirdea, Irina wrote:
> > -Original Message-
> > From: Mark Rutland [mailto:mark.rutl...@arm.com]
> > 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.

In the absence of a BIOS update, how is it expected that the relevant pinctrl
settings for a given device will be known? Does the device provide ACPI
fragments like an SSDT? Does it simply identify itself in some manner, and
leave the rest to the kernel? Is this entirely user-driven?

> 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.

To be clear, I'm not stating that having pinctrl under the OS is necessarily
wrong, and I can see why the firmware may not have all the relevant knowledge
in advance. I can certainly see why having the OS in control can be preferable.

My concern is that there is a conflict with the ACPI model, and potential
fragility, given that:

* The firmware does not have the relevant information in advance for a given
  device that may be connected (i.e. how devices may change the pinctrl setup
  is unknown).

* The firmware is to some extent expected to manage pinctrl today (for power
  management of devices it does know about), and hence a pinctrl device is
  potentially under shared management of ACPI and the OS.

* The ACPI specification says nothing about this style of pinctrl management,
  so it is unclear what the expectations are:

  - Is a given pinctrl device under shared ownership by the firmware and
kernel, or is a given device entirely under the control of just one?

  - How shared access to the pinctrl device is mediated, e.g. is any locking or
signalling mechanism required to ensure that firmware and kernel do not
access the device simultaneously in a manner that causes problems.

  - Is the firmware permitted to perform power management of devices for which
the kernel handles pinctrl? What states can either expect, and when is such
management permitted? e.g. must the OS ensure that a device is in its
default state? Can it only call power management calls from particular
states? What is the restored state?

  - What the expectations are w.r.t. ownership of pins, e.g. must the firmware
never change the state of certain pins? Must it save/restore their state in
system-wide power-management scenarios like suspend or hibernate?

I think this needs to be raised with the ASWG, and some level of model needs to
be defined for this, to cater for the issues raised above.

That might be very simple, e.g. pins are never shared, the pinctrl management
device must permit concurrent accesses by FW and kernel, the kernel is
responsible for all management of those pins after system reset.

In the absence of that, this is liable to become fragmented and fragile, and is
practically impossible to rectify post-hoc.

> 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.

I do not disagree that this is expedient. I am concerned that it permits a very
large set of avoidable 

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

[Mark Rutland]

On Tue, Apr 05, 2016 at 03:33:43PM +, Tirdea, Irina wrote:
> > -Original Message-
> > From: Mark Rutland [mailto:mark.rutl...@arm.com]
> > 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.

In the absence of a BIOS update, how is it expected that the relevant pinctrl
settings for a given device will be known? Does the device provide ACPI
fragments like an SSDT? Does it simply identify itself in some manner, and
leave the rest to the kernel? Is this entirely user-driven?

> 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.

To be clear, I'm not stating that having pinctrl under the OS is necessarily
wrong, and I can see why the firmware may not have all the relevant knowledge
in advance. I can certainly see why having the OS in control can be preferable.

My concern is that there is a conflict with the ACPI model, and potential
fragility, given that:

* The firmware does not have the relevant information in advance for a given
  device that may be connected (i.e. how devices may change the pinctrl setup
  is unknown).

* The firmware is to some extent expected to manage pinctrl today (for power
  management of devices it does know about), and hence a pinctrl device is
  potentially under shared management of ACPI and the OS.

* The ACPI specification says nothing about this style of pinctrl management,
  so it is unclear what the expectations are:

  - Is a given pinctrl device under shared ownership by the firmware and
kernel, or is a given device entirely under the control of just one?

  - How shared access to the pinctrl device is mediated, e.g. is any locking or
signalling mechanism required to ensure that firmware and kernel do not
access the device simultaneously in a manner that causes problems.

  - Is the firmware permitted to perform power management of devices for which
the kernel handles pinctrl? What states can either expect, and when is such
management permitted? e.g. must the OS ensure that a device is in its
default state? Can it only call power management calls from particular
states? What is the restored state?

  - What the expectations are w.r.t. ownership of pins, e.g. must the firmware
never change the state of certain pins? Must it save/restore their state in
system-wide power-management scenarios like suspend or hibernate?

I think this needs to be raised with the ASWG, and some level of model needs to
be defined for this, to cater for the issues raised above.

That might be very simple, e.g. pins are never shared, the pinctrl management
device must permit concurrent accesses by FW and kernel, the kernel is
responsible for all management of those pins after system reset.

In the absence of that, this is liable to become fragmented and fragile, and is
practically impossible to rectify post-hoc.

> 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.

I do not disagree that this is expedient. I am concerned that it permits a very
large set of avoidable 

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

[Mark Brown]

On Tue, Apr 05, 2016 at 03:51:18PM +0300, Octavian Purdila wrote:
> On Tue, Apr 5, 2016 at 12:40 AM, Mark Brown  wrote:

> > So this is mainly targeted at modules being added to base boards?

> That is the main use case, yes. Velocity of platform
> debugging/enabling is another one.

The speed thing sounds like someone needs to work on the tooling for
working on firmware TBH.

> >  This means that any binding of a board to an ACPI
> > using system that just uses this is going to be entirely specific to the
> > particular combination of base and expansion board even if the
> > electrical connections are standard.

> This can be solved by tools that work with high level abstractions and
> generate this specific information.

Simply stating that there are in future going to be some higher level
things which make use of this firmware interface isn't altogether
reassuring when we're still in the process of solving the issues for the
DT version of this...

> > This is something that people are currently looking at for DT, there the
> > discussion has been about defining the connectors as entities and hiding
> > the details of the muxing on the SoC behind that along with higher level
> > concepts like instantiation of buses like I2C and SPI.  It seems like if
> > we do want to try to share between DT and ACPI we should be doing it at
> > that level rather than dealing with pinmuxing at the extremely low level
> > that pinctrl does.

> At some point we still need to poke registers. We already have a
> drivers that do this (pinctrl) and a standard configuration interface
> (the pinctrl device tree bindings). It seems natural to build on top
> of this for those higher level concepts / tools.

Both DT and ACPI have a system model behind them and they're not the
stame system model.  Importing one into the other directly without
carefully thinking through how they play nicely with each other seems
likely to lead to problems further down the line, you might know exactly
how you intend this to work but how do we make sure that a firmware
author in some system integrator knows about this and is able to safely
write firmware in a few years?


signature.asc
Description: PGP signature

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

[Mark Brown]

On Tue, Apr 05, 2016 at 03:51:18PM +0300, Octavian Purdila wrote:
> On Tue, Apr 5, 2016 at 12:40 AM, Mark Brown  wrote:

> > So this is mainly targeted at modules being added to base boards?

> That is the main use case, yes. Velocity of platform
> debugging/enabling is another one.

The speed thing sounds like someone needs to work on the tooling for
working on firmware TBH.

> >  This means that any binding of a board to an ACPI
> > using system that just uses this is going to be entirely specific to the
> > particular combination of base and expansion board even if the
> > electrical connections are standard.

> This can be solved by tools that work with high level abstractions and
> generate this specific information.

Simply stating that there are in future going to be some higher level
things which make use of this firmware interface isn't altogether
reassuring when we're still in the process of solving the issues for the
DT version of this...

> > This is something that people are currently looking at for DT, there the
> > discussion has been about defining the connectors as entities and hiding
> > the details of the muxing on the SoC behind that along with higher level
> > concepts like instantiation of buses like I2C and SPI.  It seems like if
> > we do want to try to share between DT and ACPI we should be doing it at
> > that level rather than dealing with pinmuxing at the extremely low level
> > that pinctrl does.

> At some point we still need to poke registers. We already have a
> drivers that do this (pinctrl) and a standard configuration interface
> (the pinctrl device tree bindings). It seems natural to build on top
> of this for those higher level concepts / tools.

Both DT and ACPI have a system model behind them and they're not the
stame system model.  Importing one into the other directly without
carefully thinking through how they play nicely with each other seems
likely to lead to problems further down the line, you might know exactly
how you intend this to work but how do we make sure that a firmware
author in some system integrator knows about this and is able to safely
write firmware in a few years?


signature.asc
Description: PGP signature

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

[Mark Brown]

On Tue, Apr 05, 2016 at 10:43:11AM +0200, Linus Walleij wrote:

> And these products are only update-able
> with hairy BIOS patches that need to be applied
> using $SPECIAL_TOOL  that "normal users" do not want to
> concern themselves with, as this is not an "apt-get upgrade"
> kind of thing.

This bit has been addressed in UEFI - there's now a mechanism for the OS
to supply firmware updates to the BIOS via runtime sevices calls without
needing to go to the hairy tools.  There's also been facilities for some
time which allow ACPI fragments to be loaded at runtime without patching
the firmware (though they're not used so often at present).

> And I think that is what is happening, it's just that so much
> prestige is involved that no-one wants to officially admit it.

> I was once poking fun at this development model accusing
> firmware engineers of having a God complex when they claim
> to be able to engineer in these complex use cases during

That's not really a fair characterization of the situation.  The goal
that ACPI has been trying to meet is allowing new hardware to work
without needing software updates so people's installation experience
isn't miserable.  The Windows monoculture that firmware developers have
been targetting has hurt the achievement of that but it's the idea.
It's a good model for some classes of device but not for all.

> product firmware development. Now I just feel sad about this
> situation and want things to "just work" for them. I think these
> patches are good.

See Mark Rutland's reply - there's a whole model behind how ACPI
abstracts the system that needs to be taken into account, just stuffing
the existing DT in through a mechanism intended for simple vendor
specific key/value properties isn't guaranteed to give something that's
coherent and sensible.  DT design decisions will obviously not have
considered ACPI and exposing the full combination of the two system
models to system integrators seems likely to lead to unfortunate
corners, especially with things that happen to work right now but cause
problems later on.

Given the rush to shoehorn existing DT into ACPI at some point it's
looking like it would be much more sensible for x86 to just do what
arm64 has done and support both DT and ACPI in parallel and let people
(either system integrators or end users) choose the most appropriate
interface for their application.


signature.asc
Description: PGP signature

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

[Mark Brown]

On Tue, Apr 05, 2016 at 10:43:11AM +0200, Linus Walleij wrote:

> And these products are only update-able
> with hairy BIOS patches that need to be applied
> using $SPECIAL_TOOL  that "normal users" do not want to
> concern themselves with, as this is not an "apt-get upgrade"
> kind of thing.

This bit has been addressed in UEFI - there's now a mechanism for the OS
to supply firmware updates to the BIOS via runtime sevices calls without
needing to go to the hairy tools.  There's also been facilities for some
time which allow ACPI fragments to be loaded at runtime without patching
the firmware (though they're not used so often at present).

> And I think that is what is happening, it's just that so much
> prestige is involved that no-one wants to officially admit it.

> I was once poking fun at this development model accusing
> firmware engineers of having a God complex when they claim
> to be able to engineer in these complex use cases during

That's not really a fair characterization of the situation.  The goal
that ACPI has been trying to meet is allowing new hardware to work
without needing software updates so people's installation experience
isn't miserable.  The Windows monoculture that firmware developers have
been targetting has hurt the achievement of that but it's the idea.
It's a good model for some classes of device but not for all.

> product firmware development. Now I just feel sad about this
> situation and want things to "just work" for them. I think these
> patches are good.

See Mark Rutland's reply - there's a whole model behind how ACPI
abstracts the system that needs to be taken into account, just stuffing
the existing DT in through a mechanism intended for simple vendor
specific key/value properties isn't guaranteed to give something that's
coherent and sensible.  DT design decisions will obviously not have
considered ACPI and exposing the full combination of the two system
models to system integrators seems likely to lead to unfortunate
corners, especially with things that happen to work right now but cause
problems later on.

Given the rush to shoehorn existing DT into ACPI at some point it's
looking like it would be much more sensible for x86 to just do what
arm64 has done and support both DT and ACPI in parallel and let people
(either system integrators or end users) choose the most appropriate
interface for their application.


signature.asc
Description: PGP signature

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

[Mark Brown]

On Tue, Apr 05, 2016 at 11:00:50AM +0200, Linus Walleij wrote:
> On Mon, Apr 4, 2016 at 11:40 PM, Mark Brown  wrote:

> > So this is mainly targeted at modules being added to base boards?
> > Without getting into the binding at all here it seems like this is not
> > solving the problem at the right abstraction level.  It's exposing the

> I have seen the same need beyond strictly embedded (MinnowBoard)
> from the Intel camp.

> These chips with funny Atom-specific codenames (baytrail, cherryview,
> broxton, sunrisepoint etc) are not just used for these IoT use cases
> but also for e.g. laptops of the ChromeBook form factor, and the same
> pin control needs arise there, just at a different cadence related to
> product cycle.

Right, the chips are used in a broader space but in cases where they're
being used in fixed systems where we don't have to deal with repaceable
modules then the idiomatic thing for ACPI is to hide all the pinmuxing
from the operating system.

> I bet they also have funny product-specific kernel trees :(

Yup, they do.

> Agree: work is needed here. It is a big confusion, the whole model is
> based around the configuration being pretty static as I recently
> realized when just wanting to add a runtime-detected LCD panel
> to a certain driver. No runtime patching of the DT or overlays or any
> of the sort deliver what is really needed.

Yes, funnily enough the CHIP people were just talking about that
specific case at ELC yesterday (they patched u-boot to parse overlays
so the kernel never sees the hotplug).

> The only thing I heard which was actually doing something sensible
> was when Matthew Garret once told that apple mice provide a
> device tree fragment to the OS on how to handle it during bus
> discovery.

That's what people are doing with the BeagleBone/RPi/CHIP/96boards case
- it's one of the primary usecases for DT overlays but currently
everyone's final integration layer is out of tree.

> > Obviously for the more general ACPI use case the idiomatic way of
> > handling this is that the OS should never see anything about the
> > pin muxing.  With DT we need to really know what's going on with the
> > pinbox because the model is that even for things built into a single
> > board the OS is responsible for managing the pins but that's really not
> > how ACPI is expected to work.

> See my previous mail for some kind of answer to this.

Yup, will reply there.


signature.asc
Description: PGP signature

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

[Mark Brown]

On Tue, Apr 05, 2016 at 11:00:50AM +0200, Linus Walleij wrote:
> On Mon, Apr 4, 2016 at 11:40 PM, Mark Brown  wrote:

> > So this is mainly targeted at modules being added to base boards?
> > Without getting into the binding at all here it seems like this is not
> > solving the problem at the right abstraction level.  It's exposing the

> I have seen the same need beyond strictly embedded (MinnowBoard)
> from the Intel camp.

> These chips with funny Atom-specific codenames (baytrail, cherryview,
> broxton, sunrisepoint etc) are not just used for these IoT use cases
> but also for e.g. laptops of the ChromeBook form factor, and the same
> pin control needs arise there, just at a different cadence related to
> product cycle.

Right, the chips are used in a broader space but in cases where they're
being used in fixed systems where we don't have to deal with repaceable
modules then the idiomatic thing for ACPI is to hide all the pinmuxing
from the operating system.

> I bet they also have funny product-specific kernel trees :(

Yup, they do.

> Agree: work is needed here. It is a big confusion, the whole model is
> based around the configuration being pretty static as I recently
> realized when just wanting to add a runtime-detected LCD panel
> to a certain driver. No runtime patching of the DT or overlays or any
> of the sort deliver what is really needed.

Yes, funnily enough the CHIP people were just talking about that
specific case at ELC yesterday (they patched u-boot to parse overlays
so the kernel never sees the hotplug).

> The only thing I heard which was actually doing something sensible
> was when Matthew Garret once told that apple mice provide a
> device tree fragment to the OS on how to handle it during bus
> discovery.

That's what people are doing with the BeagleBone/RPi/CHIP/96boards case
- it's one of the primary usecases for DT overlays but currently
everyone's final integration layer is out of tree.

> > Obviously for the more general ACPI use case the idiomatic way of
> > handling this is that the OS should never see anything about the
> > pin muxing.  With DT we need to really know what's going on with the
> > pinbox because the model is that even for things built into a single
> > board the OS is responsible for managing the pins but that's really not
> > how ACPI is expected to work.

> See my previous mail for some kind of answer to this.

Yup, will reply there.


signature.asc
Description: PGP signature

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

[Tirdea, Irina]

> -Original Message-
> From: Mark Rutland [mailto:mark.rutl...@arm.com]
> Sent: 05 April, 2016 1:52
> To: Tirdea, Irina
> Cc: Rafael J. Wysocki; Len Brown; Mika Westerberg; Linus Walleij; 
> linux-g...@vger.kernel.org; linux-a...@vger.kernel.org; Rob
> Herring; Heikki Krogerus; Andy Shevchenko; Purdila, Octavian; Ciocan, 
> Cristina; devicet...@vger.kernel.org; linux-
> ker...@vger.kernel.org; charles.garcia-to...@arm.com
> 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/19.html

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

[Tirdea, Irina]

> -Original Message-
> From: Mark Rutland [mailto:mark.rutl...@arm.com]
> Sent: 05 April, 2016 1:52
> To: Tirdea, Irina
> Cc: Rafael J. Wysocki; Len Brown; Mika Westerberg; Linus Walleij; 
> linux-g...@vger.kernel.org; linux-a...@vger.kernel.org; Rob
> Herring; Heikki Krogerus; Andy Shevchenko; Purdila, Octavian; Ciocan, 
> Cristina; devicet...@vger.kernel.org; linux-
> ker...@vger.kernel.org; charles.garcia-to...@arm.com
> 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/19.html

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

[Octavian Purdila]

On Tue, Apr 5, 2016 at 12:40 AM, Mark Brown  wrote:
> 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.
>
> So this is mainly targeted at modules being added to base boards?

That is the main use case, yes. Velocity of platform
debugging/enabling is another one.

> Without getting into the binding at all here it seems like this is not
> solving the problem at the right abstraction level.  It's exposing the
> pins on the SoC directly without any tie in with the functionality that
> goes over those pins.

This is not completely true. The pinctrl drivers are exposing
functionality in terms of function groups (e.g., i2c1_grp, spi1_grp,
pwm1_grp, etc.) It is not enough, but it is a step in the right
direction for standard bindings and for tools that can build on top of
this.

>  This means that any binding of a board to an ACPI
> using system that just uses this is going to be entirely specific to the
> particular combination of base and expansion board even if the
> electrical connections are standard.
>

This can be solved by tools that work with high level abstractions and
generate this specific information.

> This is something that people are currently looking at for DT, there the
> discussion has been about defining the connectors as entities and hiding
> the details of the muxing on the SoC behind that along with higher level
> concepts like instantiation of buses like I2C and SPI.  It seems like if
> we do want to try to share between DT and ACPI we should be doing it at
> that level rather than dealing with pinmuxing at the extremely low level
> that pinctrl does.
>

At some point we still need to poke registers. We already have a
drivers that do this (pinctrl) and a standard configuration interface
(the pinctrl device tree bindings). It seems natural to build on top
of this for those higher level concepts / tools.

> Obviously for the more general ACPI use case the idiomatic way of
> handling this is that the OS should never see anything about the
> pin muxing. With DT we need to really know what's going on with the
> pinbox because the model is that even for things built into a single
> board the OS is responsible for managing the pins but that's really not
> how ACPI is expected to work.

Maybe. But we already expose pin control / muxing in
drivers/pinctrl/intel, yes, read-only at this point. Very useful for
debugging. Having write access would make debugging even more useful,
not to mention fast prototyping.

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

[Octavian Purdila]

On Tue, Apr 5, 2016 at 12:40 AM, Mark Brown  wrote:
> 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.
>
> So this is mainly targeted at modules being added to base boards?

That is the main use case, yes. Velocity of platform
debugging/enabling is another one.

> Without getting into the binding at all here it seems like this is not
> solving the problem at the right abstraction level.  It's exposing the
> pins on the SoC directly without any tie in with the functionality that
> goes over those pins.

This is not completely true. The pinctrl drivers are exposing
functionality in terms of function groups (e.g., i2c1_grp, spi1_grp,
pwm1_grp, etc.) It is not enough, but it is a step in the right
direction for standard bindings and for tools that can build on top of
this.

>  This means that any binding of a board to an ACPI
> using system that just uses this is going to be entirely specific to the
> particular combination of base and expansion board even if the
> electrical connections are standard.
>

This can be solved by tools that work with high level abstractions and
generate this specific information.

> This is something that people are currently looking at for DT, there the
> discussion has been about defining the connectors as entities and hiding
> the details of the muxing on the SoC behind that along with higher level
> concepts like instantiation of buses like I2C and SPI.  It seems like if
> we do want to try to share between DT and ACPI we should be doing it at
> that level rather than dealing with pinmuxing at the extremely low level
> that pinctrl does.
>

At some point we still need to poke registers. We already have a
drivers that do this (pinctrl) and a standard configuration interface
(the pinctrl device tree bindings). It seems natural to build on top
of this for those higher level concepts / tools.

> Obviously for the more general ACPI use case the idiomatic way of
> handling this is that the OS should never see anything about the
> pin muxing. With DT we need to really know what's going on with the
> pinbox because the model is that even for things built into a single
> board the OS is responsible for managing the pins but that's really not
> how ACPI is expected to work.

Maybe. But we already expose pin control / muxing in
drivers/pinctrl/intel, yes, read-only at this point. Very useful for
debugging. Having write access would make debugging even more useful,
not to mention fast prototyping.

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

[Charles Garcia-Tobin]

On 04/04/2016 23:52, "Mark Rutland"  wrote:

>Hi,
>
>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.
>
>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.

It is though latest version was posted by Rafael a bit later:
https://lists.acpica.org/pipermail/dsd/2015-December/27.html


In addition the core rules requiring that existing ACPI paradigms are not
subverted through DSD (basically the concern you express) are also
documented in the main DSD documentation itself:
http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UU
ID.pdf (section 2.3)


Cheers

Charles


>
>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.
>
>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.
>
>Thanks,
>Mark.
>
>[1] https://lists.acpica.org/pipermail/dsd/2015-September/19.html
>

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

[Charles Garcia-Tobin]

On 04/04/2016 23:52, "Mark Rutland"  wrote:

>Hi,
>
>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.
>
>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.

It is though latest version was posted by Rafael a bit later:
https://lists.acpica.org/pipermail/dsd/2015-December/27.html


In addition the core rules requiring that existing ACPI paradigms are not
subverted through DSD (basically the concern you express) are also
documented in the main DSD documentation itself:
http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UU
ID.pdf (section 2.3)


Cheers

Charles


>
>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.
>
>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.
>
>Thanks,
>Mark.
>
>[1] https://lists.acpica.org/pipermail/dsd/2015-September/19.html
>

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

[Linus Walleij]

On Mon, Apr 4, 2016 at 11:40 PM, Mark Brown  wrote:
> 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.
>
> So this is mainly targeted at modules being added to base boards?
> Without getting into the binding at all here it seems like this is not
> solving the problem at the right abstraction level.  It's exposing the
> pins on the SoC directly without any tie in with the functionality that
> goes over those pins.  This means that any binding of a board to an ACPI
> using system that just uses this is going to be entirely specific to the
> particular combination of base and expansion board even if the
> electrical connections are standard.

I have seen the same need beyond strictly embedded (MinnowBoard)
from the Intel camp.

These chips with funny Atom-specific codenames (baytrail, cherryview,
broxton, sunrisepoint etc) are not just used for these IoT use cases
but also for e.g. laptops of the ChromeBook form factor, and the same
pin control needs arise there, just at a different cadence related to
product cycle.

I bet they also have funny product-specific kernel trees :(

> This is something that people are currently looking at for DT, there the
> discussion has been about defining the connectors as entities and hiding
> the details of the muxing on the SoC behind that along with higher level
> concepts like instantiation of buses like I2C and SPI.  It seems like if
> we do want to try to share between DT and ACPI we should be doing it at
> that level rather than dealing with pinmuxing at the extremely low level
> that pinctrl does.

Agree: work is needed here. It is a big confusion, the whole model is
based around the configuration being pretty static as I recently
realized when just wanting to add a runtime-detected LCD panel
to a certain driver. No runtime patching of the DT or overlays or any
of the sort deliver what is really needed.

The only thing I heard which was actually doing something sensible
was when Matthew Garret once told that apple mice provide a
device tree fragment to the OS on how to handle it during bus
discovery.

I think that for random complex hotpluggable devices like what
greybus is trying to achieve this is needed too, despite the standard
USB-like device classes in all their glory.

> Obviously for the more general ACPI use case the idiomatic way of
> handling this is that the OS should never see anything about the
> pin muxing.  With DT we need to really know what's going on with the
> pinbox because the model is that even for things built into a single
> board the OS is responsible for managing the pins but that's really not
> how ACPI is expected to work.

See my previous mail for some kind of answer to this.

Yours,
Linus Walleij

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

[Linus Walleij]

On Mon, Apr 4, 2016 at 11:40 PM, Mark Brown  wrote:
> 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.
>
> So this is mainly targeted at modules being added to base boards?
> Without getting into the binding at all here it seems like this is not
> solving the problem at the right abstraction level.  It's exposing the
> pins on the SoC directly without any tie in with the functionality that
> goes over those pins.  This means that any binding of a board to an ACPI
> using system that just uses this is going to be entirely specific to the
> particular combination of base and expansion board even if the
> electrical connections are standard.

I have seen the same need beyond strictly embedded (MinnowBoard)
from the Intel camp.

These chips with funny Atom-specific codenames (baytrail, cherryview,
broxton, sunrisepoint etc) are not just used for these IoT use cases
but also for e.g. laptops of the ChromeBook form factor, and the same
pin control needs arise there, just at a different cadence related to
product cycle.

I bet they also have funny product-specific kernel trees :(

> This is something that people are currently looking at for DT, there the
> discussion has been about defining the connectors as entities and hiding
> the details of the muxing on the SoC behind that along with higher level
> concepts like instantiation of buses like I2C and SPI.  It seems like if
> we do want to try to share between DT and ACPI we should be doing it at
> that level rather than dealing with pinmuxing at the extremely low level
> that pinctrl does.

Agree: work is needed here. It is a big confusion, the whole model is
based around the configuration being pretty static as I recently
realized when just wanting to add a runtime-detected LCD panel
to a certain driver. No runtime patching of the DT or overlays or any
of the sort deliver what is really needed.

The only thing I heard which was actually doing something sensible
was when Matthew Garret once told that apple mice provide a
device tree fragment to the OS on how to handle it during bus
discovery.

I think that for random complex hotpluggable devices like what
greybus is trying to achieve this is needed too, despite the standard
USB-like device classes in all their glory.

> Obviously for the more general ACPI use case the idiomatic way of
> handling this is that the OS should never see anything about the
> pin muxing.  With DT we need to really know what's going on with the
> pinbox because the model is that even for things built into a single
> board the OS is responsible for managing the pins but that's really not
> how ACPI is expected to work.

See my previous mail for some kind of answer to this.

Yours,
Linus Walleij

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

[Linus Walleij]

On Tue, Apr 5, 2016 at 12:52 AM, Mark Rutland  wrote:
> On Thu, Mar 31, 2016 at 02:44:41PM +0300, Irina Tirdea wrote:

>> 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.

There are other use cases but in essence there are two common use
cases:

- Initial set-up of muxing and electrical properties

- Runtime reconfiguration for power saving (especially
  biasing and pull options)

I did see that the ACPI people's ambition has been for all things
power save to be embedded into AML and the like.

However AFAICT
the development model for deployment of ACPI seems to imply that
products are shipped with ACPI tables resident in ROM-like storage,
and at product release several devices are disabled from muxing
while others at the same time are totally lacking power save
enablement.

And these products are only update-able
with hairy BIOS patches that need to be applied
using $SPECIAL_TOOL  that "normal users" do not want to
concern themselves with, as this is not an "apt-get upgrade"
kind of thing.

And as this is server silicon the systems have a bunch of these
"normal users" that are not embedded development experts
and they are very hesitant about doing such things.

Under that scenario it is of course tempting to simply set up the
pins in a known good state and then remove
the responsibility of pin controlling from ACPI firmware altogether
and into the kernel, where "apt-get upgrade" will take care of
post-product launch upgrading of the functionality.

And I think that is what is happening, it's just that so much
prestige is involved that no-one wants to officially admit it.

I was once poking fun at this development model accusing
firmware engineers of having a God complex when they claim
to be able to engineer in these complex use cases during
product firmware development. Now I just feel sad about this
situation and want things to "just work" for them. I think these
patches are good.

Yours,
Linus Walleij

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

[Linus Walleij]

On Tue, Apr 5, 2016 at 12:52 AM, Mark Rutland  wrote:
> On Thu, Mar 31, 2016 at 02:44:41PM +0300, Irina Tirdea wrote:

>> 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.

There are other use cases but in essence there are two common use
cases:

- Initial set-up of muxing and electrical properties

- Runtime reconfiguration for power saving (especially
  biasing and pull options)

I did see that the ACPI people's ambition has been for all things
power save to be embedded into AML and the like.

However AFAICT
the development model for deployment of ACPI seems to imply that
products are shipped with ACPI tables resident in ROM-like storage,
and at product release several devices are disabled from muxing
while others at the same time are totally lacking power save
enablement.

And these products are only update-able
with hairy BIOS patches that need to be applied
using $SPECIAL_TOOL  that "normal users" do not want to
concern themselves with, as this is not an "apt-get upgrade"
kind of thing.

And as this is server silicon the systems have a bunch of these
"normal users" that are not embedded development experts
and they are very hesitant about doing such things.

Under that scenario it is of course tempting to simply set up the
pins in a known good state and then remove
the responsibility of pin controlling from ACPI firmware altogether
and into the kernel, where "apt-get upgrade" will take care of
post-product launch upgrading of the functionality.

And I think that is what is happening, it's just that so much
prestige is involved that no-one wants to officially admit it.

I was once poking fun at this development model accusing
firmware engineers of having a God complex when they claim
to be able to engineer in these complex use cases during
product firmware development. Now I just feel sad about this
situation and want things to "just work" for them. I think these
patches are good.

Yours,
Linus Walleij

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

[Mark Brown]

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.

So this is mainly targeted at modules being added to base boards?
Without getting into the binding at all here it seems like this is not
solving the problem at the right abstraction level.  It's exposing the
pins on the SoC directly without any tie in with the functionality that
goes over those pins.  This means that any binding of a board to an ACPI
using system that just uses this is going to be entirely specific to the
particular combination of base and expansion board even if the
electrical connections are standard.

This is something that people are currently looking at for DT, there the
discussion has been about defining the connectors as entities and hiding
the details of the muxing on the SoC behind that along with higher level
concepts like instantiation of buses like I2C and SPI.  It seems like if
we do want to try to share between DT and ACPI we should be doing it at
that level rather than dealing with pinmuxing at the extremely low level
that pinctrl does.

Obviously for the more general ACPI use case the idiomatic way of
handling this is that the OS should never see anything about the
pin muxing.  With DT we need to really know what's going on with the
pinbox because the model is that even for things built into a single
board the OS is responsible for managing the pins but that's really not
how ACPI is expected to work.


signature.asc
Description: PGP signature

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

[Mark Brown]

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.

So this is mainly targeted at modules being added to base boards?
Without getting into the binding at all here it seems like this is not
solving the problem at the right abstraction level.  It's exposing the
pins on the SoC directly without any tie in with the functionality that
goes over those pins.  This means that any binding of a board to an ACPI
using system that just uses this is going to be entirely specific to the
particular combination of base and expansion board even if the
electrical connections are standard.

This is something that people are currently looking at for DT, there the
discussion has been about defining the connectors as entities and hiding
the details of the muxing on the SoC behind that along with higher level
concepts like instantiation of buses like I2C and SPI.  It seems like if
we do want to try to share between DT and ACPI we should be doing it at
that level rather than dealing with pinmuxing at the extremely low level
that pinctrl does.

Obviously for the more general ACPI use case the idiomatic way of
handling this is that the OS should never see anything about the
pin muxing.  With DT we need to really know what's going on with the
pinbox because the model is that even for things built into a single
board the OS is responsible for managing the pins but that's really not
how ACPI is expected to work.


signature.asc
Description: PGP signature

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

[Mark Rutland]

Hi,

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.

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.

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.

Thanks,
Mark.

[1] https://lists.acpica.org/pipermail/dsd/2015-September/19.html

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

[Mark Rutland]

Hi,

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.

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.

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.

Thanks,
Mark.

[1] https://lists.acpica.org/pipermail/dsd/2015-September/19.html

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

[Irina Tirdea]

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.

Irina Tirdea (4):
  pinctrl: Rename pinctrl_utils_dt_free_map to pinctrl_utils_free_map
  pinctrl: pinconf-generic: Add ACPI support
  pinctrl: Add ACPI support
  pinctrl: Parse GpioInt/GpioIo resources

 Documentation/acpi/pinctrl-properties.txt| 282 
 drivers/acpi/property.c  |   8 +-
 drivers/base/property.c  |  36 +-
 drivers/pinctrl/Makefile |   1 +
 drivers/pinctrl/acpi.c   | 551 +++
 drivers/pinctrl/acpi.h   |  32 ++
 drivers/pinctrl/bcm/pinctrl-bcm281xx.c   |   2 +-
 drivers/pinctrl/bcm/pinctrl-cygnus-mux.c |   2 +-
 drivers/pinctrl/bcm/pinctrl-iproc-gpio.c |   2 +-
 drivers/pinctrl/bcm/pinctrl-nsp-gpio.c   |   2 +-
 drivers/pinctrl/berlin/berlin.c  |   2 +-
 drivers/pinctrl/core.c   |  26 ++
 drivers/pinctrl/core.h   |   2 +
 drivers/pinctrl/mediatek/pinctrl-mtk-common.c|   4 +-
 drivers/pinctrl/meson/pinctrl-meson.c|   2 +-
 drivers/pinctrl/nomadik/pinctrl-abx500.c |   4 +-
 drivers/pinctrl/nomadik/pinctrl-nomadik.c|   4 +-
 drivers/pinctrl/pinconf-generic.c| 123 +++--
 drivers/pinctrl/pinctrl-amd.c|   2 +-
 drivers/pinctrl/pinctrl-as3722.c |   2 +-
 drivers/pinctrl/pinctrl-at91-pio4.c  |   4 +-
 drivers/pinctrl/pinctrl-digicolor.c  |   2 +-
 drivers/pinctrl/pinctrl-lpc18xx.c|   2 +-
 drivers/pinctrl/pinctrl-palmas.c |   2 +-
 drivers/pinctrl/pinctrl-pic32.c  |   2 +-
 drivers/pinctrl/pinctrl-pistachio.c  |   2 +-
 drivers/pinctrl/pinctrl-tb10x.c  |   2 +-
 drivers/pinctrl/pinctrl-utils.c  |   4 +-
 drivers/pinctrl/pinctrl-utils.h  |   2 +-
 drivers/pinctrl/pinctrl-zynq.c   |   2 +-
 drivers/pinctrl/pxa/pinctrl-pxa2xx.c |   2 +-
 drivers/pinctrl/qcom/pinctrl-msm.c   |   2 +-
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c |   2 +-
 drivers/pinctrl/qcom/pinctrl-spmi-mpp.c  |   2 +-
 drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c |   2 +-
 drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c  |   2 +-
 drivers/pinctrl/stm32/pinctrl-stm32.c|   4 +-
 drivers/pinctrl/tegra/pinctrl-tegra-xusb.c   |   2 +-
 drivers/pinctrl/tegra/pinctrl-tegra.c|   4 +-
 drivers/pinctrl/uniphier/pinctrl-uniphier-core.c |   2 +-
 include/linux/acpi.h |   4 +-
 include/linux/pinctrl/pinconf-generic.h  |  27 +-
 include/linux/property.h |   9 +
 43 files changed, 1063 insertions(+), 114 deletions(-)
 create mode 100644 Documentation/acpi/pinctrl-properties.txt
 create mode 100644 drivers/pinctrl/acpi.c
 create mode 100644 drivers/pinctrl/acpi.h

-- 
1.9.1

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

[Irina Tirdea]

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.

Irina Tirdea (4):
  pinctrl: Rename pinctrl_utils_dt_free_map to pinctrl_utils_free_map
  pinctrl: pinconf-generic: Add ACPI support
  pinctrl: Add ACPI support
  pinctrl: Parse GpioInt/GpioIo resources

 Documentation/acpi/pinctrl-properties.txt| 282 
 drivers/acpi/property.c  |   8 +-
 drivers/base/property.c  |  36 +-
 drivers/pinctrl/Makefile |   1 +
 drivers/pinctrl/acpi.c   | 551 +++
 drivers/pinctrl/acpi.h   |  32 ++
 drivers/pinctrl/bcm/pinctrl-bcm281xx.c   |   2 +-
 drivers/pinctrl/bcm/pinctrl-cygnus-mux.c |   2 +-
 drivers/pinctrl/bcm/pinctrl-iproc-gpio.c |   2 +-
 drivers/pinctrl/bcm/pinctrl-nsp-gpio.c   |   2 +-
 drivers/pinctrl/berlin/berlin.c  |   2 +-
 drivers/pinctrl/core.c   |  26 ++
 drivers/pinctrl/core.h   |   2 +
 drivers/pinctrl/mediatek/pinctrl-mtk-common.c|   4 +-
 drivers/pinctrl/meson/pinctrl-meson.c|   2 +-
 drivers/pinctrl/nomadik/pinctrl-abx500.c |   4 +-
 drivers/pinctrl/nomadik/pinctrl-nomadik.c|   4 +-
 drivers/pinctrl/pinconf-generic.c| 123 +++--
 drivers/pinctrl/pinctrl-amd.c|   2 +-
 drivers/pinctrl/pinctrl-as3722.c |   2 +-
 drivers/pinctrl/pinctrl-at91-pio4.c  |   4 +-
 drivers/pinctrl/pinctrl-digicolor.c  |   2 +-
 drivers/pinctrl/pinctrl-lpc18xx.c|   2 +-
 drivers/pinctrl/pinctrl-palmas.c |   2 +-
 drivers/pinctrl/pinctrl-pic32.c  |   2 +-
 drivers/pinctrl/pinctrl-pistachio.c  |   2 +-
 drivers/pinctrl/pinctrl-tb10x.c  |   2 +-
 drivers/pinctrl/pinctrl-utils.c  |   4 +-
 drivers/pinctrl/pinctrl-utils.h  |   2 +-
 drivers/pinctrl/pinctrl-zynq.c   |   2 +-
 drivers/pinctrl/pxa/pinctrl-pxa2xx.c |   2 +-
 drivers/pinctrl/qcom/pinctrl-msm.c   |   2 +-
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c |   2 +-
 drivers/pinctrl/qcom/pinctrl-spmi-mpp.c  |   2 +-
 drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c |   2 +-
 drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c  |   2 +-
 drivers/pinctrl/stm32/pinctrl-stm32.c|   4 +-
 drivers/pinctrl/tegra/pinctrl-tegra-xusb.c   |   2 +-
 drivers/pinctrl/tegra/pinctrl-tegra.c|   4 +-
 drivers/pinctrl/uniphier/pinctrl-uniphier-core.c |   2 +-
 include/linux/acpi.h |   4 +-
 include/linux/pinctrl/pinconf-generic.h  |  27 +-
 include/linux/property.h |   9 +
 43 files changed, 1063 insertions(+), 114 deletions(-)
 create mode 100644 Documentation/acpi/pinctrl-properties.txt
 create mode 100644 drivers/pinctrl/acpi.c
 create mode 100644 drivers/pinctrl/acpi.h

-- 
1.9.1