* [PATCH 1/2] ARM: Tegra: Harmony: Register and configure WM8903 IRQ GPIO @ 2011-07-13 20:40 ` Stephen Warren 0 siblings, 0 replies; 12+ messages in thread From: Stephen Warren @ 2011-07-13 20:40 UTC (permalink / raw) To: Colin Cross, Erik Gilling, Olof Johansson Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Stephen Warren Technically, we need to request and configure the GPIO used as the WM8903 interrupt. This prevents conflicting registrations, and assures that the GPIO is correctly configured in all cases, e.g. if the bootloader left the GPIO in some unexpected state. In practice, the previous code works as-is, at least when using ChromeOS's U-Boot as the boot-loader. Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> --- arch/arm/mach-tegra/board-harmony.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-tegra/board-harmony.c b/arch/arm/mach-tegra/board-harmony.c index 846cd7d..e4548a5 100644 --- a/arch/arm/mach-tegra/board-harmony.c +++ b/arch/arm/mach-tegra/board-harmony.c @@ -109,6 +109,9 @@ static void __init harmony_i2c_init(void) platform_device_register(&tegra_i2c_device3); platform_device_register(&tegra_i2c_device4); + gpio_request(TEGRA_GPIO_CDC_IRQ, "wm8903"); + gpio_direction_input(TEGRA_GPIO_CDC_IRQ); + i2c_register_board_info(0, &wm8903_board_info, 1); } -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 1/2] ARM: Tegra: Harmony: Register and configure WM8903 IRQ GPIO @ 2011-07-13 20:40 ` Stephen Warren 0 siblings, 0 replies; 12+ messages in thread From: Stephen Warren @ 2011-07-13 20:40 UTC (permalink / raw) To: linux-arm-kernel Technically, we need to request and configure the GPIO used as the WM8903 interrupt. This prevents conflicting registrations, and assures that the GPIO is correctly configured in all cases, e.g. if the bootloader left the GPIO in some unexpected state. In practice, the previous code works as-is, at least when using ChromeOS's U-Boot as the boot-loader. Signed-off-by: Stephen Warren <swarren@nvidia.com> --- arch/arm/mach-tegra/board-harmony.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-tegra/board-harmony.c b/arch/arm/mach-tegra/board-harmony.c index 846cd7d..e4548a5 100644 --- a/arch/arm/mach-tegra/board-harmony.c +++ b/arch/arm/mach-tegra/board-harmony.c @@ -109,6 +109,9 @@ static void __init harmony_i2c_init(void) platform_device_register(&tegra_i2c_device3); platform_device_register(&tegra_i2c_device4); + gpio_request(TEGRA_GPIO_CDC_IRQ, "wm8903"); + gpio_direction_input(TEGRA_GPIO_CDC_IRQ); + i2c_register_board_info(0, &wm8903_board_info, 1); } -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
[parent not found: <1310589618-16080-1-git-send-email-swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]
* [PATCH 2/2] ARM: Tegra: Harmony: Add USB device 2011-07-13 20:40 ` Stephen Warren @ 2011-07-13 20:40 ` Stephen Warren -1 siblings, 0 replies; 12+ messages in thread From: Stephen Warren @ 2011-07-13 20:40 UTC (permalink / raw) To: Colin Cross, Erik Gilling, Olof Johansson Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Stephen Warren, Olof Johansson The Tegra USB port is attached an an SMSC9514; a combination USB hub and Ethernet controller. This change is extracted from a change in the ChromeOS 2.6.38 kernel. Signed-off-by: Olof Johansson <olofj-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> --- arch/arm/mach-tegra/board-harmony.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-tegra/board-harmony.c b/arch/arm/mach-tegra/board-harmony.c index e4548a5..ecad7cb 100644 --- a/arch/arm/mach-tegra/board-harmony.c +++ b/arch/arm/mach-tegra/board-harmony.c @@ -120,6 +120,7 @@ static struct platform_device *harmony_devices[] __initdata = { &tegra_sdhci_device1, &tegra_sdhci_device2, &tegra_sdhci_device4, + &tegra_ehci3_device, &tegra_i2s_device1, &tegra_das_device, &tegra_pcm_device, @@ -143,6 +144,7 @@ static __initdata struct tegra_clk_init_table harmony_clk_init_table[] = { { "pll_a_out0", "pll_a", 11289600, true }, { "cdev1", NULL, 0, true }, { "i2s1", "pll_a_out0", 11289600, false}, + { "usb3", "clk_m", 12000000, true }, { NULL, NULL, 0, 0}, }; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] ARM: Tegra: Harmony: Add USB device @ 2011-07-13 20:40 ` Stephen Warren 0 siblings, 0 replies; 12+ messages in thread From: Stephen Warren @ 2011-07-13 20:40 UTC (permalink / raw) To: linux-arm-kernel The Tegra USB port is attached an an SMSC9514; a combination USB hub and Ethernet controller. This change is extracted from a change in the ChromeOS 2.6.38 kernel. Signed-off-by: Olof Johansson <olofj@chromium.org> Signed-off-by: Stephen Warren <swarren@nvidia.com> --- arch/arm/mach-tegra/board-harmony.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-tegra/board-harmony.c b/arch/arm/mach-tegra/board-harmony.c index e4548a5..ecad7cb 100644 --- a/arch/arm/mach-tegra/board-harmony.c +++ b/arch/arm/mach-tegra/board-harmony.c @@ -120,6 +120,7 @@ static struct platform_device *harmony_devices[] __initdata = { &tegra_sdhci_device1, &tegra_sdhci_device2, &tegra_sdhci_device4, + &tegra_ehci3_device, &tegra_i2s_device1, &tegra_das_device, &tegra_pcm_device, @@ -143,6 +144,7 @@ static __initdata struct tegra_clk_init_table harmony_clk_init_table[] = { { "pll_a_out0", "pll_a", 11289600, true }, { "cdev1", NULL, 0, true }, { "i2s1", "pll_a_out0", 11289600, false}, + { "usb3", "clk_m", 12000000, true }, { NULL, NULL, 0, 0}, }; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] ARM: Tegra: Harmony: Register and configure WM8903 IRQ GPIO 2011-07-13 20:40 ` Stephen Warren @ 2011-07-14 12:25 ` Mark Brown -1 siblings, 0 replies; 12+ messages in thread From: Mark Brown @ 2011-07-14 12:25 UTC (permalink / raw) To: Stephen Warren Cc: Colin Cross, Erik Gilling, Olof Johansson, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On Wed, Jul 13, 2011 at 02:40:17PM -0600, Stephen Warren wrote: > + gpio_request(TEGRA_GPIO_CDC_IRQ, "wm8903"); > + gpio_direction_input(TEGRA_GPIO_CDC_IRQ); > + > i2c_register_board_info(0, &wm8903_board_info, 1); This seems silly - we should fix this in the core code rather than have every single board that ahppens to use an interrupt which is also available as a GPIO manually faff around with gpiolib. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] ARM: Tegra: Harmony: Register and configure WM8903 IRQ GPIO @ 2011-07-14 12:25 ` Mark Brown 0 siblings, 0 replies; 12+ messages in thread From: Mark Brown @ 2011-07-14 12:25 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jul 13, 2011 at 02:40:17PM -0600, Stephen Warren wrote: > + gpio_request(TEGRA_GPIO_CDC_IRQ, "wm8903"); > + gpio_direction_input(TEGRA_GPIO_CDC_IRQ); > + > i2c_register_board_info(0, &wm8903_board_info, 1); This seems silly - we should fix this in the core code rather than have every single board that ahppens to use an interrupt which is also available as a GPIO manually faff around with gpiolib. ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <20110714122520.GB14249-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>]
* RE: [PATCH 1/2] ARM: Tegra: Harmony: Register and configure WM8903 IRQ GPIO 2011-07-14 12:25 ` Mark Brown @ 2011-07-14 15:49 ` Stephen Warren -1 siblings, 0 replies; 12+ messages in thread From: Stephen Warren @ 2011-07-14 15:49 UTC (permalink / raw) To: Mark Brown Cc: Colin Cross, Erik Gilling, Olof Johansson, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Mark Brown wrote at Thursday, July 14, 2011 6:25 AM: > On Wed, Jul 13, 2011 at 02:40:17PM -0600, Stephen Warren wrote: > > > + gpio_request(TEGRA_GPIO_CDC_IRQ, "wm8903"); > > + gpio_direction_input(TEGRA_GPIO_CDC_IRQ); > > + > > i2c_register_board_info(0, &wm8903_board_info, 1); > > This seems silly - we should fix this in the core code rather than have > every single board that ahppens to use an interrupt which is also > available as a GPIO manually faff around with gpiolib. That seems a good goal. However, how does the WM8903 driver know whether the interrupt number it's passed is a straight-up dedicated interrupt (hence the calls aren't required), or a GPIO (hence they are)? I guess the answer is that there should be an interrupt API to map from interrupt to GPIO number, returning <0 when there is no GPIO backing the IRQ, and an op in struct irq_chip to implement that? However, that's not there right now as far as I can tell. Finally, there are some pinmux interactions that need to be dealt with; on Tegra, the pinmux allows the tristate/drive status of pins to be set on a group basis (a group being a set of 1-n pins). However, gpio enable (which overrides the pinmux's setting of tristate/drive) can be set on a per-pin basis. At least on Seaboard, the WM8903 IRQ is an input pin in a group that otherwise needs to contain output pins, so we really want to enable the WM8903 IRQ GPIO pin as a GPIO, and set it to input, before setting up that pinmux group to drive the pins. Without this, there may be a brief period where both Tegra and the WM8903 are driving this IRQ signal, which can't be good for hardware. -- nvpublic ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] ARM: Tegra: Harmony: Register and configure WM8903 IRQ GPIO @ 2011-07-14 15:49 ` Stephen Warren 0 siblings, 0 replies; 12+ messages in thread From: Stephen Warren @ 2011-07-14 15:49 UTC (permalink / raw) To: linux-arm-kernel Mark Brown wrote at Thursday, July 14, 2011 6:25 AM: > On Wed, Jul 13, 2011 at 02:40:17PM -0600, Stephen Warren wrote: > > > + gpio_request(TEGRA_GPIO_CDC_IRQ, "wm8903"); > > + gpio_direction_input(TEGRA_GPIO_CDC_IRQ); > > + > > i2c_register_board_info(0, &wm8903_board_info, 1); > > This seems silly - we should fix this in the core code rather than have > every single board that ahppens to use an interrupt which is also > available as a GPIO manually faff around with gpiolib. That seems a good goal. However, how does the WM8903 driver know whether the interrupt number it's passed is a straight-up dedicated interrupt (hence the calls aren't required), or a GPIO (hence they are)? I guess the answer is that there should be an interrupt API to map from interrupt to GPIO number, returning <0 when there is no GPIO backing the IRQ, and an op in struct irq_chip to implement that? However, that's not there right now as far as I can tell. Finally, there are some pinmux interactions that need to be dealt with; on Tegra, the pinmux allows the tristate/drive status of pins to be set on a group basis (a group being a set of 1-n pins). However, gpio enable (which overrides the pinmux's setting of tristate/drive) can be set on a per-pin basis. At least on Seaboard, the WM8903 IRQ is an input pin in a group that otherwise needs to contain output pins, so we really want to enable the WM8903 IRQ GPIO pin as a GPIO, and set it to input, before setting up that pinmux group to drive the pins. Without this, there may be a brief period where both Tegra and the WM8903 are driving this IRQ signal, which can't be good for hardware. -- nvpublic ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <74CDBE0F657A3D45AFBB94109FB122FF049E834971-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>]
* Re: [PATCH 1/2] ARM: Tegra: Harmony: Register and configure WM8903 IRQ GPIO 2011-07-14 15:49 ` Stephen Warren @ 2011-07-15 0:05 ` Mark Brown -1 siblings, 0 replies; 12+ messages in thread From: Mark Brown @ 2011-07-15 0:05 UTC (permalink / raw) To: Stephen Warren Cc: Colin Cross, Erik Gilling, Olof Johansson, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On Thu, Jul 14, 2011 at 08:49:28AM -0700, Stephen Warren wrote: > Mark Brown wrote at Thursday, July 14, 2011 6:25 AM: > > This seems silly - we should fix this in the core code rather than have > > every single board that ahppens to use an interrupt which is also > > available as a GPIO manually faff around with gpiolib. > That seems a good goal. > However, how does the WM8903 driver know whether the interrupt number > it's passed is a straight-up dedicated interrupt (hence the calls aren't > required), or a GPIO (hence they are)? It shouldn't - I said "core code" for a reason :) > I guess the answer is that there should be an interrupt API to map from > interrupt to GPIO number, returning <0 when there is no GPIO backing the > IRQ, and an op in struct irq_chip to implement that? However, that's not > there right now as far as I can tell. Yes, exactly - but it should not be complicated to add one and since you're doing this anyway it'd make sense to do the core change rather than faffing around the edges of the system. > Finally, there are some pinmux interactions that need to be dealt with; > on Tegra, the pinmux allows the tristate/drive status of pins to be set > on a group basis (a group being a set of 1-n pins). However, gpio enable > (which overrides the pinmux's setting of tristate/drive) can be set on > a per-pin basis. At least on Seaboard, the WM8903 IRQ is an input pin > in a group that otherwise needs to contain output pins, so we really > want to enable the WM8903 IRQ GPIO pin as a GPIO, and set it to input, > before setting up that pinmux group to drive the pins. Without this, > there may be a brief period where both Tegra and the WM8903 are driving > this IRQ signal, which can't be good for hardware. If the pinmux stuff can't work this out then it needs to be worked on until it can, this is pretty basic stuff. I had thought that the plan was to support a big block configuration done by the board which set everything up en masse and would presumably cope with stuff like this? ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] ARM: Tegra: Harmony: Register and configure WM8903 IRQ GPIO @ 2011-07-15 0:05 ` Mark Brown 0 siblings, 0 replies; 12+ messages in thread From: Mark Brown @ 2011-07-15 0:05 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 14, 2011 at 08:49:28AM -0700, Stephen Warren wrote: > Mark Brown wrote at Thursday, July 14, 2011 6:25 AM: > > This seems silly - we should fix this in the core code rather than have > > every single board that ahppens to use an interrupt which is also > > available as a GPIO manually faff around with gpiolib. > That seems a good goal. > However, how does the WM8903 driver know whether the interrupt number > it's passed is a straight-up dedicated interrupt (hence the calls aren't > required), or a GPIO (hence they are)? It shouldn't - I said "core code" for a reason :) > I guess the answer is that there should be an interrupt API to map from > interrupt to GPIO number, returning <0 when there is no GPIO backing the > IRQ, and an op in struct irq_chip to implement that? However, that's not > there right now as far as I can tell. Yes, exactly - but it should not be complicated to add one and since you're doing this anyway it'd make sense to do the core change rather than faffing around the edges of the system. > Finally, there are some pinmux interactions that need to be dealt with; > on Tegra, the pinmux allows the tristate/drive status of pins to be set > on a group basis (a group being a set of 1-n pins). However, gpio enable > (which overrides the pinmux's setting of tristate/drive) can be set on > a per-pin basis. At least on Seaboard, the WM8903 IRQ is an input pin > in a group that otherwise needs to contain output pins, so we really > want to enable the WM8903 IRQ GPIO pin as a GPIO, and set it to input, > before setting up that pinmux group to drive the pins. Without this, > there may be a brief period where both Tegra and the WM8903 are driving > this IRQ signal, which can't be good for hardware. If the pinmux stuff can't work this out then it needs to be worked on until it can, this is pretty basic stuff. I had thought that the plan was to support a big block configuration done by the board which set everything up en masse and would presumably cope with stuff like this? ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <20110715000510.GD32716-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>]
* RE: [PATCH 1/2] ARM: Tegra: Harmony: Register and configure WM8903 IRQ GPIO 2011-07-15 0:05 ` Mark Brown @ 2011-07-15 20:02 ` Stephen Warren -1 siblings, 0 replies; 12+ messages in thread From: Stephen Warren @ 2011-07-15 20:02 UTC (permalink / raw) To: Mark Brown, Grant Likely (grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org), tglx-hfZtesqFncYOwBW4kG4KsQ Cc: Colin Cross, Erik Gilling, Olof Johansson, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Mark Brown wrote at Thursday, July 14, 2011 6:05 PM: > On Thu, Jul 14, 2011 at 08:49:28AM -0700, Stephen Warren wrote: > > Mark Brown wrote at Thursday, July 14, 2011 6:25 AM: > > > > This seems silly - we should fix this in the core code rather than have > > > every single board that ahppens to use an interrupt which is also > > > available as a GPIO manually faff around with gpiolib. > > > That seems a good goal. > > > However, how does the WM8903 driver know whether the interrupt number > > it's passed is a straight-up dedicated interrupt (hence the calls aren't > > required), or a GPIO (hence they are)? > > It shouldn't - I said "core code" for a reason :) Ah, yes. [Grant, Thomas, as GPIO/IRQ maintainers, do you agree that (a) below is something the core IRQ code should be doing?] There are two aspects to this: a) The generic gpio_request/gpio_direction_input. This could be handled by the core irq code if struct irq_chip grew a to_irq function, and a __irq_to_gpio/irq_to_gpio global function were created to mirror gpio_to_irq. b) On Tegra at least, we need to call tegra_gpio_enable to configure the pin to be a GPIO. This isn't performed by the Tegra GPIO driver during gpio_request, since such configuration is supposed to be outside the domain of the GPIO driver, but deferred to perhaps a pinmux driver. Should the same be true of request_irq; should it require the user to somehow call tegra_gpio_enable elsewhere? It'd be asymmetric to how gpio_request worked, but Tegra's irq_chip could easily call tegra_gpio_enable from within irq_chip.startup, thus simplifying any code that wants to use interrupt-that-are-GPIOs. The alternative is for the board files to have a list of GPIOs for which to call tegra_gpio_enable, and indeed, that's what we do have in Tegra e.g. in board-seaboard-pinmux.c. I suppose with the new pinmux API, there will simply be a pinmux table in either the board file or Device Tree, and that will allow "GPIO" as a legal "special function" for each pin, and for pins which are really "GPIO interrupts", we'll just need to list "GPIO" in that pinmux table, so perhaps everything will just fall out cleanly that way. I suppose that a pinmux driver could even expose an "interrupt" special function for relevant pins just to clearly document the usage, even if this maps to the same thing as configuring a pin as a GPIO. > > I guess the answer is that there should be an interrupt API to map from > > interrupt to GPIO number, returning <0 when there is no GPIO backing the > > IRQ, and an op in struct irq_chip to implement that? However, that's not > > there right now as far as I can tell. > > Yes, exactly - but it should not be complicated to add one and since > you're doing this anyway it'd make sense to do the core change rather > than faffing around the edges of the system. Sure, it doesn't look too hard. A complication is that ARM doesn't define gpio_to_irq in a single central header, but rather one per subarchitecture, I suppose since not all subarchitectures implement/require gpiolib. Still, I think this mostly just means cut/pasting "#define irq_to_gpio __irq_to_gpio" into each mach-foo's gpio.h for now, so /probably/ not a big deal... > > Finally, there are some pinmux interactions that need to be dealt with; > > on Tegra, the pinmux allows the tristate/drive status of pins to be set > > on a group basis (a group being a set of 1-n pins). However, gpio enable > > (which overrides the pinmux's setting of tristate/drive) can be set on > > a per-pin basis. At least on Seaboard, the WM8903 IRQ is an input pin > > in a group that otherwise needs to contain output pins, so we really > > want to enable the WM8903 IRQ GPIO pin as a GPIO, and set it to input, > > before setting up that pinmux group to drive the pins. Without this, > > there may be a brief period where both Tegra and the WM8903 are driving > > this IRQ signal, which can't be good for hardware. > > If the pinmux stuff can't work this out then it needs to be worked on > until it can, this is pretty basic stuff. I had thought that the plan > was to support a big block configuration done by the board which set > everything up en masse and would presumably cope with stuff like this? Maybe the solution for Tegra, once we have the new pinmux API in place, is for the pinmux driver initialization to program all pins in HW as input GPIOs, and hence tri-stated. That way, there can never be any conflict with external HW. If/once a pin/group is requested as a particular special function, the tri-state can be removed, since then we'll know the board-specific direction. Some though may have to be given to not tri-stating any pins required for basic SDRAM or UART access during the boot process, but this is probably workable. -- nvpublic ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] ARM: Tegra: Harmony: Register and configure WM8903 IRQ GPIO @ 2011-07-15 20:02 ` Stephen Warren 0 siblings, 0 replies; 12+ messages in thread From: Stephen Warren @ 2011-07-15 20:02 UTC (permalink / raw) To: linux-arm-kernel Mark Brown wrote at Thursday, July 14, 2011 6:05 PM: > On Thu, Jul 14, 2011 at 08:49:28AM -0700, Stephen Warren wrote: > > Mark Brown wrote at Thursday, July 14, 2011 6:25 AM: > > > > This seems silly - we should fix this in the core code rather than have > > > every single board that ahppens to use an interrupt which is also > > > available as a GPIO manually faff around with gpiolib. > > > That seems a good goal. > > > However, how does the WM8903 driver know whether the interrupt number > > it's passed is a straight-up dedicated interrupt (hence the calls aren't > > required), or a GPIO (hence they are)? > > It shouldn't - I said "core code" for a reason :) Ah, yes. [Grant, Thomas, as GPIO/IRQ maintainers, do you agree that (a) below is something the core IRQ code should be doing?] There are two aspects to this: a) The generic gpio_request/gpio_direction_input. This could be handled by the core irq code if struct irq_chip grew a to_irq function, and a __irq_to_gpio/irq_to_gpio global function were created to mirror gpio_to_irq. b) On Tegra at least, we need to call tegra_gpio_enable to configure the pin to be a GPIO. This isn't performed by the Tegra GPIO driver during gpio_request, since such configuration is supposed to be outside the domain of the GPIO driver, but deferred to perhaps a pinmux driver. Should the same be true of request_irq; should it require the user to somehow call tegra_gpio_enable elsewhere? It'd be asymmetric to how gpio_request worked, but Tegra's irq_chip could easily call tegra_gpio_enable from within irq_chip.startup, thus simplifying any code that wants to use interrupt-that-are-GPIOs. The alternative is for the board files to have a list of GPIOs for which to call tegra_gpio_enable, and indeed, that's what we do have in Tegra e.g. in board-seaboard-pinmux.c. I suppose with the new pinmux API, there will simply be a pinmux table in either the board file or Device Tree, and that will allow "GPIO" as a legal "special function" for each pin, and for pins which are really "GPIO interrupts", we'll just need to list "GPIO" in that pinmux table, so perhaps everything will just fall out cleanly that way. I suppose that a pinmux driver could even expose an "interrupt" special function for relevant pins just to clearly document the usage, even if this maps to the same thing as configuring a pin as a GPIO. > > I guess the answer is that there should be an interrupt API to map from > > interrupt to GPIO number, returning <0 when there is no GPIO backing the > > IRQ, and an op in struct irq_chip to implement that? However, that's not > > there right now as far as I can tell. > > Yes, exactly - but it should not be complicated to add one and since > you're doing this anyway it'd make sense to do the core change rather > than faffing around the edges of the system. Sure, it doesn't look too hard. A complication is that ARM doesn't define gpio_to_irq in a single central header, but rather one per subarchitecture, I suppose since not all subarchitectures implement/require gpiolib. Still, I think this mostly just means cut/pasting "#define irq_to_gpio __irq_to_gpio" into each mach-foo's gpio.h for now, so /probably/ not a big deal... > > Finally, there are some pinmux interactions that need to be dealt with; > > on Tegra, the pinmux allows the tristate/drive status of pins to be set > > on a group basis (a group being a set of 1-n pins). However, gpio enable > > (which overrides the pinmux's setting of tristate/drive) can be set on > > a per-pin basis. At least on Seaboard, the WM8903 IRQ is an input pin > > in a group that otherwise needs to contain output pins, so we really > > want to enable the WM8903 IRQ GPIO pin as a GPIO, and set it to input, > > before setting up that pinmux group to drive the pins. Without this, > > there may be a brief period where both Tegra and the WM8903 are driving > > this IRQ signal, which can't be good for hardware. > > If the pinmux stuff can't work this out then it needs to be worked on > until it can, this is pretty basic stuff. I had thought that the plan > was to support a big block configuration done by the board which set > everything up en masse and would presumably cope with stuff like this? Maybe the solution for Tegra, once we have the new pinmux API in place, is for the pinmux driver initialization to program all pins in HW as input GPIOs, and hence tri-stated. That way, there can never be any conflict with external HW. If/once a pin/group is requested as a particular special function, the tri-state can be removed, since then we'll know the board-specific direction. Some though may have to be given to not tri-stating any pins required for basic SDRAM or UART access during the boot process, but this is probably workable. -- nvpublic ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-07-15 20:02 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-07-13 20:40 [PATCH 1/2] ARM: Tegra: Harmony: Register and configure WM8903 IRQ GPIO Stephen Warren 2011-07-13 20:40 ` Stephen Warren [not found] ` <1310589618-16080-1-git-send-email-swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2011-07-13 20:40 ` [PATCH 2/2] ARM: Tegra: Harmony: Add USB device Stephen Warren 2011-07-13 20:40 ` Stephen Warren 2011-07-14 12:25 ` [PATCH 1/2] ARM: Tegra: Harmony: Register and configure WM8903 IRQ GPIO Mark Brown 2011-07-14 12:25 ` Mark Brown [not found] ` <20110714122520.GB14249-GFdadSzt00ze9xe1eoZjHA@public.gmane.org> 2011-07-14 15:49 ` Stephen Warren 2011-07-14 15:49 ` Stephen Warren [not found] ` <74CDBE0F657A3D45AFBB94109FB122FF049E834971-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org> 2011-07-15 0:05 ` Mark Brown 2011-07-15 0:05 ` Mark Brown [not found] ` <20110715000510.GD32716-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> 2011-07-15 20:02 ` Stephen Warren 2011-07-15 20:02 ` Stephen Warren
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.