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