Re: [PATCH 2/2] Add basic devices support for Nook Color

[Mike Rapoport <mike@compulab.co.il>]

(adding Felipe to cc)

Felipe, can you comment on the mUSB part, please?

On 04/27/11 19:12, Oleg Drokin wrote:
> Hello!
> 
>  Thanks for the feedback.
> On Apr 27, 2011, at 4:49 AM, Mike Rapoport wrote:
>>> MMC/SD interface, i2c, usb.
>> You should have added these at the first patch instead of Android devices.
> 
> So, basically you want all board file changes to come as a single patch, right?

I would split you series into three patches:
1) TWL updates
2) addition of board-encore
3) defconfig updates

The patch that adds board-encore would include MMC, i2c and mUSB.

>>> ---
>>> arch/arm/configs/encore-nookcolor-defconfig | 2015 +++++++++++++++++++++++++++
>> The defconfig should be a separate patch. You should rather add encore board
>> options to omap2plus_defconfig.
> 
> Hm, ok.
> 
>>> +static struct i2c_board_info __initdata encore_i2c_bus2_info[] = {
>>> +};
>>> +
>>> +
>>> +static struct omap_musb_board_data musb_board_data = {
>>> +	.interface_type		= MUSB_INTERFACE_ULPI,
>>> +#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
>> This kind of ifdefery is handled inside the musb driver. I'd set the mode to
>> MUSB_OTG unless you want to explicitly limit it to HOST or PERIPHERAL
> 
> Actually it's not.
> If I set MUSB_OTG here and then I choose PERIPHERAL mode in the kernel config,
> the musb transceiver code will complain about board file and kernel config mismatch.
> The Nook Color is advertised as peripheral device, but OTG must be working too
> (not totally working at this point) I think there is value to be able to configure it
> in two different modes.

Frankly, I haven't tried choosing different modes in the kernel config and in
the board data. Still, I believe that board data should define desired operation
mode and the driver should do the best effort to enable the controller in the
desired mode.

> Bye,
>     Oleg


-- 
Sincerely yours,
Mike.