From f1f37abbe6fc2b1242f78157db76e48dbf9518ee Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Mon, 19 Oct 2020 15:40:46 +0200 Subject: gpio: Retire the explicit gpio irqchip code Now that all gpiolib irqchip users have been over to use the irqchip template, we can finally retire the old code path and leave just one way in to the irqchip: set up the template when registering the gpio_chip. For a while we had two code paths for this which was a bit confusing. This brings this work to a conclusion, there is now one way of doing this. Signed-off-by: Linus Walleij Reviewed-by: Andy Shevchenko Cc: Thierry Reding Link: https://lore.kernel.org/r/20201019134046.65101-1-linus.walleij@linaro.org --- Documentation/driver-api/gpio/driver.rst | 63 +++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 21 deletions(-) (limited to 'Documentation/driver-api') diff --git a/Documentation/driver-api/gpio/driver.rst b/Documentation/driver-api/gpio/driver.rst index 072a7455044e..65d708093b71 100644 --- a/Documentation/driver-api/gpio/driver.rst +++ b/Documentation/driver-api/gpio/driver.rst @@ -416,7 +416,8 @@ The preferred way to set up the helpers is to fill in the struct gpio_irq_chip inside struct gpio_chip before adding the gpio_chip. If you do this, the additional irq_chip will be set up by gpiolib at the same time as setting up the rest of the GPIO functionality. The following -is a typical example of a cascaded interrupt handler using gpio_irq_chip: +is a typical example of a chained cascaded interrupt handler using +the gpio_irq_chip: .. code-block:: c @@ -452,7 +453,46 @@ is a typical example of a cascaded interrupt handler using gpio_irq_chip: return devm_gpiochip_add_data(dev, &g->gc, g); -The helper support using hierarchical interrupt controllers as well. +The helper supports using threaded interrupts as well. Then you just request +the interrupt separately and go with it: + +.. code-block:: c + + /* Typical state container with dynamic irqchip */ + struct my_gpio { + struct gpio_chip gc; + struct irq_chip irq; + }; + + int irq; /* from platform etc */ + struct my_gpio *g; + struct gpio_irq_chip *girq; + + /* Set up the irqchip dynamically */ + g->irq.name = "my_gpio_irq"; + g->irq.irq_ack = my_gpio_ack_irq; + g->irq.irq_mask = my_gpio_mask_irq; + g->irq.irq_unmask = my_gpio_unmask_irq; + g->irq.irq_set_type = my_gpio_set_irq_type; + + ret = devm_request_threaded_irq(dev, irq, NULL, + irq_thread_fn, IRQF_ONESHOT, "my-chip", g); + if (ret < 0) + return ret; + + /* Get a pointer to the gpio_irq_chip */ + girq = &g->gc.irq; + girq->chip = &g->irq; + /* This will let us handle the parent IRQ in the driver */ + girq->parent_handler = NULL; + girq->num_parents = 0; + girq->parents = NULL; + girq->default_type = IRQ_TYPE_NONE; + girq->handler = handle_bad_irq; + + return devm_gpiochip_add_data(dev, &g->gc, g); + +The helper supports using hierarchical interrupt controllers as well. In this case the typical set-up will look like this: .. code-block:: c @@ -493,25 +533,6 @@ the parent hardware irq from a child (i.e. this gpio chip) hardware irq. As always it is good to look at examples in the kernel tree for advice on how to find the required pieces. -The old way of adding irqchips to gpiochips after registration is also still -available but we try to move away from this: - -- DEPRECATED: gpiochip_irqchip_add(): adds a chained cascaded irqchip to a - gpiochip. It will pass the struct gpio_chip* for the chip to all IRQ - callbacks, so the callbacks need to embed the gpio_chip in its state - container and obtain a pointer to the container using container_of(). - (See Documentation/driver-api/driver-model/design-patterns.rst) - -- gpiochip_irqchip_add_nested(): adds a nested cascaded irqchip to a gpiochip, - as discussed above regarding different types of cascaded irqchips. The - cascaded irq has to be handled by a threaded interrupt handler. - Apart from that it works exactly like the chained irqchip. - -- gpiochip_set_nested_irqchip(): sets up a nested cascaded irq handler for a - gpio_chip from a parent IRQ. As the parent IRQ has usually been - explicitly requested by the driver, this does very little more than - mark all the child IRQs as having the other IRQ as parent. - If there is a need to exclude certain GPIO lines from the IRQ domain handled by these helpers, we can set .irq.need_valid_mask of the gpiochip before devm_gpiochip_add_data() or gpiochip_add_data() is called. This allocates an -- cgit v1.2.3 From 8aa16335050663357281fb1f1b0483ab91b4d8de Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Mon, 19 Oct 2020 15:44:29 +0200 Subject: gpio: stmpe: Fix forgotten refactoring We actually handle the gpio_irq_chip set-up properly now despite what the comment says. Also assign this pointer along with the rest of the gpio_irq_chip setup code. Signed-off-by: Linus Walleij Link: https://lore.kernel.org/r/20201019134429.65563-1-linus.walleij@linaro.org --- Documentation/driver-api/gpio/driver.rst | 4 ++-- drivers/gpio/gpio-stmpe.c | 10 +--------- 2 files changed, 3 insertions(+), 11 deletions(-) (limited to 'Documentation/driver-api') diff --git a/Documentation/driver-api/gpio/driver.rst b/Documentation/driver-api/gpio/driver.rst index 65d708093b71..0fb57e298b41 100644 --- a/Documentation/driver-api/gpio/driver.rst +++ b/Documentation/driver-api/gpio/driver.rst @@ -538,8 +538,8 @@ these helpers, we can set .irq.need_valid_mask of the gpiochip before devm_gpiochip_add_data() or gpiochip_add_data() is called. This allocates an .irq.valid_mask with as many bits set as there are GPIO lines in the chip, each bit representing line 0..n-1. Drivers can exclude GPIO lines by clearing bits -from this mask. The mask must be filled in before gpiochip_irqchip_add() or -gpiochip_irqchip_add_nested() is called. +from this mask. The mask can be filled in the init_valid_mask() callback +that is part of the struct gpio_irq_chip. To use the helpers please keep the following in mind: diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c index b0155d6007c8..b94ef8181428 100644 --- a/drivers/gpio/gpio-stmpe.c +++ b/drivers/gpio/gpio-stmpe.c @@ -474,15 +474,6 @@ static int stmpe_gpio_probe(struct platform_device *pdev) stmpe_gpio->chip.parent = &pdev->dev; stmpe_gpio->chip.of_node = np; stmpe_gpio->chip.base = -1; - /* - * REVISIT: this makes sure the valid mask gets allocated and - * filled in when adding the gpio_chip, but the rest of the - * gpio_irqchip is still filled in using the old method - * in gpiochip_irqchip_add_nested() so clean this up once we - * get the gpio_irqchip to initialize while adding the - * gpio_chip also for threaded irqchips. - */ - stmpe_gpio->chip.irq.init_valid_mask = stmpe_init_irq_valid_mask; if (IS_ENABLED(CONFIG_DEBUG_FS)) stmpe_gpio->chip.dbg_show = stmpe_dbg_show; @@ -520,6 +511,7 @@ static int stmpe_gpio_probe(struct platform_device *pdev) girq->default_type = IRQ_TYPE_NONE; girq->handler = handle_simple_irq; girq->threaded = true; + girq->init_valid_mask = stmpe_init_irq_valid_mask; } ret = gpiochip_add_data(&stmpe_gpio->chip, stmpe_gpio); -- cgit v1.2.3 From a0de695819f63b02645e0c1c8d493324c02b0eb0 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Sun, 22 Nov 2020 18:25:48 +0900 Subject: Documentation: gpio: fix typo and unclear legacy API section The "Interacting With the Legacy GPIO Subsystem" of the documentation was unclear at best, and even included a sentence that seems to say the opposite of what it should say about the lifetime of the return value of the conversion functions. Try to clarify things a bit and hopefully make that section more readable. Reported-by: Andy Shevchenko BugLink: https://stackoverflow.com/q/64455505/2511795 Signed-off-by: Alexandre Courbot Link: https://lore.kernel.org/r/20201122092548.61979-1-gnurou@gmail.com Signed-off-by: Linus Walleij --- Documentation/driver-api/gpio/consumer.rst | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'Documentation/driver-api') diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst index 423492d125b9..173e4c7b037d 100644 --- a/Documentation/driver-api/gpio/consumer.rst +++ b/Documentation/driver-api/gpio/consumer.rst @@ -440,18 +440,20 @@ For details refer to Documentation/firmware-guide/acpi/gpio-properties.rst Interacting With the Legacy GPIO Subsystem ========================================== -Many kernel subsystems still handle GPIOs using the legacy integer-based -interface. Although it is strongly encouraged to upgrade them to the safer -descriptor-based API, the following two functions allow you to convert a GPIO -descriptor into the GPIO integer namespace and vice-versa:: +Many kernel subsystems and drivers still handle GPIOs using the legacy +integer-based interface. It is strongly recommended to update these to the new +gpiod interface. For cases where both interfaces need to be used, the following +two functions allow to convert a GPIO descriptor into the GPIO integer namespace +and vice-versa:: int desc_to_gpio(const struct gpio_desc *desc) struct gpio_desc *gpio_to_desc(unsigned gpio) -The GPIO number returned by desc_to_gpio() can be safely used as long as the -GPIO descriptor has not been freed. All the same, a GPIO number passed to -gpio_to_desc() must have been properly acquired, and usage of the returned GPIO -descriptor is only possible after the GPIO number has been released. +The GPIO number returned by desc_to_gpio() can safely be used as a parameter of +the gpio\_*() functions for as long as the GPIO descriptor `desc` is not freed. +All the same, a GPIO number passed to gpio_to_desc() must first be properly +acquired using e.g. gpio_request_one(), and the returned GPIO descriptor is only +considered valid until that GPIO number is released using gpio_free(). Freeing a GPIO obtained by one API with the other API is forbidden and an unchecked error. -- cgit v1.2.3