[PATCH 0/8] ARM: Tegra: cleanup patches from Arnd Bergmann

[PATCH 0/8] ARM: Tegra: cleanup patches from Arnd Bergmann

[Alan Ott]

Hello,

I'm helping Arnd Bergmann get some of his cleanup patches upstream. This
set covers the Tegra.

Thanks,

Alan.

Arnd Bergmann (8):
  ARM: tegra: export tegra_gpio_{en,dis}able
  ARM: tegra: do not hide dma declarations
  ARM: tegra: select CPU_FREQ_TABLE
  ARM: tegra: build localtimer support only when needed
  ARM: tegra: work around tegra THUMB2_KERNEL bug
  ARM: tegra: USB_ULPI needs USB
  ARM: tegra: export usb phy symbols
  ARM: tegra: HACK: remove set_irq_flags() from driver

 arch/arm/Kconfig                       |    1 +
 arch/arm/mach-tegra/Kconfig            |    2 ++
 arch/arm/mach-tegra/Makefile           |    3 ++-
 arch/arm/mach-tegra/include/mach/dma.h |    4 ----
 arch/arm/mach-tegra/usb_phy.c          |   10 ++++++++++
 drivers/gpio/gpio-tegra.c              |    2 ++
 drivers/usb/host/ehci-tegra.c          |    1 -
 7 files changed, 17 insertions(+), 6 deletions(-)

[PATCH 1/8] ARM: tegra: export tegra_gpio_{en,dis}able

[Alan Ott]

From: Arnd Bergmann <arnd@arndb.de>

These two functions are used in drivers that can be
modules, so they need to be exported.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpio/gpio-tegra.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
index bdc2937..96662cc 100644
--- a/drivers/gpio/gpio-tegra.c
+++ b/drivers/gpio/gpio-tegra.c
@@ -107,11 +107,13 @@ void tegra_gpio_enable(int gpio)
 {
 	tegra_gpio_mask_write(GPIO_MSK_CNF(gpio), gpio, 1);
 }
+EXPORT_SYMBOL_GPL(tegra_gpio_enable);
 
 void tegra_gpio_disable(int gpio)
 {
 	tegra_gpio_mask_write(GPIO_MSK_CNF(gpio), gpio, 0);
 }
+EXPORT_SYMBOL_GPL(tegra_gpio_disable);
 
 static void tegra_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 {
-- 
1.7.0.4

Re: [PATCH 1/8] ARM: tegra: export tegra_gpio_{en,dis}able

[Olof Johansson]

Hi,

On Thu, Mar 1, 2012 at 11:26 AM, Alan Ott <alan-yzvJWuRpmD1zbRFIqnYvSA@public.gmane.org> wrote:
> From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>
> These two functions are used in drivers that can be
> modules, so they need to be exported.
>
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

With some of the pinmux changes going in this will no longer be
needed, but it is for now so:

Acked-by: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>

Re: [PATCH 1/8] ARM: tegra: export tegra_gpio_{en,dis}able

[Grant Likely]

On Thu,  1 Mar 2012 14:26:43 -0500, Alan Ott <alan@signal11.us> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> These two functions are used in drivers that can be
> modules, so they need to be exported.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> ---
>  drivers/gpio/gpio-tegra.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
> index bdc2937..96662cc 100644
> --- a/drivers/gpio/gpio-tegra.c
> +++ b/drivers/gpio/gpio-tegra.c
> @@ -107,11 +107,13 @@ void tegra_gpio_enable(int gpio)
>  {
>  	tegra_gpio_mask_write(GPIO_MSK_CNF(gpio), gpio, 1);
>  }
> +EXPORT_SYMBOL_GPL(tegra_gpio_enable);
>  
>  void tegra_gpio_disable(int gpio)
>  {
>  	tegra_gpio_mask_write(GPIO_MSK_CNF(gpio), gpio, 0);
>  }
> +EXPORT_SYMBOL_GPL(tegra_gpio_disable);
>  
>  static void tegra_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
>  {
> -- 
> 1.7.0.4
> 

-- 
email sent from notmuch.vim plugin

Re: [PATCH v2] ARM: tegra: export tegra_gpio_{en,dis}able

[Stephen Warren]

On 03/09/2012 10:00 AM, Alan Ott wrote:
> On 03/02/2012 05:32 PM, Alan Ott wrote:
>> From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>>
>> These two functions are used in drivers that can be
>> modules, so they need to be exported.
>>
>> Acked-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>> Acked-by: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
...
> Did this patch ever get picked up into someone's tree?

It looks like Grant ack'd it which implies the thought Olof was going to
take it through the Tegra tree perhaps? But Olof did the same, so is
probably waiting for Grant to take it...

Re: [PATCH v2] ARM: tegra: export tegra_gpio_{en,dis}able

[Grant Likely]

On Fri, 09 Mar 2012 10:09:57 -0700, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 03/09/2012 10:00 AM, Alan Ott wrote:
> > On 03/02/2012 05:32 PM, Alan Ott wrote:
> >> From: Arnd Bergmann <arnd@arndb.de>
> >>
> >> These two functions are used in drivers that can be
> >> modules, so they need to be exported.
> >>
> >> Acked-by: Grant Likely <grant.likely@secretlab.ca>
> >> Acked-by: Olof Johansson <olof@lixom.net>
> ...
> > Did this patch ever get picked up into someone's tree?
> 
> It looks like Grant ack'd it which implies the thought Olof was going to
> take it through the Tegra tree perhaps? But Olof did the same, so is
> probably waiting for Grant to take it...

Applied, thx.

g.

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.

[PATCH 2/8] ARM: tegra: do not hide dma declarations

[Alan Ott]

From: Arnd Bergmann <arnd@arndb.de>

The declarations are actually required for the device
definitions, and are still valid even if the dma controller
is disabled:

arch/arm/mach-tegra/devices.c:559:12: error: 'TEGRA_DMA_REQ_SEL_I2S_1' undeclared here (not in a function)
arch/arm/mach-tegra/devices.c:577:12: error: 'TEGRA_DMA_REQ_SEL_I2S2_1' undeclared here (not in a function)

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mach-tegra/include/mach/dma.h |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-tegra/include/mach/dma.h b/arch/arm/mach-tegra/include/mach/dma.h
index 3c93390..9077092 100644
--- a/arch/arm/mach-tegra/include/mach/dma.h
+++ b/arch/arm/mach-tegra/include/mach/dma.h
@@ -51,8 +51,6 @@
 #define TEGRA_DMA_REQ_SEL_OWR			25
 #define TEGRA_DMA_REQ_SEL_INVALID		31
 
-#if defined(CONFIG_TEGRA_SYSTEM_DMA)
-
 struct tegra_dma_req;
 struct tegra_dma_channel;
 
@@ -151,5 +149,3 @@ void tegra_dma_free_channel(struct tegra_dma_channel *ch);
 int __init tegra_dma_init(void);
 
 #endif
-
-#endif
-- 
1.7.0.4

[PATCH 3/8] ARM: tegra: select CPU_FREQ_TABLE

[Alan Ott]

From: Arnd Bergmann <arnd@arndb.de>

The tegra cpufreq implementation relies on the cpu_freq_table
code, so make sure that this is always there when needed.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mach-tegra/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index 373652d..b2a038e 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -10,6 +10,7 @@ config ARCH_TEGRA_2x_SOC
 	select USB_ARCH_HAS_EHCI if USB_SUPPORT
 	select USB_ULPI if USB_SUPPORT
 	select USB_ULPI_VIEWPORT if USB_SUPPORT
+	select CPU_FREQ_TABLE if CPU_FREQ
 	help
 	  Support for NVIDIA Tegra AP20 and T20 processors, based on the
 	  ARM CortexA9MP CPU and the ARM PL310 L2 cache controller
-- 
1.7.0.4

Re: [PATCH 3/8] ARM: tegra: select CPU_FREQ_TABLE

[Olof Johansson]

Hi,

On Thu, Mar 1, 2012 at 11:26 AM, Alan Ott <alan-yzvJWuRpmD1zbRFIqnYvSA@public.gmane.org> wrote:
> From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>
> The tegra cpufreq implementation relies on the cpu_freq_table
> code, so make sure that this is always there when needed.
>
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> ---
>  arch/arm/mach-tegra/Kconfig |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
> index 373652d..b2a038e 100644
> --- a/arch/arm/mach-tegra/Kconfig
> +++ b/arch/arm/mach-tegra/Kconfig
> @@ -10,6 +10,7 @@ config ARCH_TEGRA_2x_SOC
>        select USB_ARCH_HAS_EHCI if USB_SUPPORT
>        select USB_ULPI if USB_SUPPORT
>        select USB_ULPI_VIEWPORT if USB_SUPPORT
> +       select CPU_FREQ_TABLE if CPU_FREQ

The corresponding chance is needed for T30 as well.

(the cpufreq driver should probably be moved out to drivers/cpufreq
and given its own but that's a separate issue)


-Olof

[PATCH 4/8] ARM: tegra: build localtimer support only when needed

[Alan Ott]

From: Arnd Bergmann <arnd@arndb.de>

It is possible to build a tegra kernel without localtimer
support, so the tegra specific parts should only be built
when that is indeed enabled.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mach-tegra/Makefile |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
index e120ff5..a956df8 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -13,7 +13,8 @@ obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= tegra2_emc.o
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= pinmux-tegra20-tables.o
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= pinmux-tegra30-tables.o
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= board-dt-tegra30.o
-obj-$(CONFIG_SMP)                       += platsmp.o localtimer.o headsmp.o
+obj-$(CONFIG_SMP)                       += platsmp.o headsmp.o
+obj-$(CONFIG_LOCAL_TIMERS)		+= localtimer.o
 obj-$(CONFIG_HOTPLUG_CPU)               += hotplug.o
 obj-$(CONFIG_TEGRA_SYSTEM_DMA)		+= dma.o
 obj-$(CONFIG_CPU_FREQ)                  += cpu-tegra.o
-- 
1.7.0.4

Re: [PATCH 4/8] ARM: tegra: build localtimer support only when needed

[Sergei Shtylyov]

Hello.

On 01-03-2012 23:26, Alan Ott wrote:

> From: Arnd Bergmann<arnd-r2nGTMty4D4@public.gmane.org>

> It is possible to build a tegra kernel without localtimer
> support, so the tegra specific parts should only be built
> when that is indeed enabled.

> Signed-off-by: Arnd Bergmann<arnd-r2nGTMty4D4@public.gmane.org>
> ---
>   arch/arm/mach-tegra/Makefile |    3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)

> diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
> index e120ff5..a956df8 100644
> --- a/arch/arm/mach-tegra/Makefile
> +++ b/arch/arm/mach-tegra/Makefile
> @@ -13,7 +13,8 @@ obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= tegra2_emc.o
>   obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= pinmux-tegra20-tables.o
>   obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= pinmux-tegra30-tables.o
>   obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= board-dt-tegra30.o
> -obj-$(CONFIG_SMP)                       += platsmp.o localtimer.o headsmp.o
> +obj-$(CONFIG_SMP)                       += platsmp.o headsmp.o

   Could use the chance to indent here with tabs instead of spaces, like below.

> +obj-$(CONFIG_LOCAL_TIMERS)		+= localtimer.o

WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/8] ARM: tegra: work around tegra THUMB2_KERNEL bug

[Alan Ott]

From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

The tegra smp code cannot currently be built for t2, so
disable that combination for now, as a workaround.

Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
 arch/arm/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a48aecc..ea1d59b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1598,6 +1598,7 @@ config HZ
 config THUMB2_KERNEL
 	bool "Compile the kernel in Thumb-2 mode (EXPERIMENTAL)"
 	depends on CPU_V7 && !CPU_V6 && !CPU_V6K && EXPERIMENTAL
+	depends on !SMP || !ARCH_TEGRA
 	select AEABI
 	select ARM_ASM_UNIFIED
 	select ARM_UNWIND
-- 
1.7.0.4

RE: [PATCH 5/8] ARM: tegra: work around tegra THUMB2_KERNEL bug

[Stephen Warren]

Alan Ott wrote at Thursday, March 01, 2012 12:27 PM:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The tegra smp code cannot currently be built for t2, so
> disable that combination for now, as a workaround.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

At least in next-20120229 with the .config I'm using, everything appears
to compile fine. The link step does fail, but the error messages I get
don't appear to be Tegra specific:

xxx/kernel/sched/clock.c:334: relocation truncated to fit: R_ARM_THM_JUMP11 against symbol `sched_clock' defined in .text section in arch/arm/kernel/built-in.o
kernel/built-in.o: In function `post_relocation':
xxx/kernel/module.c:2860: relocation truncated to fit: R_ARM_THM_JUMP11 against symbol `module_finalize' defined in .text section in arch/arm/kernel/built-in.o
kernel/built-in.o: In function `show_interrupts':
xxx/kernel/irq/proc.c:431: relocation truncated to fit: R_ARM_THM_JUMP11 against symbol `arch_show_interrupts' defined in .text section in arch/arm/kernel/built-in.o
drivers/built-in.o: In function `pci_set_master':
xxx/drivers/pci/pci.c:2674: relocation truncated to fit: R_ARM_THM_JUMP11 against symbol `pcibios_set_master' defined in .text section in arch/arm/kernel/built-in.o

What failure does this solve, and how do I trigger it?

-- 
nvpublic

Re: [PATCH 5/8] ARM: tegra: work around tegra THUMB2_KERNEL bug

[Russell King - ARM Linux]

On Thu, Mar 01, 2012 at 12:02:42PM -0800, Stephen Warren wrote:
> Alan Ott wrote at Thursday, March 01, 2012 12:27 PM:
> > From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> > 
> > The tegra smp code cannot currently be built for t2, so
> > disable that combination for now, as a workaround.
> > 
> > Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> 
> At least in next-20120229 with the .config I'm using, everything appears
> to compile fine. The link step does fail, but the error messages I get
> don't appear to be Tegra specific:
> 
> xxx/kernel/sched/clock.c:334: relocation truncated to fit: R_ARM_THM_JUMP11 against symbol `sched_clock' defined in .text section in arch/arm/kernel/built-in.o
> kernel/built-in.o: In function `post_relocation':
> xxx/kernel/module.c:2860: relocation truncated to fit: R_ARM_THM_JUMP11 against symbol `module_finalize' defined in .text section in arch/arm/kernel/built-in.o
> kernel/built-in.o: In function `show_interrupts':
> xxx/kernel/irq/proc.c:431: relocation truncated to fit: R_ARM_THM_JUMP11 against symbol `arch_show_interrupts' defined in .text section in arch/arm/kernel/built-in.o
> drivers/built-in.o: In function `pci_set_master':
> xxx/drivers/pci/pci.c:2674: relocation truncated to fit: R_ARM_THM_JUMP11 against symbol `pcibios_set_master' defined in .text section in arch/arm/kernel/built-in.o

At a guess, it suggests your kernel image is too big for the relocations
contained within the code.  That's something for Thumb people to look
into and solve.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/8] ARM: tegra: work around tegra THUMB2_KERNEL bug

[Arnd Bergmann]

On Thursday 01 March 2012, Russell King - ARM Linux wrote:
> On Thu, Mar 01, 2012 at 12:02:42PM -0800, Stephen Warren wrote:
> > Alan Ott wrote at Thursday, March 01, 2012 12:27 PM:
> > > From: Arnd Bergmann <arnd@arndb.de>
> > > 
> > > The tegra smp code cannot currently be built for t2, so
> > > disable that combination for now, as a workaround.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > 
> > At least in next-20120229 with the .config I'm using, everything appears
> > to compile fine. The link step does fail, but the error messages I get
> > don't appear to be Tegra specific:
> > 
> > xxx/kernel/sched/clock.c:334: relocation truncated to fit: R_ARM_THM_JUMP11 against symbol `sched_clock' defined in .text section in arch/arm/kernel/built-in.o
> > kernel/built-in.o: In function `post_relocation':
> > xxx/kernel/module.c:2860: relocation truncated to fit: R_ARM_THM_JUMP11 against symbol `module_finalize' defined in .text section in arch/arm/kernel/built-in.o
> > kernel/built-in.o: In function `show_interrupts':
> > xxx/kernel/irq/proc.c:431: relocation truncated to fit: R_ARM_THM_JUMP11 against symbol `arch_show_interrupts' defined in .text section in arch/arm/kernel/built-in.o
> > drivers/built-in.o: In function `pci_set_master':
> > xxx/drivers/pci/pci.c:2674: relocation truncated to fit: R_ARM_THM_JUMP11 against symbol `pcibios_set_master' defined in .text section in arch/arm/kernel/built-in.o
> 
> At a guess, it suggests your kernel image is too big for the relocations
> contained within the code.  That's something for Thumb people to look
> into and solve.

IIRC, that was not the issue I was trying to fix when I originally made the
patch. It's quite likely that the original bug was fixed already, so I'd suggest
we drop the patch for now and see if the problem comes back when all other
randconfig problems have been resolved on Tegra.

	Arnd

[PATCH 8/8] ARM: tegra: HACK: remove set_irq_flags() from driver

[Alan Ott]

From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

The driver should not call set_irq_flags itself, and
cannot do this from a loadable module.

Remove the call for now, which might break the driver
but at least lets the kernel link again.

Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
 drivers/usb/host/ehci-tegra.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index dbc7fe8..ab91f26 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -729,7 +729,6 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 		err = -ENODEV;
 		goto fail;
 	}
-	set_irq_flags(irq, IRQF_VALID);
 
 #ifdef CONFIG_USB_OTG_UTILS
 	if (pdata->operating_mode == TEGRA_USB_OTG) {
-- 
1.7.0.4

Re: [PATCH 8/8] ARM: tegra: HACK: remove set_irq_flags() from driver

[Greg Kroah-Hartman]

On Thu, Mar 01, 2012 at 02:26:50PM -0500, Alan Ott wrote:
> From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> 
> The driver should not call set_irq_flags itself, and
> cannot do this from a loadable module.
> 
> Remove the call for now, which might break the driver
> but at least lets the kernel link again.
> 
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

Um, I think you should test this out before submitting it anywhere :)

As it is, I can't accept it.

greg k-h

Re: [PATCH 8/8] ARM: tegra: HACK: remove set_irq_flags() from driver

[Arnd Bergmann]

On Thursday 01 March 2012, Greg Kroah-Hartman wrote:
> On Thu, Mar 01, 2012 at 02:26:50PM -0500, Alan Ott wrote:
> > From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> > 
> > The driver should not call set_irq_flags itself, and
> > cannot do this from a loadable module.
> > 
> > Remove the call for now, which might break the driver
> > but at least lets the kernel link again.
> > 
> > Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> 
> Um, I think you should test this out before submitting it anywhere :)
> 
> As it is, I can't accept it.

Right, this one was not meant for inclusion, similar to patch 5 in this
series.

Alan:
Part of the reason I asked for help getting the patches out is that
they need a lot of auditing, both for whether they still apply to
current kernels and for whether they are a good solution to the problem
at hand. When in doubt, it's probably better to leave out any patch
that looks like it might not be ready for inclusion, or that might
make thinks uglier in some way.

	Arnd

RE: [PATCH 8/8] ARM: tegra: HACK: remove set_irq_flags() from driver

[Stephen Warren]

Alan Ott wrote at Thursday, March 01, 2012 12:27 PM:
> From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> 
> The driver should not call set_irq_flags itself, and

Probably true in this case.

> cannot do this from a loadable module.

I hope that's not true; grep'ing the entire of drivers/ shows a bunch
of drivers calling this function, and many look like they'd be reasonable
as module.

From what little I understand of this, any irq_chip is going to call
that function after setting up any child/cascaded IRQs, and I assume
that irq_chips can be in modules.

> Remove the call for now, which might break the driver
> but at least lets the kernel link again.

The driver appears to work fine with this removed. At least, on Tegra20
Harmony, I was able to modprobe ehci-hcd and then use the USB Ethernet
controller for DHCP and SSH.

So,

Tested-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

-- 
nvpublic

Re: [PATCH 8/8] ARM: tegra: HACK: remove set_irq_flags() from driver

[Arnd Bergmann]

On Thursday 01 March 2012, Stephen Warren wrote:
> 
> Alan Ott wrote at Thursday, March 01, 2012 12:27 PM:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > The driver should not call set_irq_flags itself, and
> 
> Probably true in this case.
> 
> > cannot do this from a loadable module.
> 
> I hope that's not true; grep'ing the entire of drivers/ shows a bunch
> of drivers calling this function, and many look like they'd be reasonable
> as module.

The drivers that I can see using it are for the most part implementing
irq controllers by themselves, which is different from merely using an
interrupt.

There are three exceptions today:

arnd@klappe2:~/linux-arm$ git grep -l set_irq_flags  drivers/ | xargs grep -L irq_chip
drivers/tty/serial/serial_ks8695.c
drivers/tty/serial/sirfsoc_uart.c
drivers/usb/host/ehci-tegra.c

> From what little I understand of this, any irq_chip is going to call
> that function after setting up any child/cascaded IRQs, and I assume
> that irq_chips can be in modules.

But the function is not exported. I guess if we want to allow
irq_chips in loadable modules, we could export it, but I don't see
how it could ever have worked so far.
 
> > Remove the call for now, which might break the driver
> > but at least lets the kernel link again.
> 
> The driver appears to work fine with this removed. At least, on Tegra20
> Harmony, I was able to modprobe ehci-hcd and then use the USB Ethernet
> controller for DHCP and SSH.

Ok, maybe it was the right fix after all then and it just needs a proper
change description ;-)

	Arnd

Re: [PATCH 8/8] ARM: tegra: HACK: remove set_irq_flags() from driver

[Russell King - ARM Linux]

On Thu, Mar 01, 2012 at 08:38:27PM +0000, Arnd Bergmann wrote:
> On Thursday 01 March 2012, Stephen Warren wrote:
> > 
> > Alan Ott wrote at Thursday, March 01, 2012 12:27 PM:
> > > From: Arnd Bergmann <arnd@arndb.de>
> > > 
> > > The driver should not call set_irq_flags itself, and
> > 
> > Probably true in this case.
> > 
> > > cannot do this from a loadable module.
> > 
> > I hope that's not true; grep'ing the entire of drivers/ shows a bunch
> > of drivers calling this function, and many look like they'd be reasonable
> > as module.
> 
> The drivers that I can see using it are for the most part implementing
> irq controllers by themselves, which is different from merely using an
> interrupt.
> 
> There are three exceptions today:
> 
> arnd@klappe2:~/linux-arm$ git grep -l set_irq_flags  drivers/ | xargs grep -L irq_chip
> drivers/tty/serial/serial_ks8695.c

Setting NOAUTOEN should be done elsewhere (in platform code.)  Better
still, fixing genirq to allow request_irq() to permit the interrupt
to be requested *without enabling it* would be a step forwards.  I
thought my original ARM IRQ code did that.

> drivers/tty/serial/sirfsoc_uart.c

Same issue, so same comments.

> drivers/usb/host/ehci-tegra.c

This one should not be there.  Having drivers mark IRQs as being
requestable when the whole point of the flag is to stop request_irq()
being used on non-initialized interrupts is just plain stupid.

> > From what little I understand of this, any irq_chip is going to call
> > that function after setting up any child/cascaded IRQs, and I assume
> > that irq_chips can be in modules.
> 
> But the function is not exported. I guess if we want to allow
> irq_chips in loadable modules, we could export it, but I don't see
> how it could ever have worked so far.

genirq does not permit flow handlers in modules - I suspect that's
because it's pretty hard to remove them safely from the system without
a lot of effort.  Certainly, a whole bunch of genirq functions which
are required to implement external flow handlers etc aren't exported.

Re: [PATCH 8/8] ARM: tegra: HACK: remove set_irq_flags() from driver

[Linus Walleij]

On Thu, Mar 1, 2012 at 8:26 PM, Alan Ott <alan-yzvJWuRpmD1zbRFIqnYvSA@public.gmane.org> wrote:

> From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>
> The driver should not call set_irq_flags itself, and
> cannot do this from a loadable module.
>
> Remove the call for now, which might break the driver
> but at least lets the kernel link again.
>
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> ---
>  drivers/usb/host/ehci-tegra.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> index dbc7fe8..ab91f26 100644
> --- a/drivers/usb/host/ehci-tegra.c
> +++ b/drivers/usb/host/ehci-tegra.c
> @@ -729,7 +729,6 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>                err = -ENODEV;
>                goto fail;
>        }
> -       set_irq_flags(irq, IRQF_VALID);

The kernel is full of this kind of stuff (drivers/mfd/twl4030-irq.c):

#ifdef CONFIG_ARM
        /* ARM requires an extra step to clear IRQ_NOREQUEST, which it
         * sets on behalf of every irq_chip.  Also sets IRQ_NOPROBE.
         */
        set_irq_flags(irq, IRQF_VALID);
#else
        /* same effect on other architectures */
        irq_set_noprobe(irq);
#endif

Can't we try to fix this for real instead of breaking the universe,
hehe :-)

Yours,
Linus Walleij

Re: [PATCH 8/8] ARM: tegra: HACK: remove set_irq_flags() from driver

[Arnd Bergmann]

On Thursday 01 March 2012, Linus Walleij wrote:
> The kernel is full of this kind of stuff (drivers/mfd/twl4030-irq.c):
> 
> #ifdef CONFIG_ARM
>         /* ARM requires an extra step to clear IRQ_NOREQUEST, which it
>          * sets on behalf of every irq_chip.  Also sets IRQ_NOPROBE.
>          */
>         set_irq_flags(irq, IRQF_VALID);
> #else
>         /* same effect on other architectures */
>         irq_set_noprobe(irq);
> #endif

That is the implementation of an irq_chip, not a random device driver.

	Arnd

Re: [PATCH 8/8] ARM: tegra: HACK: remove set_irq_flags() from driver

[Russell King - ARM Linux]

On Thu, Mar 01, 2012 at 09:33:03PM +0100, Linus Walleij wrote:
> On Thu, Mar 1, 2012 at 8:26 PM, Alan Ott <alan@signal11.us> wrote:
> 
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > The driver should not call set_irq_flags itself, and
> > cannot do this from a loadable module.
> >
> > Remove the call for now, which might break the driver
> > but at least lets the kernel link again.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  drivers/usb/host/ehci-tegra.c |    1 -
> >  1 files changed, 0 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> > index dbc7fe8..ab91f26 100644
> > --- a/drivers/usb/host/ehci-tegra.c
> > +++ b/drivers/usb/host/ehci-tegra.c
> > @@ -729,7 +729,6 @@ static int tegra_ehci_probe(struct platform_device *pdev)
> >                err = -ENODEV;
> >                goto fail;
> >        }
> > -       set_irq_flags(irq, IRQF_VALID);
> 
> The kernel is full of this kind of stuff (drivers/mfd/twl4030-irq.c):
> 
> #ifdef CONFIG_ARM
>         /* ARM requires an extra step to clear IRQ_NOREQUEST, which it
>          * sets on behalf of every irq_chip.  Also sets IRQ_NOPROBE.
>          */
>         set_irq_flags(irq, IRQF_VALID);
> #else
>         /* same effect on other architectures */
>         irq_set_noprobe(irq);
> #endif
> 
> Can't we try to fix this for real instead of breaking the universe,
> hehe :-)

That depends whether you want 0..NR_IRQS to all be request-able before
they've been setup with their irq chips and handlers etc.

Unfortunately, genirq doesn't provide a standard helper to clear
IRQ_NOREQUEST, and that's why this pain persists.

[PATCH 6/8] ARM: tegra: USB_ULPI needs USB

[Alan Ott]

From: Arnd Bergmann <arnd@arndb.de>

The USB_ULPI driver is selected by tegra, but can only be
built if at least one of USB or USB_GADGET is enabled.
The kernel fails to link if both are off.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mach-tegra/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index b2a038e..7ac3011 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -10,6 +10,7 @@ config ARCH_TEGRA_2x_SOC
 	select USB_ARCH_HAS_EHCI if USB_SUPPORT
 	select USB_ULPI if USB_SUPPORT
 	select USB_ULPI_VIEWPORT if USB_SUPPORT
+	select USB_GADGET if USB_SUPPORT && !USB
 	select CPU_FREQ_TABLE if CPU_FREQ
 	help
 	  Support for NVIDIA Tegra AP20 and T20 processors, based on the
-- 
1.7.0.4

RE: [PATCH 6/8] ARM: tegra: USB_ULPI needs USB

[Stephen Warren]

Alan Ott wrote at Thursday, March 01, 2012 12:27 PM:
> From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> 
> The USB_ULPI driver is selected by tegra, but can only be
> built if at least one of USB or USB_GADGET is enabled.
> The kernel fails to link if both are off.

> diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
...
> @@ -10,6 +10,7 @@ config ARCH_TEGRA_2x_SOC
...
> +	select USB_GADGET if USB_SUPPORT && !USB

I don't believe the Tegra USB driver supports gadget mode at present.
Would the above be better written as:

    select USB if USB_SUPPORT

And I wonder if the Tegra EHCI driver shouldn't be doing some of these
selects, not the ARCH_TEGRA_* option.

And this is probably required for Tegra30 too not just Tegra20.

-- 
nvpublic

Re: [PATCH 6/8] ARM: tegra: USB_ULPI needs USB

[Arnd Bergmann]

On Thursday 01 March 2012, Stephen Warren wrote:
> Alan Ott wrote at Thursday, March 01, 2012 12:27 PM:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > The USB_ULPI driver is selected by tegra, but can only be
> > built if at least one of USB or USB_GADGET is enabled.
> > The kernel fails to link if both are off.
> 
> > diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
> ...
> > @@ -10,6 +10,7 @@ config ARCH_TEGRA_2x_SOC
> ...
> > +	select USB_GADGET if USB_SUPPORT && !USB
> 
> I don't believe the Tegra USB driver supports gadget mode at present.

ok.

> Would the above be better written as:
> 
>     select USB if USB_SUPPORT

That does not look correct either. There is no reason why USB
can't be disabled with USB_SUPPORT enabled on other platforms.

Maybe it's better to change the 'select USB_ULPI if USB_SUPPORT'
into 'select USB_ULPI if USB' instead? Or maybe just don't select
USB_ULPI to start with and put it into the defconfig instead.

> And this is probably required for Tegra30 too not just Tegra20.

Right. Tegra30 was not there when I did these patches, so it's
probably missing in other places, too.

	Arnd

RE: [PATCH 6/8] ARM: tegra: USB_ULPI needs USB

[Stephen Warren]

Arnd Bergmann wrote at Thursday, March 01, 2012 1:26 PM:
> On Thursday 01 March 2012, Stephen Warren wrote:
> > Alan Ott wrote at Thursday, March 01, 2012 12:27 PM:
> > > From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> > >
> > > The USB_ULPI driver is selected by tegra, but can only be
> > > built if at least one of USB or USB_GADGET is enabled.
> > > The kernel fails to link if both are off.
> >
> > > diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
> > ...
> > > @@ -10,6 +10,7 @@ config ARCH_TEGRA_2x_SOC
> > ...
> > > +	select USB_GADGET if USB_SUPPORT && !USB
> >
> > I don't believe the Tegra USB driver supports gadget mode at present.
> 
> ok.
> 
> > Would the above be better written as:
> >
> >     select USB if USB_SUPPORT
> 
> That does not look correct either. There is no reason why USB
> can't be disabled with USB_SUPPORT enabled on other platforms.
> 
> Maybe it's better to change the 'select USB_ULPI if USB_SUPPORT'
> into 'select USB_ULPI if USB' instead?

That sounds reasonable.

> Or maybe just don't select
> USB_ULPI to start with and put it into the defconfig instead.

Perhaps USB_EHCI_TEGRA should depend on USB, and select USB_EHCI?

-- 
nvpublic

Re: [PATCH 6/8] ARM: tegra: USB_ULPI needs USB

[Arnd Bergmann]

On Thursday 01 March 2012, Stephen Warren wrote:
> > Or maybe just don't select
> > USB_ULPI to start with and put it into the defconfig instead.
> 
> Perhaps USB_EHCI_TEGRA should depend on USB, and select USB_EHCI?

I think it should, but that is a different issue. All EHCI bus
glues currently depend on USB_EHCI_HCD instead of selecting it.

Changing that would likely break a number of other things that
have come to depend on the current behavior.

	Arnd

[PATCH 7/8] ARM: tegra: export usb phy symbols

[Alan Ott]

From: Arnd Bergmann <arnd@arndb.de>

The ehci driver can be a module, so the functions provided
by the tegra platform code used by ehci-tegra need to
be exported.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mach-tegra/usb_phy.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-tegra/usb_phy.c b/arch/arm/mach-tegra/usb_phy.c
index 37576a7..3c45264 100644
--- a/arch/arm/mach-tegra/usb_phy.c
+++ b/arch/arm/mach-tegra/usb_phy.c
@@ -730,6 +730,7 @@ err0:
 	kfree(phy);
 	return ERR_PTR(err);
 }
+EXPORT_SYMBOL_GPL(tegra_usb_phy_open);
 
 int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
 {
@@ -738,6 +739,7 @@ int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
 	else
 		return utmi_phy_power_on(phy);
 }
+EXPORT_SYMBOL_GPL(tegra_usb_phy_power_on);
 
 void tegra_usb_phy_power_off(struct tegra_usb_phy *phy)
 {
@@ -746,18 +748,21 @@ void tegra_usb_phy_power_off(struct tegra_usb_phy *phy)
 	else
 		utmi_phy_power_off(phy);
 }
+EXPORT_SYMBOL_GPL(tegra_usb_phy_power_off);
 
 void tegra_usb_phy_preresume(struct tegra_usb_phy *phy)
 {
 	if (!phy_is_ulpi(phy))
 		utmi_phy_preresume(phy);
 }
+EXPORT_SYMBOL_GPL(tegra_usb_phy_preresume);
 
 void tegra_usb_phy_postresume(struct tegra_usb_phy *phy)
 {
 	if (!phy_is_ulpi(phy))
 		utmi_phy_postresume(phy);
 }
+EXPORT_SYMBOL_GPL(tegra_usb_phy_postresume);
 
 void tegra_ehci_phy_restore_start(struct tegra_usb_phy *phy,
 				 enum tegra_usb_phy_port_speed port_speed)
@@ -765,24 +770,28 @@ void tegra_ehci_phy_restore_start(struct tegra_usb_phy *phy,
 	if (!phy_is_ulpi(phy))
 		utmi_phy_restore_start(phy, port_speed);
 }
+EXPORT_SYMBOL_GPL(tegra_ehci_phy_restore_start);
 
 void tegra_ehci_phy_restore_end(struct tegra_usb_phy *phy)
 {
 	if (!phy_is_ulpi(phy))
 		utmi_phy_restore_end(phy);
 }
+EXPORT_SYMBOL_GPL(tegra_ehci_phy_restore_end);
 
 void tegra_usb_phy_clk_disable(struct tegra_usb_phy *phy)
 {
 	if (!phy_is_ulpi(phy))
 		utmi_phy_clk_disable(phy);
 }
+EXPORT_SYMBOL_GPL(tegra_usb_phy_clk_disable);
 
 void tegra_usb_phy_clk_enable(struct tegra_usb_phy *phy)
 {
 	if (!phy_is_ulpi(phy))
 		utmi_phy_clk_enable(phy);
 }
+EXPORT_SYMBOL_GPL(tegra_usb_phy_clk_enable);
 
 void tegra_usb_phy_close(struct tegra_usb_phy *phy)
 {
@@ -794,3 +803,4 @@ void tegra_usb_phy_close(struct tegra_usb_phy *phy)
 	clk_put(phy->pll_u);
 	kfree(phy);
 }
+EXPORT_SYMBOL_GPL(tegra_usb_phy_close);
-- 
1.7.0.4

RE: [PATCH 7/8] ARM: tegra: export usb phy symbols

[Stephen Warren]

Alan Ott wrote at Thursday, March 01, 2012 12:27 PM:
> From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> 
> The ehci driver can be a module, so the functions provided
> by the tegra platform code used by ehci-tegra need to
> be exported.
> 
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

I think this one need to add:

#include <linux/export.h>

In the relevant place.

-- 
nvpublic

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] ARM: Tegra: cleanup patches from Arnd Bergmann

[Arnd Bergmann]

On Thursday 01 March 2012, Alan Ott wrote:
> I'm helping Arnd Bergmann get some of his cleanup patches upstream. This
> set covers the Tegra.
> 
> Thanks,
> 
> Alan.
> 
> Arnd Bergmann (8):
>   ARM: tegra: export tegra_gpio_{en,dis}able
>   ARM: tegra: do not hide dma declarations
>   ARM: tegra: select CPU_FREQ_TABLE
>   ARM: tegra: build localtimer support only when needed
>   ARM: tegra: work around tegra THUMB2_KERNEL bug
>   ARM: tegra: USB_ULPI needs USB
>   ARM: tegra: export usb phy symbols
>   ARM: tegra: HACK: remove set_irq_flags() from driver
> 

Hi Alan,

Thanks for getting this started!

As Greg already commented, there are some patches that you
should probably drop. When you send out a series with the
name of a subarchitecture, it's best to leave out the patches
that only address device drivers. I had initially sorted these
into the subarch series instead of the drivers series but that
turned out to be a bad idea. Sorry I didn't remember about this
detail when we first discussed the way forward on my patches.

Also, it's better if you have a slightly shorter Cc list and
for each patch take only the people on Cc that are actually
interested, rather than send the whole series to everyone
that get_maintainer.pl lists for one of the patches. If the
Cc list gets too long, that often means that the mails get
dropped from the mailing lists, and people might ignore them
when they don't understand why they were on Cc to start with.
It does help to have everyone on Cc for the initial mail though.

You can add 'Cc:' tags in the individual patches to help you
with that. This is another thing I never got around to for
the patches I did not send out.

Finally, when sending patches from someone else, you have
to add your own Signed-off-by: line below the others (i.e. mine).
Everyone who is handling the patch by sending it to someone
else should have a Signed-off-by in the email until the
point when it gets applied to a git tree that is pulled into
the Linus' tree.

	Arnd