* [PATCH] ARM: mx28evk: Simplify GPIO requests
@ 2012-01-23 14:41 Fabio Estevam
2012-01-26 12:19 ` Shawn Guo
2012-01-27 14:52 ` [PATCH v2] " Fabio Estevam
0 siblings, 2 replies; 6+ messages in thread
From: Fabio Estevam @ 2012-01-23 14:41 UTC (permalink / raw)
To: linux-arm-kernel
Requesting all the GPIOs on a single array (mx28evk_gpios[]) can make the
error handling of gpio_requests much simpler.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
This approach was suggested by Sascha Hauer:
http://patchwork.ozlabs.org/patch/124661/
arch/arm/mach-mxs/mach-mx28evk.c | 72 ++++++++-----------------------------
1 files changed, 16 insertions(+), 56 deletions(-)
diff --git a/arch/arm/mach-mxs/mach-mx28evk.c b/arch/arm/mach-mxs/mach-mx28evk.c
index fdb0a56..6260ec3 100644
--- a/arch/arm/mach-mxs/mach-mx28evk.c
+++ b/arch/arm/mach-mxs/mach-mx28evk.c
@@ -223,7 +223,6 @@ static const struct gpio_led_platform_data mx28evk_led_data __initconst = {
/* fec */
static void __init mx28evk_fec_reset(void)
{
- int ret;
struct clk *clk;
/* Enable fec phy clock */
@@ -231,32 +230,6 @@ static void __init mx28evk_fec_reset(void)
if (!IS_ERR(clk))
clk_prepare_enable(clk);
- /* Power up fec phy */
- ret = gpio_request(MX28EVK_FEC_PHY_POWER, "fec-phy-power");
- if (ret) {
- pr_err("Failed to request gpio fec-phy-%s: %d\n", "power", ret);
- return;
- }
-
- ret = gpio_direction_output(MX28EVK_FEC_PHY_POWER, 0);
- if (ret) {
- pr_err("Failed to drive gpio fec-phy-%s: %d\n", "power", ret);
- return;
- }
-
- /* Reset fec phy */
- ret = gpio_request(MX28EVK_FEC_PHY_RESET, "fec-phy-reset");
- if (ret) {
- pr_err("Failed to request gpio fec-phy-%s: %d\n", "reset", ret);
- return;
- }
-
- gpio_direction_output(MX28EVK_FEC_PHY_RESET, 0);
- if (ret) {
- pr_err("Failed to drive gpio fec-phy-%s: %d\n", "reset", ret);
- return;
- }
-
mdelay(1);
gpio_set_value(MX28EVK_FEC_PHY_RESET, 1);
}
@@ -417,9 +390,14 @@ static void __init mx28evk_add_regulators(void)
static void __init mx28evk_add_regulators(void) {}
#endif
-static struct gpio mx28evk_lcd_gpios[] = {
+static const __initconst struct gpio mx28evk_gpios[] = {
{ MX28EVK_LCD_ENABLE, GPIOF_OUT_INIT_HIGH, "lcd-enable" },
{ MX28EVK_BL_ENABLE, GPIOF_OUT_INIT_HIGH, "bl-enable" },
+ { MX28EVK_FLEXCAN_SWITCH, GPIOF_DIR_OUT, "flexcan-switch" },
+ { MX28EVK_MMC0_SLOT_POWER, GPIOF_OUT_INIT_LOW, "mmc0-slot-power" },
+ { MX28EVK_MMC1_SLOT_POWER, GPIOF_OUT_INIT_LOW, "mmc1-slot-power" },
+ { MX28EVK_FEC_PHY_POWER, GPIOF_OUT_INIT_LOW, "fec-phy-power" },
+ { MX28EVK_FEC_PHY_RESET, GPIOF_OUT_INIT_LOW, "fec-phy-reset" },
};
static const struct mxs_saif_platform_data
@@ -447,25 +425,19 @@ static void __init mx28evk_init(void)
if (mx28evk_fec_get_mac())
pr_warn("%s: failed on fec mac setup\n", __func__);
+ ret = gpio_request_array(mx28evk_gpios, ARRAY_SIZE(mx28evk_gpios));
+
+ if (ret)
+ pr_err("One or more GPIOs failed to be requested: %d\n", ret);
+
mx28evk_fec_reset();
mx28_add_fec(0, &mx28_fec_pdata[0]);
mx28_add_fec(1, &mx28_fec_pdata[1]);
- ret = gpio_request_one(MX28EVK_FLEXCAN_SWITCH, GPIOF_DIR_OUT,
- "flexcan-switch");
- if (ret) {
- pr_err("failed to request gpio flexcan-switch: %d\n", ret);
- } else {
- mx28_add_flexcan(0, &mx28evk_flexcan_pdata[0]);
- mx28_add_flexcan(1, &mx28evk_flexcan_pdata[1]);
- }
+ mx28_add_flexcan(0, &mx28evk_flexcan_pdata[0]);
+ mx28_add_flexcan(1, &mx28evk_flexcan_pdata[1]);
- ret = gpio_request_array(mx28evk_lcd_gpios,
- ARRAY_SIZE(mx28evk_lcd_gpios));
- if (ret)
- pr_warn("failed to request gpio pins for lcd: %d\n", ret);
- else
- mx28_add_mxsfb(&mx28evk_mxsfb_pdata);
+ mx28_add_mxsfb(&mx28evk_mxsfb_pdata);
mxs_saif_clkmux_select(MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0);
mx28_add_saif(0, &mx28evk_mxs_saif_pdata[0]);
@@ -480,20 +452,8 @@ static void __init mx28evk_init(void)
mxs_add_platform_device("mxs-sgtl5000", 0, NULL, 0,
NULL, 0);
- /* power on mmc slot by writing 0 to the gpio */
- ret = gpio_request_one(MX28EVK_MMC0_SLOT_POWER, GPIOF_OUT_INIT_LOW,
- "mmc0-slot-power");
- if (ret)
- pr_warn("failed to request gpio mmc0-slot-power: %d\n", ret);
- else
- mx28_add_mxs_mmc(0, &mx28evk_mmc_pdata[0]);
-
- ret = gpio_request_one(MX28EVK_MMC1_SLOT_POWER, GPIOF_OUT_INIT_LOW,
- "mmc1-slot-power");
- if (ret)
- pr_warn("failed to request gpio mmc1-slot-power: %d\n", ret);
- else
- mx28_add_mxs_mmc(1, &mx28evk_mmc_pdata[1]);
+ mx28_add_mxs_mmc(0, &mx28evk_mmc_pdata[0]);
+ mx28_add_mxs_mmc(1, &mx28evk_mmc_pdata[1]);
mx28_add_rtc_stmp3xxx();
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] ARM: mx28evk: Simplify GPIO requests
2012-01-23 14:41 [PATCH] ARM: mx28evk: Simplify GPIO requests Fabio Estevam
@ 2012-01-26 12:19 ` Shawn Guo
2012-01-26 12:49 ` Lothar Waßmann
2012-01-27 14:52 ` [PATCH v2] " Fabio Estevam
1 sibling, 1 reply; 6+ messages in thread
From: Shawn Guo @ 2012-01-26 12:19 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jan 23, 2012 at 12:41:49PM -0200, Fabio Estevam wrote:
> Requesting all the GPIOs on a single array (mx28evk_gpios[]) can make the
> error handling of gpio_requests much simpler.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Looks good. A couple trivial comments below though.
> ---
> This approach was suggested by Sascha Hauer:
> http://patchwork.ozlabs.org/patch/124661/
>
> arch/arm/mach-mxs/mach-mx28evk.c | 72 ++++++++-----------------------------
> 1 files changed, 16 insertions(+), 56 deletions(-)
>
> diff --git a/arch/arm/mach-mxs/mach-mx28evk.c b/arch/arm/mach-mxs/mach-mx28evk.c
> index fdb0a56..6260ec3 100644
> --- a/arch/arm/mach-mxs/mach-mx28evk.c
> +++ b/arch/arm/mach-mxs/mach-mx28evk.c
> @@ -223,7 +223,6 @@ static const struct gpio_led_platform_data mx28evk_led_data __initconst = {
> /* fec */
> static void __init mx28evk_fec_reset(void)
> {
> - int ret;
> struct clk *clk;
>
> /* Enable fec phy clock */
> @@ -231,32 +230,6 @@ static void __init mx28evk_fec_reset(void)
> if (!IS_ERR(clk))
> clk_prepare_enable(clk);
>
> - /* Power up fec phy */
> - ret = gpio_request(MX28EVK_FEC_PHY_POWER, "fec-phy-power");
> - if (ret) {
> - pr_err("Failed to request gpio fec-phy-%s: %d\n", "power", ret);
> - return;
> - }
> -
> - ret = gpio_direction_output(MX28EVK_FEC_PHY_POWER, 0);
> - if (ret) {
> - pr_err("Failed to drive gpio fec-phy-%s: %d\n", "power", ret);
> - return;
> - }
> -
> - /* Reset fec phy */
> - ret = gpio_request(MX28EVK_FEC_PHY_RESET, "fec-phy-reset");
> - if (ret) {
> - pr_err("Failed to request gpio fec-phy-%s: %d\n", "reset", ret);
> - return;
> - }
> -
> - gpio_direction_output(MX28EVK_FEC_PHY_RESET, 0);
> - if (ret) {
> - pr_err("Failed to drive gpio fec-phy-%s: %d\n", "reset", ret);
> - return;
> - }
> -
> mdelay(1);
This delay is supposed to be in the middle of the two calls here. As
we are moving around the first call, can you give it a test to see if
the mdelay is still mandatory? I would remove it if it's not necessary.
> gpio_set_value(MX28EVK_FEC_PHY_RESET, 1);
> }
> @@ -417,9 +390,14 @@ static void __init mx28evk_add_regulators(void)
> static void __init mx28evk_add_regulators(void) {}
> #endif
>
> -static struct gpio mx28evk_lcd_gpios[] = {
> +static const __initconst struct gpio mx28evk_gpios[] = {
Put __initconst after mx28evk_gpios[], please.
> { MX28EVK_LCD_ENABLE, GPIOF_OUT_INIT_HIGH, "lcd-enable" },
> { MX28EVK_BL_ENABLE, GPIOF_OUT_INIT_HIGH, "bl-enable" },
> + { MX28EVK_FLEXCAN_SWITCH, GPIOF_DIR_OUT, "flexcan-switch" },
> + { MX28EVK_MMC0_SLOT_POWER, GPIOF_OUT_INIT_LOW, "mmc0-slot-power" },
> + { MX28EVK_MMC1_SLOT_POWER, GPIOF_OUT_INIT_LOW, "mmc1-slot-power" },
> + { MX28EVK_FEC_PHY_POWER, GPIOF_OUT_INIT_LOW, "fec-phy-power" },
> + { MX28EVK_FEC_PHY_RESET, GPIOF_OUT_INIT_LOW, "fec-phy-reset" },
> };
>
> static const struct mxs_saif_platform_data
> @@ -447,25 +425,19 @@ static void __init mx28evk_init(void)
> if (mx28evk_fec_get_mac())
> pr_warn("%s: failed on fec mac setup\n", __func__);
>
> + ret = gpio_request_array(mx28evk_gpios, ARRAY_SIZE(mx28evk_gpios));
> +
Unnecessary blank line.
Regards,
Shawn
> + if (ret)
> + pr_err("One or more GPIOs failed to be requested: %d\n", ret);
> +
> mx28evk_fec_reset();
> mx28_add_fec(0, &mx28_fec_pdata[0]);
> mx28_add_fec(1, &mx28_fec_pdata[1]);
>
> - ret = gpio_request_one(MX28EVK_FLEXCAN_SWITCH, GPIOF_DIR_OUT,
> - "flexcan-switch");
> - if (ret) {
> - pr_err("failed to request gpio flexcan-switch: %d\n", ret);
> - } else {
> - mx28_add_flexcan(0, &mx28evk_flexcan_pdata[0]);
> - mx28_add_flexcan(1, &mx28evk_flexcan_pdata[1]);
> - }
> + mx28_add_flexcan(0, &mx28evk_flexcan_pdata[0]);
> + mx28_add_flexcan(1, &mx28evk_flexcan_pdata[1]);
>
> - ret = gpio_request_array(mx28evk_lcd_gpios,
> - ARRAY_SIZE(mx28evk_lcd_gpios));
> - if (ret)
> - pr_warn("failed to request gpio pins for lcd: %d\n", ret);
> - else
> - mx28_add_mxsfb(&mx28evk_mxsfb_pdata);
> + mx28_add_mxsfb(&mx28evk_mxsfb_pdata);
>
> mxs_saif_clkmux_select(MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0);
> mx28_add_saif(0, &mx28evk_mxs_saif_pdata[0]);
> @@ -480,20 +452,8 @@ static void __init mx28evk_init(void)
> mxs_add_platform_device("mxs-sgtl5000", 0, NULL, 0,
> NULL, 0);
>
> - /* power on mmc slot by writing 0 to the gpio */
> - ret = gpio_request_one(MX28EVK_MMC0_SLOT_POWER, GPIOF_OUT_INIT_LOW,
> - "mmc0-slot-power");
> - if (ret)
> - pr_warn("failed to request gpio mmc0-slot-power: %d\n", ret);
> - else
> - mx28_add_mxs_mmc(0, &mx28evk_mmc_pdata[0]);
> -
> - ret = gpio_request_one(MX28EVK_MMC1_SLOT_POWER, GPIOF_OUT_INIT_LOW,
> - "mmc1-slot-power");
> - if (ret)
> - pr_warn("failed to request gpio mmc1-slot-power: %d\n", ret);
> - else
> - mx28_add_mxs_mmc(1, &mx28evk_mmc_pdata[1]);
> + mx28_add_mxs_mmc(0, &mx28evk_mmc_pdata[0]);
> + mx28_add_mxs_mmc(1, &mx28evk_mmc_pdata[1]);
>
> mx28_add_rtc_stmp3xxx();
>
> --
> 1.7.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ARM: mx28evk: Simplify GPIO requests
2012-01-26 12:19 ` Shawn Guo
@ 2012-01-26 12:49 ` Lothar Waßmann
2012-01-26 14:17 ` Shawn Guo
0 siblings, 1 reply; 6+ messages in thread
From: Lothar Waßmann @ 2012-01-26 12:49 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Shawn Guo writes:
> On Mon, Jan 23, 2012 at 12:41:49PM -0200, Fabio Estevam wrote:
> > Requesting all the GPIOs on a single array (mx28evk_gpios[]) can make the
> > error handling of gpio_requests much simpler.
> >
> > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>
> Looks good. A couple trivial comments below though.
>
> > ---
> > This approach was suggested by Sascha Hauer:
> > http://patchwork.ozlabs.org/patch/124661/
> >
> > arch/arm/mach-mxs/mach-mx28evk.c | 72 ++++++++-----------------------------
> > 1 files changed, 16 insertions(+), 56 deletions(-)
> >
> > diff --git a/arch/arm/mach-mxs/mach-mx28evk.c b/arch/arm/mach-mxs/mach-mx28evk.c
> > index fdb0a56..6260ec3 100644
> > --- a/arch/arm/mach-mxs/mach-mx28evk.c
> > +++ b/arch/arm/mach-mxs/mach-mx28evk.c
> > @@ -223,7 +223,6 @@ static const struct gpio_led_platform_data mx28evk_led_data __initconst = {
> > /* fec */
> > static void __init mx28evk_fec_reset(void)
> > {
> > - int ret;
> > struct clk *clk;
> >
> > /* Enable fec phy clock */
> > @@ -231,32 +230,6 @@ static void __init mx28evk_fec_reset(void)
> > if (!IS_ERR(clk))
> > clk_prepare_enable(clk);
> >
> > - /* Power up fec phy */
> > - ret = gpio_request(MX28EVK_FEC_PHY_POWER, "fec-phy-power");
> > - if (ret) {
> > - pr_err("Failed to request gpio fec-phy-%s: %d\n", "power", ret);
> > - return;
> > - }
> > -
> > - ret = gpio_direction_output(MX28EVK_FEC_PHY_POWER, 0);
> > - if (ret) {
> > - pr_err("Failed to drive gpio fec-phy-%s: %d\n", "power", ret);
> > - return;
> > - }
> > -
> > - /* Reset fec phy */
> > - ret = gpio_request(MX28EVK_FEC_PHY_RESET, "fec-phy-reset");
> > - if (ret) {
> > - pr_err("Failed to request gpio fec-phy-%s: %d\n", "reset", ret);
> > - return;
> > - }
> > -
> > - gpio_direction_output(MX28EVK_FEC_PHY_RESET, 0);
> > - if (ret) {
> > - pr_err("Failed to drive gpio fec-phy-%s: %d\n", "reset", ret);
> > - return;
> > - }
> > -
> > mdelay(1);
>
> This delay is supposed to be in the middle of the two calls here. As
> we are moving around the first call, can you give it a test to see if
> the mdelay is still mandatory? I would remove it if it's not necessary.
>
"Empirical Programming Part One"? The HW will have some timing
requirements that need to be fullfilled. That cannot be done by
'giving it a test' but by reading the appropriate datasheet and
implementing the code accordingly.
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ARM: mx28evk: Simplify GPIO requests
2012-01-26 12:49 ` Lothar Waßmann
@ 2012-01-26 14:17 ` Shawn Guo
0 siblings, 0 replies; 6+ messages in thread
From: Shawn Guo @ 2012-01-26 14:17 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jan 26, 2012 at 01:49:52PM +0100, Lothar Wa?mann wrote:
> Hi,
>
> Shawn Guo writes:
> > On Mon, Jan 23, 2012 at 12:41:49PM -0200, Fabio Estevam wrote:
> > > Requesting all the GPIOs on a single array (mx28evk_gpios[]) can make the
> > > error handling of gpio_requests much simpler.
> > >
> > > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> >
> > Looks good. A couple trivial comments below though.
> >
> > > ---
> > > This approach was suggested by Sascha Hauer:
> > > http://patchwork.ozlabs.org/patch/124661/
> > >
> > > arch/arm/mach-mxs/mach-mx28evk.c | 72 ++++++++-----------------------------
> > > 1 files changed, 16 insertions(+), 56 deletions(-)
> > >
> > > diff --git a/arch/arm/mach-mxs/mach-mx28evk.c b/arch/arm/mach-mxs/mach-mx28evk.c
> > > index fdb0a56..6260ec3 100644
> > > --- a/arch/arm/mach-mxs/mach-mx28evk.c
> > > +++ b/arch/arm/mach-mxs/mach-mx28evk.c
> > > @@ -223,7 +223,6 @@ static const struct gpio_led_platform_data mx28evk_led_data __initconst = {
> > > /* fec */
> > > static void __init mx28evk_fec_reset(void)
> > > {
> > > - int ret;
> > > struct clk *clk;
> > >
> > > /* Enable fec phy clock */
> > > @@ -231,32 +230,6 @@ static void __init mx28evk_fec_reset(void)
> > > if (!IS_ERR(clk))
> > > clk_prepare_enable(clk);
> > >
> > > - /* Power up fec phy */
> > > - ret = gpio_request(MX28EVK_FEC_PHY_POWER, "fec-phy-power");
> > > - if (ret) {
> > > - pr_err("Failed to request gpio fec-phy-%s: %d\n", "power", ret);
> > > - return;
> > > - }
> > > -
> > > - ret = gpio_direction_output(MX28EVK_FEC_PHY_POWER, 0);
> > > - if (ret) {
> > > - pr_err("Failed to drive gpio fec-phy-%s: %d\n", "power", ret);
> > > - return;
> > > - }
> > > -
> > > - /* Reset fec phy */
> > > - ret = gpio_request(MX28EVK_FEC_PHY_RESET, "fec-phy-reset");
> > > - if (ret) {
> > > - pr_err("Failed to request gpio fec-phy-%s: %d\n", "reset", ret);
> > > - return;
> > > - }
> > > -
> > > - gpio_direction_output(MX28EVK_FEC_PHY_RESET, 0);
> > > - if (ret) {
> > > - pr_err("Failed to drive gpio fec-phy-%s: %d\n", "reset", ret);
> > > - return;
> > > - }
> > > -
> > > mdelay(1);
> >
> > This delay is supposed to be in the middle of the two calls here. As
> > we are moving around the first call, can you give it a test to see if
> > the mdelay is still mandatory? I would remove it if it's not necessary.
> >
> "Empirical Programming Part One"? The HW will have some timing
> requirements that need to be fullfilled. That cannot be done by
> 'giving it a test' but by reading the appropriate datasheet and
> implementing the code accordingly.
>
I was thinking that the period of code execution between
gpio_request_array() and this point may have fulfilled the timing
requirement of LAN8720 reset (100us minimum). But you are right,
this mdelay should not be removed simply by a testing.
But the way that the patch moves thing around leaves this mdelay
a little unreadable to me. I would like see an explicit call of
gpio_set_value(MX28EVK_FEC_PHY_RESET, 0) before the mdelay.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] ARM: mx28evk: Simplify GPIO requests
2012-01-23 14:41 [PATCH] ARM: mx28evk: Simplify GPIO requests Fabio Estevam
2012-01-26 12:19 ` Shawn Guo
@ 2012-01-27 14:52 ` Fabio Estevam
2012-01-31 15:31 ` Shawn Guo
1 sibling, 1 reply; 6+ messages in thread
From: Fabio Estevam @ 2012-01-27 14:52 UTC (permalink / raw)
To: linux-arm-kernel
Requesting all the GPIOs on a single array (mx28evk_gpios[]) can make the
error handling of gpio_requests much simpler.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v1:
- Removed empty line
- Leave gpio_set_value(MX28EVK_FEC_PHY_RESET, 0) prior to the delay line in fec_reset
- Put __initconst after mx28evk_gpios[]
arch/arm/mach-mxs/mach-mx28evk.c | 71 ++++++++------------------------------
1 files changed, 15 insertions(+), 56 deletions(-)
diff --git a/arch/arm/mach-mxs/mach-mx28evk.c b/arch/arm/mach-mxs/mach-mx28evk.c
index fdb0a56..dad053d 100644
--- a/arch/arm/mach-mxs/mach-mx28evk.c
+++ b/arch/arm/mach-mxs/mach-mx28evk.c
@@ -223,7 +223,6 @@ static const struct gpio_led_platform_data mx28evk_led_data __initconst = {
/* fec */
static void __init mx28evk_fec_reset(void)
{
- int ret;
struct clk *clk;
/* Enable fec phy clock */
@@ -231,32 +230,7 @@ static void __init mx28evk_fec_reset(void)
if (!IS_ERR(clk))
clk_prepare_enable(clk);
- /* Power up fec phy */
- ret = gpio_request(MX28EVK_FEC_PHY_POWER, "fec-phy-power");
- if (ret) {
- pr_err("Failed to request gpio fec-phy-%s: %d\n", "power", ret);
- return;
- }
-
- ret = gpio_direction_output(MX28EVK_FEC_PHY_POWER, 0);
- if (ret) {
- pr_err("Failed to drive gpio fec-phy-%s: %d\n", "power", ret);
- return;
- }
-
- /* Reset fec phy */
- ret = gpio_request(MX28EVK_FEC_PHY_RESET, "fec-phy-reset");
- if (ret) {
- pr_err("Failed to request gpio fec-phy-%s: %d\n", "reset", ret);
- return;
- }
-
- gpio_direction_output(MX28EVK_FEC_PHY_RESET, 0);
- if (ret) {
- pr_err("Failed to drive gpio fec-phy-%s: %d\n", "reset", ret);
- return;
- }
-
+ gpio_set_value(MX28EVK_FEC_PHY_RESET, 0);
mdelay(1);
gpio_set_value(MX28EVK_FEC_PHY_RESET, 1);
}
@@ -417,9 +391,14 @@ static void __init mx28evk_add_regulators(void)
static void __init mx28evk_add_regulators(void) {}
#endif
-static struct gpio mx28evk_lcd_gpios[] = {
+static const struct gpio mx28evk_gpios[] __initconst = {
{ MX28EVK_LCD_ENABLE, GPIOF_OUT_INIT_HIGH, "lcd-enable" },
{ MX28EVK_BL_ENABLE, GPIOF_OUT_INIT_HIGH, "bl-enable" },
+ { MX28EVK_FLEXCAN_SWITCH, GPIOF_DIR_OUT, "flexcan-switch" },
+ { MX28EVK_MMC0_SLOT_POWER, GPIOF_OUT_INIT_LOW, "mmc0-slot-power" },
+ { MX28EVK_MMC1_SLOT_POWER, GPIOF_OUT_INIT_LOW, "mmc1-slot-power" },
+ { MX28EVK_FEC_PHY_POWER, GPIOF_OUT_INIT_LOW, "fec-phy-power" },
+ { MX28EVK_FEC_PHY_RESET, GPIOF_DIR_OUT, "fec-phy-reset" },
};
static const struct mxs_saif_platform_data
@@ -447,25 +426,17 @@ static void __init mx28evk_init(void)
if (mx28evk_fec_get_mac())
pr_warn("%s: failed on fec mac setup\n", __func__);
+ ret = gpio_request_array(mx28evk_gpios, ARRAY_SIZE(mx28evk_gpios));
+ if (ret)
+ pr_err("One or more GPIOs failed to be requested: %d\n", ret);
mx28evk_fec_reset();
mx28_add_fec(0, &mx28_fec_pdata[0]);
mx28_add_fec(1, &mx28_fec_pdata[1]);
- ret = gpio_request_one(MX28EVK_FLEXCAN_SWITCH, GPIOF_DIR_OUT,
- "flexcan-switch");
- if (ret) {
- pr_err("failed to request gpio flexcan-switch: %d\n", ret);
- } else {
- mx28_add_flexcan(0, &mx28evk_flexcan_pdata[0]);
- mx28_add_flexcan(1, &mx28evk_flexcan_pdata[1]);
- }
+ mx28_add_flexcan(0, &mx28evk_flexcan_pdata[0]);
+ mx28_add_flexcan(1, &mx28evk_flexcan_pdata[1]);
- ret = gpio_request_array(mx28evk_lcd_gpios,
- ARRAY_SIZE(mx28evk_lcd_gpios));
- if (ret)
- pr_warn("failed to request gpio pins for lcd: %d\n", ret);
- else
- mx28_add_mxsfb(&mx28evk_mxsfb_pdata);
+ mx28_add_mxsfb(&mx28evk_mxsfb_pdata);
mxs_saif_clkmux_select(MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0);
mx28_add_saif(0, &mx28evk_mxs_saif_pdata[0]);
@@ -480,20 +451,8 @@ static void __init mx28evk_init(void)
mxs_add_platform_device("mxs-sgtl5000", 0, NULL, 0,
NULL, 0);
- /* power on mmc slot by writing 0 to the gpio */
- ret = gpio_request_one(MX28EVK_MMC0_SLOT_POWER, GPIOF_OUT_INIT_LOW,
- "mmc0-slot-power");
- if (ret)
- pr_warn("failed to request gpio mmc0-slot-power: %d\n", ret);
- else
- mx28_add_mxs_mmc(0, &mx28evk_mmc_pdata[0]);
-
- ret = gpio_request_one(MX28EVK_MMC1_SLOT_POWER, GPIOF_OUT_INIT_LOW,
- "mmc1-slot-power");
- if (ret)
- pr_warn("failed to request gpio mmc1-slot-power: %d\n", ret);
- else
- mx28_add_mxs_mmc(1, &mx28evk_mmc_pdata[1]);
+ mx28_add_mxs_mmc(0, &mx28evk_mmc_pdata[0]);
+ mx28_add_mxs_mmc(1, &mx28evk_mmc_pdata[1]);
mx28_add_rtc_stmp3xxx();
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] ARM: mx28evk: Simplify GPIO requests
2012-01-27 14:52 ` [PATCH v2] " Fabio Estevam
@ 2012-01-31 15:31 ` Shawn Guo
0 siblings, 0 replies; 6+ messages in thread
From: Shawn Guo @ 2012-01-31 15:31 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jan 27, 2012 at 12:52:32PM -0200, Fabio Estevam wrote:
> Requesting all the GPIOs on a single array (mx28evk_gpios[]) can make the
> error handling of gpio_requests much simpler.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Applied with one makeup blank line below added. Thanks.
> ---
> Changes since v1:
> - Removed empty line
> - Leave gpio_set_value(MX28EVK_FEC_PHY_RESET, 0) prior to the delay line in fec_reset
> - Put __initconst after mx28evk_gpios[]
> arch/arm/mach-mxs/mach-mx28evk.c | 71 ++++++++------------------------------
> 1 files changed, 15 insertions(+), 56 deletions(-)
>
> diff --git a/arch/arm/mach-mxs/mach-mx28evk.c b/arch/arm/mach-mxs/mach-mx28evk.c
> index fdb0a56..dad053d 100644
> --- a/arch/arm/mach-mxs/mach-mx28evk.c
> +++ b/arch/arm/mach-mxs/mach-mx28evk.c
> @@ -223,7 +223,6 @@ static const struct gpio_led_platform_data mx28evk_led_data __initconst = {
> /* fec */
> static void __init mx28evk_fec_reset(void)
> {
> - int ret;
> struct clk *clk;
>
> /* Enable fec phy clock */
> @@ -231,32 +230,7 @@ static void __init mx28evk_fec_reset(void)
> if (!IS_ERR(clk))
> clk_prepare_enable(clk);
>
> - /* Power up fec phy */
> - ret = gpio_request(MX28EVK_FEC_PHY_POWER, "fec-phy-power");
> - if (ret) {
> - pr_err("Failed to request gpio fec-phy-%s: %d\n", "power", ret);
> - return;
> - }
> -
> - ret = gpio_direction_output(MX28EVK_FEC_PHY_POWER, 0);
> - if (ret) {
> - pr_err("Failed to drive gpio fec-phy-%s: %d\n", "power", ret);
> - return;
> - }
> -
> - /* Reset fec phy */
> - ret = gpio_request(MX28EVK_FEC_PHY_RESET, "fec-phy-reset");
> - if (ret) {
> - pr_err("Failed to request gpio fec-phy-%s: %d\n", "reset", ret);
> - return;
> - }
> -
> - gpio_direction_output(MX28EVK_FEC_PHY_RESET, 0);
> - if (ret) {
> - pr_err("Failed to drive gpio fec-phy-%s: %d\n", "reset", ret);
> - return;
> - }
> -
> + gpio_set_value(MX28EVK_FEC_PHY_RESET, 0);
> mdelay(1);
> gpio_set_value(MX28EVK_FEC_PHY_RESET, 1);
> }
> @@ -417,9 +391,14 @@ static void __init mx28evk_add_regulators(void)
> static void __init mx28evk_add_regulators(void) {}
> #endif
>
> -static struct gpio mx28evk_lcd_gpios[] = {
> +static const struct gpio mx28evk_gpios[] __initconst = {
> { MX28EVK_LCD_ENABLE, GPIOF_OUT_INIT_HIGH, "lcd-enable" },
> { MX28EVK_BL_ENABLE, GPIOF_OUT_INIT_HIGH, "bl-enable" },
> + { MX28EVK_FLEXCAN_SWITCH, GPIOF_DIR_OUT, "flexcan-switch" },
> + { MX28EVK_MMC0_SLOT_POWER, GPIOF_OUT_INIT_LOW, "mmc0-slot-power" },
> + { MX28EVK_MMC1_SLOT_POWER, GPIOF_OUT_INIT_LOW, "mmc1-slot-power" },
> + { MX28EVK_FEC_PHY_POWER, GPIOF_OUT_INIT_LOW, "fec-phy-power" },
> + { MX28EVK_FEC_PHY_RESET, GPIOF_DIR_OUT, "fec-phy-reset" },
> };
>
> static const struct mxs_saif_platform_data
> @@ -447,25 +426,17 @@ static void __init mx28evk_init(void)
> if (mx28evk_fec_get_mac())
> pr_warn("%s: failed on fec mac setup\n", __func__);
>
> + ret = gpio_request_array(mx28evk_gpios, ARRAY_SIZE(mx28evk_gpios));
> + if (ret)
> + pr_err("One or more GPIOs failed to be requested: %d\n", ret);
Here, I added one blank line.
Regards,
Shawn
> mx28evk_fec_reset();
> mx28_add_fec(0, &mx28_fec_pdata[0]);
> mx28_add_fec(1, &mx28_fec_pdata[1]);
>
> - ret = gpio_request_one(MX28EVK_FLEXCAN_SWITCH, GPIOF_DIR_OUT,
> - "flexcan-switch");
> - if (ret) {
> - pr_err("failed to request gpio flexcan-switch: %d\n", ret);
> - } else {
> - mx28_add_flexcan(0, &mx28evk_flexcan_pdata[0]);
> - mx28_add_flexcan(1, &mx28evk_flexcan_pdata[1]);
> - }
> + mx28_add_flexcan(0, &mx28evk_flexcan_pdata[0]);
> + mx28_add_flexcan(1, &mx28evk_flexcan_pdata[1]);
>
> - ret = gpio_request_array(mx28evk_lcd_gpios,
> - ARRAY_SIZE(mx28evk_lcd_gpios));
> - if (ret)
> - pr_warn("failed to request gpio pins for lcd: %d\n", ret);
> - else
> - mx28_add_mxsfb(&mx28evk_mxsfb_pdata);
> + mx28_add_mxsfb(&mx28evk_mxsfb_pdata);
>
> mxs_saif_clkmux_select(MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0);
> mx28_add_saif(0, &mx28evk_mxs_saif_pdata[0]);
> @@ -480,20 +451,8 @@ static void __init mx28evk_init(void)
> mxs_add_platform_device("mxs-sgtl5000", 0, NULL, 0,
> NULL, 0);
>
> - /* power on mmc slot by writing 0 to the gpio */
> - ret = gpio_request_one(MX28EVK_MMC0_SLOT_POWER, GPIOF_OUT_INIT_LOW,
> - "mmc0-slot-power");
> - if (ret)
> - pr_warn("failed to request gpio mmc0-slot-power: %d\n", ret);
> - else
> - mx28_add_mxs_mmc(0, &mx28evk_mmc_pdata[0]);
> -
> - ret = gpio_request_one(MX28EVK_MMC1_SLOT_POWER, GPIOF_OUT_INIT_LOW,
> - "mmc1-slot-power");
> - if (ret)
> - pr_warn("failed to request gpio mmc1-slot-power: %d\n", ret);
> - else
> - mx28_add_mxs_mmc(1, &mx28evk_mmc_pdata[1]);
> + mx28_add_mxs_mmc(0, &mx28evk_mmc_pdata[0]);
> + mx28_add_mxs_mmc(1, &mx28evk_mmc_pdata[1]);
>
> mx28_add_rtc_stmp3xxx();
>
> --
> 1.7.1
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-01-31 15:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-23 14:41 [PATCH] ARM: mx28evk: Simplify GPIO requests Fabio Estevam
2012-01-26 12:19 ` Shawn Guo
2012-01-26 12:49 ` Lothar Waßmann
2012-01-26 14:17 ` Shawn Guo
2012-01-27 14:52 ` [PATCH v2] " Fabio Estevam
2012-01-31 15:31 ` Shawn Guo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.