linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ARM: mx28evk: Simplify GPIO requests for mx28evk_fec_reset
@ 2011-11-09 19:09 Fabio Estevam
  2011-11-09 19:13 ` Uwe Kleine-König
  2011-11-10  7:53 ` Sascha Hauer
  0 siblings, 2 replies; 5+ messages in thread
From: Fabio Estevam @ 2011-11-09 19:09 UTC (permalink / raw)
  To: linux-arm-kernel

Simplify GPIO requests inside mx28evk_fec_reset by using gpio_request_array.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 arch/arm/mach-mxs/mach-mx28evk.c |   29 ++++++++---------------------
 1 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-mxs/mach-mx28evk.c b/arch/arm/mach-mxs/mach-mx28evk.c
index ac2316d..c565c33 100644
--- a/arch/arm/mach-mxs/mach-mx28evk.c
+++ b/arch/arm/mach-mxs/mach-mx28evk.c
@@ -219,6 +219,11 @@ static const struct gpio_led_platform_data mx28evk_led_data __initconst = {
 	.num_leds = ARRAY_SIZE(mx28evk_leds),
 };
 
+static struct gpio mx28evk_fec_gpios[] = {
+	{ MX28EVK_FEC_PHY_POWER, GPIOF_OUT_INIT_LOW, "fec-power" },
+	{ MX28EVK_FEC_PHY_RESET, GPIOF_OUT_INIT_LOW, "fec-enable" },
+};
+
 /* fec */
 static void __init mx28evk_fec_reset(void)
 {
@@ -231,28 +236,10 @@ static void __init mx28evk_fec_reset(void)
 		clk_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);
+	ret = gpio_request_array(mx28evk_fec_gpios,
+				ARRAY_SIZE(mx28evk_fec_gpios));
 	if (ret) {
-		pr_err("Failed to drive gpio fec-phy-%s: %d\n", "reset", ret);
+		pr_err("Failed to request FEC gpios: %d\n", ret);
 		return;
 	}
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] ARM: mx28evk: Simplify GPIO requests for mx28evk_fec_reset
  2011-11-09 19:09 [PATCH] ARM: mx28evk: Simplify GPIO requests for mx28evk_fec_reset Fabio Estevam
@ 2011-11-09 19:13 ` Uwe Kleine-König
  2011-11-10  7:53 ` Sascha Hauer
  1 sibling, 0 replies; 5+ messages in thread
From: Uwe Kleine-König @ 2011-11-09 19:13 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On Wed, Nov 09, 2011 at 05:09:02PM -0200, Fabio Estevam wrote:
> Simplify GPIO requests inside mx28evk_fec_reset by using gpio_request_array.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  arch/arm/mach-mxs/mach-mx28evk.c |   29 ++++++++---------------------
>  1 files changed, 8 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm/mach-mxs/mach-mx28evk.c b/arch/arm/mach-mxs/mach-mx28evk.c
> index ac2316d..c565c33 100644
> --- a/arch/arm/mach-mxs/mach-mx28evk.c
> +++ b/arch/arm/mach-mxs/mach-mx28evk.c
> @@ -219,6 +219,11 @@ static const struct gpio_led_platform_data mx28evk_led_data __initconst = {
>  	.num_leds = ARRAY_SIZE(mx28evk_leds),
>  };
>  
> +static struct gpio mx28evk_fec_gpios[] = {
const + __initconst please.

> +	{ MX28EVK_FEC_PHY_POWER, GPIOF_OUT_INIT_LOW, "fec-power" },
> +	{ MX28EVK_FEC_PHY_RESET, GPIOF_OUT_INIT_LOW, "fec-enable" },
> +};
> +
>  /* fec */
>  static void __init mx28evk_fec_reset(void)
>  {
> @@ -231,28 +236,10 @@ static void __init mx28evk_fec_reset(void)
>  		clk_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);
> +	ret = gpio_request_array(mx28evk_fec_gpios,
> +				ARRAY_SIZE(mx28evk_fec_gpios));
>  	if (ret) {
> -		pr_err("Failed to drive gpio fec-phy-%s: %d\n", "reset", ret);
> +		pr_err("Failed to request FEC gpios: %d\n", ret);
>  		return;
>  	}
>  
> -- 
> 1.7.1
> 
> 
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] ARM: mx28evk: Simplify GPIO requests for mx28evk_fec_reset
  2011-11-09 19:09 [PATCH] ARM: mx28evk: Simplify GPIO requests for mx28evk_fec_reset Fabio Estevam
  2011-11-09 19:13 ` Uwe Kleine-König
@ 2011-11-10  7:53 ` Sascha Hauer
  2011-11-10  8:11   ` Dong Aisheng-B29396
  1 sibling, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2011-11-10  7:53 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Fabio,

On Wed, Nov 09, 2011 at 05:09:02PM -0200, Fabio Estevam wrote:
> Simplify GPIO requests inside mx28evk_fec_reset by using gpio_request_array.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  arch/arm/mach-mxs/mach-mx28evk.c |   29 ++++++++---------------------
>  1 files changed, 8 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm/mach-mxs/mach-mx28evk.c b/arch/arm/mach-mxs/mach-mx28evk.c
> index ac2316d..c565c33 100644
> --- a/arch/arm/mach-mxs/mach-mx28evk.c
> +++ b/arch/arm/mach-mxs/mach-mx28evk.c
> @@ -219,6 +219,11 @@ static const struct gpio_led_platform_data mx28evk_led_data __initconst = {
>  	.num_leds = ARRAY_SIZE(mx28evk_leds),
>  };
>  
> +static struct gpio mx28evk_fec_gpios[] = {
> +	{ MX28EVK_FEC_PHY_POWER, GPIOF_OUT_INIT_LOW, "fec-power" },
> +	{ MX28EVK_FEC_PHY_RESET, GPIOF_OUT_INIT_LOW, "fec-enable" },
> +};
> +
>  /* fec */
>  static void __init mx28evk_fec_reset(void)
>  {
> @@ -231,28 +236,10 @@ static void __init mx28evk_fec_reset(void)
>  		clk_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);
> +	ret = gpio_request_array(mx28evk_fec_gpios,
> +				ARRAY_SIZE(mx28evk_fec_gpios));

I think it makes more sense to create an array per board and not per
board function. In the mx28evk file we use gpios for the fec, we have
a gpio_request_one for the flexcan switch and a gpio_request_array for
the lcd pins. All these could be added to a single mx28evk_gpios array.
Then it would be easy to see which gpios a board uses and it would be
trivial to add additional gpios. The same applies for other boards
aswell of course.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] ARM: mx28evk: Simplify GPIO requests for mx28evk_fec_reset
  2011-11-10  7:53 ` Sascha Hauer
@ 2011-11-10  8:11   ` Dong Aisheng-B29396
  2011-11-10  8:47     ` Sascha Hauer
  0 siblings, 1 reply; 5+ messages in thread
From: Dong Aisheng-B29396 @ 2011-11-10  8:11 UTC (permalink / raw)
  To: linux-arm-kernel

> -----Original Message-----
> From: linux-arm-kernel-bounces at lists.infradead.org [mailto:linux-arm-
> kernel-bounces at lists.infradead.org] On Behalf Of Sascha Hauer
> Sent: Thursday, November 10, 2011 3:53 PM
> To: Estevam Fabio-R49496
> Cc: Guo Shawn-R65073; kernel at pengutronix.de; linux-arm-
> kernel at lists.infradead.org
> Subject: Re: [PATCH] ARM: mx28evk: Simplify GPIO requests for
> mx28evk_fec_reset
> 
> Hi Fabio,
> 
> On Wed, Nov 09, 2011 at 05:09:02PM -0200, Fabio Estevam wrote:
> > Simplify GPIO requests inside mx28evk_fec_reset by using
> gpio_request_array.
> >
> > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> > ---
> >  arch/arm/mach-mxs/mach-mx28evk.c |   29 ++++++++---------------------
> >  1 files changed, 8 insertions(+), 21 deletions(-)
> >
> > diff --git a/arch/arm/mach-mxs/mach-mx28evk.c
> > b/arch/arm/mach-mxs/mach-mx28evk.c
> > index ac2316d..c565c33 100644
> > --- a/arch/arm/mach-mxs/mach-mx28evk.c
> > +++ b/arch/arm/mach-mxs/mach-mx28evk.c
> > @@ -219,6 +219,11 @@ static const struct gpio_led_platform_data
> mx28evk_led_data __initconst = {
> >  	.num_leds = ARRAY_SIZE(mx28evk_leds),  };
> >
> > +static struct gpio mx28evk_fec_gpios[] = {
> > +	{ MX28EVK_FEC_PHY_POWER, GPIOF_OUT_INIT_LOW, "fec-power" },
> > +	{ MX28EVK_FEC_PHY_RESET, GPIOF_OUT_INIT_LOW, "fec-enable" }, };
> > +
> >  /* fec */
> >  static void __init mx28evk_fec_reset(void)  { @@ -231,28 +236,10 @@
> > static void __init mx28evk_fec_reset(void)
> >  		clk_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);
> > +	ret = gpio_request_array(mx28evk_fec_gpios,
> > +				ARRAY_SIZE(mx28evk_fec_gpios));
> 
> I think it makes more sense to create an array per board and not per
> board function. In the mx28evk file we use gpios for the fec, we have a
> gpio_request_one for the flexcan switch and a gpio_request_array for the
> lcd pins. All these could be added to a single mx28evk_gpios array.
> Then it would be easy to see which gpios a board uses and it would be
> trivial to add additional gpios. The same applies for other boards aswell
> of course.
> 
One question is that if one gpio, assume in one function like fec, request fail
will cause the whole gpio array request fail.
How should we handle the error for each function?

Originally we may do like:
If (!gpio_request_array(fec_gpios))
	mxs_add_fec(0);
but after change, we do not know which function gpio request fails.

> Sascha
> 
> --
> Pengutronix e.K.                           |
> |
> Industrial Linux Solutions                 | http://www.pengutronix.de/
> |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555
> |
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Regards
Dong Aisheng

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] ARM: mx28evk: Simplify GPIO requests for mx28evk_fec_reset
  2011-11-10  8:11   ` Dong Aisheng-B29396
@ 2011-11-10  8:47     ` Sascha Hauer
  0 siblings, 0 replies; 5+ messages in thread
From: Sascha Hauer @ 2011-11-10  8:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Nov 10, 2011 at 08:11:10AM +0000, Dong Aisheng-B29396 wrote:
> > 
> > I think it makes more sense to create an array per board and not per
> > board function. In the mx28evk file we use gpios for the fec, we have a
> > gpio_request_one for the flexcan switch and a gpio_request_array for the
> > lcd pins. All these could be added to a single mx28evk_gpios array.
> > Then it would be easy to see which gpios a board uses and it would be
> > trivial to add additional gpios. The same applies for other boards aswell
> > of course.
> > 
> One question is that if one gpio, assume in one function like fec, request fail
> will cause the whole gpio array request fail.

They don't fail. If they do it means we have a duplicate entry in the
table or otherwise screwed up the kernel. I think a single pr_err("bad
things may happen, continue and hope for the best") is enough here.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-11-10  8:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-09 19:09 [PATCH] ARM: mx28evk: Simplify GPIO requests for mx28evk_fec_reset Fabio Estevam
2011-11-09 19:13 ` Uwe Kleine-König
2011-11-10  7:53 ` Sascha Hauer
2011-11-10  8:11   ` Dong Aisheng-B29396
2011-11-10  8:47     ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).