[PATCH 1/5] ARM: PXA: Add z2-usb-switch driver

[PATCH 1/5] ARM: PXA: Add z2-usb-switch driver

[Vasily Khoruzhick]

This driver controls mode of USB port #2 pins - device or host.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm/mach-pxa/Kconfig  | 7 +++++++
 arch/arm/mach-pxa/Makefile | 1 +
 2 files changed, 8 insertions(+)

diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig
index 11aa739..5fffc4b 100644
--- a/arch/arm/mach-pxa/Kconfig
+++ b/arch/arm/mach-pxa/Kconfig
@@ -630,6 +630,13 @@ config MACH_ZIPIT2
 	bool "Zipit Z2 Handheld"
 	select HAVE_PWM
 	select PXA27x
+
+config Z2_USB_SWITCH
+	tristate "Control the state of USB port on Zipit Z2"
+	depends on MACH_ZIPIT2
+	help
+	  This is a simple driver that is able to control
+	  usb mode of Zipit Z2
 endif
 endmenu
 
diff --git a/arch/arm/mach-pxa/Makefile b/arch/arm/mach-pxa/Makefile
index ee88d6e..86a032f 100644
--- a/arch/arm/mach-pxa/Makefile
+++ b/arch/arm/mach-pxa/Makefile
@@ -99,3 +99,4 @@ obj-$(CONFIG_MACH_RAUMFELD_SPEAKER)	+= raumfeld.o
 obj-$(CONFIG_MACH_ZIPIT2)	+= z2.o
 
 obj-$(CONFIG_TOSA_BT)		+= tosa-bt.o
+obj-$(CONFIG_Z2_USB_SWITCH)	+= z2-usb-switch.o
-- 
1.7.12.4

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

[Vasily Khoruzhick]

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
+
+/******************************************************************************
+ * 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;
 }
-- 
1.7.12.4

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

[Marek Vasut]

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

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

[Vasily Khoruzhick]

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

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

[Marek Vasut]

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

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

[Daniel Mack]

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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121028/5f3462f4/attachment-0001.html>

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

[Vasily Khoruzhick]

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

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

[Daniel Mack]

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

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

[Vasily Khoruzhick]

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

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

[Marek Vasut]

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

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

[Vasily Khoruzhick]

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

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

[Daniel Mack]

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

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

[Vasily Khoruzhick]

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

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

[Daniel Mack]

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?

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

[Vasily Khoruzhick]

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

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

[Daniel Mack]

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.

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

[Vasily Khoruzhick]

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

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

[Daniel Mack]

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

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

[Vasily Khoruzhick]

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

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

[Haojian Zhuang]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121031/bcac3740/attachment-0001.html>

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

[Haojian Zhuang]

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

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

[Vasily Khoruzhick]

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

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

[Vasily Khoruzhick]

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

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

[Vasily Khoruzhick]

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

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

[Haojian Zhuang]

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.

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

[Daniel Mack]

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).

[PATCH 3/5] ARM: PXA: Zipit Z2: Fix oops in z2_power_off

[Vasily Khoruzhick]

pxa27x_set_pwrmode is called from z2_power_off, so it shouldn't
be marked with __init.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm/mach-pxa/pxa27x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
index 8047ee0..88ff275 100644
--- a/arch/arm/mach-pxa/pxa27x.c
+++ b/arch/arm/mach-pxa/pxa27x.c
@@ -242,7 +242,7 @@ static struct clk_lookup pxa27x_clkregs[] = {
  */
 static unsigned int pwrmode = PWRMODE_SLEEP;
 
-int __init pxa27x_set_pwrmode(unsigned int mode)
+int pxa27x_set_pwrmode(unsigned int mode)
 {
 	switch (mode) {
 	case PWRMODE_SLEEP:
-- 
1.7.12.4

[PATCH 4/5] ARM: PXA: Zipit Z2: Change active_state of power button

[Vasily Khoruzhick]

From: mcmajeres <mark@engine12.com>

Otherwise userspace might be confused about button state

Signed-off-by: mcmajeres <mark@engine12.com>
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm/mach-pxa/z2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-pxa/z2.c b/arch/arm/mach-pxa/z2.c
index ce90fa9..fac7cba 100644
--- a/arch/arm/mach-pxa/z2.c
+++ b/arch/arm/mach-pxa/z2.c
@@ -437,7 +437,7 @@ static struct gpio_keys_button z2_pxa_buttons[] = {
 	{
 		.code		= KEY_POWER,
 		.gpio		= GPIO1_ZIPITZ2_POWER_BUTTON,
-		.active_low	= 0,
+		.active_low	= 1,
 		.desc		= "Power Button",
 		.wakeup		= 1,
 		.type		= EV_KEY,
-- 
1.7.12.4

[PATCH 4/5] ARM: PXA: Zipit Z2: Change active_state of power button

[Marek Vasut]

Dear Vasily Khoruzhick,

> From: mcmajeres <mark@engine12.com>

Read [1]

> Otherwise userspace might be confused about button state
> 
> Signed-off-by: mcmajeres <mark@engine12.com>

Attribution of code shall be done only towards people known by their name.

> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  arch/arm/mach-pxa/z2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-pxa/z2.c b/arch/arm/mach-pxa/z2.c
> index ce90fa9..fac7cba 100644
> --- a/arch/arm/mach-pxa/z2.c
> +++ b/arch/arm/mach-pxa/z2.c
> @@ -437,7 +437,7 @@ static struct gpio_keys_button z2_pxa_buttons[] = {
>  	{
>  		.code		= KEY_POWER,
>  		.gpio		= GPIO1_ZIPITZ2_POWER_BUTTON,
> -		.active_low	= 0,
> +		.active_low	= 1,
>  		.desc		= "Power Button",
>  		.wakeup		= 1,
>  		.type		= EV_KEY,

[1] http://lwn.net/Articles/195643/

Best regards,
Marek Vasut

[PATCH 4/5] ARM: PXA: Zipit Z2: Change active_state of power button

[mark at engine12.com]

full name include:

On 2012-10-28 15:01, Marek Vasut wrote:
> Dear Vasily Khoruzhick,
>
>> From: Mark Majeres <mark@engine12.com>
>
> Read [1]
>
>> Otherwise userspace might be confused about button state
>>
>> Signed-off-by: Mark Majeres <mark@engine12.com>
>
> Attribution of code shall be done only towards people known by their 
> name.
>
>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>> ---
>>  arch/arm/mach-pxa/z2.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-pxa/z2.c b/arch/arm/mach-pxa/z2.c
>> index ce90fa9..fac7cba 100644
>> --- a/arch/arm/mach-pxa/z2.c
>> +++ b/arch/arm/mach-pxa/z2.c
>> @@ -437,7 +437,7 @@ static struct gpio_keys_button z2_pxa_buttons[] 
>> = {
>>  	{
>>  		.code		= KEY_POWER,
>>  		.gpio		= GPIO1_ZIPITZ2_POWER_BUTTON,
>> -		.active_low	= 0,
>> +		.active_low	= 1,
>>  		.desc		= "Power Button",
>>  		.wakeup		= 1,
>>  		.type		= EV_KEY,
>
> [1] http://lwn.net/Articles/195643/
>
> Best regards,
> Marek Vasut

[PATCH 5/5] ARM: PXA: Zipit Z2: Fix backlight PWM device number

[Vasily Khoruzhick]

Recent changes to PXA PWM support changed the PXA27X PWM device
numbering scheme, so keyboard and LCD backlight is not
working anymore on Z2.

Fix it and move from legacy to new PWM API.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm/mach-pxa/z2.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-pxa/z2.c b/arch/arm/mach-pxa/z2.c
index fac7cba..06b8078 100644
--- a/arch/arm/mach-pxa/z2.c
+++ b/arch/arm/mach-pxa/z2.c
@@ -16,6 +16,7 @@
 #include <linux/platform_device.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
+#include <linux/pwm.h>
 #include <linux/pwm_backlight.h>
 #include <linux/z2_battery.h>
 #include <linux/dma-mapping.h>
@@ -206,17 +207,22 @@ static inline void z2_nor_init(void) {}
  * Backlight
  ******************************************************************************/
 #if defined(CONFIG_BACKLIGHT_PWM) || defined(CONFIG_BACKLIGHT_PWM_MODULE)
+static struct pwm_lookup z2_pwm_lookup[] = {
+	PWM_LOOKUP("pxa27x-pwm.0", 1, "pwm-backlight.0", NULL),
+	PWM_LOOKUP("pxa27x-pwm.1", 0, "pwm-backlight.1", NULL),
+};
+
 static struct platform_pwm_backlight_data z2_backlight_data[] = {
 	[0] = {
 		/* Keypad Backlight */
-		.pwm_id		= 1,
+		.pwm_id		= -1,
 		.max_brightness	= 1023,
 		.dft_brightness	= 0,
 		.pwm_period_ns	= 1260320,
 	},
 	[1] = {
 		/* LCD Backlight */
-		.pwm_id		= 2,
+		.pwm_id		= -1,
 		.max_brightness	= 1023,
 		.dft_brightness	= 512,
 		.pwm_period_ns	= 1260320,
@@ -243,6 +249,7 @@ static void __init z2_pwm_init(void)
 {
 	platform_device_register(&z2_backlight_devices[0]);
 	platform_device_register(&z2_backlight_devices[1]);
+	pwm_add_table(z2_pwm_lookup, ARRAY_SIZE(z2_pwm_lookup));
 }
 #else
 static inline void z2_pwm_init(void) {}
-- 
1.7.12.4

[PATCH 5/5] ARM: PXA: Zipit Z2: Fix backlight PWM device number

[Marek Vasut]

Dear Vasily Khoruzhick,

> Recent changes to PXA PWM support changed the PXA27X PWM device
> numbering scheme, so keyboard and LCD backlight is not
> working anymore on Z2.
> 
> Fix it and move from legacy to new PWM API.

The proper fix here would be to start moving towards DT.

> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
[...]

Best regards,
Marek Vasut

[PATCH 5/5] ARM: PXA: Zipit Z2: Fix backlight PWM device number

[Vasily Khoruzhick]

On Mon, Oct 29, 2012 at 1:01 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Vasily Khoruzhick,
>
>> Recent changes to PXA PWM support changed the PXA27X PWM device
>> numbering scheme, so keyboard and LCD backlight is not
>> working anymore on Z2.
>>
>> Fix it and move from legacy to new PWM API.
>
> The proper fix here would be to start moving towards DT.
>
>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> [...]
>
> Best regards,
> Marek Vasut

Dear Marek Vasut,

Sure, but it'll take some time. For now this simple fix is sufficient.
I'll get DT working for pxa27x to familiarize myself with DT.

Regards
Vasily

[PATCH 5/5] ARM: PXA: Zipit Z2: Fix backlight PWM device number

[Marek Vasut]

Dear Vasily Khoruzhick,

> On Mon, Oct 29, 2012 at 1:01 AM, Marek Vasut <marex@denx.de> wrote:
> > Dear Vasily Khoruzhick,
> > 
> >> Recent changes to PXA PWM support changed the PXA27X PWM device
> >> numbering scheme, so keyboard and LCD backlight is not
> >> working anymore on Z2.
> >> 
> >> Fix it and move from legacy to new PWM API.
> > 
> > The proper fix here would be to start moving towards DT.
> > 
> >> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > 
> > [...]
> > 
> > Best regards,
> > Marek Vasut
> 
> Dear Marek Vasut,
> 
> Sure, but it'll take some time. For now this simple fix is sufficient.
> I'll get DT working for pxa27x to familiarize myself with DT.

Good, please focus yourself onto DT as the legacy implementations are not too 
favored anymore.

Best regards,
Marek Vasut

[PATCH 1/5] ARM: PXA: Add z2-usb-switch driver

[Vasily Khoruzhick]

Ooops, sorry, please discard this patch. One file is missing after
rebase, will resend this patch shortly

On Sun, Oct 28, 2012 at 6:35 PM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> This driver controls mode of USB port #2 pins - device or host.
>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  arch/arm/mach-pxa/Kconfig  | 7 +++++++
>  arch/arm/mach-pxa/Makefile | 1 +
>  2 files changed, 8 insertions(+)
>
> diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig
> index 11aa739..5fffc4b 100644
> --- a/arch/arm/mach-pxa/Kconfig
> +++ b/arch/arm/mach-pxa/Kconfig
> @@ -630,6 +630,13 @@ config MACH_ZIPIT2
>         bool "Zipit Z2 Handheld"
>         select HAVE_PWM
>         select PXA27x
> +
> +config Z2_USB_SWITCH
> +       tristate "Control the state of USB port on Zipit Z2"
> +       depends on MACH_ZIPIT2
> +       help
> +         This is a simple driver that is able to control
> +         usb mode of Zipit Z2
>  endif
>  endmenu
>
> diff --git a/arch/arm/mach-pxa/Makefile b/arch/arm/mach-pxa/Makefile
> index ee88d6e..86a032f 100644
> --- a/arch/arm/mach-pxa/Makefile
> +++ b/arch/arm/mach-pxa/Makefile
> @@ -99,3 +99,4 @@ obj-$(CONFIG_MACH_RAUMFELD_SPEAKER)   += raumfeld.o
>  obj-$(CONFIG_MACH_ZIPIT2)      += z2.o
>
>  obj-$(CONFIG_TOSA_BT)          += tosa-bt.o
> +obj-$(CONFIG_Z2_USB_SWITCH)    += z2-usb-switch.o
> --
> 1.7.12.4
>

[PATCH v2 1/5] ARM: PXA: Add z2-usb-switch driver

[Vasily Khoruzhick]

This driver controls mode of USB port #2 pins - device or host.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm/mach-pxa/Kconfig         |   7 +++
 arch/arm/mach-pxa/Makefile        |   1 +
 arch/arm/mach-pxa/z2-usb-switch.c | 100 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 108 insertions(+)
 create mode 100644 arch/arm/mach-pxa/z2-usb-switch.c

diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig
index 11aa739..5fffc4b 100644
--- a/arch/arm/mach-pxa/Kconfig
+++ b/arch/arm/mach-pxa/Kconfig
@@ -630,6 +630,13 @@ config MACH_ZIPIT2
 	bool "Zipit Z2 Handheld"
 	select HAVE_PWM
 	select PXA27x
+
+config Z2_USB_SWITCH
+	tristate "Control the state of USB port on Zipit Z2"
+	depends on MACH_ZIPIT2
+	help
+	  This is a simple driver that is able to control
+	  usb mode of Zipit Z2
 endif
 endmenu
 
diff --git a/arch/arm/mach-pxa/Makefile b/arch/arm/mach-pxa/Makefile
index ee88d6e..86a032f 100644
--- a/arch/arm/mach-pxa/Makefile
+++ b/arch/arm/mach-pxa/Makefile
@@ -99,3 +99,4 @@ obj-$(CONFIG_MACH_RAUMFELD_SPEAKER)	+= raumfeld.o
 obj-$(CONFIG_MACH_ZIPIT2)	+= z2.o
 
 obj-$(CONFIG_TOSA_BT)		+= tosa-bt.o
+obj-$(CONFIG_Z2_USB_SWITCH)	+= z2-usb-switch.o
diff --git a/arch/arm/mach-pxa/z2-usb-switch.c b/arch/arm/mach-pxa/z2-usb-switch.c
new file mode 100644
index 0000000..9583092
--- /dev/null
+++ b/arch/arm/mach-pxa/z2-usb-switch.c
@@ -0,0 +1,100 @@
+/*
+ * USB mode switcher for Z2
+ *
+ * Copyright (c) 2011 Vasily Khoruzhick
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+
+#include <mach/pxa27x.h>
+#include <mach/pxa27x-udc.h>
+
+#include <asm/io.h>
+
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+
+static ssize_t usb_mode_show(struct device *dev, struct device_attribute *attr,
+	char *buf)
+{
+	if (UP2OCR & UP2OCR_HXS)
+		return sprintf(buf, "host\n");
+	else
+		return sprintf(buf, "device\n");
+}
+
+static ssize_t usb_mode_set(struct device *dev, struct device_attribute *attr,
+	const char *buf, size_t count)
+{
+	if (strncmp(buf, "host", MIN(count, 4)) == 0) {
+		UP2OCR = UP2OCR_HXS | UP2OCR_HXOE | UP2OCR_DPPDE | UP2OCR_DMPDE;
+		return count;
+	} else if (strncmp(buf, "device", MIN(count, 6)) == 0) {
+		UP2OCR = UP2OCR_HXOE | UP2OCR_DPPUE;
+		return count;
+	}
+	return -EINVAL;
+}
+
+static DEVICE_ATTR(usb_mode, 0644, usb_mode_show, usb_mode_set);
+
+static const struct attribute *attrs[] = {
+	&dev_attr_usb_mode.attr,
+	NULL,
+};
+
+static const struct attribute_group attr_group = {
+	.attrs	= (struct attribute **)attrs,
+};
+
+static int z2_usb_switch_probe(struct platform_device *dev)
+{
+	int res;
+
+	res = sysfs_create_group(&dev->dev.kobj, &attr_group);
+	if (res)
+		return res;
+
+	UP2OCR = UP2OCR_HXOE | UP2OCR_DPPUE;
+
+	return 0;
+}
+
+static int __devexit z2_usb_switch_remove(struct platform_device *dev)
+{
+	UP2OCR = UP2OCR_HXOE;
+	sysfs_remove_group(&dev->dev.kobj, &attr_group);
+
+	return 0;
+}
+
+static struct platform_driver z2_usb_switch_driver = {
+	.probe = z2_usb_switch_probe,
+	.remove = __devexit_p(z2_usb_switch_remove),
+
+	.driver = {
+		.name = "z2-usb-switch",
+		.owner = THIS_MODULE,
+	},
+};
+
+
+static int __init z2_usb_switch_init(void)
+{
+	return platform_driver_register(&z2_usb_switch_driver);
+}
+
+static void __exit z2_usb_switch_exit(void)
+{
+	platform_driver_unregister(&z2_usb_switch_driver);
+}
+
+module_init(z2_usb_switch_init);
+module_exit(z2_usb_switch_exit);
-- 
1.7.12.4

[PATCH v2 1/5] ARM: PXA: Add z2-usb-switch driver

[Marko Katić]

On Sun, Oct 28, 2012 at 4:42 PM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> This driver controls mode of USB port #2 pins - device or host.
>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  arch/arm/mach-pxa/Kconfig         |   7 +++
>  arch/arm/mach-pxa/Makefile        |   1 +
>  arch/arm/mach-pxa/z2-usb-switch.c | 100 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 108 insertions(+)
>  create mode 100644 arch/arm/mach-pxa/z2-usb-switch.c
>
> diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig
> index 11aa739..5fffc4b 100644
> --- a/arch/arm/mach-pxa/Kconfig
> +++ b/arch/arm/mach-pxa/Kconfig
> @@ -630,6 +630,13 @@ config MACH_ZIPIT2
>         bool "Zipit Z2 Handheld"
>         select HAVE_PWM
>         select PXA27x
> +
> +config Z2_USB_SWITCH
> +       tristate "Control the state of USB port on Zipit Z2"
> +       depends on MACH_ZIPIT2
> +       help
> +         This is a simple driver that is able to control
> +         usb mode of Zipit Z2
>  endif
>  endmenu
>
> diff --git a/arch/arm/mach-pxa/Makefile b/arch/arm/mach-pxa/Makefile
> index ee88d6e..86a032f 100644
> --- a/arch/arm/mach-pxa/Makefile
> +++ b/arch/arm/mach-pxa/Makefile
> @@ -99,3 +99,4 @@ obj-$(CONFIG_MACH_RAUMFELD_SPEAKER)   += raumfeld.o
>  obj-$(CONFIG_MACH_ZIPIT2)      += z2.o
>
>  obj-$(CONFIG_TOSA_BT)          += tosa-bt.o
> +obj-$(CONFIG_Z2_USB_SWITCH)    += z2-usb-switch.o
> diff --git a/arch/arm/mach-pxa/z2-usb-switch.c b/arch/arm/mach-pxa/z2-usb-switch.c
> new file mode 100644
> index 0000000..9583092
> --- /dev/null
> +++ b/arch/arm/mach-pxa/z2-usb-switch.c
> @@ -0,0 +1,100 @@
> +/*
> + * USB mode switcher for Z2
> + *
> + * Copyright (c) 2011 Vasily Khoruzhick
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/delay.h>
> +
> +#include <mach/pxa27x.h>
> +#include <mach/pxa27x-udc.h>
> +
> +#include <asm/io.h>
> +
> +#define MIN(a, b) ((a) < (b) ? (a) : (b))
> +
> +static ssize_t usb_mode_show(struct device *dev, struct device_attribute *attr,
> +       char *buf)
> +{
> +       if (UP2OCR & UP2OCR_HXS)
> +               return sprintf(buf, "host\n");
> +       else
> +               return sprintf(buf, "device\n");
> +}
> +
> +static ssize_t usb_mode_set(struct device *dev, struct device_attribute *attr,
> +       const char *buf, size_t count)
> +{
> +       if (strncmp(buf, "host", MIN(count, 4)) == 0) {
> +               UP2OCR = UP2OCR_HXS | UP2OCR_HXOE | UP2OCR_DPPDE | UP2OCR_DMPDE;
> +               return count;
> +       } else if (strncmp(buf, "device", MIN(count, 6)) == 0) {
> +               UP2OCR = UP2OCR_HXOE | UP2OCR_DPPUE;
> +               return count;
> +       }
> +       return -EINVAL;
> +}
> +
> +static DEVICE_ATTR(usb_mode, 0644, usb_mode_show, usb_mode_set);
> +
> +static const struct attribute *attrs[] = {
> +       &dev_attr_usb_mode.attr,
> +       NULL,
> +};
> +
> +static const struct attribute_group attr_group = {
> +       .attrs  = (struct attribute **)attrs,
> +};
> +
> +static int z2_usb_switch_probe(struct platform_device *dev)
> +{
> +       int res;
> +
> +       res = sysfs_create_group(&dev->dev.kobj, &attr_group);
> +       if (res)
> +               return res;
> +
> +       UP2OCR = UP2OCR_HXOE | UP2OCR_DPPUE;
> +
> +       return 0;
> +}
> +
> +static int __devexit z2_usb_switch_remove(struct platform_device *dev)
> +{
> +       UP2OCR = UP2OCR_HXOE;
> +       sysfs_remove_group(&dev->dev.kobj, &attr_group);
> +
> +       return 0;
> +}
> +
> +static struct platform_driver z2_usb_switch_driver = {
> +       .probe = z2_usb_switch_probe,
> +       .remove = __devexit_p(z2_usb_switch_remove),
> +
> +       .driver = {
> +               .name = "z2-usb-switch",
> +               .owner = THIS_MODULE,
> +       },
> +};
> +
> +
> +static int __init z2_usb_switch_init(void)
> +{
> +       return platform_driver_register(&z2_usb_switch_driver);
> +}
> +
> +static void __exit z2_usb_switch_exit(void)
> +{
> +       platform_driver_unregister(&z2_usb_switch_driver);
> +}
> +
> +module_init(z2_usb_switch_init);
> +module_exit(z2_usb_switch_exit);
> --
> 1.7.12.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


A more fitting name might be something like "pxa270-usb-switch", since
this isn't really z2 specific. Other pxa270 based devices could use it
aswell.

[PATCH v2 1/5] ARM: PXA: Add z2-usb-switch driver

[Vasily Khoruzhick]

On Sun, Oct 28, 2012 at 8:03 PM, Marko Kati? <dromede@gmail.com> wrote:
>
> A more fitting name might be something like "pxa270-usb-switch", since
> this isn't really z2 specific. Other pxa270 based devices could use it
> aswell.

Sure. I'll wait for few more comments before sending v3 with fixed name.

Regards
Vasily

[PATCH v2 1/5] ARM: PXA: Add z2-usb-switch driver

[Marek Vasut]

Dear Vasily Khoruzhick,

> This driver controls mode of USB port #2 pins - device or host.

Please supply proper commit message. This short message describes nothing.

[...]

> @@ -0,0 +1,100 @@
> +/*
> + * USB mode switcher for Z2
> + *
> + * Copyright (c) 2011 Vasily Khoruzhick

2012

> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/delay.h>
> +
> +#include <mach/pxa27x.h>
> +#include <mach/pxa27x-udc.h>
> +
> +#include <asm/io.h>
> +
> +#define MIN(a, b) ((a) < (b) ? (a) : (b))

min() is already defined in kernel.h

> +static ssize_t usb_mode_show(struct device *dev, struct device_attribute
> *attr, +	char *buf)
> +{
> +	if (UP2OCR & UP2OCR_HXS)
> +		return sprintf(buf, "host\n");
> +	else
> +		return sprintf(buf, "device\n");
> +}
> +
> +static ssize_t usb_mode_set(struct device *dev, struct device_attribute
> *attr, +	const char *buf, size_t count)
> +{
> +	if (strncmp(buf, "host", MIN(count, 4)) == 0) {
> +		UP2OCR = UP2OCR_HXS | UP2OCR_HXOE | UP2OCR_DPPDE | UP2OCR_DMPDE;
> +		return count;
> +	} else if (strncmp(buf, "device", MIN(count, 6)) == 0) {
> +		UP2OCR = UP2OCR_HXOE | UP2OCR_DPPUE;
> +		return count;
> +	}
> +	return -EINVAL;
> +}
> +
> +static DEVICE_ATTR(usb_mode, 0644, usb_mode_show, usb_mode_set);

I wonder if we have no better means to control enforcement of mode.

> +static const struct attribute *attrs[] = {
> +	&dev_attr_usb_mode.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group attr_group = {
> +	.attrs	= (struct attribute **)attrs,
> +};

Isn't there some macro to do this assignment?

> +static int z2_usb_switch_probe(struct platform_device *dev)

Missing __devinit

> +{
> +	int res;
> +
> +	res = sysfs_create_group(&dev->dev.kobj, &attr_group);
> +	if (res)
> +		return res;
> +
> +	UP2OCR = UP2OCR_HXOE | UP2OCR_DPPUE;
> +
> +	return 0;
> +}
> +
> +static int __devexit z2_usb_switch_remove(struct platform_device *dev)
> +{
> +	UP2OCR = UP2OCR_HXOE;
> +	sysfs_remove_group(&dev->dev.kobj, &attr_group);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver z2_usb_switch_driver = {
> +	.probe = z2_usb_switch_probe,
> +	.remove = __devexit_p(z2_usb_switch_remove),
> +
> +	.driver = {
> +		.name = "z2-usb-switch",
> +		.owner = THIS_MODULE,
> +	},
> +};
> +
> +
> +static int __init z2_usb_switch_init(void)
> +{
> +	return platform_driver_register(&z2_usb_switch_driver);
> +}
> +
> +static void __exit z2_usb_switch_exit(void)
> +{
> +	platform_driver_unregister(&z2_usb_switch_driver);
> +}
> +
> +module_init(z2_usb_switch_init);
> +module_exit(z2_usb_switch_exit);

module_platform_driver()

Best regards,
Marek Vasut

[PATCH v2 1/5] ARM: PXA: Add z2-usb-switch driver

[Vasily Khoruzhick]

On Mon, Oct 29, 2012 at 12:57 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Vasily Khoruzhick,

Dear Marek Vasut,

>> This driver controls mode of USB port #2 pins - device or host.
>
> Please supply proper commit message. This short message describes nothing.

OK, "This driver allows user to choose USB port #2 mode between device
and host" - that would be OK?

> [...]
>
>> @@ -0,0 +1,100 @@
>> +/*
>> + * USB mode switcher for Z2
>> + *
>> + * Copyright (c) 2011 Vasily Khoruzhick
>
> 2012

Actually, it was implemented early in 2011, so 2011-2012

>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/delay.h>
>> +
>> +#include <mach/pxa27x.h>
>> +#include <mach/pxa27x-udc.h>
>> +
>> +#include <asm/io.h>
>> +
>> +#define MIN(a, b) ((a) < (b) ? (a) : (b))
>
> min() is already defined in kernel.h

OK

>> +static ssize_t usb_mode_show(struct device *dev, struct device_attribute
>> *attr, +      char *buf)
>> +{
>> +     if (UP2OCR & UP2OCR_HXS)
>> +             return sprintf(buf, "host\n");
>> +     else
>> +             return sprintf(buf, "device\n");
>> +}
>> +
>> +static ssize_t usb_mode_set(struct device *dev, struct device_attribute
>> *attr, +      const char *buf, size_t count)
>> +{
>> +     if (strncmp(buf, "host", MIN(count, 4)) == 0) {
>> +             UP2OCR = UP2OCR_HXS | UP2OCR_HXOE | UP2OCR_DPPDE | UP2OCR_DMPDE;
>> +             return count;
>> +     } else if (strncmp(buf, "device", MIN(count, 6)) == 0) {
>> +             UP2OCR = UP2OCR_HXOE | UP2OCR_DPPUE;
>> +             return count;
>> +     }
>> +     return -EINVAL;
>> +}
>> +
>> +static DEVICE_ATTR(usb_mode, 0644, usb_mode_show, usb_mode_set);
>
> I wonder if we have no better means to control enforcement of mode.

Why? sysfs fits nicely.

>> +static const struct attribute *attrs[] = {
>> +     &dev_attr_usb_mode.attr,
>> +     NULL,
>> +};
>> +
>> +static const struct attribute_group attr_group = {
>> +     .attrs  = (struct attribute **)attrs,
>> +};
>
> Isn't there some macro to do this assignment?

Will check

>> +static int z2_usb_switch_probe(struct platform_device *dev)
>
> Missing __devinit

OK

>> +{
>> +     int res;
>> +
>> +     res = sysfs_create_group(&dev->dev.kobj, &attr_group);
>> +     if (res)
>> +             return res;
>> +
>> +     UP2OCR = UP2OCR_HXOE | UP2OCR_DPPUE;
>> +
>> +     return 0;
>> +}
>> +
>> +static int __devexit z2_usb_switch_remove(struct platform_device *dev)
>> +{
>> +     UP2OCR = UP2OCR_HXOE;
>> +     sysfs_remove_group(&dev->dev.kobj, &attr_group);
>> +
>> +     return 0;
>> +}
>> +
>> +static struct platform_driver z2_usb_switch_driver = {
>> +     .probe = z2_usb_switch_probe,
>> +     .remove = __devexit_p(z2_usb_switch_remove),
>> +
>> +     .driver = {
>> +             .name = "z2-usb-switch",
>> +             .owner = THIS_MODULE,
>> +     },
>> +};
>> +
>> +
>> +static int __init z2_usb_switch_init(void)
>> +{
>> +     return platform_driver_register(&z2_usb_switch_driver);
>> +}
>> +
>> +static void __exit z2_usb_switch_exit(void)
>> +{
>> +     platform_driver_unregister(&z2_usb_switch_driver);
>> +}
>> +
>> +module_init(z2_usb_switch_init);
>> +module_exit(z2_usb_switch_exit);
>
> module_platform_driver()

OK

> Best regards,
> Marek Vasut

Thanks for review!

Regards
Vasily

[PATCH v2 1/5] ARM: PXA: Add z2-usb-switch driver

[Marek Vasut]

Dear Vasily Khoruzhick,

> On Mon, Oct 29, 2012 at 12:57 AM, Marek Vasut <marex@denx.de> wrote:
> > Dear Vasily Khoruzhick,
> 
> Dear Marek Vasut,
> 
> >> This driver controls mode of USB port #2 pins - device or host.
> > 
> > Please supply proper commit message. This short message describes
> > nothing.
> 
> OK, "This driver allows user to choose USB port #2 mode between device
> and host" - that would be OK?

Please describe why is this needed at all and what it does.

[...]

> > I wonder if we have no better means to control enforcement of mode.
> 
> Why? sysfs fits nicely.

Because if there is already an API to implement this OTG in kernel, you should 
use that and not reinvent wheel.

[...]

Best regards,
Marek Vasut

[PATCH v2 1/5] ARM: PXA: Add z2-usb-switch driver

[Vasily Khoruzhick]

On Mon, Oct 29, 2012 at 1:38 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Vasily Khoruzhick,
>
>> On Mon, Oct 29, 2012 at 12:57 AM, Marek Vasut <marex@denx.de> wrote:
>> > Dear Vasily Khoruzhick,
>>
>> Dear Marek Vasut,
>>
>> >> This driver controls mode of USB port #2 pins - device or host.
>> >
>> > Please supply proper commit message. This short message describes
>> > nothing.
>>
>> OK, "This driver allows user to choose USB port #2 mode between device
>> and host" - that would be OK?
>
> Please describe why is this needed at all and what it does.

Because one might need USB device and USB host functionality provided
by USB port #2?
It switches pins mode from device to host and backwards.

> [...]
>
>> > I wonder if we have no better means to control enforcement of mode.
>>
>> Why? sysfs fits nicely.
>
> Because if there is already an API to implement this OTG in kernel, you should
> use that and not reinvent wheel.

One needs to select USB mode manually (btw, Z2 can't even power USB
device properly, only 3.3v or external 5v).
There's no way to determine what type of device (device/host)
connected to Z2 at the moment.

> [...]
>
> Best regards,
> Marek Vasut

Regards
Vasily

[PATCH v2 1/5] ARM: PXA: Add z2-usb-switch driver

[Marek Vasut]

Dear Vasily Khoruzhick,

> On Mon, Oct 29, 2012 at 1:38 AM, Marek Vasut <marex@denx.de> wrote:
> > Dear Vasily Khoruzhick,
> > 
> >> On Mon, Oct 29, 2012 at 12:57 AM, Marek Vasut <marex@denx.de> wrote:
> >> > Dear Vasily Khoruzhick,
> >> 
> >> Dear Marek Vasut,
> >> 
> >> >> This driver controls mode of USB port #2 pins - device or host.
> >> > 
> >> > Please supply proper commit message. This short message describes
> >> > nothing.
> >> 
> >> OK, "This driver allows user to choose USB port #2 mode between device
> >> and host" - that would be OK?
> > 
> > Please describe why is this needed at all and what it does.
> 
> Because one might need USB device and USB host functionality provided
> by USB port #2?
> It switches pins mode from device to host and backwards.

Good, now compile it into the commit message for v3 please.

> > [...]
> > 
> >> > I wonder if we have no better means to control enforcement of mode.
> >> 
> >> Why? sysfs fits nicely.
> > 
> > Because if there is already an API to implement this OTG in kernel, you
> > should use that and not reinvent wheel.
> 
> One needs to select USB mode manually (btw, Z2 can't even power USB
> device properly, only 3.3v or external 5v).
> There's no way to determine what type of device (device/host)
> connected to Z2 at the moment.

Good, so your OTG is software controlled.

[...]

Best regards,
Marek Vasut