On 05.04.2013 15:03, Felipe Balbi wrote:
> On Thu, Apr 04, 2013 at 09:50:09PM +0200, Daniel Mack wrote:
>> 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 allocation musb directly, and keep the hcd around as pointer.
>
> Fix this by *allocating*
>
>> 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 elminiated later.
>>
>> Signed-off-by: Daniel Mack <zonque@xxxxxxxxx>
>> ---
>> drivers/usb/musb/musb_core.c | 60 ++++++++++++++++++--------------------
>> drivers/usb/musb/musb_core.h | 1 +
>> drivers/usb/musb/musb_gadget.c | 10 -------
>> drivers/usb/musb/musb_host.c | 65 ++++++++++++++++++++++++++++++++++++++++--
>> drivers/usb/musb/musb_host.h | 21 +++++++-------
>> 5 files changed, 102 insertions(+), 55 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
>> index 37a261a..d5e9794 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);
>> @@ -484,7 +485,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
>>
>> musb->xceiv->state = OTG_STATE_A_HOST;
>> musb->is_active = 1;
>> - usb_hcd_resume_root_hub(musb_to_hcd(musb));
>> + musb_host_resume_root_hub(musb);
>
> this small re-factoring should be done in a separate patch coming before
> $subject.
>
>> @@ -734,17 +736,13 @@ b_host:
>> if ((devctl & MUSB_DEVCTL_VBUS)
>> == (3 << MUSB_DEVCTL_VBUS_SHIFT)) {
>> musb->xceiv->state = OTG_STATE_A_HOST;
>> - hcd->self.is_b_host = 0;
>> + if (hcd)
>> + hcd->self.is_b_host = 0;
>> }
>> break;
>> }
>>
>> - /* poke the root hub */
>> - MUSB_HST_MODE(musb);
>> - if (hcd->status_urb)
>> - usb_hcd_poll_rh_status(hcd);
>> - else
>> - usb_hcd_resume_root_hub(hcd);
>> + musb_host_poke_root_hub(musb);
>
> likewise for this one.
>
>> @@ -1763,24 +1762,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 = kzalloc(sizeof(*musb), GFP_KERNEL);
>
> devm_* ? Or perhaps in a later patch.
>
>> +void musb_host_cleanup(struct musb *musb)
>> +{
>> + usb_remove_hcd(musb->hcd);
>> + musb->hcd = NULL;
>> +}
>> +
>> +void musb_host_free(struct musb *musb)
>> +{
>> + usb_put_hcd(musb->hcd);
>> +}
>> +
>> +void musb_host_resume_root_hub(struct musb *musb)
>> +{
>> + usb_hcd_resume_root_hub(musb->hcd);
>> +}
>> +
>> +void musb_host_poll_rh_status(struct musb *musb)
>> +{
>> + usb_hcd_poll_rh_status(musb->hcd);
>> +}
>> +
>> +void musb_host_poke_root_hub(struct musb *musb)
>> +{
>> + MUSB_HST_MODE(musb);
>> + if (musb->hcd->status_urb)
>> + usb_hcd_poll_rh_status(musb->hcd);
>> + else
>> + usb_hcd_resume_root_hub(musb->hcd);
>> +}
>
> no need to check for NULL hcd in any of these ? I guess you're relying
> on those being stubbed out when Host isn't compiled in, am I right ?
Yes, exactly. musb->hcd will be a NULL pointer then, which is unused by
the static inline function.
Thanks for the review though, prepare a new series.
Daniel
>
> just being sure...
>
>> diff --git a/drivers/usb/musb/musb_host.h b/drivers/usb/musb/musb_host.h
>> index 9670269..fb24422 100644
>> --- a/drivers/usb/musb/musb_host.h
>> +++ b/drivers/usb/musb/musb_host.h
>> @@ -37,15 +37,9 @@
>>
>> #include <linux/scatterlist.h>
>>
>> -static inline struct usb_hcd *musb_to_hcd(struct musb *musb)
>> -{
>> - return container_of((void *) musb, struct usb_hcd, hcd_priv);
>> -}
>> +#define musb_to_hcd(MUSB) ((MUSB)->hcd)
>
> all lower cases, please.
>
--
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
