Re: [PATCH v2] USB: Add MSM USB Device Controller driver

[Igor Grinberg <grinberg@compulab.co.il>]

 On 11/19/10 07:50, Pavan Kondeti wrote:
> Hi Igor,
>
> On Thu, Nov 11, 2010 at 07:40:28AM +0530, Pavan Kondeti wrote:
>> Hi Igor,
>>
>> On Wed, Nov 10, 2010 at 08:47:16AM +0200, Igor Grinberg wrote:
>>>  On 11/10/10 04:19, Pavan Kondeti wrote:
>>>> On Tue, Nov 09, 2010 at 05:36:28PM +0200, Igor Grinberg wrote:
>>>>> On 11/09/10 15:52, Matthieu CASTET wrote:
>>>>>> +static void ulpi_init(struct msm_otg *motg)
>>>>>> +{
>>>>>> +       struct msm_otg_platform_data *pdata = motg->pdata;
>>>>>> +       int *seq = pdata->phy_init_seq;
>>>>>> +
>>>>>> +       if (!seq)
>>>>>> +               return;
>>>>>> +
>>>>>> +       while (seq[0] >= 0) {
>>>>>> +               dev_vdbg(motg->otg.dev, "ulpi: write 0x%02x to 0x%02x\n",
>>>>>> +                               seq[0], seq[1]);
>>>>>> +               ulpi_write(&motg->otg, seq[0], seq[1]);
>>>>>> +               seq += 2;
>>>>>> +       }
>>>>>> +}
>>>>>>
>>>>>>
>>>>>> can't you share the ulpi fonctions from udc and otg ?
>>>>> The best would be to use the usb/otg/ulpi.c driver for the ulpi access.
>>>> This driver will eventually use msm72k_otg.c transceiver.
>>> Well, I mean, in addition to defining the struct otg_io_access_ops in some place,
>>> where both device and host drivers can share the same ops code, you can also
>>> use the ulpi generic driver as the protocol driver, instead of writing private code.
>> I have not looked at generic ULPI driver. I will see if we can re-use it in
>> our msm72k_otg.c.
> The generic ULPI driver seems to be supporting initializing the PHY,
> configuring the PHY in Host mode and driving VBUS.

Currently, yes, it is correct.
The reason I haven't added some more functionality
(e.g. set_peripheral()) is because I don't have a h/w that supports it.

> The PHY we have in our MSM chips does not have an integrated charge pump. So we
> depend on external voltage regulators driver VBUS. So can not use ulpi_set_vbus
> method.

You don't have to...
ulpi driver relies on otg_transceiver, so if you don't call otg_set_vbus(),
ulpi_set_vbus() will not be called and instead you can do your own set_vbus().

> IMO, ulpi_set_host is useful for only Hosts but not OTG (i.e support both gadget
> and host). We kick OTG state machine only after both gadget and host are
> registered. So we use our own set_host method and can not use ulpi_set_host.

IMHO, ULPI driver is not meant to somehow affect the OTG state machine.
I think the initial design of the ULPI driver was a mistake.
It should not have implemented the OTG transceiver, but only the communication
with the ULPI transceiver and implementation of the ULPI standard.
It should be used only for setting up/changing modes of ULPI transceivers.

It already has the ability to detect/check reliability of view port of the ULPI transceiver,
supply the wiring information to the ULPI transceiver.
I thought also of giving it the ability to reset the ULPI transceiver, and also add
set_peripheral() routine, but the h/w I have does not support peripheral mode
and does not need to reset the transceiver.
I can work on adding those, if someone find it useful.

Bottom line, I'd expect the communication with the ULPI transceiver handled by the
ULPI driver and the rest by the OTG implementation (core or platform).

-- 
Regards,
Igor.