Re: [PATCH 1/2] Documentation/gpio.txt: Explain expected pinctrl interaction
On Tue, 21 Feb 2012 11:46:03 +0100, Linus Walleij linus.wall...@linaro.org wrote: On Mon, Feb 20, 2012 at 7:27 AM, Stephen Warren swar...@nvidia.com wrote: Update gpio.txt based on recent discussions regarding interaction with the pinctrl subsystem. Previously, gpio_request() was described as explicitly not performing any required mux setup operations etc. Now, gpio_request() is explicitly as explicitly performing any required mux setup operations where possible. In the case it isn't, platform code is required to have set up any required muxing or other configuration prior to gpio_request() being called, in order to maintain the same semantics. This is achieved by gpiolib drivers calling e.g. pinctrl_request_gpio() in their .request() operation. Signed-off-by: Stephen Warren swar...@nvidia.com Acked-by: Linus Walleij linus.wall...@linaro.org Grant can you take this one? I'd prefer for you to have a look at it as well. I've taken this one, but left the 2nd for Olof. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Documentation/gpio.txt: Explain expected pinctrl interaction
On 03/12/2012 10:32 AM, Grant Likely wrote: On Tue, 21 Feb 2012 11:46:03 +0100, Linus Walleij linus.wall...@linaro.org wrote: On Mon, Feb 20, 2012 at 7:27 AM, Stephen Warren swar...@nvidia.com wrote: Update gpio.txt based on recent discussions regarding interaction with the pinctrl subsystem. Previously, gpio_request() was described as explicitly not performing any required mux setup operations etc. Now, gpio_request() is explicitly as explicitly performing any required mux setup operations where possible. In the case it isn't, platform code is required to have set up any required muxing or other configuration prior to gpio_request() being called, in order to maintain the same semantics. This is achieved by gpiolib drivers calling e.g. pinctrl_request_gpio() in their .request() operation. Signed-off-by: Stephen Warren swar...@nvidia.com Acked-by: Linus Walleij linus.wall...@linaro.org Grant can you take this one? I'd prefer for you to have a look at it as well. I've taken this one, but left the 2nd for Olof. Grant, did you take V2 of this patch? I assume not since you replied to V1. For reference, that's: http://lkml.org/lkml/2012/3/5/533 Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Documentation/gpio.txt: Explain expected pinctrl interaction
On Mon, Feb 20, 2012 at 8:39 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Sun, Feb 19, 2012 at 11:27:42PM -0700, Stephen Warren wrote: Update gpio.txt based on recent discussions regarding interaction with the pinctrl subsystem. Previously, gpio_request() was described as explicitly not performing any required mux setup operations etc. Now, gpio_request() is explicitly as explicitly performing any required mux setup operations where possible. In the case it isn't, platform code is required to have set up any required muxing or other configuration prior to gpio_request() being called, in order to maintain the same semantics. So what if you need to have the pin as a GPIO, manipulate it as a GPIO, and then hand it off to a special function, and then take it back as a GPIO before you shut the special function down ? I remember this case very well and we designed for it, so it should be handled by pin control and GPIO thusly: Example: use pins 1,2 as I2C, then convert them to GPIO for a while then back again: // This call looks up a map containing pins 1,2 and reserve them p = pinctrl_get(dev, i2c); if (IS_ERR(p)) ... pinctrl_enable(p); pinctrl_disable(p); // This will free up the pins again pinctrl_put(p); // So now we can do this... // NB: the GPIO driver calls pinctr_request_gpio() to check // that it can take these pins gpio_request(1, gpio1): gpio_request(2, gpio2); // This will trigger a reset or something gpio_direction_output(1, 1); gpio_direction_output(2, 1); // Release pins again gpio_free(1); gpio_free(2); // Take them back for this function p = pinctrl_get(dev, i2c); It's a bit kludgy but works and makes sure the pins are only used for one thing at a time. BTW: Russell, which specific platform and driver was it that had this usecase? I'd like to have a look at the code to educate myself. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Documentation/gpio.txt: Explain expected pinctrl interaction
On Mon, Feb 20, 2012 at 7:27 AM, Stephen Warren swar...@nvidia.com wrote: Update gpio.txt based on recent discussions regarding interaction with the pinctrl subsystem. Previously, gpio_request() was described as explicitly not performing any required mux setup operations etc. Now, gpio_request() is explicitly as explicitly performing any required mux setup operations where possible. In the case it isn't, platform code is required to have set up any required muxing or other configuration prior to gpio_request() being called, in order to maintain the same semantics. This is achieved by gpiolib drivers calling e.g. pinctrl_request_gpio() in their .request() operation. Signed-off-by: Stephen Warren swar...@nvidia.com Acked-by: Linus Walleij linus.wall...@linaro.org Grant can you take this one? I'd prefer for you to have a look at it as well. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Documentation/gpio.txt: Explain expected pinctrl interaction
On Tue, Feb 21, 2012 at 11:41:31AM +0100, Linus Walleij wrote: I remember this case very well and we designed for it, so it should be handled by pin control and GPIO thusly: Example: use pins 1,2 as I2C, then convert them to GPIO for a while then back again: // This call looks up a map containing pins 1,2 and reserve them p = pinctrl_get(dev, i2c); if (IS_ERR(p)) ... pinctrl_enable(p); pinctrl_disable(p); // This will free up the pins again pinctrl_put(p); // So now we can do this... // NB: the GPIO driver calls pinctr_request_gpio() to check // that it can take these pins gpio_request(1, gpio1): gpio_request(2, gpio2); // This will trigger a reset or something gpio_direction_output(1, 1); gpio_direction_output(2, 1); // Release pins again gpio_free(1); gpio_free(2); // Take them back for this function p = pinctrl_get(dev, i2c); It's a bit kludgy but works and makes sure the pins are only used for one thing at a time. No. The case which I'm thinking of is for the Assabet audio, where we have the following situation. We have the SSP bus [TXD,RXD,SFRM,SCLK] which is handled by the SSP core. This is connected through a FPGA to a UDA1341 codec. The UDA1341 codec has DI,DO,WS,BCLK signals. WS (which is effectively the LRCK and therefore identifies the left and right channels) is connected through a flip-flop in the FPGA which toggles at every SFRM pulse. WS is held high for the left sample. This is the default setting. Unfortunately, some FPGA builds out there do not tristate the WS output when the codec is powered off. This leads to a high leakage current from the FPGA, into the UDA1341 codec and into other devices. The solution is to wait for SSP to finish, and while SSP is still enabled, switch the pins from the SSP block to the GPIO block and toggle the SFRM signal to flip the WS output before removing power. When starting up, with the pins under control of GPIO, power up and them toggle hte SFRM signal to restore the WS state before giving it back to the SSP block. However, here's the thing: SSP must be enabled at the point in time when the pins are given or taken away from it, otherwise SFRM is indeterminant. We can't allow gpio_request() to fail at the points where we toggle this pin - failure is not really an option where causing hardware stress is involved. We can't allow the GPIO block to have the state of these pins change state while the pins are given back to the GPIO block before the GPIOs have been requested (glitches on the SFRM line will cause the state of the FIFO to change.) Another case where this kind of run-time switching needs to occur is the PXA IrDA driver, where the pins need to be switched between their FIR and SIR modes. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Documentation/gpio.txt: Explain expected pinctrl interaction
On Tue, Feb 21, 2012 at 01:40:05PM +0100, Linus Walleij wrote: Of course it assumes the SA1100 being converted to use pin control, I looked at it a bit and it seems simple enough since the GAFR register is a single GPIO or something else-switch for the GPIOs. (It'd probably need the SA1100 to be a bit more strict in using gpiolib in place for the direct assignments though, else the abstractions get a bit pointless anyway.) That's mostly happened through my recent set of 100 or so patches. There's a few areas where that's not quite as easy as it should be, but on the whole, it's mostly complete. The other thing I forgot to mention, and I suspect it's particular to SA11x0, is that the GPDR must be set correctly according to the special function as well as GAFR. So, if a special function involves driving a pin, the pin must be set as an output in GPDR. Conversely, if the special function involves input only, the pin must be set as an input in GPDR. So, on SA11x0, gpio and pin configuration are intimately linked. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] Documentation/gpio.txt: Explain expected pinctrl interaction
Linus Walleij wrote at Tuesday, February 21, 2012 3:42 AM: On Mon, Feb 20, 2012 at 8:39 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Sun, Feb 19, 2012 at 11:27:42PM -0700, Stephen Warren wrote: Update gpio.txt based on recent discussions regarding interaction with the pinctrl subsystem. Previously, gpio_request() was described as explicitly not performing any required mux setup operations etc. Now, gpio_request() is explicitly as explicitly performing any required mux setup operations where possible. In the case it isn't, platform code is required to have set up any required muxing or other configuration prior to gpio_request() being called, in order to maintain the same semantics. So what if you need to have the pin as a GPIO, manipulate it as a GPIO, and then hand it off to a special function, and then take it back as a GPIO before you shut the special function down ? I remember this case very well and we designed for it, so it should be handled by pin control and GPIO thusly: Example: use pins 1,2 as I2C, then convert them to GPIO for a while then back again: The code below doesn't necessarily do exactly what Russell needs... // This call looks up a map containing pins 1,2 and reserve them p = pinctrl_get(dev, i2c); if (IS_ERR(p)) ... pinctrl_enable(p); pinctrl_disable(p); Here, the pinctrl core will call the pinctrl driver's pinmux_ops.free() function. The whole point of this function is to change the pin's mux selection to something that is guaranteed not to conflict with any other pin's mux setting. Now, if the HW only allows each signal to be routed to a specific pin (or not routed), then free() might be a no-op. However, if the HW allows the I2C module's signals to be routed to pins A+B or X+Y, then free() on pins A/B would need to reprogram the mux to route something else to pins A+B (or disable those pins or whatever the HW needs) so that if I2C was later selected on pins X+Y, there would be no conflict; it wouldn't be the case that both pins A+B and X+Y had I2C mux'd on to them. Hence, in general at least, pinctrl_disable() might glitch the output signals. As far as how to fix this; see my future responses to your future emails :-) // This will free up the pins again pinctrl_put(p); // So now we can do this... // NB: the GPIO driver calls pinctr_request_gpio() to check // that it can take these pins gpio_request(1, gpio1): gpio_request(2, gpio2); // This will trigger a reset or something gpio_direction_output(1, 1); gpio_direction_output(2, 1); // Release pins again gpio_free(1); gpio_free(2); // Take them back for this function p = pinctrl_get(dev, i2c); ... -- nvpublic -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] Documentation/gpio.txt: Explain expected pinctrl interaction
Linus Walleij wrote at Tuesday, February 21, 2012 5:40 AM:
On Tue, Feb 21, 2012 at 12:06 PM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
It's a bit kludgy but works and makes sure the pins are only used
for one thing at a time.
No. The case which I'm thinking of is for the Assabet audio, where
we have the following situation.
OK... (thanks for the explanation!)
The solution is to wait for SSP to finish, and while SSP is still enabled,
switch the pins from the SSP block to the GPIO block and toggle the SFRM
signal to flip the WS output before removing power. When starting up,
with the pins under control of GPIO, power up and them toggle hte SFRM
signal to restore the WS state before giving it back to the SSP block.
However, here's the thing: SSP must be enabled at the point in time when
the pins are given or taken away from it, otherwise SFRM is indeterminant.
We can't allow gpio_request() to fail at the points where we toggle this
pin - failure is not really an option where causing hardware stress is
involved. We can't allow the GPIO block to have the state of these pins
change state while the pins are given back to the GPIO block before the
GPIOs have been requested (glitches on the SFRM line will cause the state
of the FIFO to change.)
This can probably be done, mainly I'd say that control over the pins
need not mean that they are decoupled from some specific hardware, or
that the hardware is disabled or so, the semantics of doing say
pinctrl_get/enable/disable/put aren't set in stone, it's just callbacks into
the device driver so it could avoid doing anything in this case, keep
the pins muxed for the SSP if the transition goes from SSP-GPIO
and back.
However I guess it would be conceptually closer if say the pins could
be used for a device and GPIO at the *same time*.
If we allowed that, it would also solve the following comment in patch
2 in this series, in the Tegra GPIO driver:
int tegra_gpio_request(struct gpio_chip *chip, unsigned offset)
{
/*
* We should call pinctrl_request_gpio() here. However, we can't. Tegra
* muxes pins in groups not individually, and real boards exist where
* most of a pin group needs to be used for some function, yet some of
* the pins aren't actually used by that function in the mode the
* controller operates on the board, and so those pins can be used as
* GPIOs.
*
* This is true in practice on Seaboard/Springbank, where pin group ATC
* is used by the NAND controller, but pin GMI_IORDY_PI5 is used as a
* GPIO for the SD card slot's CD signal.
*
* On Tegra30, pins are muxed in groups and hence we could call
* pinctrl here, but we don't for now.
*/
tegra_gpio_mask_write(GPIO_MSK_CNF(offset), offset, 1);
return 0;
}
i.e. we actually /could/ call pinctrl_request_gpio() here.
And in turn, the I2C driver (in your example) and SPI driver (in Russell's)
would never have to call pinctrl_disable() and hence the issue I raised
in my previous email would not occur.
The one downside here:
Ignoring WARs like we're discussing, it's typically true that a given pin
should either be a special function or a GPIO for any given board. If
we do allow a pin to be owned/used by both, then how do we indicate, on
a per-board rather than per-SoC basis, which pins we should allow both
gpio_request() and pinmux usage? The following considerations exist:
a) On Tegra, a pin group might include 10 pins that are mux'd as a group,
hence all owned e.g. by a NAND driver. If a few of those aren't used on a
particular board due to the way the NAND is hooked up and the driver
configured, do we only allow gpio_request() only on those pins we know the
NAND driver isn't actually using, to prevent someone using unexpected
pins as GPIO? We'd need a per-GPIO per-board way to represent this if
we care about this level of error-checking.
b) In Linus's snooping example, how do we know the SoC can physically
implement enabling a pin as both a GPIO and a special function, such
that the gpio_request() for the snooping won't interfere with the mux'd
function? On Tegra, any GPIO usage is mutually exclusive with mux usage;
enabling GPIO even for input, disconnects the input side of the pin from
any HW module other than GPIO irrespective of mux function. IIRC,
somewhere some per-pin data was mentioned for this.
We could either:
1) Just ignore these issues, and allow both mux and GPIO ownership at
once of any pin, and leave it up to platform data or device tree authors
to give the correct GPIO IDs to drivers. This is possibly reasonable, but
I just want us to make this a conscious decision.
2) Extend the pinctrl mapping table to explicitly represent GPIO usage.
This is required anyway for HW where there isn't a 1:1 mapping between
GPIO ID and pin ID, e.g. 1 pin could be used for two different
[PATCH 1/2] Documentation/gpio.txt: Explain expected pinctrl interaction
Update gpio.txt based on recent discussions regarding interaction with the pinctrl subsystem. Previously, gpio_request() was described as explicitly not performing any required mux setup operations etc. Now, gpio_request() is explicitly as explicitly performing any required mux setup operations where possible. In the case it isn't, platform code is required to have set up any required muxing or other configuration prior to gpio_request() being called, in order to maintain the same semantics. This is achieved by gpiolib drivers calling e.g. pinctrl_request_gpio() in their .request() operation. Signed-off-by: Stephen Warren swar...@nvidia.com --- Documentation/gpio.txt | 22 +++--- 1 files changed, 19 insertions(+), 3 deletions(-) diff --git a/Documentation/gpio.txt b/Documentation/gpio.txt index 792faa3..3c4a702 100644 --- a/Documentation/gpio.txt +++ b/Documentation/gpio.txt @@ -271,9 +271,25 @@ Some platforms may also use knowledge about what GPIOs are active for power management, such as by powering down unused chip sectors and, more easily, gating off unused clocks. -Note that requesting a GPIO does NOT cause it to be configured in any -way; it just marks that GPIO as in use. Separate code must handle any -pin setup (e.g. controlling which pin the GPIO uses, pullup/pulldown). +Requesting a GPIO should cause any pin multiplexing hardware to be programmed +to route the GPIO signal to the appropriate pin. In some cases, this requires +programming separate pin multiplexing hardware outside the GPIO controller. +Equally, for GPIOs that use pins known to the pinctrl subsystem, that +subsystem should be informed of their use. These requirements may be satisfied +by having a gpiolib driver's .request() operation call pinctrl_request_gpio(). +Similarly, a gpiolib driver's .free() operation may call pinctrl_free_gpio(). + +Some platforms allow some or all GPIO signals to be routed to different pins. +Similarly, other aspects of the GPIO or pin may need to be configured, such as +pullup/pulldown. Platform software should arrange that any such details are +configured priored to gpio_request() being called for those GPIOs, such that +GPIO users need not be aware of these details. + +gpiolib drivers may need to call additional pinctrl APIs to implement certain +operations. This would be the case if e.g. GPIO input/output value is +controlled by a GPIO HW module, whereas GPIO direction is controlled by a +separate pin controller HW module. Functions pinctrl_gpio_direction_input() +and pinctrl_gpio_direction_output() may be used to implement this. Also note that it's your responsibility to have stopped using a GPIO before you free it. -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Documentation/gpio.txt: Explain expected pinctrl interaction
On Sun, Feb 19, 2012 at 11:27:42PM -0700, Stephen Warren wrote: Update gpio.txt based on recent discussions regarding interaction with the pinctrl subsystem. Previously, gpio_request() was described as explicitly not performing any required mux setup operations etc. Now, gpio_request() is explicitly as explicitly performing any required mux setup operations where possible. In the case it isn't, platform code is required to have set up any required muxing or other configuration prior to gpio_request() being called, in order to maintain the same semantics. So what if you need to have the pin as a GPIO, manipulate it as a GPIO, and then hand it off to a special function, and then take it back as a GPIO before you shut the special function down ? -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
