Message-ID: <CAAVeFuJjMDfz7TZexkYnYrCGkAmRwubs32UhSPO-4qZWbMJjuQ@mail.gmail.com>
Date:	Thu, 22 Jan 2015 11:57:55 +0900
From:	Alexandre Courbot <gnurou@...il.com>
To:	"Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:	Linus Walleij <linus.walleij@...aro.org>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	Darren Hart <dvhart@...ux.intel.com>,
	Arnd Bergmann <arnd@...db.de>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	ACPI Devel Maling List <linux-acpi@...r.kernel.org>
Subject: Re: [RFC PATCH] gpio: support for GPIO forwarding

On Wed, Jan 21, 2015 at 6:25 AM, Rafael J. Wysocki <rjw@...ysocki.net> wrote:
> On Tuesday, January 20, 2015 01:16:06 PM Linus Walleij wrote:
>> On Mon, Jan 19, 2015 at 6:59 AM, Alexandre Courbot <gnurou@...il.com> wrote:
>>
>> > I am not really fond of this idea since it adds complexity to the
>> > (already too complex) GPIO lookup, and only solves to a local level
>> > (GPIO) what is a more global problem (bad ACPI tables that can affect
>> > any subsystem).
>> (...)
>> > it
>> > seems more to-the-point to find a way to fix/patch the ACPI tables at
>> > runtime, if that is possible at all, to provide a more general
>> > solution to this issue.
>>
>> This is my position as well, until proven that this cannot be done.
>
> Well, that goes against the current practice, mind you, which *is* to put
> workarounds for buggy ACPI tables into the kernel.  I'm not going to defend
> that, but it has been done for several years now.
>
> Also someone may say to that: "Why don't *you* demonstrate that it can be done
> in the first place?"  And what if it can be done, but is too complex to be
> practical or similar?
>
> My personal opinion is that having a way to apply a fix on top of broken ACPI
> tables (or an extension on top of correct ones for that matter) without touching
> the kernel would be very useful indeed, but making it secure may be somewhat
> challenging, because in principle there's no reason why the kernel should trust
> such "external" fixes.
>
>> In device tree the same mechanism is called "device tree overlays"
>> and I just have some vague feeling that such stuff is patched around in
>> some Intel platforms already, but maybe that involves replacing
>> the whole DSDT from userspace,
>
> From initramfs rather than from user space, but yes, it does.
>
>> surely the mechanism can be refined?
>
> Yes, it can (in principle).  In fact, we have a plan to refine it, but it is
> going to take some time.  Once we've done that, we'll see how painful it is to
> "patch" ACPI tables this way in practice.
>
> Also there is an ecosystem problem related to distributing such "patches".
> Today, distributions don't need to worry about patching buggy platform
> firmware, because they get workarounds in the kernel, but if we switch over
> to the model in which platform firmware "overlays" need to be provided in
> addition to it, then suddenly questions arise about who should be responsible
> for making them available, how to avoid duplication of efforts between
> distributions etc.
>
> All of that needs to be clarified before we start making hard statements like
> "No in-kernel workarounds for that!"
>
> And, of course, there's the question of what the kernel should do if the given
> firmware patch is not effective, so it doesn't really fix the problem it is
> supposed to fix or it fixes that problem only partially or, worse yet, it
> introuces more bugs than it fixes.  Should the kernel simply fail then (and
> in what way if so) or should it try to carry out some default "sanitization"
> of what the firmare (and patch) tells it and try to continue on the best
> effort basis?

If we decide to go ahead with the solution proposed by this patch for
practical reasons (which are good reasons indeed), I still have one
problem with its current form.

As the discussion highlighted, this is an ACPI problem, so I'd very
much like it to be confined to the ACPI GPIO code, to be enabled only
when ACPI is, and to use function names that start with acpi_gpio. The
current implementation leverages platform lookup, making said lookup
less efficient in the process and bringing confusion about its
purpose. Although the two processes are indeed similar, they are
separate things: one is a legitimate way to map GPIOs, the other is a
fixup for broken firmware.

I suppose we all agree this is a hackish fix, so let's confine it as
much as we can.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists