| Message ID | 1323767568-9565-1-git-send-email-w.sang@pengutronix.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Anatolij Gustschin |
| Headers | show |
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
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
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 >
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.
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.
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;
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(-)