Re: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[David Brownell]

--- On Mon, 5/31/10, Grazvydas Ignotas  wrote:

> Another issue is that OTG depends on PM, so if I build  PM-less kernel
> (PM is still more or less in development on OMAPs), Kconfig will only
> allow host or peripheral for musb, which doesn't match OTG selection
> in the board file, and the driver fails.

As Felipe partially explained:  OTG implies
HNP, and HNP requires suspending the USB link,
which requires some PM support...



--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[Felipe Balbi]

On Mon, May 31, 2010 at 11:44:05AM +0200, ext Grazvydas Ignotas wrote:

If a board is configured as host and driver is set to only support
peripheral it is perfectly fine for the driver not to allow that. But
why is the driver refusing OTG board setting if it's compiled to only
support host/peripheral? If I apply [1], it works perfectly fine for
me. What's wrong with degrading OTG to host/peripheral, did you really
mean to NAK [1], David? Why can't I compile musb in a
periperal/host-only mode if I have OTG connector?


otg depends on usb suspend, which depends on runtime pm (I guess), 
without suspend, host negotiation protocol will never work which is 
required on OTG.


I believe that was Dave's reasoning.

--
balbi

DefectiveByDesign.org
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[Grazvydas Ignotas]

On Fri, May 28, 2010 at 9:32 PM, David Brownell  wrote:
> --- On Fri, 5/28/10, Steve Sakoman  wrote:
>
>> > By the way ... the #ifdeffery should indeed vanish
>> from all board
>> > configs except the Davinci DM6446 EVM.  That board is
>> kind of quirky
>> > in terms of USB support, and needs jumpering to get
>> host or peripheral mode (and can't do OTG).
>
>
>> For Overo this patch actually makes quite a bit of sense
>> (and I have
>> been using something similar for quite some time).
>>
>> The Overo COM itself has no USB connector -- the customer
>> can choose
>> or create a daughter card with the type of USB connector
>> appropriate> to their application,
>
> So that would be another exception ... because it doesn't
> have a fixed USB connector (and thus does not convey to
> users any specific details about what modes it supports).

Another issue is that OTG depends on PM, so if I build PM-less kernel
(PM is still more or less in development on OMAPs), Kconfig will only
allow host or peripheral for musb, which doesn't match OTG selection
in the board file, and the driver fails.

If a board is configured as host and driver is set to only support
peripheral it is perfectly fine for the driver not to allow that. But
why is the driver refusing OTG board setting if it's compiled to only
support host/peripheral? If I apply [1], it works perfectly fine for
me. What's wrong with degrading OTG to host/peripheral, did you really
mean to NAK [1], David? Why can't I compile musb in a
periperal/host-only mode if I have OTG connector?

I have to patch around this every time :(

[1] http://marc.info/?l=linux-usb&m=127367875600618&w=2

>
> - Dave
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[David Brownell]

--- On Fri, 5/28/10, Steve Sakoman  wrote:

> > By the way ... the #ifdeffery should indeed vanish
> from all board
> > configs except the Davinci DM6446 EVM.  That board is
> kind of quirky
> > in terms of USB support, and needs jumpering to get
> host or peripheral mode (and can't do OTG).


> For Overo this patch actually makes quite a bit of sense
> (and I have
> been using something similar for quite some time).
> 
> The Overo COM itself has no USB connector -- the customer
> can choose
> or create a daughter card with the type of USB connector
> appropriate> to their application,

So that would be another exception ... because it doesn't
have a fixed USB connector (and thus does not convey to
users any specific details about what modes it supports).

- Dave


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[Steve Sakoman]

On Thu, May 20, 2010 at 8:48 AM, David Brownell  wrote:
>
>> +#ifdef CONFIG_USB_MUSB_OTG
>> +    .mode
>>         = MUSB_OTG,
>> +#elif defined(CONFIG_USB_MUSB_HDRC_HCD)
>> +    .mode
>>         = MUSB_HOST,
>> +#elif defined(CONFIG_USB_GADGET_MUSB_HDRC)
>>  .mode
>>         = MUSB_PERIPHERAL,
>> +#endif
>
>>         = MUSB_PERIPHERAL,
>> +#endif
>
> By the way ... the #ifdeffery should indeed vanish from all board
> configs except the Davinci DM6446 EVM.  That board is kind of quirky
> in terms of USB support, and needs jumpering to get host or peripheral
> mode (and can't do OTG).  I suspect some of the other boards did a bad
> thing and tried emulating what that board did.

For Overo this patch actually makes quite a bit of sense (and I have
been using something similar for quite some time).

The Overo COM itself has no USB connector -- the customer can choose
or create a daughter card with the type of USB connector appropriate
to their application, be it host, peripheral, or OTG.  Having the mode
easily settable via a config option allows them to use the standard
board file without modification.

Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[Ming Lei]

sorry, some fixes.

2010/5/28 Ming Lei :
> 2010/5/28 Gadiyar, Anand :
>> David Brownell wrote:
>>
>> (Okay, I do know anyone who compiles a custom kernel of
>> this sort is likely capable of patching the board files
>> as needed to downgrade to the desired mode. But I still
>> think the patch at [1] is useful to have.)
>
> David NAK the patch and it is the reason:
>
> 1, don't support mini-A plug connected into mini-ab receptacle if
> degrading to host-only role;

 1, don't support mini-B plug connected into mini-ab receptacle if
  degrading to host-only role;

>
> and
>
> 2,don't support mini-B plug connected into mini-ab receptacle if
> degrading to peripheral-only role.

2,don't support mini-A plug connected into mini-ab receptacle if
degrading to peripheral-only role.

> This two cases may confuse users.
>
> Maybe we can print messages to user to show the cases above.
> Anyway, I don't mind if the patch is accepted or not.
>
>
> --
> Lei Ming
>



-- 
Lei Ming
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[Ming Lei]

2010/5/28 Gadiyar, Anand :
> David Brownell wrote:
>
> (Okay, I do know anyone who compiles a custom kernel of
> this sort is likely capable of patching the board files
> as needed to downgrade to the desired mode. But I still
> think the patch at [1] is useful to have.)

David NAK the patch and it is the reason:

1, don't support mini-A plug connected into mini-ab receptacle if
degrading to host-only role;

and

2,don't support mini-B plug connected into mini-ab receptacle if
degrading to peripheral-only role.

This two cases may confuse users.

Maybe we can print messages to user to show the cases above.
Anyway, I don't mind if the patch is accepted or not.


-- 
Lei Ming
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[Gadiyar, Anand]

David Brownell wrote:
> > 
> > >"incompatible Kconfig role setting"
> > 
> > there's a patch making that a warning instead of an #error
> > if I'm not wrong.
> 
> It's not an #error, it's a dev_err().
> 
> But more to the point, it's reporting a real error.  As
> the comment in the code explains:
> 
> /* The driver might handle more features than
>  * the board; OK.
>  * Fail when the board needs a feature that's
>  * not enabled.
>  */
> 
> Ut;s simple:  OTG connectors require OTG support.  Host
> connectors require host support.  Peripheral connectors 
> require peripheral support.  Why is anyone confused??


A board which has an OTG connector, and supports OTG mode
with this driver, very likely also supports host-only and
peripheral-only usage on that port.

Many times, we build kernels without USB host support
or without USB gadget support compiled in (if only for
testing). Instead of failing init, wouldn't it be useful
to report a big warning and downgrading to host-only
or peripheral-only roles as configured in the kernel?

(which is almost what Ming Lei's patch at [1] does - except
that it uses dev_info instead of something more severe).

(Okay, I do know anyone who compiles a custom kernel of
this sort is likely capable of patching the board files
as needed to downgrade to the desired mode. But I still
think the patch at [1] is useful to have.)

- Anand

[1] 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[David Brownell]

> 
> >"incompatible Kconfig role setting"
> 
> there's a patch making that a warning instead of an #error
> if I'm not wrong.

It's not an #error, it's a dev_err().

But more to the point, it's reporting a real error.  As
the comment in the code explains:

/* The driver might handle more features than
 * the board; OK.
 * Fail when the board needs a feature that's
 * not enabled.
 */

Ut;s simple:  OTG connectors require OTG support.  Host
connectors require host support.  Peripheral connectors require peripheral 
support.  Why is anyone confused??


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[David Brownell]

--- On Fri, 5/21/10, Gupta, Ajay Kumar  wrote:

> > >Then I think these #ifdefferys are required in all
> the board files.

As I already explained:  *NO*.

Unless the board has been designed in a flakey
manner (like the DM6446 EVM) it should
have no #ifdeffery in the MUSB configuration.


> > >Then if I choose to compile the Kernel only for
> peripheral mode
> > >(Case-B above) then I would get below error from
> musb_core.c.
> > >
> > >"incompatible Kconfig role setting"

That's a *correct* report that your config is broken.
The hardware has an OTG connector, so the kernel should
be supporting OTG mode.  If it's not, that's an error;
don't try to hide such errors.

- Dave
 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[Felipe Balbi]

Hi,

On Fri, May 21, 2010 at 09:43:29AM +0200, ext Gupta, Ajay Kumar wrote:

I meant OTG with ID pin based role.


that's the only OTG mode. If your board has OTG support, it will have to 
act on ID pin.


--
balbi

DefectiveByDesign.org
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[Gupta, Ajay Kumar]

Hi,

> On Fri, May 21, 2010 at 07:15:37AM +0200, ext Gupta, Ajay Kumar wrote:
> >Hi,
> >> > +#ifdef CONFIG_USB_MUSB_OTG
> >> > +    .mode
> >> >         = MUSB_OTG,
> >> > +#elif defined(CONFIG_USB_MUSB_HDRC_HCD)
> >> > +    .mode
> >> >         = MUSB_HOST,
> >> > +#elif defined(CONFIG_USB_GADGET_MUSB_HDRC)
> >> >      .mode
> >> >         = MUSB_PERIPHERAL,
> >> > +#endif
> >>
> >> >         = MUSB_PERIPHERAL,
> >> > +#endif
> >>
> >> By the way ... the #ifdeffery should indeed vanish from all board
> >> configs except the Davinci DM6446 EVM.
> >
> >If we are claiming to support:
> > A. Same kernel would work on multiple boards
> > B. Single kernel for one single board
> >
> >Then I think these #ifdefferys are required in all the board files.
> >
> >Let's consider the OMAP3EVM which supports all the three modes and
> 
> you don't support "all three modes" you support OTG. If you want to
> build for peripheral only, that's your choice, but the board is wired so
> that it has OTG support.

I meant OTG with ID pin based role.

> 
> >If these #ifdefferys are not present and .mode is set to OTG.
> >Then if I choose to compile the Kernel only for peripheral mode
> >(Case-B above) then I would get below error from musb_core.c.
> >
> >"incompatible Kconfig role setting"
> 
> there's a patch making that a warning instead of an #error if I'm not
> wrong.

If that patch ([1] below) is taken then we can avoid all these #ifdefs.

[1] http://marc.info/?l=linux-usb&m=127367875600618&w=2

But I see comment from David on above patch [1] to fix board data
Instead which means using #ifdefs again in board files...

With all these discussion it's best to accept the above [1] patch and
Avoid having ugly #ifdefs from board files.

-Ajay
> 
> --
> balbi
> 
> DefectiveByDesign.org
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[Felipe Balbi]

On Fri, May 21, 2010 at 07:15:37AM +0200, ext Gupta, Ajay Kumar wrote:

Hi,

> +#ifdef CONFIG_USB_MUSB_OTG
> +    .mode
>         = MUSB_OTG,
> +#elif defined(CONFIG_USB_MUSB_HDRC_HCD)
> +    .mode
>         = MUSB_HOST,
> +#elif defined(CONFIG_USB_GADGET_MUSB_HDRC)
>      .mode
>         = MUSB_PERIPHERAL,
> +#endif

>         = MUSB_PERIPHERAL,
> +#endif

By the way ... the #ifdeffery should indeed vanish from all board
configs except the Davinci DM6446 EVM.


If we are claiming to support:
A. Same kernel would work on multiple boards
B. Single kernel for one single board

Then I think these #ifdefferys are required in all the board files.

Let's consider the OMAP3EVM which supports all the three modes and


you don't support "all three modes" you support OTG. If you want to 
build for peripheral only, that's your choice, but the board is wired so 
that it has OTG support.



If these #ifdefferys are not present and .mode is set to OTG.
Then if I choose to compile the Kernel only for peripheral mode
(Case-B above) then I would get below error from musb_core.c.

"incompatible Kconfig role setting"


there's a patch making that a warning instead of an #error if I'm not 
wrong.


--
balbi

DefectiveByDesign.org
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[Gupta, Ajay Kumar]

Hi,
> > +#ifdef CONFIG_USB_MUSB_OTG
> > +    .mode
> >         = MUSB_OTG,
> > +#elif defined(CONFIG_USB_MUSB_HDRC_HCD)
> > +    .mode
> >         = MUSB_HOST,
> > +#elif defined(CONFIG_USB_GADGET_MUSB_HDRC)
> >      .mode
> >         = MUSB_PERIPHERAL,
> > +#endif
> 
> >         = MUSB_PERIPHERAL,
> > +#endif
> 
> By the way ... the #ifdeffery should indeed vanish from all board
> configs except the Davinci DM6446 EVM.

If we are claiming to support:
A. Same kernel would work on multiple boards
B. Single kernel for one single board

Then I think these #ifdefferys are required in all the board files.

Let's consider the OMAP3EVM which supports all the three modes and
If these #ifdefferys are not present and .mode is set to OTG.
Then if I choose to compile the Kernel only for peripheral mode
(Case-B above) then I would get below error from musb_core.c.

"incompatible Kconfig role setting"

-Ajay

>  That board is kind of quirky
> in terms of USB support, and needs jumpering to get host or peripheral
> mode (and can't do OTG).  I suspect some of the other boards did a bad
> thing and tried emulating what that board did.
> 
> But the description for such a patch would emphasize removing bogus
> ifdeffery, to match the board hardware (which is not conditional but
> is instead fixed).
> 
> --
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[David Brownell]

> +#ifdef CONFIG_USB_MUSB_OTG
> +    .mode   
>         = MUSB_OTG,
> +#elif defined(CONFIG_USB_MUSB_HDRC_HCD)
> +    .mode   
>         = MUSB_HOST,
> +#elif defined(CONFIG_USB_GADGET_MUSB_HDRC)
>      .mode   
>         = MUSB_PERIPHERAL,
> +#endif

>         = MUSB_PERIPHERAL,
> +#endif

By the way ... the #ifdeffery should indeed vanish from all board
configs except the Davinci DM6446 EVM.  That board is kind of quirky
in terms of USB support, and needs jumpering to get host or peripheral
mode (and can't do OTG).  I suspect some of the other boards did a bad
thing and tried emulating what that board did.

But the description for such a patch would emphasize removing bogus
ifdeffery, to match the board hardware (which is not conditional but
is instead fixed).

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[David Brownell]

> > Subject: [PATCH] ARM: OMAP: Fix board data to support
> device only, host only and OTG roles.
> 
> > Fix board data to support device only, host only and
> OTG roles.
> 
>  >The board data hardcodes the mode to OTG or
> Peripheral.

Or host.  That's correct... because the *BOARD* is hard-wired
 that way.  Based on what connector is present, the driver uses
that board-specific data to ensure it's exposing the right
capabilities.  (Example, a host-only board won't expose either
OTG or peripheral capabilities, etc.)


>  >This fix will allow to use Peripheral, Host and OTG
>  >roles independently.

The driver has had that capability almost forever...

If you're saying that somebody broke the mode setup logic in the
driver, which originally tested the board capabilities before it
enabled the relevant driver capabilities, then the solution is to
un-break that code ... not to further break it.

ISTR it was pretty straightforward: boards with a host or peripheral
connector could only enter that driver mode, but boards with an OTG
connector could enter either driver mode.  This affected initialization
(one or both sides of the USB stack needed initialization before the
driver could properly start) as well as role switching at runtime
(either based on ID-sensing in the connector, or HNP).

As of something like a year and a half ago, the cable based role switching 
worked pretty well (ID sensing) and HNP behaved at least
partially, although there were some OTG conformance test failures
(timing etc, but the roles did switch correctly via HNP, verified
with both OMAP3 and DaVinci boards).

Now the PITA was that the Mentor IP doesn't clearly document some of
the key initialization or role switching steps.  I recall losing over
a month trying to make sense of what they wanted, since their state
machine didn't match the OTG spec.  I could easily believe that some
patch broke that again.  Which means someone might need to bisect
things to fix a bug they inserted...  Peripheral mode init was fragile to an 
extreme; changing the sequence was almost guaranteed to break
something.

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[Felipe Balbi]

On Thu, May 20, 2010 at 05:15:30PM +0200, ext David Brownell wrote:



 I think better remove .mode parameter from board_data if
its not needed anyways


But it *IS* needed, as I explained before...




if some board
 supports only some specific mode they can select those
specifc config only.


No they can't.  Recall that kernels are expected to run on
multiple boards.  Which means that the same kernel should be
able to work correctly on a board with a peripheral-only USB
connector ... and on a board with an OTG connector.  The way
to pass that information to the MUSB driver is through the
platform data.  On the peripheral-only config the host code
won't kick in.  On the OTG-enabled config, it will kick in
as needed (as will the peripheral mode).  And so on.

If you were to rely on Kconfig options, then at least one of
those boards would work incorrectly.  So that's not an option.


that's basically why I had made mode a parameter to usb_musb_init() so 
boards could pass they're own configuration. We still have the Kconfig 
to allow single board peripheral only defconfigs (line n900_defconfig) 
to remove unnecessary code from the kernel.


--
balbi

DefectiveByDesign.org
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[David Brownell]

>  I think better remove .mode parameter from board_data if
> its not needed anyways

But it *IS* needed, as I explained before...



> if some board
>  supports only some specific mode they can select those
> specifc config only.

No they can't.  Recall that kernels are expected to run on
multiple boards.  Which means that the same kernel should be
able to work correctly on a board with a peripheral-only USB
connector ... and on a board with an OTG connector.  The way
to pass that information to the MUSB driver is through the
platform data.  On the peripheral-only config the host code
won't kick in.  On the OTG-enabled config, it will kick in
as needed (as will the peripheral mode).  And so on.

If you were to rely on Kconfig options, then at least one of
those boards would work incorrectly.  So that's not an option.


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.

[Gupta, Ajay Kumar]

Hi,

> Subject: [PATCH] ARM: OMAP: Fix board data to support device only, host only 
> and OTG roles.

> Fix board data to support device only, host only and OTG roles.

 >The board data hardcodes the mode to OTG or Peripheral.
 >This fix will allow to use Peripheral, Host and OTG
 >roles independently.

 >Signed-off-by: Maulik Mankad 

Same set of code snippet getting repeated across the files. Isn't it good to 
have
them at same commom file usb-musb.c ?

 I think better remove .mode parameter from board_data if its not needed 
anyways if some board
 supports only some specific mode they can select those specifc config only.

-Ajay--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html