diff mbox

[U-Boot] imx: mx6 sabreauto: Add board support for USB EHCI

Message ID 1412931700-17476-1-git-send-email-B37916@freescale.com
State Changes Requested
Delegated to: Stefano Babic
Headers show

Commit Message

Ye.Li Oct. 10, 2014, 9:01 a.m. UTC
On mx6 sabreauto board, there are two USB ports:
0: OTG
1: HOST
The EHCI driver is enabled for this board, but the IOMUX and VBUS power
control is not implemented, which cause both USB port failed to work.
This patch fix the problem by adding the BSP support.

Since the power control uses the GPIO pin from port expander MAX7310,
the PCA953X driver is enabled for accessing the MAX7310.

The ID pin of OTG Port needs to configure the GPR1 bit 13 for selecting
its daisy chain. Add a new function "imx_iomux_set_gpr_register" to
handle GPR register setting.

Signed-off-by: Ye.Li <B37916@freescale.com>
---
 arch/arm/imx-common/iomux-v3.c                |   17 +++++-
 arch/arm/include/asm/imx-common/iomux-v3.h    |    7 ++-
 board/freescale/mx6qsabreauto/mx6qsabreauto.c |   89 ++++++++++++++++++++++++-
 include/configs/mx6qsabreauto.h               |    5 +-
 4 files changed, 114 insertions(+), 4 deletions(-)

Comments

Fabio Estevam Oct. 10, 2014, 11:46 a.m. UTC | #1
On Fri, Oct 10, 2014 at 6:01 AM, Ye.Li <B37916@freescale.com> wrote:
> On mx6 sabreauto board, there are two USB ports:
> 0: OTG
> 1: HOST
> The EHCI driver is enabled for this board, but the IOMUX and VBUS power
> control is not implemented, which cause both USB port failed to work.
> This patch fix the problem by adding the BSP support.

BSP is a broad term here.

>
> Since the power control uses the GPIO pin from port expander MAX7310,
> the PCA953X driver is enabled for accessing the MAX7310.
>
> The ID pin of OTG Port needs to configure the GPR1 bit 13 for selecting
> its daisy chain. Add a new function "imx_iomux_set_gpr_register" to
> handle GPR register setting.
>
> Signed-off-by: Ye.Li <B37916@freescale.com>
> ---
>  arch/arm/imx-common/iomux-v3.c                |   17 +++++-
>  arch/arm/include/asm/imx-common/iomux-v3.h    |    7 ++-
>  board/freescale/mx6qsabreauto/mx6qsabreauto.c |   89 ++++++++++++++++++++++++-
>  include/configs/mx6qsabreauto.h               |    5 +-
>  4 files changed, 114 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/imx-common/iomux-v3.c b/arch/arm/imx-common/iomux-v3.c
> index 22cd11a..2d96655 100644
> --- a/arch/arm/imx-common/iomux-v3.c
> +++ b/arch/arm/imx-common/iomux-v3.c
> @@ -4,7 +4,7 @@
>   * Copyright (C) 2009 by Jan Weitzel Phytec Messtechnik GmbH,
>   *                       <armlinux@phytec.de>
>   *
> - * Copyright (C) 2004-2011 Freescale Semiconductor, Inc.
> + * Copyright (C) 2004-2014 Freescale Semiconductor, Inc.

I don't see the need for changing the copyright everytime you touch a
file. Same apply to other files in this patch.

> +static int port_exp_direction_output(unsigned gpio, int value)
> +{
> +       int ret;
> +
> +       i2c_set_bus_num(2);
> +       if (i2c_probe(PORTEXP_IO_TO_CHIP(gpio)))
> +               return -ENXIO;

Would be better like this:

ret = i2c_probe(PORTEXP_IO_TO_CHIP(gpio))
if (ret)
    return ret;

> +
> +       ret = pca953x_set_dir(PORTEXP_IO_TO_CHIP(gpio),
> +               (1 << PORTEXP_IO_TO_PIN(gpio)),
> +               (PCA953X_DIR_OUT << PORTEXP_IO_TO_PIN(gpio)));

Here you can do:

if (ret)
  return ret;

> +
> +       if (!ret)

Then you don't need to check for !ret here

> +               ret = pca953x_set_val(PORTEXP_IO_TO_CHIP(gpio),
> +                       (1 << PORTEXP_IO_TO_PIN(gpio)),
> +                       (value << PORTEXP_IO_TO_PIN(gpio)));
> +
> +       if (ret)
> +               ret = -EIO;
> +
> +       return ret;
> +}

if (ret)
   return ret;

return 0;

> +int board_ehci_hcd_init(int port)
> +{
> +       switch (port) {
> +       case 0:
> +               imx_iomux_v3_setup_multiple_pads(usb_otg_pads,
> +                       ARRAY_SIZE(usb_otg_pads));
> +
> +               /*set daisy chain for otg_pin_id on 6q. for 6dl, this bit is reserved*/
> +               imx_iomux_set_gpr_register(1, 13, 1, 0);
> +               break;
> +       case 1:
> +               break;
> +       default:
> +               printf("MXC USB port %d not yet supported\n", port);
> +               return 1;

return -EINVAL;

> +int board_ehci_power(int port, int on)
> +{
> +       switch (port) {
> +       case 0:
> +               if (on)
> +                       port_exp_direction_output(USB_OTG_PWR, 1);
> +               else
> +                       port_exp_direction_output(USB_OTG_PWR, 0);
> +               break;
> +       case 1:
> +               if (on)
> +                       port_exp_direction_output(USB_HOST1_PWR, 1);
> +               else
> +                       port_exp_direction_output(USB_HOST1_PWR, 0);
> +               break;
> +       default:
> +               printf("MXC USB port %d not yet supported\n", port);
> +               return 1;

return -EINVAL;
Ye.Li Oct. 13, 2014, 5:56 a.m. UTC | #2
On 10/10/2014 7:46 PM, Fabio Estevam wrote:
> On Fri, Oct 10, 2014 at 6:01 AM, Ye.Li <B37916@freescale.com> wrote:
>> On mx6 sabreauto board, there are two USB ports:
>> 0: OTG
>> 1: HOST
>> The EHCI driver is enabled for this board, but the IOMUX and VBUS power
>> control is not implemented, which cause both USB port failed to work.
>> This patch fix the problem by adding the BSP support.
> BSP is a broad term here.
>
>> Since the power control uses the GPIO pin from port expander MAX7310,
>> the PCA953X driver is enabled for accessing the MAX7310.
>>
>> The ID pin of OTG Port needs to configure the GPR1 bit 13 for selecting
>> its daisy chain. Add a new function "imx_iomux_set_gpr_register" to
>> handle GPR register setting.
>>
>> Signed-off-by: Ye.Li <B37916@freescale.com>
>> ---
>>  arch/arm/imx-common/iomux-v3.c                |   17 +++++-
>>  arch/arm/include/asm/imx-common/iomux-v3.h    |    7 ++-
>>  board/freescale/mx6qsabreauto/mx6qsabreauto.c |   89 ++++++++++++++++++++++++-
>>  include/configs/mx6qsabreauto.h               |    5 +-
>>  4 files changed, 114 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/imx-common/iomux-v3.c b/arch/arm/imx-common/iomux-v3.c
>> index 22cd11a..2d96655 100644
>> --- a/arch/arm/imx-common/iomux-v3.c
>> +++ b/arch/arm/imx-common/iomux-v3.c
>> @@ -4,7 +4,7 @@
>>   * Copyright (C) 2009 by Jan Weitzel Phytec Messtechnik GmbH,
>>   *                       <armlinux@phytec.de>
>>   *
>> - * Copyright (C) 2004-2011 Freescale Semiconductor, Inc.
>> + * Copyright (C) 2004-2014 Freescale Semiconductor, Inc.
> I don't see the need for changing the copyright everytime you touch a
> file. Same apply to other files in this patch.
>
>> +static int port_exp_direction_output(unsigned gpio, int value)
>> +{
>> +       int ret;
>> +
>> +       i2c_set_bus_num(2);
>> +       if (i2c_probe(PORTEXP_IO_TO_CHIP(gpio)))
>> +               return -ENXIO;
> Would be better like this:
>
> ret = i2c_probe(PORTEXP_IO_TO_CHIP(gpio))
> if (ret)
>     return ret;
>
>> +
>> +       ret = pca953x_set_dir(PORTEXP_IO_TO_CHIP(gpio),
>> +               (1 << PORTEXP_IO_TO_PIN(gpio)),
>> +               (PCA953X_DIR_OUT << PORTEXP_IO_TO_PIN(gpio)));
> Here you can do:
>
> if (ret)
>   return ret;
>
>> +
>> +       if (!ret)
> Then you don't need to check for !ret here
>
>> +               ret = pca953x_set_val(PORTEXP_IO_TO_CHIP(gpio),
>> +                       (1 << PORTEXP_IO_TO_PIN(gpio)),
>> +                       (value << PORTEXP_IO_TO_PIN(gpio)));
>> +
>> +       if (ret)
>> +               ret = -EIO;
>> +
>> +       return ret;
>> +}
> if (ret)
>    return ret;
>
> return 0;
>
>> +int board_ehci_hcd_init(int port)
>> +{
>> +       switch (port) {
>> +       case 0:
>> +               imx_iomux_v3_setup_multiple_pads(usb_otg_pads,
>> +                       ARRAY_SIZE(usb_otg_pads));
>> +
>> +               /*set daisy chain for otg_pin_id on 6q. for 6dl, this bit is reserved*/
>> +               imx_iomux_set_gpr_register(1, 13, 1, 0);
>> +               break;
>> +       case 1:
>> +               break;
>> +       default:
>> +               printf("MXC USB port %d not yet supported\n", port);
>> +               return 1;
> return -EINVAL;
>
>> +int board_ehci_power(int port, int on)
>> +{
>> +       switch (port) {
>> +       case 0:
>> +               if (on)
>> +                       port_exp_direction_output(USB_OTG_PWR, 1);
>> +               else
>> +                       port_exp_direction_output(USB_OTG_PWR, 0);
>> +               break;
>> +       case 1:
>> +               if (on)
>> +                       port_exp_direction_output(USB_HOST1_PWR, 1);
>> +               else
>> +                       port_exp_direction_output(USB_HOST1_PWR, 0);
>> +               break;
>> +       default:
>> +               printf("MXC USB port %d not yet supported\n", port);
>> +               return 1;
> return -EINVAL;

I will update per your comments.

For the copyright, in FSL we are asked to update it when applying any change to the file. what's the rule used in community?

Best regards,
Ye Li
Fabio Estevam Oct. 13, 2014, 11:03 a.m. UTC | #3
Hi Ye Li,

On Mon, Oct 13, 2014 at 2:56 AM, Li Ye-B37916 <b37916@freescale.com> wrote:

> I will update per your comments.
>
> For the copyright, in FSL we are asked to update it when applying any change to the file. what's the rule used in community?

Yes, that internal script does not make sense. I would say just leave
the copyright lines untouched in patches like this.

Regards,

Fabio Estevam
diff mbox

Patch

diff --git a/arch/arm/imx-common/iomux-v3.c b/arch/arm/imx-common/iomux-v3.c
index 22cd11a..2d96655 100644
--- a/arch/arm/imx-common/iomux-v3.c
+++ b/arch/arm/imx-common/iomux-v3.c
@@ -4,7 +4,7 @@ 
  * Copyright (C) 2009 by Jan Weitzel Phytec Messtechnik GmbH,
  *                       <armlinux@phytec.de>
  *
- * Copyright (C) 2004-2011 Freescale Semiconductor, Inc.
+ * Copyright (C) 2004-2014 Freescale Semiconductor, Inc.
  *
  * SPDX-License-Identifier:	GPL-2.0+
  */
@@ -77,3 +77,18 @@  void imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t const *pad_list,
 		p += stride;
 	}
 }
+
+void imx_iomux_set_gpr_register(int group, int start_bit,
+					int num_bits, int value)
+{
+	int i = 0;
+	u32 reg;
+	reg = readl(base + group * 4);
+	while (num_bits) {
+		reg &= ~(1<<(start_bit + i));
+		i++;
+		num_bits--;
+	}
+	reg |= (value << start_bit);
+	writel(reg, base + group * 4);
+}
diff --git a/arch/arm/include/asm/imx-common/iomux-v3.h b/arch/arm/include/asm/imx-common/iomux-v3.h
index 70ee86c..bc090fd 100644
--- a/arch/arm/include/asm/imx-common/iomux-v3.h
+++ b/arch/arm/include/asm/imx-common/iomux-v3.h
@@ -3,7 +3,7 @@ 
  * Copyright (C) 2009 by Jan Weitzel Phytec Messtechnik GmbH,
  *			<armlinux@phytec.de>
  *
- * Copyright (C) 2011 Freescale Semiconductor, Inc.
+ * Copyright (C) 2011-2014 Freescale Semiconductor, Inc.
  *
  * SPDX-License-Identifier:	GPL-2.0+
  */
@@ -180,6 +180,11 @@  typedef u64 iomux_v3_cfg_t;
 void imx_iomux_v3_setup_pad(iomux_v3_cfg_t pad);
 void imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t const *pad_list,
 				     unsigned count);
+/*
+* Set bits for general purpose registers
+*/
+void imx_iomux_set_gpr_register(int group, int start_bit,
+					 int num_bits, int value);
 
 /* macros for declaring and using pinmux array */
 #if defined(CONFIG_MX6QDL)
diff --git a/board/freescale/mx6qsabreauto/mx6qsabreauto.c b/board/freescale/mx6qsabreauto/mx6qsabreauto.c
index dd6d2a6..e48b08a 100644
--- a/board/freescale/mx6qsabreauto/mx6qsabreauto.c
+++ b/board/freescale/mx6qsabreauto/mx6qsabreauto.c
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ * Copyright (C) 2012-2014 Freescale Semiconductor, Inc.
  *
  * Author: Fabio Estevam <fabio.estevam@freescale.com>
  *
@@ -23,6 +23,7 @@ 
 #include <netdev.h>
 #include <asm/arch/sys_proto.h>
 #include <i2c.h>
+#include <pca953x.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -112,6 +113,41 @@  static iomux_v3_cfg_t const port_exp[] = {
 	MX6_PAD_SD2_DAT0__GPIO1_IO15		| MUX_PAD_CTRL(NO_PAD_CTRL),
 };
 
+/*Define for building port exp gpio, pin starts from 0*/
+#define PORTEXP_IO_NR(chip, pin) \
+	((chip << 5) + pin)
+
+/*Get the chip addr from a ioexp gpio*/
+#define PORTEXP_IO_TO_CHIP(gpio_nr) \
+	(gpio_nr >> 5)
+
+/*Get the pin number from a ioexp gpio*/
+#define PORTEXP_IO_TO_PIN(gpio_nr) \
+	(gpio_nr & 0x1f)
+
+static int port_exp_direction_output(unsigned gpio, int value)
+{
+	int ret;
+
+	i2c_set_bus_num(2);
+	if (i2c_probe(PORTEXP_IO_TO_CHIP(gpio)))
+		return -ENXIO;
+
+	ret = pca953x_set_dir(PORTEXP_IO_TO_CHIP(gpio),
+		(1 << PORTEXP_IO_TO_PIN(gpio)),
+		(PCA953X_DIR_OUT << PORTEXP_IO_TO_PIN(gpio)));
+
+	if (!ret)
+		ret = pca953x_set_val(PORTEXP_IO_TO_CHIP(gpio),
+			(1 << PORTEXP_IO_TO_PIN(gpio)),
+			(value << PORTEXP_IO_TO_PIN(gpio)));
+
+	if (ret)
+		ret = -EIO;
+
+	return ret;
+}
+
 static void setup_iomux_enet(void)
 {
 	imx_iomux_v3_setup_multiple_pads(enet_pads, ARRAY_SIZE(enet_pads));
@@ -295,3 +331,54 @@  int checkboard(void)
 
 	return 0;
 }
+
+#ifdef CONFIG_USB_EHCI_MX6
+#define USB_HOST1_PWR     PORTEXP_IO_NR(0x32, 7)
+#define USB_OTG_PWR       PORTEXP_IO_NR(0x34, 1)
+
+iomux_v3_cfg_t const usb_otg_pads[] = {
+	MX6_PAD_ENET_RX_ER__USB_OTG_ID | MUX_PAD_CTRL(NO_PAD_CTRL),
+};
+
+int board_ehci_hcd_init(int port)
+{
+	switch (port) {
+	case 0:
+		imx_iomux_v3_setup_multiple_pads(usb_otg_pads,
+			ARRAY_SIZE(usb_otg_pads));
+
+		/*set daisy chain for otg_pin_id on 6q. for 6dl, this bit is reserved*/
+		imx_iomux_set_gpr_register(1, 13, 1, 0);
+		break;
+	case 1:
+		break;
+	default:
+		printf("MXC USB port %d not yet supported\n", port);
+		return 1;
+	}
+	return 0;
+}
+
+int board_ehci_power(int port, int on)
+{
+	switch (port) {
+	case 0:
+		if (on)
+			port_exp_direction_output(USB_OTG_PWR, 1);
+		else
+			port_exp_direction_output(USB_OTG_PWR, 0);
+		break;
+	case 1:
+		if (on)
+			port_exp_direction_output(USB_HOST1_PWR, 1);
+		else
+			port_exp_direction_output(USB_HOST1_PWR, 0);
+		break;
+	default:
+		printf("MXC USB port %d not yet supported\n", port);
+		return 1;
+	}
+
+	return 0;
+}
+#endif
diff --git a/include/configs/mx6qsabreauto.h b/include/configs/mx6qsabreauto.h
index 0ab3127..4c76db0 100644
--- a/include/configs/mx6qsabreauto.h
+++ b/include/configs/mx6qsabreauto.h
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ * Copyright (C) 2012-2014 Freescale Semiconductor, Inc.
  *
  * Configuration settings for the Freescale i.MX6Q SabreAuto board.
  *
@@ -32,6 +32,9 @@ 
 #define CONFIG_MXC_USB_PORTSC	(PORT_PTS_UTMI | PORT_PTS_PTW)
 #define CONFIG_MXC_USB_FLAGS	0
 
+#define CONFIG_PCA953X
+#define CONFIG_SYS_I2C_PCA953X_WIDTH	{ {0x30, 8}, {0x32, 8}, {0x34, 8} }
+
 #include "mx6sabre_common.h"
 
 #define CONFIG_SYS_FSL_USDHC_NUM	2