Re: [PATCH 3/5] omap3: cm-t3517: add support for usb host.

2010-09-27 Thread Tony Lindgren 
* Igor Grinberg grinb...@compulab.co.il [100922 01:16]:
 
  It should be okay to execute this code independent of whether
  the driver is built or not. The device registration can be unconditional
  and if there is no driver present, we won't probe anyway.

The current patch looks OK to me, adding all of them into omap-for-linus.

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] omap3: cm-t3517: add support for usb host.

2010-09-22 Thread Igor Grinberg 
 On 09/21/10 18:26, Gadiyar, Anand wrote:
 On Tue, Sep 21, 2010 at 9:33 PM, Igor Grinberg grinb...@compulab.co.il 
 wrote:
 add support for hsusb host ports 1, 2 and on-module usb hub.

 Signed-off-by: Igor Grinberg grinb...@compulab.co.il
 ---
  arch/arm/mach-omap2/board-cm-t3517.c |   50 
 ++
 ...

 @@ -100,6 +102,47 @@ static void __init cm_t3517_init_rtc(void)
  static inline void cm_t3517_init_rtc(void) {}
  #endif

 +#if defined(CONFIG_USB_EHCI_HCD) || defined(CONFIG_USB_EHCI_HCD_MODULE)
 Hi Igor,

Hi

 Do we really need to make these conditional on the driver being built?

This is depends on what are we trying to achieve...
I can think of two things:
[1] Simple, clear and nice code without ifdefs
[2] Smaller kernel, a bit less resources wasted and a bit faster boot time

 The hardware is present on the board at all times, right?

Yes, it is inside the SoC, except for the usb hub.

 It should be okay to execute this code independent of whether
 the driver is built or not. The device registration can be unconditional
 and if there is no driver present, we won't probe anyway.

It is ok, but again it depends on what we are trying to achieve here.

If we want [1], then indeed we need to remove the ifdefs also inside usb-ehci.c,
so pros:
 * we'll get nice and clean code,
cons:
 * a bit bigger kernel,
 * floating device without a driver,
 * a bit resources wasted for this device,
 * a bit slower startup time (device registration, gpio toggling, etc.).

If we want [2], then we'll have the opposite of [1] ;)

Personally, I don't mind either way... ,
but I want the code to be consistent (in this case with usb-ehci.c).

 (I see some similar logic in usb-ehci.c - probably a good idea to remove
 it as well).

...

 - Anand

 +#define HSUSB1_RESET_GPIO  (146)
 +#define HSUSB2_RESET_GPIO  (147)
 +#define USB_HUB_RESET_GPIO (152)
 +
 +static struct ehci_hcd_omap_platform_data cm_t3517_ehci_pdata __initdata = {
 +   .port_mode[0] = EHCI_HCD_OMAP_MODE_PHY,
 +   .port_mode[1] = EHCI_HCD_OMAP_MODE_PHY,
 +   .port_mode[2] = EHCI_HCD_OMAP_MODE_UNKNOWN,
 +
 +   .phy_reset  = true,
 +   .reset_gpio_port[0]  = HSUSB1_RESET_GPIO,
 +   .reset_gpio_port[1]  = HSUSB2_RESET_GPIO,
 +   .reset_gpio_port[2]  = -EINVAL,
 +};
 +
 +static int cm_t3517_init_usbh(void)
 +{
 +   int err;
 +
 +   err = gpio_request(USB_HUB_RESET_GPIO, usb hub rst);
 +   if (err) {
 +   pr_err(CM-T3517: usb hub rst gpio request failed: %d\n, 
 err);
 +   } else {
 +   gpio_direction_output(USB_HUB_RESET_GPIO, 0);
 +   udelay(10);
 +   gpio_set_value(USB_HUB_RESET_GPIO, 1);
 +   msleep(1);
 +   }
 +
 +   usb_ehci_init(cm_t3517_ehci_pdata);
 +
 +   return 0;
 +}
 +#else
 +static inline int cm_t3517_init_usbh(void)
 +{
 +   return 0;
 +}
 +#endif
 +
 --
 To unsubscribe from this list: send the line unsubscribe linux-omap in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Regards,
Igor.

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] omap3: cm-t3517: add support for usb host.

2010-09-21 Thread Felipe Balbi 

On Mon, Sep 20, 2010 at 09:46:34AM -0500, Igor Grinberg wrote:

On 09/20/10 08:39, Felipe Balbi wrote:

On Thu, Sep 16, 2010 at 04:12:06AM -0500, Igor Grinberg wrote:

Yes it will, but even if the hub reset gpio is for some reason unavailable,
we don't want to disable ehci completely...


then you should set the gpio to an invalid number, otherwise you might
cause lots of WARN() to appear due to fiddling with unrequested gpio.


What gpio should be set to invalid number?


the one which fails to be requested.


It is a reset gpio for usb _hub_ and it has nothing to do with ehci.
Can you please, explain the case when I might cause those WARN()'s?


ok, I misunderstood then. Nevermind

--
balbi
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] omap3: cm-t3517: add support for usb host.

2010-09-21 Thread Igor Grinberg 
add support for hsusb host ports 1, 2 and on-module usb hub.

Signed-off-by: Igor Grinberg grinb...@compulab.co.il
---
 arch/arm/mach-omap2/board-cm-t3517.c |   50 ++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/board-cm-t3517.c 
b/arch/arm/mach-omap2/board-cm-t3517.c
index 2b41c6d..2c7082d 100644
--- a/arch/arm/mach-omap2/board-cm-t3517.c
+++ b/arch/arm/mach-omap2/board-cm-t3517.c
@@ -25,6 +25,7 @@
 #include linux/kernel.h
 #include linux/init.h
 #include linux/platform_device.h
+#include linux/delay.h
 #include linux/gpio.h
 #include linux/leds.h
 #include linux/rtc-v3020.h
@@ -36,6 +37,7 @@
 #include plat/board.h
 #include plat/common.h
 #include plat/control.h
+#include plat/usb.h
 
 #include mux.h
 
@@ -100,6 +102,47 @@ static void __init cm_t3517_init_rtc(void)
 static inline void cm_t3517_init_rtc(void) {}
 #endif
 
+#if defined(CONFIG_USB_EHCI_HCD) || defined(CONFIG_USB_EHCI_HCD_MODULE)
+#define HSUSB1_RESET_GPIO  (146)
+#define HSUSB2_RESET_GPIO  (147)
+#define USB_HUB_RESET_GPIO (152)
+
+static struct ehci_hcd_omap_platform_data cm_t3517_ehci_pdata __initdata = {
+   .port_mode[0] = EHCI_HCD_OMAP_MODE_PHY,
+   .port_mode[1] = EHCI_HCD_OMAP_MODE_PHY,
+   .port_mode[2] = EHCI_HCD_OMAP_MODE_UNKNOWN,
+
+   .phy_reset  = true,
+   .reset_gpio_port[0]  = HSUSB1_RESET_GPIO,
+   .reset_gpio_port[1]  = HSUSB2_RESET_GPIO,
+   .reset_gpio_port[2]  = -EINVAL,
+};
+
+static int cm_t3517_init_usbh(void)
+{
+   int err;
+
+   err = gpio_request(USB_HUB_RESET_GPIO, usb hub rst);
+   if (err) {
+   pr_err(CM-T3517: usb hub rst gpio request failed: %d\n, err);
+   } else {
+   gpio_direction_output(USB_HUB_RESET_GPIO, 0);
+   udelay(10);
+   gpio_set_value(USB_HUB_RESET_GPIO, 1);
+   msleep(1);
+   }
+
+   usb_ehci_init(cm_t3517_ehci_pdata);
+
+   return 0;
+}
+#else
+static inline int cm_t3517_init_usbh(void)
+{
+   return 0;
+}
+#endif
+
 static struct omap_board_config_kernel cm_t3517_config[] __initdata = {
 };
 
@@ -121,6 +164,12 @@ static struct omap_board_mux board_mux[] __initdata = {
OMAP3_MUX(MCBSP4_DX, OMAP_MUX_MODE4 | OMAP_PIN_INPUT),
OMAP3_MUX(MCBSP_CLKS, OMAP_MUX_MODE4 | OMAP_PIN_INPUT),
OMAP3_MUX(UART3_CTS_RCTX, OMAP_MUX_MODE4 | OMAP_PIN_INPUT),
+   /* HSUSB1 RESET */
+   OMAP3_MUX(UART2_TX, OMAP_MUX_MODE4 | OMAP_PIN_OUTPUT),
+   /* HSUSB2 RESET */
+   OMAP3_MUX(UART2_RX, OMAP_MUX_MODE4 | OMAP_PIN_OUTPUT),
+   /* CM-T3517 USB HUB nRESET */
+   OMAP3_MUX(MCBSP4_CLKX, OMAP_MUX_MODE4 | OMAP_PIN_OUTPUT),
 
{ .reg_offset = OMAP_MUX_TERMINATOR },
 };
@@ -131,6 +180,7 @@ static void __init cm_t3517_init(void)
omap_serial_init();
cm_t3517_init_leds();
cm_t3517_init_rtc();
+   cm_t3517_init_usbh();
 }
 
 MACHINE_START(CM_T3517, Compulab CM-T3517)
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] omap3: cm-t3517: add support for usb host.

2010-09-21 Thread Gadiyar, Anand 
On Tue, Sep 21, 2010 at 9:33 PM, Igor Grinberg grinb...@compulab.co.il wrote:
 add support for hsusb host ports 1, 2 and on-module usb hub.

 Signed-off-by: Igor Grinberg grinb...@compulab.co.il
 ---
  arch/arm/mach-omap2/board-cm-t3517.c |   50 
 ++

...

 @@ -100,6 +102,47 @@ static void __init cm_t3517_init_rtc(void)
  static inline void cm_t3517_init_rtc(void) {}
  #endif

 +#if defined(CONFIG_USB_EHCI_HCD) || defined(CONFIG_USB_EHCI_HCD_MODULE)

Hi Igor,


Do we really need to make these conditional on the driver being built?

The hardware is present on the board at all times, right?
It should be okay to execute this code independent of whether
the driver is built or not. The device registration can be unconditional
and if there is no driver present, we won't probe anyway.

(I see some similar logic in usb-ehci.c - probably a good idea to remove
it as well).

- Anand

 +#define HSUSB1_RESET_GPIO      (146)
 +#define HSUSB2_RESET_GPIO      (147)
 +#define USB_HUB_RESET_GPIO     (152)
 +
 +static struct ehci_hcd_omap_platform_data cm_t3517_ehci_pdata __initdata = {
 +       .port_mode[0] = EHCI_HCD_OMAP_MODE_PHY,
 +       .port_mode[1] = EHCI_HCD_OMAP_MODE_PHY,
 +       .port_mode[2] = EHCI_HCD_OMAP_MODE_UNKNOWN,
 +
 +       .phy_reset  = true,
 +       .reset_gpio_port[0]  = HSUSB1_RESET_GPIO,
 +       .reset_gpio_port[1]  = HSUSB2_RESET_GPIO,
 +       .reset_gpio_port[2]  = -EINVAL,
 +};
 +
 +static int cm_t3517_init_usbh(void)
 +{
 +       int err;
 +
 +       err = gpio_request(USB_HUB_RESET_GPIO, usb hub rst);
 +       if (err) {
 +               pr_err(CM-T3517: usb hub rst gpio request failed: %d\n, 
 err);
 +       } else {
 +               gpio_direction_output(USB_HUB_RESET_GPIO, 0);
 +               udelay(10);
 +               gpio_set_value(USB_HUB_RESET_GPIO, 1);
 +               msleep(1);
 +       }
 +
 +       usb_ehci_init(cm_t3517_ehci_pdata);
 +
 +       return 0;
 +}
 +#else
 +static inline int cm_t3517_init_usbh(void)
 +{
 +       return 0;
 +}
 +#endif
 +
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] omap3: cm-t3517: add support for usb host.

2010-09-20 Thread Igor Grinberg 
 On 09/20/10 08:39, Felipe Balbi wrote:
 On Thu, Sep 16, 2010 at 04:12:06AM -0500, Igor Grinberg wrote:
 Yes it will, but even if the hub reset gpio is for some reason unavailable,
 we don't want to disable ehci completely...

 then you should set the gpio to an invalid number, otherwise you might
 cause lots of WARN() to appear due to fiddling with unrequested gpio.

What gpio should be set to invalid number?
It is a reset gpio for usb _hub_ and it has nothing to do with ehci.
Can you please, explain the case when I might cause those WARN()'s?

-- 
Regards,
Igor.

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] omap3: cm-t3517: add support for usb host.

2010-09-19 Thread Igor Grinberg 


On 09/16/10 19:48, Tony Lindgren wrote:
 * Igor Grinberg grinb...@compulab.co.il [100916 02:04]:
  Hi,

 On 09/16/10 11:00, Felipe Balbi wrote:
 Hi,

 On Thu, Sep 16, 2010 at 03:54:39AM -0500, Igor Grinberg wrote:

 put a description here.
 This is a relatively small patch, is the subject not descriptive enough?
 In that case just copy the subject (without the [PATCH] part) to the
 description for all of your patches.

Ok, will do and re-spin.

Thanks

 Regards,

 Tony


-- 
Regards,
Igor.

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] omap3: cm-t3517: add support for usb host.

2010-09-16 Thread Igor Grinberg 

Signed-off-by: Igor Grinberg grinb...@compulab.co.il
---
 arch/arm/mach-omap2/board-cm-t3517.c |   51 ++
 1 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/board-cm-t3517.c 
b/arch/arm/mach-omap2/board-cm-t3517.c
index 2b41c6d..23cd9a1 100644
--- a/arch/arm/mach-omap2/board-cm-t3517.c
+++ b/arch/arm/mach-omap2/board-cm-t3517.c
@@ -25,6 +25,7 @@
 #include linux/kernel.h
 #include linux/init.h
 #include linux/platform_device.h
+#include linux/delay.h
 #include linux/gpio.h
 #include linux/leds.h
 #include linux/rtc-v3020.h
@@ -36,6 +37,7 @@
 #include plat/board.h
 #include plat/common.h
 #include plat/control.h
+#include plat/usb.h
 
 #include mux.h
 
@@ -100,6 +102,48 @@ static void __init cm_t3517_init_rtc(void)
 static inline void cm_t3517_init_rtc(void) {}
 #endif
 
+#if defined(CONFIG_USB_EHCI_HCD) || defined(CONFIG_USB_EHCI_HCD_MODULE)
+#define HSUSB1_RESET_GPIO  (146)
+#define HSUSB2_RESET_GPIO  (147)
+#define USB_HUB_RESET_GPIO (152)
+
+static struct ehci_hcd_omap_platform_data cm_t3517_ehci_pdata __initdata = {
+   .port_mode[0] = EHCI_HCD_OMAP_MODE_PHY,
+   .port_mode[1] = EHCI_HCD_OMAP_MODE_PHY,
+   .port_mode[2] = EHCI_HCD_OMAP_MODE_UNKNOWN,
+
+   .phy_reset  = true,
+   .reset_gpio_port[0]  = HSUSB1_RESET_GPIO,
+   .reset_gpio_port[1]  = HSUSB2_RESET_GPIO,
+   .reset_gpio_port[2]  = -EINVAL,
+};
+
+static int cm_t3517_init_usbh(void)
+{
+   int err;
+
+   err = gpio_request(USB_HUB_RESET_GPIO, usb hub rst);
+   if (err) {
+   pr_err(CM-T3517: usb hub reset gpio request failed: %d\n,
+  err);
+   } else {
+   gpio_direction_output(USB_HUB_RESET_GPIO, 0);
+   udelay(10);
+   gpio_set_value(USB_HUB_RESET_GPIO, 1);
+   msleep(1);
+   }
+
+   usb_ehci_init(cm_t3517_ehci_pdata);
+
+   return 0;
+}
+#else
+static inline int cm_t3517_init_usbh(void)
+{
+   return 0;
+}
+#endif
+
 static struct omap_board_config_kernel cm_t3517_config[] __initdata = {
 };
 
@@ -121,6 +165,12 @@ static struct omap_board_mux board_mux[] __initdata = {
OMAP3_MUX(MCBSP4_DX, OMAP_MUX_MODE4 | OMAP_PIN_INPUT),
OMAP3_MUX(MCBSP_CLKS, OMAP_MUX_MODE4 | OMAP_PIN_INPUT),
OMAP3_MUX(UART3_CTS_RCTX, OMAP_MUX_MODE4 | OMAP_PIN_INPUT),
+   /* HSUSB1 RESET */
+   OMAP3_MUX(UART2_TX, OMAP_MUX_MODE4 | OMAP_PIN_OUTPUT),
+   /* HSUSB2 RESET */
+   OMAP3_MUX(UART2_RX, OMAP_MUX_MODE4 | OMAP_PIN_OUTPUT),
+   /* CM-T3517 USB HUB nRESET */
+   OMAP3_MUX(MCBSP4_CLKX, OMAP_MUX_MODE4 | OMAP_PIN_OUTPUT),
 
{ .reg_offset = OMAP_MUX_TERMINATOR },
 };
@@ -131,6 +181,7 @@ static void __init cm_t3517_init(void)
omap_serial_init();
cm_t3517_init_leds();
cm_t3517_init_rtc();
+   cm_t3517_init_usbh();
 }
 
 MACHINE_START(CM_T3517, Compulab CM-T3517)
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] omap3: cm-t3517: add support for usb host.

2010-09-16 Thread Felipe Balbi 

Hi,

On Thu, Sep 16, 2010 at 03:54:39AM -0500, Igor Grinberg wrote:

put a description here.


Signed-off-by: Igor Grinberg grinb...@compulab.co.il
---


[snip]


+static int cm_t3517_init_usbh(void)
+{
+   int err;
+
+   err = gpio_request(USB_HUB_RESET_GPIO, usb hub rst);
+   if (err) {
+   pr_err(CM-T3517: usb hub reset gpio request failed: %d\n,
+  err);
+   } else {


if (err) {
pr_err(...);
return err;
}

gpio_direction_output(...);

this will save you on identation level.

--
balbi
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] omap3: cm-t3517: add support for usb host.

2010-09-16 Thread Igor Grinberg 
 Hi,

On 09/16/10 11:00, Felipe Balbi wrote:
 Hi,

 On Thu, Sep 16, 2010 at 03:54:39AM -0500, Igor Grinberg wrote:

 put a description here.

This is a relatively small patch, is the subject not descriptive enough?


 Signed-off-by: Igor Grinberg grinb...@compulab.co.il
 ---

 [snip]

 +static int cm_t3517_init_usbh(void)
 +{
 +int err;
 +
 +err = gpio_request(USB_HUB_RESET_GPIO, usb hub rst);
 +if (err) {
 +pr_err(CM-T3517: usb hub reset gpio request failed: %d\n,
 +   err);
 +} else {

 if (err) {
 pr_err(...);
 return err;
 }

 gpio_direction_output(...);

 this will save you on identation level.

Yes it will, but even if the hub reset gpio is for some reason unavailable,
we don't want to disable ehci completely...
There are other ports, that are not wired through the hub.

-- 
Regards,
Igor.

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] omap3: cm-t3517: add support for usb host.

2010-09-16 Thread Tony Lindgren 
* Igor Grinberg grinb...@compulab.co.il [100916 02:04]:
  Hi,
 
 On 09/16/10 11:00, Felipe Balbi wrote:
  Hi,
 
  On Thu, Sep 16, 2010 at 03:54:39AM -0500, Igor Grinberg wrote:
 
  put a description here.
 
 This is a relatively small patch, is the subject not descriptive enough?

In that case just copy the subject (without the [PATCH] part) to the
description for all of your patches.

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html