[PATCH] Voipac PXA270: UDC and UHC support

[PATCH] Voipac PXA270: UDC and UHC support

[Marek Vasut]

Hi,
this patch adds UDC and UHC support (and formating fix, I hope you dont mind 
Eric).

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Voipac-PXA270-UDC-and-UHC-support.patch
Type: text/x-patch
Size: 3741 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100310/c93850bd/attachment.bin>

[PATCH] Voipac PXA270: UDC and UHC support

[Marek Vasut]

Dne St 10. b?ezna 2010 05:43:00 Marek Vasut napsal(a):
> Hi,
> this patch adds UDC and UHC support (and formating fix, I hope you dont
>  mind Eric).
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> 
revised patch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Voipac-PXA270-UDC-and-UHC-support.patch
Type: text/x-patch
Size: 3192 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100310/8990412b/attachment.bin>

[PATCH] Voipac PXA270: UDC and UHC support

[Robert Jarzmik]

Marek Vasut <marek.vasut@gmail.com> writes:

> Dne St 10. b?ezna 2010 05:43:00 Marek Vasut napsal(a):
>> Hi,
>> this patch adds UDC and UHC support (and formating fix, I hope you dont
>>  mind Eric).
>> 
>> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
>> 
> revised patch

snip
> +/******************************************************************************
> + * USB Gadget
> + ******************************************************************************/
> +#if defined(CONFIG_USB_GADGET_PXA27X)||defined(CONFIG_USB_GADGET_PXA27X_MODULE)
> +static struct pxa2xx_udc_mach_info vpac270_udc_info __initdata = {
> +	.gpio_vbus		= GPIO41_VPAC270_UDC_DETECT,
> +};
That won't work I think. gpio_vbus is not dealt with by pxa27x_udc.
The way to go would rather be something like :

#include <linux/usb/gpio_vbus.h>
static struct pxa2xx_udc_mach_info vpac270_udc_info __initdata = {
        .gpio_pullup      = <to be filled in>,
};

struct gpio_vbus_mach_info gpio_vbus_data = {
        .gpio_vbus = GPIO41_VPAC270_UDC_DETECT,
        .gpio_vbus_inverted = 0,
        .gpio_pullup = -1,
};

Add add a "gpio_vbus" device, with platform_data set to gpio_vbus_data.

As a matter of fact, there are not a lot of cases where an UDC can work without :
 - a GPIO for pullup
 - or the udc_command() callback implemented
The only working case I see is a hardwired D+ pin to the Vdd, but that seems an
exterme in board design I've never met before. This lets me think you should
find and the add the correct GPIO for pullup.

Cheers.

--
Robert

[PATCH] Voipac PXA270: UDC and UHC support

[Marek Vasut]

Dne P? 12. b?ezna 2010 18:49:54 Robert Jarzmik napsal(a):
> Marek Vasut <marek.vasut@gmail.com> writes:
> > Dne St 10. b?ezna 2010 05:43:00 Marek Vasut napsal(a):
> >> Hi,
> >> this patch adds UDC and UHC support (and formating fix, I hope you dont
> >>  mind Eric).
> >>
> >> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> >
> > revised patch
> 
> snip
> 
> > +/***********************************************************************
> >******* + * USB Gadget
> > +
> > *************************************************************************
> >*****/ +#if
> > defined(CONFIG_USB_GADGET_PXA27X)||defined(CONFIG_USB_GADGET_PXA27X_MODUL
> >E) +static struct pxa2xx_udc_mach_info vpac270_udc_info __initdata = {
> > +	.gpio_vbus		= GPIO41_VPAC270_UDC_DETECT,
> > +};
> 
> That won't work I think. gpio_vbus is not dealt with by pxa27x_udc.
> The way to go would rather be something like :
> 
> #include <linux/usb/gpio_vbus.h>
> static struct pxa2xx_udc_mach_info vpac270_udc_info __initdata = {
>         .gpio_pullup      = <to be filled in>,
> };
> 
> struct gpio_vbus_mach_info gpio_vbus_data = {
>         .gpio_vbus = GPIO41_VPAC270_UDC_DETECT,
>         .gpio_vbus_inverted = 0,
>         .gpio_pullup = -1,
> };
> 
> Add add a "gpio_vbus" device, with platform_data set to gpio_vbus_data.
> 
> As a matter of fact, there are not a lot of cases where an UDC can work
>  without : - a GPIO for pullup
>  - or the udc_command() callback implemented
> The only working case I see is a hardwired D+ pin to the Vdd, but that
>  seems an exterme in board design I've never met before. This lets me think
>  you should find and the add the correct GPIO for pullup.

Could you tell me which one pullup gpio is ? Here's the schematic:
http://www.voipac.com/downloads/pxa/270/baseboard/doc/bb270v22sch.pdf

Thanks
> 
> Cheers.
> 
> --
> Robert
> 

[PATCH] Voipac PXA270: UDC and UHC support

[Marek Vasut]

Dne P? 12. b?ezna 2010 18:49:54 Robert Jarzmik napsal(a):
> Marek Vasut <marek.vasut@gmail.com> writes:
> > Dne St 10. b?ezna 2010 05:43:00 Marek Vasut napsal(a):
> >> Hi,
> >> this patch adds UDC and UHC support (and formating fix, I hope you dont
> >>  mind Eric).
> >>
> >> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> >
> > revised patch
> 
> snip
> 
> > +/***********************************************************************
> >******* + * USB Gadget
> > +
> > *************************************************************************
> >*****/ +#if
> > defined(CONFIG_USB_GADGET_PXA27X)||defined(CONFIG_USB_GADGET_PXA27X_MODUL
> >E) +static struct pxa2xx_udc_mach_info vpac270_udc_info __initdata = {
> > +	.gpio_vbus		= GPIO41_VPAC270_UDC_DETECT,
> > +};
> 
> That won't work I think. gpio_vbus is not dealt with by pxa27x_udc.
> The way to go would rather be something like :
> 
> #include <linux/usb/gpio_vbus.h>
> static struct pxa2xx_udc_mach_info vpac270_udc_info __initdata = {
>         .gpio_pullup      = <to be filled in>,
> };
> 
> struct gpio_vbus_mach_info gpio_vbus_data = {
>         .gpio_vbus = GPIO41_VPAC270_UDC_DETECT,
>         .gpio_vbus_inverted = 0,
>         .gpio_pullup = -1,
> };
> 
> Add add a "gpio_vbus" device, with platform_data set to gpio_vbus_data.
> 
> As a matter of fact, there are not a lot of cases where an UDC can work
>  without : - a GPIO for pullup
>  - or the udc_command() callback implemented
> The only working case I see is a hardwired D+ pin to the Vdd, but that
>  seems an exterme in board design I've never met before. This lets me think
>  you should find and the add the correct GPIO for pullup.
> 
> Cheers.
> 
> --
> Robert
> 
Updated patch attached.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Voipac-PXA270-UDC-and-UHC-support.patch
Type: text/x-patch
Size: 4188 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100313/3ecfd342/attachment.bin>

[PATCH] Voipac PXA270: UDC and UHC support

[Robert Jarzmik]

Marek Vasut <marek.vasut@gmail.com> writes:

> Dne P? 12. b?ezna 2010 18:49:54 Robert Jarzmik napsal(a):
>> Marek Vasut <marek.vasut@gmail.com> writes:
>> > Dne St 10. b?ezna 2010 05:43:00 Marek Vasut napsal(a):
>> >> Hi,
>> >> this patch adds UDC and UHC support (and formating fix, I hope you dont
>> >>  mind Eric).
>> >>
>> >> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
>> >
>> > revised patch
Yes, I feel that one much better.

One minor comments remain for me.
> +/******************************************************************************
> + * USB Gadget
> + ******************************************************************************/
> +#if defined(CONFIG_USB_GADGET_PXA27X)||defined(CONFIG_USB_GADGET_PXA27X_MODULE)
> +static struct gpio_vbus_mach_info vpac270_gpio_vbus_info = {
> +	.gpio_vbus		= GPIO41_VPAC270_UDC_DETECT,
> +	.gpio_pullup		= -1,
> +};
> +
> +static struct platform_device vpac270_gpio_vbus = {
> +	.name	= "gpio-vbus",
> +	.id	= -1,
> +	.dev	= {
> +		.platform_data	= &vpac270_gpio_vbus_info,
> +	},
> +};
> +
> +static void vpac270_udc_command(int cmd) 
> +{
> +	if (cmd == PXA2XX_UDC_CMD_CONNECT)
> +		UP2OCR |= UP2OCR_HXOE | UP2OCR_DPPUE | UP2OCR_DPPUBE;
> +	else if (cmd == PXA2XX_UDC_CMD_DISCONNECT)
> +		UP2OCR &= ~(UP2OCR_HXOE | UP2OCR_DPPUE | UP2OCR_DPPUBE);

In the "DISCONNECT" case, you reset UP2OCR_HXOE, meaning the usb port isn't a
USB Device anymore (neither a USB host). If that's on purpose, then all good, in
the other case, don't reset that one.

Otherwise, you can add for the UDC part :
Reviewed-by: Robert Jarzmik <robert.jarzmik@free.fr>

-- 
Robert

[PATCH] Voipac PXA270: UDC and UHC support

[Eric Miao]

>> +static void vpac270_udc_command(int cmd)
>> +{
>> + ? ? if (cmd == PXA2XX_UDC_CMD_CONNECT)
>> + ? ? ? ? ? ? UP2OCR |= UP2OCR_HXOE | UP2OCR_DPPUE | UP2OCR_DPPUBE;
>> + ? ? else if (cmd == PXA2XX_UDC_CMD_DISCONNECT)
>> + ? ? ? ? ? ? UP2OCR &= ~(UP2OCR_HXOE | UP2OCR_DPPUE | UP2OCR_DPPUBE);
>
> In the "DISCONNECT" case, you reset UP2OCR_HXOE, meaning the usb port isn't a
> USB Device anymore (neither a USB host). If that's on purpose, then all good, in
> the other case, don't reset that one.
>

Marek??

> Otherwise, you can add for the UDC part :
> Reviewed-by: Robert Jarzmik <robert.jarzmik@free.fr>
>
> --
> Robert
>

[PATCH] Voipac PXA270: UDC and UHC support

[Marek Vasut]

Dne Po 22. b?ezna 2010 02:57:33 Eric Miao napsal(a):
> >> +static void vpac270_udc_command(int cmd)
> >> +{
> >> +     if (cmd == PXA2XX_UDC_CMD_CONNECT)
> >> +             UP2OCR |= UP2OCR_HXOE | UP2OCR_DPPUE | UP2OCR_DPPUBE;
> >> +     else if (cmd == PXA2XX_UDC_CMD_DISCONNECT)
> >> +             UP2OCR &= ~(UP2OCR_HXOE | UP2OCR_DPPUE | UP2OCR_DPPUBE);
> >
> > In the "DISCONNECT" case, you reset UP2OCR_HXOE, meaning the usb port
> > isn't a USB Device anymore (neither a USB host). If that's on purpose,
> > then all good, in the other case, don't reset that one.
> 
> Marek??

I wasn't able to get it running without reseting the HXOE (so it's there based 
on my tests with the hardware). I'll have to investigate it properly when I have 
more time.

So Eric, could you push this in? I'll send a fix if I figure out something's 
wrong here.

Thanks
> 
> > Otherwise, you can add for the UDC part :
> > Reviewed-by: Robert Jarzmik <robert.jarzmik@free.fr>
> >
> > --
> > Robert
> 

[PATCH] Voipac PXA270: UDC and UHC support

[Eric Miao]

On Mon, Mar 22, 2010 at 11:28 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Dne Po 22. b?ezna 2010 02:57:33 Eric Miao napsal(a):
>> >> +static void vpac270_udc_command(int cmd)
>> >> +{
>> >> + ? ? if (cmd == PXA2XX_UDC_CMD_CONNECT)
>> >> + ? ? ? ? ? ? UP2OCR |= UP2OCR_HXOE | UP2OCR_DPPUE | UP2OCR_DPPUBE;
>> >> + ? ? else if (cmd == PXA2XX_UDC_CMD_DISCONNECT)
>> >> + ? ? ? ? ? ? UP2OCR &= ~(UP2OCR_HXOE | UP2OCR_DPPUE | UP2OCR_DPPUBE);
>> >
>> > In the "DISCONNECT" case, you reset UP2OCR_HXOE, meaning the usb port
>> > isn't a USB Device anymore (neither a USB host). If that's on purpose,
>> > then all good, in the other case, don't reset that one.
>>
>> Marek??
>
> I wasn't able to get it running without reseting the HXOE (so it's there based
> on my tests with the hardware). I'll have to investigate it properly when I have
> more time.
>
> So Eric, could you push this in? I'll send a fix if I figure out something's
> wrong here.
>

Could you modify the patch a bit to leave a NOTE there in the
commit message so we can track? As well as Robert's reviewed-by.
Thanks.

[PATCH] Voipac PXA270: UDC and UHC support

[Marek Vasut]

Dne Po 22. b?ezna 2010 05:04:28 Eric Miao napsal(a):
> On Mon, Mar 22, 2010 at 11:28 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Dne Po 22. b?ezna 2010 02:57:33 Eric Miao napsal(a):
> >> >> +static void vpac270_udc_command(int cmd)
> >> >> +{
> >> >> +     if (cmd == PXA2XX_UDC_CMD_CONNECT)
> >> >> +             UP2OCR |= UP2OCR_HXOE | UP2OCR_DPPUE | UP2OCR_DPPUBE;
> >> >> +     else if (cmd == PXA2XX_UDC_CMD_DISCONNECT)
> >> >> +             UP2OCR &= ~(UP2OCR_HXOE | UP2OCR_DPPUE |
> >> >> UP2OCR_DPPUBE);
> >> >
> >> > In the "DISCONNECT" case, you reset UP2OCR_HXOE, meaning the usb port
> >> > isn't a USB Device anymore (neither a USB host). If that's on purpose,
> >> > then all good, in the other case, don't reset that one.
> >>
> >> Marek??
> >
> > I wasn't able to get it running without reseting the HXOE (so it's there
> > based on my tests with the hardware). I'll have to investigate it
> > properly when I have more time.
> >
> > So Eric, could you push this in? I'll send a fix if I figure out
> > something's wrong here.
> 
> Could you modify the patch a bit to leave a NOTE there in the
> commit message so we can track? As well as Robert's reviewed-by.
> Thanks.
> 
Updated patch included
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Voipac-PXA270-UDC-and-UHC-support.patch
Type: text/x-patch
Size: 4421 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100322/f9f50e9e/attachment.bin>

[PATCH] Voipac PXA270: UDC and UHC support

[Eric Miao]

On Mon, Mar 22, 2010 at 9:06 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Dne Po 22. b?ezna 2010 05:04:28 Eric Miao napsal(a):
>> On Mon, Mar 22, 2010 at 11:28 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> > Dne Po 22. b?ezna 2010 02:57:33 Eric Miao napsal(a):
>> >> >> +static void vpac270_udc_command(int cmd)
>> >> >> +{
>> >> >> + ? ? if (cmd == PXA2XX_UDC_CMD_CONNECT)
>> >> >> + ? ? ? ? ? ? UP2OCR |= UP2OCR_HXOE | UP2OCR_DPPUE | UP2OCR_DPPUBE;
>> >> >> + ? ? else if (cmd == PXA2XX_UDC_CMD_DISCONNECT)
>> >> >> + ? ? ? ? ? ? UP2OCR &= ~(UP2OCR_HXOE | UP2OCR_DPPUE |
>> >> >> UP2OCR_DPPUBE);
>> >> >
>> >> > In the "DISCONNECT" case, you reset UP2OCR_HXOE, meaning the usb port
>> >> > isn't a USB Device anymore (neither a USB host). If that's on purpose,
>> >> > then all good, in the other case, don't reset that one.
>> >>
>> >> Marek??
>> >
>> > I wasn't able to get it running without reseting the HXOE (so it's there
>> > based on my tests with the hardware). I'll have to investigate it
>> > properly when I have more time.
>> >
>> > So Eric, could you push this in? I'll send a fix if I figure out
>> > something's wrong here.
>>
>> Could you modify the patch a bit to leave a NOTE there in the
>> commit message so we can track? As well as Robert's reviewed-by.
>> Thanks.
>>
> Updated patch included
>

Applied to 'devel'.