[PATCH v3 06/11] usb: musb: factor out hcd initalization
The musb struct is currently allocated along with the hcd, which makes
it difficult to build a driver that only acts as gadget device.
Fix this by allocating musb directly, and keep the hcd around as
a pointer in the musb struct.
struct hc_driver musb_hc_driver can now also be static to musb_host.c,
and the macro musb_to_hcd() is just a pointer dereferencer for now, and
will be eliminated later.
Signed-off-by: Daniel Mack
Acked-by: Peter Korsgaard
---
drivers/usb/musb/musb_core.c | 40
drivers/usb/musb/musb_core.h | 1 +
drivers/usb/musb/musb_host.c | 54 ++--
drivers/usb/musb/musb_host.h | 19 +++-
4 files changed, 75 insertions(+), 39 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index adf069d..f3519d3 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -404,7 +404,8 @@ void musb_hnp_stop(struct musb *musb)
break;
case OTG_STATE_B_HOST:
dev_dbg(musb->controller, "HNP: Disabling HR\n");
- hcd->self.is_b_host = 0;
+ if (hcd)
+ hcd->self.is_b_host = 0;
musb->xceiv->state = OTG_STATE_B_PERIPHERAL;
MUSB_DEV_MODE(musb);
reg = musb_readb(mbase, MUSB_POWER);
@@ -726,7 +727,8 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8
int_usb,
dev_dbg(musb->controller, "HNP: CONNECT, now b_host\n");
b_host:
musb->xceiv->state = OTG_STATE_B_HOST;
- hcd->self.is_b_host = 1;
+ if (hcd)
+ hcd->self.is_b_host = 1;
musb->ignore_disconnect = 0;
del_timer(&musb->otg_timer);
break;
@@ -768,7 +770,8 @@ b_host:
* in hnp_stop() is currently not used...
*/
musb_root_disconnect(musb);
- musb_to_hcd(musb)->self.is_b_host = 0;
+ if (musb->hcd)
+ musb->hcd->self.is_b_host = 0;
musb->xceiv->state = OTG_STATE_B_PERIPHERAL;
MUSB_DEV_MODE(musb);
musb_g_disconnect(musb);
@@ -1714,24 +1717,18 @@ static struct musb *allocate_instance(struct device
*dev,
struct musb *musb;
struct musb_hw_ep *ep;
int epnum;
- struct usb_hcd *hcd;
+ int ret;
- hcd = usb_create_hcd(&musb_hc_driver, dev, dev_name(dev));
- if (!hcd)
+ musb = devm_kzalloc(dev, sizeof(*musb), GFP_KERNEL);
+ if (!musb)
return NULL;
- /* usbcore sets dev->driver_data to hcd, and sometimes uses that... */
- musb = hcd_to_musb(hcd);
INIT_LIST_HEAD(&musb->control);
INIT_LIST_HEAD(&musb->in_bulk);
INIT_LIST_HEAD(&musb->out_bulk);
- hcd->uses_new_polling = 1;
- hcd->has_tt = 1;
-
musb->vbuserr_retry = VBUSERR_RETRY_COUNT;
musb->a_wait_bcon = OTG_TIME_A_WAIT_BCON;
- dev_set_drvdata(dev, musb);
musb->mregs = mbase;
musb->ctrl_base = mbase;
musb->nIrq = -ENODEV;
@@ -1746,7 +1743,16 @@ static struct musb *allocate_instance(struct device *dev,
musb->controller = dev;
+ ret = musb_host_alloc(musb);
+ if (ret < 0)
+ goto err_free;
+
+ dev_set_drvdata(dev, musb);
+
return musb;
+
+err_free:
+ return NULL;
}
static void musb_free(struct musb *musb)
@@ -1772,7 +1778,7 @@ static void musb_free(struct musb *musb)
dma_controller_destroy(c);
}
- usb_put_hcd(musb_to_hcd(musb));
+ musb_host_free(musb);
}
/*
@@ -1789,7 +1795,6 @@ musb_init_controller(struct device *dev, int nIrq, void
__iomem *ctrl)
int status;
struct musb *musb;
struct musb_hdrc_platform_data *plat = dev->platform_data;
- struct usb_hcd *hcd;
/* The driver might handle more features than the board; OK.
* Fail when the board needs a feature that's not enabled.
@@ -1890,13 +1895,6 @@ musb_init_controller(struct device *dev, int nIrq, void
__iomem *ctrl)
musb->irq_wake = 0;
}
- /* host side needs more setup */
- hcd = musb_to_hcd(musb);
- otg_set_host(musb->xceiv->otg, &hcd->self);
- hcd->self.otg_port = 1;
- musb->xceiv->otg->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);
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 87da041..6b65847 100644
-
Re: [PATCH v3 06/11] usb: musb: factor out hcd initalization
> "Daniel" == Daniel Mack writes:
Daniel> The musb struct is currently allocated along with the hcd, which makes
Daniel> it difficult to build a driver that only acts as gadget device.
Daniel> Fix this by allocating musb directly, and keep the hcd around as
Daniel> a pointer in the musb struct.
Daniel> struct hc_driver musb_hc_driver can now also be static to musb_host.c,
Daniel> and the macro musb_to_hcd() is just a pointer dereferencer for now, and
Daniel> will be eliminated later.
Daniel> Signed-off-by: Daniel Mack
Daniel> Acked-by: Peter Korsgaard
Daniel> ---
Daniel> drivers/usb/musb/musb_core.c | 40
Daniel> drivers/usb/musb/musb_core.h | 1 +
Daniel> drivers/usb/musb/musb_host.c | 54
++--
Daniel> drivers/usb/musb/musb_host.h | 19 +++-
Daniel> 4 files changed, 75 insertions(+), 39 deletions(-)
Daniel> +struct musb_hcd_link {
Daniel> + struct musb *musb;
Daniel> +};
Daniel> +
Daniel> +struct musb *hcd_to_musb(struct usb_hcd *hcd)
Daniel> +{
Daniel> + struct musb_hcd_link *link = (struct musb_hcd_link *)
hcd->hcd_priv;
Daniel> + return link->musb;
Daniel> +}
Daniel> +
Sorry, I missed this first time around - But why the indirection with
musb_hcd_link? Why not simply directly store a pointer to struct musb in
hcd_priv?
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 06/11] usb: musb: factor out hcd initalization
On 10.04.2013 20:54, Peter Korsgaard wrote:
>> "Daniel" == Daniel Mack writes:
>
> Daniel> The musb struct is currently allocated along with the hcd, which
> makes
> Daniel> it difficult to build a driver that only acts as gadget device.
>
> Daniel> Fix this by allocating musb directly, and keep the hcd around as
> Daniel> a pointer in the musb struct.
>
> Daniel> struct hc_driver musb_hc_driver can now also be static to
> musb_host.c,
> Daniel> and the macro musb_to_hcd() is just a pointer dereferencer for now,
> and
> Daniel> will be eliminated later.
>
> Daniel> Signed-off-by: Daniel Mack
> Daniel> Acked-by: Peter Korsgaard
> Daniel> ---
> Daniel> drivers/usb/musb/musb_core.c | 40
> Daniel> drivers/usb/musb/musb_core.h | 1 +
> Daniel> drivers/usb/musb/musb_host.c | 54
> ++--
> Daniel> drivers/usb/musb/musb_host.h | 19 +++-
> Daniel> 4 files changed, 75 insertions(+), 39 deletions(-)
>
>
> Daniel> +struct musb_hcd_link {
> Daniel> +struct musb *musb;
> Daniel> +};
> Daniel> +
> Daniel> +struct musb *hcd_to_musb(struct usb_hcd *hcd)
> Daniel> +{
> Daniel> +struct musb_hcd_link *link = (struct musb_hcd_link *)
> hcd->hcd_priv;
> Daniel> +return link->musb;
> Daniel> +}
> Daniel> +
>
> Sorry, I missed this first time around - But why the indirection with
> musb_hcd_link? Why not simply directly store a pointer to struct musb in
> hcd_priv?
Well, that's also possible. I just thought it's nicer (more readable)
that way. But I can as well rework it so the struct isn't needed. It
won't safe us any binary size or anything though. So I'm not sure.
Any particular reason why you don't like the struct? :)
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 06/11] usb: musb: factor out hcd initalization
> "Daniel" == Daniel Mack writes:
Hi,
Daniel> +struct musb_hcd_link {
Daniel> + struct musb *musb;
Daniel> +};
Daniel> +
Daniel> +struct musb *hcd_to_musb(struct usb_hcd *hcd)
Daniel> +{
Daniel> + struct musb_hcd_link *link = (struct musb_hcd_link *)
hcd->hcd_priv;
Daniel> + return link->musb;
Daniel> +}
Daniel> +
>>
>> Sorry, I missed this first time around - But why the indirection with
>> musb_hcd_link? Why not simply directly store a pointer to struct musb in
>> hcd_priv?
Daniel> Well, that's also possible. I just thought it's nicer (more readable)
Daniel> that way. But I can as well rework it so the struct isn't needed. It
Daniel> won't safe us any binary size or anything though. So I'm not sure.
Well, it will save one level of indirection (hcd_priv->link->musb vs
hcd_priv->musb).
Daniel> Any particular reason why you don't like the struct? :)
Just that it's an unneeded extra level of indirection.
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 06/11] usb: musb: factor out hcd initalization
On 10.04.2013 21:15, Peter Korsgaard wrote:
>> "Daniel" == Daniel Mack writes:
>
> Hi,
>
> Daniel> +struct musb_hcd_link {
> Daniel> +struct musb *musb;
> Daniel> +};
> Daniel> +
> Daniel> +struct musb *hcd_to_musb(struct usb_hcd *hcd)
> Daniel> +{
> Daniel> +struct musb_hcd_link *link = (struct musb_hcd_link *)
> hcd->hcd_priv;
> Daniel> +return link->musb;
> Daniel> +}
> Daniel> +
> >>
> >> Sorry, I missed this first time around - But why the indirection with
> >> musb_hcd_link? Why not simply directly store a pointer to struct musb in
> >> hcd_priv?
>
> Daniel> Well, that's also possible. I just thought it's nicer (more readable)
> Daniel> that way. But I can as well rework it so the struct isn't needed. It
> Daniel> won't safe us any binary size or anything though. So I'm not sure.
>
> Well, it will save one level of indirection (hcd_priv->link->musb vs
> hcd_priv->musb).
>
> Daniel> Any particular reason why you don't like the struct? :)
>
> Just that it's an unneeded extra level of indirection.
>
Ok, alright. Sent v4 right now.
Thanks for your feedback! Much appreciated. Btw - did you try that on
your board yet? Does it work for you as well?
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 06/11] usb: musb: factor out hcd initalization
> "Daniel" == Daniel Mack writes: Hi, >> Just that it's an unneeded extra level of indirection. Daniel> Ok, alright. Sent v4 right now. Thanks! Daniel> Thanks for your feedback! Much appreciated. Btw - did you try Daniel> that on your board yet? Does it work for you as well? Sorry, not yet. I'm temporarily working on a dm8168 based board (which unfortunately doesn't have mainline support), and I'm afraid I won't be able to test it on our am335x based board until a few weeks. I will need USB host support though, so I am interested in getting it fixed. Thanks for doing this work! -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
