[PATCH v9 1/6] davinci: EMAC support for Omapl138-Hawkboard

[PATCH v9 1/6] davinci: EMAC support for Omapl138-Hawkboard

[vm.rod25 at gmail.com]

From: Victor Rodriguez <victor.rodriguez@sasken.com>

This patch adds EMAC support for the Hawkboard-L138 system

Signed-off-by: Victor Rodriguez <victor.rodriguez@sasken.com>
Tested-by: Rene Gonzalez <renegs.2378@gmail.com>
---
 arch/arm/mach-davinci/board-omapl138-hawk.c |   43 +++++++++++++++++++++++++++
 1 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index 0b8dbdb..75b3fe2 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -19,6 +19,47 @@
 
 #include <mach/cp_intc.h>
 #include <mach/da8xx.h>
+#include <mach/mux.h>
+
+#define HAWKBOARD_PHY_ID		"0:07"
+
+static short omapl138_hawk_mii_pins[] __initdata = {
+	DA850_MII_TXEN, DA850_MII_TXCLK, DA850_MII_COL, DA850_MII_TXD_3,
+	DA850_MII_TXD_2, DA850_MII_TXD_1, DA850_MII_TXD_0, DA850_MII_RXER,
+	DA850_MII_CRS, DA850_MII_RXCLK, DA850_MII_RXDV, DA850_MII_RXD_3,
+	DA850_MII_RXD_2, DA850_MII_RXD_1, DA850_MII_RXD_0, DA850_MDIO_CLK,
+	DA850_MDIO_D,
+	-1
+};
+
+static __init void omapl138_hawk_config_emac(void)
+{
+	void __iomem *cfgchip3 = DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP3_REG);
+	int ret;
+	u32 val;
+	struct davinci_soc_info *soc_info = &davinci_soc_info;
+
+	val = __raw_readl(cfgchip3);
+	val &= ~BIT(8);
+	ret = davinci_cfg_reg_list(omapl138_hawk_mii_pins);
+	if (ret) {
+		pr_warning("%s: cpgmac/mii mux setup failed: %d\n",
+			__func__, ret);
+		return;
+	}
+
+	/* configure the CFGCHIP3 register for MII */
+	__raw_writel(val, cfgchip3);
+	pr_info("EMAC: MII PHY configured\n");
+
+	soc_info->emac_pdata->phy_id = HAWKBOARD_PHY_ID;
+
+	ret = da8xx_register_emac();
+	if (ret)
+		pr_warning("%s: emac registration failed: %d\n",
+			__func__, ret);
+}
+
 
 static struct davinci_uart_config omapl138_hawk_uart_config __initdata = {
 	.enabled_uarts = 0x7,
@@ -30,6 +71,8 @@ static __init void omapl138_hawk_init(void)
 
 	davinci_serial_init(&omapl138_hawk_uart_config);
 
+	omapl138_hawk_config_emac();
+
 	ret = da8xx_register_watchdog();
 	if (ret)
 		pr_warning("omapl138_hawk_init: "
-- 
1.7.0.4

[PATCH v9 2/6] davinci: EDMA support for Omapl138-Hawkboard

[vm.rod25 at gmail.com]

From: Victor Rodriguez <victor.rodriguez@sasken.com>

This patch adds EDMA support for the Hawkboard-L138 system

Signed-off-by: Victor Rodriguez <victor.rodriguez@sasken.com>
Tested-by: Rene Gonzalez <renegs.2378@gmail.com>
---
 arch/arm/mach-davinci/board-omapl138-hawk.c |   54 +++++++++++++++++++++++++++
 1 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index 75b3fe2..cefff9b 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -60,6 +60,55 @@ static __init void omapl138_hawk_config_emac(void)
 			__func__, ret);
 }
 
+/*
+ * The following EDMA channels/slots are not being used by drivers (for
+ * example: Timer, GPIO, UART events etc) on da850/omap-l138 EVM/Hawkboard,
+ * hence they are being reserved for codecs on the DSP side.
+ */
+static const s16 da850_dma0_rsv_chans[][2] = {
+	/* (offset, number) */
+	{ 8,  6},
+	{24,  4},
+	{30,  2},
+	{-1, -1}
+};
+
+static const s16 da850_dma0_rsv_slots[][2] = {
+	/* (offset, number) */
+	{ 8,  6},
+	{24,  4},
+	{30, 50},
+	{-1, -1}
+};
+
+static const s16 da850_dma1_rsv_chans[][2] = {
+	/* (offset, number) */
+	{ 0, 28},
+	{30,  2},
+	{-1, -1}
+};
+
+static const s16 da850_dma1_rsv_slots[][2] = {
+	/* (offset, number) */
+	{ 0, 28},
+	{30, 90},
+	{-1, -1}
+};
+
+static struct edma_rsv_info da850_edma_cc0_rsv = {
+	.rsv_chans	= da850_dma0_rsv_chans,
+	.rsv_slots	= da850_dma0_rsv_slots,
+};
+
+static struct edma_rsv_info da850_edma_cc1_rsv = {
+	.rsv_chans	= da850_dma1_rsv_chans,
+	.rsv_slots	= da850_dma1_rsv_slots,
+};
+
+static struct edma_rsv_info *da850_edma_rsv[2] = {
+	&da850_edma_cc0_rsv,
+	&da850_edma_cc1_rsv,
+};
 
 static struct davinci_uart_config omapl138_hawk_uart_config __initdata = {
 	.enabled_uarts = 0x7,
@@ -73,6 +122,11 @@ static __init void omapl138_hawk_init(void)
 
 	omapl138_hawk_config_emac();
 
+	ret = da850_register_edma(da850_edma_rsv);
+	if (ret)
+		pr_warning("%s: EDMA registration failed: %d\n",
+			__func__, ret);
+
 	ret = da8xx_register_watchdog();
 	if (ret)
 		pr_warning("omapl138_hawk_init: "
-- 
1.7.0.4

[PATCH v9 3/6] davinci: MMC/SD and USB-OHCI configuration for Omapl138-Hawkboard

[vm.rod25 at gmail.com]

From: Victor Rodriguez <victor.rodriguez@sasken.com>

This patch defines Pin Mux configuration to enable MMC/SD
and USB-OHCI on the Hawkboard-L138 system

Signed-off-by: Victor Rodriguez <victor.rodriguez@sasken.com>
Tested-by: Rene Gonzalez <renegs.2378@gmail.com>
---
 arch/arm/mach-davinci/da850.c            |    4 ++++
 arch/arm/mach-davinci/include/mach/mux.h |    4 ++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 63916b9..80803fa 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -543,11 +543,15 @@ static const struct mux_config da850_pins[] = {
 	MUX_CFG(DA850, EMA_WAIT_1,	6,	24,	15,	1,	false)
 	MUX_CFG(DA850, NEMA_CS_2,	7,	0,	15,	1,	false)
 	/* GPIO function */
+	MUX_CFG(DA850, GPIO2_4,		6,	12,	15,	8,	false)
 	MUX_CFG(DA850, GPIO2_6,		6,	4,	15,	8,	false)
 	MUX_CFG(DA850, GPIO2_8,		5,	28,	15,	8,	false)
 	MUX_CFG(DA850, GPIO2_15,	5,	0,	15,	8,	false)
+	MUX_CFG(DA850, GPIO3_12,	7,	12,	15,	8,	false)
+	MUX_CFG(DA850, GPIO3_13,	7,	8,	15,	8,	false)
 	MUX_CFG(DA850, GPIO4_0,		10,	28,	15,	8,	false)
 	MUX_CFG(DA850, GPIO4_1,		10,	24,	15,	8,	false)
+	MUX_CFG(DA850, GPIO6_13,	13,	8,	15,	8,	false)
 	MUX_CFG(DA850, RTC_ALARM,	0,	28,	15,	2,	false)
 #endif
 };
diff --git a/arch/arm/mach-davinci/include/mach/mux.h b/arch/arm/mach-davinci/include/mach/mux.h
index de11aac..5d4e0fe 100644
--- a/arch/arm/mach-davinci/include/mach/mux.h
+++ b/arch/arm/mach-davinci/include/mach/mux.h
@@ -908,11 +908,15 @@ enum davinci_da850_index {
 	DA850_NEMA_CS_2,
 
 	/* GPIO function */
+	DA850_GPIO2_4,
 	DA850_GPIO2_6,
 	DA850_GPIO2_8,
 	DA850_GPIO2_15,
+	DA850_GPIO3_12,
+	DA850_GPIO3_13,
 	DA850_GPIO4_0,
 	DA850_GPIO4_1,
+	DA850_GPIO6_13,
 	DA850_RTC_ALARM,
 };
 
-- 
1.7.0.4

[PATCH v9 4/6] davinci: MMC/SD support for Omapl138-Hawkboard

[vm.rod25 at gmail.com]

From: Victor Rodriguez <victor.rodriguez@sasken.com>

This patch adds MMC/SD support for the Hawkboard-L138 system

Signed-off-by: Victor Rodriguez <victor.rodriguez@sasken.com>
Tested-by: Rene Gonzalez <renegs.2378@gmail.com>
---
Notes:

This patch works with da8xx_omapl_defconfig
In order to test it select in menuconfig like insmodule

MMC/SD/SDIO card support  --->
      MMC block device driver
      Use bounce buffer for simple hosts
      TI DAVINCI Multimedia Card Interface support

 arch/arm/mach-davinci/board-omapl138-hawk.c |   63 +++++++++++++++++++++++++++
 1 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index cefff9b..d0c853f 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -22,6 +22,8 @@
 #include <mach/mux.h>
 
 #define HAWKBOARD_PHY_ID		"0:07"
+#define DA850_HAWK_MMCSD_CD_PIN		GPIO_TO_PIN(3, 12)
+#define DA850_HAWK_MMCSD_WP_PIN		GPIO_TO_PIN(3, 13)
 
 static short omapl138_hawk_mii_pins[] __initdata = {
 	DA850_MII_TXEN, DA850_MII_TXCLK, DA850_MII_COL, DA850_MII_TXD_3,
@@ -110,6 +112,65 @@ static struct edma_rsv_info *da850_edma_rsv[2] = {
 	&da850_edma_cc1_rsv,
 };
 
+static const short hawk_mmcsd0_pins[] = {
+	DA850_MMCSD0_DAT_0, DA850_MMCSD0_DAT_1, DA850_MMCSD0_DAT_2,
+	DA850_MMCSD0_DAT_3, DA850_MMCSD0_CLK, DA850_MMCSD0_CMD,
+	DA850_GPIO3_12, DA850_GPIO3_13,
+	-1
+};
+
+static int da850_hawk_mmc_get_ro(int index)
+{
+	return gpio_get_value(DA850_HAWK_MMCSD_WP_PIN);
+}
+
+static int da850_hawk_mmc_get_cd(int index)
+{
+	return !gpio_get_value(DA850_HAWK_MMCSD_CD_PIN);
+}
+
+static struct davinci_mmc_config da850_mmc_config = {
+	.get_ro		= da850_hawk_mmc_get_ro,
+	.get_cd		= da850_hawk_mmc_get_cd,
+	.wires		= 4,
+	.max_freq	= 50000000,
+	.caps		= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED,
+	.version	= MMC_CTLR_VERSION_2,
+};
+
+static __init void omapl138_hawk_mmc_init(void)
+{
+	int ret;
+
+	ret = davinci_cfg_reg_list(hawk_mmcsd0_pins);
+	if (ret) {
+		pr_warning("%s: MMC/SD0 mux setup failed: %d\n",
+			__func__, ret);
+		return;
+	}
+
+	ret = gpio_request_one(DA850_HAWK_MMCSD_CD_PIN,
+			GPIOF_DIR_IN, "MMC CD");
+	if (ret < 0) {
+		pr_warning("%s: can not open GPIO %d\n",
+			__func__, DA850_HAWK_MMCSD_CD_PIN);
+		return;
+	}
+
+	ret = gpio_request_one(DA850_HAWK_MMCSD_WP_PIN,
+			GPIOF_DIR_IN, "MMC WP");
+	if (ret < 0) {
+		pr_warning("%s: can not open GPIO %d\n",
+			__func__, DA850_HAWK_MMCSD_WP_PIN);
+		return;
+	}
+
+	ret = da8xx_register_mmcsd0(&da850_mmc_config);
+	if (ret)
+		pr_warning("%s: MMC/SD0 registration failed: %d\n",
+			__func__, ret);
+}
+
 static struct davinci_uart_config omapl138_hawk_uart_config __initdata = {
 	.enabled_uarts = 0x7,
 };
@@ -127,6 +188,8 @@ static __init void omapl138_hawk_init(void)
 		pr_warning("%s: EDMA registration failed: %d\n",
 			__func__, ret);
 
+	omapl138_hawk_mmc_init();
+
 	ret = da8xx_register_watchdog();
 	if (ret)
 		pr_warning("omapl138_hawk_init: "
-- 
1.7.0.4

[PATCH v9 5/6] davinci: USB clocks for Omapl138-Hawkboard

[vm.rod25 at gmail.com]

From: Victor Rodriguez <victor.rodriguez@sasken.com>

This patch adds USB1.1 and USB2.0 clocks for the Hawkboard-L138 system

Signed-off-by: Victor Rodriguez <victor.rodriguez@sasken.com>
Tested-by: Rene Gonzalez <renegs.2378@gmail.com>
---
 arch/arm/mach-davinci/da850.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 80803fa..93a4df4 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -345,6 +345,20 @@ static struct clk aemif_clk = {
 	.flags		= ALWAYS_ENABLED,
 };
 
+static struct clk usb11_clk = {
+	.name		= "usb11",
+	.parent		= &pll0_sysclk4,
+	.lpsc		= DA8XX_LPSC1_USB11,
+	.gpsc		= 1,
+};
+
+static struct clk usb20_clk = {
+	.name		= "usb20",
+	.parent		= &pll0_sysclk2,
+	.lpsc		= DA8XX_LPSC1_USB20,
+	.gpsc		= 1,
+};
+
 static struct clk_lookup da850_clks[] = {
 	CLK(NULL,		"ref",		&ref_clk),
 	CLK(NULL,		"pll0",		&pll0_clk),
@@ -387,6 +401,8 @@ static struct clk_lookup da850_clks[] = {
 	CLK("davinci_mmc.0",	NULL,		&mmcsd0_clk),
 	CLK("davinci_mmc.1",	NULL,		&mmcsd1_clk),
 	CLK(NULL,		"aemif",	&aemif_clk),
+	CLK(NULL,		"usb11",	&usb11_clk),
+	CLK(NULL,		"usb20",	&usb20_clk),
 	CLK(NULL,		NULL,		NULL),
 };
 
-- 
1.7.0.4

[PATCH v9 6/6] davinci: USB1.1 support for Omapl138-Hawkboard

[vm.rod25 at gmail.com]

From: Victor Rodriguez <victor.rodriguez@sasken.com>

This patch adds USB1.1 support for the Hawkboard-L138 system

Signed-off-by: Victor Rodriguez <victor.rodriguez@sasken.com>
Tested-by: Rene Gonzalez <renegs.2378@gmail.com>
---
Notes:

This patch works with da8xx_omapl_defconfig
In order to test it select in menuconfig like insmodule

 Device Drivers  --->
    SCSI device support  --->
       SCSI device support
       legacy /proc/scsi/ support
       SCSI disk support
       SCSI low-level drivers

       USB support  --->
               Support for Host-side USB
               OHCI HCD support (NEW)
               USB Mass Storage support (NEW)
               USB Gadget Support  --->
               USB Gadget Drivers (Ethernet Gadget\
                (with CDC Ethernet support))  --->
               NOP USB Transceiver Driver

And you will be able to mount and USB pen drive

In order to connect a keyboard or a mouse on a USB-hub
select in menuconfig like insmodule

HID Devices  --->
       Generic HID support
       USB Human Interface Device (full HID) support

 arch/arm/mach-davinci/board-omapl138-hawk.c |  106 +++++++++++++++++++++++++++
 1 files changed, 106 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index d0c853f..da51136 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -25,6 +25,9 @@
 #define DA850_HAWK_MMCSD_CD_PIN		GPIO_TO_PIN(3, 12)
 #define DA850_HAWK_MMCSD_WP_PIN		GPIO_TO_PIN(3, 13)
 
+#define DA850_USB1_VBUS_PIN		GPIO_TO_PIN(2, 4)
+#define DA850_USB1_OC_PIN		GPIO_TO_PIN(6, 13)
+
 static short omapl138_hawk_mii_pins[] __initdata = {
 	DA850_MII_TXEN, DA850_MII_TXCLK, DA850_MII_COL, DA850_MII_TXD_3,
 	DA850_MII_TXD_2, DA850_MII_TXD_1, DA850_MII_TXD_0, DA850_MII_RXER,
@@ -171,6 +174,107 @@ static __init void omapl138_hawk_mmc_init(void)
 			__func__, ret);
 }
 
+static irqreturn_t omapl138_hawk_usb_ocic_irq(int irq, void *dev_id);
+static da8xx_ocic_handler_t hawk_usb_ocic_handler;
+
+static const short da850_hawk_usb11_pins[] = {
+	DA850_GPIO2_4, DA850_GPIO6_13,
+	-1
+};
+
+static int hawk_usb_set_power(unsigned port, int on)
+{
+	gpio_set_value(DA850_USB1_VBUS_PIN, on);
+	return 0;
+}
+
+static int hawk_usb_get_power(unsigned port)
+{
+	return gpio_get_value(DA850_USB1_VBUS_PIN);
+}
+
+static int hawk_usb_get_oci(unsigned port)
+{
+	return !gpio_get_value(DA850_USB1_OC_PIN);
+}
+
+static int hawk_usb_ocic_notify(da8xx_ocic_handler_t handler)
+{
+	int irq         = gpio_to_irq(DA850_USB1_OC_PIN);
+	int error       = 0;
+
+	if (handler != NULL) {
+		hawk_usb_ocic_handler = handler;
+
+		error = request_irq(irq, omapl138_hawk_usb_ocic_irq,
+					IRQF_DISABLED | IRQF_TRIGGER_RISING |
+					IRQF_TRIGGER_FALLING,
+					"OHCI over-current indicator", NULL);
+		if (error)
+			pr_err(KERN_ERR "%s: could not request IRQ to watch "
+				"over-current indicator changes\n", __func__);
+	} else {
+		free_irq(irq, NULL);
+	}
+	return error;
+}
+
+static struct da8xx_ohci_root_hub omapl138_hawk_usb11_pdata = {
+	.set_power      = hawk_usb_set_power,
+	.get_power      = hawk_usb_get_power,
+	.get_oci        = hawk_usb_get_oci,
+	.ocic_notify    = hawk_usb_ocic_notify,
+	/* TPS2087 switch @ 5V */
+	.potpgt         = (3 + 1) / 2,  /* 3 ms max */
+};
+
+static irqreturn_t omapl138_hawk_usb_ocic_irq(int irq, void *dev_id)
+{
+	hawk_usb_ocic_handler(&omapl138_hawk_usb11_pdata, 1);
+	return IRQ_HANDLED;
+}
+
+static __init void omapl138_hawk_usb_init(void)
+{
+	int ret;
+	u32 cfgchip2;
+
+	ret = davinci_cfg_reg_list(da850_hawk_usb11_pins);
+	if (ret) {
+		pr_warning("%s: USB 1.1 PinMux setup failed: %d\n",
+			__func__, ret);
+		return;
+	}
+
+	/* Setup the Ref. clock frequency for the HAWK at 24 MHz. */
+
+	cfgchip2 = __raw_readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+	cfgchip2 &= ~CFGCHIP2_REFFREQ;
+	cfgchip2 |=  CFGCHIP2_REFFREQ_24MHZ;
+	__raw_writel(cfgchip2, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+
+	ret = gpio_request_one(DA850_USB1_VBUS_PIN,
+			GPIOF_DIR_OUT, "USB1 VBUS");
+	if (ret < 0) {
+		pr_err(KERN_ERR "%s: failed to request GPIO for USB 1.1 port "
+			"power control: %d\n", __func__, ret);
+		return;
+	}
+
+	ret = gpio_request_one(DA850_USB1_OC_PIN,
+			GPIOF_DIR_IN, "USB1 OC");
+	if (ret < 0) {
+		pr_err(KERN_ERR "%s: failed to request GPIO for USB 1.1 port "
+			"over-current indicator: %d\n", __func__, ret);
+		return;
+	}
+
+	ret = da8xx_register_usb11(&omapl138_hawk_usb11_pdata);
+	if (ret)
+		pr_warning("%s: USB 1.1 registration failed: %d\n",
+			__func__, ret);
+}
+
 static struct davinci_uart_config omapl138_hawk_uart_config __initdata = {
 	.enabled_uarts = 0x7,
 };
@@ -190,6 +294,8 @@ static __init void omapl138_hawk_init(void)
 
 	omapl138_hawk_mmc_init();
 
+	omapl138_hawk_usb_init();
+
 	ret = da8xx_register_watchdog();
 	if (ret)
 		pr_warning("omapl138_hawk_init: "
-- 
1.7.0.4

[PATCH v9 6/6] davinci: USB1.1 support for Omapl138-Hawkboard

[Nori, Sekhar]

On Thu, Dec 02, 2010 at 01:02:29, vm.rod25 at gmail.com wrote:
> +static int hawk_usb_ocic_notify(da8xx_ocic_handler_t handler)
> +{
> +     int irq         = gpio_to_irq(DA850_USB1_OC_PIN);
> +     int error       = 0;
> +
> +     if (handler != NULL) {
> +             hawk_usb_ocic_handler = handler;
> +
> +             error = request_irq(irq, omapl138_hawk_usb_ocic_irq,
> +                                     IRQF_DISABLED | IRQF_TRIGGER_RISING |
> +                                     IRQF_TRIGGER_FALLING,
> +                                     "OHCI over-current indicator", NULL);
> +             if (error)
> +                     pr_err(KERN_ERR "%s: could not request IRQ to watch "
> +                             "over-current indicator changes\n", __func__);

pr_err adds a KERN_ERR already.

> +static __init void omapl138_hawk_usb_init(void)
> +{
> +     int ret;
> +     u32 cfgchip2;
> +
> +     ret = davinci_cfg_reg_list(da850_hawk_usb11_pins);
> +     if (ret) {
> +             pr_warning("%s: USB 1.1 PinMux setup failed: %d\n",
> +                     __func__, ret);
> +             return;
> +     }
> +
> +     /* Setup the Ref. clock frequency for the HAWK at 24 MHz. */
> +
> +     cfgchip2 = __raw_readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> +     cfgchip2 &= ~CFGCHIP2_REFFREQ;
> +     cfgchip2 |=  CFGCHIP2_REFFREQ_24MHZ;
> +     __raw_writel(cfgchip2, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> +
> +     ret = gpio_request_one(DA850_USB1_VBUS_PIN,
> +                     GPIOF_DIR_OUT, "USB1 VBUS");
> +     if (ret < 0) {
> +             pr_err(KERN_ERR "%s: failed to request GPIO for USB 1.1 port "
> +                     "power control: %d\n", __func__, ret);
> +             return;
> +     }
> +
> +     ret = gpio_request_one(DA850_USB1_OC_PIN,
> +                     GPIOF_DIR_IN, "USB1 OC");
> +     if (ret < 0) {
> +             pr_err(KERN_ERR "%s: failed to request GPIO for USB 1.1 port "
> +                     "over-current indicator: %d\n", __func__, ret);
> +             return;
> +     }

Should free the gpio DA850_USB1_VBUS_PIN in this error
path. This is also valid for MMC/SD gpio acquisition in
patch 4/6.

Thanks,
Sekhar

[PATCH v9 6/6] davinci: USB1.1 support for Omapl138-Hawkboard

[Victor Rodriguez]

On Thu, Dec 2, 2010 at 12:49 AM, Nori, Sekhar <nsekhar@ti.com> wrote:
> On Thu, Dec 02, 2010 at 01:02:29, vm.rod25 at gmail.com wrote:
>> +static int hawk_usb_ocic_notify(da8xx_ocic_handler_t handler)
>> +{
>> + ? ? int irq ? ? ? ? = gpio_to_irq(DA850_USB1_OC_PIN);
>> + ? ? int error ? ? ? = 0;
>> +
>> + ? ? if (handler != NULL) {
>> + ? ? ? ? ? ? hawk_usb_ocic_handler = handler;
>> +
>> + ? ? ? ? ? ? error = request_irq(irq, omapl138_hawk_usb_ocic_irq,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? IRQF_DISABLED | IRQF_TRIGGER_RISING |
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? IRQF_TRIGGER_FALLING,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "OHCI over-current indicator", NULL);
>> + ? ? ? ? ? ? if (error)
>> + ? ? ? ? ? ? ? ? ? ? pr_err(KERN_ERR "%s: could not request IRQ to watch "
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? "over-current indicator changes\n", __func__);
>
> pr_err adds a KERN_ERR already.

Changed to this

static int hawk_usb_ocic_notify(da8xx_ocic_handler_t handler)
{
	int irq         = gpio_to_irq(DA850_USB1_OC_PIN);
	int error       = 0;

	if (handler != NULL) {
		hawk_usb_ocic_handler = handler;

		error = request_irq(irq, omapl138_hawk_usb_ocic_irq,
					IRQF_DISABLED | IRQF_TRIGGER_RISING |
					IRQF_TRIGGER_FALLING,
					"OHCI over-current indicator", NULL);
		if (error)
			pr_err("%s: could not request IRQ to watch "
				"over-current indicator changes\n", __func__);
	} else {
		free_irq(irq, NULL);
	}
	return error;
}



>> +static __init void omapl138_hawk_usb_init(void)
>> +{
>> + ? ? int ret;
>> + ? ? u32 cfgchip2;
>> +
>> + ? ? ret = davinci_cfg_reg_list(da850_hawk_usb11_pins);
>> + ? ? if (ret) {
>> + ? ? ? ? ? ? pr_warning("%s: USB 1.1 PinMux setup failed: %d\n",
>> + ? ? ? ? ? ? ? ? ? ? __func__, ret);
>> + ? ? ? ? ? ? return;
>> + ? ? }
>> +
>> + ? ? /* Setup the Ref. clock frequency for the HAWK at 24 MHz. */
>> +
>> + ? ? cfgchip2 = __raw_readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
>> + ? ? cfgchip2 &= ~CFGCHIP2_REFFREQ;
>> + ? ? cfgchip2 |= ?CFGCHIP2_REFFREQ_24MHZ;
>> + ? ? __raw_writel(cfgchip2, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
>> +
>> + ? ? ret = gpio_request_one(DA850_USB1_VBUS_PIN,
>> + ? ? ? ? ? ? ? ? ? ? GPIOF_DIR_OUT, "USB1 VBUS");
>> + ? ? if (ret < 0) {
>> + ? ? ? ? ? ? pr_err(KERN_ERR "%s: failed to request GPIO for USB 1.1 port "
>> + ? ? ? ? ? ? ? ? ? ? "power control: %d\n", __func__, ret);
>> + ? ? ? ? ? ? return;
>> + ? ? }
>> +
>> + ? ? ret = gpio_request_one(DA850_USB1_OC_PIN,
>> + ? ? ? ? ? ? ? ? ? ? GPIOF_DIR_IN, "USB1 OC");
>> + ? ? if (ret < 0) {
>> + ? ? ? ? ? ? pr_err(KERN_ERR "%s: failed to request GPIO for USB 1.1 port "
>> + ? ? ? ? ? ? ? ? ? ? "over-current indicator: %d\n", __func__, ret);
>> + ? ? ? ? ? ? return;
>> + ? ? }
>
> Should free the gpio DA850_USB1_VBUS_PIN in this error
> path. This is also valid for MMC/SD gpio acquisition in
> patch 4/6.

USB part

Changed to this

	if (ret < 0) {
		pr_err("%s: failed to request GPIO for USB 1.1 port "
			"power control: %d\n", __func__, ret);
		gpio_free(DA850_USB1_VBUS_PIN);
		return;
	}

	ret = gpio_request_one(DA850_USB1_OC_PIN,
			GPIOF_DIR_IN, "USB1 OC");
	if (ret < 0) {
		pr_err("%s: failed to request GPIO for USB 1.1 port "
			"over-current indicator: %d\n", __func__, ret);
		gpio_free(DA850_USB1_OC_PIN);
		return;
	}

Patch 4/6

	ret = gpio_request_one(DA850_HAWK_MMCSD_CD_PIN,
			GPIOF_DIR_IN, "MMC CD");
	if (ret < 0) {
		pr_warning("%s: can not open GPIO %d\n",
			__func__, DA850_HAWK_MMCSD_CD_PIN);
		gpio_free(DA850_HAWK_MMCSD_CD_PIN);
		return;
	}

	ret = gpio_request_one(DA850_HAWK_MMCSD_WP_PIN,
			GPIOF_DIR_IN, "MMC WP");
	if (ret < 0) {
		pr_warning("%s: can not open GPIO %d\n",
			__func__, DA850_HAWK_MMCSD_WP_PIN);
		gpio_free(DA850_HAWK_MMCSD_WP_PIN);
		return;
	}


Is this ok ? or should I free the GPIO on the next section ? Same with USB

	ret = da8xx_register_mmcsd0(&da850_mmc_config);
	if (ret)
		pr_warning("%s: MMC/SD0 registration failed: %d\n",
			__func__, ret);

Thanks for your comments

Regards

Victor Rodriguez



> Thanks,
> Sekhar
>
>

[PATCH v9 6/6] davinci: USB1.1 support for Omapl138-Hawkboard

[Nori, Sekhar]

On Fri, Dec 03, 2010 at 06:14:39, Victor Rodriguez wrote:
> On Thu, Dec 2, 2010 at 12:49 AM, Nori, Sekhar <nsekhar@ti.com> wrote:
> > On Thu, Dec 02, 2010 at 01:02:29, vm.rod25 at gmail.com wrote:
> >> +static int hawk_usb_ocic_notify(da8xx_ocic_handler_t handler)
> >> +{
> >> +     int irq         = gpio_to_irq(DA850_USB1_OC_PIN);
> >> +     int error       = 0;
> >> +
> >> +     if (handler != NULL) {
> >> +             hawk_usb_ocic_handler = handler;
> >> +
> >> +             error = request_irq(irq, omapl138_hawk_usb_ocic_irq,
> >> +                                     IRQF_DISABLED | IRQF_TRIGGER_RISING |
> >> +                                     IRQF_TRIGGER_FALLING,
> >> +                                     "OHCI over-current indicator", NULL);
> >> +             if (error)
> >> +                     pr_err(KERN_ERR "%s: could not request IRQ to watch "
> >> +                             "over-current indicator changes\n", __func__);
> >
> > pr_err adds a KERN_ERR already.
>
> Changed to this
>
> static int hawk_usb_ocic_notify(da8xx_ocic_handler_t handler)
> {
>       int irq         = gpio_to_irq(DA850_USB1_OC_PIN);
>       int error       = 0;
>
>       if (handler != NULL) {
>               hawk_usb_ocic_handler = handler;
>
>               error = request_irq(irq, omapl138_hawk_usb_ocic_irq,
>                                       IRQF_DISABLED | IRQF_TRIGGER_RISING |
>                                       IRQF_TRIGGER_FALLING,
>                                       "OHCI over-current indicator", NULL);
>               if (error)
>                       pr_err("%s: could not request IRQ to watch "
>                               "over-current indicator changes\n", __func__);

Looks good. Thanks. The same error is present elsewhere in the
patch and other patches too. Please fix those too.

>
> USB part
>
> Changed to this
>
>       if (ret < 0) {
>               pr_err("%s: failed to request GPIO for USB 1.1 port "
>                       "power control: %d\n", __func__, ret);
>               gpio_free(DA850_USB1_VBUS_PIN);
>               return;
>       }
>
>       ret = gpio_request_one(DA850_USB1_OC_PIN,
>                       GPIOF_DIR_IN, "USB1 OC");
>       if (ret < 0) {
>               pr_err("%s: failed to request GPIO for USB 1.1 port "
>                       "over-current indicator: %d\n", __func__, ret);
>               gpio_free(DA850_USB1_OC_PIN);
>               return;
>       }
>
> Patch 4/6
>
>       ret = gpio_request_one(DA850_HAWK_MMCSD_CD_PIN,
>                       GPIOF_DIR_IN, "MMC CD");
>       if (ret < 0) {
>               pr_warning("%s: can not open GPIO %d\n",
>                       __func__, DA850_HAWK_MMCSD_CD_PIN);
>               gpio_free(DA850_HAWK_MMCSD_CD_PIN);
>               return;
>       }
>
>       ret = gpio_request_one(DA850_HAWK_MMCSD_WP_PIN,
>                       GPIOF_DIR_IN, "MMC WP");
>       if (ret < 0) {
>               pr_warning("%s: can not open GPIO %d\n",
>                       __func__, DA850_HAWK_MMCSD_WP_PIN);
>               gpio_free(DA850_HAWK_MMCSD_WP_PIN);
>               return;
>       }
>
>
> Is this ok ? or should I free the GPIO on the next section ? Same with USB

Not sure what you mean by "next section"? goto based error recovery is
more commonly used and probably more preferred as it is easier to extend.

You can look at function da850_evm_ui_expander_setup() of
arch/arm/mach-davinci/board-da850-evm.c for an example.

Thanks,
Sekhar

[PATCH v9 6/6] davinci: USB1.1 support for Omapl138-Hawkboard

[Victor Rodriguez]

On Fri, Dec 3, 2010 at 5:51 AM, Nori, Sekhar <nsekhar@ti.com> wrote:
> On Fri, Dec 03, 2010 at 06:14:39, Victor Rodriguez wrote:
>> On Thu, Dec 2, 2010 at 12:49 AM, Nori, Sekhar <nsekhar@ti.com> wrote:
>> > On Thu, Dec 02, 2010 at 01:02:29, vm.rod25 at gmail.com wrote:
>> >> +static int hawk_usb_ocic_notify(da8xx_ocic_handler_t handler)
>> >> +{
>> >> + ? ? int irq ? ? ? ? = gpio_to_irq(DA850_USB1_OC_PIN);
>> >> + ? ? int error ? ? ? = 0;
>> >> +
>> >> + ? ? if (handler != NULL) {
>> >> + ? ? ? ? ? ? hawk_usb_ocic_handler = handler;
>> >> +
>> >> + ? ? ? ? ? ? error = request_irq(irq, omapl138_hawk_usb_ocic_irq,
>> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? IRQF_DISABLED | IRQF_TRIGGER_RISING |
>> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? IRQF_TRIGGER_FALLING,
>> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "OHCI over-current indicator", NULL);
>> >> + ? ? ? ? ? ? if (error)
>> >> + ? ? ? ? ? ? ? ? ? ? pr_err(KERN_ERR "%s: could not request IRQ to watch "
>> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? "over-current indicator changes\n", __func__);
>> >
>> > pr_err adds a KERN_ERR already.
>>
>> Changed to this
>>
>> static int hawk_usb_ocic_notify(da8xx_ocic_handler_t handler)
>> {
>> ? ? ? int irq ? ? ? ? = gpio_to_irq(DA850_USB1_OC_PIN);
>> ? ? ? int error ? ? ? = 0;
>>
>> ? ? ? if (handler != NULL) {
>> ? ? ? ? ? ? ? hawk_usb_ocic_handler = handler;
>>
>> ? ? ? ? ? ? ? error = request_irq(irq, omapl138_hawk_usb_ocic_irq,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? IRQF_DISABLED | IRQF_TRIGGER_RISING |
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? IRQF_TRIGGER_FALLING,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "OHCI over-current indicator", NULL);
>> ? ? ? ? ? ? ? if (error)
>> ? ? ? ? ? ? ? ? ? ? ? pr_err("%s: could not request IRQ to watch "
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "over-current indicator changes\n", __func__);
>
> Looks good. Thanks. The same error is present elsewhere in the
> patch and other patches too. Please fix those too.

Ok thanks for check it, I am sending the changes in a patch format ,
any way I will resend the series with the correction if it is ok
>>
>> USB part
>>
>> Changed to this
>>
>> ? ? ? if (ret < 0) {
>> ? ? ? ? ? ? ? pr_err("%s: failed to request GPIO for USB 1.1 port "
>> ? ? ? ? ? ? ? ? ? ? ? "power control: %d\n", __func__, ret);
>> ? ? ? ? ? ? ? gpio_free(DA850_USB1_VBUS_PIN);
>> ? ? ? ? ? ? ? return;
>> ? ? ? }
>>
>> ? ? ? ret = gpio_request_one(DA850_USB1_OC_PIN,
>> ? ? ? ? ? ? ? ? ? ? ? GPIOF_DIR_IN, "USB1 OC");
>> ? ? ? if (ret < 0) {
>> ? ? ? ? ? ? ? pr_err("%s: failed to request GPIO for USB 1.1 port "
>> ? ? ? ? ? ? ? ? ? ? ? "over-current indicator: %d\n", __func__, ret);
>> ? ? ? ? ? ? ? gpio_free(DA850_USB1_OC_PIN);
>> ? ? ? ? ? ? ? return;
>> ? ? ? }
>>
>> Patch 4/6
>>
>> ? ? ? ret = gpio_request_one(DA850_HAWK_MMCSD_CD_PIN,
>> ? ? ? ? ? ? ? ? ? ? ? GPIOF_DIR_IN, "MMC CD");
>> ? ? ? if (ret < 0) {
>> ? ? ? ? ? ? ? pr_warning("%s: can not open GPIO %d\n",
>> ? ? ? ? ? ? ? ? ? ? ? __func__, DA850_HAWK_MMCSD_CD_PIN);
>> ? ? ? ? ? ? ? gpio_free(DA850_HAWK_MMCSD_CD_PIN);
>> ? ? ? ? ? ? ? return;
>> ? ? ? }
>>
>> ? ? ? ret = gpio_request_one(DA850_HAWK_MMCSD_WP_PIN,
>> ? ? ? ? ? ? ? ? ? ? ? GPIOF_DIR_IN, "MMC WP");
>> ? ? ? if (ret < 0) {
>> ? ? ? ? ? ? ? pr_warning("%s: can not open GPIO %d\n",
>> ? ? ? ? ? ? ? ? ? ? ? __func__, DA850_HAWK_MMCSD_WP_PIN);
>> ? ? ? ? ? ? ? gpio_free(DA850_HAWK_MMCSD_WP_PIN);
>> ? ? ? ? ? ? ? return;
>> ? ? ? }
>>
>>
>> Is this ok ? or should I free the GPIO on the next section ? Same with USB
>
> Not sure what you mean by "next section"? goto based error recovery is
> more commonly used and probably more preferred as it is easier to extend.

I mean the next if

ret = da8xx_register_usb11(&omapl138_hawk_usb11_pdata);
	if (ret) {
		pr_warning("%s: USB 1.1 registration failed: %d\n",
			__func__, ret);
	}

> You can look at function da850_evm_ui_expander_setup() of
> arch/arm/mach-davinci/board-da850-evm.c for an example.

Thanks for the help I have free  the two gpio on the last registration
request error handler

check it on the next patch

---
 arch/arm/mach-davinci/board-omapl138-hawk.c |   38 ++++++++++++++++++++------
 1 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c
b/arch/arm/mach-davinci/board-omapl138-hawk.c
index da51136..8fc78f2 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -157,7 +157,7 @@ static __init void omapl138_hawk_mmc_init(void)
 	if (ret < 0) {
 		pr_warning("%s: can not open GPIO %d\n",
 			__func__, DA850_HAWK_MMCSD_CD_PIN);
-		return;
+		goto exp_setup_cd_fail;
 	}

 	ret = gpio_request_one(DA850_HAWK_MMCSD_WP_PIN,
@@ -165,13 +165,23 @@ static __init void omapl138_hawk_mmc_init(void)
 	if (ret < 0) {
 		pr_warning("%s: can not open GPIO %d\n",
 			__func__, DA850_HAWK_MMCSD_WP_PIN);
-		return;
+		goto exp_setup_wp_fail;
 	}

 	ret = da8xx_register_mmcsd0(&da850_mmc_config);
-	if (ret)
+	if (ret) {
 		pr_warning("%s: MMC/SD0 registration failed: %d\n",
 			__func__, ret);
+		goto exp_setup_mmcsd_fail;
+	}
+		return;
+
+exp_setup_mmcsd_fail:
+	gpio_free(DA850_HAWK_MMCSD_WP_PIN);
+exp_setup_wp_fail:
+	gpio_free(DA850_HAWK_MMCSD_CD_PIN);
+exp_setup_cd_fail:
+		return;
 }

 static irqreturn_t omapl138_hawk_usb_ocic_irq(int irq, void *dev_id);
@@ -211,7 +221,7 @@ static int
hawk_usb_ocic_notify(da8xx_ocic_handler_t handler)
 					IRQF_TRIGGER_FALLING,
 					"OHCI over-current indicator", NULL);
 		if (error)
-			pr_err(KERN_ERR "%s: could not request IRQ to watch "
+			pr_err("%s: could not request IRQ to watch "
 				"over-current indicator changes\n", __func__);
 	} else {
 		free_irq(irq, NULL);
@@ -256,23 +266,33 @@ static __init void omapl138_hawk_usb_init(void)
 	ret = gpio_request_one(DA850_USB1_VBUS_PIN,
 			GPIOF_DIR_OUT, "USB1 VBUS");
 	if (ret < 0) {
-		pr_err(KERN_ERR "%s: failed to request GPIO for USB 1.1 port "
+		pr_err("%s: failed to request GPIO for USB 1.1 port "
 			"power control: %d\n", __func__, ret);
-		return;
+		goto exp_setup_vbus_fail;
 	}

 	ret = gpio_request_one(DA850_USB1_OC_PIN,
 			GPIOF_DIR_IN, "USB1 OC");
 	if (ret < 0) {
-		pr_err(KERN_ERR "%s: failed to request GPIO for USB 1.1 port "
+		pr_err("%s: failed to request GPIO for USB 1.1 port "
 			"over-current indicator: %d\n", __func__, ret);
-		return;
+		goto exp_setup_oc_fail;
 	}

 	ret = da8xx_register_usb11(&omapl138_hawk_usb11_pdata);
-	if (ret)
+	if (ret) {
 		pr_warning("%s: USB 1.1 registration failed: %d\n",
 			__func__, ret);
+		goto exp_setup_usb11_fail;
+	}
+		return;
+
+exp_setup_usb11_fail:
+	gpio_free(DA850_USB1_OC_PIN);
+exp_setup_oc_fail:
+	gpio_free(DA850_USB1_VBUS_PIN);
+exp_setup_vbus_fail:
+		return;
 }

 static struct davinci_uart_config omapl138_hawk_uart_config __initdata = {
-- 
1.7.0.4


I have already compiled and tested. If it is ok for you I will resend
the patches with the corrections

Thanks a lot for your help

Regards

Victor Rodriguez


> Thanks,
> Sekhar
>
>

[PATCH v9 6/6] davinci: USB1.1 support for Omapl138-Hawkboard

[Nori, Sekhar]

On Fri, Dec 03, 2010 at 22:40:44, Victor Rodriguez wrote:

>  arch/arm/mach-davinci/board-omapl138-hawk.c |   38 ++++++++++++++++++++------
>  1 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c
> b/arch/arm/mach-davinci/board-omapl138-hawk.c
> index da51136..8fc78f2 100644
> --- a/arch/arm/mach-davinci/board-omapl138-hawk.c
> +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
> @@ -157,7 +157,7 @@ static __init void omapl138_hawk_mmc_init(void)
>       if (ret < 0) {
>               pr_warning("%s: can not open GPIO %d\n",
>                       __func__, DA850_HAWK_MMCSD_CD_PIN);
> -             return;
> +             goto exp_setup_cd_fail;
>       }
>
>       ret = gpio_request_one(DA850_HAWK_MMCSD_WP_PIN,
> @@ -165,13 +165,23 @@ static __init void omapl138_hawk_mmc_init(void)
>       if (ret < 0) {
>               pr_warning("%s: can not open GPIO %d\n",
>                       __func__, DA850_HAWK_MMCSD_WP_PIN);
> -             return;
> +             goto exp_setup_wp_fail;
>       }
>
>       ret = da8xx_register_mmcsd0(&da850_mmc_config);
> -     if (ret)
> +     if (ret) {
>               pr_warning("%s: MMC/SD0 registration failed: %d\n",
>                       __func__, ret);
> +             goto exp_setup_mmcsd_fail;
> +     }
> +             return;

This return has extra indentation.

> +
> +exp_setup_mmcsd_fail:
> +     gpio_free(DA850_HAWK_MMCSD_WP_PIN);
> +exp_setup_wp_fail:
> +     gpio_free(DA850_HAWK_MMCSD_CD_PIN);
> +exp_setup_cd_fail:
> +             return;

This one too. Other than that, it all looks good to me.

Thanks,
Sekhar

[PATCH v9 6/6] davinci: USB1.1 support for Omapl138-Hawkboard

[Sergei Shtylyov]

Hello.

On 06-12-2010 14:12, Nori, Sekhar wrote:

>>   arch/arm/mach-davinci/board-omapl138-hawk.c |   38 ++++++++++++++++++++------
>>   1 files changed, 29 insertions(+), 9 deletions(-)

>> diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c
>> b/arch/arm/mach-davinci/board-omapl138-hawk.c
>> index da51136..8fc78f2 100644
>> --- a/arch/arm/mach-davinci/board-omapl138-hawk.c
>> +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
[...]
>> @@ -165,13 +165,23 @@ static __init void omapl138_hawk_mmc_init(void)
>>        if (ret<  0) {
>>                pr_warning("%s: can not open GPIO %d\n",
>>                        __func__, DA850_HAWK_MMCSD_WP_PIN);
>> -             return;
>> +             goto exp_setup_wp_fail;
>>        }
>>
>>        ret = da8xx_register_mmcsd0(&da850_mmc_config);
>> -     if (ret)
>> +     if (ret) {
>>                pr_warning("%s: MMC/SD0 registration failed: %d\n",
>>                        __func__, ret);
>> +             goto exp_setup_mmcsd_fail;
>> +     }
>> +             return;

> This return has extra indentation.

>> +
>> +exp_setup_mmcsd_fail:
>> +     gpio_free(DA850_HAWK_MMCSD_WP_PIN);
>> +exp_setup_wp_fail:
>> +     gpio_free(DA850_HAWK_MMCSD_CD_PIN);
>> +exp_setup_cd_fail:
>> +             return;

> This one too.

    Moreover, it's not needed at all.

> Other than that, it all looks good to me.

    Except those 'exp_setup_'prefixes which I'm not sure where are coming from...

> Thanks,
> Sekhar

WBR, Sergei

[PATCH v9 6/6] davinci: USB1.1 support for Omapl138-Hawkboard

[Victor Rodriguez]

On Mon, Dec 6, 2010 at 5:43 AM, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> Hello.
>
> On 06-12-2010 14:12, Nori, Sekhar wrote:
>
>>> ?arch/arm/mach-davinci/board-omapl138-hawk.c | ? 38
>>> ++++++++++++++++++++------
>>> ?1 files changed, 29 insertions(+), 9 deletions(-)
>
>>> diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c
>>> b/arch/arm/mach-davinci/board-omapl138-hawk.c
>>> index da51136..8fc78f2 100644
>>> --- a/arch/arm/mach-davinci/board-omapl138-hawk.c
>>> +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
>
> [...]
>>>
>>> @@ -165,13 +165,23 @@ static __init void omapl138_hawk_mmc_init(void)
>>> ? ? ? if (ret< ?0) {
>>> ? ? ? ? ? ? ? pr_warning("%s: can not open GPIO %d\n",
>>> ? ? ? ? ? ? ? ? ? ? ? __func__, DA850_HAWK_MMCSD_WP_PIN);
>>> - ? ? ? ? ? ? return;
>>> + ? ? ? ? ? ? goto exp_setup_wp_fail;
>>> ? ? ? }
>>>
>>> ? ? ? ret = da8xx_register_mmcsd0(&da850_mmc_config);
>>> - ? ? if (ret)
>>> + ? ? if (ret) {
>>> ? ? ? ? ? ? ? pr_warning("%s: MMC/SD0 registration failed: %d\n",
>>> ? ? ? ? ? ? ? ? ? ? ? __func__, ret);
>>> + ? ? ? ? ? ? goto exp_setup_mmcsd_fail;
>>> + ? ? }
>>> + ? ? ? ? ? ? return;
>
>> This return has extra indentation.
>
>>> +
>>> +exp_setup_mmcsd_fail:
>>> + ? ? gpio_free(DA850_HAWK_MMCSD_WP_PIN);
>>> +exp_setup_wp_fail:
>>> + ? ? gpio_free(DA850_HAWK_MMCSD_CD_PIN);
>>> +exp_setup_cd_fail:
>>> + ? ? ? ? ? ? return;
>
>> This one too.
>
> ? Moreover, it's not needed at all.

Ok
>> Other than that, it all looks good to me.
>
> ? Except those 'exp_setup_'prefixes which I'm not sure where are coming
> from...


it comes from

arch/arm/mach-davinci/board-da850-evm.c

I took this as a template and I think that is better to keep this
exp_setup_  as a template

but if you have any other suggestion please tell me

Thanks for check it helps me a lot

Regards

Victor Rodriguez


>> Thanks,
>> Sekhar
>
> WBR, Sergei
>

[PATCH v9 6/6] davinci: USB1.1 support for Omapl138-Hawkboard

[Ben Gardiner]

On Mon, Dec 6, 2010 at 10:47 AM, Victor Rodriguez <vm.rod25@gmail.com> wrote:
> On Mon, Dec 6, 2010 at 5:43 AM, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
>> Hello.
>>
>> On 06-12-2010 14:12, Nori, Sekhar wrote:
>>
>>>> ?arch/arm/mach-davinci/board-omapl138-hawk.c | ? 38
>>>> ++++++++++++++++++++------
>>>> ?1 files changed, 29 insertions(+), 9 deletions(-)
>>
>>>> diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c
>>>> b/arch/arm/mach-davinci/board-omapl138-hawk.c
>>>> index da51136..8fc78f2 100644
>>>> --- a/arch/arm/mach-davinci/board-omapl138-hawk.c
>>>> +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
>>
>> [...]
>>>>
>>>> @@ -165,13 +165,23 @@ static __init void omapl138_hawk_mmc_init(void)
>>>> ? ? ? if (ret< ?0) {
>>>> ? ? ? ? ? ? ? pr_warning("%s: can not open GPIO %d\n",
>>>> ? ? ? ? ? ? ? ? ? ? ? __func__, DA850_HAWK_MMCSD_WP_PIN);
>>>> - ? ? ? ? ? ? return;
>>>> + ? ? ? ? ? ? goto exp_setup_wp_fail;
>>>> ? ? ? }
>>>>
>>>> ? ? ? ret = da8xx_register_mmcsd0(&da850_mmc_config);
>>>> - ? ? if (ret)
>>>> + ? ? if (ret) {
>>>> ? ? ? ? ? ? ? pr_warning("%s: MMC/SD0 registration failed: %d\n",
>>>> ? ? ? ? ? ? ? ? ? ? ? __func__, ret);
>>>> + ? ? ? ? ? ? goto exp_setup_mmcsd_fail;
>>>> + ? ? }
>>>> + ? ? ? ? ? ? return;
>>
>>> This return has extra indentation.
>>
>>>> +
>>>> +exp_setup_mmcsd_fail:
>>>> + ? ? gpio_free(DA850_HAWK_MMCSD_WP_PIN);
>>>> +exp_setup_wp_fail:
>>>> + ? ? gpio_free(DA850_HAWK_MMCSD_CD_PIN);
>>>> +exp_setup_cd_fail:
>>>> + ? ? ? ? ? ? return;
>>
>>> This one too.
>>
>> ? Moreover, it's not needed at all.
>
> Ok
>>> Other than that, it all looks good to me.
>>
>> ? Except those 'exp_setup_'prefixes which I'm not sure where are coming
>> from...
>
>
> it comes from
>
> arch/arm/mach-davinci/board-da850-evm.c
>
> I took this as a template and I think that is better to keep this
> exp_setup_ ?as a template
>
> but if you have any other suggestion please tell me

I think those are based on the error handling in
da850_evm_ui_expander_setup() which means that the 'exp' in
'exp_setup_' stands-for IO-expander.

Based on the function names where the your exp_setup_* labels were
introduced I think that the label could be renamed 'mmc_setup_*' and
'usb11_setup_*'.


Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

[PATCH v9 6/6] davinci: USB1.1 support for Omapl138-Hawkboard

[Victor Rodriguez]

On Mon, Dec 6, 2010 at 9:53 AM, Ben Gardiner <bengardiner@nanometrics.ca> wrote:
> On Mon, Dec 6, 2010 at 10:47 AM, Victor Rodriguez <vm.rod25@gmail.com> wrote:
>> On Mon, Dec 6, 2010 at 5:43 AM, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
>>> Hello.
>>>
>>> On 06-12-2010 14:12, Nori, Sekhar wrote:
>>>
>>>>> ?arch/arm/mach-davinci/board-omapl138-hawk.c | ? 38
>>>>> ++++++++++++++++++++------
>>>>> ?1 files changed, 29 insertions(+), 9 deletions(-)
>>>
>>>>> diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c
>>>>> b/arch/arm/mach-davinci/board-omapl138-hawk.c
>>>>> index da51136..8fc78f2 100644
>>>>> --- a/arch/arm/mach-davinci/board-omapl138-hawk.c
>>>>> +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
>>>
>>> [...]
>>>>>
>>>>> @@ -165,13 +165,23 @@ static __init void omapl138_hawk_mmc_init(void)
>>>>> ? ? ? if (ret< ?0) {
>>>>> ? ? ? ? ? ? ? pr_warning("%s: can not open GPIO %d\n",
>>>>> ? ? ? ? ? ? ? ? ? ? ? __func__, DA850_HAWK_MMCSD_WP_PIN);
>>>>> - ? ? ? ? ? ? return;
>>>>> + ? ? ? ? ? ? goto exp_setup_wp_fail;
>>>>> ? ? ? }
>>>>>
>>>>> ? ? ? ret = da8xx_register_mmcsd0(&da850_mmc_config);
>>>>> - ? ? if (ret)
>>>>> + ? ? if (ret) {
>>>>> ? ? ? ? ? ? ? pr_warning("%s: MMC/SD0 registration failed: %d\n",
>>>>> ? ? ? ? ? ? ? ? ? ? ? __func__, ret);
>>>>> + ? ? ? ? ? ? goto exp_setup_mmcsd_fail;
>>>>> + ? ? }
>>>>> + ? ? ? ? ? ? return;
>>>
>>>> This return has extra indentation.
>>>
>>>>> +
>>>>> +exp_setup_mmcsd_fail:
>>>>> + ? ? gpio_free(DA850_HAWK_MMCSD_WP_PIN);
>>>>> +exp_setup_wp_fail:
>>>>> + ? ? gpio_free(DA850_HAWK_MMCSD_CD_PIN);
>>>>> +exp_setup_cd_fail:
>>>>> + ? ? ? ? ? ? return;
>>>
>>>> This one too.
>>>
>>> ? Moreover, it's not needed at all.
>>
>> Ok
>>>> Other than that, it all looks good to me.
>>>
>>> ? Except those 'exp_setup_'prefixes which I'm not sure where are coming
>>> from...
>>
>>
>> it comes from
>>
>> arch/arm/mach-davinci/board-da850-evm.c
>>
>> I took this as a template and I think that is better to keep this
>> exp_setup_ ?as a template
>>
>> but if you have any other suggestion please tell me
>
> I think those are based on the error handling in
> da850_evm_ui_expander_setup() which means that the 'exp' in
> 'exp_setup_' stands-for IO-expander.
>
> Based on the function names where the your exp_setup_* labels were
> introduced I think that the label could be renamed 'mmc_setup_*' and
> 'usb11_setup_*'.
>
I like it thanks I will submit the patches with that names. Sergei are
you ok with this ?

Regards

Victor Rodriguez


> Best Regards,
> Ben Gardiner
>
> ---
> Nanometrics Inc.
> http://www.nanometrics.ca
>

[PATCH v9 6/6] davinci: USB1.1 support for Omapl138-Hawkboard

[Sergei Shtylyov]

Hello.

Victor Rodriguez wrote:

>>>>>>  arch/arm/mach-davinci/board-omapl138-hawk.c |   38
>>>>>> ++++++++++++++++++++------
>>>>>>  1 files changed, 29 insertions(+), 9 deletions(-)
>>>>>> diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c
>>>>>> b/arch/arm/mach-davinci/board-omapl138-hawk.c
>>>>>> index da51136..8fc78f2 100644
>>>>>> --- a/arch/arm/mach-davinci/board-omapl138-hawk.c
>>>>>> +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
>>>> [...]
>>>>>> @@ -165,13 +165,23 @@ static __init void omapl138_hawk_mmc_init(void)
>>>>>>       if (ret<  0) {
>>>>>>               pr_warning("%s: can not open GPIO %d\n",
>>>>>>                       __func__, DA850_HAWK_MMCSD_WP_PIN);
>>>>>> -             return;
>>>>>> +             goto exp_setup_wp_fail;
>>>>>>       }
>>>>>>
>>>>>>       ret = da8xx_register_mmcsd0(&da850_mmc_config);
>>>>>> -     if (ret)
>>>>>> +     if (ret) {
>>>>>>               pr_warning("%s: MMC/SD0 registration failed: %d\n",
>>>>>>                       __func__, ret);
>>>>>> +             goto exp_setup_mmcsd_fail;
>>>>>> +     }
>>>>>> +             return;

>>>>> This return has extra indentation.

>>>>>> +
>>>>>> +exp_setup_mmcsd_fail:
>>>>>> +     gpio_free(DA850_HAWK_MMCSD_WP_PIN);
>>>>>> +exp_setup_wp_fail:
>>>>>> +     gpio_free(DA850_HAWK_MMCSD_CD_PIN);
>>>>>> +exp_setup_cd_fail:
>>>>>> +             return;

>>>>> This one too.

>>>>   Moreover, it's not needed at all.

>>> Ok

    ... except you can't probably put a label next to }.

>>>>> Other than that, it all looks good to me.

>>>>   Except those 'exp_setup_'prefixes which I'm not sure where are coming
>>>> from...

>>> it comes from

>>> arch/arm/mach-davinci/board-da850-evm.c

>>> I took this as a template and I think that is better to keep this
>>> exp_setup_  as a template

>>> but if you have any other suggestion please tell me

>> I think those are based on the error handling in
>> da850_evm_ui_expander_setup() which means that the 'exp' in
>> 'exp_setup_' stands-for IO-expander.

>> Based on the function names where the your exp_setup_* labels were
>> introduced I think that the label could be renamed 'mmc_setup_*' and
>> 'usb11_setup_*'.

> I like it thanks I will submit the patches with that names. Sergei are
> you ok with this ?

    I'm not sure the prefixes are necessary at all, but won't have objection to 
those ones...

> Regards

> Victor Rodriguez

WBR, Sergei