ARM: OMAP: Fix usb gadget related compile errors and typo

PATCH In review Dirk Behme 4 participants 12 messages Oct 29, 2007 – Nov 15, 2007

Subsystems (2)

USB OMAP USB support

lore

Share & tools

lore

Dirk Behme

author

2007-10-29 19:55 UTC #0 lore raw

PATCH 1/1 ARM: OMAP: Fix usb gadget related compile errors and typo

ARM: OMAP: Fix usb gadget related compile errors and typo

Signed-off-by Dirk Behme <[email protected]>

drivers/i2c/chips/isp1301_omap.c +1 -1

Index: linux-osk/drivers/i2c/chips/isp1301_omap.c
===================================================================
@@ -28,7 +28,7 @@
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/usb/ch9.h>
-#include <linux/usb_gadget.h>
+#include <linux/usb/gadget.h>
 #include <linux/usb.h>
 #include <linux/usb/otg.h>
 #include <linux/i2c.h>

drivers/usb/gadget/omap_udc.c +3 -3

Index: linux-osk/drivers/usb/gadget/omap_udc.c
===================================================================
@@ -1241,14 +1241,14 @@ static void pullup_enable(struct omap_ud
 	udc->gadget.dev.parent->power.power_state = PMSG_ON;
 	udc->gadget.dev.power.power_state = PMSG_ON;
 	UDC_SYSCON1_REG |= UDC_PULLUP_EN;
-	if (!gadget_is_otg(udc->gadget) && !cpu_is_omap15xx())
+	if (!gadget_is_otg(&(udc->gadget)) && !cpu_is_omap15xx())
 		OTG_CTRL_REG |= OTG_BSESSVLD;
 	UDC_IRQ_EN_REG = UDC_DS_CHG_IE;
 }
 
 static void pullup_disable(struct omap_udc *udc)
 {
-	if (!gadget_is_otg(udc->gadget) && !cpu_is_omap15xx())
+        if (!gadget_is_otg(&(udc->gadget)) && !cpu_is_omap15xx())
 		OTG_CTRL_REG &= ~OTG_BSESSVLD;
 	UDC_IRQ_EN_REG = UDC_DS_CHG_IE;
 	UDC_SYSCON1_REG &= ~UDC_PULLUP_EN;

@@ -1386,7 +1386,7 @@ static void update_otg(struct omap_udc *
 {
 	u16	devstat;
 
-	if (!gadget_is_otg(udc->gadget))
+	if (!gadget_is_otg(&(udc->gadget)))
 		return;
 
 	if (OTG_CTRL_REG & OTG_ID)

include/linux/usb/gadget.h +2 -2

Index: linux-osk/include/linux/usb/gadget.h
===================================================================
@@ -480,7 +480,7 @@ static inline void *get_gadget_data (str
 
 
 /**
- * gadget_is_dualspeed - return true iff the hardware handles high speed
+ * gadget_is_dualspeed - return true if the hardware handles high speed
  * @g: controller that might support both high and full speeds
  */
 static inline int gadget_is_dualspeed(struct usb_gadget *g)

@@ -496,7 +496,7 @@ static inline int gadget_is_dualspeed(st
 }
 
 /**
- * gadget_is_otg - return true iff the hardware is OTG-ready
+ * gadget_is_otg - return true if the hardware is OTG-ready
  * @g: controller that might have a Mini-AB connector
  *
  * This is a runtime test, since kernels with a USB-OTG stack sometimes

David Brownell

2007-10-29 20:11 UTC #1 lore raw

Quoted 2 lines

- * gadget_is_dualspeed - return true iff the hardware handles high speed
+ * gadget_is_dualspeed - return true if the hardware handles high speed

Not a typo; "iff" == "if and only if". NAK to that part. Re the other, please make those parts match what's upstream; I think you added extra parens.

Until the version of this code works on both H2 and H3, the two drivers will stay out of sync. Right now the upstream version works on H2 (unless someone broke it somehow) and the one in the OMAP tree works on H3. Sigh. Dunno about other incarnations of the full speed OTG engines.

- Dave

Felipe Balbi

2007-10-29 20:44 UTC #2 lore raw

Hi Dave,

Quoted 7 lines · David Brownell

Quoted 2 lines
- * gadget_is_dualspeed - return true iff the hardware handles high speed
+ * gadget_is_dualspeed - return true if the hardware handles high speed
Not a typo; "iff" == "if and only if". NAK to that part. Re the other, please make those parts match what's upstream; I think you added extra parens.

Quoted 5 lines

Until the version of this code works on both H2 and H3, the two drivers will stay out of sync. Right now the upstream version works on H2 (unless someone broke it somehow) and the one in the OMAP tree works on H3. Sigh. Dunno about other incarnations of the full speed OTG engines.

is there any plans on upgrading isp1301 driver to the new style i2c driver also?? how could i help you with that?

tps65010 driver is really better with that i2c_board_info structure in board files and there's a note to make isp1301 driver work the same way.

Quoted 8 lines

- Dave
_______________________________________________
Linux-omap-open-source mailing list
[email protected]
http://linux.omap.com/mailman/listinfo/linux-omap-open-source

Signature · 4 lines

Best Regards,

Felipe Balbi
[email protected]

David Brownell

2007-10-29 21:33 UTC #3 lore raw

Quoted 2 lines

is there any plans on upgrading isp1301 driver to the new style i2c driver also?? how could i help you with that?

ISTR sending some code like that to the I2C list a while back, but it's not yet been merged.

You could finish that, if it's not already sitting in Jean's I2C queue. And if it is, you could maybe get it into the OMAP tree before 2.6.25 merges begin. :)

Quoted 3 lines

tps65010 driver is really better with that i2c_board_info structure in board files and there's a note to make isp1301 driver work the same way.

Yep. In fact, all the I2C code that OMAP uses should switch over to new-style drivers ...

That includes the pcf875x i/o expanders found on many TI devel boards. Making that change involves some updates to the GPIO framework. I've got some code to address all that, which I'll be posting in a few days.

- Dave

Felipe Balbi

2007-10-29 21:45 UTC #4 lore raw

Quoted 5 lines · David Brownell

Quoted 2 lines
is there any plans on upgrading isp1301 driver to the new style i2c driver also?? how could i help you with that?
ISTR sending some code like that to the I2C list a while back, but it's not yet been merged.

I see...

Quoted 4 lines

You could finish that, if it's not already sitting in Jean's I2C queue. And if it is, you could maybe get it into the OMAP tree before 2.6.25 merges begin. :)

do Jean have a gittree in kernel.org, couldn't find search the name... it would be easier just git-pull from there.. but if there isn't I'll do it manually and test with omap h2 and omap h3 boards once I'm back in brazil (next week).

well, in any case, I'll try to get these in sync between i2c/mainline and linux-omap tree.

Quoted 6 lines

Quoted 5 lines
tps65010 driver is really better with that i2c_board_info structure in board files and there's a note to make isp1301 driver work the same way.

Quoted 7 lines

Yep. In fact, all the I2C code that OMAP uses should switch over to new-style drivers ...
That includes the pcf875x i/o expanders found on many TI devel boards. Making that change involves some updates to the GPIO framework. I've got some code to address all that, which I'll be posting in a few days.

ok.. that'll help, could you please cc me so I can get those as soon as you publish them to apply on linux and test/validate with omap boards ?

tks in advance

Quoted 4 lines

- Dave

Signature · 4 lines

Best Regards,

Felipe Balbi
[email protected]

David Brownell

2007-10-29 22:42 UTC #5 lore raw

Quoted 7 lines · Felipe Balbi

Quoted 5 lines · David Brownell
Quoted 2 lines
is there any plans on upgrading isp1301 driver to the new style i2c driver also?? how could i help you with that?
ISTR sending some code like that to the I2C list a while back, but it's not yet been merged.

Quoted 8 lines

I see...
Quoted 4 lines
You could finish that, if it's not already sitting in Jean's I2C queue. And if it is, you could maybe get it into the OMAP tree before 2.6.25 merges begin. :)
do Jean have a gittree in kernel.org, couldn't find search the name...

He's got a quilt patch tree. I have it bookmarked as:

http://khali.linux-fr.org/devel/linux-2.6/jdelvare-i2c/?M=A

Seems not to have the isp1301 driver stuff ... that should be in the i2c list archives.

Quoted 3 lines

it would be easier just git-pull from there.. but if there isn't I'll do it manually and test with omap h2 and omap h3 boards once I'm back in brazil (next week).

... And it'd be nice if someone could update the H3 support to include the IRQ used to hook up the tps65013 chip. ;)

Quoted 2 lines

well, in any case, I'll try to get these in sync between i2c/mainline and linux-omap tree.

Not straightforward, as I recall. Or it'd be working already. The issue is that the OTG state machine changed, so that various transitions (I forget which ones!) behave incompatibly.

Quoted 8 lines

Quoted 8 lines
Quoted 4 lines
tps65010 driver is really better with that i2c_board_info structure in board files and there's a note to make isp1301 driver work the same way.
Yep. In fact, all the I2C code that OMAP uses should switch over to new-style drivers ...

Quoted 8 lines

Quoted 4 lines
That includes the pcf875x i/o expanders found on many TI devel boards. Making that change involves some updates to the GPIO framework. I've got some code to address all that, which I'll be posting in a few days.
ok.. that'll help, could you please cc me so I can get those as soon as you publish them to apply on linux and test/validate with omap boards ?

What I'll do is forward the OMAP patches to this list ... both the generic stuff (affecting the plat-omap/gpio.c code), and board-specific stuff (affecting debug boards, tps6501x, etc). The board-specific stuff will need more work, it's not all that current.

- Dave

Felipe Balbi

2007-10-29 22:52 UTC #6 lore raw

On 10/30/07, David Brownell <[email protected]> wrote:
<snip>

Quoted 5 lines

Quoted 1 line
do Jean have a gittree in kernel.org, couldn't find search the name...
He's got a quilt patch tree. I have it bookmarked as:
http://khali.linux-fr.org/devel/linux-2.6/jdelvare-i2c/?M=A

tks

Quoted 3 lines

Seems not to have the isp1301 driver stuff ... that should be in the i2c list archives.

yeah... i'll track down

Quoted 8 lines

Quoted 5 lines
it would be easier just git-pull from there.. but if there isn't I'll do it manually and test with omap h2 and omap h3 boards once I'm back in brazil (next week).
... And it'd be nice if someone could update the H3 support to include the IRQ used to hook up the tps65013 chip. ;)

hmm... k... I'll check that tomorrow morning.

Quoted 8 lines

Quoted 4 lines
well, in any case, I'll try to get these in sync between i2c/mainline and linux-omap tree.
Not straightforward, as I recall. Or it'd be working already. The issue is that the OTG state machine changed, so that various transitions (I forget which ones!) behave incompatibly.

yeah... maybe running opt tests with omap h[23] boards running gadget zero will help tracking down which issues are we talking about and hopefully will help fixing it.

<snip>

Quoted 8 lines

Quoted 3 lines
ok.. that'll help, could you please cc me so I can get those as soon as you publish them to apply on linux and test/validate with omap boards ?
What I'll do is forward the OMAP patches to this list ... both the generic stuff (affecting the plat-omap/gpio.c code), and board-specific stuff (affecting debug boards, tps6501x, etc). The board-specific stuff will need more work, it's not all that current.

that's even better so everybody can comment/work on them

Signature · 4 lines

Best Regards,

Felipe Balbi
[email protected]

Dirk Behme

author

2007-10-30 16:37 UTC #7 lore raw

In reply to David Brownell

Quoted 4 lines · David Brownell

Quoted 2 lines
- * gadget_is_dualspeed - return true iff the hardware handles high speed
+ * gadget_is_dualspeed - return true if the hardware handles high speed
Not a typo; "iff" == "if and only if". NAK to that part.

Sorry. Sometimes its hard to be a non-native speaker ;)

Quoted 1 line

Re the other, please make those parts match what's upstream;

Creating the patch ~3 hours after Tonys "update linux-omap to mainline tree" I assumed OMAP git was in sync with mainline.

Quoted 1 line

I think you added extra parens.

Yes. I tried to fix this compilation error:

CC drivers/usb/gadget/omap_udc.o
drivers/usb/gadget/omap_udc.c: In function 'pullup_enable':
drivers/usb/gadget/omap_udc.c:1244: error: incompatible type for argument 1 of 'gadget_is_otg' drivers/usb/gadget/omap_udc.c: In function 'pullup_disable':
drivers/usb/gadget/omap_udc.c:1251: error: incompatible type for argument 1 of 'gadget_is_otg' drivers/usb/gadget/omap_udc.c: In function 'update_otg':
drivers/usb/gadget/omap_udc.c:1389: error: incompatible type for argument 1 of 'gadget_is_otg'

gadget_is_otg() is defined as

static inline int gadget_is_otg(struct usb_gadget *g) { ...

and the call is

gadget_is_otg(udc->gadget)

Which fix do you propose for this?

Best regards

Dirk

David Brownell

2007-10-30 17:00 UTC #8 lore raw

Quoted 9 lines · Dirk Behme

gadget_is_otg() is defined as
static inline int gadget_is_otg(struct usb_gadget *g) { ...
and the call is
gadget_is_otg(udc->gadget)
Which fix do you propose for this?

http://marc.info/?l=linux-usb-devel&m=119343277601632&w=2

which is in the USB patch queue ... I thought I saw it get sent to Linus recently, but maybe not. It's not in the kernel.org tree yet, unfortunately.

- Dave

Tony Lindgren

2007-11-01 09:00 UTC #9 lore raw

Quoted 9 lines · David Brownell

Quoted 8 lines · Dirk Behme
gadget_is_otg() is defined as
static inline int gadget_is_otg(struct usb_gadget *g) { ...
and the call is
gadget_is_otg(udc->gadget)

Quoted 7 lines

Quoted 1 line
Which fix do you propose for this?
http://marc.info/?l=linux-usb-devel&m=119343277601632&w=2
which is in the USB patch queue ... I thought I saw it get sent to Linus recently, but maybe not. It's not in the kernel.org tree yet, unfortunately.

Pushing Dave's fix today.

Tony

Dirk Behme

author

2007-11-01 16:28 UTC #10 lore raw

Quoted 14 lines · Tony Lindgren

Quoted 12 lines · David Brownell
Quoted 9 lines · Dirk Behme
gadget_is_otg() is defined as
static inline int gadget_is_otg(struct usb_gadget *g) { ...
and the call is
gadget_is_otg(udc->gadget)
Which fix do you propose for this?

Quoted 7 lines

Quoted 5 lines
http://marc.info/?l=linux-usb-devel&m=119343277601632&w=2
which is in the USB patch queue ... I thought I saw it get sent to Linus recently, but maybe not. It's not in the kernel.org tree yet, unfortunately.
Pushing Dave's fix today.

And what's about the

-- cut --

drivers/i2c/chips/isp1301_omap.c +0 -0

@@ -28,7 +28,7 @@

...
-#include <linux/usb_gadget.h>
+#include <linux/usb/gadget.h>
...
-- cut --

part of

http://linux.omap.com/pipermail/linux-omap-open-source/2007-October/011804.html

Dirk

Tony Lindgren

2007-11-15 20:40 UTC #11 lore raw

Hi,

Quoted 33 lines · Dirk Behme

Quoted 11 lines · Tony Lindgren
Quoted 10 lines · David Brownell
Quoted 8 lines · Dirk Behme
gadget_is_otg() is defined as
static inline int gadget_is_otg(struct usb_gadget *g) { ...
and the call is
gadget_is_otg(udc->gadget)
Quoted 8 lines
Quoted 7 lines
Quoted 1 line
Which fix do you propose for this?
http://marc.info/?l=linux-usb-devel&m=119343277601632&w=2
which is in the USB patch queue ... I thought I saw it get sent to Linus recently, but maybe not. It's not in the kernel.org tree yet, unfortunately.
Pushing Dave's fix today.
And what's about the
-- cut --
drivers/i2c/chips/isp1301_omap.c +1 -2
@@ -28,7 +28,7 @@
...
-#include <linux/usb_gadget.h>
+#include <linux/usb/gadget.h>
...
-- cut --

Quoted 5 lines

part of
http://linux.omap.com/pipermail/linux-omap-open-source/2007-October/011804.html
?

Oops, sorry I guess I've been a bit behind with mails. I'll push a fix for that today.

Tony