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 <zon...@gmail.com>
>> ---
>>  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to