Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1498177 > unrolled thread

[PATCH v2] RFC: staging: greybus: shape up greybus GPIO

Started byLinus Walleij <linus.walleij@linaro.org>
First post2016-10-10 10:50 +0200
Last post2016-10-12 04:30 +0200
Articles 3 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] RFC: staging: greybus: shape up greybus GPIO Linus Walleij <linus.walleij@linaro.org> - 2016-10-10 10:50 +0200
    Re: [PATCH v2] RFC: staging: greybus: shape up greybus GPIO Rui Miguel Silva <rmfrfs@gmail.com> - 2016-10-10 16:30 +0200
    Re: [PATCH v2] RFC: staging: greybus: shape up greybus GPIO Viresh Kumar <viresh.kumar@linaro.org> - 2016-10-12 04:30 +0200

#1498177 — [PATCH v2] RFC: staging: greybus: shape up greybus GPIO

FromLinus Walleij <linus.walleij@linaro.org>
Date2016-10-10 10:50 +0200
Subject[PATCH v2] RFC: staging: greybus: shape up greybus GPIO
Message-ID<sqErw-7rt-23@gated-at.bofh.it>
Greybus GPIO seems to reimplement the already existing generic
gpiolib irqchip. Also use gpiochip_get_data(). Also use
devm_gpiochip_add_data().

Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Axel Haslam <haslam_axel@projectara.com>
Cc: Johan Hovold <johan@hovoldconsulting.com>
Cc: Sandeep Patil <patil_sandeep@projectara.com>
Cc: Rui Miguel Silva <rui.silva@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Add the hunk adding select GPIOLIB_IRQCHIP to Kconfig,
  sorry for sending patches too late in the night.

Greybus folks: please look at this. I expect something like this
to be applied before migrating from staging, I can't test it
obviously.
---
 drivers/staging/greybus/Kconfig |   1 +
 drivers/staging/greybus/gpio.c  | 183 +++++-----------------------------------
 2 files changed, 24 insertions(+), 160 deletions(-)

diff --git a/drivers/staging/greybus/Kconfig b/drivers/staging/greybus/Kconfig
index 50de2d72dde0..b05a8f4657e3 100644
--- a/drivers/staging/greybus/Kconfig
+++ b/drivers/staging/greybus/Kconfig
@@ -148,6 +148,7 @@ if GREYBUS_BRIDGED_PHY
 config GREYBUS_GPIO
 	tristate "Greybus GPIO Bridged PHY driver"
 	depends on GPIOLIB
+	select GPIOLIB_IRQCHIP
 	---help---
 	  Select this option if you have a device that follows the
 	  Greybus GPIO Bridged PHY Class specification.
diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c
index 5e06e4229e42..07ccd819ee1c 100644
--- a/drivers/staging/greybus/gpio.c
+++ b/drivers/staging/greybus/gpio.c
@@ -10,7 +10,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>
-#include <linux/gpio.h>
+#include <linux/gpio/driver.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/mutex.h>
@@ -41,14 +41,8 @@ struct gb_gpio_controller {
 	struct gpio_chip	chip;
 	struct irq_chip		irqc;
 	struct irq_chip		*irqchip;
-	struct irq_domain	*irqdomain;
-	unsigned int		irq_base;
-	irq_flow_handler_t	irq_handler;
-	unsigned int		irq_default_type;
 	struct mutex		irq_lock;
 };
-#define gpio_chip_to_gb_gpio_controller(chip) \
-	container_of(chip, struct gb_gpio_controller, chip)
 #define irq_data_to_gpio_chip(d) (d->domain->host_data)
 
 static int gb_gpio_line_count_operation(struct gb_gpio_controller *ggc)
@@ -277,7 +271,7 @@ static void _gb_gpio_irq_set_type(struct gb_gpio_controller *ggc,
 static void gb_gpio_irq_mask(struct irq_data *d)
 {
 	struct gpio_chip *chip = irq_data_to_gpio_chip(d);
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
 	struct gb_gpio_line *line = &ggc->lines[d->hwirq];
 
 	line->masked = true;
@@ -287,7 +281,7 @@ static void gb_gpio_irq_mask(struct irq_data *d)
 static void gb_gpio_irq_unmask(struct irq_data *d)
 {
 	struct gpio_chip *chip = irq_data_to_gpio_chip(d);
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
 	struct gb_gpio_line *line = &ggc->lines[d->hwirq];
 
 	line->masked = false;
@@ -297,7 +291,7 @@ static void gb_gpio_irq_unmask(struct irq_data *d)
 static int gb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 {
 	struct gpio_chip *chip = irq_data_to_gpio_chip(d);
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
 	struct gb_gpio_line *line = &ggc->lines[d->hwirq];
 	struct device *dev = &ggc->gbphy_dev->dev;
 	u8 irq_type;
@@ -335,7 +329,7 @@ static int gb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 static void gb_gpio_irq_bus_lock(struct irq_data *d)
 {
 	struct gpio_chip *chip = irq_data_to_gpio_chip(d);
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
 
 	mutex_lock(&ggc->irq_lock);
 }
@@ -343,7 +337,7 @@ static void gb_gpio_irq_bus_lock(struct irq_data *d)
 static void gb_gpio_irq_bus_sync_unlock(struct irq_data *d)
 {
 	struct gpio_chip *chip = irq_data_to_gpio_chip(d);
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
 	struct gb_gpio_line *line = &ggc->lines[d->hwirq];
 
 	if (line->irq_type_pending) {
@@ -392,7 +386,7 @@ static int gb_gpio_request_handler(struct gb_operation *op)
 		return -EINVAL;
 	}
 
-	irq = irq_find_mapping(ggc->irqdomain, event->which);
+	irq = irq_find_mapping(ggc->chip.irqdomain, event->which);
 	if (!irq) {
 		dev_err(dev, "failed to find IRQ\n");
 		return -EINVAL;
@@ -412,21 +406,21 @@ static int gb_gpio_request_handler(struct gb_operation *op)
 
 static int gb_gpio_request(struct gpio_chip *chip, unsigned offset)
 {
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
 
 	return gb_gpio_activate_operation(ggc, (u8)offset);
 }
 
 static void gb_gpio_free(struct gpio_chip *chip, unsigned offset)
 {
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
 
 	gb_gpio_deactivate_operation(ggc, (u8)offset);
 }
 
 static int gb_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
 {
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
 	u8 which;
 	int ret;
 
@@ -440,7 +434,7 @@ static int gb_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
 
 static int gb_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
 {
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
 
 	return gb_gpio_direction_in_operation(ggc, (u8)offset);
 }
@@ -448,14 +442,14 @@ static int gb_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
 static int gb_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
 					int value)
 {
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
 
 	return gb_gpio_direction_out_operation(ggc, (u8)offset, !!value);
 }
 
 static int gb_gpio_get(struct gpio_chip *chip, unsigned offset)
 {
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
 	u8 which;
 	int ret;
 
@@ -469,7 +463,7 @@ static int gb_gpio_get(struct gpio_chip *chip, unsigned offset)
 
 static void gb_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 {
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
 
 	gb_gpio_set_value_operation(ggc, (u8)offset, !!value);
 }
@@ -477,7 +471,7 @@ static void gb_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 static int gb_gpio_set_debounce(struct gpio_chip *chip, unsigned offset,
 					unsigned debounce)
 {
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
 	u16 usec;
 
 	if (debounce > U16_MAX)
@@ -504,134 +498,6 @@ static int gb_gpio_controller_setup(struct gb_gpio_controller *ggc)
 	return ret;
 }
 
-/**
- * gb_gpio_irq_map() - maps an IRQ into a GB gpio irqchip
- * @d: the irqdomain used by this irqchip
- * @irq: the global irq number used by this GB gpio irqchip irq
- * @hwirq: the local IRQ/GPIO line offset on this GB gpio
- *
- * This function will set up the mapping for a certain IRQ line on a
- * GB gpio by assigning the GB gpio as chip data, and using the irqchip
- * stored inside the GB gpio.
- */
-static int gb_gpio_irq_map(struct irq_domain *domain, unsigned int irq,
-			   irq_hw_number_t hwirq)
-{
-	struct gpio_chip *chip = domain->host_data;
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
-
-	irq_set_chip_data(irq, ggc);
-	irq_set_chip_and_handler(irq, ggc->irqchip, ggc->irq_handler);
-	irq_set_noprobe(irq);
-	/*
-	 * No set-up of the hardware will happen if IRQ_TYPE_NONE
-	 * is passed as default type.
-	 */
-	if (ggc->irq_default_type != IRQ_TYPE_NONE)
-		irq_set_irq_type(irq, ggc->irq_default_type);
-
-	return 0;
-}
-
-static void gb_gpio_irq_unmap(struct irq_domain *d, unsigned int irq)
-{
-	irq_set_chip_and_handler(irq, NULL, NULL);
-	irq_set_chip_data(irq, NULL);
-}
-
-static const struct irq_domain_ops gb_gpio_domain_ops = {
-	.map	= gb_gpio_irq_map,
-	.unmap	= gb_gpio_irq_unmap,
-};
-
-/**
- * gb_gpio_irqchip_remove() - removes an irqchip added to a gb_gpio_controller
- * @ggc: the gb_gpio_controller to remove the irqchip from
- *
- * This is called only from gb_gpio_remove()
- */
-static void gb_gpio_irqchip_remove(struct gb_gpio_controller *ggc)
-{
-	unsigned int offset;
-
-	/* Remove all IRQ mappings and delete the domain */
-	if (ggc->irqdomain) {
-		for (offset = 0; offset < (ggc->line_max + 1); offset++)
-			irq_dispose_mapping(irq_find_mapping(ggc->irqdomain, offset));
-		irq_domain_remove(ggc->irqdomain);
-	}
-
-	if (ggc->irqchip)
-		ggc->irqchip = NULL;
-}
-
-/**
- * gb_gpio_irqchip_add() - adds an irqchip to a gpio chip
- * @chip: the gpio chip to add the irqchip to
- * @irqchip: the irqchip to add to the adapter
- * @first_irq: if not dynamically assigned, the base (first) IRQ to
- * allocate gpio irqs from
- * @handler: the irq handler to use (often a predefined irq core function)
- * @type: the default type for IRQs on this irqchip, pass IRQ_TYPE_NONE
- * to have the core avoid setting up any default type in the hardware.
- *
- * This function closely associates a certain irqchip with a certain
- * gpio chip, providing an irq domain to translate the local IRQs to
- * global irqs, and making sure that the gpio chip
- * is passed as chip data to all related functions. Driver callbacks
- * need to use container_of() to get their local state containers back
- * from the gpio chip passed as chip data. An irqdomain will be stored
- * in the gpio chip that shall be used by the driver to handle IRQ number
- * translation. The gpio chip will need to be initialized and registered
- * before calling this function.
- */
-static int gb_gpio_irqchip_add(struct gpio_chip *chip,
-			 struct irq_chip *irqchip,
-			 unsigned int first_irq,
-			 irq_flow_handler_t handler,
-			 unsigned int type)
-{
-	struct gb_gpio_controller *ggc;
-	unsigned int offset;
-	unsigned irq_base;
-
-	if (!chip || !irqchip)
-		return -EINVAL;
-
-	ggc = gpio_chip_to_gb_gpio_controller(chip);
-
-	ggc->irqchip = irqchip;
-	ggc->irq_handler = handler;
-	ggc->irq_default_type = type;
-	ggc->irqdomain = irq_domain_add_simple(NULL,
-					ggc->line_max + 1, first_irq,
-					&gb_gpio_domain_ops, chip);
-	if (!ggc->irqdomain) {
-		ggc->irqchip = NULL;
-		return -EINVAL;
-	}
-
-	/*
-	 * Prepare the mapping since the irqchip shall be orthogonal to
-	 * any gpio calls. If the first_irq was zero, this is
-	 * necessary to allocate descriptors for all IRQs.
-	 */
-	for (offset = 0; offset < (ggc->line_max + 1); offset++) {
-		irq_base = irq_create_mapping(ggc->irqdomain, offset);
-		if (offset == 0)
-			ggc->irq_base = irq_base;
-	}
-
-	return 0;
-}
-
-static int gb_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
-{
-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
-
-	return irq_find_mapping(ggc->irqdomain, offset);
-}
-
 static int gb_gpio_probe(struct gbphy_device *gbphy_dev,
 			 const struct gbphy_device_id *id)
 {
@@ -690,7 +556,6 @@ static int gb_gpio_probe(struct gbphy_device *gbphy_dev,
 	gpio->get = gb_gpio_get;
 	gpio->set = gb_gpio_set;
 	gpio->set_debounce = gb_gpio_set_debounce;
-	gpio->to_irq = gb_gpio_to_irq;
 	gpio->base = -1;		/* Allocate base dynamically */
 	gpio->ngpio = ggc->line_max + 1;
 	gpio->can_sleep = true;
@@ -699,26 +564,26 @@ static int gb_gpio_probe(struct gbphy_device *gbphy_dev,
 	if (ret)
 		goto exit_line_free;
 
-	ret = gb_gpio_irqchip_add(gpio, irqc, 0,
-				   handle_level_irq, IRQ_TYPE_NONE);
+	ret = devm_gpiochip_add_data(&connection->bundle->dev,
+				     gpio, ggc);
 	if (ret) {
 		dev_err(&connection->bundle->dev,
-			"failed to add irq chip: %d\n", ret);
+			"failed to add gpio chip: %d\n", ret);
 		goto exit_line_free;
 	}
 
-	ret = gpiochip_add(gpio);
+	ret = gpiochip_irqchip_add(gpio, irqc,
+				   0, handle_level_irq,
+				   IRQ_TYPE_NONE);
 	if (ret) {
 		dev_err(&connection->bundle->dev,
-			"failed to add gpio chip: %d\n", ret);
-		goto exit_gpio_irqchip_remove;
+			"failed to add gpio irqchip: %d\n", ret);
+		goto exit_line_free;
 	}
 
 	gbphy_runtime_put_autosuspend(gbphy_dev);
 	return 0;
 
-exit_gpio_irqchip_remove:
-	gb_gpio_irqchip_remove(ggc);
 exit_line_free:
 	kfree(ggc->lines);
 exit_connection_disable:
@@ -741,8 +606,6 @@ static void gb_gpio_remove(struct gbphy_device *gbphy_dev)
 		gbphy_runtime_get_noresume(gbphy_dev);
 
 	gb_connection_disable_rx(connection);
-	gpiochip_remove(&ggc->chip);
-	gb_gpio_irqchip_remove(ggc);
 	gb_connection_disable(connection);
 	gb_connection_destroy(connection);
 	kfree(ggc->lines);
-- 
2.7.4

[toc] | [next] | [standalone]

#1498356

FromRui Miguel Silva <rmfrfs@gmail.com>
Date2016-10-10 16:30 +0200
Message-ID<sqJKy-2la-9@gated-at.bofh.it>
In reply to#1498177
Hi Linus,
On Mon, Oct 10, 2016 at 10:39:32AM +0200, Linus Walleij wrote:
>Greybus GPIO seems to reimplement the already existing generic
>gpiolib irqchip. Also use gpiochip_get_data(). Also use
>devm_gpiochip_add_data().
>
>Cc: Viresh Kumar <viresh.kumar@linaro.org>
>Cc: Axel Haslam <haslam_axel@projectara.com>
>Cc: Johan Hovold <johan@hovoldconsulting.com>
>Cc: Sandeep Patil <patil_sandeep@projectara.com>
>Cc: Rui Miguel Silva <rui.silva@linaro.org>
>Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

This was a necessary change for recent kernels.

Looks good to me. Thanks.
Reviewed-by: Rui Miguel Silva <rmfrfs@gmail.com>

>---
>ChangeLog v1->v2:
>- Add the hunk adding select GPIOLIB_IRQCHIP to Kconfig,
>  sorry for sending patches too late in the night.
>
>Greybus folks: please look at this. I expect something like this
>to be applied before migrating from staging, I can't test it
>obviously.
>---
> drivers/staging/greybus/Kconfig |   1 +
> drivers/staging/greybus/gpio.c  | 183 +++++-----------------------------------
> 2 files changed, 24 insertions(+), 160 deletions(-)
>
>diff --git a/drivers/staging/greybus/Kconfig b/drivers/staging/greybus/Kconfig
>index 50de2d72dde0..b05a8f4657e3 100644
>--- a/drivers/staging/greybus/Kconfig
>+++ b/drivers/staging/greybus/Kconfig
>@@ -148,6 +148,7 @@ if GREYBUS_BRIDGED_PHY
> config GREYBUS_GPIO
> 	tristate "Greybus GPIO Bridged PHY driver"
> 	depends on GPIOLIB
>+	select GPIOLIB_IRQCHIP
> 	---help---
> 	  Select this option if you have a device that follows the
> 	  Greybus GPIO Bridged PHY Class specification.
>diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c
>index 5e06e4229e42..07ccd819ee1c 100644
>--- a/drivers/staging/greybus/gpio.c
>+++ b/drivers/staging/greybus/gpio.c
>@@ -10,7 +10,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/slab.h>
>-#include <linux/gpio.h>
>+#include <linux/gpio/driver.h>
> #include <linux/irq.h>
> #include <linux/irqdomain.h>
> #include <linux/mutex.h>
>@@ -41,14 +41,8 @@ struct gb_gpio_controller {
> 	struct gpio_chip	chip;
> 	struct irq_chip		irqc;
> 	struct irq_chip		*irqchip;
>-	struct irq_domain	*irqdomain;
>-	unsigned int		irq_base;
>-	irq_flow_handler_t	irq_handler;
>-	unsigned int		irq_default_type;
> 	struct mutex		irq_lock;
> };
>-#define gpio_chip_to_gb_gpio_controller(chip) \
>-	container_of(chip, struct gb_gpio_controller, chip)
> #define irq_data_to_gpio_chip(d) (d->domain->host_data)
>
> static int gb_gpio_line_count_operation(struct gb_gpio_controller *ggc)
>@@ -277,7 +271,7 @@ static void _gb_gpio_irq_set_type(struct gb_gpio_controller *ggc,
> static void gb_gpio_irq_mask(struct irq_data *d)
> {
> 	struct gpio_chip *chip = irq_data_to_gpio_chip(d);
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
> 	struct gb_gpio_line *line = &ggc->lines[d->hwirq];
>
> 	line->masked = true;
>@@ -287,7 +281,7 @@ static void gb_gpio_irq_mask(struct irq_data *d)
> static void gb_gpio_irq_unmask(struct irq_data *d)
> {
> 	struct gpio_chip *chip = irq_data_to_gpio_chip(d);
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
> 	struct gb_gpio_line *line = &ggc->lines[d->hwirq];
>
> 	line->masked = false;
>@@ -297,7 +291,7 @@ static void gb_gpio_irq_unmask(struct irq_data *d)
> static int gb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> {
> 	struct gpio_chip *chip = irq_data_to_gpio_chip(d);
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
> 	struct gb_gpio_line *line = &ggc->lines[d->hwirq];
> 	struct device *dev = &ggc->gbphy_dev->dev;
> 	u8 irq_type;
>@@ -335,7 +329,7 @@ static int gb_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> static void gb_gpio_irq_bus_lock(struct irq_data *d)
> {
> 	struct gpio_chip *chip = irq_data_to_gpio_chip(d);
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
>
> 	mutex_lock(&ggc->irq_lock);
> }
>@@ -343,7 +337,7 @@ static void gb_gpio_irq_bus_lock(struct irq_data *d)
> static void gb_gpio_irq_bus_sync_unlock(struct irq_data *d)
> {
> 	struct gpio_chip *chip = irq_data_to_gpio_chip(d);
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
> 	struct gb_gpio_line *line = &ggc->lines[d->hwirq];
>
> 	if (line->irq_type_pending) {
>@@ -392,7 +386,7 @@ static int gb_gpio_request_handler(struct gb_operation *op)
> 		return -EINVAL;
> 	}
>
>-	irq = irq_find_mapping(ggc->irqdomain, event->which);
>+	irq = irq_find_mapping(ggc->chip.irqdomain, event->which);
> 	if (!irq) {
> 		dev_err(dev, "failed to find IRQ\n");
> 		return -EINVAL;
>@@ -412,21 +406,21 @@ static int gb_gpio_request_handler(struct gb_operation *op)
>
> static int gb_gpio_request(struct gpio_chip *chip, unsigned offset)
> {
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
>
> 	return gb_gpio_activate_operation(ggc, (u8)offset);
> }
>
> static void gb_gpio_free(struct gpio_chip *chip, unsigned offset)
> {
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
>
> 	gb_gpio_deactivate_operation(ggc, (u8)offset);
> }
>
> static int gb_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
> {
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
> 	u8 which;
> 	int ret;
>
>@@ -440,7 +434,7 @@ static int gb_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
>
> static int gb_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
> {
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
>
> 	return gb_gpio_direction_in_operation(ggc, (u8)offset);
> }
>@@ -448,14 +442,14 @@ static int gb_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
> static int gb_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
> 					int value)
> {
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
>
> 	return gb_gpio_direction_out_operation(ggc, (u8)offset, !!value);
> }
>
> static int gb_gpio_get(struct gpio_chip *chip, unsigned offset)
> {
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
> 	u8 which;
> 	int ret;
>
>@@ -469,7 +463,7 @@ static int gb_gpio_get(struct gpio_chip *chip, unsigned offset)
>
> static void gb_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> {
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
>
> 	gb_gpio_set_value_operation(ggc, (u8)offset, !!value);
> }
>@@ -477,7 +471,7 @@ static void gb_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> static int gb_gpio_set_debounce(struct gpio_chip *chip, unsigned offset,
> 					unsigned debounce)
> {
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>+	struct gb_gpio_controller *ggc = gpiochip_get_data(chip);
> 	u16 usec;
>
> 	if (debounce > U16_MAX)
>@@ -504,134 +498,6 @@ static int gb_gpio_controller_setup(struct gb_gpio_controller *ggc)
> 	return ret;
> }
>
>-/**
>- * gb_gpio_irq_map() - maps an IRQ into a GB gpio irqchip
>- * @d: the irqdomain used by this irqchip
>- * @irq: the global irq number used by this GB gpio irqchip irq
>- * @hwirq: the local IRQ/GPIO line offset on this GB gpio
>- *
>- * This function will set up the mapping for a certain IRQ line on a
>- * GB gpio by assigning the GB gpio as chip data, and using the irqchip
>- * stored inside the GB gpio.
>- */
>-static int gb_gpio_irq_map(struct irq_domain *domain, unsigned int irq,
>-			   irq_hw_number_t hwirq)
>-{
>-	struct gpio_chip *chip = domain->host_data;
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>-
>-	irq_set_chip_data(irq, ggc);
>-	irq_set_chip_and_handler(irq, ggc->irqchip, ggc->irq_handler);
>-	irq_set_noprobe(irq);
>-	/*
>-	 * No set-up of the hardware will happen if IRQ_TYPE_NONE
>-	 * is passed as default type.
>-	 */
>-	if (ggc->irq_default_type != IRQ_TYPE_NONE)
>-		irq_set_irq_type(irq, ggc->irq_default_type);
>-
>-	return 0;
>-}
>-
>-static void gb_gpio_irq_unmap(struct irq_domain *d, unsigned int irq)
>-{
>-	irq_set_chip_and_handler(irq, NULL, NULL);
>-	irq_set_chip_data(irq, NULL);
>-}
>-
>-static const struct irq_domain_ops gb_gpio_domain_ops = {
>-	.map	= gb_gpio_irq_map,
>-	.unmap	= gb_gpio_irq_unmap,
>-};
>-
>-/**
>- * gb_gpio_irqchip_remove() - removes an irqchip added to a gb_gpio_controller
>- * @ggc: the gb_gpio_controller to remove the irqchip from
>- *
>- * This is called only from gb_gpio_remove()
>- */
>-static void gb_gpio_irqchip_remove(struct gb_gpio_controller *ggc)
>-{
>-	unsigned int offset;
>-
>-	/* Remove all IRQ mappings and delete the domain */
>-	if (ggc->irqdomain) {
>-		for (offset = 0; offset < (ggc->line_max + 1); offset++)
>-			irq_dispose_mapping(irq_find_mapping(ggc->irqdomain, offset));
>-		irq_domain_remove(ggc->irqdomain);
>-	}
>-
>-	if (ggc->irqchip)
>-		ggc->irqchip = NULL;
>-}
>-
>-/**
>- * gb_gpio_irqchip_add() - adds an irqchip to a gpio chip
>- * @chip: the gpio chip to add the irqchip to
>- * @irqchip: the irqchip to add to the adapter
>- * @first_irq: if not dynamically assigned, the base (first) IRQ to
>- * allocate gpio irqs from
>- * @handler: the irq handler to use (often a predefined irq core function)
>- * @type: the default type for IRQs on this irqchip, pass IRQ_TYPE_NONE
>- * to have the core avoid setting up any default type in the hardware.
>- *
>- * This function closely associates a certain irqchip with a certain
>- * gpio chip, providing an irq domain to translate the local IRQs to
>- * global irqs, and making sure that the gpio chip
>- * is passed as chip data to all related functions. Driver callbacks
>- * need to use container_of() to get their local state containers back
>- * from the gpio chip passed as chip data. An irqdomain will be stored
>- * in the gpio chip that shall be used by the driver to handle IRQ number
>- * translation. The gpio chip will need to be initialized and registered
>- * before calling this function.
>- */
>-static int gb_gpio_irqchip_add(struct gpio_chip *chip,
>-			 struct irq_chip *irqchip,
>-			 unsigned int first_irq,
>-			 irq_flow_handler_t handler,
>-			 unsigned int type)
>-{
>-	struct gb_gpio_controller *ggc;
>-	unsigned int offset;
>-	unsigned irq_base;
>-
>-	if (!chip || !irqchip)
>-		return -EINVAL;
>-
>-	ggc = gpio_chip_to_gb_gpio_controller(chip);
>-
>-	ggc->irqchip = irqchip;
>-	ggc->irq_handler = handler;
>-	ggc->irq_default_type = type;
>-	ggc->irqdomain = irq_domain_add_simple(NULL,
>-					ggc->line_max + 1, first_irq,
>-					&gb_gpio_domain_ops, chip);
>-	if (!ggc->irqdomain) {
>-		ggc->irqchip = NULL;
>-		return -EINVAL;
>-	}
>-
>-	/*
>-	 * Prepare the mapping since the irqchip shall be orthogonal to
>-	 * any gpio calls. If the first_irq was zero, this is
>-	 * necessary to allocate descriptors for all IRQs.
>-	 */
>-	for (offset = 0; offset < (ggc->line_max + 1); offset++) {
>-		irq_base = irq_create_mapping(ggc->irqdomain, offset);
>-		if (offset == 0)
>-			ggc->irq_base = irq_base;
>-	}
>-
>-	return 0;
>-}
>-
>-static int gb_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
>-{
>-	struct gb_gpio_controller *ggc = gpio_chip_to_gb_gpio_controller(chip);
>-
>-	return irq_find_mapping(ggc->irqdomain, offset);
>-}
>-
> static int gb_gpio_probe(struct gbphy_device *gbphy_dev,
> 			 const struct gbphy_device_id *id)
> {
>@@ -690,7 +556,6 @@ static int gb_gpio_probe(struct gbphy_device *gbphy_dev,
> 	gpio->get = gb_gpio_get;
> 	gpio->set = gb_gpio_set;
> 	gpio->set_debounce = gb_gpio_set_debounce;
>-	gpio->to_irq = gb_gpio_to_irq;
> 	gpio->base = -1;		/* Allocate base dynamically */
> 	gpio->ngpio = ggc->line_max + 1;
> 	gpio->can_sleep = true;
>@@ -699,26 +564,26 @@ static int gb_gpio_probe(struct gbphy_device *gbphy_dev,
> 	if (ret)
> 		goto exit_line_free;
>
>-	ret = gb_gpio_irqchip_add(gpio, irqc, 0,
>-				   handle_level_irq, IRQ_TYPE_NONE);
>+	ret = devm_gpiochip_add_data(&connection->bundle->dev,
>+				     gpio, ggc);
> 	if (ret) {
> 		dev_err(&connection->bundle->dev,
>-			"failed to add irq chip: %d\n", ret);
>+			"failed to add gpio chip: %d\n", ret);
> 		goto exit_line_free;
> 	}
>
>-	ret = gpiochip_add(gpio);
>+	ret = gpiochip_irqchip_add(gpio, irqc,
>+				   0, handle_level_irq,
>+				   IRQ_TYPE_NONE);
> 	if (ret) {
> 		dev_err(&connection->bundle->dev,
>-			"failed to add gpio chip: %d\n", ret);
>-		goto exit_gpio_irqchip_remove;
>+			"failed to add gpio irqchip: %d\n", ret);
>+		goto exit_line_free;
> 	}
>
> 	gbphy_runtime_put_autosuspend(gbphy_dev);
> 	return 0;
>
>-exit_gpio_irqchip_remove:
>-	gb_gpio_irqchip_remove(ggc);
> exit_line_free:
> 	kfree(ggc->lines);
> exit_connection_disable:
>@@ -741,8 +606,6 @@ static void gb_gpio_remove(struct gbphy_device *gbphy_dev)
> 		gbphy_runtime_get_noresume(gbphy_dev);
>
> 	gb_connection_disable_rx(connection);
>-	gpiochip_remove(&ggc->chip);
>-	gb_gpio_irqchip_remove(ggc);
> 	gb_connection_disable(connection);
> 	gb_connection_destroy(connection);
> 	kfree(ggc->lines);
>-- 
>2.7.4
>
>_______________________________________________
>devel mailing list
>devel@linuxdriverproject.org
>http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[toc] | [prev] | [next] | [standalone]

#1499324

FromViresh Kumar <viresh.kumar@linaro.org>
Date2016-10-12 04:30 +0200
Message-ID<srhsR-6yG-5@gated-at.bofh.it>
In reply to#1498177
I was looking to exactly do thanks, thanks for doing it before us :)

On 10-10-16, 10:39, Linus Walleij wrote:
>  static int gb_gpio_probe(struct gbphy_device *gbphy_dev,
>  			 const struct gbphy_device_id *id)
>  {
> @@ -690,7 +556,6 @@ static int gb_gpio_probe(struct gbphy_device *gbphy_dev,
>  	gpio->get = gb_gpio_get;
>  	gpio->set = gb_gpio_set;
>  	gpio->set_debounce = gb_gpio_set_debounce;
> -	gpio->to_irq = gb_gpio_to_irq;
>  	gpio->base = -1;		/* Allocate base dynamically */
>  	gpio->ngpio = ggc->line_max + 1;
>  	gpio->can_sleep = true;
> @@ -699,26 +564,26 @@ static int gb_gpio_probe(struct gbphy_device *gbphy_dev,
>  	if (ret)
>  		goto exit_line_free;
>  
> -	ret = gb_gpio_irqchip_add(gpio, irqc, 0,
> -				   handle_level_irq, IRQ_TYPE_NONE);
> +	ret = devm_gpiochip_add_data(&connection->bundle->dev,

We should be using gbphy_dev->dev here instead.

> +				     gpio, ggc);
>  	if (ret) {
>  		dev_err(&connection->bundle->dev,

And also at these places. I will send a patch to fix these separately though.

> -			"failed to add irq chip: %d\n", ret);
> +			"failed to add gpio chip: %d\n", ret);
>  		goto exit_line_free;
>  	}
>  
> -	ret = gpiochip_add(gpio);
> +	ret = gpiochip_irqchip_add(gpio, irqc,
> +				   0, handle_level_irq,
> +				   IRQ_TYPE_NONE);
>  	if (ret) {
>  		dev_err(&connection->bundle->dev,
> -			"failed to add gpio chip: %d\n", ret);
> -		goto exit_gpio_irqchip_remove;
> +			"failed to add gpio irqchip: %d\n", ret);
> +		goto exit_line_free;
>  	}
>  
>  	gbphy_runtime_put_autosuspend(gbphy_dev);
>  	return 0;
>  
> -exit_gpio_irqchip_remove:
> -	gb_gpio_irqchip_remove(ggc);
>  exit_line_free:
>  	kfree(ggc->lines);
>  exit_connection_disable:
> @@ -741,8 +606,6 @@ static void gb_gpio_remove(struct gbphy_device *gbphy_dev)
>  		gbphy_runtime_get_noresume(gbphy_dev);
>  
>  	gb_connection_disable_rx(connection);
> -	gpiochip_remove(&ggc->chip);

This should stay as is, right?

> -	gb_gpio_irqchip_remove(ggc);
>  	gb_connection_disable(connection);
>  	gb_connection_destroy(connection);
>  	kfree(ggc->lines);

-- 
viresh

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web