[patch-2.6.35-rc3+ 0/8] musb patches

[patch-2.6.35-rc3+ 0/8] musb patches

[Ajay Kumar Gupta]

Greg,

These are reviewed and non-controversial musb bugfix patches. Sanity have
been tested on OMAP3EVM for Host-MSC, HID, audio, video class and
device-CDC class.

Thanks,
Ajay

Ajay Kumar Gupta (1):
  musb: fix compilation warning in host only mode

Anand Gadiyar (1):
  musb: Kill board specific pinmux from driver file

Hema HK (1):
  usb: musb: Enable the maximum supported burst mode for DMA

Jon Povey (1):
  USB: musb: suppress warning about unused flags

Maulik Mankad (1):
  usb: musb: Fix suspend interrupt issue in device mode

Mike Frysinger (1):
  usb: musb: fix Blackfin ulpi stubs

Sergei Shtylyov (2):
  musb_core: make disconnect and suspend interrupts work again
  MUSB: make non-OMAP platforms build with CONFIG_PM=y

 drivers/usb/musb/cppi_dma.c  |    2 +-
 drivers/usb/musb/musb_core.c |   23 ++++++++---------------
 drivers/usb/musb/musb_core.h |    6 ++++--
 drivers/usb/musb/musbhsdma.c |   13 ++-----------
 drivers/usb/musb/omap2430.c  |    6 ------
 5 files changed, 15 insertions(+), 35 deletions(-)

[PATCH 1/8] musb_core: make disconnect and suspend interrupts work again

[Ajay Kumar Gupta]

From: Sergei Shtylyov <sshtylyov@ru.mvista.com>

Commit 1c25fda4a09e8229800979986ef399401053b46e (usb: musb: handle irqs in the
order dictated by programming guide) forgot to get rid of the old 'STAGE0_MASK'
filter for calling musb_stage0_irq(), so now disconnect and suspend interrupts
are effectively ignored...

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Cc: stable@kernel.org
Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
---
 drivers/usb/musb/musb_core.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index fad70bc..f0ff893 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -451,10 +451,6 @@ void musb_hnp_stop(struct musb *musb)
  * @param power
  */
 
-#define STAGE0_MASK (MUSB_INTR_RESUME | MUSB_INTR_SESSREQ \
-		| MUSB_INTR_VBUSERROR | MUSB_INTR_CONNECT \
-		| MUSB_INTR_RESET)
-
 static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 				u8 devctl, u8 power)
 {
@@ -1597,7 +1593,7 @@ irqreturn_t musb_interrupt(struct musb *musb)
 	/* the core can interrupt us for multiple reasons; docs have
 	 * a generic interrupt flowchart to follow
 	 */
-	if (musb->int_usb & STAGE0_MASK)
+	if (musb->int_usb)
 		retval |= musb_stage0_irq(musb, musb->int_usb,
 				devctl, power);
 
-- 
1.6.2.4

[PATCH 2/8] MUSB: make non-OMAP platforms build with CONFIG_PM=y

[Ajay Kumar Gupta]

From: Sergei Shtylyov <sshtylyov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>

Attempt to build MUSB driver with CONFIG_PM=y (e.g. in the OTG mode) on DaVinci
results in these link errors:

drivers/built-in.o: In function `musb_restore_context':
led-triggers.c:(.text+0x714d8): undefined reference to
`musb_platform_restore_context'
drivers/built-in.o: In function `musb_save_context':
led-triggers.c:(.text+0x71788): undefined reference to
`musb_platform_save_context'

This turned out to be caused by commit 9957dd97ec5e98dd334f87ade1d9a0b24d1f86eb
(usb: musb: Fix compile error for omaps for musb_hdrc). Revert it, taking into
account the rename of CONFIG_ARCH_OMAP34XX into CONFIG_ARCH_OMAP3 (which that
commit fixed in a completely inappropriate way) and the recent addition of
OMAP4 support.

Signed-off-by: Sergei Shtylyov <sshtylyov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
---
 drivers/usb/musb/musb_core.h |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index b22d02d..91d6779 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -470,7 +470,8 @@ struct musb_csr_regs {
 
 struct musb_context_registers {
 
-#ifdef CONFIG_PM
+#if defined(CONFIG_ARCH_OMAP2430) || defined(CONFIG_ARCH_OMAP3) || \
+    defined(CONFIG_ARCH_OMAP4)
 	u32 otg_sysconfig, otg_forcestandby;
 #endif
 	u8 power;
@@ -484,7 +485,8 @@ struct musb_context_registers {
 	struct musb_csr_regs index_regs[MUSB_C_NUM_EPS];
 };
 
-#ifdef CONFIG_PM
+#if defined(CONFIG_ARCH_OMAP2430) || defined(CONFIG_ARCH_OMAP3) || \
+    defined(CONFIG_ARCH_OMAP4)
 extern void musb_platform_save_context(struct musb *musb,
 		struct musb_context_registers *musb_context);
 extern void musb_platform_restore_context(struct musb *musb,
-- 
1.6.2.4

--
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 3/8] musb: fix compilation warning in host only mode

[Ajay Kumar Gupta]

Fixes below compilation warning when host only configuration is
selected.
drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq':
drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase'

Also removed definition of 'mbase' from multiple places to only
at function top.

Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
---
 drivers/usb/musb/musb_core.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index f0ff893..7cc8398 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -455,6 +455,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 				u8 devctl, u8 power)
 {
 	irqreturn_t handled = IRQ_NONE;
+#ifdef CONFIG_USB_MUSB_HDRC_HCD
+	void __iomem	*mbase = musb->mregs;
+#endif
 
 	DBG(3, "<== Power=%02x, DevCtl=%02x, int_usb=0x%x\n", power, devctl,
 		int_usb);
@@ -469,8 +472,6 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 
 		if (devctl & MUSB_DEVCTL_HM) {
 #ifdef CONFIG_USB_MUSB_HDRC_HCD
-			void __iomem *mbase = musb->mregs;
-
 			switch (musb->xceiv->state) {
 			case OTG_STATE_A_SUSPEND:
 				/* remote wakeup?  later, GetPortStatus
@@ -548,8 +549,6 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 #ifdef CONFIG_USB_MUSB_HDRC_HCD
 	/* see manual for the order of the tests */
 	if (int_usb & MUSB_INTR_SESSREQ) {
-		void __iomem *mbase = musb->mregs;
-
 		DBG(1, "SESSION_REQUEST (%s)\n", otg_state_string(musb));
 
 		/* IRQ arrives from ID pin sense or (later, if VBUS power
@@ -598,8 +597,6 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 		case OTG_STATE_A_WAIT_BCON:
 		case OTG_STATE_A_WAIT_VRISE:
 			if (musb->vbuserr_retry) {
-				void __iomem *mbase = musb->mregs;
-
 				musb->vbuserr_retry--;
 				ignore = 1;
 				devctl |= MUSB_DEVCTL_SESSION;
@@ -703,7 +700,6 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 
 	if (int_usb & MUSB_INTR_CONNECT) {
 		struct usb_hcd *hcd = musb_to_hcd(musb);
-		void __iomem *mbase = musb->mregs;
 
 		handled = IRQ_HANDLED;
 		musb->is_active = 1;
-- 
1.6.2.4

[PATCH 4/8] USB: musb: suppress warning about unused flags

[Ajay Kumar Gupta]

From: Jon Povey <jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org>

Wrap flags with uninitialized_var() to suppress this:

drivers/usb/musb/cppi_dma.c:1158: warning: 'flags' may be used uninitialized
in this function

Signed-off-by: Jon Povey <jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org>
Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
---
 drivers/usb/musb/cppi_dma.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/musb/cppi_dma.c b/drivers/usb/musb/cppi_dma.c
index 59dc3d3..e3753ba 100644
--- a/drivers/usb/musb/cppi_dma.c
+++ b/drivers/usb/musb/cppi_dma.c
@@ -1155,7 +1155,7 @@ irqreturn_t cppi_interrupt(int irq, void *dev_id)
 	struct musb_hw_ep	*hw_ep = NULL;
 	u32			rx, tx;
 	int			i, index;
-	unsigned long		flags;
+	unsigned long		uninitialized_var(flags);
 
 	cppi = container_of(musb->dma_controller, struct cppi, controller);
 	if (cppi->irq)
-- 
1.6.2.4

--
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] usb: musb: fix Blackfin ulpi stubs

[Ajay Kumar Gupta]

From: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>

The new ulpi code defines fallback stubs for the Blackfin arch, but does
so incorrectly leading to a build failure:
drivers/usb/musb/musb_core.c:227: error: 'musb_ulpi_read' undeclared here (not in a function)
drivers/usb/musb/musb_core.c:228: error: 'musb_ulpi_write' undeclared here (not in a function)

Tweak the fallback stubs so that they do work as intended.

Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
---
 drivers/usb/musb/musb_core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 7cc8398..4f43db7 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -219,8 +219,8 @@ static int musb_ulpi_write(struct otg_transceiver *otg,
 	return 0;
 }
 #else
-#define musb_ulpi_read(a, b)		NULL
-#define musb_ulpi_write(a, b, c)	NULL
+#define musb_ulpi_read		NULL
+#define musb_ulpi_write		NULL
 #endif
 
 static struct otg_io_access_ops musb_ulpi_access = {
-- 
1.6.2.4

--
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 6/8] usb: musb: Enable the maximum supported burst mode for DMA

[Ajay Kumar Gupta]

From: Hema HK <hemahk@ti.com>

Setting MUSB Burst Mode 3 automatically enables support for
lower burst modes (BURST4, BURST8, BURST16 or bursts of unspecified
length). There is no need to set these burst modes based on the
packet size. Also enable the burst mode for both mode1 and mode0.

This is a fix for buggy hardware - having the lower burst modes
enabled can potentially cause lockups of the DMA engine used in
OMAP2/3/4 chips.

Signed-off-by: Hema HK <hemahk@ti.com>
Signed-off-by: Anand Gadiyar <gadiyar@ti.com>
Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
---
 drivers/usb/musb/musbhsdma.c |   13 ++-----------
 1 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c
index 1008044..dc66e43 100644
--- a/drivers/usb/musb/musbhsdma.c
+++ b/drivers/usb/musb/musbhsdma.c
@@ -132,18 +132,9 @@ static void configure_channel(struct dma_channel *channel,
 	if (mode) {
 		csr |= 1 << MUSB_HSDMA_MODE1_SHIFT;
 		BUG_ON(len < packet_sz);
-
-		if (packet_sz >= 64) {
-			csr |= MUSB_HSDMA_BURSTMODE_INCR16
-					<< MUSB_HSDMA_BURSTMODE_SHIFT;
-		} else if (packet_sz >= 32) {
-			csr |= MUSB_HSDMA_BURSTMODE_INCR8
-					<< MUSB_HSDMA_BURSTMODE_SHIFT;
-		} else if (packet_sz >= 16) {
-			csr |= MUSB_HSDMA_BURSTMODE_INCR4
-					<< MUSB_HSDMA_BURSTMODE_SHIFT;
-		}
 	}
+	csr |= MUSB_HSDMA_BURSTMODE_INCR16
+				<< MUSB_HSDMA_BURSTMODE_SHIFT;
 
 	csr |= (musb_channel->epnum << MUSB_HSDMA_ENDPOINT_SHIFT)
 		| (1 << MUSB_HSDMA_ENABLE_SHIFT)
-- 
1.6.2.4

[PATCH 7/8] musb: Kill board specific pinmux from driver file

[Ajay Kumar Gupta]

From: Anand Gadiyar <gadiyar-l0cyMroinI0@public.gmane.org>

This pin-muxing is best done in the board files. The driver should
not do this explicitly.

Also, this code causes a warning to be thrown when OMAP2430 and OMAP3/4
support are enabled in the same kernel.

Signed-off-by: Anand Gadiyar <gadiyar-l0cyMroinI0@public.gmane.org>
Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
---
 drivers/usb/musb/omap2430.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index e06d65e..2111a24 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -32,8 +32,6 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 
-#include <plat/mux.h>
-
 #include "musb_core.h"
 #include "omap2430.h"
 
@@ -194,10 +192,6 @@ int __init musb_platform_init(struct musb *musb, void *board_data)
 	u32 l;
 	struct omap_musb_board_data *data = board_data;
 
-#if defined(CONFIG_ARCH_OMAP2430)
-	omap_cfg_reg(AE5_2430_USB0HS_STP);
-#endif

[PATCH 8/8] usb: musb: Fix suspend interrupt issue in device mode

[Ajay Kumar Gupta]

From: Maulik Mankad <x0082077@ti.com>

As a part of aligning the ISR code for MUSB with the specs, the
ISR code was re-written.

See Commit 1c25fda4a09e8229800979986ef399401053b46e (usb: musb: handle
irqs in the order dictated by programming guide)

With this the suspend interrupt came accidently under CONFIG_USB_MUSB_HDRC_HCD.

The fix brings suspend interrupt handling outside
CONFIG_USB_MUSB_HDRC_HCD.

Signed-off-by: Maulik Mankad <x0082077@ti.com>
Acked-by: Felipe Balbi <felipe.balbi@nokia.com>
Cc: David Brownell <david-b@pacbell.net>
Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
---
 drivers/usb/musb/musb_core.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 4f43db7..64b08f9 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -635,7 +635,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 		handled = IRQ_HANDLED;
 	}
 
-
+#endif
 	if (int_usb & MUSB_INTR_SUSPEND) {
 		DBG(1, "SUSPEND (%s) devctl %02x power %02x\n",
 				otg_state_string(musb), devctl, power);
@@ -698,6 +698,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 		}
 	}
 
+#ifdef CONFIG_USB_MUSB_HDRC_HCD
 	if (int_usb & MUSB_INTR_CONNECT) {
 		struct usb_hcd *hcd = musb_to_hcd(musb);
 
-- 
1.6.2.4

[PATCH 8/8] usb: musb: Fix suspend interrupt issue in device mode

[Ajay Kumar Gupta]

From: Maulik Mankad <x0082077-l0cyMroinI0@public.gmane.org>

As a part of aligning the ISR code for MUSB with the specs, the
ISR code was re-written.

See Commit 1c25fda4a09e8229800979986ef399401053b46e (usb: musb: handle
irqs in the order dictated by programming guide)

With this the suspend interrupt came accidently under CONFIG_USB_MUSB_HDRC_HCD.

The fix brings suspend interrupt handling outside
CONFIG_USB_MUSB_HDRC_HCD.

Signed-off-by: Maulik Mankad <x0082077-l0cyMroinI0@public.gmane.org>
Acked-by: Felipe Balbi <felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
Cc: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
---
 drivers/usb/musb/musb_core.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 4f43db7..64b08f9 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -635,7 +635,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 		handled = IRQ_HANDLED;
 	}
 

Re: [PATCH 8/8] usb: musb: Fix suspend interrupt issue in device mode

[Sergei Shtylyov]

Hello.

Ajay Kumar Gupta wrote:

> From: Maulik Mankad <x0082077-l0cyMroinI0@public.gmane.org>

> As a part of aligning the ISR code for MUSB with the specs, the
> ISR code was re-written.

> See Commit 1c25fda4a09e8229800979986ef399401053b46e (usb: musb: handle
> irqs in the order dictated by programming guide)

> With this the suspend interrupt came accidently under CONFIG_USB_MUSB_HDRC_HCD.

> The fix brings suspend interrupt handling outside
> CONFIG_USB_MUSB_HDRC_HCD.

> Signed-off-by: Maulik Mankad <x0082077-l0cyMroinI0@public.gmane.org>
> Acked-by: Felipe Balbi <felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> Cc: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
> Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>

[...]

> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 4f43db7..64b08f9 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -635,7 +635,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
>  		handled = IRQ_HANDLED;
>  	}
>  
> -
> +#endif

    Could you move #endif one line up, so that it closely embraces the *if* 
statement?

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

RE: [PATCH 8/8] usb: musb: Fix suspend interrupt issue in device mode

[Maulik]

> -----Original Message-----
> From: Sergei Shtylyov [mailto:sshtylyov-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org]
> Sent: Thursday, June 17, 2010 4:57 PM
> To: Ajay Kumar Gupta
> Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org; gregkh-l3A5Bk7waGM@public.gmane.org; Maulik Mankad; David Brownell
> Subject: Re: [PATCH 8/8] usb: musb: Fix suspend interrupt issue in device
> mode
> 
> Hello.
> 
> Ajay Kumar Gupta wrote:
> 
> > From: Maulik Mankad <x0082077-l0cyMroinI0@public.gmane.org>
> 
> > As a part of aligning the ISR code for MUSB with the specs, the
> > ISR code was re-written.
> 
> > See Commit 1c25fda4a09e8229800979986ef399401053b46e (usb: musb: handle
> > irqs in the order dictated by programming guide)
> 
> > With this the suspend interrupt came accidently under
> CONFIG_USB_MUSB_HDRC_HCD.
> 
> > The fix brings suspend interrupt handling outside
> > CONFIG_USB_MUSB_HDRC_HCD.
> 
> > Signed-off-by: Maulik Mankad <x0082077-l0cyMroinI0@public.gmane.org>
> > Acked-by: Felipe Balbi <felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> > Cc: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
> > Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
> 
> [...]
> 
> > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> > index 4f43db7..64b08f9 100644
> > --- a/drivers/usb/musb/musb_core.c
> > +++ b/drivers/usb/musb/musb_core.c
> > @@ -635,7 +635,7 @@ static irqreturn_t musb_stage0_irq(struct musb
> *musb, u8 int_usb,
> >  		handled = IRQ_HANDLED;
> >  	}
> >
> > -
> > +#endif
> 
>     Could you move #endif one line up, so that it closely embraces the
> *if*
> statement?

The patch is already in Greg's queue. 

Greg,

Do you want me to resend this patch?

Thanks,
Maulik

--
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 8/8] usb: musb: Fix suspend interrupt issue in device mode

[Sergei Shtylyov]

Hello.

Maulik wrote:

>> -----Original Message-----
>> From: Sergei Shtylyov [mailto:sshtylyov@mvista.com]
>> Sent: Thursday, June 17, 2010 4:57 PM
>> To: Ajay Kumar Gupta
>> Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org;
>> felipe.balbi@nokia.com; gregkh@suse.de; Maulik Mankad; David Brownell
>> Subject: Re: [PATCH 8/8] usb: musb: Fix suspend interrupt issue in device
>> mode
>>
>> Hello.
>>
>> Ajay Kumar Gupta wrote:
>>
>>> From: Maulik Mankad <x0082077@ti.com>
>>> As a part of aligning the ISR code for MUSB with the specs, the
>>> ISR code was re-written.
>>> See Commit 1c25fda4a09e8229800979986ef399401053b46e (usb: musb: handle
>>> irqs in the order dictated by programming guide)
>>> With this the suspend interrupt came accidently under
>> CONFIG_USB_MUSB_HDRC_HCD.
>>
>>> The fix brings suspend interrupt handling outside
>>> CONFIG_USB_MUSB_HDRC_HCD.
>>> Signed-off-by: Maulik Mankad <x0082077@ti.com>
>>> Acked-by: Felipe Balbi <felipe.balbi@nokia.com>
>>> Cc: David Brownell <david-b@pacbell.net>
>>> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
>> [...]
>>
>>> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
>>> index 4f43db7..64b08f9 100644
>>> --- a/drivers/usb/musb/musb_core.c
>>> +++ b/drivers/usb/musb/musb_core.c
>>> @@ -635,7 +635,7 @@ static irqreturn_t musb_stage0_irq(struct musb
>> *musb, u8 int_usb,
>>>  		handled = IRQ_HANDLED;
>>>  	}
>>>
>>> -
>>> +#endif

>>     Could you move #endif one line up, so that it closely embraces the
>> *if* statement?

> The patch is already in Greg's queue. 

    Hm, how come it ended up there before the patches submitted earlier 
-- namely before the patch that's needed for the suspend interrupt to be 
handled at all? :-/
    Felipe, could you also ACK the other pending patches, not this 
single one?

> Greg,

> Do you want me to resend this patch?

> Thanks,
> Maulik

WBR, Sergei

Re: [PATCH 8/8] usb: musb: Fix suspend interrupt issue in device mode

[Greg KH]

On Thu, Jun 17, 2010 at 05:45:15PM +0530, Maulik wrote:
> > -----Original Message-----
> > From: Sergei Shtylyov [mailto:sshtylyov-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org]
> > Sent: Thursday, June 17, 2010 4:57 PM
> > To: Ajay Kumar Gupta
> > Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> > felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org; gregkh-l3A5Bk7waGM@public.gmane.org; Maulik Mankad; David Brownell
> > Subject: Re: [PATCH 8/8] usb: musb: Fix suspend interrupt issue in device
> > mode
> > 
> > Hello.
> > 
> > Ajay Kumar Gupta wrote:
> > 
> > > From: Maulik Mankad <x0082077-l0cyMroinI0@public.gmane.org>
> > 
> > > As a part of aligning the ISR code for MUSB with the specs, the
> > > ISR code was re-written.
> > 
> > > See Commit 1c25fda4a09e8229800979986ef399401053b46e (usb: musb: handle
> > > irqs in the order dictated by programming guide)
> > 
> > > With this the suspend interrupt came accidently under
> > CONFIG_USB_MUSB_HDRC_HCD.
> > 
> > > The fix brings suspend interrupt handling outside
> > > CONFIG_USB_MUSB_HDRC_HCD.
> > 
> > > Signed-off-by: Maulik Mankad <x0082077-l0cyMroinI0@public.gmane.org>
> > > Acked-by: Felipe Balbi <felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> > > Cc: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
> > > Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
> > 
> > [...]
> > 
> > > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> > > index 4f43db7..64b08f9 100644
> > > --- a/drivers/usb/musb/musb_core.c
> > > +++ b/drivers/usb/musb/musb_core.c
> > > @@ -635,7 +635,7 @@ static irqreturn_t musb_stage0_irq(struct musb
> > *musb, u8 int_usb,
> > >  		handled = IRQ_HANDLED;
> > >  	}
> > >
> > > -
> > > +#endif
> > 
> >     Could you move #endif one line up, so that it closely embraces the
> > *if*
> > statement?
> 
> The patch is already in Greg's queue. 
> 
> Greg,
> 
> Do you want me to resend this patch?

No, it's already in my queue, so I don't need to reapply it :)

thanks,

greg k-h
--
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 8/8] usb: musb: Fix suspend interrupt issue in device mode

[David Brownell]

> As a part of aligning the ISR code for MUSB with the specs,the
> ISR code was re-written.
> 
>
Best say "re-aligning".  When that code was merged, it was fully aligned with a version of the MUSB
specification.

Alternatively, specify which version of which spec
was used this time around ... :)

Re: [PATCH 7/8] musb: Kill board specific pinmux from driver file

[Felipe Balbi]

On Thu, Jun 17, 2010 at 12:40:40PM +0200, ext Ajay Kumar Gupta wrote:
>From: Anand Gadiyar <gadiyar-l0cyMroinI0@public.gmane.org>
>
>This pin-muxing is best done in the board files. The driver should
>not do this explicitly.
>
>Also, this code causes a warning to be thrown when OMAP2430 and OMAP3/4
>support are enabled in the same kernel.
>
>Signed-off-by: Anand Gadiyar <gadiyar-l0cyMroinI0@public.gmane.org>
>Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>

Acked-by: Felipe Balbi <felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>

Note this is not a bugfix, so should wait until next merge window.

-- 
balbi

DefectiveByDesign.org
--
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 7/8] musb: Kill board specific pinmux from driver file

[Gadiyar, Anand]

Felipe Balbi wrote:
> On Thu, Jun 17, 2010 at 12:40:40PM +0200, ext Ajay Kumar Gupta wrote:
> >From: Anand Gadiyar <gadiyar@ti.com>
> >
> >This pin-muxing is best done in the board files. The driver should
> >not do this explicitly.
> >
> >Also, this code causes a warning to be thrown when OMAP2430 and OMAP3/4
> >support are enabled in the same kernel.
> >
> >Signed-off-by: Anand Gadiyar <gadiyar@ti.com>
> >Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> 
> Acked-by: Felipe Balbi <felipe.balbi@nokia.com>
> 
> Note this is not a bugfix, so should wait until next merge window.
> 

It's a bug-fix to the extent that it caused a WARN() to be triggered
at bootup. But it's not serious and can wait.

- Anand

Re: [PATCH 6/8] usb: musb: Enable the maximum supported burst mode for DMA

[Felipe Balbi]

On Thu, Jun 17, 2010 at 12:40:39PM +0200, ext Ajay Kumar Gupta wrote:
>From: Hema HK <hemahk-l0cyMroinI0@public.gmane.org>
>
>Setting MUSB Burst Mode 3 automatically enables support for
>lower burst modes (BURST4, BURST8, BURST16 or bursts of unspecified
>length). There is no need to set these burst modes based on the
>packet size. Also enable the burst mode for both mode1 and mode0.
>
>This is a fix for buggy hardware - having the lower burst modes
>enabled can potentially cause lockups of the DMA engine used in
>OMAP2/3/4 chips.
>
>Signed-off-by: Hema HK <hemahk-l0cyMroinI0@public.gmane.org>
>Signed-off-by: Anand Gadiyar <gadiyar-l0cyMroinI0@public.gmane.org>
>Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>

Acked-by: Felipe Balbi <felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>

-- 
balbi

DefectiveByDesign.org
--
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] usb: musb: fix Blackfin ulpi stubs

[Felipe Balbi]

On Thu, Jun 17, 2010 at 12:40:38PM +0200, ext Ajay Kumar Gupta wrote:
>From: Mike Frysinger <vapier@gentoo.org>
>
>The new ulpi code defines fallback stubs for the Blackfin arch, but does
>so incorrectly leading to a build failure:
>drivers/usb/musb/musb_core.c:227: error: 'musb_ulpi_read' undeclared here (not in a function)
>drivers/usb/musb/musb_core.c:228: error: 'musb_ulpi_write' undeclared here (not in a function)
>
>Tweak the fallback stubs so that they do work as intended.
>
>Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>

Acked-by: Felipe Balbi <felipe.balbi@nokia.com>

-- 
balbi

DefectiveByDesign.org

Re: [PATCH 4/8] USB: musb: suppress warning about unused flags

[Felipe Balbi]

On Thu, Jun 17, 2010 at 12:40:37PM +0200, ext Ajay Kumar Gupta wrote:
>From: Jon Povey <jon.povey@racelogic.co.uk>
>
>Wrap flags with uninitialized_var() to suppress this:
>
>drivers/usb/musb/cppi_dma.c:1158: warning: 'flags' may be used uninitialized
>in this function
>
>Signed-off-by: Jon Povey <jon.povey@racelogic.co.uk>
>Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
>---
> drivers/usb/musb/cppi_dma.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
>diff --git a/drivers/usb/musb/cppi_dma.c b/drivers/usb/musb/cppi_dma.c
>index 59dc3d3..e3753ba 100644
>--- a/drivers/usb/musb/cppi_dma.c
>+++ b/drivers/usb/musb/cppi_dma.c
>@@ -1155,7 +1155,7 @@ irqreturn_t cppi_interrupt(int irq, void *dev_id)
> 	struct musb_hw_ep	*hw_ep = NULL;
> 	u32			rx, tx;
> 	int			i, index;
>-	unsigned long		flags;
>+	unsigned long		uninitialized_var(flags);

I think the problem here is different. First of all, an IRQ of number 0 
is valid and your locking would be broken in that case. Is there really 
any board that doesn't have the irq line routed ? To me it just looks 
that branching if (irq) is completely bogus.

-- 
balbi

DefectiveByDesign.org

Re: [PATCH 4/8] USB: musb: suppress warning about unused flags

[Sergei Shtylyov]

Hello.

Felipe Balbi wrote:

>> From: Jon Povey <jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org>

>> Wrap flags with uninitialized_var() to suppress this:

>> drivers/usb/musb/cppi_dma.c:1158: warning: 'flags' may be used 
>> uninitialized
>> in this function

>> Signed-off-by: Jon Povey <jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org>
>> Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
>> ---
>> drivers/usb/musb/cppi_dma.c |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)

>> diff --git a/drivers/usb/musb/cppi_dma.c b/drivers/usb/musb/cppi_dma.c
>> index 59dc3d3..e3753ba 100644
>> --- a/drivers/usb/musb/cppi_dma.c
>> +++ b/drivers/usb/musb/cppi_dma.c
>> @@ -1155,7 +1155,7 @@ irqreturn_t cppi_interrupt(int irq, void *dev_id)
>>     struct musb_hw_ep    *hw_ep = NULL;
>>     u32            rx, tx;
>>     int            i, index;
>> -    unsigned long        flags;
>> +    unsigned long        uninitialized_var(flags);

> I think the problem here is different. First of all, an IRQ of number 0 
> is valid

    It's not valid for this driver.

> and your locking would be broken in that case. Is there really 
> any board that doesn't have the irq line routed ?

    Only DM646x have dedicated CPPI IRQ, for other DaVincis MUSB/CPPI IRQ is 
shared.

> To me it just looks  hat branching if (irq) is completely bogus.

    Look at dma_controller_create() in this file.

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

Re: [PATCH 4/8] USB: musb: suppress warning about unused flags

[Felipe Balbi]

On Thu, Jun 24, 2010 at 01:50:04PM +0400, Sergei Shtylyov wrote:
> > I think the problem here is different. First of all, an IRQ of number 0 
> > is valid
> 
>     It's not valid for this driver.

it's not valid for the boards we have available so far.

-- 
balbi
--
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 4/8] USB: musb: suppress warning about unused flags

[Sergei Shtylyov]

Hello.

Felipe Balbi wrote:

>>> I think the problem here is different. First of all, an IRQ of number 0 
>>> is valid

>>     It's not valid for this driver.

> it's not valid for the boards we have available so far.

    And the current driver doesn't accept it as a valid IRQ.

WBR, Sergei

Re: [PATCH 3/8] musb: fix compilation warning in host only mode

[Sergei Shtylyov]

Hello.

Ajay Kumar Gupta wrote:

> Fixes below compilation warning when host only configuration is
> selected.
> drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq':
> drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase'

> Also removed definition of 'mbase' from multiple places to only
> at function top.

    AFAIR, it was intentionally removed from the function top and declared in 
the multiple plcase instead by the former Felipe's patch [1] to fix exactly 
the same issue, if I don't mistake. So, it hasn't worked out?

> Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
[...]
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index f0ff893..7cc8398 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -455,6 +455,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
>  				u8 devctl, u8 power)
>  {
>  	irqreturn_t handled = IRQ_NONE;
> +#ifdef CONFIG_USB_MUSB_HDRC_HCD
> +	void __iomem	*mbase = musb->mregs;
> +#endif

    I'd rather see it declared multiple times...

> @@ -703,7 +700,6 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
>  
>  	if (int_usb & MUSB_INTR_CONNECT) {
>  		struct usb_hcd *hcd = musb_to_hcd(musb);
> -		void __iomem *mbase = musb->mregs;

    We could also move this into the #ifdef CONFIG_USB_MUSB_OTG block.

>  
>  		handled = IRQ_HANDLED;
>  		musb->is_active = 1;

[1] 
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=aa4714560b4ea359bb7830188ebd06bce71bcdea

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

RE: [PATCH 3/8] musb: fix compilation warning in host only mode

[Gupta, Ajay Kumar]

> > Fixes below compilation warning when host only configuration is
> > selected.
> > drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq':
> > drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase'
> 
> > Also removed definition of 'mbase' from multiple places to only
> > at function top.
> 
>     AFAIR, it was intentionally removed from the function top and declared
> in the multiple plcase instead by the former Felipe's patch [1] to fix
> exactly the same issue, if I don't mistake. So, it hasn't worked out?

Yes, it was removed by Felipe's below patch but it introduced
compilation warning issue as reported.

---------------
commit aa4714560b4ea359bb7830188ebd06bce71bcdea
usb: musb: core: declare mbase only where it's used

... and avoid a compilation if we disable host side
of musb.
--------------

> 
> > Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> [...]
> > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> > index f0ff893..7cc8398 100644
> > --- a/drivers/usb/musb/musb_core.c
> > +++ b/drivers/usb/musb/musb_core.c
> > @@ -455,6 +455,9 @@ static irqreturn_t musb_stage0_irq(struct musb
> *musb, u8 int_usb,
> >  				u8 devctl, u8 power)
> >  {
> >  	irqreturn_t handled = IRQ_NONE;
> > +#ifdef CONFIG_USB_MUSB_HDRC_HCD
> > +	void __iomem	*mbase = musb->mregs;
> > +#endif
> 
>     I'd rather see it declared multiple times...

I was thinking it's better to have it at one place and avoid multiple
#ifdefs.

-Ajay
> 
> > @@ -703,7 +700,6 @@ static irqreturn_t musb_stage0_irq(struct musb
> *musb, u8 int_usb,
> >
> >  	if (int_usb & MUSB_INTR_CONNECT) {
> >  		struct usb_hcd *hcd = musb_to_hcd(musb);
> > -		void __iomem *mbase = musb->mregs;
> 
>     We could also move this into the #ifdef CONFIG_USB_MUSB_OTG block.
> 
> >
> >  		handled = IRQ_HANDLED;
> >  		musb->is_active = 1;
> 
> [1]
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-
> 2.6.git;a=commitdiff;h=aa4714560b4ea359bb7830188ebd06bce71bcdea
> 
> WBR, Sergei

Re: [PATCH 3/8] musb: fix compilation warning in host only mode

[Sergei Shtylyov]

Gupta, Ajay Kumar wrote:

>>> Fixes below compilation warning when host only configuration is
>>> selected.

>>> drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq':
>>> drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase'
>>> Also removed definition of 'mbase' from multiple places to only
>>> at function top.

>>     AFAIR, it was intentionally removed from the function top and declared
>> in the multiple plcase instead by the former Felipe's patch [1] to fix
>> exactly the same issue, if I don't mistake. So, it hasn't worked out?

> Yes, it was removed by Felipe's below patch but it introduced
> compilation warning issue as reported.

> ---------------
> commit aa4714560b4ea359bb7830188ebd06bce71bcdea
> usb: musb: core: declare mbase only where it's used

> ... and avoid a compilation if we disable host side
> of musb.
> --------------

>>> Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
>> [...]
>>> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
>>> index f0ff893..7cc8398 100644
>>> --- a/drivers/usb/musb/musb_core.c
>>> +++ b/drivers/usb/musb/musb_core.c
>>> @@ -455,6 +455,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
>>>  				u8 devctl, u8 power)
>>>  {
>>>  	irqreturn_t handled = IRQ_NONE;
>>> +#ifdef CONFIG_USB_MUSB_HDRC_HCD
>>> +	void __iomem	*mbase = musb->mregs;
>>> +#endif

>>     I'd rather see it declared multiple times...

> I was thinking it's better to have it at one place and avoid multiple
> #ifdefs.

    You'd only need one #ifdef: in the placae where the warning was 
reported. And you can open a block under #ifdef CONFIG_USB_MUSB_OTG to 
declare it in. Other declarations are already covered by #ifdef's, 
aren't they?

> -Ajay
>>> @@ -703,7 +700,6 @@ static irqreturn_t musb_stage0_irq(struct musb
>> *musb, u8 int_usb,
>>>  	if (int_usb & MUSB_INTR_CONNECT) {
>>>  		struct usb_hcd *hcd = musb_to_hcd(musb);
>>> -		void __iomem *mbase = musb->mregs;

>>     We could also move this into the #ifdef CONFIG_USB_MUSB_OTG block.

    Yeah, the block where 'mbase' is actually used.

    BTW, your patch didn't cover the declaration in #if 0'ed out SOF 
handler.

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

RE: [PATCH 3/8] musb: fix compilation warning in host only mode

[Gupta, Ajay Kumar]

> >>     AFAIR, it was intentionally removed from the function top and
> declared
> >> in the multiple plcase instead by the former Felipe's patch [1] to fix
> >> exactly the same issue, if I don't mistake. So, it hasn't worked out?
> 
> > Yes, it was removed by Felipe's below patch but it introduced
> > compilation warning issue as reported.
> 
> > ---------------
> > commit aa4714560b4ea359bb7830188ebd06bce71bcdea
> > usb: musb: core: declare mbase only where it's used
> 
> > ... and avoid a compilation if we disable host side
> > of musb.
> > --------------
> 
> >>> Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
> >> [...]
> >>> diff --git a/drivers/usb/musb/musb_core.c
> b/drivers/usb/musb/musb_core.c
> >>> index f0ff893..7cc8398 100644
> >>> --- a/drivers/usb/musb/musb_core.c
> >>> +++ b/drivers/usb/musb/musb_core.c
> >>> @@ -455,6 +455,9 @@ static irqreturn_t musb_stage0_irq(struct musb
> *musb, u8 int_usb,
> >>>  				u8 devctl, u8 power)
> >>>  {
> >>>  	irqreturn_t handled = IRQ_NONE;
> >>> +#ifdef CONFIG_USB_MUSB_HDRC_HCD
> >>> +	void __iomem	*mbase = musb->mregs;
> >>> +#endif
> 
> >>     I'd rather see it declared multiple times...
> 
> > I was thinking it's better to have it at one place and avoid multiple
> > #ifdefs.
> 
>     You'd only need one #ifdef: in the placae where the warning was
> reported. And you can open a block under #ifdef CONFIG_USB_MUSB_OTG to
> declare it in. Other declarations are already covered by #ifdef's,
> aren't they?

Here is the version-2 of this patch.

--------------- cut here -------------------
Fixes below compilation warning when host only configuration is
selected.
drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq':
drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase'

Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
---
 drivers/usb/musb/musb_core.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index a8b0440..cfae447 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -704,7 +704,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 #ifdef CONFIG_USB_MUSB_HDRC_HCD
 	if (int_usb & MUSB_INTR_CONNECT) {
 		struct usb_hcd *hcd = musb_to_hcd(musb);
+#ifdef CONFIG_USB_MUSB_OTG
 		void __iomem *mbase = musb->mregs;
+#endif
 
 		handled = IRQ_HANDLED;
 		musb->is_active = 1;
-- 
1.6.2.4
--------------------------------------------


Thanks,
Ajay
> 
> > -Ajay
> >>> @@ -703,7 +700,6 @@ static irqreturn_t musb_stage0_irq(struct musb
> >> *musb, u8 int_usb,
> >>>  	if (int_usb & MUSB_INTR_CONNECT) {
> >>>  		struct usb_hcd *hcd = musb_to_hcd(musb);
> >>> -		void __iomem *mbase = musb->mregs;
> 
> >>     We could also move this into the #ifdef CONFIG_USB_MUSB_OTG block.
> 
>     Yeah, the block where 'mbase' is actually used.
> 
>     BTW, your patch didn't cover the declaration in #if 0'ed out SOF
> handler.
> 
> 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

Re: [PATCH 3/8] musb: fix compilation warning in host only mode

[Felipe Balbi]

On Thu, Jun 17, 2010 at 12:40:36PM +0200, ext Ajay Kumar Gupta wrote:
>+#ifdef CONFIG_USB_MUSB_HDRC_HCD
>+	void __iomem	*mbase = musb->mregs;
>+#endif

then you add another ifdef to this file, which is already insane. I'd 
rather see you either keep the local variables and just fix what needs 
to be fixed, or use musb->mregs directly.

-- 
balbi

DefectiveByDesign.org
--
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 3/8] musb: fix compilation warning in host only mode

[Gupta, Ajay Kumar]

> >+#ifdef CONFIG_USB_MUSB_HDRC_HCD
> >+	void __iomem	*mbase = musb->mregs;
> >+#endif
> 
> then you add another ifdef to this file, which is already insane. I'd
> rather see you either keep the local variables and just fix what needs
> to be fixed, 

> or use musb->mregs directly.
This one seems to be a much better one. Copying the v-3 with this fix below.

-------------- cut here --------------------
Fixes below compilation warning when host only configuration is
selected.
drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq':
drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase'

Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
---
 drivers/usb/musb/musb_core.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index a8b0440..ed6e1a4 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -704,7 +704,6 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 #ifdef CONFIG_USB_MUSB_HDRC_HCD
 	if (int_usb & MUSB_INTR_CONNECT) {
 		struct usb_hcd *hcd = musb_to_hcd(musb);
-		void __iomem *mbase = musb->mregs;
 
 		handled = IRQ_HANDLED;
 		musb->is_active = 1;
@@ -717,9 +716,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 		if (is_peripheral_active(musb)) {
 			/* REVISIT HNP; just force disconnect */
 		}
-		musb_writew(mbase, MUSB_INTRTXE, musb->epmask);
-		musb_writew(mbase, MUSB_INTRRXE, musb->epmask & 0xfffe);
-		musb_writeb(mbase, MUSB_INTRUSBE, 0xf7);
+		musb_writew(musb->mregs, MUSB_INTRTXE, musb->epmask);
+		musb_writew(musb->mregs, MUSB_INTRRXE, musb->epmask & 0xfffe);
+		musb_writeb(musb->mregs, MUSB_INTRUSBE, 0xf7);
 #endif
 		musb->port1_status &= ~(USB_PORT_STAT_LOW_SPEED
 					|USB_PORT_STAT_HIGH_SPEED
-- 
1.6.2.4
--------------------------------------------

Thanks,
Ajay
> 

Re: [PATCH 3/8] musb: fix compilation warning in host only mode

[Sergei Shtylyov]

Hello.

Gupta, Ajay Kumar wrote:

>>> +#ifdef CONFIG_USB_MUSB_HDRC_HCD
>>> +	void __iomem	*mbase = musb->mregs;
>>> +#endif

>> then you add another ifdef to this file, which is already insane. I'd
>> rather see you either keep the local variables and just fix what needs
>> to be fixed, 

>> or use musb->mregs directly.

> This one seems to be a much better one. Copying the v-3 with this fix below.

    I don't see how it's *much* better...

> -------------- cut here --------------------
> Fixes below compilation warning when host only configuration is
> selected.
> drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq':
> drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase'

> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>

> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index a8b0440..ed6e1a4 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -704,7 +704,6 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
>  #ifdef CONFIG_USB_MUSB_HDRC_HCD
>  	if (int_usb & MUSB_INTR_CONNECT) {
>  		struct usb_hcd *hcd = musb_to_hcd(musb);
> -		void __iomem *mbase = musb->mregs;
>  
>  		handled = IRQ_HANDLED;
>  		musb->is_active = 1;
> @@ -717,9 +716,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
>  		if (is_peripheral_active(musb)) {
>  			/* REVISIT HNP; just force disconnect */
>  		}
> -		musb_writew(mbase, MUSB_INTRTXE, musb->epmask);
> -		musb_writew(mbase, MUSB_INTRRXE, musb->epmask & 0xfffe);
> -		musb_writeb(mbase, MUSB_INTRUSBE, 0xf7);
> +		musb_writew(musb->mregs, MUSB_INTRTXE, musb->epmask);
> +		musb_writew(musb->mregs, MUSB_INTRRXE, musb->epmask & 0xfffe);
> +		musb_writeb(musb->mregs, MUSB_INTRUSBE, 0xf7);
>  #endif

    I propose the following:

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index a8b0440..ed6e1a4 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -704,7 +704,6 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8
  #ifdef CONFIG_USB_MUSB_HDRC_HCD
  	if (int_usb & MUSB_INTR_CONNECT) {
  		struct usb_hcd *hcd = musb_to_hcd(musb);
-		void __iomem *mbase = musb->mregs;

  		handled = IRQ_HANDLED;
  		musb->is_active = 1;
@@ -714,12 +713,16 @@ static irqreturn_t musb_stage0_irq(struct musb

  #ifdef CONFIG_USB_MUSB_OTG
  		/* flush endpoints when transitioning from Device Mode */
- 		if (is_peripheral_active(musb)) {
- 			/* REVISIT HNP; just force disconnect */
- 		}
-		musb_writew(mbase, MUSB_INTRTXE, musb->epmask);
-		musb_writew(mbase, MUSB_INTRRXE, musb->epmask & 0xfffe);
-		musb_writeb(mbase, MUSB_INTRUSBE, 0xf7);
+		{
+			void __iomem *mbase = musb->mregs;
+
+			if (is_peripheral_active(musb)) {
+				/* REVISIT HNP; just force disconnect */
+			}
+			musb_writew(mbase, MUSB_INTRTXE, musb->epmask);
+			musb_writew(mbase, MUSB_INTRRXE, musb->epmask & 0xfffe);
+			musb_writeb(mbase, MUSB_INTRUSBE, 0xf7);
+		}
#endif
  		musb->port1_status &= ~(USB_PORT_STAT_LOW_SPEED
  					|USB_PORT_STAT_HIGH_SPEED

WBR, Sergei

RE: [PATCH 3/8] musb: fix compilation warning in host only mode

[Gupta, Ajay Kumar]

> >> then you add another ifdef to this file, which is already insane. I'd
> >> rather see you either keep the local variables and just fix what needs
> >> to be fixed,
> 
> >> or use musb->mregs directly.
> 
> > This one seems to be a much better one. Copying the v-3 with this fix
> below.
> 
>     I don't see how it's *much* better...

As it doesn't require to create another variable and moreover within
ugly #ifdefs of three different flavors (HOST, DEVCIE and OTG).

I think it's better to kill all 'mbase' variables and use musb->mregs
directly for a better code readability.

-Ajay 
> 

--
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 3/8] musb: fix compilation warning in host only mode

[Felipe Balbi]

On Thu, Jun 24, 2010 at 03:10:01PM +0200, ext Sergei Shtylyov wrote:
>@@ -714,12 +713,16 @@ static irqreturn_t musb_stage0_irq(struct musb
>
>  #ifdef CONFIG_USB_MUSB_OTG
>  		/* flush endpoints when transitioning from Device Mode */
>- 		if (is_peripheral_active(musb)) {
>- 			/* REVISIT HNP; just force disconnect */
>- 		}
>-		musb_writew(mbase, MUSB_INTRTXE, musb->epmask);
>-		musb_writew(mbase, MUSB_INTRRXE, musb->epmask & 0xfffe);
>-		musb_writeb(mbase, MUSB_INTRUSBE, 0xf7);
>+		{

and adding unneded identation level ?? no thanks. Dereferencing musb to 
access mbase sounds better to me.

-- 
balbi

DefectiveByDesign.org

Re: [PATCH 3/8] musb: fix compilation warning in host only mode

[Felipe Balbi]

Hi,

On Thu, Jun 24, 2010 at 01:17:54PM +0200, ext Gupta, Ajay Kumar wrote:
>Fixes below compilation warning when host only configuration is
>selected.
>drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq':
>drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase'
>
>Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>

Acked-by: Felipe Balbi <felipe.balbi@nokia.com>

>---
> drivers/usb/musb/musb_core.c |    7 +++----
> 1 files changed, 3 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
>index a8b0440..ed6e1a4 100644
>--- a/drivers/usb/musb/musb_core.c
>+++ b/drivers/usb/musb/musb_core.c
>@@ -704,7 +704,6 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
> #ifdef CONFIG_USB_MUSB_HDRC_HCD
> 	if (int_usb & MUSB_INTR_CONNECT) {
> 		struct usb_hcd *hcd = musb_to_hcd(musb);
>-		void __iomem *mbase = musb->mregs;
>
> 		handled = IRQ_HANDLED;
> 		musb->is_active = 1;
>@@ -717,9 +716,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
> 		if (is_peripheral_active(musb)) {
> 			/* REVISIT HNP; just force disconnect */
> 		}
>-		musb_writew(mbase, MUSB_INTRTXE, musb->epmask);
>-		musb_writew(mbase, MUSB_INTRRXE, musb->epmask & 0xfffe);
>-		musb_writeb(mbase, MUSB_INTRUSBE, 0xf7);
>+		musb_writew(musb->mregs, MUSB_INTRTXE, musb->epmask);
>+		musb_writew(musb->mregs, MUSB_INTRRXE, musb->epmask & 0xfffe);
>+		musb_writeb(musb->mregs, MUSB_INTRUSBE, 0xf7);
> #endif
> 		musb->port1_status &= ~(USB_PORT_STAT_LOW_SPEED
> 					|USB_PORT_STAT_HIGH_SPEED

-- 
balbi

DefectiveByDesign.org

Re: [PATCH 3/8] musb: fix compilation warning in host only mode

[Greg KH]

On Tue, Jul 06, 2010 at 12:13:14PM +0300, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Jun 24, 2010 at 01:17:54PM +0200, ext Gupta, Ajay Kumar wrote:
> >Fixes below compilation warning when host only configuration is
> >selected.
> >drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq':
> >drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase'
> >
> >Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> 
> Acked-by: Felipe Balbi <felipe.balbi@nokia.com>

You've acked a random ammount of patches recently, are you going to be
resending them to me?  I have a hard time going back and digging through
the archives to pick out the ones that you acked vs. the ones you
didn't.

thanks,

greg k-h

RE: [PATCH 3/8] musb: fix compilation warning in host only mode

[Gupta, Ajay Kumar]

Hi,
> > On Thu, Jun 24, 2010 at 01:17:54PM +0200, ext Gupta, Ajay Kumar wrote:
> > >Fixes below compilation warning when host only configuration is
> > >selected.
> > >drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq':
> > >drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase'
> > >
> > >Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> >
> > Acked-by: Felipe Balbi <felipe.balbi@nokia.com>
> 
> You've acked a random ammount of patches recently, are you going to be
> resending them to me?  I have a hard time going back and digging through
> the archives to pick out the ones that you acked vs. the ones you
> didn't.

Greg,

There are 2 musb and 1 ehci-omap bug fix patches which are acked.
Another related one is on ULPI and is trivial.

Apart from these, 2 musb patches for v2.6.36 window are also acked.

I will prepare the set and send you after sanity testing.

Thanks,
Ajay 
> 
> thanks,
> 
> greg k

Re: [PATCH 3/8] musb: fix compilation warning in host only mode

[Greg KH]

On Thu, Jul 08, 2010 at 07:23:44AM +0530, Gupta, Ajay Kumar wrote:
> Hi,
> > > On Thu, Jun 24, 2010 at 01:17:54PM +0200, ext Gupta, Ajay Kumar wrote:
> > > >Fixes below compilation warning when host only configuration is
> > > >selected.
> > > >drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq':
> > > >drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase'
> > > >
> > > >Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
> > >
> > > Acked-by: Felipe Balbi <felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> > 
> > You've acked a random ammount of patches recently, are you going to be
> > resending them to me?  I have a hard time going back and digging through
> > the archives to pick out the ones that you acked vs. the ones you
> > didn't.
> 
> Greg,
> 
> There are 2 musb and 1 ehci-omap bug fix patches which are acked.
> Another related one is on ULPI and is trivial.
> 
> Apart from these, 2 musb patches for v2.6.36 window are also acked.
> 
> I will prepare the set and send you after sanity testing.

That sounds great, thanks for doing this.

greg k-h
--
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 2/8] MUSB: make non-OMAP platforms build with CONFIG_PM=y

[Felipe Balbi]

On Thu, Jun 17, 2010 at 12:40:35PM +0200, ext Ajay Kumar Gupta wrote:
>From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>
>Attempt to build MUSB driver with CONFIG_PM=y (e.g. in the OTG mode) on DaVinci
>results in these link errors:
>
>drivers/built-in.o: In function `musb_restore_context':
>led-triggers.c:(.text+0x714d8): undefined reference to
>`musb_platform_restore_context'
>drivers/built-in.o: In function `musb_save_context':
>led-triggers.c:(.text+0x71788): undefined reference to
>`musb_platform_save_context'
>
>This turned out to be caused by commit 9957dd97ec5e98dd334f87ade1d9a0b24d1f86eb
>(usb: musb: Fix compile error for omaps for musb_hdrc). Revert it, taking into
>account the rename of CONFIG_ARCH_OMAP34XX into CONFIG_ARCH_OMAP3 (which that
>commit fixed in a completely inappropriate way) and the recent addition of
>OMAP4 support.
>
>Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>

Acked-by: Felipe Balbi <felipe.balbi@nokia.com>
-- 
balbi

DefectiveByDesign.org

Re: [PATCH 1/8] musb_core: make disconnect and suspend interrupts work again

[Felipe Balbi]

On Thu, Jun 17, 2010 at 12:40:34PM +0200, ext Ajay Kumar Gupta wrote:
>From: Sergei Shtylyov <sshtylyov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
>
>Commit 1c25fda4a09e8229800979986ef399401053b46e (usb: musb: handle irqs in the
>order dictated by programming guide) forgot to get rid of the old 'STAGE0_MASK'
>filter for calling musb_stage0_irq(), so now disconnect and suspend interrupts
>are effectively ignored...
>
>Signed-off-by: Sergei Shtylyov <sshtylyov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
>Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
>Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>

Acked-by: Felipe Balbi <felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>

-- 
balbi

DefectiveByDesign.org
--
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 1/8] musb_core: make disconnect and suspend interrupts work again

[Greg KH]

On Thu, Jun 24, 2010 at 09:01:51AM +0300, Felipe Balbi wrote:
> On Thu, Jun 17, 2010 at 12:40:34PM +0200, ext Ajay Kumar Gupta wrote:
> >From: Sergei Shtylyov <sshtylyov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
> >
> >Commit 1c25fda4a09e8229800979986ef399401053b46e (usb: musb: handle irqs in the
> >order dictated by programming guide) forgot to get rid of the old 'STAGE0_MASK'
> >filter for calling musb_stage0_irq(), so now disconnect and suspend interrupts
> >are effectively ignored...
> >
> >Signed-off-by: Sergei Shtylyov <sshtylyov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
> >Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
> >Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
> 
> Acked-by: Felipe Balbi <felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>

I don't have the original versions of any of these anymore.  Can you
resend them to me with your Ack?  Or Ajay, can you?

thanks,

greg k-h
--
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-2.6.35-rc3+ 0/8] musb patches

[Greg KH]

On Thu, Jun 17, 2010 at 04:10:33PM +0530, Ajay Kumar Gupta wrote:
> Greg,
> 
> These are reviewed and non-controversial musb bugfix patches. Sanity have
> been tested on OMAP3EVM for Host-MSC, HID, audio, video class and
> device-CDC class.

Sorry, I need Felipe to send these to me, or at the very least, give me
an ack for them, before I will apply them.

thanks,

greg k-h
--
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-2.6.35-rc3+ 0/8] musb patches

[Gupta, Ajay Kumar]

> > These are reviewed and non-controversial musb bugfix patches. Sanity
> have
> > been tested on OMAP3EVM for Host-MSC, HID, audio, video class and
> > device-CDC class.
> 
> Sorry, I need Felipe to send these to me, or at the very least, give me
> an ack for them, before I will apply them.

Felipe, Can you please forward these patches to Greg?

-Ajay
> 
> thanks,
> 
> greg k-h