From 5b2d3326cdf83fde3590f45c93507c1c852327e8 Mon Sep 17 00:00:00 2001 From: Alexandru Ardelean Date: Fri, 14 May 2021 12:32:26 +0300 Subject: [PATCH 1/6] gpio: crystalcove: remove platform_set_drvdata() + cleanup probe The platform_set_drvdata() call is only useful if we need to retrieve back the private information. Since the driver doesn't do that, it's not useful to have it. If this is removed, we can also just do a direct return on devm_gpiochip_add_data(). We don't need to print that this call failed as there are other ways to log/see this during probe. Signed-off-by: Alexandru Ardelean Signed-off-by: Andy Shevchenko --- drivers/gpio/gpio-crystalcove.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/drivers/gpio/gpio-crystalcove.c b/drivers/gpio/gpio-crystalcove.c index 2ba225720086..5a909f3c79e8 100644 --- a/drivers/gpio/gpio-crystalcove.c +++ b/drivers/gpio/gpio-crystalcove.c @@ -339,8 +339,6 @@ static int crystalcove_gpio_probe(struct platform_device *pdev) if (!cg) return -ENOMEM; - platform_set_drvdata(pdev, cg); - mutex_init(&cg->buslock); cg->chip.label = KBUILD_MODNAME; cg->chip.direction_input = crystalcove_gpio_dir_in; @@ -372,13 +370,7 @@ static int crystalcove_gpio_probe(struct platform_device *pdev) return retval; } - retval = devm_gpiochip_add_data(&pdev->dev, &cg->chip, cg); - if (retval) { - dev_warn(&pdev->dev, "add gpio chip error: %d\n", retval); - return retval; - } - - return 0; + return devm_gpiochip_add_data(&pdev->dev, &cg->chip, cg); } static struct platform_driver crystalcove_gpio_driver = { From f3019092eca09f3d093dbc306bf118daff15bfd2 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 20 May 2021 12:46:05 +0300 Subject: [PATCH 2/6] gpio: wcove: Use IRQ hardware number getter instead of direct access IRQ framework provides special type and getter to transform Linux IRQ to the hardware pin. Use that type and getter function instead of direct access. No functional changes intended. While at it, remove unneeded check in wcove_update_irq_ctrl() since it is guaranteed that function will be called with valid parameter. Signed-off-by: Andy Shevchenko --- drivers/gpio/gpio-wcove.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c index a19eeef6cf1e..923a37a3d6d3 100644 --- a/drivers/gpio/gpio-wcove.c +++ b/drivers/gpio/gpio-wcove.c @@ -129,7 +129,7 @@ static inline int to_ireg(int gpio, enum ctrl_register type, unsigned int *mask) return reg; } -static void wcove_update_irq_mask(struct wcove_gpio *wg, int gpio) +static void wcove_update_irq_mask(struct wcove_gpio *wg, irq_hw_number_t gpio) { unsigned int mask, reg = to_ireg(gpio, IRQ_MASK, &mask); @@ -139,13 +139,10 @@ static void wcove_update_irq_mask(struct wcove_gpio *wg, int gpio) regmap_clear_bits(wg->regmap, reg, mask); } -static void wcove_update_irq_ctrl(struct wcove_gpio *wg, int gpio) +static void wcove_update_irq_ctrl(struct wcove_gpio *wg, irq_hw_number_t gpio) { int reg = to_reg(gpio, CTRL_IN); - if (reg < 0) - return; - regmap_update_bits(wg->regmap, reg, CTLI_INTCNT_BE, wg->intcnt); } @@ -248,8 +245,9 @@ static int wcove_irq_type(struct irq_data *data, unsigned int type) { struct gpio_chip *chip = irq_data_get_irq_chip_data(data); struct wcove_gpio *wg = gpiochip_get_data(chip); + irq_hw_number_t gpio = irqd_to_hwirq(data); - if (data->hwirq >= WCOVE_GPIO_NUM) + if (gpio >= WCOVE_GPIO_NUM) return 0; switch (type) { @@ -286,7 +284,7 @@ static void wcove_bus_sync_unlock(struct irq_data *data) { struct gpio_chip *chip = irq_data_get_irq_chip_data(data); struct wcove_gpio *wg = gpiochip_get_data(chip); - int gpio = data->hwirq; + irq_hw_number_t gpio = irqd_to_hwirq(data); if (wg->update & UPDATE_IRQ_TYPE) wcove_update_irq_ctrl(wg, gpio); @@ -301,8 +299,9 @@ static void wcove_irq_unmask(struct irq_data *data) { struct gpio_chip *chip = irq_data_get_irq_chip_data(data); struct wcove_gpio *wg = gpiochip_get_data(chip); + irq_hw_number_t gpio = irqd_to_hwirq(data); - if (data->hwirq >= WCOVE_GPIO_NUM) + if (gpio >= WCOVE_GPIO_NUM) return; wg->set_irq_mask = false; @@ -313,8 +312,9 @@ static void wcove_irq_mask(struct irq_data *data) { struct gpio_chip *chip = irq_data_get_irq_chip_data(data); struct wcove_gpio *wg = gpiochip_get_data(chip); + irq_hw_number_t gpio = irqd_to_hwirq(data); - if (data->hwirq >= WCOVE_GPIO_NUM) + if (gpio >= WCOVE_GPIO_NUM) return; wg->set_irq_mask = true; From 5d9936648285b8ccb6b61257dd2ddd76f9cd719a Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 20 May 2021 13:00:01 +0300 Subject: [PATCH 3/6] gpio: wcove: Unify style of to_reg() with to_ireg() Use ternary and rename parameter to 'type' in to_reg() to be in the same style as to_ireg(). Signed-off-by: Andy Shevchenko --- drivers/gpio/gpio-wcove.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c index 923a37a3d6d3..5ce315f64dc5 100644 --- a/drivers/gpio/gpio-wcove.c +++ b/drivers/gpio/gpio-wcove.c @@ -99,19 +99,14 @@ struct wcove_gpio { bool set_irq_mask; }; -static inline int to_reg(int gpio, enum ctrl_register reg_type) +static inline int to_reg(int gpio, enum ctrl_register type) { - unsigned int reg; + unsigned int reg = type == CTRL_IN ? GPIO_IN_CTRL_BASE : GPIO_OUT_CTRL_BASE; if (gpio >= WCOVE_GPIO_NUM) return -EOPNOTSUPP; - if (reg_type == CTRL_IN) - reg = GPIO_IN_CTRL_BASE + gpio; - else - reg = GPIO_OUT_CTRL_BASE + gpio; - - return reg; + return reg + gpio; } static inline int to_ireg(int gpio, enum ctrl_register type, unsigned int *mask) From 2b71b66ac0415db5e4b9e67b11e3af7b485bb421 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 20 May 2021 15:17:15 +0300 Subject: [PATCH 4/6] gpio: wcove: Split error handling for CTRL and IRQ registers The wcove_gpio_dbg_show() uses one message to all possible error reads. Split it to two for CTRL and IRQ registers. While at it, switch to use dev_err() instead of pr_err(). Signed-off-by: Andy Shevchenko --- drivers/gpio/gpio-wcove.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c index 5ce315f64dc5..16a0fae1e32e 100644 --- a/drivers/gpio/gpio-wcove.c +++ b/drivers/gpio/gpio-wcove.c @@ -364,8 +364,7 @@ static irqreturn_t wcove_gpio_irq_handler(int irq, void *data) return IRQ_HANDLED; } -static void wcove_gpio_dbg_show(struct seq_file *s, - struct gpio_chip *chip) +static void wcove_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip) { unsigned int ctlo, ctli, irq_mask, irq_status; struct wcove_gpio *wg = gpiochip_get_data(chip); @@ -374,10 +373,15 @@ static void wcove_gpio_dbg_show(struct seq_file *s, for (gpio = 0; gpio < WCOVE_GPIO_NUM; gpio++) { ret += regmap_read(wg->regmap, to_reg(gpio, CTRL_OUT), &ctlo); ret += regmap_read(wg->regmap, to_reg(gpio, CTRL_IN), &ctli); + if (ret) { + dev_err(wg->dev, "Failed to read registers: CTRL out/in\n"); + break; + } + ret += regmap_read(wg->regmap, to_ireg(gpio, IRQ_MASK, &mask), &irq_mask); ret += regmap_read(wg->regmap, to_ireg(gpio, IRQ_STATUS, &mask), &irq_status); if (ret) { - pr_err("Failed to read registers: ctrl out/in or irq status/mask\n"); + dev_err(wg->dev, "Failed to read registers: IRQ status/mask\n"); break; } From 43582f29b161d820717bc13f562bca27af12e3cf Mon Sep 17 00:00:00 2001 From: Daniel Scally Date: Thu, 3 Jun 2021 23:40:04 +0100 Subject: [PATCH 5/6] gpiolib: acpi: Introduce acpi_get_and_request_gpiod() helper We need to be able to translate GPIO resources in an ACPI device's _CRS into GPIO descriptor array. Those are represented in _CRS as a pathname to a GPIO device plus the pin's index number: the acpi_get_gpiod() function is perfect for that purpose. As it's currently only used internally within the GPIO layer, provide and export a wrapper function that additionally holds a reference to the GPIO device. Reviewed-by: Andy Shevchenko Signed-off-by: Daniel Scally Signed-off-by: Andy Shevchenko --- drivers/gpio/gpiolib-acpi.c | 28 ++++++++++++++++++++++++++++ include/linux/gpio/consumer.h | 2 ++ 2 files changed, 30 insertions(+) diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index 3ef22a3c104d..75cd0c5a5cc4 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -128,6 +128,34 @@ static struct gpio_desc *acpi_get_gpiod(char *path, int pin) return gpiochip_get_desc(chip, pin); } +/** + * acpi_get_and_request_gpiod - Translate ACPI GPIO pin to GPIO descriptor and + * hold a refcount to the GPIO device. + * @path: ACPI GPIO controller full path name, (e.g. "\\_SB.GPO1") + * @pin: ACPI GPIO pin number (0-based, controller-relative) + * @label: Label to pass to gpiod_request() + * + * This function is a simple pass-through to acpi_get_gpiod(), except that + * as it is intended for use outside of the GPIO layer (in a similar fashion to + * gpiod_get_index() for example) it also holds a reference to the GPIO device. + */ +struct gpio_desc *acpi_get_and_request_gpiod(char *path, int pin, char *label) +{ + struct gpio_desc *gpio; + int ret; + + gpio = acpi_get_gpiod(path, pin); + if (IS_ERR(gpio)) + return gpio; + + ret = gpiod_request(gpio, label); + if (ret) + return ERR_PTR(ret); + + return gpio; +} +EXPORT_SYMBOL_GPL(acpi_get_and_request_gpiod); + static irqreturn_t acpi_gpio_irq_handler(int irq, void *data) { struct acpi_gpio_event *event = data; diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h index c73b25bc9213..566feb56601f 100644 --- a/include/linux/gpio/consumer.h +++ b/include/linux/gpio/consumer.h @@ -692,6 +692,8 @@ int devm_acpi_dev_add_driver_gpios(struct device *dev, const struct acpi_gpio_mapping *gpios); void devm_acpi_dev_remove_driver_gpios(struct device *dev); +struct gpio_desc *acpi_get_and_request_gpiod(char *path, int pin, char *label); + #else /* CONFIG_GPIOLIB && CONFIG_ACPI */ struct acpi_device; From 043d7f09bf614809c10c4acbf0695ef731958300 Mon Sep 17 00:00:00 2001 From: Daniel Scally Date: Thu, 3 Jun 2021 23:40:05 +0100 Subject: [PATCH 6/6] gpiolib: acpi: Add acpi_gpio_get_io_resource() Add a function to verify that a given ACPI resource represents a GpioIo() type of resource, and return it if so. Reviewed-by: Andy Shevchenko Signed-off-by: Daniel Scally Signed-off-by: Andy Shevchenko --- drivers/gpio/gpiolib-acpi.c | 23 +++++++++++++++++++++++ include/linux/acpi.h | 7 +++++++ 2 files changed, 30 insertions(+) diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index 75cd0c5a5cc4..cf99a5752fe0 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -196,6 +196,29 @@ bool acpi_gpio_get_irq_resource(struct acpi_resource *ares, } EXPORT_SYMBOL_GPL(acpi_gpio_get_irq_resource); +/** + * acpi_gpio_get_io_resource - Fetch details of an ACPI resource if it is a GPIO + * I/O resource or return False if not. + * @ares: Pointer to the ACPI resource to fetch + * @agpio: Pointer to a &struct acpi_resource_gpio to store the output pointer + */ +bool acpi_gpio_get_io_resource(struct acpi_resource *ares, + struct acpi_resource_gpio **agpio) +{ + struct acpi_resource_gpio *gpio; + + if (ares->type != ACPI_RESOURCE_TYPE_GPIO) + return false; + + gpio = &ares->data.gpio; + if (gpio->connection_type != ACPI_RESOURCE_GPIO_TYPE_IO) + return false; + + *agpio = gpio; + return true; +} +EXPORT_SYMBOL_GPL(acpi_gpio_get_io_resource); + static void acpi_gpiochip_request_irq(struct acpi_gpio_chip *acpi_gpio, struct acpi_gpio_event *event) { diff --git a/include/linux/acpi.h b/include/linux/acpi.h index c60745f657e9..a74d37a3b618 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -1096,6 +1096,8 @@ void __acpi_handle_debug(struct _ddebug *descriptor, acpi_handle handle, const c #if defined(CONFIG_ACPI) && defined(CONFIG_GPIOLIB) bool acpi_gpio_get_irq_resource(struct acpi_resource *ares, struct acpi_resource_gpio **agpio); +bool acpi_gpio_get_io_resource(struct acpi_resource *ares, + struct acpi_resource_gpio **agpio); int acpi_dev_gpio_irq_get_by(struct acpi_device *adev, const char *name, int index); #else static inline bool acpi_gpio_get_irq_resource(struct acpi_resource *ares, @@ -1103,6 +1105,11 @@ static inline bool acpi_gpio_get_irq_resource(struct acpi_resource *ares, { return false; } +static inline bool acpi_gpio_get_io_resource(struct acpi_resource *ares, + struct acpi_resource_gpio **agpio) +{ + return false; +} static inline int acpi_dev_gpio_irq_get_by(struct acpi_device *adev, const char *name, int index) {