Message ID | 1329719263-18971-1-git-send-email-swarren@nvidia.com |
---|---|
State | Superseded, archived |
Headers | show |
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-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 20, 2012 at 8:39 AM, Russell King - ARM Linux <linux@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-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 20, 2012 at 7:27 AM, Stephen Warren <swarren@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 <swarren@nvidia.com> Acked-by: Linus Walleij <linus.walleij@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-tegra" 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 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-tegra" 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 21, 2012 at 12:06 PM, Russell King - ARM Linux <linux@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*. I had this similar example where a pin on U300 could be in GPIO mode and "spy" on the peripheral using that pin, by just sampling the value or triggering on edge transitions. Since each pin has a pin descriptor, we could solve this by marking pins as "gpio dualmode", i.e. add some bool dualmode to the descriptor and augment the core to allow pins with this property to be in GPIO and "for device X" mode at the same time. So in pin_request() in pinmux.c we check for this property and allow dual-mode on a case-by-case basis (assuming this will be pretty rare, so the default is not to use it). 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.) > 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. This sounds more mutually exclusive and should be possible to support with a get/enable/disable/put sequence AFAICT. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" 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 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-tegra" 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 21, 2012 at 12:44:09PM +0000, Russell King - ARM Linux wrote: > 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. I should have added - the only places which directly accesses one of the GPDR/GPSR/GPCR registers are: drivers/pcmcia/sa1100_shannon.c: unsigned long levels = GPLR; drivers/video/sa1100fb.c: GPDR |= mask; drivers/input/touchscreen/jornada720_ts.c: if (GPLR & GPIO_GPIO(9)) { drivers/input/touchscreen/h3600_ts_input.c: int down = (GPLR & GPIO_BITSY_ACTION_BUTTON) ? 0 : 1; drivers/input/touchscreen/h3600_ts_input.c: int down = (GPLR & GPIO_BITSY_NPOWER_BUTTON) ? 0 : 1; The shannon thing looks like a bug in my PCMCIA patch series - as soc_common now deals with GPIOs itself (which I've now fixed.) The sa1100fb thing is a case of what I described above (correctly configuring the direction for the pins for the special function in use.) The touchscreen stuff needs someone who knows that stuff to fix it - I think the jornada folk have been around recently so maybe they can look at their driver. The h3600 ts stuff also looks fairly easy to convert to gpiolib if someone has the time. Again, maybe if there's an interested party with a device that they could test, it could happen. All other cases of direct GPDR/GPLR/GPSR/GPCR access are in platform initialization code in arch/arm/mach-sa1100. So, we're actually very close to having sa11x0 fully converted to gpiolib. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" 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 21, 2012 at 1:44 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: >> (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. Excellent! > 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. It's quite common I think, many platforms have an intimate relation between GPIO and muxes/altfunctions. For example it is common that the hardware engineer doesn't helpfully enable on-die pull-ups on the I2C bus even though the I2C block is muxed in, you have to go in and set the pull-up bits separately from muxing the I2C in... Basically it's expected from a generic pad I/O cell being arrayed into a GPIO block to expose these things in the same set of registers. I made some presentation last week partly describing how some hardware engineers I know go about creating GPIO controllers from simpler I/O pad cells: http://www.df.lth.se/~triad/papers/pincontrol.pdf Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Linus Walleij wrote at Tuesday, February 21, 2012 3:42 AM: > On Mon, Feb 20, 2012 at 8:39 AM, Russell King - ARM Linux > <linux@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"); ...
Linus Walleij wrote at Tuesday, February 21, 2012 5:40 AM: > On Tue, Feb 21, 2012 at 12:06 PM, Russell King - ARM Linux > <linux@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 GPIO signals, e.g. 1 from each of 2 different GPIO HW modules, with different capabilities, or where 1 GPIO could be routed to one of 2 or more pins. In other words, make "gpio" an explicit mux function, thus allowing pinctrl_select_state() to directly transition between special function and GPIO usage, rather than having separate pinctrl and GPIO APIs for this. This way would entail dropping my Documentation/gpio.txt patch completely, I think. If we do this though, I think we'd want to extend pinctrl to allow muxing on pins too, so that we don't need to create a pin group for every pin to support per-pin GPIO ownership. This would also help out all the HW that already muxes everything per-pin, since they need to create these single- pin groups right now... I somewhat anticpated this in my naming of the mapping table convenience macros in the most recent pinctrl patch I posted to add pin config mapping table entries. Thoughts?
On Tue, Feb 21, 2012 at 8:14 PM, Stephen Warren <swarren@nvidia.com> wrote: > 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? I was thinking about making this a property of the physical pin i.e. struct pin_desc and not of the board data. It seems to me like this arbitration has to be very close to the driver, since not all or even many controllers will support it (AFAICT). > 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. In this case I would strive not to present unused pins to the functions in the first place. But maybe this collides with the new paradigm to assign aquire all possible pins on pinctrl_get() time? > 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? I think on U300 and SA1100 we can flag that in the pin descriptor as described above. > 2) Extend the pinctrl mapping table to explicitly represent GPIO usage. Seems like it could get complicated. Since the only hardwares we have that can do this would be fine with having it in their pin descriptor we can do it there? > 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 GPIO > signals, e.g. 1 from each of 2 different GPIO HW modules, with different > capabilities, or where 1 GPIO could be routed to one of 2 or more pins. > > In other words, make "gpio" an explicit mux function, thus allowing > pinctrl_select_state() to directly transition between special function > and GPIO usage, rather than having separate pinctrl and GPIO APIs for > this. This way would entail dropping my Documentation/gpio.txt patch > completely, I think. > > If we do this though, I think we'd want to extend pinctrl to allow muxing > on pins too, so that we don't need to create a pin group for every pin to > support per-pin GPIO ownership. This would also help out all the HW that > already muxes everything per-pin, since they need to create these single- > pin groups right now... I somewhat anticpated this in my naming of the > mapping table convenience macros in the most recent pinctrl patch I posted > to add pin config mapping table entries. Hm I think Tony also mentioned that he wanted to allow muxing of single pins from the mapping sometime so I sort of like the idea. Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Linus Walleij wrote at Tuesday, February 21, 2012 11:01 PM: > On Tue, Feb 21, 2012 at 8:14 PM, Stephen Warren <swarren@nvidia.com> wrote: > > 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? > > I was thinking about making this a property of the physical pin i.e. > struct pin_desc and not of the board data. > > It seems to me like this arbitration has to be very close to the driver, > since not all or even many controllers will support it (AFAICT). Well, there are two aspects: a) Can the pin controller do it, which is something per-SoC/driver? b) Does it make sense for the board, given what each pin is connected to? So unless we decide to ignore (b) (which may be perfectly fine but as I mentioned I'd just like to explicitly decide this), there needs to be some per-SoC way of representing this capability, /and/ some per-board way. > > 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. > > In this case I would strive not to present unused pins to the functions > in the first place. But maybe this collides with the new paradigm to > assign aquire all possible pins on pinctrl_get() time? The pins are part of the pin group in HW. It doesn't make sense to say that they aren't, and indeed the SoC driver has no board knowledge and couldn't exclude them anyway.
On Tue, 21 Feb 2012 11:46:03 +0100, Linus Walleij <linus.walleij@linaro.org> wrote: > On Mon, Feb 20, 2012 at 7:27 AM, Stephen Warren <swarren@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 <swarren@nvidia.com> > > Acked-by: Linus Walleij <linus.walleij@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-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/12/2012 10:32 AM, Grant Likely wrote: > On Tue, 21 Feb 2012 11:46:03 +0100, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Mon, Feb 20, 2012 at 7:27 AM, Stephen Warren <swarren@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 <swarren@nvidia.com> >> >> Acked-by: Linus Walleij <linus.walleij@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-tegra" 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.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.
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 <swarren@nvidia.com> --- Documentation/gpio.txt | 22 +++++++++++++++++++--- 1 files changed, 19 insertions(+), 3 deletions(-)