* [RFT][PATCH v2 1/3] gpio: mpc8xxx: Fixup setting gpio direction output
@ 2016-02-22 7:22 Axel Lin
2016-02-22 7:24 ` [RFT][PATCH v2 2/3] gpio: mpc8xxx: Remove *read_reg and *write_reg from struct mpc8xxx_gpio_chip Axel Lin
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Axel Lin @ 2016-02-22 7:22 UTC (permalink / raw)
To: Linus Walleij
Cc: Liu Gang, Alexandre Courbot, Ricardo Ribalda Delgado,
Alexander Stein, linux-gpio
For devtype with specific gpio_dir_out implementation, current code is
wrong because below code sets both gc->direction_output and
mpc8xxx_gc->direction_output to the same function.
gc->direction_output = devtype->gpio_dir_out ?: gc->direction_output;
mpc8xxx_gc->direction_output = gc->direction_output;
Set mpc8xxx_gc->direction_output = gc->direction_output first to fix it.
This way mpc8xxx_gc->direction_output actually calls the standard
bgpio_dir_out() to update register.
Fixes: commit 42178e2a1e42 ("drivers/gpio: Switch gpio-mpc8xxx to use gpio-generic")
Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
drivers/gpio/gpio-mpc8xxx.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
index ecdb27a..d2472c5 100644
--- a/drivers/gpio/gpio-mpc8xxx.c
+++ b/drivers/gpio/gpio-mpc8xxx.c
@@ -334,6 +334,7 @@ static int mpc8xxx_probe(struct platform_device *pdev)
mpc8xxx_gc->read_reg = gc->read_reg;
mpc8xxx_gc->write_reg = gc->write_reg;
+ mpc8xxx_gc->direction_output = gc->direction_output;
if (!devtype)
devtype = &mpc8xxx_gpio_devtype_default;
@@ -348,8 +349,6 @@ static int mpc8xxx_probe(struct platform_device *pdev)
gc->get = devtype->gpio_get ?: gc->get;
gc->to_irq = mpc8xxx_gpio_to_irq;
- mpc8xxx_gc->direction_output = gc->direction_output;
-
ret = gpiochip_add_data(gc, mpc8xxx_gc);
if (ret) {
pr_err("%s: GPIO chip registration failed with status %d\n",
--
2.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [RFT][PATCH v2 2/3] gpio: mpc8xxx: Remove *read_reg and *write_reg from struct mpc8xxx_gpio_chip 2016-02-22 7:22 [RFT][PATCH v2 1/3] gpio: mpc8xxx: Fixup setting gpio direction output Axel Lin @ 2016-02-22 7:24 ` Axel Lin 2016-03-02 7:31 ` Gang Liu 2016-03-09 3:43 ` Linus Walleij 2016-02-22 7:24 ` [RFT][PATCH v2 3/3] gpio: mpc8xxx: Slightly update the code for better readability Axel Lin ` (2 subsequent siblings) 3 siblings, 2 replies; 9+ messages in thread From: Axel Lin @ 2016-02-22 7:24 UTC (permalink / raw) To: Linus Walleij Cc: Liu Gang, Alexandre Courbot, Ricardo Ribalda Delgado, Alexander Stein, linux-gpio *read_reg and *write_reg can be removed because at all the places to call them, we can just use gc->read_reg/gc->write_reg instead. Signed-off-by: Axel Lin <axel.lin@ingics.com> --- drivers/gpio/gpio-mpc8xxx.c | 48 ++++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c index d2472c5..bc042ad6 100644 --- a/drivers/gpio/gpio-mpc8xxx.c +++ b/drivers/gpio/gpio-mpc8xxx.c @@ -37,9 +37,6 @@ struct mpc8xxx_gpio_chip { void __iomem *regs; raw_spinlock_t lock; - unsigned long (*read_reg)(void __iomem *reg); - void (*write_reg)(void __iomem *reg, unsigned long data); - int (*direction_output)(struct gpio_chip *chip, unsigned offset, int value); @@ -58,8 +55,8 @@ static int mpc8572_gpio_get(struct gpio_chip *gc, unsigned int gpio) struct mpc8xxx_gpio_chip *mpc8xxx_gc = gpiochip_get_data(gc); u32 out_mask, out_shadow; - out_mask = mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_DIR); - val = mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_DAT) & ~out_mask; + out_mask = gc->read_reg(mpc8xxx_gc->regs + GPIO_DIR); + val = gc->read_reg(mpc8xxx_gc->regs + GPIO_DAT) & ~out_mask; out_shadow = gc->bgpio_data & out_mask; return !!((val | out_shadow) & gc->pin2mask(gc, gpio)); @@ -101,10 +98,11 @@ static void mpc8xxx_gpio_irq_cascade(struct irq_desc *desc) { struct mpc8xxx_gpio_chip *mpc8xxx_gc = irq_desc_get_handler_data(desc); struct irq_chip *chip = irq_desc_get_chip(desc); + struct gpio_chip *gc = &mpc8xxx_gc->gc; unsigned int mask; - mask = mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_IER) - & mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_IMR); + mask = gc->read_reg(mpc8xxx_gc->regs + GPIO_IER) + & gc->read_reg(mpc8xxx_gc->regs + GPIO_IMR); if (mask) generic_handle_irq(irq_linear_revmap(mpc8xxx_gc->irq, 32 - ffs(mask))); @@ -120,8 +118,8 @@ static void mpc8xxx_irq_unmask(struct irq_data *d) raw_spin_lock_irqsave(&mpc8xxx_gc->lock, flags); - mpc8xxx_gc->write_reg(mpc8xxx_gc->regs + GPIO_IMR, - mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_IMR) + gc->write_reg(mpc8xxx_gc->regs + GPIO_IMR, + gc->read_reg(mpc8xxx_gc->regs + GPIO_IMR) | gc->pin2mask(gc, irqd_to_hwirq(d))); raw_spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags); @@ -135,8 +133,8 @@ static void mpc8xxx_irq_mask(struct irq_data *d) raw_spin_lock_irqsave(&mpc8xxx_gc->lock, flags); - mpc8xxx_gc->write_reg(mpc8xxx_gc->regs + GPIO_IMR, - mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_IMR) + gc->write_reg(mpc8xxx_gc->regs + GPIO_IMR, + gc->read_reg(mpc8xxx_gc->regs + GPIO_IMR) & ~(gc->pin2mask(gc, irqd_to_hwirq(d)))); raw_spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags); @@ -147,8 +145,8 @@ static void mpc8xxx_irq_ack(struct irq_data *d) struct mpc8xxx_gpio_chip *mpc8xxx_gc = irq_data_get_irq_chip_data(d); struct gpio_chip *gc = &mpc8xxx_gc->gc; - mpc8xxx_gc->write_reg(mpc8xxx_gc->regs + GPIO_IER, - gc->pin2mask(gc, irqd_to_hwirq(d))); + gc->write_reg(mpc8xxx_gc->regs + GPIO_IER, + gc->pin2mask(gc, irqd_to_hwirq(d))); } static int mpc8xxx_irq_set_type(struct irq_data *d, unsigned int flow_type) @@ -160,16 +158,16 @@ static int mpc8xxx_irq_set_type(struct irq_data *d, unsigned int flow_type) switch (flow_type) { case IRQ_TYPE_EDGE_FALLING: raw_spin_lock_irqsave(&mpc8xxx_gc->lock, flags); - mpc8xxx_gc->write_reg(mpc8xxx_gc->regs + GPIO_ICR, - mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_ICR) + gc->write_reg(mpc8xxx_gc->regs + GPIO_ICR, + gc->read_reg(mpc8xxx_gc->regs + GPIO_ICR) | gc->pin2mask(gc, irqd_to_hwirq(d))); raw_spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags); break; case IRQ_TYPE_EDGE_BOTH: raw_spin_lock_irqsave(&mpc8xxx_gc->lock, flags); - mpc8xxx_gc->write_reg(mpc8xxx_gc->regs + GPIO_ICR, - mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_ICR) + gc->write_reg(mpc8xxx_gc->regs + GPIO_ICR, + gc->read_reg(mpc8xxx_gc->regs + GPIO_ICR) & ~(gc->pin2mask(gc, irqd_to_hwirq(d)))); raw_spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags); break; @@ -184,6 +182,7 @@ static int mpc8xxx_irq_set_type(struct irq_data *d, unsigned int flow_type) static int mpc512x_irq_set_type(struct irq_data *d, unsigned int flow_type) { struct mpc8xxx_gpio_chip *mpc8xxx_gc = irq_data_get_irq_chip_data(d); + struct gpio_chip *gc = &mpc8xxx_gc->gc; unsigned long gpio = irqd_to_hwirq(d); void __iomem *reg; unsigned int shift; @@ -201,8 +200,7 @@ static int mpc512x_irq_set_type(struct irq_data *d, unsigned int flow_type) case IRQ_TYPE_EDGE_FALLING: case IRQ_TYPE_LEVEL_LOW: raw_spin_lock_irqsave(&mpc8xxx_gc->lock, flags); - mpc8xxx_gc->write_reg(reg, - (mpc8xxx_gc->read_reg(reg) & ~(3 << shift)) + gc->write_reg(reg, (gc->read_reg(reg) & ~(3 << shift)) | (2 << shift)); raw_spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags); break; @@ -210,16 +208,14 @@ static int mpc512x_irq_set_type(struct irq_data *d, unsigned int flow_type) case IRQ_TYPE_EDGE_RISING: case IRQ_TYPE_LEVEL_HIGH: raw_spin_lock_irqsave(&mpc8xxx_gc->lock, flags); - mpc8xxx_gc->write_reg(reg, - (mpc8xxx_gc->read_reg(reg) & ~(3 << shift)) + gc->write_reg(reg, (gc->read_reg(reg) & ~(3 << shift)) | (1 << shift)); raw_spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags); break; case IRQ_TYPE_EDGE_BOTH: raw_spin_lock_irqsave(&mpc8xxx_gc->lock, flags); - mpc8xxx_gc->write_reg(reg, - (mpc8xxx_gc->read_reg(reg) & ~(3 << shift))); + gc->write_reg(reg, (gc->read_reg(reg) & ~(3 << shift))); raw_spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags); break; @@ -332,8 +328,6 @@ static int mpc8xxx_probe(struct platform_device *pdev) dev_dbg(&pdev->dev, "GPIO registers are BIG endian\n"); } - mpc8xxx_gc->read_reg = gc->read_reg; - mpc8xxx_gc->write_reg = gc->write_reg; mpc8xxx_gc->direction_output = gc->direction_output; if (!devtype) @@ -366,8 +360,8 @@ static int mpc8xxx_probe(struct platform_device *pdev) return 0; /* ack and mask all irqs */ - mpc8xxx_gc->write_reg(mpc8xxx_gc->regs + GPIO_IER, 0xffffffff); - mpc8xxx_gc->write_reg(mpc8xxx_gc->regs + GPIO_IMR, 0); + gc->write_reg(mpc8xxx_gc->regs + GPIO_IER, 0xffffffff); + gc->write_reg(mpc8xxx_gc->regs + GPIO_IMR, 0); irq_set_chained_handler_and_data(mpc8xxx_gc->irqn, mpc8xxx_gpio_irq_cascade, mpc8xxx_gc); -- 2.1.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFT][PATCH v2 2/3] gpio: mpc8xxx: Remove *read_reg and *write_reg from struct mpc8xxx_gpio_chip 2016-02-22 7:24 ` [RFT][PATCH v2 2/3] gpio: mpc8xxx: Remove *read_reg and *write_reg from struct mpc8xxx_gpio_chip Axel Lin @ 2016-03-02 7:31 ` Gang Liu 2016-03-09 3:43 ` Linus Walleij 1 sibling, 0 replies; 9+ messages in thread From: Gang Liu @ 2016-03-02 7:31 UTC (permalink / raw) To: Axel Lin, Linus Walleij Cc: Alexandre Courbot, Ricardo Ribalda Delgado, Alexander Stein, linux-gpio@vger.kernel.org On 2/22/2016 3:24 PM, Axel Lin wrote: > *read_reg and *write_reg can be removed because at all the places to call > them, we can just use gc->read_reg/gc->write_reg instead. > > Signed-off-by: Axel Lin <axel.lin@ingics.com> > --- > drivers/gpio/gpio-mpc8xxx.c | 48 ++++++++++++++++++++------------------------- > 1 file changed, 21 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c > index d2472c5..bc042ad6 100644 > --- a/drivers/gpio/gpio-mpc8xxx.c > +++ b/drivers/gpio/gpio-mpc8xxx.c > @@ -37,9 +37,6 @@ struct mpc8xxx_gpio_chip { > void __iomem *regs; > raw_spinlock_t lock; > > - unsigned long (*read_reg)(void __iomem *reg); > - void (*write_reg)(void __iomem *reg, unsigned long data); > - > int (*direction_output)(struct gpio_chip *chip, > unsigned offset, int value); > > @@ -58,8 +55,8 @@ static int mpc8572_gpio_get(struct gpio_chip *gc, unsigned int gpio) > struct mpc8xxx_gpio_chip *mpc8xxx_gc = gpiochip_get_data(gc); > u32 out_mask, out_shadow; > > - out_mask = mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_DIR); > - val = mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_DAT) & ~out_mask; > + out_mask = gc->read_reg(mpc8xxx_gc->regs + GPIO_DIR); > + val = gc->read_reg(mpc8xxx_gc->regs + GPIO_DAT) & ~out_mask; > out_shadow = gc->bgpio_data & out_mask; > > return !!((val | out_shadow) & gc->pin2mask(gc, gpio)); > @@ -101,10 +98,11 @@ static void mpc8xxx_gpio_irq_cascade(struct irq_desc *desc) > { > struct mpc8xxx_gpio_chip *mpc8xxx_gc = irq_desc_get_handler_data(desc); > struct irq_chip *chip = irq_desc_get_chip(desc); > + struct gpio_chip *gc = &mpc8xxx_gc->gc; > unsigned int mask; > > - mask = mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_IER) > - & mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_IMR); > + mask = gc->read_reg(mpc8xxx_gc->regs + GPIO_IER) > + & gc->read_reg(mpc8xxx_gc->regs + GPIO_IMR); > if (mask) > generic_handle_irq(irq_linear_revmap(mpc8xxx_gc->irq, > 32 - ffs(mask))); > @@ -120,8 +118,8 @@ static void mpc8xxx_irq_unmask(struct irq_data *d) > > raw_spin_lock_irqsave(&mpc8xxx_gc->lock, flags); > > - mpc8xxx_gc->write_reg(mpc8xxx_gc->regs + GPIO_IMR, > - mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_IMR) > + gc->write_reg(mpc8xxx_gc->regs + GPIO_IMR, > + gc->read_reg(mpc8xxx_gc->regs + GPIO_IMR) > | gc->pin2mask(gc, irqd_to_hwirq(d))); > > raw_spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags); > @@ -135,8 +133,8 @@ static void mpc8xxx_irq_mask(struct irq_data *d) > > raw_spin_lock_irqsave(&mpc8xxx_gc->lock, flags); > > - mpc8xxx_gc->write_reg(mpc8xxx_gc->regs + GPIO_IMR, > - mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_IMR) > + gc->write_reg(mpc8xxx_gc->regs + GPIO_IMR, > + gc->read_reg(mpc8xxx_gc->regs + GPIO_IMR) > & ~(gc->pin2mask(gc, irqd_to_hwirq(d)))); > > raw_spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags); > @@ -147,8 +145,8 @@ static void mpc8xxx_irq_ack(struct irq_data *d) > struct mpc8xxx_gpio_chip *mpc8xxx_gc = irq_data_get_irq_chip_data(d); > struct gpio_chip *gc = &mpc8xxx_gc->gc; > > - mpc8xxx_gc->write_reg(mpc8xxx_gc->regs + GPIO_IER, > - gc->pin2mask(gc, irqd_to_hwirq(d))); > + gc->write_reg(mpc8xxx_gc->regs + GPIO_IER, > + gc->pin2mask(gc, irqd_to_hwirq(d))); > } > > static int mpc8xxx_irq_set_type(struct irq_data *d, unsigned int flow_type) > @@ -160,16 +158,16 @@ static int mpc8xxx_irq_set_type(struct irq_data *d, unsigned int flow_type) > switch (flow_type) { > case IRQ_TYPE_EDGE_FALLING: > raw_spin_lock_irqsave(&mpc8xxx_gc->lock, flags); > - mpc8xxx_gc->write_reg(mpc8xxx_gc->regs + GPIO_ICR, > - mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_ICR) > + gc->write_reg(mpc8xxx_gc->regs + GPIO_ICR, > + gc->read_reg(mpc8xxx_gc->regs + GPIO_ICR) > | gc->pin2mask(gc, irqd_to_hwirq(d))); > raw_spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags); > break; > > case IRQ_TYPE_EDGE_BOTH: > raw_spin_lock_irqsave(&mpc8xxx_gc->lock, flags); > - mpc8xxx_gc->write_reg(mpc8xxx_gc->regs + GPIO_ICR, > - mpc8xxx_gc->read_reg(mpc8xxx_gc->regs + GPIO_ICR) > + gc->write_reg(mpc8xxx_gc->regs + GPIO_ICR, > + gc->read_reg(mpc8xxx_gc->regs + GPIO_ICR) > & ~(gc->pin2mask(gc, irqd_to_hwirq(d)))); > raw_spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags); > break; > @@ -184,6 +182,7 @@ static int mpc8xxx_irq_set_type(struct irq_data *d, unsigned int flow_type) > static int mpc512x_irq_set_type(struct irq_data *d, unsigned int flow_type) > { > struct mpc8xxx_gpio_chip *mpc8xxx_gc = irq_data_get_irq_chip_data(d); > + struct gpio_chip *gc = &mpc8xxx_gc->gc; > unsigned long gpio = irqd_to_hwirq(d); > void __iomem *reg; > unsigned int shift; > @@ -201,8 +200,7 @@ static int mpc512x_irq_set_type(struct irq_data *d, unsigned int flow_type) > case IRQ_TYPE_EDGE_FALLING: > case IRQ_TYPE_LEVEL_LOW: > raw_spin_lock_irqsave(&mpc8xxx_gc->lock, flags); > - mpc8xxx_gc->write_reg(reg, > - (mpc8xxx_gc->read_reg(reg) & ~(3 << shift)) > + gc->write_reg(reg, (gc->read_reg(reg) & ~(3 << shift)) > | (2 << shift)); > raw_spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags); > break; > @@ -210,16 +208,14 @@ static int mpc512x_irq_set_type(struct irq_data *d, unsigned int flow_type) > case IRQ_TYPE_EDGE_RISING: > case IRQ_TYPE_LEVEL_HIGH: > raw_spin_lock_irqsave(&mpc8xxx_gc->lock, flags); > - mpc8xxx_gc->write_reg(reg, > - (mpc8xxx_gc->read_reg(reg) & ~(3 << shift)) > + gc->write_reg(reg, (gc->read_reg(reg) & ~(3 << shift)) > | (1 << shift)); > raw_spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags); > break; > > case IRQ_TYPE_EDGE_BOTH: > raw_spin_lock_irqsave(&mpc8xxx_gc->lock, flags); > - mpc8xxx_gc->write_reg(reg, > - (mpc8xxx_gc->read_reg(reg) & ~(3 << shift))); > + gc->write_reg(reg, (gc->read_reg(reg) & ~(3 << shift))); > raw_spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags); > break; > > @@ -332,8 +328,6 @@ static int mpc8xxx_probe(struct platform_device *pdev) > dev_dbg(&pdev->dev, "GPIO registers are BIG endian\n"); > } > > - mpc8xxx_gc->read_reg = gc->read_reg; > - mpc8xxx_gc->write_reg = gc->write_reg; > mpc8xxx_gc->direction_output = gc->direction_output; > > if (!devtype) > @@ -366,8 +360,8 @@ static int mpc8xxx_probe(struct platform_device *pdev) > return 0; > > /* ack and mask all irqs */ > - mpc8xxx_gc->write_reg(mpc8xxx_gc->regs + GPIO_IER, 0xffffffff); > - mpc8xxx_gc->write_reg(mpc8xxx_gc->regs + GPIO_IMR, 0); > + gc->write_reg(mpc8xxx_gc->regs + GPIO_IER, 0xffffffff); > + gc->write_reg(mpc8xxx_gc->regs + GPIO_IMR, 0); > > irq_set_chained_handler_and_data(mpc8xxx_gc->irqn, > mpc8xxx_gpio_irq_cascade, mpc8xxx_gc); It's OK for me. Liu Gang ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFT][PATCH v2 2/3] gpio: mpc8xxx: Remove *read_reg and *write_reg from struct mpc8xxx_gpio_chip 2016-02-22 7:24 ` [RFT][PATCH v2 2/3] gpio: mpc8xxx: Remove *read_reg and *write_reg from struct mpc8xxx_gpio_chip Axel Lin 2016-03-02 7:31 ` Gang Liu @ 2016-03-09 3:43 ` Linus Walleij 1 sibling, 0 replies; 9+ messages in thread From: Linus Walleij @ 2016-03-09 3:43 UTC (permalink / raw) To: Axel Lin Cc: Liu Gang, Alexandre Courbot, Ricardo Ribalda Delgado, Alexander Stein, linux-gpio@vger.kernel.org On Mon, Feb 22, 2016 at 2:24 PM, Axel Lin <axel.lin@ingics.com> wrote: > *read_reg and *write_reg can be removed because at all the places to call > them, we can just use gc->read_reg/gc->write_reg instead. > > Signed-off-by: Axel Lin <axel.lin@ingics.com> Patch applied. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFT][PATCH v2 3/3] gpio: mpc8xxx: Slightly update the code for better readability 2016-02-22 7:22 [RFT][PATCH v2 1/3] gpio: mpc8xxx: Fixup setting gpio direction output Axel Lin 2016-02-22 7:24 ` [RFT][PATCH v2 2/3] gpio: mpc8xxx: Remove *read_reg and *write_reg from struct mpc8xxx_gpio_chip Axel Lin @ 2016-02-22 7:24 ` Axel Lin 2016-03-02 7:32 ` Gang Liu 2016-03-09 3:44 ` Linus Walleij 2016-03-02 7:20 ` [RFT][PATCH v2 1/3] gpio: mpc8xxx: Fixup setting gpio direction output Gang Liu 2016-03-09 3:42 ` Linus Walleij 3 siblings, 2 replies; 9+ messages in thread From: Axel Lin @ 2016-02-22 7:24 UTC (permalink / raw) To: Linus Walleij Cc: Liu Gang, Alexandre Courbot, Ricardo Ribalda Delgado, Alexander Stein, linux-gpio Setting gc->direction_output to gc->direction_output looks strange. I think this change makes the intention more clear. Signed-off-by: Axel Lin <axel.lin@ingics.com> --- drivers/gpio/gpio-mpc8xxx.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c index bc042ad6..425501c 100644 --- a/drivers/gpio/gpio-mpc8xxx.c +++ b/drivers/gpio/gpio-mpc8xxx.c @@ -339,8 +339,11 @@ static int mpc8xxx_probe(struct platform_device *pdev) */ mpc8xxx_irq_chip.irq_set_type = devtype->irq_set_type; - gc->direction_output = devtype->gpio_dir_out ?: gc->direction_output; - gc->get = devtype->gpio_get ?: gc->get; + if (devtype->gpio_dir_out) + gc->direction_output = devtype->gpio_dir_out; + if (devtype->gpio_get) + gc->get = devtype->gpio_get; + gc->to_irq = mpc8xxx_gpio_to_irq; ret = gpiochip_add_data(gc, mpc8xxx_gc); -- 2.1.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFT][PATCH v2 3/3] gpio: mpc8xxx: Slightly update the code for better readability 2016-02-22 7:24 ` [RFT][PATCH v2 3/3] gpio: mpc8xxx: Slightly update the code for better readability Axel Lin @ 2016-03-02 7:32 ` Gang Liu 2016-03-09 3:44 ` Linus Walleij 1 sibling, 0 replies; 9+ messages in thread From: Gang Liu @ 2016-03-02 7:32 UTC (permalink / raw) To: Axel Lin, Linus Walleij Cc: Alexandre Courbot, Ricardo Ribalda Delgado, Alexander Stein, linux-gpio@vger.kernel.org On 2/22/2016 3:25 PM, Axel Lin wrote: > Setting gc->direction_output to gc->direction_output looks strange. > I think this change makes the intention more clear. > > Signed-off-by: Axel Lin <axel.lin@ingics.com> > --- > drivers/gpio/gpio-mpc8xxx.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c > index bc042ad6..425501c 100644 > --- a/drivers/gpio/gpio-mpc8xxx.c > +++ b/drivers/gpio/gpio-mpc8xxx.c > @@ -339,8 +339,11 @@ static int mpc8xxx_probe(struct platform_device *pdev) > */ > mpc8xxx_irq_chip.irq_set_type = devtype->irq_set_type; > > - gc->direction_output = devtype->gpio_dir_out ?: gc->direction_output; > - gc->get = devtype->gpio_get ?: gc->get; > + if (devtype->gpio_dir_out) > + gc->direction_output = devtype->gpio_dir_out; > + if (devtype->gpio_get) > + gc->get = devtype->gpio_get; > + > gc->to_irq = mpc8xxx_gpio_to_irq; > > ret = gpiochip_add_data(gc, mpc8xxx_gc); Ok, maybe it's more clear. Liu Gang ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFT][PATCH v2 3/3] gpio: mpc8xxx: Slightly update the code for better readability 2016-02-22 7:24 ` [RFT][PATCH v2 3/3] gpio: mpc8xxx: Slightly update the code for better readability Axel Lin 2016-03-02 7:32 ` Gang Liu @ 2016-03-09 3:44 ` Linus Walleij 1 sibling, 0 replies; 9+ messages in thread From: Linus Walleij @ 2016-03-09 3:44 UTC (permalink / raw) To: Axel Lin Cc: Liu Gang, Alexandre Courbot, Ricardo Ribalda Delgado, Alexander Stein, linux-gpio@vger.kernel.org On Mon, Feb 22, 2016 at 2:24 PM, Axel Lin <axel.lin@ingics.com> wrote: > Setting gc->direction_output to gc->direction_output looks strange. > I think this change makes the intention more clear. > > Signed-off-by: Axel Lin <axel.lin@ingics.com> Patch applied. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFT][PATCH v2 1/3] gpio: mpc8xxx: Fixup setting gpio direction output 2016-02-22 7:22 [RFT][PATCH v2 1/3] gpio: mpc8xxx: Fixup setting gpio direction output Axel Lin 2016-02-22 7:24 ` [RFT][PATCH v2 2/3] gpio: mpc8xxx: Remove *read_reg and *write_reg from struct mpc8xxx_gpio_chip Axel Lin 2016-02-22 7:24 ` [RFT][PATCH v2 3/3] gpio: mpc8xxx: Slightly update the code for better readability Axel Lin @ 2016-03-02 7:20 ` Gang Liu 2016-03-09 3:42 ` Linus Walleij 3 siblings, 0 replies; 9+ messages in thread From: Gang Liu @ 2016-03-02 7:20 UTC (permalink / raw) To: Axel Lin, Linus Walleij Cc: Alexandre Courbot, Ricardo Ribalda Delgado, Alexander Stein, linux-gpio@vger.kernel.org On 2/22/2016 3:23 PM, Axel Lin wrote: > For devtype with specific gpio_dir_out implementation, current code is > wrong because below code sets both gc->direction_output and > mpc8xxx_gc->direction_output to the same function. > > gc->direction_output = devtype->gpio_dir_out ?: gc->direction_output; > mpc8xxx_gc->direction_output = gc->direction_output; > > Set mpc8xxx_gc->direction_output = gc->direction_output first to fix it. > This way mpc8xxx_gc->direction_output actually calls the standard > bgpio_dir_out() to update register. > > Fixes: commit 42178e2a1e42 ("drivers/gpio: Switch gpio-mpc8xxx to use gpio-generic") > > Signed-off-by: Axel Lin <axel.lin@ingics.com> > --- > drivers/gpio/gpio-mpc8xxx.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c > index ecdb27a..d2472c5 100644 > --- a/drivers/gpio/gpio-mpc8xxx.c > +++ b/drivers/gpio/gpio-mpc8xxx.c > @@ -334,6 +334,7 @@ static int mpc8xxx_probe(struct platform_device *pdev) > > mpc8xxx_gc->read_reg = gc->read_reg; > mpc8xxx_gc->write_reg = gc->write_reg; > + mpc8xxx_gc->direction_output = gc->direction_output; > > if (!devtype) > devtype = &mpc8xxx_gpio_devtype_default; > @@ -348,8 +349,6 @@ static int mpc8xxx_probe(struct platform_device *pdev) > gc->get = devtype->gpio_get ?: gc->get; > gc->to_irq = mpc8xxx_gpio_to_irq; > > - mpc8xxx_gc->direction_output = gc->direction_output; > - > ret = gpiochip_add_data(gc, mpc8xxx_gc); > if (ret) { > pr_err("%s: GPIO chip registration failed with status %d\n", This is OK for me. Liu Gang ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFT][PATCH v2 1/3] gpio: mpc8xxx: Fixup setting gpio direction output 2016-02-22 7:22 [RFT][PATCH v2 1/3] gpio: mpc8xxx: Fixup setting gpio direction output Axel Lin ` (2 preceding siblings ...) 2016-03-02 7:20 ` [RFT][PATCH v2 1/3] gpio: mpc8xxx: Fixup setting gpio direction output Gang Liu @ 2016-03-09 3:42 ` Linus Walleij 3 siblings, 0 replies; 9+ messages in thread From: Linus Walleij @ 2016-03-09 3:42 UTC (permalink / raw) To: Axel Lin Cc: Liu Gang, Alexandre Courbot, Ricardo Ribalda Delgado, Alexander Stein, linux-gpio@vger.kernel.org On Mon, Feb 22, 2016 at 2:22 PM, Axel Lin <axel.lin@ingics.com> wrote: > For devtype with specific gpio_dir_out implementation, current code is > wrong because below code sets both gc->direction_output and > mpc8xxx_gc->direction_output to the same function. > > gc->direction_output = devtype->gpio_dir_out ?: gc->direction_output; > mpc8xxx_gc->direction_output = gc->direction_output; > > Set mpc8xxx_gc->direction_output = gc->direction_output first to fix it. > This way mpc8xxx_gc->direction_output actually calls the standard > bgpio_dir_out() to update register. > > Fixes: commit 42178e2a1e42 ("drivers/gpio: Switch gpio-mpc8xxx to use gpio-generic") > > Signed-off-by: Axel Lin <axel.lin@ingics.com> Patch applied. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-03-09 3:44 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-22 7:22 [RFT][PATCH v2 1/3] gpio: mpc8xxx: Fixup setting gpio direction output Axel Lin 2016-02-22 7:24 ` [RFT][PATCH v2 2/3] gpio: mpc8xxx: Remove *read_reg and *write_reg from struct mpc8xxx_gpio_chip Axel Lin 2016-03-02 7:31 ` Gang Liu 2016-03-09 3:43 ` Linus Walleij 2016-02-22 7:24 ` [RFT][PATCH v2 3/3] gpio: mpc8xxx: Slightly update the code for better readability Axel Lin 2016-03-02 7:32 ` Gang Liu 2016-03-09 3:44 ` Linus Walleij 2016-03-02 7:20 ` [RFT][PATCH v2 1/3] gpio: mpc8xxx: Fixup setting gpio direction output Gang Liu 2016-03-09 3:42 ` Linus Walleij
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.