| Message ID | 1418890998-23811-1-git-send-email-heikki.krogerus@linux.intel.com |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Dec 18, 2014 at 10:23:18AM +0200, Heikki Krogerus wrote: > This makes it possible to assign GPIOs at runtime. The > motivation for it is because of need to forward GPIOs from > one device to an other. That feature may be useful for > example with some mfd devices, but initially it is needed > because on some Intel Braswell based ACPI platforms the GPIO > resources controlling signals of the USB PHY are given to > the controller device object for whatever reason, so the > driver of that controller needs be able to pass them to the > PHY device somehow. > > So basically this is meant to allow patching of bad (bad > from Linux kernels point of view) hardware descriptions > provided by system fw in the drivers. > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > > Hi, > > I'm sending this first as a RFC in case you guys have some better > idea how to solve our problem. I actually already have couple > platforms where the GPIO resources are given to the "wrong" device > objects now. > > Originally we were thinking about simply handling our problem with > hacks to the PHY drivers, basically also checking if the parent has > GPIOs. That would only work if the controller is always the parent, > which it's not, but even if it was, it would be too risky. The PHY > drivers don't know which controller they are attached to, so what is > to say that the GPIOs aren't really attached to the controller. > > So the safest way to handle our problem is to deal with it in quirks > in the controller drivers. Solving it by bouncing the GPIOs did not > feel ideal there doesn't seem to be any other way. The API is copied > from clkdev (clk_register_clkdev). In the end it's really only one > function for adding the lookups and one for removing them. > > The way it's implemented is by modifying the current style of handling > the lookups a little bit. The per device lookup table is basically > reverted back to the single linked-list format since it seems that > these lookups may have to be assigned from several sources. > > Thanks, Gentle ping on this.
On Thu, Dec 18, 2014 at 9:23 AM, Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > This makes it possible to assign GPIOs at runtime. The > motivation for it is because of need to forward GPIOs from > one device to an other. That feature may be useful for > example with some mfd devices, but initially it is needed > because on some Intel Braswell based ACPI platforms the GPIO > resources controlling signals of the USB PHY are given to > the controller device object for whatever reason, so the > driver of that controller needs be able to pass them to the > PHY device somehow. > > So basically this is meant to allow patching of bad (bad > from Linux kernels point of view) hardware descriptions > provided by system fw in the drivers. > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > > Hi, > > I'm sending this first as a RFC in case you guys have some better > idea how to solve our problem. I actually already have couple > platforms where the GPIO resources are given to the "wrong" device > objects now. > > Originally we were thinking about simply handling our problem with > hacks to the PHY drivers, basically also checking if the parent has > GPIOs. That would only work if the controller is always the parent, > which it's not, but even if it was, it would be too risky. The PHY > drivers don't know which controller they are attached to, so what is > to say that the GPIOs aren't really attached to the controller. > > So the safest way to handle our problem is to deal with it in quirks > in the controller drivers. Solving it by bouncing the GPIOs did not > feel ideal there doesn't seem to be any other way. The API is copied > from clkdev (clk_register_clkdev). In the end it's really only one > function for adding the lookups and one for removing them. > > The way it's implemented is by modifying the current style of handling > the lookups a little bit. The per device lookup table is basically > reverted back to the single linked-list format since it seems that > these lookups may have to be assigned from several sources. Oh ain't that great. So now we start patching around the kernel because the ACPI tables are erroneous for GPIOs. I'd like some feedback from Rafael &| Darren on this patch, i.e. if you two think this is a good way of accounting for bad GPIO descriptions in ACPI tables then ACK this patch. I guess the other option would be to fix up the ACPI DSDT itself to put resources right, correct? Is this not possible? Alexandre also need to ACK it before I dare do anything with it. Do we want to use the same mechanism for augmenting bad device trees too? What I like about it so far is the create/remove symmetry though. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 1/14/15 4:58 AM, Linus Walleij wrote: > On Thu, Dec 18, 2014 at 9:23 AM, Heikki Krogerus > <heikki.krogerus@linux.intel.com> wrote: > >> This makes it possible to assign GPIOs at runtime. The >> motivation for it is because of need to forward GPIOs from >> one device to an other. That feature may be useful for >> example with some mfd devices, but initially it is needed +Samuel Ortiz for his thoughts on applicability to MFD. >> because on some Intel Braswell based ACPI platforms the GPIO >> resources controlling signals of the USB PHY are given to >> the controller device object for whatever reason, so the >> driver of that controller needs be able to pass them to the >> PHY device somehow. >> >> So basically this is meant to allow patching of bad (bad >> from Linux kernels point of view) hardware descriptions >> provided by system fw in the drivers. >> >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> >> --- >> >> Hi, >> >> I'm sending this first as a RFC in case you guys have some better >> idea how to solve our problem. I actually already have couple >> platforms where the GPIO resources are given to the "wrong" device >> objects now. >> >> Originally we were thinking about simply handling our problem with >> hacks to the PHY drivers, basically also checking if the parent has >> GPIOs. That would only work if the controller is always the parent, >> which it's not, but even if it was, it would be too risky. The PHY >> drivers don't know which controller they are attached to, so what is >> to say that the GPIOs aren't really attached to the controller. >> >> So the safest way to handle our problem is to deal with it in quirks >> in the controller drivers. Solving it by bouncing the GPIOs did not >> feel ideal there doesn't seem to be any other way. The API is copied >> from clkdev (clk_register_clkdev). In the end it's really only one >> function for adding the lookups and one for removing them. >> >> The way it's implemented is by modifying the current style of handling >> the lookups a little bit. The per device lookup table is basically >> reverted back to the single linked-list format since it seems that >> these lookups may have to be assigned from several sources. > > Oh ain't that great. > > So now we start patching around the kernel because the ACPI > tables are erroneous for GPIOs. I'd like some feedback from > Rafael &| Darren on this patch, i.e. if you two think this is a good > way of accounting for bad GPIO descriptions in ACPI tables then > ACK this patch. > > I guess the other option would be to fix up the ACPI DSDT > itself to put resources right, correct? Is this not possible? This is my gut reaction as well. Heikki, why can't we correct the firmware tables? That needs to be made clear before we start adding hacks to the kernel. You said, "Bad for Linux", why is this not a problem for other operating systems? > > Alexandre also need to ACK it before I dare do anything with > it. > > Do we want to use the same mechanism for augmenting > bad device trees too? > > What I like about it so far is the create/remove symmetry > though. > > Yours, > Linus Walleij >
Ugh, Samuel actually Cc'd this time... On 1/14/15 4:58 AM, Linus Walleij wrote: > On Thu, Dec 18, 2014 at 9:23 AM, Heikki Krogerus > <heikki.krogerus@linux.intel.com> wrote: > >> This makes it possible to assign GPIOs at runtime. The >> motivation for it is because of need to forward GPIOs from >> one device to an other. That feature may be useful for >> example with some mfd devices, but initially it is needed >> because on some Intel Braswell based ACPI platforms the GPIO >> resources controlling signals of the USB PHY are given to >> the controller device object for whatever reason, so the >> driver of that controller needs be able to pass them to the >> PHY device somehow. >> >> So basically this is meant to allow patching of bad (bad >> from Linux kernels point of view) hardware descriptions >> provided by system fw in the drivers. >> >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> >> --- >> >> Hi, >> >> I'm sending this first as a RFC in case you guys have some better >> idea how to solve our problem. I actually already have couple >> platforms where the GPIO resources are given to the "wrong" device >> objects now. >> >> Originally we were thinking about simply handling our problem with >> hacks to the PHY drivers, basically also checking if the parent has >> GPIOs. That would only work if the controller is always the parent, >> which it's not, but even if it was, it would be too risky. The PHY >> drivers don't know which controller they are attached to, so what is >> to say that the GPIOs aren't really attached to the controller. >> >> So the safest way to handle our problem is to deal with it in quirks >> in the controller drivers. Solving it by bouncing the GPIOs did not >> feel ideal there doesn't seem to be any other way. The API is copied >> from clkdev (clk_register_clkdev). In the end it's really only one >> function for adding the lookups and one for removing them. >> >> The way it's implemented is by modifying the current style of handling >> the lookups a little bit. The per device lookup table is basically >> reverted back to the single linked-list format since it seems that >> these lookups may have to be assigned from several sources. > > Oh ain't that great. > > So now we start patching around the kernel because the ACPI > tables are erroneous for GPIOs. I'd like some feedback from > Rafael &| Darren on this patch, i.e. if you two think this is a good > way of accounting for bad GPIO descriptions in ACPI tables then > ACK this patch. > > I guess the other option would be to fix up the ACPI DSDT > itself to put resources right, correct? Is this not possible? > > Alexandre also need to ACK it before I dare do anything with > it. > > Do we want to use the same mechanism for augmenting > bad device trees too? > > What I like about it so far is the create/remove symmetry > though. > > Yours, > Linus Walleij >
On Thursday, January 08, 2015 10:25:10 AM Heikki Krogerus wrote: > On Thu, Dec 18, 2014 at 10:23:18AM +0200, Heikki Krogerus wrote: > > This makes it possible to assign GPIOs at runtime. The > > motivation for it is because of need to forward GPIOs from > > one device to an other. That feature may be useful for > > example with some mfd devices, but initially it is needed > > because on some Intel Braswell based ACPI platforms the GPIO > > resources controlling signals of the USB PHY are given to > > the controller device object for whatever reason, so the > > driver of that controller needs be able to pass them to the > > PHY device somehow. > > > > So basically this is meant to allow patching of bad (bad > > from Linux kernels point of view) hardware descriptions > > provided by system fw in the drivers. > > > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > --- > > > > Hi, > > > > I'm sending this first as a RFC in case you guys have some better > > idea how to solve our problem. I actually already have couple > > platforms where the GPIO resources are given to the "wrong" device > > objects now. > > > > Originally we were thinking about simply handling our problem with > > hacks to the PHY drivers, basically also checking if the parent has > > GPIOs. That would only work if the controller is always the parent, > > which it's not, but even if it was, it would be too risky. The PHY > > drivers don't know which controller they are attached to, so what is > > to say that the GPIOs aren't really attached to the controller. > > > > So the safest way to handle our problem is to deal with it in quirks > > in the controller drivers. Solving it by bouncing the GPIOs did not > > feel ideal there doesn't seem to be any other way. The API is copied > > from clkdev (clk_register_clkdev). In the end it's really only one > > function for adding the lookups and one for removing them. > > > > The way it's implemented is by modifying the current style of handling > > the lookups a little bit. The per device lookup table is basically > > reverted back to the single linked-list format since it seems that > > these lookups may have to be assigned from several sources. > > > > Thanks, > > Gentle ping on this. Please CC patches involving ACPI in *any* *way* way to linux-acpi@vger.kernel.org. Thanks!
On Wednesday, January 14, 2015 08:32:12 AM Darren Hart wrote: > > On 1/14/15 4:58 AM, Linus Walleij wrote: > > On Thu, Dec 18, 2014 at 9:23 AM, Heikki Krogerus > > <heikki.krogerus@linux.intel.com> wrote: > > > >> This makes it possible to assign GPIOs at runtime. The > >> motivation for it is because of need to forward GPIOs from > >> one device to an other. That feature may be useful for > >> example with some mfd devices, but initially it is needed > > +Samuel Ortiz for his thoughts on applicability to MFD. > > >> because on some Intel Braswell based ACPI platforms the GPIO > >> resources controlling signals of the USB PHY are given to > >> the controller device object for whatever reason, so the > >> driver of that controller needs be able to pass them to the > >> PHY device somehow. > >> > >> So basically this is meant to allow patching of bad (bad > >> from Linux kernels point of view) hardware descriptions > >> provided by system fw in the drivers. > >> > >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > >> --- > >> > >> Hi, > >> > >> I'm sending this first as a RFC in case you guys have some better > >> idea how to solve our problem. I actually already have couple > >> platforms where the GPIO resources are given to the "wrong" device > >> objects now. > >> > >> Originally we were thinking about simply handling our problem with > >> hacks to the PHY drivers, basically also checking if the parent has > >> GPIOs. That would only work if the controller is always the parent, > >> which it's not, but even if it was, it would be too risky. The PHY > >> drivers don't know which controller they are attached to, so what is > >> to say that the GPIOs aren't really attached to the controller. > >> > >> So the safest way to handle our problem is to deal with it in quirks > >> in the controller drivers. Solving it by bouncing the GPIOs did not > >> feel ideal there doesn't seem to be any other way. The API is copied > >> from clkdev (clk_register_clkdev). In the end it's really only one > >> function for adding the lookups and one for removing them. > >> > >> The way it's implemented is by modifying the current style of handling > >> the lookups a little bit. The per device lookup table is basically > >> reverted back to the single linked-list format since it seems that > >> these lookups may have to be assigned from several sources. > > > > Oh ain't that great. > > > > So now we start patching around the kernel because the ACPI > > tables are erroneous for GPIOs. I'd like some feedback from > > Rafael &| Darren on this patch, i.e. if you two think this is a good > > way of accounting for bad GPIO descriptions in ACPI tables then > > ACK this patch. > > > > I guess the other option would be to fix up the ACPI DSDT > > itself to put resources right, correct? Is this not possible? > > This is my gut reaction as well. Pretty much agreed. > Heikki, why can't we correct the firmware tables? That needs to be made clear > before we start adding hacks to the kernel. We need to start working in that direction really. Fixing problems in ACPI tables via kernel code is not going to scale sufficiently IMO. > You said, "Bad for Linux", why is this not a problem for other operating > systems? Good question. :-)
On Thu, Jan 15, 2015 at 10:28:03AM +0100, Rafael J. Wysocki wrote: > On Wednesday, January 14, 2015 08:32:12 AM Darren Hart wrote: > > > > On 1/14/15 4:58 AM, Linus Walleij wrote: > > > On Thu, Dec 18, 2014 at 9:23 AM, Heikki Krogerus > > > <heikki.krogerus@linux.intel.com> wrote: > > > > > >> This makes it possible to assign GPIOs at runtime. The > > >> motivation for it is because of need to forward GPIOs from > > >> one device to an other. That feature may be useful for > > >> example with some mfd devices, but initially it is needed > > > > +Samuel Ortiz for his thoughts on applicability to MFD. > > > > >> because on some Intel Braswell based ACPI platforms the GPIO > > >> resources controlling signals of the USB PHY are given to > > >> the controller device object for whatever reason, so the > > >> driver of that controller needs be able to pass them to the > > >> PHY device somehow. > > >> > > >> So basically this is meant to allow patching of bad (bad > > >> from Linux kernels point of view) hardware descriptions > > >> provided by system fw in the drivers. > > >> > > >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > >> --- > > >> > > >> Hi, > > >> > > >> I'm sending this first as a RFC in case you guys have some better > > >> idea how to solve our problem. I actually already have couple > > >> platforms where the GPIO resources are given to the "wrong" device > > >> objects now. > > >> > > >> Originally we were thinking about simply handling our problem with > > >> hacks to the PHY drivers, basically also checking if the parent has > > >> GPIOs. That would only work if the controller is always the parent, > > >> which it's not, but even if it was, it would be too risky. The PHY > > >> drivers don't know which controller they are attached to, so what is > > >> to say that the GPIOs aren't really attached to the controller. > > >> > > >> So the safest way to handle our problem is to deal with it in quirks > > >> in the controller drivers. Solving it by bouncing the GPIOs did not > > >> feel ideal there doesn't seem to be any other way. The API is copied > > >> from clkdev (clk_register_clkdev). In the end it's really only one > > >> function for adding the lookups and one for removing them. > > >> > > >> The way it's implemented is by modifying the current style of handling > > >> the lookups a little bit. The per device lookup table is basically > > >> reverted back to the single linked-list format since it seems that > > >> these lookups may have to be assigned from several sources. > > > > > > Oh ain't that great. > > > > > > So now we start patching around the kernel because the ACPI > > > tables are erroneous for GPIOs. I'd like some feedback from > > > Rafael &| Darren on this patch, i.e. if you two think this is a good > > > way of accounting for bad GPIO descriptions in ACPI tables then > > > ACK this patch. > > > > > > I guess the other option would be to fix up the ACPI DSDT > > > itself to put resources right, correct? Is this not possible? > > > > This is my gut reaction as well. > > Pretty much agreed. > > > Heikki, why can't we correct the firmware tables? That needs to be made clear > > before we start adding hacks to the kernel. > > We need to start working in that direction really. Fixing problems in ACPI > tables via kernel code is not going to scale sufficiently IMO. Fixing the DSDT produced by the firmware was my first suggesting for this, but it just does not seem to be happening. These products are already out on the market (this is one of them [1]) and what I have understood is that the firmware just is what it is. It's almost as if there is nobody even developing it for these products anymore. Even fixes would not go in and this is not even considered a fix. Things work just find for them with their hacked kernel. So the firmware and the ACPI tables it provides are not going to be fixed. What else can we do? Can we expect the users to always use custom DSDT, or maybe somekind of custom AML snipped (is something like that even possible), when using these products? > > You said, "Bad for Linux", why is this not a problem for other operating > > systems? > > Good question. :-) I would imagine certain operating systems consider a component like PHY as something that should always be handled by the controller driver. It does not seem to be a problem for them even if every second product using the same SoC happens to have different PHY. All of them will have a custom controller driver in any case. Even though the example product below is an Android tablet, to me it looks like the style of writing software comes straight from those "other" operation systems. At least it seems clear that these guys don't understand that it's not possible to do things like write a new driver for every product using the same SoC in Linux. [1] http://www.trekstor.de/detail-surftabs-en/product/surftab-xintron-i-70.html Cheers,
On Wed, Jan 14, 2015 at 9:58 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Thu, Dec 18, 2014 at 9:23 AM, Heikki Krogerus > <heikki.krogerus@linux.intel.com> wrote: > >> This makes it possible to assign GPIOs at runtime. The >> motivation for it is because of need to forward GPIOs from >> one device to an other. That feature may be useful for >> example with some mfd devices, but initially it is needed >> because on some Intel Braswell based ACPI platforms the GPIO >> resources controlling signals of the USB PHY are given to >> the controller device object for whatever reason, so the >> driver of that controller needs be able to pass them to the >> PHY device somehow. >> >> So basically this is meant to allow patching of bad (bad >> from Linux kernels point of view) hardware descriptions >> provided by system fw in the drivers. >> >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> >> --- >> >> Hi, >> >> I'm sending this first as a RFC in case you guys have some better >> idea how to solve our problem. I actually already have couple >> platforms where the GPIO resources are given to the "wrong" device >> objects now. >> >> Originally we were thinking about simply handling our problem with >> hacks to the PHY drivers, basically also checking if the parent has >> GPIOs. That would only work if the controller is always the parent, >> which it's not, but even if it was, it would be too risky. The PHY >> drivers don't know which controller they are attached to, so what is >> to say that the GPIOs aren't really attached to the controller. >> >> So the safest way to handle our problem is to deal with it in quirks >> in the controller drivers. Solving it by bouncing the GPIOs did not >> feel ideal there doesn't seem to be any other way. The API is copied >> from clkdev (clk_register_clkdev). In the end it's really only one >> function for adding the lookups and one for removing them. >> >> The way it's implemented is by modifying the current style of handling >> the lookups a little bit. The per device lookup table is basically >> reverted back to the single linked-list format since it seems that >> these lookups may have to be assigned from several sources. > > Oh ain't that great. > > So now we start patching around the kernel because the ACPI > tables are erroneous for GPIOs. I'd like some feedback from > Rafael &| Darren on this patch, i.e. if you two think this is a good > way of accounting for bad GPIO descriptions in ACPI tables then > ACK this patch. > > I guess the other option would be to fix up the ACPI DSDT > itself to put resources right, correct? Is this not possible? > > Alexandre also need to ACK it before I dare do anything with > it. 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). Also I don't think any new functions is actually needed: on an ACPI system we can safely assume that the platform lookup tables are not used at all. So, although it is a hack as well, you can just call gpiod_add_lookup_table() with a runtime-built table from the driver that needs to pass the GPIO so the receipient can receive it through the lookup table fallback gpiod_get() uses if no GPIO is defined via ACPI. So I think that even with the current state of the code you can achieve what you need. Should you do it, that's another question - 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. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 19, 2015 at 02:59:54PM +0900, Alexandre Courbot 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). > > Also I don't think any new functions is actually needed: on an ACPI > system we can safely assume that the platform lookup tables are not > used at all. So, although it is a hack as well, you can just call > gpiod_add_lookup_table() with a runtime-built table from the driver > that needs to pass the GPIO so the receipient can receive it through > the lookup table fallback gpiod_get() uses if no GPIO is defined via > ACPI. Unfortunately that would require us to determine details about the gpios in the consumer driver, details like the chip_label, chip_hwnum and flags. Stuff that I don't think the consumers should touch. In order to dig those out of the gpio description, the driver would either need to include linux/gpio/machine.h, linux/gpio/driver.h and linux/gpio/consumer.h or we would need to add helper functions to gpiolib.c that do the digging for us. Most likely it would not be possible to avoid the helpers. We would in any case have to add removal function for the lookups and gpiod_add_lookup_table would need to be exported. It simply ended up being less of a hack to add the simple function explicitly for consumers to be used in a case like this for creating the lookups. Br,
On Mon, Jan 19, 2015 at 6:59 AM, Alexandre Courbot <gnurou@gmail.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. 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, surely the mechanism can be refined? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, January 20, 2015 01:16:06 PM Linus Walleij wrote: > On Mon, Jan 19, 2015 at 6:59 AM, Alexandre Courbot <gnurou@gmail.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?
On Wed, Jan 21, 2015 at 6:25 AM, Rafael J. Wysocki <rjw@rjwysocki.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@gmail.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-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 20, 2015 at 10:25 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > 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!" OK so why can't the patching happen in the kernel? If the kernel anyway has to supply some kind of workaround for the issue, it is more a question of where to place it. Whether it does so by patching the ACPI tables or by detecting a bad ACPI thing and working around it at runtime in a certain driver doesn't really matter, does it? They are both in-kernel ACPI fixes, just that one of the mechanisms is generic. I don't understand why this obsession with userspace having to do the ACPI table patching - if kernels should "just work" then put this stuff behind Kconfig and have it in the kernel. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday, January 22, 2015 09:17:38 AM Linus Walleij wrote: > On Tue, Jan 20, 2015 at 10:25 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > > 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!" > > OK so why can't the patching happen in the kernel? > > If the kernel anyway has to supply some kind of workaround for > the issue, it is more a question of where to place it. Whether it does > so by patching the ACPI tables or by detecting a bad ACPI thing > and working around it at runtime in a certain driver doesn't really > matter, does it? It needs to know what to patch and how so the result is still consistent. How do you think the kernel is going to figure that out? > They are both in-kernel ACPI fixes, just that one > of the mechanisms is generic. I'm not following you here, sorry. > I don't understand why this obsession with userspace having > to do the ACPI table patching - if kernels should "just work" then > put this stuff behind Kconfig and have it in the kernel. This is not an obsession and your suggestion here leads to having custom per-board kernels which is not supportable in the long term.
On Thursday, January 22, 2015 11:57:55 AM Alexandre Courbot wrote: > On Wed, Jan 21, 2015 at 6:25 AM, Rafael J. Wysocki <rjw@rjwysocki.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@gmail.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. I can agree with that. > 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. OK Heikki, any comments?
On Thu, Jan 22, 2015 at 5:12 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Thursday, January 22, 2015 09:17:38 AM Linus Walleij wrote: >> If the kernel anyway has to supply some kind of workaround for >> the issue, it is more a question of where to place it. Whether it does >> so by patching the ACPI tables or by detecting a bad ACPI thing >> and working around it at runtime in a certain driver doesn't really >> matter, does it? > > It needs to know what to patch and how so the result is still consistent. > > How do you think the kernel is going to figure that out? Isn't every DSDT (etc) unique? So you could detect one by making a checksum of the binary or something. And then you'd know that the table with this checksum needs patching? >> They are both in-kernel ACPI fixes, just that one >> of the mechanisms is generic. > > I'm not following you here, sorry. As per above? Fix the firmware table, not work around the fact that it is buggy. >> I don't understand why this obsession with userspace having >> to do the ACPI table patching - if kernels should "just work" then >> put this stuff behind Kconfig and have it in the kernel. > > This is not an obsession and your suggestion here leads to having custom > per-board kernels which is not supportable in the long term. Not at all. The other suggestion leads to having custom per-board fixes in every driver for which broken ACPI descriptions exists, which is just as bad, isn't it? Instead of collecting the problem in one place (patch any broken ACPI table) it is distrubuted across N drivers, where each and every one has to detect that it is being malinformed by a broken ACPI table. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday, January 30, 2015 03:48:30 PM Linus Walleij wrote: > On Thu, Jan 22, 2015 at 5:12 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > On Thursday, January 22, 2015 09:17:38 AM Linus Walleij wrote: > > >> If the kernel anyway has to supply some kind of workaround for > >> the issue, it is more a question of where to place it. Whether it does > >> so by patching the ACPI tables or by detecting a bad ACPI thing > >> and working around it at runtime in a certain driver doesn't really > >> matter, does it? > > > > It needs to know what to patch and how so the result is still consistent. > > > > How do you think the kernel is going to figure that out? > > Isn't every DSDT (etc) unique? Formally, it doesn't have to be. And it doesn't have to be a DSDT, it may be an SSDT too or even a plain table that's buggy. > So you could detect one by making a checksum of the binary or something. > > And then you'd know that the table with this checksum needs patching? At a single table level it is generally difficult to say whether or not things are going to work. What needs to work is the namespace which is built from all of the tables provided combined. So the namespace needs to be populated first and then fixes applied on top of that (presumably by deleting, adding or replacing objects). Now, in theory, you *may* be able to figure out that combination of tables A produces namespace B which then will require fix X if the system is Y, but quite frankly I wouldn't count on that. Moreover, fixups (or "patches" as I called them, but that wasn't exactly correct) need to be provided in the form of AML definition blocks to apply on top of an already populated namespace and if you want to use a binary kernel image, you can't really afford putting all that stuff for all systems it can possibly run on into it. This means that distros need to be able to combine a fixup for the ACPI tables with the binary kernel and install the result into the system's boot medium (whatever it is). Also it should be possible to update the fixup and the kernel image separately if necessary. Now from the kernel's perspective that raises the question: "What if the ACPI tables fixup provided by the distro is not sufficient?" That needs to be addressed somehow in the code. > >> They are both in-kernel ACPI fixes, just that one > >> of the mechanisms is generic. > > > > I'm not following you here, sorry. > > As per above? Fix the firmware table, not work around the > fact that it is buggy. > > >> I don't understand why this obsession with userspace having > >> to do the ACPI table patching - if kernels should "just work" then > >> put this stuff behind Kconfig and have it in the kernel. > > > > This is not an obsession and your suggestion here leads to having custom > > per-board kernels which is not supportable in the long term. > > Not at all. The other suggestion leads to having custom per-board > fixes in every driver for which broken ACPI descriptions exists, So I'm not arguing for that. > which is just as bad, isn't it? Instead of collecting the problem > in one place (patch any broken ACPI table) it is distrubuted across > N drivers, where each and every one has to detect that it is > being malinformed by a broken ACPI table. In the general-purpose binary kernel image distribution model drivers generally need to treat information provided by the platform firmware, be it ACPI or a DT or what-not, as untrusted input that needs to be validated if possible. That is not always possible and in those cases we have no choice but to try to use that information, fingers crossed. Sometimes we can validate it, though, and then we should and do something if it turns out to be invalid. Overall, my view on that is that (1) there needs to be a way for a distro to provide an ACPI tables fixup for the kernel to apply on top of the already populated ACPI namespace on a given system and (2) drivers need to be careful about using firmware data and possibly be able to recover from errors in there.
On Fri, Jan 30, 2015 at 5:17 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Friday, January 30, 2015 03:48:30 PM Linus Walleij wrote: >> So you could detect one by making a checksum of the binary or something. >> >> And then you'd know that the table with this checksum needs patching? > > At a single table level it is generally difficult to say whether or not > things are going to work. > > What needs to work is the namespace which is built from all of the tables > provided combined. So the namespace needs to be populated first and then > fixes applied on top of that (presumably by deleting, adding or replacing > objects). > > Now, in theory, you *may* be able to figure out that combination of tables > A produces namespace B which then will require fix X if the system is Y, > but quite frankly I wouldn't count on that. > > Moreover, fixups (or "patches" as I called them, but that wasn't exactly > correct) need to be provided in the form of AML definition blocks to apply on > top of an already populated namespace and if you want to use a binary kernel image, > you can't really afford putting all that stuff for all systems it can possibly > run on into it. This means that distros need to be able to combine a fixup for > the ACPI tables with the binary kernel and install the result into the system's > boot medium (whatever it is). Also it should be possible to update the fixup > and the kernel image separately if necessary. > > Now from the kernel's perspective that raises the question: "What if the > ACPI tables fixup provided by the distro is not sufficient?" > > That needs to be addressed somehow in the code. Yeah I guess I'm convinced that we need to handle this particular weirdness in the gpio-acpi code... if it can be contained there as expressed by Alexandre. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 04, 2015 at 10:51:27AM +0100, Linus Walleij wrote: > On Fri, Jan 30, 2015 at 5:17 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > On Friday, January 30, 2015 03:48:30 PM Linus Walleij wrote: > > >> So you could detect one by making a checksum of the binary or something. > >> > >> And then you'd know that the table with this checksum needs patching? > > > > At a single table level it is generally difficult to say whether or not > > things are going to work. > > > > What needs to work is the namespace which is built from all of the tables > > provided combined. So the namespace needs to be populated first and then > > fixes applied on top of that (presumably by deleting, adding or replacing > > objects). > > > > Now, in theory, you *may* be able to figure out that combination of tables > > A produces namespace B which then will require fix X if the system is Y, > > but quite frankly I wouldn't count on that. > > > > Moreover, fixups (or "patches" as I called them, but that wasn't exactly > > correct) need to be provided in the form of AML definition blocks to apply on > > top of an already populated namespace and if you want to use a binary kernel image, > > you can't really afford putting all that stuff for all systems it can possibly > > run on into it. This means that distros need to be able to combine a fixup for > > the ACPI tables with the binary kernel and install the result into the system's > > boot medium (whatever it is). Also it should be possible to update the fixup > > and the kernel image separately if necessary. > > > > Now from the kernel's perspective that raises the question: "What if the > > ACPI tables fixup provided by the distro is not sufficient?" > > > > That needs to be addressed somehow in the code. > > Yeah I guess I'm convinced that we need to handle this particular > weirdness in the gpio-acpi code... if it can be contained there as > expressed by Alexandre. I'm still fine if we want to confine this "gpio forwarding" to acpi if you guys want it, but I was looking at the Sound SoC drivers and I realised that we do have places which pass gpio descriptors to other devices in platform data. And these of course aren't always used on acpi platforms. By greping gpio_desc I saw at least these files are passing it in platform data structures: include/sound/soc.h include/linux/leds.h include/linux/usb/usb_phy_generic.h There are probable others but I checked those. And of course now there is nothing preventing people from adding more of them. So we could use my original idea with them. I think most of the drivers using those are tied to separate handling for dt and pdata only because of the way the gpio descriptor is delivered to them. What do you guys think? Cheers,
On Wed, Feb 4, 2015 at 11:11 PM, Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > On Wed, Feb 04, 2015 at 10:51:27AM +0100, Linus Walleij wrote: >> On Fri, Jan 30, 2015 at 5:17 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> > On Friday, January 30, 2015 03:48:30 PM Linus Walleij wrote: >> >> >> So you could detect one by making a checksum of the binary or something. >> >> >> >> And then you'd know that the table with this checksum needs patching? >> > >> > At a single table level it is generally difficult to say whether or not >> > things are going to work. >> > >> > What needs to work is the namespace which is built from all of the tables >> > provided combined. So the namespace needs to be populated first and then >> > fixes applied on top of that (presumably by deleting, adding or replacing >> > objects). >> > >> > Now, in theory, you *may* be able to figure out that combination of tables >> > A produces namespace B which then will require fix X if the system is Y, >> > but quite frankly I wouldn't count on that. >> > >> > Moreover, fixups (or "patches" as I called them, but that wasn't exactly >> > correct) need to be provided in the form of AML definition blocks to apply on >> > top of an already populated namespace and if you want to use a binary kernel image, >> > you can't really afford putting all that stuff for all systems it can possibly >> > run on into it. This means that distros need to be able to combine a fixup for >> > the ACPI tables with the binary kernel and install the result into the system's >> > boot medium (whatever it is). Also it should be possible to update the fixup >> > and the kernel image separately if necessary. >> > >> > Now from the kernel's perspective that raises the question: "What if the >> > ACPI tables fixup provided by the distro is not sufficient?" >> > >> > That needs to be addressed somehow in the code. >> >> Yeah I guess I'm convinced that we need to handle this particular >> weirdness in the gpio-acpi code... if it can be contained there as >> expressed by Alexandre. > > I'm still fine if we want to confine this "gpio forwarding" to acpi > if you guys want it, but I was looking at the Sound SoC drivers and I > realised that we do have places which pass gpio descriptors to other > devices in platform data. And these of course aren't always used on > acpi platforms. By greping gpio_desc I saw at least these files are > passing it in platform data structures: > > include/sound/soc.h > include/linux/leds.h > include/linux/usb/usb_phy_generic.h > > There are probable others but I checked those. And of course now there > is nothing preventing people from adding more of them. For sound/soc.h, the member is indeed public but I don't see it being used to pass descriptors around between drivers. For linux/leds.h, I think this is the reason why we introduced devm_get_gpiod_from_child() These looks more like a bad usage of GPIO descriptors, but AFAICT they can be fixed by fixing the drivers and, by all means, this is where we should do it. This is a different situation from yours where we are trying to deal with broken firmware and need to resort to tricks at one point or the other. Or am I missing your point? -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 10, 2015 at 06:44:34PM +0900, Alexandre Courbot wrote: > On Wed, Feb 4, 2015 at 11:11 PM, Heikki Krogerus > <heikki.krogerus@linux.intel.com> wrote: > > On Wed, Feb 04, 2015 at 10:51:27AM +0100, Linus Walleij wrote: > >> On Fri, Jan 30, 2015 at 5:17 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > >> > On Friday, January 30, 2015 03:48:30 PM Linus Walleij wrote: > >> > >> >> So you could detect one by making a checksum of the binary or something. > >> >> > >> >> And then you'd know that the table with this checksum needs patching? > >> > > >> > At a single table level it is generally difficult to say whether or not > >> > things are going to work. > >> > > >> > What needs to work is the namespace which is built from all of the tables > >> > provided combined. So the namespace needs to be populated first and then > >> > fixes applied on top of that (presumably by deleting, adding or replacing > >> > objects). > >> > > >> > Now, in theory, you *may* be able to figure out that combination of tables > >> > A produces namespace B which then will require fix X if the system is Y, > >> > but quite frankly I wouldn't count on that. > >> > > >> > Moreover, fixups (or "patches" as I called them, but that wasn't exactly > >> > correct) need to be provided in the form of AML definition blocks to apply on > >> > top of an already populated namespace and if you want to use a binary kernel image, > >> > you can't really afford putting all that stuff for all systems it can possibly > >> > run on into it. This means that distros need to be able to combine a fixup for > >> > the ACPI tables with the binary kernel and install the result into the system's > >> > boot medium (whatever it is). Also it should be possible to update the fixup > >> > and the kernel image separately if necessary. > >> > > >> > Now from the kernel's perspective that raises the question: "What if the > >> > ACPI tables fixup provided by the distro is not sufficient?" > >> > > >> > That needs to be addressed somehow in the code. > >> > >> Yeah I guess I'm convinced that we need to handle this particular > >> weirdness in the gpio-acpi code... if it can be contained there as > >> expressed by Alexandre. > > > > I'm still fine if we want to confine this "gpio forwarding" to acpi > > if you guys want it, but I was looking at the Sound SoC drivers and I > > realised that we do have places which pass gpio descriptors to other > > devices in platform data. And these of course aren't always used on > > acpi platforms. By greping gpio_desc I saw at least these files are > > passing it in platform data structures: > > > > include/sound/soc.h > > include/linux/leds.h > > include/linux/usb/usb_phy_generic.h > > > > There are probable others but I checked those. And of course now there > > is nothing preventing people from adding more of them. > > For sound/soc.h, the member is indeed public but I don't see it being > used to pass descriptors around between drivers. > > For linux/leds.h, I think this is the reason why we introduced > devm_get_gpiod_from_child() > > These looks more like a bad usage of GPIO descriptors, but AFAICT they > can be fixed by fixing the drivers and, by all means, this is where we > should do it. > > This is a different situation from yours where we are trying to deal > with broken firmware and need to resort to tricks at one point or the > other. > > Or am I missing your point? Well, in this case the motivation would be to make it possible to live without platform data with these drivers, but in the end the situation would be exactly the same. We need to give a gpio descriptor to some other device in a driver. So the goal would be that in the actual consumer driver of the gpio we could request it always in one way, ideally always with gpiod_get() call. But if there is no real need in other places for this thing, then let's forget about it. Thanks,
Hi, > 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. Are we considering MFD cases hackish as well? i.e. if we have a driver that needs to register children devices and this driver needs to pass GPIO to a child. Br, David -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 25, 2015 at 5:34 AM, David Cohen <david.a.cohen@linux.intel.com> wrote: > Hi, > >> 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. > > Are we considering MFD cases hackish as well? > i.e. if we have a driver that needs to register children devices and this > driver needs to pass GPIO to a child. In that case wouldn't the GPIO be best defined in the child node itself, for the child device's driver to directly probe? -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 25, 2015 at 10:34:45AM +0900, Alexandre Courbot wrote: > On Wed, Feb 25, 2015 at 5:34 AM, David Cohen > <david.a.cohen@linux.intel.com> wrote: > > Hi, > > > >> 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. > > > > Are we considering MFD cases hackish as well? > > i.e. if we have a driver that needs to register children devices and this > > driver needs to pass GPIO to a child. > > In that case wouldn't the GPIO be best defined in the child node > itself, for the child device's driver to directly probe? In my case [1] I need 2 "virtual devices" (and more in future) to be part of an USB OTG port control. I call it virtual because they are too simple components connected to no bus and controlled by GPIOs: - a fixed regulator controlled by GPIO - a generic mux controlled by GPIO I'd need to request official ACPI HID for them in order to make them self-sufficient. I can go ahead with this approach, but we have many examples of drivers on upstream that are platform driver expecting to receive gpio via platform data (e.g. extcon-gpio). The ACPI table of some products on market were defined following this concept and won't change anymore. Br, David [1] https://lkml.org/lkml/2015/2/19/411 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 25, 2015 at 7:25 PM, David Cohen <david.a.cohen@linux.intel.com> wrote: > In my case [1] I need 2 "virtual devices" (and more in future) to be > part of an USB OTG port control. I call it virtual because they are too > simple components connected to no bus and controlled by GPIOs: > - a fixed regulator controlled by GPIO > - a generic mux controlled by GPIO > > I'd need to request official ACPI HID for them in order to make them > self-sufficient. > > I can go ahead with this approach, but we have many examples of drivers > on upstream that are platform driver expecting to receive gpio via > platform data (e.g. extcon-gpio). The ACPI table of some products on > market were defined following this concept and won't change anymore. So it's not just going to be GPIOs I take it? There is going to be regulator forwarding, clock forwarding, pin control forwarding, IRQ forwarding and DMA channel forwarding etc at the end of the day? I think it's strange that we see this so much, is the real problem that ACPI and the kernel have different ideas of what constitutes a device? And how come the DT seems to be a much better fit and not experience this? Because we haven't had to deal with deployed device trees with resources (GPIOs, regulators, etc) bound to some complex MFD device? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/gpio/board.txt b/Documentation/gpio/board.txt index 4452786..58a0eaf 100644 --- a/Documentation/gpio/board.txt +++ b/Documentation/gpio/board.txt @@ -90,20 +90,17 @@ Note that GPIO_LOOKUP() is just a shortcut to GPIO_LOOKUP_IDX() where idx = 0. A lookup table can then be defined as follows, with an empty entry defining its end: -struct gpiod_lookup_table gpios_table = { - .dev_id = "foo.0", - .table = { - GPIO_LOOKUP_IDX("gpio.0", 15, "led", 0, GPIO_ACTIVE_HIGH), - GPIO_LOOKUP_IDX("gpio.0", 16, "led", 1, GPIO_ACTIVE_HIGH), - GPIO_LOOKUP_IDX("gpio.0", 17, "led", 2, GPIO_ACTIVE_HIGH), - GPIO_LOOKUP("gpio.0", 1, "power", GPIO_ACTIVE_LOW), - { }, - }, +struct gpiod_lookup gpios_table[] = { + GPIO_LOOKUP_IDX("gpio.0", 15, "foo.0", "led", 0, GPIO_ACTIVE_HIGH), + GPIO_LOOKUP_IDX("gpio.0", 16, "foo.0", "led", 1, GPIO_ACTIVE_HIGH), + GPIO_LOOKUP_IDX("gpio.0", 17, "foo.0", "led", 2, GPIO_ACTIVE_HIGH), + GPIO_LOOKUP("gpio.0", 1, "foo.0", "power", GPIO_ACTIVE_LOW), + { }, }; And the table can be added by the board code as follows: - gpiod_add_lookup_table(&gpios_table); + gpiod_add_lookup_table(gpios_table); The driver controlling "foo.0" will then be able to obtain its GPIOs as follows: diff --git a/Documentation/gpio/consumer.txt b/Documentation/gpio/consumer.txt index d85fbae..21b2eee 100644 --- a/Documentation/gpio/consumer.txt +++ b/Documentation/gpio/consumer.txt @@ -281,3 +281,18 @@ descriptor is only possible after the GPIO number has been released. Freeing a GPIO obtained by one API with the other API is forbidden and an unchecked error. + +Runtime GPIO Mappings +===================== +If a GPIO needs to be assigning to consumer when the system is already up and +running, a lookup can be created and later removed with the following +functions: + + int gpiod_create_lookup(struct gpio_desc *desc, const char *con_id, + const char *dev_id) + void gpiod_remove_lookup(struct gpio_desc *desc, const char *con_id, + const char *dev_id) + +Where "dev_id" is the device name of the consumer and "con_id" the connection +identifier that can be used to identify the GPIO when it's requested by that +consumer. diff --git a/arch/arm/mach-integrator/impd1.c b/arch/arm/mach-integrator/impd1.c index 38b0da3..70ff0b5 100644 --- a/arch/arm/mach-integrator/impd1.c +++ b/arch/arm/mach-integrator/impd1.c @@ -386,16 +386,14 @@ static int __init_refok impd1_probe(struct lm_device *dev) /* Add GPIO descriptor lookup table for the PL061 block */ if (idev->offset == 0x00400000) { - struct gpiod_lookup_table *lookup; + struct gpiod_lookup *lookup; char *chipname; char *mmciname; - lookup = devm_kzalloc(&dev->dev, - sizeof(*lookup) + 3 * sizeof(struct gpiod_lookup), + lookup = devm_kzalloc(&dev->dev, sizeof(*lookup) * 3, GFP_KERNEL); chipname = devm_kstrdup(&dev->dev, devname, GFP_KERNEL); mmciname = kasprintf(GFP_KERNEL, "lm%x:00700", dev->id); - lookup->dev_id = mmciname; /* * Offsets on GPIO block 1: * 3 = MMC WP (write protect) @@ -410,13 +408,15 @@ static int __init_refok impd1_probe(struct lm_device *dev) * 5 = Key lower right */ /* We need the two MMCI GPIO entries */ - lookup->table[0].chip_label = chipname; - lookup->table[0].chip_hwnum = 3; - lookup->table[0].con_id = "wp"; - lookup->table[1].chip_label = chipname; - lookup->table[1].chip_hwnum = 4; - lookup->table[1].con_id = "cd"; - lookup->table[1].flags = GPIO_ACTIVE_LOW; + lookup[0].chip_label = chipname; + lookup[0].chip_hwnum = 3; + lookup[0].dev_id = mmciname; + lookup[0].con_id = "wp"; + lookup[1].chip_label = chipname; + lookup[1].chip_hwnum = 4; + lookup[1].dev_id = mmciname; + lookup[1].con_id = "cd"; + lookup[1].flags = GPIO_ACTIVE_LOW; gpiod_add_lookup_table(lookup); } diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c b/arch/arm/mach-omap2/board-rx51-peripherals.c index 14edcd7..606ba16 100644 --- a/arch/arm/mach-omap2/board-rx51-peripherals.c +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c @@ -756,17 +756,14 @@ static struct regulator_init_data rx51_vintdig = { }, }; -static struct gpiod_lookup_table rx51_fmtx_gpios_table = { - .dev_id = "2-0063", - .table = { - GPIO_LOOKUP("gpio.6", 3, "reset", GPIO_ACTIVE_HIGH), /* 163 */ - { }, - }, +static struct gpiod_lookup rx51_fmtx_gpios_table[] = { + GPIO_LOOKUP("gpio.6", 3, "2-0063", "reset", GPIO_ACTIVE_HIGH), /* 163 */ + { }, }; static __init void rx51_gpio_init(void) { - gpiod_add_lookup_table(&rx51_fmtx_gpios_table); + gpiod_add_lookup_table(rx51_fmtx_gpios_table); } static int rx51_twlgpio_setup(struct device *dev, unsigned gpio, unsigned n) diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c index fbe74c6..fedd7d5 100644 --- a/arch/arm/mach-tegra/board-paz00.c +++ b/arch/arm/mach-tegra/board-paz00.c @@ -36,17 +36,14 @@ static struct platform_device wifi_rfkill_device = { }, }; -static struct gpiod_lookup_table wifi_gpio_lookup = { - .dev_id = "rfkill_gpio", - .table = { - GPIO_LOOKUP_IDX("tegra-gpio", 25, NULL, 0, 0), - GPIO_LOOKUP_IDX("tegra-gpio", 85, NULL, 1, 0), - { }, - }, +static struct gpiod_lookup wifi_gpio_lookup[] = { + GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio", NULL, 0, 0), + GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio", NULL, 1, 0), + { }, }; void __init tegra_paz00_wifikill_init(void) { - gpiod_add_lookup_table(&wifi_gpio_lookup); + gpiod_add_lookup_table(wifi_gpio_lookup); platform_device_register(&wifi_rfkill_device); } diff --git a/arch/mips/jz4740/board-qi_lb60.c b/arch/mips/jz4740/board-qi_lb60.c index c454525..7b94feb 100644 --- a/arch/mips/jz4740/board-qi_lb60.c +++ b/arch/mips/jz4740/board-qi_lb60.c @@ -426,13 +426,10 @@ static struct platform_device qi_lb60_audio_device = { .id = -1, }; -static struct gpiod_lookup_table qi_lb60_audio_gpio_table = { - .dev_id = "qi-lb60-audio", - .table = { - GPIO_LOOKUP("Bank B", 29, "snd", 0), - GPIO_LOOKUP("Bank D", 4, "amp", 0), - { }, - }, +static struct gpiod_lookup qi_lb60_audio_gpio_table[] = { + GPIO_LOOKUP("Bank B", 29, "qi-lb60-audio", "snd", 0), + GPIO_LOOKUP("Bank D", 4, "qi-lb60-audio", "amp", 0), + { }, }; static struct platform_device *jz_platform_devices[] __initdata = { @@ -471,7 +468,7 @@ static int __init qi_lb60_init_platform_devices(void) jz4740_adc_device.dev.platform_data = &qi_lb60_battery_pdata; jz4740_mmc_device.dev.platform_data = &qi_lb60_mmc_pdata; - gpiod_add_lookup_table(&qi_lb60_audio_gpio_table); + gpiod_add_lookup_table(qi_lb60_audio_gpio_table); jz4740_serial_device_register(); diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 487afe6..36dc2a9 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1644,14 +1644,62 @@ EXPORT_SYMBOL_GPL(gpiod_set_array_cansleep); * gpiod_add_lookup_table() - register GPIO device consumers * @table: table of consumers to register */ -void gpiod_add_lookup_table(struct gpiod_lookup_table *table) +void gpiod_add_lookup_table(struct gpiod_lookup *table) { mutex_lock(&gpio_lookup_lock); + for ( ; table->chip_label; table++) + list_add_tail(&table->list, &gpio_lookup_list); + mutex_unlock(&gpio_lookup_lock); +} - list_add_tail(&table->list, &gpio_lookup_list); +/** + * gpiod_create_lookup() - register consumer for a GPIO + * @desc: the GPIO + * @con_id: name of the GPIO from the device's point of view + * @dev_id: device name of the consumer for this GPIO + */ +int gpiod_create_lookup(struct gpio_desc *desc, const char *con_id, + const char *dev_id) +{ + struct gpiod_lookup *gl; + + gl = kzalloc(sizeof(*gl), GFP_KERNEL); + if (!gl) + return -ENOMEM; + gl->con_id = con_id; + gl->dev_id = dev_id; + gl->desc = desc; + + mutex_lock(&gpio_lookup_lock); + list_add_tail(&gl->list, &gpio_lookup_list); mutex_unlock(&gpio_lookup_lock); + return 0; } +EXPORT_SYMBOL_GPL(gpiod_create_lookup); + +/** + * gpiod_remove_lookup() - remove GPIO consumer + * @desc: the GPIO + * @con_id: name of the GPIO from the device's point of view + * @dev_id: device name of the consumer for this GPIO + */ +void gpiod_remove_lookup(struct gpio_desc *desc, const char *con_id, + const char *dev_id) +{ + struct gpiod_lookup *gl; + + mutex_lock(&gpio_lookup_lock); + list_for_each_entry(gl, &gpio_lookup_list, list) + if (gl->desc == desc && !strcmp(gl->dev_id, dev_id) && + !strcmp(gl->con_id, con_id)) { + list_del(&gl->list); + kfree(gl); + break; + } + mutex_unlock(&gpio_lookup_lock); +} +EXPORT_SYMBOL_GPL(gpiod_remove_lookup); static struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, unsigned int idx, @@ -1723,52 +1771,22 @@ static struct gpio_desc *acpi_find_gpio(struct device *dev, const char *con_id, return desc; } -static struct gpiod_lookup_table *gpiod_find_lookup_table(struct device *dev) -{ - const char *dev_id = dev ? dev_name(dev) : NULL; - struct gpiod_lookup_table *table; - - mutex_lock(&gpio_lookup_lock); - - list_for_each_entry(table, &gpio_lookup_list, list) { - if (table->dev_id && dev_id) { - /* - * Valid strings on both ends, must be identical to have - * a match - */ - if (!strcmp(table->dev_id, dev_id)) - goto found; - } else { - /* - * One of the pointers is NULL, so both must be to have - * a match - */ - if (dev_id == table->dev_id) - goto found; - } - } - table = NULL; - -found: - mutex_unlock(&gpio_lookup_lock); - return table; -} - static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, unsigned int idx, enum gpio_lookup_flags *flags) { + const char *dev_id = dev ? dev_name(dev) : NULL; struct gpio_desc *desc = ERR_PTR(-ENOENT); - struct gpiod_lookup_table *table; struct gpiod_lookup *p; - table = gpiod_find_lookup_table(dev); - if (!table) - return desc; - - for (p = &table->table[0]; p->chip_label; p++) { + mutex_lock(&gpio_lookup_lock); + list_for_each_entry(p, &gpio_lookup_list, list) { struct gpio_chip *chip; + /* If the lookup entry has a dev_id, require exact match */ + if (p->dev_id && (!dev_id || strcmp(p->dev_id, dev_id))) + continue; + /* idx must always match exactly */ if (p->idx != idx) continue; @@ -1777,27 +1795,33 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, if (p->con_id && (!con_id || strcmp(p->con_id, con_id))) continue; + if (p->desc) { + desc = p->desc; + break; + } + chip = find_chip_by_name(p->chip_label); if (!chip) { dev_err(dev, "cannot find GPIO chip %s\n", p->chip_label); - return ERR_PTR(-ENODEV); + break; } if (chip->ngpio <= p->chip_hwnum) { dev_err(dev, "requested GPIO %d is out of range [0..%d] for chip %s\n", idx, chip->ngpio, chip->label); - return ERR_PTR(-EINVAL); + desc = ERR_PTR(-EINVAL); + break; } desc = gpiochip_get_desc(chip, p->chip_hwnum); *flags = p->flags; - return desc; + break; } - + mutex_unlock(&gpio_lookup_lock); return desc; } diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h index fd85cb1..d6e7579 100644 --- a/include/linux/gpio/consumer.h +++ b/include/linux/gpio/consumer.h @@ -111,6 +111,13 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode, const char *propname); struct gpio_desc *devm_get_gpiod_from_child(struct device *dev, struct fwnode_handle *child); + +/* For passing forward GPIOs */ +int gpiod_create_lookup(struct gpio_desc *desc, const char *con_id, + const char *dev_id); +void gpiod_remove_lookup(struct gpio_desc *desc, const char *con_id, + const char *dev_id); + #else /* CONFIG_GPIOLIB */ static inline struct gpio_desc *__must_check __gpiod_get(struct device *dev, @@ -329,6 +336,21 @@ static inline int desc_to_gpio(const struct gpio_desc *desc) return -EINVAL; } +static inline int gpiod_create_lookup(struct gpio_desc *desc, + const char *con_id, const char *dev_id) +{ + /* GPIO can never have been requested */ + WARN_ON(1); + return -ENOSYS; +} + +static inline void gpiod_remove_lookup(struct gpio_desc *desc, + const char *con_id, const char *dev_id) +{ + /* GPIO can never have been requested */ + WARN_ON(1); +} + #endif /* CONFIG_GPIOLIB */ /* diff --git a/include/linux/gpio/machine.h b/include/linux/gpio/machine.h index e270614..b367a19 100644 --- a/include/linux/gpio/machine.h +++ b/include/linux/gpio/machine.h @@ -15,6 +15,7 @@ enum gpio_lookup_flags { * struct gpiod_lookup - lookup table * @chip_label: name of the chip the GPIO belongs to * @chip_hwnum: hardware number (i.e. relative to the chip) of the GPIO + * @dev_id: name of the consumer device for this GPIO * @con_id: name of the GPIO from the device's point of view * @idx: index of the GPIO in case several GPIOs share the same name * @flags: mask of GPIO_* values @@ -23,39 +24,38 @@ enum gpio_lookup_flags { * functions using platform data. */ struct gpiod_lookup { + struct list_head list; const char *chip_label; u16 chip_hwnum; + const char *dev_id; const char *con_id; unsigned int idx; enum gpio_lookup_flags flags; -}; - -struct gpiod_lookup_table { - struct list_head list; - const char *dev_id; - struct gpiod_lookup table[]; + void *desc; }; /* * Simple definition of a single GPIO under a con_id */ -#define GPIO_LOOKUP(_chip_label, _chip_hwnum, _con_id, _flags) \ - GPIO_LOOKUP_IDX(_chip_label, _chip_hwnum, _con_id, 0, _flags) +#define GPIO_LOOKUP(_chip_label, _chip_hwnum, _dev_id, _con_id, _flags) \ + GPIO_LOOKUP_IDX(_chip_label, _chip_hwnum, _dev_id, _con_id, 0, _flags) /* * Use this macro if you need to have several GPIOs under the same con_id. * Each GPIO needs to use a different index and can be accessed using * gpiod_get_index() */ -#define GPIO_LOOKUP_IDX(_chip_label, _chip_hwnum, _con_id, _idx, _flags) \ +#define GPIO_LOOKUP_IDX(_chip_label, _chip_hwnum, _dev_id, _con_id, _idx, \ + _flags) \ { \ .chip_label = _chip_label, \ .chip_hwnum = _chip_hwnum, \ + .dev_id = _dev_id, \ .con_id = _con_id, \ .idx = _idx, \ .flags = _flags, \ } -void gpiod_add_lookup_table(struct gpiod_lookup_table *table); +void gpiod_add_lookup_table(struct gpiod_lookup *table); #endif /* __LINUX_GPIO_MACHINE_H */
This makes it possible to assign GPIOs at runtime. The motivation for it is because of need to forward GPIOs from one device to an other. That feature may be useful for example with some mfd devices, but initially it is needed because on some Intel Braswell based ACPI platforms the GPIO resources controlling signals of the USB PHY are given to the controller device object for whatever reason, so the driver of that controller needs be able to pass them to the PHY device somehow. So basically this is meant to allow patching of bad (bad from Linux kernels point of view) hardware descriptions provided by system fw in the drivers. Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> --- Hi, I'm sending this first as a RFC in case you guys have some better idea how to solve our problem. I actually already have couple platforms where the GPIO resources are given to the "wrong" device objects now. Originally we were thinking about simply handling our problem with hacks to the PHY drivers, basically also checking if the parent has GPIOs. That would only work if the controller is always the parent, which it's not, but even if it was, it would be too risky. The PHY drivers don't know which controller they are attached to, so what is to say that the GPIOs aren't really attached to the controller. So the safest way to handle our problem is to deal with it in quirks in the controller drivers. Solving it by bouncing the GPIOs did not feel ideal there doesn't seem to be any other way. The API is copied from clkdev (clk_register_clkdev). In the end it's really only one function for adding the lookups and one for removing them. The way it's implemented is by modifying the current style of handling the lookups a little bit. The per device lookup table is basically reverted back to the single linked-list format since it seems that these lookups may have to be assigned from several sources. Thanks, Documentation/gpio/board.txt | 17 ++--- Documentation/gpio/consumer.txt | 15 ++++ arch/arm/mach-integrator/impd1.c | 22 +++--- arch/arm/mach-omap2/board-rx51-peripherals.c | 11 +-- arch/arm/mach-tegra/board-paz00.c | 13 ++-- arch/mips/jz4740/board-qi_lb60.c | 13 ++-- drivers/gpio/gpiolib.c | 110 ++++++++++++++++----------- include/linux/gpio/consumer.h | 22 ++++++ include/linux/gpio/machine.h | 20 ++--- 9 files changed, 146 insertions(+), 97 deletions(-)