Re: [RFC PATCH 0/4] Add ACPI support for pinctrl configuration
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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 problem
Re: [RFC PATCH 0/4] Add ACPI support for pinctrl configuration
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
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
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
> -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
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
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
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
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
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
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