Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1426274 > unrolled thread

Re: [PATCH v10 06/14] usb: gadget.h: Add OTG to gadget interface

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v10 06/14] usb: gadget.h: Add OTG to gadget interface Felipe Balbi <balbi@kernel.org> - 2016-06-20 09:30 +0200
    Re: [PATCH v10 06/14] usb: gadget.h: Add OTG to gadget interface Roger Quadros <rogerq@ti.com> - 2016-06-20 09:30 +0200
      Re: [PATCH v10 06/14] usb: gadget.h: Add OTG to gadget interface Felipe Balbi <balbi@kernel.org> - 2016-06-20 10:20 +0200
        Re: [PATCH v10 06/14] usb: gadget.h: Add OTG to gadget interface Roger Quadros <rogerq@ti.com> - 2016-06-20 10:40 +0200
          Re: [PATCH v10 06/14] usb: gadget.h: Add OTG to gadget interface Felipe Balbi <balbi@kernel.org> - 2016-06-20 11:30 +0200
            Re: [PATCH v10 06/14] usb: gadget.h: Add OTG to gadget interface Roger Quadros <rogerq@ti.com> - 2016-06-20 12:00 +0200

#1426274 — Re: [PATCH v10 06/14] usb: gadget.h: Add OTG to gadget interface

[Multipart message — attachments visible in raw view] — view raw

Hi,

Roger Quadros <rogerq@ti.com> writes:
> The OTG core will use struct otg_gadget_ops to
> start/stop the gadget controller.
>
> The main purpose of this interface is to avoid directly
> calling usb_gadget_start/stop() from the OTG core as they
> wouldn't be defined in the built-in symbol table if
> CONFIG_USB_GADGET is m.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  include/linux/usb/gadget.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index 2dd9e6b..f4fc0aa 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -639,6 +639,22 @@ struct usb_gadget_driver {
>  };
>  
>  
> +/*-------------------------------------------------------------------------*/
> +
> +/**
> + * struct otg_gadget_ops - Interface between OTG core and gadget
> + *
> + * Provided by the gadget core to allow the OTG core to start/stop the gadget
> + *
> + * @start: function to start the gadget
> + * @stop: function to stop the gadget
> + * @connect_control: function to connect/disconnect from the bus
> + */
> +struct otg_gadget_ops {
> +	int (*start)(struct usb_gadget *gadget);
> +	int (*stop)(struct usb_gadget *gadget);
> +	int (*connect_control)(struct usb_gadget *gadget, bool connect);
> +};

you shouldn't need these at all. They are already part of the gadget
framework as ->udc_start(), ->udc_stop() and ->pullup()

-- 
balbi

[toc] | [next] | [standalone]

#1426280

[Multipart message — attachments visible in raw view] — view raw

On 20/06/16 10:21, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> The OTG core will use struct otg_gadget_ops to
>> start/stop the gadget controller.
>>
>> The main purpose of this interface is to avoid directly
>> calling usb_gadget_start/stop() from the OTG core as they
>> wouldn't be defined in the built-in symbol table if
>> CONFIG_USB_GADGET is m.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  include/linux/usb/gadget.h | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>> index 2dd9e6b..f4fc0aa 100644
>> --- a/include/linux/usb/gadget.h
>> +++ b/include/linux/usb/gadget.h
>> @@ -639,6 +639,22 @@ struct usb_gadget_driver {
>>  };
>>  
>>  
>> +/*-------------------------------------------------------------------------*/
>> +
>> +/**
>> + * struct otg_gadget_ops - Interface between OTG core and gadget
>> + *
>> + * Provided by the gadget core to allow the OTG core to start/stop the gadget
>> + *
>> + * @start: function to start the gadget
>> + * @stop: function to stop the gadget
>> + * @connect_control: function to connect/disconnect from the bus
>> + */
>> +struct otg_gadget_ops {
>> +	int (*start)(struct usb_gadget *gadget);
>> +	int (*stop)(struct usb_gadget *gadget);
>> +	int (*connect_control)(struct usb_gadget *gadget, bool connect);
>> +};
> 
> you shouldn't need these at all. They are already part of the gadget
> framework as ->udc_start(), ->udc_stop() and ->pullup()
> 

This is to avoid the undefined symbol errors during build when OTG has is
built-in because USB (host) is built-in but GADGET is still a module.

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1426319

[Multipart message — attachments visible in raw view] — view raw

Hi,

Roger Quadros <rogerq@ti.com> writes:
> [ Unknown signature status ]
> On 20/06/16 10:21, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Roger Quadros <rogerq@ti.com> writes:
>>> The OTG core will use struct otg_gadget_ops to
>>> start/stop the gadget controller.
>>>
>>> The main purpose of this interface is to avoid directly
>>> calling usb_gadget_start/stop() from the OTG core as they
>>> wouldn't be defined in the built-in symbol table if
>>> CONFIG_USB_GADGET is m.
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>>  include/linux/usb/gadget.h | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>>> index 2dd9e6b..f4fc0aa 100644
>>> --- a/include/linux/usb/gadget.h
>>> +++ b/include/linux/usb/gadget.h
>>> @@ -639,6 +639,22 @@ struct usb_gadget_driver {
>>>  };
>>>  
>>>  
>>> +/*-------------------------------------------------------------------------*/
>>> +
>>> +/**
>>> + * struct otg_gadget_ops - Interface between OTG core and gadget
>>> + *
>>> + * Provided by the gadget core to allow the OTG core to start/stop the gadget
>>> + *
>>> + * @start: function to start the gadget
>>> + * @stop: function to stop the gadget
>>> + * @connect_control: function to connect/disconnect from the bus
>>> + */
>>> +struct otg_gadget_ops {
>>> +	int (*start)(struct usb_gadget *gadget);
>>> +	int (*stop)(struct usb_gadget *gadget);
>>> +	int (*connect_control)(struct usb_gadget *gadget, bool connect);
>>> +};
>> 
>> you shouldn't need these at all. They are already part of the gadget
>> framework as ->udc_start(), ->udc_stop() and ->pullup()
>> 
>
> This is to avoid the undefined symbol errors during build when OTG has is
> built-in because USB (host) is built-in but GADGET is still a module.

change your Kconfig dependencies. OTG layer shouldn't be built-in unless
both Gadget and Host are built-in.

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1426329

[Multipart message — attachments visible in raw view] — view raw

On 20/06/16 11:13, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>> [ Unknown signature status ]
>> On 20/06/16 10:21, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> Roger Quadros <rogerq@ti.com> writes:
>>>> The OTG core will use struct otg_gadget_ops to
>>>> start/stop the gadget controller.
>>>>
>>>> The main purpose of this interface is to avoid directly
>>>> calling usb_gadget_start/stop() from the OTG core as they
>>>> wouldn't be defined in the built-in symbol table if
>>>> CONFIG_USB_GADGET is m.
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>> ---
>>>>  include/linux/usb/gadget.h | 16 ++++++++++++++++
>>>>  1 file changed, 16 insertions(+)
>>>>
>>>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>>>> index 2dd9e6b..f4fc0aa 100644
>>>> --- a/include/linux/usb/gadget.h
>>>> +++ b/include/linux/usb/gadget.h
>>>> @@ -639,6 +639,22 @@ struct usb_gadget_driver {
>>>>  };
>>>>  
>>>>  
>>>> +/*-------------------------------------------------------------------------*/
>>>> +
>>>> +/**
>>>> + * struct otg_gadget_ops - Interface between OTG core and gadget
>>>> + *
>>>> + * Provided by the gadget core to allow the OTG core to start/stop the gadget
>>>> + *
>>>> + * @start: function to start the gadget
>>>> + * @stop: function to stop the gadget
>>>> + * @connect_control: function to connect/disconnect from the bus
>>>> + */
>>>> +struct otg_gadget_ops {
>>>> +	int (*start)(struct usb_gadget *gadget);
>>>> +	int (*stop)(struct usb_gadget *gadget);
>>>> +	int (*connect_control)(struct usb_gadget *gadget, bool connect);
>>>> +};
>>>
>>> you shouldn't need these at all. They are already part of the gadget
>>> framework as ->udc_start(), ->udc_stop() and ->pullup()
>>>
>>
>> This is to avoid the undefined symbol errors during build when OTG has is
>> built-in because USB (host) is built-in but GADGET is still a module.
> 
> change your Kconfig dependencies. OTG layer shouldn't be built-in unless
> both Gadget and Host are built-in.
> 

That is only one side of the story.
What happens if OTG is (m), Host is (m) but Gadget is (built in).

Gadget build will fail because of undefined symbol errors for all of the
OTG APIs.

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1426381

[Multipart message — attachments visible in raw view] — view raw

Hi,

Roger Quadros <rogerq@ti.com> writes:
>>>> Roger Quadros <rogerq@ti.com> writes:
>>>>> The OTG core will use struct otg_gadget_ops to
>>>>> start/stop the gadget controller.
>>>>>
>>>>> The main purpose of this interface is to avoid directly
>>>>> calling usb_gadget_start/stop() from the OTG core as they
>>>>> wouldn't be defined in the built-in symbol table if
>>>>> CONFIG_USB_GADGET is m.
>>>>>
>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>>> ---
>>>>>  include/linux/usb/gadget.h | 16 ++++++++++++++++
>>>>>  1 file changed, 16 insertions(+)
>>>>>
>>>>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>>>>> index 2dd9e6b..f4fc0aa 100644
>>>>> --- a/include/linux/usb/gadget.h
>>>>> +++ b/include/linux/usb/gadget.h
>>>>> @@ -639,6 +639,22 @@ struct usb_gadget_driver {
>>>>>  };
>>>>>  
>>>>>  
>>>>> +/*-------------------------------------------------------------------------*/
>>>>> +
>>>>> +/**
>>>>> + * struct otg_gadget_ops - Interface between OTG core and gadget
>>>>> + *
>>>>> + * Provided by the gadget core to allow the OTG core to start/stop the gadget
>>>>> + *
>>>>> + * @start: function to start the gadget
>>>>> + * @stop: function to stop the gadget
>>>>> + * @connect_control: function to connect/disconnect from the bus
>>>>> + */
>>>>> +struct otg_gadget_ops {
>>>>> +	int (*start)(struct usb_gadget *gadget);
>>>>> +	int (*stop)(struct usb_gadget *gadget);
>>>>> +	int (*connect_control)(struct usb_gadget *gadget, bool connect);
>>>>> +};
>>>>
>>>> you shouldn't need these at all. They are already part of the gadget
>>>> framework as ->udc_start(), ->udc_stop() and ->pullup()
>>>>
>>>
>>> This is to avoid the undefined symbol errors during build when OTG has is
>>> built-in because USB (host) is built-in but GADGET is still a module.
>> 
>> change your Kconfig dependencies. OTG layer shouldn't be built-in unless
>> both Gadget and Host are built-in.
>> 
>
> That is only one side of the story.
> What happens if OTG is (m), Host is (m) but Gadget is (built in).
>
> Gadget build will fail because of undefined symbol errors for all of
> the OTG APIs.

So you created a circular dependency, right? That's not good either. I
suggest you clean that up because this is bound to create randconfig
build problems in the long run.

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1426450

[Multipart message — attachments visible in raw view] — view raw

On 20/06/16 12:24, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>>>>> Roger Quadros <rogerq@ti.com> writes:
>>>>>> The OTG core will use struct otg_gadget_ops to
>>>>>> start/stop the gadget controller.
>>>>>>
>>>>>> The main purpose of this interface is to avoid directly
>>>>>> calling usb_gadget_start/stop() from the OTG core as they
>>>>>> wouldn't be defined in the built-in symbol table if
>>>>>> CONFIG_USB_GADGET is m.
>>>>>>
>>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>>>> ---
>>>>>>  include/linux/usb/gadget.h | 16 ++++++++++++++++
>>>>>>  1 file changed, 16 insertions(+)
>>>>>>
>>>>>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>>>>>> index 2dd9e6b..f4fc0aa 100644
>>>>>> --- a/include/linux/usb/gadget.h
>>>>>> +++ b/include/linux/usb/gadget.h
>>>>>> @@ -639,6 +639,22 @@ struct usb_gadget_driver {
>>>>>>  };
>>>>>>  
>>>>>>  
>>>>>> +/*-------------------------------------------------------------------------*/
>>>>>> +
>>>>>> +/**
>>>>>> + * struct otg_gadget_ops - Interface between OTG core and gadget
>>>>>> + *
>>>>>> + * Provided by the gadget core to allow the OTG core to start/stop the gadget
>>>>>> + *
>>>>>> + * @start: function to start the gadget
>>>>>> + * @stop: function to stop the gadget
>>>>>> + * @connect_control: function to connect/disconnect from the bus
>>>>>> + */
>>>>>> +struct otg_gadget_ops {
>>>>>> +	int (*start)(struct usb_gadget *gadget);
>>>>>> +	int (*stop)(struct usb_gadget *gadget);
>>>>>> +	int (*connect_control)(struct usb_gadget *gadget, bool connect);
>>>>>> +};
>>>>>
>>>>> you shouldn't need these at all. They are already part of the gadget
>>>>> framework as ->udc_start(), ->udc_stop() and ->pullup()
>>>>>
>>>>
>>>> This is to avoid the undefined symbol errors during build when OTG has is
>>>> built-in because USB (host) is built-in but GADGET is still a module.
>>>
>>> change your Kconfig dependencies. OTG layer shouldn't be built-in unless
>>> both Gadget and Host are built-in.
>>>
>>
>> That is only one side of the story.
>> What happens if OTG is (m), Host is (m) but Gadget is (built in).
>>
>> Gadget build will fail because of undefined symbol errors for all of
>> the OTG APIs.
> 
> So you created a circular dependency, right? That's not good either. I
> suggest you clean that up because this is bound to create randconfig
> build problems in the long run.
> 

We can't avoid the circular dependency.
Gadget/Host has to register with otg. otg has to use gadget & host functions.

If you have better ideas, please let us know.

cheers,
-roger

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web