linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121
@ 2011-12-13  9:12 Wolfram Sang
  2011-12-13  9:52 ` Anatolij Gustschin
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Wolfram Sang @ 2011-12-13  9:12 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Anatolij Gustschin, linux-kernel, Linus Walleij

Add a 5121-custom reject if an input-only pin is requested to be output
(see 18.3.1.1 in the refman). Also, rewrite mach-specific quirk setup to
consume less lines which scales better.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/gpio/gpio-mpc8xxx.c |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
index ec3fcf0..25dc736 100644
--- a/drivers/gpio/gpio-mpc8xxx.c
+++ b/drivers/gpio/gpio-mpc8xxx.c
@@ -115,6 +115,14 @@ static int mpc8xxx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
 	return 0;
 }
 
+static int mpc5121_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	/* GPIO 28..31 are input only on MPC5121 */
+	if (gpio >= 28)
+		return -EINVAL;
+
+	return mpc8xxx_gpio_dir_out(gc, gpio, val);
+}
 static int mpc8xxx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
 {
 	struct of_mm_gpio_chip *mm = to_of_mm_gpio_chip(gc);
@@ -340,11 +348,10 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
 	mm_gc->save_regs = mpc8xxx_gpio_save_regs;
 	gc->ngpio = MPC8XXX_GPIO_PINS;
 	gc->direction_input = mpc8xxx_gpio_dir_in;
-	gc->direction_output = mpc8xxx_gpio_dir_out;
-	if (of_device_is_compatible(np, "fsl,mpc8572-gpio"))
-		gc->get = mpc8572_gpio_get;
-	else
-		gc->get = mpc8xxx_gpio_get;
+	gc->direction_output = of_device_is_compatible(np, "fsl,mpc5121-gpio") ?
+		mpc5121_gpio_dir_out : mpc8xxx_gpio_dir_out;
+	gc->get = of_device_is_compatible(np, "fsl,mpc8572-gpio") ?
+		mpc8572_gpio_get : mpc8xxx_gpio_get;
 	gc->set = mpc8xxx_gpio_set;
 	gc->to_irq = mpc8xxx_gpio_to_irq;
 
-- 
1.7.7.1

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

* Re: [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121
  2011-12-13  9:12 [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121 Wolfram Sang
@ 2011-12-13  9:52 ` Anatolij Gustschin
  2011-12-13 10:03   ` Wolfram Sang
  2011-12-13 18:07 ` Grant Likely
  2011-12-13 18:16 ` Grant Likely
  2 siblings, 1 reply; 6+ messages in thread
From: Anatolij Gustschin @ 2011-12-13  9:52 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linuxppc-dev, linux-kernel, Linus Walleij

Hi Wolfram,

Looks mostly good to me. Please see minor comments below.

On Tue, 13 Dec 2011 10:12:48 +0100
Wolfram Sang <w.sang@pengutronix.de> wrote:
...
> diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> index ec3fcf0..25dc736 100644
> --- a/drivers/gpio/gpio-mpc8xxx.c
> +++ b/drivers/gpio/gpio-mpc8xxx.c
> @@ -115,6 +115,14 @@ static int mpc8xxx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
>  	return 0;
>  }
>  
> +static int mpc5121_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)

Line over 80 chars.

> +{
> +	/* GPIO 28..31 are input only on MPC5121 */
> +	if (gpio >= 28)
> +		return -EINVAL;
> +
> +	return mpc8xxx_gpio_dir_out(gc, gpio, val);
> +}
>  static int mpc8xxx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)

Please separate by an empty line. Thanks.

Anatolij

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

* Re: [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121
  2011-12-13  9:52 ` Anatolij Gustschin
@ 2011-12-13 10:03   ` Wolfram Sang
  0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2011-12-13 10:03 UTC (permalink / raw)
  To: Anatolij Gustschin; +Cc: linuxppc-dev, linux-kernel, Linus Walleij

[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]

On Tue, Dec 13, 2011 at 10:52:43AM +0100, Anatolij Gustschin wrote:
> Hi Wolfram,
> 
> Looks mostly good to me. Please see minor comments below.
> 
> On Tue, 13 Dec 2011 10:12:48 +0100
> Wolfram Sang <w.sang@pengutronix.de> wrote:
> ...
> > diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> > index ec3fcf0..25dc736 100644
> > --- a/drivers/gpio/gpio-mpc8xxx.c
> > +++ b/drivers/gpio/gpio-mpc8xxx.c
> > @@ -115,6 +115,14 @@ static int mpc8xxx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> >  	return 0;
> >  }
> >  
> > +static int mpc5121_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> 
> Line over 80 chars.

In this case, I think the alternative would be far less readable.

> > +{
> > +	/* GPIO 28..31 are input only on MPC5121 */
> > +	if (gpio >= 28)
> > +		return -EINVAL;
> > +
> > +	return mpc8xxx_gpio_dir_out(gc, gpio, val);
> > +}
> >  static int mpc8xxx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> 
> Please separate by an empty line. Thanks.

Ups, yes.

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

* Re: [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121
  2011-12-13  9:12 [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121 Wolfram Sang
  2011-12-13  9:52 ` Anatolij Gustschin
@ 2011-12-13 18:07 ` Grant Likely
  2011-12-13 18:16 ` Grant Likely
  2 siblings, 0 replies; 6+ messages in thread
From: Grant Likely @ 2011-12-13 18:07 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linuxppc-dev, Anatolij Gustschin, linux-kernel, Linus Walleij

On Tue, Dec 13, 2011 at 10:12:48AM +0100, Wolfram Sang wrote:
> Add a 5121-custom reject if an input-only pin is requested to be output
> (see 18.3.1.1 in the refman). Also, rewrite mach-specific quirk setup to
> consume less lines which scales better.
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>

Applied, thanks.

g.

> ---
>  drivers/gpio/gpio-mpc8xxx.c |   17 ++++++++++++-----
>  1 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> index ec3fcf0..25dc736 100644
> --- a/drivers/gpio/gpio-mpc8xxx.c
> +++ b/drivers/gpio/gpio-mpc8xxx.c
> @@ -115,6 +115,14 @@ static int mpc8xxx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
>  	return 0;
>  }
>  
> +static int mpc5121_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> +	/* GPIO 28..31 are input only on MPC5121 */
> +	if (gpio >= 28)
> +		return -EINVAL;
> +
> +	return mpc8xxx_gpio_dir_out(gc, gpio, val);
> +}
>  static int mpc8xxx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
>  {
>  	struct of_mm_gpio_chip *mm = to_of_mm_gpio_chip(gc);
> @@ -340,11 +348,10 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
>  	mm_gc->save_regs = mpc8xxx_gpio_save_regs;
>  	gc->ngpio = MPC8XXX_GPIO_PINS;
>  	gc->direction_input = mpc8xxx_gpio_dir_in;
> -	gc->direction_output = mpc8xxx_gpio_dir_out;
> -	if (of_device_is_compatible(np, "fsl,mpc8572-gpio"))
> -		gc->get = mpc8572_gpio_get;
> -	else
> -		gc->get = mpc8xxx_gpio_get;
> +	gc->direction_output = of_device_is_compatible(np, "fsl,mpc5121-gpio") ?
> +		mpc5121_gpio_dir_out : mpc8xxx_gpio_dir_out;
> +	gc->get = of_device_is_compatible(np, "fsl,mpc8572-gpio") ?
> +		mpc8572_gpio_get : mpc8xxx_gpio_get;
>  	gc->set = mpc8xxx_gpio_set;
>  	gc->to_irq = mpc8xxx_gpio_to_irq;
>  
> -- 
> 1.7.7.1
> 

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

* Re: [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121
  2011-12-13  9:12 [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121 Wolfram Sang
  2011-12-13  9:52 ` Anatolij Gustschin
  2011-12-13 18:07 ` Grant Likely
@ 2011-12-13 18:16 ` Grant Likely
  2011-12-13 18:23   ` Wolfram Sang
  2 siblings, 1 reply; 6+ messages in thread
From: Grant Likely @ 2011-12-13 18:16 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linuxppc-dev, Anatolij Gustschin, linux-kernel, Linus Walleij

On Tue, Dec 13, 2011 at 10:12:48AM +0100, Wolfram Sang wrote:
> Add a 5121-custom reject if an input-only pin is requested to be output
> (see 18.3.1.1 in the refman). Also, rewrite mach-specific quirk setup to
> consume less lines which scales better.
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
>  drivers/gpio/gpio-mpc8xxx.c |   17 ++++++++++++-----
>  1 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> index ec3fcf0..25dc736 100644
> --- a/drivers/gpio/gpio-mpc8xxx.c
> +++ b/drivers/gpio/gpio-mpc8xxx.c
> @@ -115,6 +115,14 @@ static int mpc8xxx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
>  	return 0;
>  }
>  
> +static int mpc5121_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> +	/* GPIO 28..31 are input only on MPC5121 */
> +	if (gpio >= 28)
> +		return -EINVAL;
> +
> +	return mpc8xxx_gpio_dir_out(gc, gpio, val);
> +}
>  static int mpc8xxx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)

This actually caused a build failure.  mpc8xxx_gpio_dir_out() was used before
it was declared.  I moved the symbol to immediately below and applied anyway,
but how did it compile for you?  Should I drop this patch until you retest?

g.

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

* Re: [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121
  2011-12-13 18:16 ` Grant Likely
@ 2011-12-13 18:23   ` Wolfram Sang
  0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2011-12-13 18:23 UTC (permalink / raw)
  To: Grant Likely
  Cc: linuxppc-dev, Anatolij Gustschin, linux-kernel, Linus Walleij

[-- Attachment #1: Type: text/plain, Size: 1681 bytes --]

On Tue, Dec 13, 2011 at 11:16:59AM -0700, Grant Likely wrote:
> On Tue, Dec 13, 2011 at 10:12:48AM +0100, Wolfram Sang wrote:
> > Add a 5121-custom reject if an input-only pin is requested to be output
> > (see 18.3.1.1 in the refman). Also, rewrite mach-specific quirk setup to
> > consume less lines which scales better.
> > 
> > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> > ---
> >  drivers/gpio/gpio-mpc8xxx.c |   17 ++++++++++++-----
> >  1 files changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> > index ec3fcf0..25dc736 100644
> > --- a/drivers/gpio/gpio-mpc8xxx.c
> > +++ b/drivers/gpio/gpio-mpc8xxx.c
> > @@ -115,6 +115,14 @@ static int mpc8xxx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> >  	return 0;
> >  }
> >  
> > +static int mpc5121_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> > +{
> > +	/* GPIO 28..31 are input only on MPC5121 */
> > +	if (gpio >= 28)
> > +		return -EINVAL;
> > +
> > +	return mpc8xxx_gpio_dir_out(gc, gpio, val);
> > +}
> >  static int mpc8xxx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> 
> This actually caused a build failure.  mpc8xxx_gpio_dir_out() was used before
> it was declared.  I moved the symbol to immediately below and applied anyway,
> but how did it compile for you?  Should I drop this patch until you retest?

Huh, I am surprised as well. Will investigate tomorrow. Sorry for the
inconvenience.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

end of thread, other threads:[~2011-12-13 18:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-13  9:12 [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121 Wolfram Sang
2011-12-13  9:52 ` Anatolij Gustschin
2011-12-13 10:03   ` Wolfram Sang
2011-12-13 18:07 ` Grant Likely
2011-12-13 18:16 ` Grant Likely
2011-12-13 18:23   ` Wolfram Sang

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).