diff mbox

[2/5] ARM: PXA: Zipit Z2: Add USB host and device support

Message ID 1351438555-4668-2-git-send-email-anarsoul@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vasily Khoruzhick Oct. 28, 2012, 3:35 p.m. UTC
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm/mach-pxa/z2.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

Comments

Marek Vasut Oct. 28, 2012, 9:59 p.m. UTC | #1
Dear Vasily Khoruzhick,

missing commit message.

> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  arch/arm/mach-pxa/z2.c | 52
> ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52
> insertions(+)
> 
> diff --git a/arch/arm/mach-pxa/z2.c b/arch/arm/mach-pxa/z2.c
> index c97485f..ce90fa9 100644
> --- a/arch/arm/mach-pxa/z2.c
> +++ b/arch/arm/mach-pxa/z2.c
> @@ -41,6 +41,9 @@
>  #include <linux/platform_data/mmc-pxamci.h>
>  #include <linux/platform_data/keypad-pxa27x.h>
>  #include <mach/pm.h>
> +#include <mach/pxa27x-udc.h>
> +#include <mach/udc.h>
> +#include <linux/platform_data/usb-ohci-pxa27x.h>
> 
>  #include "generic.h"
>  #include "devices.h"
> @@ -680,6 +683,52 @@ static void __init z2_pmic_init(void)
>  static inline void z2_pmic_init(void) {}
>  #endif
> 
> +/*************************************************************************
> ***** + * USB Switch
> +
> **************************************************************************
> ****/ +static struct platform_device z2_usb_switch = {
> +	.name		= "z2-usb-switch",
> +	.id		= -1,
> +};
> +
> +static void __init z2_usb_switch_init(void)
> +{
> +	platform_device_register(&z2_usb_switch);
> +}
> +
> +/*************************************************************************
> ***** + * USB Gadget
> +
> **************************************************************************
> ****/ +#if defined(CONFIG_USB_GADGET_PXA27X) \
> +	|| defined(CONFIG_USB_GADGET_PXA27X_MODULE)
> +static int z2_udc_is_connected(void)
> +{
> +	return 1;
> +}
> +
> +static struct pxa2xx_udc_mach_info z2_udc_info __initdata = {
> +	.udc_is_connected	= z2_udc_is_connected,
> +	.gpio_pullup		= -1,
> +};
> +
> +static void __init z2_udc_init(void)
> +{
> +	pxa_set_udc_info(&z2_udc_info);
> +}
> +#else
> +static inline void z2_udc_init(void) {}
> +#endif

We really should work on the DT here.

> +/*************************************************************************
> ***** + * USB Host (OHCI)
> +
> **************************************************************************
> ****/ +static struct pxaohci_platform_data z2_ohci_platform_data = {
> +	.port_mode	= PMM_PERPORT_MODE,
> +	.flags		= ENABLE_PORT2 | NO_OC_PROTECTION,
> +	.power_on_delay	= 10,
> +	.power_budget	= 500,
> +};
> +
>  #ifdef CONFIG_PM
>  static void z2_power_off(void)
>  {
> @@ -705,10 +754,12 @@ static void __init z2_init(void)
>  	pxa_set_ffuart_info(NULL);
>  	pxa_set_btuart_info(NULL);
>  	pxa_set_stuart_info(NULL);
> +	pxa_set_ohci_info(&z2_ohci_platform_data);
> 
>  	z2_lcd_init();
>  	z2_mmc_init();
>  	z2_mkp_init();
> +	z2_udc_init();

This patch adds _host_ ? So why do you have udc in here ?

Besides, pxa_set_ohci_info() should also be wrapped in some z2_uhc_init()

>  	z2_i2c_init();
>  	z2_spi_init();
>  	z2_nor_init();
> @@ -716,6 +767,7 @@ static void __init z2_init(void)
>  	z2_leds_init();
>  	z2_keys_init();
>  	z2_pmic_init();
> +	z2_usb_switch_init();
> 
>  	pm_power_off = z2_power_off;
>  }

Best regards,
Marek Vasut
Vasily Khoruzhick Oct. 28, 2012, 10:38 p.m. UTC | #2
On Mon, Oct 29, 2012 at 12:59 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Vasily Khoruzhick,
>
> missing commit message.

OK

>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>> ---
>>  arch/arm/mach-pxa/z2.c | 52
>> ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52
>> insertions(+)
>>
>> diff --git a/arch/arm/mach-pxa/z2.c b/arch/arm/mach-pxa/z2.c
>> index c97485f..ce90fa9 100644
>> --- a/arch/arm/mach-pxa/z2.c
>> +++ b/arch/arm/mach-pxa/z2.c
>> @@ -41,6 +41,9 @@
>>  #include <linux/platform_data/mmc-pxamci.h>
>>  #include <linux/platform_data/keypad-pxa27x.h>
>>  #include <mach/pm.h>
>> +#include <mach/pxa27x-udc.h>
>> +#include <mach/udc.h>
>> +#include <linux/platform_data/usb-ohci-pxa27x.h>
>>
>>  #include "generic.h"
>>  #include "devices.h"
>> @@ -680,6 +683,52 @@ static void __init z2_pmic_init(void)
>>  static inline void z2_pmic_init(void) {}
>>  #endif
>>
>> +/*************************************************************************
>> ***** + * USB Switch
>> +
>> **************************************************************************
>> ****/ +static struct platform_device z2_usb_switch = {
>> +     .name           = "z2-usb-switch",
>> +     .id             = -1,
>> +};
>> +
>> +static void __init z2_usb_switch_init(void)
>> +{
>> +     platform_device_register(&z2_usb_switch);
>> +}
>> +
>> +/*************************************************************************
>> ***** + * USB Gadget
>> +
>> **************************************************************************
>> ****/ +#if defined(CONFIG_USB_GADGET_PXA27X) \
>> +     || defined(CONFIG_USB_GADGET_PXA27X_MODULE)
>> +static int z2_udc_is_connected(void)
>> +{
>> +     return 1;
>> +}
>> +
>> +static struct pxa2xx_udc_mach_info z2_udc_info __initdata = {
>> +     .udc_is_connected       = z2_udc_is_connected,
>> +     .gpio_pullup            = -1,
>> +};
>> +
>> +static void __init z2_udc_init(void)
>> +{
>> +     pxa_set_udc_info(&z2_udc_info);
>> +}
>> +#else
>> +static inline void z2_udc_init(void) {}
>> +#endif
>
> We really should work on the DT here.

Any volunteers? :) It requires huge amount of work, and right now my goal
is to get every piece of Z2 HW working with vanilla kernel, before those changes
are lost in my github repo. I see no advantages in moving Z2 to DT except
self-education. I'm OK with DT, but it needs some time. Let's get non-DT
version working properly first.

>> +/*************************************************************************
>> ***** + * USB Host (OHCI)
>> +
>> **************************************************************************
>> ****/ +static struct pxaohci_platform_data z2_ohci_platform_data = {
>> +     .port_mode      = PMM_PERPORT_MODE,
>> +     .flags          = ENABLE_PORT2 | NO_OC_PROTECTION,
>> +     .power_on_delay = 10,
>> +     .power_budget   = 500,
>> +};
>> +
>>  #ifdef CONFIG_PM
>>  static void z2_power_off(void)
>>  {
>> @@ -705,10 +754,12 @@ static void __init z2_init(void)
>>       pxa_set_ffuart_info(NULL);
>>       pxa_set_btuart_info(NULL);
>>       pxa_set_stuart_info(NULL);
>> +     pxa_set_ohci_info(&z2_ohci_platform_data);
>>
>>       z2_lcd_init();
>>       z2_mmc_init();
>>       z2_mkp_init();
>> +     z2_udc_init();
>
> This patch adds _host_ ? So why do you have udc in here ?

Both, host and device.

> Besides, pxa_set_ohci_info() should also be wrapped in some z2_uhc_init()

OK

>>       z2_i2c_init();
>>       z2_spi_init();
>>       z2_nor_init();
>> @@ -716,6 +767,7 @@ static void __init z2_init(void)
>>       z2_leds_init();
>>       z2_keys_init();
>>       z2_pmic_init();
>> +     z2_usb_switch_init();
>>
>>       pm_power_off = z2_power_off;
>>  }
>
> Best regards,
> Marek Vasut

Regards
Vasily
Marek Vasut Oct. 28, 2012, 10:43 p.m. UTC | #3
Dear Vasily Khoruzhick,

[...]

> > We really should work on the DT here.
> 
> Any volunteers? :)

Daniel, did you get anywhere with migrating PXA to DT please?

> It requires huge amount of work, and right now my goal
> is to get every piece of Z2 HW working with vanilla kernel, before those
> changes are lost in my github repo.

Good, but next time please focus on DT instead. It will need a massive amount of 
work, indeed, but then it will also weed out some legacy code.

> I see no advantages in moving Z2 to DT
> except self-education.

One less platform file after the platform is fully DT, that's always good. 
Besides, the legacy binding code is really annoying.

> I'm OK with DT, but it needs some time. Let's get
> non-DT version working properly first.

While I'm not saying I'm NAKing these, such attitude usually leads to people 
abandoning the DT idea and running away right after the patches are applied. 
Just my $0.02.

[...]

> >> +     z2_udc_init();
> > 
> > This patch adds _host_ ? So why do you have udc in here ?
> 
> Both, host and device.

UDC == usb device controller.

[...]

Best regards,
Marek Vasut
Daniel Mack Oct. 28, 2012, 10:58 p.m. UTC | #4
On Oct 28, 2012 11:44 PM, "Vasily Khoruzhick" <anarsoul@gmail.com> wrote:
>
> On Mon, Oct 29, 2012 at 12:59 AM, Marek Vasut <marex@denx.de> wrote:
> > Dear Vasily Khoruzhick,
> >
> > missing commit message.
>
> OK
>
> >> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> >> ---
> >>  arch/arm/mach-pxa/z2.c | 52
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52
> >> insertions(+)
> >>
> >> diff --git a/arch/arm/mach-pxa/z2.c b/arch/arm/mach-pxa/z2.c
> >> index c97485f..ce90fa9 100644
> >> --- a/arch/arm/mach-pxa/z2.c
> >> +++ b/arch/arm/mach-pxa/z2.c
> >> @@ -41,6 +41,9 @@
> >>  #include <linux/platform_data/mmc-pxamci.h>
> >>  #include <linux/platform_data/keypad-pxa27x.h>
> >>  #include <mach/pm.h>
> >> +#include <mach/pxa27x-udc.h>
> >> +#include <mach/udc.h>
> >> +#include <linux/platform_data/usb-ohci-pxa27x.h>
> >>
> >>  #include "generic.h"
> >>  #include "devices.h"
> >> @@ -680,6 +683,52 @@ static void __init z2_pmic_init(void)
> >>  static inline void z2_pmic_init(void) {}
> >>  #endif
> >>
> >>
+/*************************************************************************
> >> ***** + * USB Switch
> >> +
> >>
**************************************************************************
> >> ****/ +static struct platform_device z2_usb_switch = {
> >> +     .name           = "z2-usb-switch",
> >> +     .id             = -1,
> >> +};
> >> +
> >> +static void __init z2_usb_switch_init(void)
> >> +{
> >> +     platform_device_register(&z2_usb_switch);
> >> +}
> >> +
> >>
+/*************************************************************************
> >> ***** + * USB Gadget
> >> +
> >>
**************************************************************************
> >> ****/ +#if defined(CONFIG_USB_GADGET_PXA27X) \
> >> +     || defined(CONFIG_USB_GADGET_PXA27X_MODULE)
> >> +static int z2_udc_is_connected(void)
> >> +{
> >> +     return 1;
> >> +}
> >> +
> >> +static struct pxa2xx_udc_mach_info z2_udc_info __initdata = {
> >> +     .udc_is_connected       = z2_udc_is_connected,
> >> +     .gpio_pullup            = -1,
> >> +};
> >> +
> >> +static void __init z2_udc_init(void)
> >> +{
> >> +     pxa_set_udc_info(&z2_udc_info);
> >> +}
> >> +#else
> >> +static inline void z2_udc_init(void) {}
> >> +#endif
> >
> > We really should work on the DT here.
>
> Any volunteers? :) It requires huge amount of work,

Well, it's actually quite straight forward. With the changes that got
merged to 3.7, pxa3xx platforms boot, and I also ported some pxa specific
peripheral drivers that should work for both 2xx and 3xx. Adding CPU
support for 27x should also just be a matter of some extra lines.

So I would clearly say you should give the DT approach a try first and see
which bits are missing. And I would vote for not taking any new features
for the legay board support files but just bugfixes.

> and right now my goal
> is to get every piece of Z2 HW working with vanilla kernel, before those
changes
> are lost in my github repo. I see no advantages in moving Z2 to DT except
> self-education. I'm OK with DT, but it needs some time. Let's get non-DT
> version working properly first.

At least there is a reference :-)  For mainline though, things should be
done right in the first place.

Daniel

>
> >>
+/*************************************************************************
> >> ***** + * USB Host (OHCI)
> >> +
> >>
**************************************************************************
> >> ****/ +static struct pxaohci_platform_data z2_ohci_platform_data = {
> >> +     .port_mode      = PMM_PERPORT_MODE,
> >> +     .flags          = ENABLE_PORT2 | NO_OC_PROTECTION,
> >> +     .power_on_delay = 10,
> >> +     .power_budget   = 500,
> >> +};
> >> +
> >>  #ifdef CONFIG_PM
> >>  static void z2_power_off(void)
> >>  {
> >> @@ -705,10 +754,12 @@ static void __init z2_init(void)
> >>       pxa_set_ffuart_info(NULL);
> >>       pxa_set_btuart_info(NULL);
> >>       pxa_set_stuart_info(NULL);
> >> +     pxa_set_ohci_info(&z2_ohci_platform_data);
> >>
> >>       z2_lcd_init();
> >>       z2_mmc_init();
> >>       z2_mkp_init();
> >> +     z2_udc_init();
> >
> > This patch adds _host_ ? So why do you have udc in here ?
>
> Both, host and device.
>
> > Besides, pxa_set_ohci_info() should also be wrapped in some
z2_uhc_init()
>
> OK
>
> >>       z2_i2c_init();
> >>       z2_spi_init();
> >>       z2_nor_init();
> >> @@ -716,6 +767,7 @@ static void __init z2_init(void)
> >>       z2_leds_init();
> >>       z2_keys_init();
> >>       z2_pmic_init();
> >> +     z2_usb_switch_init();
> >>
> >>       pm_power_off = z2_power_off;
> >>  }
> >
> > Best regards,
> > Marek Vasut
>
> Regards
> Vasily
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Vasily Khoruzhick Oct. 29, 2012, 9:33 a.m. UTC | #5
On Mon, Oct 29, 2012 at 1:58 AM, Daniel Mack <zonque@gmail.com> wrote:
>> Any volunteers? :) It requires huge amount of work,
>
> Well, it's actually quite straight forward. With the changes that got merged
> to 3.7, pxa3xx platforms boot, and I also ported some pxa specific
> peripheral drivers that should work for both 2xx and 3xx. Adding CPU support
> for 27x should also just be a matter of some extra lines.
>
> So I would clearly say you should give the DT approach a try first and see
> which bits are missing. And I would vote for not taking any new features for
> the legacy board support files but just bugfixes.

Fair enough, I'll resend series without USB stuff (so it'll contain
only bugfixes)

Regards
Vasily
Daniel Mack Oct. 29, 2012, 9:42 a.m. UTC | #6
On 29.10.2012 10:33, Vasily Khoruzhick wrote:
> On Mon, Oct 29, 2012 at 1:58 AM, Daniel Mack <zonque@gmail.com> wrote:
>>> Any volunteers? :) It requires huge amount of work,
>>
>> Well, it's actually quite straight forward. With the changes that got merged
>> to 3.7, pxa3xx platforms boot, and I also ported some pxa specific
>> peripheral drivers that should work for both 2xx and 3xx. Adding CPU support
>> for 27x should also just be a matter of some extra lines.
>>
>> So I would clearly say you should give the DT approach a try first and see
>> which bits are missing. And I would vote for not taking any new features for
>> the legacy board support files but just bugfixes.
> 
> Fair enough, I'll resend series without USB stuff (so it'll contain
> only bugfixes)

I rather wanted to encourage you to work on the PXA2xx DT stuff :)

Please have a quick look at arch/arm/mach-pxa/pxa-dt.c - what it takes
for basic 270 support is just copying the logic that is there already
for the 3xx models.

Some parts like the uart, host-mode usb, mmc, gpio-leds, gpio-keys, i2c
etc are already finished and should work instantly. What's missing is
the spi controller and the display, but maybe you can help getting these
done?


Thanks,
Daniel
Vasily Khoruzhick Oct. 29, 2012, 10:07 a.m. UTC | #7
On Mon, Oct 29, 2012 at 12:42 PM, Daniel Mack <zonque@gmail.com> wrote:
> On 29.10.2012 10:33, Vasily Khoruzhick wrote:
>> On Mon, Oct 29, 2012 at 1:58 AM, Daniel Mack <zonque@gmail.com> wrote:
>>>> Any volunteers? :) It requires huge amount of work,
>>>
>>> Well, it's actually quite straight forward. With the changes that got merged
>>> to 3.7, pxa3xx platforms boot, and I also ported some pxa specific
>>> peripheral drivers that should work for both 2xx and 3xx. Adding CPU support
>>> for 27x should also just be a matter of some extra lines.
>>>
>>> So I would clearly say you should give the DT approach a try first and see
>>> which bits are missing. And I would vote for not taking any new features for
>>> the legacy board support files but just bugfixes.
>>
>> Fair enough, I'll resend series without USB stuff (so it'll contain
>> only bugfixes)
>
> I rather wanted to encourage you to work on the PXA2xx DT stuff :)

I'm not refusing to work on PXA2xx DT stuff, just want to get bugfixes in place,
some of them are serious enough (like fixing oops on poweroff)

> Please have a quick look at arch/arm/mach-pxa/pxa-dt.c - what it takes
> for basic 270 support is just copying the logic that is there already
> for the 3xx models.

Sure, but I need to get DT-capable bootloader working on Z2 in first
place, right?

> Some parts like the uart, host-mode usb, mmc, gpio-leds, gpio-keys, i2c
> etc are already finished and should work instantly. What's missing is
> the spi controller and the display, but maybe you can help getting these
> done?

What about libertas? I still don't understand how DT handles platform-specific
callbacks like set_power(). And what about pm_power_off hook?

Regards
Vasily
Marek Vasut Oct. 29, 2012, 10:22 a.m. UTC | #8
Dear Vasily Khoruzhick,

[...]

> > 
> > I rather wanted to encourage you to work on the PXA2xx DT stuff :)
> 
> I'm not refusing to work on PXA2xx DT stuff, just want to get bugfixes in
> place, some of them are serious enough (like fixing oops on poweroff)
> 
> > Please have a quick look at arch/arm/mach-pxa/pxa-dt.c - what it takes
> > for basic 270 support is just copying the logic that is there already
> > for the 3xx models.
> 
> Sure, but I need to get DT-capable bootloader working on Z2 in first
> place, right?

Uh, you already have that. U-Boot can boot kernel and pass DT properly, just use 
latest 2012.10 release which just got out a few weeks ago.

> > Some parts like the uart, host-mode usb, mmc, gpio-leds, gpio-keys, i2c
> > etc are already finished and should work instantly. What's missing is
> > the spi controller and the display, but maybe you can help getting these
> > done?
> 
> What about libertas? I still don't understand how DT handles
> platform-specific callbacks like set_power(). And what about pm_power_off
> hook?

Start small, platform crap and DT goo can co-exist for a while.

Best regards,
Marek Vasut
Vasily Khoruzhick Oct. 29, 2012, 10:26 a.m. UTC | #9
On Mon, Oct 29, 2012 at 1:22 PM, Marek Vasut <marex@denx.de> wrote:
>
> Uh, you already have that. U-Boot can boot kernel and pass DT properly, just use
> latest 2012.10 release which just got out a few weeks ago.

It still lacks pxa27x keypad support, same for fb rotation :) So I
need to port some older patches

>> > Some parts like the uart, host-mode usb, mmc, gpio-leds, gpio-keys, i2c
>> > etc are already finished and should work instantly. What's missing is
>> > the spi controller and the display, but maybe you can help getting these
>> > done?
>>
>> What about libertas? I still don't understand how DT handles
>> platform-specific callbacks like set_power(). And what about pm_power_off
>> hook?
>
> Start small, platform crap and DT goo can co-exist for a while.

OK

> Best regards,
> Marek Vasut

Regards
Vasily
Daniel Mack Oct. 29, 2012, 10:44 a.m. UTC | #10
On 29.10.2012 11:07, Vasily Khoruzhick wrote:
> On Mon, Oct 29, 2012 at 12:42 PM, Daniel Mack <zonque@gmail.com> wrote:
>> On 29.10.2012 10:33, Vasily Khoruzhick wrote:
>>> On Mon, Oct 29, 2012 at 1:58 AM, Daniel Mack <zonque@gmail.com> wrote:
>>>>> Any volunteers? :) It requires huge amount of work,
>>>>
>>>> Well, it's actually quite straight forward. With the changes that got merged
>>>> to 3.7, pxa3xx platforms boot, and I also ported some pxa specific
>>>> peripheral drivers that should work for both 2xx and 3xx. Adding CPU support
>>>> for 27x should also just be a matter of some extra lines.
>>>>
>>>> So I would clearly say you should give the DT approach a try first and see
>>>> which bits are missing. And I would vote for not taking any new features for
>>>> the legacy board support files but just bugfixes.
>>>
>>> Fair enough, I'll resend series without USB stuff (so it'll contain
>>> only bugfixes)
>>
>> I rather wanted to encourage you to work on the PXA2xx DT stuff :)
> 
> I'm not refusing to work on PXA2xx DT stuff, just want to get bugfixes in place,
> some of them are serious enough (like fixing oops on poweroff)
> 
>> Please have a quick look at arch/arm/mach-pxa/pxa-dt.c - what it takes
>> for basic 270 support is just copying the logic that is there already
>> for the 3xx models.
> 
> Sure, but I need to get DT-capable bootloader working on Z2 in first
> place, right?
> 
>> Some parts like the uart, host-mode usb, mmc, gpio-leds, gpio-keys, i2c
>> etc are already finished and should work instantly. What's missing is
>> the spi controller and the display, but maybe you can help getting these
>> done?
> 
> What about libertas?

I haven't seen any patches for DT bindings of this yet.

> I still don't understand how DT handles platform-specific
> callbacks like set_power(). And what about pm_power_off hook?

They require consideration.

In general, a driver should be aware of everything that affects the
peripheral, including all the voltages and clocks it needs and also all
extra pins like for example reset GPIOs etc. And all that properties are
to be exported via DT as well (things that you would have normally set
up using pdata).

If drivers lack that kind of full awareness of their environment, they
need to be augmented, because - to stick with the example of reset GPIOs
- the driver of all parts is the only piece that knows when to driver
that pin and how.

About the hack to put the z2 to deep-sleep when it should actually power
down, I don't know what to do. But I would say that it's the userspace
that should be fixed here, if possible.


Daniel
Daniel Mack Oct. 29, 2012, 10:48 a.m. UTC | #11
On 29.10.2012 11:07, Vasily Khoruzhick wrote:
> On Mon, Oct 29, 2012 at 12:42 PM, Daniel Mack <zonque@gmail.com> wrote:
>> Please have a quick look at arch/arm/mach-pxa/pxa-dt.c - what it takes
>> for basic 270 support is just copying the logic that is there already
>> for the 3xx models.
> 
> Sure, but I need to get DT-capable bootloader working on Z2 in first
> place, right?

No, you don't necessarily. You can also append a dtb blob to the kernel
image (CONFIG_ARM_APPENDED_DTB).
Vasily Khoruzhick Oct. 29, 2012, 10:52 a.m. UTC | #12
On Mon, Oct 29, 2012 at 1:44 PM, Daniel Mack <zonque@gmail.com> wrote:

> I haven't seen any patches for DT bindings of this yet.
>
>> I still don't understand how DT handles platform-specific
>> callbacks like set_power(). And what about pm_power_off hook?
>
> They require consideration.
>
> In general, a driver should be aware of everything that affects the
> peripheral, including all the voltages and clocks it needs and also all
> extra pins like for example reset GPIOs etc. And all that properties are
> to be exported via DT as well (things that you would have normally set
> up using pdata).
>
> If drivers lack that kind of full awareness of their environment, they
> need to be augmented, because - to stick with the example of reset GPIOs
> - the driver of all parts is the only piece that knows when to driver
> that pin and how.
>
> About the hack to put the z2 to deep-sleep when it should actually power
> down, I don't know what to do. But I would say that it's the userspace
> that should be fixed here, if possible.

Well, there's an issue - Z2 does not preserve memory contents in deep sleep
(but it does in sleep), so userspace can't be fixed here unfortunatelly.
There's no another possibility to turn Z2 off, and plain sleep is too
power hungry.
So the only way to keep Z2 in low-power mode is fake power off, which just puts
Z2 in deep sleep.

> Daniel

Regards
Vasily
Daniel Mack Oct. 29, 2012, 11 a.m. UTC | #13
On 29.10.2012 11:52, Vasily Khoruzhick wrote:
> On Mon, Oct 29, 2012 at 1:44 PM, Daniel Mack <zonque@gmail.com> wrote:
> 
>> I haven't seen any patches for DT bindings of this yet.
>>
>>> I still don't understand how DT handles platform-specific
>>> callbacks like set_power(). And what about pm_power_off hook?
>>
>> They require consideration.
>>
>> In general, a driver should be aware of everything that affects the
>> peripheral, including all the voltages and clocks it needs and also all
>> extra pins like for example reset GPIOs etc. And all that properties are
>> to be exported via DT as well (things that you would have normally set
>> up using pdata).
>>
>> If drivers lack that kind of full awareness of their environment, they
>> need to be augmented, because - to stick with the example of reset GPIOs
>> - the driver of all parts is the only piece that knows when to driver
>> that pin and how.
>>
>> About the hack to put the z2 to deep-sleep when it should actually power
>> down, I don't know what to do. But I would say that it's the userspace
>> that should be fixed here, if possible.
> 
> Well, there's an issue - Z2 does not preserve memory contents in deep sleep
> (but it does in sleep), so userspace can't be fixed here unfortunatelly.
> There's no another possibility to turn Z2 off, and plain sleep is too
> power hungry.
> So the only way to keep Z2 in low-power mode is fake power off, which just puts
> Z2 in deep sleep.

Why can't the userspace trigger a deep sleep then instead of powering
off? Which details do I lack?
Vasily Khoruzhick Oct. 29, 2012, 11:12 a.m. UTC | #14
On Mon, Oct 29, 2012 at 2:00 PM, Daniel Mack <zonque@gmail.com> wrote:

>> Well, there's an issue - Z2 does not preserve memory contents in deep sleep
>> (but it does in sleep), so userspace can't be fixed here unfortunatelly.
>> There's no another possibility to turn Z2 off, and plain sleep is too
>> power hungry.
>> So the only way to keep Z2 in low-power mode is fake power off, which just puts
>> Z2 in deep sleep.
>
> Why can't the userspace trigger a deep sleep then instead of powering
> off? Which details do I lack?

How? echo mem >/sys/power/state puts system into non-deep sleep. Anyway, kernel
is not ready for fake power off instead of suspend (we can't resume
from deep sleep,
memory content is not preserved), so there can be some data loss.
Adding some sysfs file to control sleep type does not look like a good
idea to me.

Btw, how other DT-capable boards handle power off?

Regards
Vasily
Daniel Mack Oct. 29, 2012, 11:14 a.m. UTC | #15
On 29.10.2012 12:12, Vasily Khoruzhick wrote:
> On Mon, Oct 29, 2012 at 2:00 PM, Daniel Mack <zonque@gmail.com> wrote:
> 
>>> Well, there's an issue - Z2 does not preserve memory contents in deep sleep
>>> (but it does in sleep), so userspace can't be fixed here unfortunatelly.
>>> There's no another possibility to turn Z2 off, and plain sleep is too
>>> power hungry.
>>> So the only way to keep Z2 in low-power mode is fake power off, which just puts
>>> Z2 in deep sleep.
>>
>> Why can't the userspace trigger a deep sleep then instead of powering
>> off? Which details do I lack?
> 
> How? echo mem >/sys/power/state puts system into non-deep sleep. Anyway, kernel
> is not ready for fake power off instead of suspend (we can't resume
> from deep sleep,
> memory content is not preserved), so there can be some data loss.
> Adding some sysfs file to control sleep type does not look like a good
> idea to me.
> 
> Btw, how other DT-capable boards handle power off?

No idea. I never actually used that kind of power state.
Vasily Khoruzhick Oct. 30, 2012, 8:01 p.m. UTC | #16
On Mon, Oct 29, 2012 at 2:14 PM, Daniel Mack <zonque@gmail.com> wrote:
> On 29.10.2012 12:12, Vasily Khoruzhick wrote:
>> On Mon, Oct 29, 2012 at 2:00 PM, Daniel Mack <zonque@gmail.com> wrote:
>>
>>>> Well, there's an issue - Z2 does not preserve memory contents in deep sleep
>>>> (but it does in sleep), so userspace can't be fixed here unfortunatelly.
>>>> There's no another possibility to turn Z2 off, and plain sleep is too
>>>> power hungry.
>>>> So the only way to keep Z2 in low-power mode is fake power off, which just puts
>>>> Z2 in deep sleep.
>>>
>>> Why can't the userspace trigger a deep sleep then instead of powering
>>> off? Which details do I lack?
>>
>> How? echo mem >/sys/power/state puts system into non-deep sleep. Anyway, kernel
>> is not ready for fake power off instead of suspend (we can't resume
>> from deep sleep,
>> memory content is not preserved), so there can be some data loss.
>> Adding some sysfs file to control sleep type does not look like a good
>> idea to me.
>>
>> Btw, how other DT-capable boards handle power off?
>
> No idea. I never actually used that kind of power state.

Hi Daniel,

One more question: what should I use instead of pxa2xx_mfp_config in
DT board to configure pin modes?

Regards
Vasily
Daniel Mack Oct. 30, 2012, 8:05 p.m. UTC | #17
On 30.10.2012 21:01, Vasily Khoruzhick wrote:
> On Mon, Oct 29, 2012 at 2:14 PM, Daniel Mack <zonque@gmail.com> wrote:
>> On 29.10.2012 12:12, Vasily Khoruzhick wrote:
>>> On Mon, Oct 29, 2012 at 2:00 PM, Daniel Mack <zonque@gmail.com> wrote:
>>>
>>>>> Well, there's an issue - Z2 does not preserve memory contents in deep sleep
>>>>> (but it does in sleep), so userspace can't be fixed here unfortunatelly.
>>>>> There's no another possibility to turn Z2 off, and plain sleep is too
>>>>> power hungry.
>>>>> So the only way to keep Z2 in low-power mode is fake power off, which just puts
>>>>> Z2 in deep sleep.
>>>>
>>>> Why can't the userspace trigger a deep sleep then instead of powering
>>>> off? Which details do I lack?
>>>
>>> How? echo mem >/sys/power/state puts system into non-deep sleep. Anyway, kernel
>>> is not ready for fake power off instead of suspend (we can't resume
>>> from deep sleep,
>>> memory content is not preserved), so there can be some data loss.
>>> Adding some sysfs file to control sleep type does not look like a good
>>> idea to me.
>>>
>>> Btw, how other DT-capable boards handle power off?
>>
>> No idea. I never actually used that kind of power state.
> 
> Hi Daniel,
> 
> One more question: what should I use instead of pxa2xx_mfp_config in
> DT board to configure pin modes?

Haojian was working on the PXA pinctrl drivers, but I don't know how far
that is yet.

If that's not yet there, have a look at the pinctrl-single driver. It's
admittedly not as nice to use as the constants in the board file as DT
lacks defines for numerical constants, but's at least a workaround.

On a more general note, it's arguable whether this kind of setup should
be done in the bootloader after all.


Daniel
Vasily Khoruzhick Oct. 30, 2012, 9:20 p.m. UTC | #18
On Tue, Oct 30, 2012 at 11:05 PM, Daniel Mack <zonque@gmail.com> wrote:
>> One more question: what should I use instead of pxa2xx_mfp_config in
>> DT board to configure pin modes?
>
> Haojian was working on the PXA pinctrl drivers, but I don't know how far
> that is yet.
>
> If that's not yet there, have a look at the pinctrl-single driver. It's
> admittedly not as nice to use as the constants in the board file as DT
> lacks defines for numerical constants, but's at least a workaround.
>
> On a more general note, it's arguable whether this kind of setup should
> be done in the bootloader after all.
>
>
> Daniel

OK, thanks for suggestion!

Regards
Vasily
Haojian Zhuang Oct. 31, 2012, 2 p.m. UTC | #19
On Wed, Oct 31, 2012 at 4:05 AM, Daniel Mack <zonque@gmail.com> wrote:

> On 30.10.2012 21:01, Vasily Khoruzhick wrote:
> > On Mon, Oct 29, 2012 at 2:14 PM, Daniel Mack <zonque@gmail.com> wrote:
> >> On 29.10.2012 12:12, Vasily Khoruzhick wrote:
> >>> On Mon, Oct 29, 2012 at 2:00 PM, Daniel Mack <zonque@gmail.com> wrote:
> >>>
> >>>>> Well, there's an issue - Z2 does not preserve memory contents in
> deep sleep
> >>>>> (but it does in sleep), so userspace can't be fixed here
> unfortunatelly.
> >>>>> There's no another possibility to turn Z2 off, and plain sleep is too
> >>>>> power hungry.
> >>>>> So the only way to keep Z2 in low-power mode is fake power off,
> which just puts
> >>>>> Z2 in deep sleep.
> >>>>
> >>>> Why can't the userspace trigger a deep sleep then instead of powering
> >>>> off? Which details do I lack?
> >>>
> >>> How? echo mem >/sys/power/state puts system into non-deep sleep.
> Anyway, kernel
> >>> is not ready for fake power off instead of suspend (we can't resume
> >>> from deep sleep,
> >>> memory content is not preserved), so there can be some data loss.
> >>> Adding some sysfs file to control sleep type does not look like a good
> >>> idea to me.
> >>>
> >>> Btw, how other DT-capable boards handle power off?
> >>
> >> No idea. I never actually used that kind of power state.
> >
> > Hi Daniel,
> >
> > One more question: what should I use instead of pxa2xx_mfp_config in
> > DT board to configure pin modes?
>
> Haojian was working on the PXA pinctrl drivers, but I don't know how far
> that is yet.
>
> If that's not yet there, have a look at the pinctrl-single driver. It's
> admittedly not as nice to use as the constants in the board file as DT
> lacks defines for numerical constants, but's at least a workaround.
>
> On a more general note, it's arguable whether this kind of setup should
> be done in the bootloader after all.
>
>
> Daniel
>
>
Using pinctrl-single driver is preferred. I'm converting mmp pinctrl driver
into pinctrl-single
driver. I'll submit a new round in these days.

Thanks
Haojian
Haojian Zhuang Oct. 31, 2012, 3:31 p.m. UTC | #20
On Wed, Oct 31, 2012 at 4:05 AM, Daniel Mack <zonque@gmail.com> wrote:
>
> On 30.10.2012 21:01, Vasily Khoruzhick wrote:
> > On Mon, Oct 29, 2012 at 2:14 PM, Daniel Mack <zonque@gmail.com> wrote:
> >> On 29.10.2012 12:12, Vasily Khoruzhick wrote:
> >>> On Mon, Oct 29, 2012 at 2:00 PM, Daniel Mack <zonque@gmail.com> wrote:
> >>>
> >>>>> Well, there's an issue - Z2 does not preserve memory contents in deep sleep
> >>>>> (but it does in sleep), so userspace can't be fixed here unfortunatelly.
> >>>>> There's no another possibility to turn Z2 off, and plain sleep is too
> >>>>> power hungry.
> >>>>> So the only way to keep Z2 in low-power mode is fake power off, which just puts
> >>>>> Z2 in deep sleep.
> >>>>
> >>>> Why can't the userspace trigger a deep sleep then instead of powering
> >>>> off? Which details do I lack?
> >>>
> >>> How? echo mem >/sys/power/state puts system into non-deep sleep. Anyway, kernel
> >>> is not ready for fake power off instead of suspend (we can't resume
> >>> from deep sleep,
> >>> memory content is not preserved), so there can be some data loss.
> >>> Adding some sysfs file to control sleep type does not look like a good
> >>> idea to me.
> >>>
> >>> Btw, how other DT-capable boards handle power off?
> >>
> >> No idea. I never actually used that kind of power state.
> >
> > Hi Daniel,
> >
> > One more question: what should I use instead of pxa2xx_mfp_config in
> > DT board to configure pin modes?
>
> Haojian was working on the PXA pinctrl drivers, but I don't know how far
> that is yet.
>
> If that's not yet there, have a look at the pinctrl-single driver. It's
> admittedly not as nice to use as the constants in the board file as DT
> lacks defines for numerical constants, but's at least a workaround.
>
> On a more general note, it's arguable whether this kind of setup should
> be done in the bootloader after all.
>
>
> Daniel
>

I prefer to use pinctrl-single driver instead. I'm updating mmp
pinctrol driver to
pinctrl-single driver framework. You can find them in mailist. I'm submitting
a third round in these days.

Regards
Haojian
Vasily Khoruzhick Nov. 2, 2012, 8:52 p.m. UTC | #21
On Wed, Oct 31, 2012 at 6:31 PM, Haojian Zhuang
<haojian.zhuang@gmail.com> wrote:

Hi Haojian Zhuang,

> I prefer to use pinctrl-single driver instead. I'm updating mmp
> pinctrol driver to
> pinctrl-single driver framework. You can find them in mailist. I'm submitting
> a third round in these days.

Do you have public git repo with those changes somewhere?

Regards
Vasily
Vasily Khoruzhick Nov. 2, 2012, 9:29 p.m. UTC | #22
On Wed, Oct 31, 2012 at 6:31 PM, Haojian Zhuang
<haojian.zhuang@gmail.com> wrote:
> On Wed, Oct 31, 2012 at 4:05 AM, Daniel Mack <zonque@gmail.com> wrote:
>>
>> On 30.10.2012 21:01, Vasily Khoruzhick wrote:
>> > On Mon, Oct 29, 2012 at 2:14 PM, Daniel Mack <zonque@gmail.com> wrote:
>> >> On 29.10.2012 12:12, Vasily Khoruzhick wrote:
>> >>> On Mon, Oct 29, 2012 at 2:00 PM, Daniel Mack <zonque@gmail.com> wrote:
>> >>>
>> >>>>> Well, there's an issue - Z2 does not preserve memory contents in deep sleep
>> >>>>> (but it does in sleep), so userspace can't be fixed here unfortunatelly.
>> >>>>> There's no another possibility to turn Z2 off, and plain sleep is too
>> >>>>> power hungry.
>> >>>>> So the only way to keep Z2 in low-power mode is fake power off, which just puts
>> >>>>> Z2 in deep sleep.
>> >>>>
>> >>>> Why can't the userspace trigger a deep sleep then instead of powering
>> >>>> off? Which details do I lack?
>> >>>
>> >>> How? echo mem >/sys/power/state puts system into non-deep sleep. Anyway, kernel
>> >>> is not ready for fake power off instead of suspend (we can't resume
>> >>> from deep sleep,
>> >>> memory content is not preserved), so there can be some data loss.
>> >>> Adding some sysfs file to control sleep type does not look like a good
>> >>> idea to me.
>> >>>
>> >>> Btw, how other DT-capable boards handle power off?
>> >>
>> >> No idea. I never actually used that kind of power state.
>> >
>> > Hi Daniel,
>> >
>> > One more question: what should I use instead of pxa2xx_mfp_config in
>> > DT board to configure pin modes?
>>
>> Haojian was working on the PXA pinctrl drivers, but I don't know how far
>> that is yet.
>>
>> If that's not yet there, have a look at the pinctrl-single driver. It's
>> admittedly not as nice to use as the constants in the board file as DT
>> lacks defines for numerical constants, but's at least a workaround.
>>
>> On a more general note, it's arguable whether this kind of setup should
>> be done in the bootloader after all.
>>
>>
>> Daniel
>>
>
> I prefer to use pinctrl-single driver instead. I'm updating mmp
> pinctrol driver to
> pinctrl-single driver framework. You can find them in mailist. I'm submitting
> a third round in these days.
>
> Regards
> Haojian

OK, one more question:

On PXA2xx GPIO and PINMUX are not separate blocks. Each pin can be
either input or output and have 4 modes (gpio + 3 alternate functions)
== 8 states , so we have: gpio in, gpio out, 3 alt in, 3 alt out.
To control pin mode one should write to GPDR (direction register) and
GAFR (alternate function register). pxa-gpio driver controls both of
those registers, so does pxa2xx-mfp.

I'm not sure what to do in this case. Should I move code controlling
GPDR and GAFR into some pinctrl-pxa2xx driver, and modify pxa-gpio
driver to use it?

Regards
Vasily
Vasily Khoruzhick Nov. 5, 2012, 5:31 p.m. UTC | #23
On Sat, Nov 3, 2012 at 12:29 AM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:

>>
>> I prefer to use pinctrl-single driver instead. I'm updating mmp
>> pinctrol driver to
>> pinctrl-single driver framework. You can find them in mailist. I'm submitting
>> a third round in these days.
>>
>> Regards
>> Haojian
>
> OK, one more question:
>
> On PXA2xx GPIO and PINMUX are not separate blocks. Each pin can be
> either input or output and have 4 modes (gpio + 3 alternate functions)
> == 8 states , so we have: gpio in, gpio out, 3 alt in, 3 alt out.
> To control pin mode one should write to GPDR (direction register) and
> GAFR (alternate function register). pxa-gpio driver controls both of
> those registers, so does pxa2xx-mfp.
>
> I'm not sure what to do in this case. Should I move code controlling
> GPDR and GAFR into some pinctrl-pxa2xx driver, and modify pxa-gpio
> driver to use it?
>
> Regards
> Vasily

Daniel, Haojian, any suggestion?

I'm stuck at the moment, and have no idea how to handle it...
pinmux/gpio are connected in hardware
and I don't see a way to handle them with 2 separate drivers (gpio and
pinmux) in software...

Regards
Vasily
Haojian Zhuang Dec. 4, 2012, 8:30 a.m. UTC | #24
On Tue, Nov 6, 2012 at 1:31 AM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> On Sat, Nov 3, 2012 at 12:29 AM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>
>>>
>>> I prefer to use pinctrl-single driver instead. I'm updating mmp
>>> pinctrol driver to
>>> pinctrl-single driver framework. You can find them in mailist. I'm submitting
>>> a third round in these days.
>>>
>>> Regards
>>> Haojian
>>
>> OK, one more question:
>>
>> On PXA2xx GPIO and PINMUX are not separate blocks. Each pin can be
>> either input or output and have 4 modes (gpio + 3 alternate functions)
>> == 8 states , so we have: gpio in, gpio out, 3 alt in, 3 alt out.
>> To control pin mode one should write to GPDR (direction register) and
>> GAFR (alternate function register). pxa-gpio driver controls both of
>> those registers, so does pxa2xx-mfp.
>>
>> I'm not sure what to do in this case. Should I move code controlling
>> GPDR and GAFR into some pinctrl-pxa2xx driver, and modify pxa-gpio
>> driver to use it?
>>
>> Regards
>> Vasily
>
> Daniel, Haojian, any suggestion?
>
> I'm stuck at the moment, and have no idea how to handle it...
> pinmux/gpio are connected in hardware
> and I don't see a way to handle them with 2 separate drivers (gpio and
> pinmux) in software...
>
> Regards
> Vasily

I checked the PXA25x spec. pinmux functions are contained in gpio registers.

It doesn't matter. You can still you pinmux register. You only need to control
GAFP in pinmux driver.
diff mbox

Patch

diff --git a/arch/arm/mach-pxa/z2.c b/arch/arm/mach-pxa/z2.c
index c97485f..ce90fa9 100644
--- a/arch/arm/mach-pxa/z2.c
+++ b/arch/arm/mach-pxa/z2.c
@@ -41,6 +41,9 @@ 
 #include <linux/platform_data/mmc-pxamci.h>
 #include <linux/platform_data/keypad-pxa27x.h>
 #include <mach/pm.h>
+#include <mach/pxa27x-udc.h>
+#include <mach/udc.h>
+#include <linux/platform_data/usb-ohci-pxa27x.h>
 
 #include "generic.h"
 #include "devices.h"
@@ -680,6 +683,52 @@  static void __init z2_pmic_init(void)
 static inline void z2_pmic_init(void) {}
 #endif
 
+/******************************************************************************
+ * USB Switch
+ ******************************************************************************/
+static struct platform_device z2_usb_switch = {
+	.name		= "z2-usb-switch",
+	.id		= -1,
+};
+
+static void __init z2_usb_switch_init(void)
+{
+	platform_device_register(&z2_usb_switch);
+}
+
+/******************************************************************************
+ * USB Gadget
+ ******************************************************************************/
+#if defined(CONFIG_USB_GADGET_PXA27X) \
+	|| defined(CONFIG_USB_GADGET_PXA27X_MODULE)
+static int z2_udc_is_connected(void)
+{
+	return 1;
+}
+
+static struct pxa2xx_udc_mach_info z2_udc_info __initdata = {
+	.udc_is_connected	= z2_udc_is_connected,
+	.gpio_pullup		= -1,
+};
+
+static void __init z2_udc_init(void)
+{
+	pxa_set_udc_info(&z2_udc_info);
+}
+#else
+static inline void z2_udc_init(void) {}
+#endif
+
+/******************************************************************************
+ * USB Host (OHCI)
+ ******************************************************************************/
+static struct pxaohci_platform_data z2_ohci_platform_data = {
+	.port_mode	= PMM_PERPORT_MODE,
+	.flags		= ENABLE_PORT2 | NO_OC_PROTECTION,
+	.power_on_delay	= 10,
+	.power_budget	= 500,
+};
+
 #ifdef CONFIG_PM
 static void z2_power_off(void)
 {
@@ -705,10 +754,12 @@  static void __init z2_init(void)
 	pxa_set_ffuart_info(NULL);
 	pxa_set_btuart_info(NULL);
 	pxa_set_stuart_info(NULL);
+	pxa_set_ohci_info(&z2_ohci_platform_data);
 
 	z2_lcd_init();
 	z2_mmc_init();
 	z2_mkp_init();
+	z2_udc_init();
 	z2_i2c_init();
 	z2_spi_init();
 	z2_nor_init();
@@ -716,6 +767,7 @@  static void __init z2_init(void)
 	z2_leds_init();
 	z2_keys_init();
 	z2_pmic_init();
+	z2_usb_switch_init();
 
 	pm_power_off = z2_power_off;
 }