Re: [PATCH v6 08/13] gpio: regmap: support write_data_after_dir and girq
From: sashiko-bot@kernel.org
Date: 2026-09-04 06:54:58
Also in:
linux-clk, linux-serial
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: New issues: - [High] Unconditional assignment to the conditionally compiled struct member `chip->irq` causes a build failure when `CONFIG_GPIOLIB_IRQCHIP` is disabled. - [High] The return value of the newly added `gpio_regmap_set()` call is ignored, silently dropping I/O errors. Pre-existing issues: - [High] Hardcoded calls to `gpio_regmap_set()` in `gpio_regmap_direction_output()` break hardware that uses a separate `reg_clr_base`. - [High] Missing `regmap_del_irq_chip()` in the error path of `gpio_regmap_register()` leads to a resource leak and potential Use-After-Free of the regmap. -- commit 52a1a780fb0893e26861b516090b05377ddeade2 Author: Long Zhao [off-list ref] Subject: gpio: regmap: support write_data_after_dir and girq Add an optional write-after-direction-output quirk for controllers that ignore data-register writes while a line is still configured as input. Also allow drivers to pass an existing gpio_irq_chip through gpio_regmap_register().
quoted hunk ↗ jump to hunk
diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c index 51b4d69b87403..b4b492a1765c4 100644 --- a/drivers/gpio/gpio-regmap.c +++ b/drivers/gpio/gpio-regmap.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -273,7 +274,15 @@ static int gpio_regmap_direction_output(struct gpio_chip *chip, gpio_regmap_set(chip, offset, value); - return gpio_regmap_set_direction(chip, offset, true); + ret = gpio_regmap_set_direction(chip, offset, true); + if (ret) + return ret; + + /* Some controllers ignore data writes while the line is still an input. */ + if (gpio->write_data_after_dir) + gpio_regmap_set(chip, offset, value);
[Severity: High] This is a pre-existing issue, but this duplicates the hardcoded call to gpio_regmap_set() instead of using chip->set(). Because hardware with a separate reg_clr_base expects gpio_regmap_set_with_clear(), calling gpio_regmap_set() directly fails to clear bits when configuring the line as an output with an initial value of 0. Could this be updated to use chip->set(chip, offset, value) so that controllers using reg_clr_base will function correctly? [Severity: High] Does this newly added call drop potential regmap I/O errors? gpio_regmap_set() returns an int to indicate success or failure, but the return value here is ignored, which could silently drop hardware communication failures.
+ + return 0; } void *gpio_regmap_get_drvdata(struct gpio_regmap *gpio)
[ ... ]
quoted hunk ↗ jump to hunk
@@ -390,6 +409,9 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config if (!gpio->reg_mask_xlate) gpio->reg_mask_xlate = gpio_regmap_simple_xlate; + if (config->girq) + chip->irq = *config->girq; +
[Severity: High]
Can this assignment cause a build failure?
The irq member of struct gpio_chip is conditionally compiled based on
CONFIG_GPIOLIB_IRQCHIP:
include/linux/gpio/driver.h:
#ifdef CONFIG_GPIOLIB_IRQCHIP
...
struct gpio_irq_chip irq;
#endif
If CONFIG_GPIO_REGMAP is enabled but CONFIG_GPIOLIB_IRQCHIP is disabled, this
unconditional assignment will fail to compile. Should this be wrapped in an
#ifdef guard?
[Severity: High]
This is a pre-existing issue, but does the error path in this function leak
the regmap IRQ chip and thread?
Later in gpio_regmap_register(), if gpiochip_irqchip_add_domain() fails:
if (irq_domain) {
ret = gpiochip_irqchip_add_domain(chip, irq_domain);
if (ret)
goto err_remove_gpiochip;
}
The code jumps to err_remove_gpiochip, which does not call
regmap_del_irq_chip(). This leaves the IRQ thread active and referencing a
regmap that will soon be freed, potentially causing a use-after-free.
ret = gpiochip_add_data(chip, gpio); if (ret < 0) goto err_free_bitmap_output;
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260904-cv75-v5-v6-0-e918514cb3b1@ambarella.com?part=8