[PATCH] gpiolib: reset lookup flags for each hogged line

[PATCH] gpiolib: reset lookup flags for each hogged line

[Jinseob Kim]

gpiochip_add_hog() initializes lflags once before iterating over the GPIO
specifiers. In the non-OF path, an active-low line sets GPIO_ACTIVE_LOW,
but a later active-high line does not clear it. A single firmware hog
with mixed polarities therefore configures later active-high lines as
active-low, including inverting their initial output value.

Initialize the lookup flags for each line before translating its flags.
The OF helper already overwrites the flags for each specifier; the
non-OF path must also start with a fresh value.

Add a KUnit case using one software-node hog with four alternating line
polarities. Check both the descriptor polarity and raw output level for
a logical high using the existing software GPIO provider. On UML, the
eight existing cases pass before the fix while the new case fails;
all nine cases pass after the fix with the same configuration.

Clang W=1 and private Sparse C=2 builds of both changed files complete
without diagnostics. No physical GPIO hardware was tested.

Codex assisted with the source analysis, fix, regression test and commit
message.

Fixes: 802c51a83e9a ("gpiolib: fix hogs with multiple lines")
Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
---
Target: brgl/linux gpio/for-current at 9e4b35c95fede87bad36f68b7ff1eae94ea6b910.
Tool use: Codex assisted a bounded review of public kernel code, the
minimal fix, test and description. The task requested a reproducible
general functional bug fix, not a fuzzing or security campaign.
Testing: actual before/after UML KUnit and Clang/private Sparse builds.
Physical GPIO hardware was not tested.

 drivers/gpio/gpiolib-kunit.c | 88 ++++++++++++++++++++++++++++++++++++
 drivers/gpio/gpiolib.c       |  3 +-
 2 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-kunit.c b/drivers/gpio/gpiolib-kunit.c
index f005854e7f..01205acf62 100644
--- a/drivers/gpio/gpiolib-kunit.c
+++ b/drivers/gpio/gpiolib-kunit.c
@@ -862,11 +862,99 @@ static struct kunit_suite gpio_swnode_hog_test_suite = {
 	.init = gpio_swnode_register_drivers,
 };
 
+/* A single non-OF hog may describe lines with different polarities. */
+static const struct software_node gpio_mixed_hog_provider_swnode = {
+	.name = "gpio-mixed-hog-provider",
+};
+
+static const u32 gpio_mixed_hog_gpios[] = {
+	0, GPIO_ACTIVE_LOW,
+	1, GPIO_ACTIVE_HIGH,
+	2, GPIO_ACTIVE_LOW,
+	3, GPIO_ACTIVE_HIGH,
+};
+
+static const struct property_entry gpio_mixed_hog_properties[] = {
+	PROPERTY_ENTRY_U32_ARRAY("gpios", gpio_mixed_hog_gpios),
+	PROPERTY_ENTRY_BOOL("output-high"),
+	PROPERTY_ENTRY_BOOL("gpio-hog"),
+	{ }
+};
+
+static const struct software_node gpio_mixed_hog_swnode =
+	SOFTWARE_NODE("mixed-hog", gpio_mixed_hog_properties,
+		      &gpio_mixed_hog_provider_swnode);
+
+static const struct software_node *const gpio_mixed_hog_swnodes[] = {
+	&gpio_mixed_hog_provider_swnode,
+	&gpio_mixed_hog_swnode,
+	NULL
+};
+
+static void gpio_hog_mixed_polarities(struct kunit *test)
+{
+	struct platform_device_info pdevinfo = {
+		.name = GPIO_TEST_PROVIDER,
+		.id = PLATFORM_DEVID_NONE,
+		.fwnode = software_node_fwnode(&gpio_mixed_hog_provider_swnode),
+	};
+	struct platform_device *pdev;
+	struct gpio_device *gdev;
+	struct gpio_desc *desc;
+	bool active_low;
+	int ret;
+
+	pdev = kunit_platform_device_register_full(test, &pdevinfo);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+
+	wait_for_device_probe();
+
+	gdev = gpio_device_find_by_label(GPIO_CONSUMER_NAME);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, gdev);
+
+	ret = kunit_add_action_or_reset(test, gpio_device_put_wrapper, gdev);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	for (unsigned int i = 0; i < GPIO_TEST_PROVIDER_NGPIO; i++) {
+		desc = gpio_device_get_desc(gdev, i);
+		KUNIT_ASSERT_NOT_ERR_OR_NULL(test, desc);
+
+		active_low = gpio_mixed_hog_gpios[i * 2 + 1] == GPIO_ACTIVE_LOW;
+		KUNIT_EXPECT_EQ(test, gpiod_is_active_low(desc), active_low);
+		/* A logical high must drive the inverse level on active-low lines. */
+		KUNIT_EXPECT_EQ(test, gpiod_get_raw_value_cansleep(desc), !active_low);
+	}
+}
+
+static int gpio_mixed_hog_suite_init(struct kunit_suite *suite)
+{
+	return software_node_register_node_group(gpio_mixed_hog_swnodes);
+}
+
+static void gpio_mixed_hog_suite_exit(struct kunit_suite *suite)
+{
+	software_node_unregister_node_group(gpio_mixed_hog_swnodes);
+}
+
+static struct kunit_case gpio_mixed_hog_tests[] = {
+	KUNIT_CASE(gpio_hog_mixed_polarities),
+	{ }
+};
+
+static struct kunit_suite gpio_mixed_hog_test_suite = {
+	.name = "gpio-mixed-hog-polarity",
+	.test_cases = gpio_mixed_hog_tests,
+	.suite_init = gpio_mixed_hog_suite_init,
+	.suite_exit = gpio_mixed_hog_suite_exit,
+	.init = gpio_swnode_register_drivers,
+};
+
 kunit_test_suites(
 	&gpio_swnode_lookup_test_suite,
 	&gpio_swnode_probe_order_test_suite,
 	&gpio_unbind_with_consumers_test_suite,
 	&gpio_swnode_hog_test_suite,
+	&gpio_mixed_hog_test_suite,
 );
 
 MODULE_DESCRIPTION("Test module for the GPIO subsystem");
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index fbdeda2f27..db613770dc 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -975,7 +975,6 @@ int gpiochip_add_hog(struct gpio_chip *gc, struct fwnode_handle *fwnode)
 	 */
 	u32 cells = 2;
 
-	lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
 	dflags = GPIOD_ASIS;
 	name = NULL;
 
@@ -1014,6 +1013,8 @@ int gpiochip_add_hog(struct gpio_chip *gc, struct fwnode_handle *fwnode)
 	fwnode_property_read_string(fwnode, "line-name", &name);
 
 	for (unsigned int i = 0; i < num_hogs; i++) {
+		lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
+
 		if (is_of_node(fwnode)) {
 			/*
 			 * OF-nodes need some additional special handling for

base-commit: 9e4b35c95fede87bad36f68b7ff1eae94ea6b910

Re: [PATCH] gpiolib: reset lookup flags for each hogged line

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] gpiolib: kunit: avoid duplicate hog test suite in gpiolib-kunit.c
- [Low] gpiolib: drop build/test log and tool commentary from commit message
--

commit 089aff495b510f5bab7031abfc65e971b196d1ef
Author: Jinseob Kim <kimjinseob88@gmail.com>

gpiolib: reset lookup flags for each hogged line

This commit resets the lookup flags variable inside the per-line loop in
gpiochip_add_hog() so that active-low flags do not carry over to subsequent
active-high lines in non-OF multi-line GPIO hogs, and adds a KUnit test case
for mixed-polarity software-node hogs.

>     gpiolib: reset lookup flags for each hogged line
>     
>     gpiochip_add_hog() initializes lflags once before iterating over the GPIO
>     specifiers. In the non-OF path, an active-low line sets GPIO_ACTIVE_LOW,
>     but a later active-high line does not clear it. A single firmware hog
>     with mixed polarities therefore configures later active-high lines as
>     active-low, including inverting their initial output value.
>     
>     Initialize the lookup flags for each line before translating its flags.
>     The OF helper already overwrites the flags for each specifier; the
>     non-OF path must also start with a fresh value.
>     
>     Add a KUnit case using one software-node hog with four alternating line
>     polarities. Check both the descriptor polarity and raw output level for
>     a logical high using the existing software GPIO provider. On UML, the
>     eight existing cases pass before the fix while the new case fails;
>     all nine cases pass after the fix with the same configuration.
>     
>     Clang W=1 and private Sparse C=2 builds of both changed files complete
>     without diagnostics. No physical GPIO hardware was tested.
>     
>     Codex assisted with the source analysis, fix, regression test and commit
>     message.
>     
>     Fixes: 802c51a83e9a ("gpiolib: fix hogs with multiple lines")
>     Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>

[Severity: Low]
This isn't a bug, but the commit message body includes before/after UML test
case counts, Clang W=1 and Sparse C=2 build check notes, a hardware testing
disclaimer, and a paragraph describing Codex assistance.

Since this meta-information is already provided below the tear line in the
patch notes, could these details be dropped from the permanent changelog so
the commit message stays focused on the bug, the fix, and the added test?

> diff --git a/drivers/gpio/gpiolib-kunit.c b/drivers/gpio/gpiolib-kunit.c
> index f005854e7ff7c..01205acf62651 100644
> --- a/drivers/gpio/gpiolib-kunit.c
> +++ b/drivers/gpio/gpiolib-kunit.c
> @@ -862,11 +862,99 @@ static struct kunit_suite gpio_swnode_hog_test_suite = {
>  	.init = gpio_swnode_register_drivers,
>  };
>  
> +/* A single non-OF hog may describe lines with different polarities. */
> +static const struct software_node gpio_mixed_hog_provider_swnode = {
> +	.name = "gpio-mixed-hog-provider",
> +};

[ ... ]

> +static int gpio_mixed_hog_suite_init(struct kunit_suite *suite)
> +{
> +	return software_node_register_node_group(gpio_mixed_hog_swnodes);
> +}
> +
> +static void gpio_mixed_hog_suite_exit(struct kunit_suite *suite)
> +{
> +	software_node_unregister_node_group(gpio_mixed_hog_swnodes);
> +}
> +
> +static struct kunit_case gpio_mixed_hog_tests[] = {
> +	KUNIT_CASE(gpio_hog_mixed_polarities),
> +	{ }
> +};
> +
> +static struct kunit_suite gpio_mixed_hog_test_suite = {
> +	.name = "gpio-mixed-hog-polarity",
> +	.test_cases = gpio_mixed_hog_tests,
> +	.suite_init = gpio_mixed_hog_suite_init,
> +	.suite_exit = gpio_mixed_hog_suite_exit,
> +	.init = gpio_swnode_register_drivers,
> +};

[Severity: Low]
This isn't a bug, but drivers/gpio/gpiolib-kunit.c already defines
gpio_swnode_hog_test_suite to group software-node GPIO hog test cases:

static int gpio_hog_suite_init(struct kunit_suite *suite)
{
	return software_node_register_node_group(gpio_hog_swnodes);
}

static void gpio_hog_suite_exit(struct kunit_suite *suite)
{
	software_node_unregister_node_group(gpio_hog_swnodes);
}

static struct kunit_case gpio_swnode_hog_tests[] = {
	KUNIT_CASE(gpio_hog_output_high),
	KUNIT_CASE(gpio_hog_output_low),
	KUNIT_CASE(gpio_hog_input),
	{ }
};

static struct kunit_suite gpio_swnode_hog_test_suite = {
	.name = "gpio-swnode-hog",
	.test_cases = gpio_swnode_hog_tests,
	.suite_init = gpio_hog_suite_init,
	.suite_exit = gpio_hog_suite_exit,
	.init = gpio_swnode_register_drivers,
};

Could gpio_mixed_hog_provider_swnode and gpio_mixed_hog_swnode be added to
gpio_hog_swnodes (or registered inside gpio_hog_mixed_polarities()), with
KUNIT_CASE(gpio_hog_mixed_polarities) added directly to
gpio_swnode_hog_tests[], instead of duplicating the suite definition and the
gpio_mixed_hog_suite_init() and gpio_mixed_hog_suite_exit() wrappers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103224339.1180-1-kimjinseob88@gmail.com?part=1