by Mark Rutland

[permalink] [raw]

Subject: 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/000019.html

2016-04-05 02:15:35

On Tue, Apr 05, 2016 at 11:00:50AM +0200, Linus Walleij wrote:
> On Mon, Apr 4, 2016 at 11:40 PM, Mark Brown <[email protected]> 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.

Attachments:

(No filename) (2.39 kB)
signature.asc (473.00 B)
Download all attachments

2016-04-05 16:59:51

by Mark Brown

[permalink] [raw]

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

Attachments:

(No filename) (2.34 kB)
signature.asc (473.00 B)
Download all attachments

2016-04-05 17:31:48

2016-04-13 05:08:49

by Rafael J. Wysocki

[permalink] [raw]

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

On Tue, Apr 12, 2016 at 2:15 PM, Mark Brown <[email protected]> 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