Hello.
On 11/24/2011 04:46 PM, Felipe Balbi wrote:
we are compiling the driver always with full OTG
capabilities, so that board_mode trick becomes
useless.
Signed-off-by: Felipe Balbi<balbi@xxxxxx>
---
I would like to get Acks from blackfin, da8xx and davinci guys, please.
After having looked thru the patch, I have a few questions...
diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
index e233d2b..34c91ac 100644
--- a/drivers/usb/musb/am35x.c
+++ b/drivers/usb/musb/am35x.c
[...]
@@ -269,8 +262,7 @@ static irqreturn_t am35x_musb_interrupt(int irq, void *hci)
u8 devctl = musb_readb(mregs, MUSB_DEVCTL);
int err;
- err = is_host_enabled(musb) && (musb->int_usb&
- MUSB_INTR_VBUSERROR);
+ err = (musb->int_usb & MUSB_INTR_VBUSERROR);
Could drop now useless parens...
@@ -324,8 +316,7 @@ eoi:
}
/* Poll for ID change */
- if (is_otg_enabled(musb) && musb->xceiv->state == OTG_STATE_B_IDLE)
- mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+ mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
Er, not sure why you dropped the whole *if* here, I think it should've been:
if (musb->xceiv->state == OTG_STATE_B_IDLE)
diff --git a/drivers/usb/musb/blackfin.c b/drivers/usb/musb/blackfin.c
index 5e7cfba..1817dc8 100644
--- a/drivers/usb/musb/blackfin.c
+++ b/drivers/usb/musb/blackfin.c
[...]
@@ -324,8 +316,7 @@ static int bfin_musb_set_power(struct otg_transceiver *x, unsigned mA)
static void bfin_musb_try_idle(struct musb *musb, unsigned long timeout)
{
- if (!is_otg_enabled(musb) && is_host_enabled(musb))
- mod_timer(&musb_conn_timer, jiffies + TIMER_DELAY);
+ /* REVISIT is this really correct ? */
Looks like the whole function needs to be dropped now...
diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 2613bfd..30614e7 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
[...]
@@ -331,8 +324,7 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
u8 devctl = musb_readb(mregs, MUSB_DEVCTL);
int err;
- err = is_host_enabled(musb) && (musb->int_usb&
- MUSB_INTR_VBUSERROR);
+ err = (musb->int_usb & USB_INTR_VBUSERROR);
Could drop now useless parens...
if (err) {
/*
* The Mentor core doesn't debounce VBUS as needed
diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
index f9a3f62..e160c5e 100644
--- a/drivers/usb/musb/davinci.c
+++ b/drivers/usb/musb/davinci.c
[...]
@@ -315,8 +312,7 @@ static irqreturn_t davinci_musb_interrupt(int irq, void *__hci)
u8 devctl = musb_readb(mregs, MUSB_DEVCTL);
int err = musb->int_usb & MUSB_INTR_VBUSERROR;
- err = is_host_enabled(musb)
- && (musb->int_usb & MUSB_INTR_VBUSERROR);
+ err = (musb->int_usb & MUSB_INTR_VBUSERROR);
Could drop now useless parens...
@@ -419,12 +413,8 @@ static int davinci_musb_init(struct musb *musb)
if (cpu_is_davinci_dm355()) {
u32 deepsleep = __raw_readl(DM355_DEEPSLEEP);
- if (is_host_enabled(musb)) {
- deepsleep&= ~DRVVBUS_OVERRIDE;
- } else {
- deepsleep&= ~DRVVBUS_FORCE;
- deepsleep |= DRVVBUS_OVERRIDE;
- }
+ deepsleep&= ~DRVVBUS_FORCE;
+ deepsleep |= DRVVBUS_OVERRIDE;
Not clear why -- is_host_enabled(musb) is true in OTG mode...
[...]
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 5793095..f925fea 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
[...]
@@ -1957,59 +1943,11 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
musb->irq_wake = 0;
}
- /* host side needs more setup */
- if (is_host_enabled(musb)) {
- struct usb_hcd *hcd = musb_to_hcd(musb);
-
- otg_set_host(musb->xceiv,&hcd->self);
+ MUSB_DEV_MODE(musb);
+ musb->xceiv->default_a = 0;
+ musb->xceiv->state = OTG_STATE_B_IDLE;
- if (is_otg_enabled(musb))
- hcd->self.otg_port = 1;
- musb->xceiv->host =&hcd->self;
- hcd->power_budget = 2 * (plat->power ? : 250);
-
- /* program PHY to use external vBus if required */
- if (plat->extvbus) {
- u8 busctl = musb_read_ulpi_buscontrol(musb->mregs);
- busctl |= MUSB_ULPI_USE_EXTVBUS;
- musb_write_ulpi_buscontrol(musb->mregs, busctl);
- }
- }
Not clear why are dropping the above block -- is_host_enabled() is true in
OTG mode, so I think you should have kept it, just moved out of *if*.
[...]
- } else /* peripheral is enabled */ {
- MUSB_DEV_MODE(musb);
- musb->xceiv->default_a = 0;
- musb->xceiv->state = OTG_STATE_B_IDLE;
-
- status = musb_gadget_setup(musb);
-
- dev_dbg(musb->controller, "%s mode, status %d, dev%02x\n",
- is_otg_enabled(musb) ? "OTG" : "PERIPHERAL",
- status,
- musb_readb(musb->mregs, MUSB_DEVCTL));
You think this dev_dbg() became worthless?
-
- }
+ status = musb_gadget_setup(musb);
if (status< 0)
goto fail3;
@@ -2025,28 +1963,13 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
goto fail5;
#endif
- dev_info(dev, "USB %s mode controller at %p using %s, IRQ %d\n",
- ({char *s;
- switch (musb->board_mode) {
- case MUSB_HOST: s = "Host"; break;
- case MUSB_PERIPHERAL: s = "Peripheral"; break;
- default: s = "OTG"; break;
- }; s; }),
- ctrl,
- (is_dma_capable()&& musb->dma_controller)
- ? "DMA" : "PIO",
- musb->nIrq);
Er, this dev_info() is useful even without musb->board_mode...
[...]
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 3d28fb8..af94d10 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -71,10 +71,6 @@ struct musb_ep;
#include<linux/usb/hcd.h>
#include "musb_host.h"
-#define is_peripheral_enabled(musb) ((musb)->board_mode != MUSB_HOST)
-#define is_host_enabled(musb) ((musb)->board_mode != MUSB_PERIPHERAL)
-#define is_otg_enabled(musb) ((musb)->board_mode == MUSB_OTG)
-
/* NOTE: otg and peripheral-only state machines start at B_IDLE.
* OTG or host-only go to A_IDLE when ID is sensed.
*/
You didn't drop 'board_mode' itself from 'struct musb'...
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 87d78c0..22220fe 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
[...]
@@ -1898,11 +1897,14 @@ static int musb_gadget_start(struct usb_gadget *g,
struct usb_gadget_driver *driver)
{
struct musb *musb = gadget_to_musb(g);
+ struct usb_hcd *hcd = musb_to_hcd(musb);
unsigned long flags;
- int retval = -EINVAL;
+ int retval = 0;
[...]
@@ -1916,49 +1918,28 @@ static int musb_gadget_start(struct usb_gadget *g,
otg_set_peripheral(musb->xceiv,&musb->g);
musb->xceiv->state = OTG_STATE_B_IDLE;
+ spin_unlock_irqrestore(&musb->lock, flags);
/*
- * FIXME this ignores the softconnect flag. Drivers are
- * allowed hold the peripheral inactive until for example
- * userspace hooks up printer hardware or DSP codecs, so
- * hosts only see fully functional devices.
+ * REVISIT: funcall to other code, which also
+ * handles power budgeting ... this way also
+ * ensures HdrcStart is indirectly called.
*/
+ retval = usb_add_hcd(musb_to_hcd(musb), -1, 0);
Er, you declared/init'ed 'hcd' abobe, why didn't you use it here?
diff --git a/drivers/usb/musb/musb_virthub.c b/drivers/usb/musb/musb_virthub.c
index e9f80ad..56ca2e4 100644
--- a/drivers/usb/musb/musb_virthub.c
+++ b/drivers/usb/musb/musb_virthub.c
[...]
@@ -270,8 +267,6 @@ int musb_hub_control(
musb_port_suspend(musb, false);
break;
case USB_PORT_FEAT_POWER:
- if (!(is_otg_enabled(musb) && hcd->self.is_b_host))
Er, not sure this *if* should be completely dropped, shouldn't it be:
if (!hcd->self.is_b_host)
- musb_platform_set_vbus(musb, 0);
break;
case USB_PORT_FEAT_C_CONNECTION:
case USB_PORT_FEAT_C_ENABLE:
@@ -356,18 +351,6 @@ int musb_hub_control(
switch (wValue) {
case USB_PORT_FEAT_POWER:
- /* NOTE: this controller has a strange state machine
- * that involves "requesting sessions" according to
- * magic side effects from incompletely-described
- * rules about startup...
- *
- * This call is what really starts the host mode; be
- * very careful about side effects if you reorder any
- * initialization logic, e.g. for OTG, or change any
- * logic relating to VBUS power-up.
- */
- if (!(is_otg_enabled(musb) && hcd->self.is_b_host))
Same comment as above.
- musb_start(musb);
break;
case USB_PORT_FEAT_RESET:
musb_port_reset(musb, true);
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
