Everyone:
This series contains a number of fixes/improvements I came up with
while working VF610 GPIO code. Hopefully each commit is
self-explanatory.
Feedback is welcome!
Thanks,
Andrey Smirnov
Andrey Smirnov (7):
gpio: vf610: Do not share irq_chip
gpio: vf610: Simplify vf610_gpio_set()
gpio: vf610: Simplify vf610_gpio_get()
gpio: vf610: Use devres to disable clk_port
gpio: vf610: Use devres to disable clk_gpio
gpio: vf610: Use devres to remove gpiochip
gpio: vf610: Don't use explicit &pdev->dev in vf610_gpio_probe()
drivers/gpio/gpio-vf610.c | 85 +++++++++++++++++----------------------
1 file changed, 36 insertions(+), 49 deletions(-)
--
2.20.1
Fix the warning produced by gpiochip_set_irq_hooks() by allocating a
dedicated IRQ chip per GPIO chip/port.
Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Bartosz Golaszewski <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Heiner Kallweit <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/gpio/gpio-vf610.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
index 541fa6ac399d..7e9451f47efe 100644
--- a/drivers/gpio/gpio-vf610.c
+++ b/drivers/gpio/gpio-vf610.c
@@ -29,6 +29,7 @@ struct fsl_gpio_soc_data {
struct vf610_gpio_port {
struct gpio_chip gc;
+ struct irq_chip ic;
void __iomem *base;
void __iomem *gpio_base;
const struct fsl_gpio_soc_data *sdata;
@@ -60,8 +61,6 @@ struct vf610_gpio_port {
#define PORT_INT_EITHER_EDGE 0xb
#define PORT_INT_LOGIC_ONE 0xc
-static struct irq_chip vf610_gpio_irq_chip;
-
static const struct fsl_gpio_soc_data imx_data = {
.have_paddr = true,
};
@@ -237,15 +236,6 @@ static int vf610_gpio_irq_set_wake(struct irq_data *d, u32 enable)
return 0;
}
-static struct irq_chip vf610_gpio_irq_chip = {
- .name = "gpio-vf610",
- .irq_ack = vf610_gpio_irq_ack,
- .irq_mask = vf610_gpio_irq_mask,
- .irq_unmask = vf610_gpio_irq_unmask,
- .irq_set_type = vf610_gpio_irq_set_type,
- .irq_set_wake = vf610_gpio_irq_set_wake,
-};
-
static int vf610_gpio_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -253,6 +243,7 @@ static int vf610_gpio_probe(struct platform_device *pdev)
struct vf610_gpio_port *port;
struct resource *iores;
struct gpio_chip *gc;
+ struct irq_chip *ic;
int i;
int ret;
@@ -316,6 +307,14 @@ static int vf610_gpio_probe(struct platform_device *pdev)
gc->direction_output = vf610_gpio_direction_output;
gc->set = vf610_gpio_set;
+ ic = &port->ic;
+ ic->name = "gpio-vf610";
+ ic->irq_ack = vf610_gpio_irq_ack;
+ ic->irq_mask = vf610_gpio_irq_mask;
+ ic->irq_unmask = vf610_gpio_irq_unmask;
+ ic->irq_set_type = vf610_gpio_irq_set_type;
+ ic->irq_set_wake = vf610_gpio_irq_set_wake;
+
ret = gpiochip_add_data(gc, port);
if (ret < 0)
return ret;
@@ -327,14 +326,13 @@ static int vf610_gpio_probe(struct platform_device *pdev)
/* Clear the interrupt status register for all GPIO's */
vf610_gpio_writel(~0, port->base + PORT_ISFR);
- ret = gpiochip_irqchip_add(gc, &vf610_gpio_irq_chip, 0,
- handle_edge_irq, IRQ_TYPE_NONE);
+ ret = gpiochip_irqchip_add(gc, ic, 0, handle_edge_irq, IRQ_TYPE_NONE);
if (ret) {
dev_err(dev, "failed to add irqchip\n");
gpiochip_remove(gc);
return ret;
}
- gpiochip_set_chained_irqchip(gc, &vf610_gpio_irq_chip, port->irq,
+ gpiochip_set_chained_irqchip(gc, ic, port->irq,
vf610_gpio_irq_handler);
return 0;
--
2.20.1
Clk_gpio should be disabled in all error paths in the code that
follws, including the case when either gpiochip_add_data() or
gpiochip_irqchip_add() fail. To simplify things fix this by using
devm_add_action() to disable corresponding clock in case of any erros
as well as driver/device removal.
Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Bartosz Golaszewski <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Heiner Kallweit <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/gpio/gpio-vf610.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
index 78c1f8ebbe8f..f7445468677d 100644
--- a/drivers/gpio/gpio-vf610.c
+++ b/drivers/gpio/gpio-vf610.c
@@ -289,6 +289,10 @@ static int vf610_gpio_probe(struct platform_device *pdev)
ret = clk_prepare_enable(port->clk_gpio);
if (ret)
return ret;
+ ret = devm_add_action_or_reset(dev, vf610_gpio_disable_clk,
+ port->clk_gpio);
+ if (ret)
+ return ret;
} else if (port->clk_gpio == ERR_PTR(-EPROBE_DEFER)) {
return PTR_ERR(port->clk_gpio);
}
@@ -345,8 +349,6 @@ static int vf610_gpio_remove(struct platform_device *pdev)
struct vf610_gpio_port *port = platform_get_drvdata(pdev);
gpiochip_remove(&port->gc);
- if (!IS_ERR(port->clk_gpio))
- clk_disable_unprepare(port->clk_gpio);
return 0;
}
--
2.20.1
The code already defines "dev" variable to help with that, so make
sure all of the code uses it.
Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Bartosz Golaszewski <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Heiner Kallweit <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/gpio/gpio-vf610.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
index 7db2fa229035..6f6558715b88 100644
--- a/drivers/gpio/gpio-vf610.c
+++ b/drivers/gpio/gpio-vf610.c
@@ -248,7 +248,7 @@ static int vf610_gpio_probe(struct platform_device *pdev)
int i;
int ret;
- port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
+ port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
if (!port)
return -ENOMEM;
@@ -267,7 +267,7 @@ static int vf610_gpio_probe(struct platform_device *pdev)
if (port->irq < 0)
return port->irq;
- port->clk_port = devm_clk_get(&pdev->dev, "port");
+ port->clk_port = devm_clk_get(dev, "port");
if (!IS_ERR(port->clk_port)) {
ret = clk_prepare_enable(port->clk_port);
if (ret)
@@ -284,7 +284,7 @@ static int vf610_gpio_probe(struct platform_device *pdev)
return PTR_ERR(port->clk_port);
}
- port->clk_gpio = devm_clk_get(&pdev->dev, "gpio");
+ port->clk_gpio = devm_clk_get(dev, "gpio");
if (!IS_ERR(port->clk_gpio)) {
ret = clk_prepare_enable(port->clk_gpio);
if (ret)
--
2.20.1
Both branches of the if statement do exactly the same thing, just at
different offsets. Simplify the code a bit by moving shared action
code outside of the if statement. No functional change intended.
Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Bartosz Golaszewski <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Heiner Kallweit <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/gpio/gpio-vf610.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
index 2ea17870e9da..bb35cedd05e3 100644
--- a/drivers/gpio/gpio-vf610.c
+++ b/drivers/gpio/gpio-vf610.c
@@ -85,17 +85,15 @@ static int vf610_gpio_get(struct gpio_chip *gc, unsigned int gpio)
{
struct vf610_gpio_port *port = gpiochip_get_data(gc);
unsigned long mask = BIT(gpio);
- void __iomem *addr;
+ unsigned long offset = GPIO_PDIR;
if (port->sdata && port->sdata->have_paddr) {
mask &= vf610_gpio_readl(port->gpio_base + GPIO_PDDR);
- addr = mask ? port->gpio_base + GPIO_PDOR :
- port->gpio_base + GPIO_PDIR;
- return !!(vf610_gpio_readl(addr) & BIT(gpio));
- } else {
- return !!(vf610_gpio_readl(port->gpio_base + GPIO_PDIR)
- & BIT(gpio));
+ if (mask)
+ offset = GPIO_PDOR;
}
+
+ return !!(vf610_gpio_readl(port->gpio_base + offset) & BIT(gpio));
}
static void vf610_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
--
2.20.1
The only difference between two codepaths is register offset
used. Simplify the code a bit by replacing explicit calls with a
single call with a variable offset. No functional change intended.
Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Bartosz Golaszewski <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Heiner Kallweit <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/gpio/gpio-vf610.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
index 7e9451f47efe..2ea17870e9da 100644
--- a/drivers/gpio/gpio-vf610.c
+++ b/drivers/gpio/gpio-vf610.c
@@ -102,11 +102,9 @@ static void vf610_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
{
struct vf610_gpio_port *port = gpiochip_get_data(gc);
unsigned long mask = BIT(gpio);
+ unsigned long offset = val ? GPIO_PSOR : GPIO_PCOR;
- if (val)
- vf610_gpio_writel(mask, port->gpio_base + GPIO_PSOR);
- else
- vf610_gpio_writel(mask, port->gpio_base + GPIO_PCOR);
+ vf610_gpio_writel(mask, port->gpio_base + offset);
}
static int vf610_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
--
2.20.1
Clk_port should be disabled in all error paths in the code that
follws, including the case when either gpiochip_add_data() or
gpiochip_irqchip_add() fail. To simplify things fix this by using
devm_add_action_or_reset() to disable corresponding clock in case of
any erros as well as driver/device removal.
Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Bartosz Golaszewski <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Heiner Kallweit <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/gpio/gpio-vf610.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
index bb35cedd05e3..78c1f8ebbe8f 100644
--- a/drivers/gpio/gpio-vf610.c
+++ b/drivers/gpio/gpio-vf610.c
@@ -232,6 +232,11 @@ static int vf610_gpio_irq_set_wake(struct irq_data *d, u32 enable)
return 0;
}
+static void vf610_gpio_disable_clk(void *data)
+{
+ clk_disable_unprepare(data);
+}
+
static int vf610_gpio_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -267,6 +272,10 @@ static int vf610_gpio_probe(struct platform_device *pdev)
ret = clk_prepare_enable(port->clk_port);
if (ret)
return ret;
+ ret = devm_add_action_or_reset(dev, vf610_gpio_disable_clk,
+ port->clk_port);
+ if (ret)
+ return ret;
} else if (port->clk_port == ERR_PTR(-EPROBE_DEFER)) {
/*
* Percolate deferrals, for anything else,
@@ -278,12 +287,9 @@ static int vf610_gpio_probe(struct platform_device *pdev)
port->clk_gpio = devm_clk_get(&pdev->dev, "gpio");
if (!IS_ERR(port->clk_gpio)) {
ret = clk_prepare_enable(port->clk_gpio);
- if (ret) {
- clk_disable_unprepare(port->clk_port);
+ if (ret)
return ret;
- }
} else if (port->clk_gpio == ERR_PTR(-EPROBE_DEFER)) {
- clk_disable_unprepare(port->clk_port);
return PTR_ERR(port->clk_gpio);
}
@@ -339,8 +345,6 @@ static int vf610_gpio_remove(struct platform_device *pdev)
struct vf610_gpio_port *port = platform_get_drvdata(pdev);
gpiochip_remove(&port->gc);
- if (!IS_ERR(port->clk_port))
- clk_disable_unprepare(port->clk_port);
if (!IS_ERR(port->clk_gpio))
clk_disable_unprepare(port->clk_gpio);
--
2.20.1
Now that the driver's custom remove hook contains only a single
action, replace it by converting the code to use
devm_gpiochip_add_data() to simplify things.
Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Bartosz Golaszewski <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Heiner Kallweit <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/gpio/gpio-vf610.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
index f7445468677d..7db2fa229035 100644
--- a/drivers/gpio/gpio-vf610.c
+++ b/drivers/gpio/gpio-vf610.c
@@ -297,8 +297,6 @@ static int vf610_gpio_probe(struct platform_device *pdev)
return PTR_ERR(port->clk_gpio);
}
- platform_set_drvdata(pdev, port);
-
gc = &port->gc;
gc->of_node = np;
gc->parent = dev;
@@ -321,7 +319,7 @@ static int vf610_gpio_probe(struct platform_device *pdev)
ic->irq_set_type = vf610_gpio_irq_set_type;
ic->irq_set_wake = vf610_gpio_irq_set_wake;
- ret = gpiochip_add_data(gc, port);
+ ret = devm_gpiochip_add_data(dev, gc, port);
if (ret < 0)
return ret;
@@ -335,7 +333,6 @@ static int vf610_gpio_probe(struct platform_device *pdev)
ret = gpiochip_irqchip_add(gc, ic, 0, handle_edge_irq, IRQ_TYPE_NONE);
if (ret) {
dev_err(dev, "failed to add irqchip\n");
- gpiochip_remove(gc);
return ret;
}
gpiochip_set_chained_irqchip(gc, ic, port->irq,
@@ -344,22 +341,12 @@ static int vf610_gpio_probe(struct platform_device *pdev)
return 0;
}
-static int vf610_gpio_remove(struct platform_device *pdev)
-{
- struct vf610_gpio_port *port = platform_get_drvdata(pdev);
-
- gpiochip_remove(&port->gc);
-
- return 0;
-}
-
static struct platform_driver vf610_gpio_driver = {
.driver = {
.name = "gpio-vf610",
.of_match_table = vf610_gpio_dt_ids,
},
.probe = vf610_gpio_probe,
- .remove = vf610_gpio_remove,
};
builtin_platform_driver(vf610_gpio_driver);
--
2.20.1
pon., 11 mar 2019 o 07:28 Andrey Smirnov <[email protected]> napisał(a):
>
> Everyone:
>
> This series contains a number of fixes/improvements I came up with
> while working VF610 GPIO code. Hopefully each commit is
> self-explanatory.
>
> Feedback is welcome!
>
> Thanks,
> Andrey Smirnov
>
> Andrey Smirnov (7):
> gpio: vf610: Do not share irq_chip
> gpio: vf610: Simplify vf610_gpio_set()
> gpio: vf610: Simplify vf610_gpio_get()
> gpio: vf610: Use devres to disable clk_port
> gpio: vf610: Use devres to disable clk_gpio
> gpio: vf610: Use devres to remove gpiochip
> gpio: vf610: Don't use explicit &pdev->dev in vf610_gpio_probe()
>
> drivers/gpio/gpio-vf610.c | 85 +++++++++++++++++----------------------
> 1 file changed, 36 insertions(+), 49 deletions(-)
>
> --
> 2.20.1
>
Nice cleanups! Series applied, thanks!
Bart