From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

We currently only notify user-space about line config changes that are
made from user-space. Any kernel config changes are not signalled.

Let's improve the situation by emitting the events closer to the source.
To that end let's call the relevant notifier chain from the functions
setting direction, gpiod_set_config(), gpiod_set_consumer_name() and
gpiod_toggle_active_low(). This covers all the options that we can
inform the user-space about. We ignore events which don't have
corresponding flags exported to user-space on purpose - otherwise the
user would see a config-changed event but the associated line-info would
remain unchanged.

gpiod_direction_output/input() can be called from any context.
Fortunately, we now emit line state events using an atomic notifier
chain, so it's no longer an issue.

Let's also add non-notifying wrappers around the direction setters in
order to not emit superfluous reconfigure events when requesting the
lines as the initial config should be part of the request notification.

Use gpio_do_set_config() instead of gpiod_set_debounce() for configuring
debouncing via hardware from the character device code to avoid multiple
reconfigure events.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib-cdev.c | 25 ++++++++-----
 drivers/gpio/gpiolib.c      | 89 ++++++++++++++++++++++++++++++++++++++-------
 drivers/gpio/gpiolib.h      |  3 ++
 3 files changed, 94 insertions(+), 23 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index cb4fb55e2696..13d83675bf4f 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -187,11 +187,11 @@ static long linehandle_set_config(struct linehandle_state *lh,
 		if (lflags & GPIOHANDLE_REQUEST_OUTPUT) {
 			int val = !!gcnf.default_values[i];
 
-			ret = gpiod_direction_output(desc, val);
+			ret = gpiod_direction_output_nonotify(desc, val);
 			if (ret)
 				return ret;
 		} else {
-			ret = gpiod_direction_input(desc);
+			ret = gpiod_direction_input_nonotify(desc);
 			if (ret)
 				return ret;
 		}
@@ -362,11 +362,11 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 		if (lflags & GPIOHANDLE_REQUEST_OUTPUT) {
 			int val = !!handlereq.default_values[i];
 
-			ret = gpiod_direction_output(desc, val);
+			ret = gpiod_direction_output_nonotify(desc, val);
 			if (ret)
 				goto out_free_lh;
 		} else if (lflags & GPIOHANDLE_REQUEST_INPUT) {
-			ret = gpiod_direction_input(desc);
+			ret = gpiod_direction_input_nonotify(desc);
 			if (ret)
 				goto out_free_lh;
 		}
@@ -922,8 +922,13 @@ static int debounce_setup(struct line *line, unsigned int debounce_period_us)
 	int ret, level, irq;
 	char *label;
 
-	/* try hardware */
-	ret = gpiod_set_debounce(line->desc, debounce_period_us);
+	/*
+	 * Try hardware. Skip gpiod_set_config() to avoid emitting two
+	 * CHANGED_CONFIG line state events.
+	 */
+	ret = gpio_do_set_config(line->desc,
+			pinconf_to_config_packed(PIN_CONFIG_INPUT_DEBOUNCE,
+						 debounce_period_us));
 	if (!ret) {
 		WRITE_ONCE(line->desc->debounce_period_us, debounce_period_us);
 		return ret;
@@ -1433,11 +1438,11 @@ static long linereq_set_config(struct linereq *lr, void __user *ip)
 			int val = gpio_v2_line_config_output_value(&lc, i);
 
 			edge_detector_stop(line);
-			ret = gpiod_direction_output(desc, val);
+			ret = gpiod_direction_output_nonotify(desc, val);
 			if (ret)
 				return ret;
 		} else {
-			ret = gpiod_direction_input(desc);
+			ret = gpiod_direction_input_nonotify(desc);
 			if (ret)
 				return ret;
 
@@ -1700,11 +1705,11 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
 		if (flags & GPIO_V2_LINE_FLAG_OUTPUT) {
 			int val = gpio_v2_line_config_output_value(lc, i);
 
-			ret = gpiod_direction_output(desc, val);
+			ret = gpiod_direction_output_nonotify(desc, val);
 			if (ret)
 				goto out_free_linereq;
 		} else if (flags & GPIO_V2_LINE_FLAG_INPUT) {
-			ret = gpiod_direction_input(desc);
+			ret = gpiod_direction_input_nonotify(desc);
 			if (ret)
 				goto out_free_linereq;
 
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 83e85dbfdeed..ae758ba6dc3d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2564,7 +2564,7 @@ EXPORT_SYMBOL_GPL(gpiochip_free_own_desc);
  * rely on gpio_request() having been called beforehand.
  */
 
-static int gpio_do_set_config(struct gpio_desc *desc, unsigned long config)
+int gpio_do_set_config(struct gpio_desc *desc, unsigned long config)
 {
 	int ret;
 
@@ -2670,9 +2670,15 @@ static int gpio_set_bias(struct gpio_desc *desc)
  */
 int gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce)
 {
-	return gpio_set_config_with_argument_optional(desc,
-						      PIN_CONFIG_INPUT_DEBOUNCE,
-						      debounce);
+	int ret;
+
+	ret = gpio_set_config_with_argument_optional(desc,
+						     PIN_CONFIG_INPUT_DEBOUNCE,
+						     debounce);
+	if (!ret)
+		gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG);
+
+	return ret;
 }
 
 /**
@@ -2686,6 +2692,18 @@ int gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce)
  * 0 on success, or negative errno on failure.
  */
 int gpiod_direction_input(struct gpio_desc *desc)
+{
+	int ret;
+
+	ret = gpiod_direction_input_nonotify(desc);
+	if (ret == 0)
+		gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(gpiod_direction_input);
+
+int gpiod_direction_input_nonotify(struct gpio_desc *desc)
 {
 	int ret = 0;
 
@@ -2733,7 +2751,6 @@ int gpiod_direction_input(struct gpio_desc *desc)
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(gpiod_direction_input);
 
 static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
 {
@@ -2795,8 +2812,15 @@ static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
  */
 int gpiod_direction_output_raw(struct gpio_desc *desc, int value)
 {
+	int ret;
+
 	VALIDATE_DESC(desc);
-	return gpiod_direction_output_raw_commit(desc, value);
+
+	ret = gpiod_direction_output_raw_commit(desc, value);
+	if (ret == 0)
+		gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(gpiod_direction_output_raw);
 
@@ -2814,6 +2838,18 @@ EXPORT_SYMBOL_GPL(gpiod_direction_output_raw);
  * 0 on success, or negative errno on failure.
  */
 int gpiod_direction_output(struct gpio_desc *desc, int value)
+{
+	int ret;
+
+	ret = gpiod_direction_output_nonotify(desc, value);
+	if (ret == 0)
+		gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(gpiod_direction_output);
+
+int gpiod_direction_output_nonotify(struct gpio_desc *desc, int value)
 {
 	unsigned long flags;
 	int ret;
@@ -2843,7 +2879,7 @@ int gpiod_direction_output(struct gpio_desc *desc, int value)
 			goto set_output_value;
 		/* Emulate open drain by not actively driving the line high */
 		if (value) {
-			ret = gpiod_direction_input(desc);
+			ret = gpiod_direction_input_nonotify(desc);
 			goto set_output_flag;
 		}
 	} else if (test_bit(FLAG_OPEN_SOURCE, &flags)) {
@@ -2852,7 +2888,7 @@ int gpiod_direction_output(struct gpio_desc *desc, int value)
 			goto set_output_value;
 		/* Emulate open source by not actively driving the line low */
 		if (!value) {
-			ret = gpiod_direction_input(desc);
+			ret = gpiod_direction_input_nonotify(desc);
 			goto set_output_flag;
 		}
 	} else {
@@ -2876,7 +2912,6 @@ int gpiod_direction_output(struct gpio_desc *desc, int value)
 		set_bit(FLAG_IS_OUT, &desc->flags);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(gpiod_direction_output);
 
 /**
  * gpiod_enable_hw_timestamp_ns - Enable hardware timestamp in nanoseconds.
@@ -2955,9 +2990,30 @@ EXPORT_SYMBOL_GPL(gpiod_disable_hw_timestamp_ns);
  */
 int gpiod_set_config(struct gpio_desc *desc, unsigned long config)
 {
+	int ret;
+
 	VALIDATE_DESC(desc);
 
-	return gpio_do_set_config(desc, config);
+	ret = gpio_do_set_config(desc, config);
+	if (!ret) {
+		/* These are the only options we notify the userspace about. */
+		switch (pinconf_to_config_param(config)) {
+		case PIN_CONFIG_BIAS_DISABLE:
+		case PIN_CONFIG_BIAS_PULL_DOWN:
+		case PIN_CONFIG_BIAS_PULL_UP:
+		case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+		case PIN_CONFIG_DRIVE_OPEN_SOURCE:
+		case PIN_CONFIG_DRIVE_PUSH_PULL:
+		case PIN_CONFIG_INPUT_DEBOUNCE:
+			gpiod_line_state_notify(desc,
+						GPIO_V2_LINE_CHANGED_CONFIG);
+			break;
+		default:
+			break;
+		}
+	}
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(gpiod_set_config);
 
@@ -3024,6 +3080,7 @@ void gpiod_toggle_active_low(struct gpio_desc *desc)
 {
 	VALIDATE_DESC_VOID(desc);
 	change_bit(FLAG_ACTIVE_LOW, &desc->flags);
+	gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG);
 }
 EXPORT_SYMBOL_GPL(gpiod_toggle_active_low);
 
@@ -3668,9 +3725,15 @@ EXPORT_SYMBOL_GPL(gpiod_cansleep);
  */
 int gpiod_set_consumer_name(struct gpio_desc *desc, const char *name)
 {
+	int ret;
+
 	VALIDATE_DESC(desc);
 
-	return desc_set_label(desc, name);
+	ret = desc_set_label(desc, name);
+	if (ret == 0)
+		gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(gpiod_set_consumer_name);
 
@@ -4548,10 +4611,10 @@ int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
 
 	/* Process flags */
 	if (dflags & GPIOD_FLAGS_BIT_DIR_OUT)
-		ret = gpiod_direction_output(desc,
+		ret = gpiod_direction_output_nonotify(desc,
 				!!(dflags & GPIOD_FLAGS_BIT_DIR_VAL));
 	else
-		ret = gpiod_direction_input(desc);
+		ret = gpiod_direction_input_nonotify(desc);
 
 	return ret;
 }
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index a54be597d21a..83690f72f7e5 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -155,6 +155,8 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 int gpiod_set_transitory(struct gpio_desc *desc, bool transitory);
 
 void gpiod_line_state_notify(struct gpio_desc *desc, unsigned long action);
+int gpiod_direction_output_nonotify(struct gpio_desc *desc, int value);
+int gpiod_direction_input_nonotify(struct gpio_desc *desc);
 
 struct gpio_desc_label {
 	struct rcu_head rh;
@@ -258,6 +260,7 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
 					 const char *label,
 					 bool platform_lookup_allowed);
 
+int gpio_do_set_config(struct gpio_desc *desc, unsigned long config);
 int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
 		unsigned long lflags, enum gpiod_flags dflags);
 int gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce);

-- 
2.43.0

On Fri, Oct 18, 2024 at 11:10:16AM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> We currently only notify user-space about line config changes that are
> made from user-space. Any kernel config changes are not signalled.
> 
> Let's improve the situation by emitting the events closer to the source.
> To that end let's call the relevant notifier chain from the functions
> setting direction, gpiod_set_config(), gpiod_set_consumer_name() and
> gpiod_toggle_active_low(). This covers all the options that we can
> inform the user-space about. We ignore events which don't have
> corresponding flags exported to user-space on purpose - otherwise the
> user would see a config-changed event but the associated line-info would
> remain unchanged.

Today's -next is not booting on several of my platforms, including
beaglebone-black, i.MX8MP-EVK and pine64plus.  Bisects are pointing at
this commit, and i.MX8MP-EVK is actually giving some console output:

[    2.502208] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[    2.511036] Mem abort info:

...

[    2.679934] Call trace:
[    2.682379]  gpiod_direction_output+0x34/0x5c
[    2.686745]  i2c_register_adapter+0x59c/0x670
[    2.691111]  __i2c_add_numbered_adapter+0x58/0xa8
[    2.695822]  i2c_add_adapter+0xa0/0xd0
[    2.699578]  i2c_add_numbered_adapter+0x2c/0x38
[    2.704117]  i2c_imx_probe+0x2d0/0x640

which looks plausible given the change.

Full boot log for i.MX8MP-EVK:

   https://lava.sirena.org.uk/scheduler/job/887083

Bisect log for that, the others look similar (the long run of good/bad
tags at the start for random commits is my automation pulling test
results it already knows about in the affected range to try to speed up
the bisect):

# bad: [ceab669fdf7b7510b4e4997b33d6f66e433a96db] Add linux-next specific files for 20241023
# good: [ad023864550daf9a5062e68f7925320146404919] Merge branch 'for-linux-next-fixes' of https://gitlab.freedesktop.org/drm/misc/kernel.git
# good: [db7e59e6a39a4d3d54ca8197c796557e6d480b0d] ASoC: qcom: sc7280: Fix missing Soundwire runtime stream alloc
# good: [d0ccf760a405d243a49485be0a43bd5b66ed17e2] spi: geni-qcom: Fix boot warning related to pm_runtime and devres
# good: [1e5d0f106164d2089826c16bb521891d1d06d3ad] ASoC: fsl_xcvr: reset RX dpath after wrong preamble
# good: [602ff58ae4fe4289b0ca71cba9fb82f7de92cd64] regulator: core: remove machine init callback from config
# good: [e6d20a9b0f376fda3e3c3709a59cefa6c0021784] ASoC: dt-bindings: everest,es8328: Document audio graph port
# good: [6a646e6de58f4aedf5f6c7a4605a0393c4490ef1] ASoC: dt-bindings: qcom: Add SM8750 LPASS macro codecs
# good: [5337ff41d37d4171868bb7b34dade68e269743f0] ASoC: soc-utils: Remove PAGE_SIZE compile-time constant assumption
# good: [f45a4399c1b582c6ddc179cc940aed73907b9453] spi: dt-bindings: samsung: Add a compatible for samsung,exynos8895-spi
# good: [42d20a6a61b8fccbb57d80df1ccde7dd82d5bbd6] spi: spi-mem: Add Realtek SPI-NAND controller
# good: [36dbe4521a381fd4d2561a90200ae4a2a3efb222] spi: make class structs const
# good: [1b9971a4e01b80afbf061ad7cdf84ac6fbbbde8d] ASoC: nau8821: check regmap_raw_read/regmap_raw_write for failure
# good: [e92edcf8023d425c7abcf1d7abb5dcac53d106f5] ASoC: SOF: Intel: hda: use machine_check() for SoundWire
# good: [4de1cdb3c299bb98d70198c1fa20c71f0893835c] spi: dt-bindings: brcm,bcm2835-aux-spi: Convert to dtschema
# good: [83c062ae81e89f73e3ab85953111a8b3daaaf98e] ASoC: Intel: sof_sdw: Add quirks for some new Lenovo laptops
# good: [9cb86a9cf12504c8dd60b40a6a200856852c1813] ASoC: SOF: sof-of-dev: add parameter to override tplg/fw_filename
# good: [c6631ceea573ae364e4fe913045f2aad10a10784] ASoC: rt-sdw-common: Enhance switch case to prevent uninitialized variable
# good: [45b3605089b41b81ba36b231fbb97e3037a51beb] ASoC: loongson: Fix build warning when !CONFIG_PCI
# good: [f5a0ea8936a640d8229d5219515141fc496ec5d8] ASoC: mediatek: mt8188: Remove unnecessary variable assignments
# good: [e0941775e6bdcf45e6e20b7ff3bb87dbb7d92fbb] ASoC/SoundWire: Intel: lnl: enable interrupts after first power-up/before last power-down
# good: [c1789209701143b50cba3783fa800a23df30a088] ASoC: codecs: Fix error check in es8323_i2c_probe
# good: [a0aae96be5ffc5b456ca07bfe1385b721c20e184] ASoC: Intel: avs: Fix return status of avs_pcm_hw_constraints_init()
# good: [941584e2f3ddde26e4d71941ebc0836ece181594] spi: stm32: fix missing device mode capability in stm32mp25
# good: [b39eec95b84d5dc326c3d7c89e4e08b898dbc73c] ASoC: imx-card: Add CS42888 support
# good: [8658c4eb9d6b76311322c1b74b3d4e0dec3599d8] ASoC: rt721-sdca: Clean logically deadcode in rt721-sdca.c
# good: [fceffbfe57af7d9941d08e1a995cccf558d08451] regulator: max5970: Drop unused structs
# good: [b1258105f9ce5203f48a47fd2f2cec8c38c41841] spi: intel: Add protected and locked attributes
# good: [ba4c5fad598c07492844e514add3ccda467063b2] ASoC: loongson: Add I2S controller driver as platform device
# good: [4e9a2c91bff44336157eefd8d80b8ceb27918737] regulator: dt-bindings: vctrl-regulator: convert to YAML
# good: [47701a85af0c0d655e06dd23f6b8761848147450] ASoC: SOF: ipc4-topology: Add helper function to print the module's in/out audio format
# good: [f3a59ab98cfc18c7b2fb1d8164bedbb1569a7e76] spi: spi-imx: Fix casting warnings
# good: [e2fc05873905f2ee96b38a116ae86f45fe7d8e49] spi: rockchip: Use dev_{err,warn}_probe() in the probe path
# good: [5cd575a87f141e438b3e062533bf0c6cc9eba99a] ASoC: dt-bindings: rockchip,rk3036-codec: convert to yaml
# good: [e5553cb6612989d18229c2b03948d6b4ba5d45f2] ASoC: rt721-sdca: Fix issue of warning message
# good: [86ce355c1f9ab943bbe099ea7d0b8a3af2247f65] ASoC: rt721-sdca: Add RT721 SDCA driver
# good: [846a8d3cf3bace9f235c38caf1d8d853c323dbd4] ASoC: Intel: soc-acpi-intel-ptl-match: Add rt721 support
# good: [0372abfcd81a4db94070d235e1ae3ff928efcab9] ASoC: amd: acp: refactor sof_card_dai_links_create() function
# good: [c6e86e19e778553dbedab617aafb25b6bbaf4cd9] ASoC: fsl: fsl_qmc_audio: Remove the logging when parsing channels
# good: [eb6c65049a274c37f9b6fdf632843b609a0b8fa8] spi: Provide defer reason if getting irq during probe fails
# good: [56d3705e4b36bf454965e66d8264356a23135aa7] ASoC: Intel: sof_rt5682: Add support for ptl_max98360a_rt5682
# good: [e58b3914ab8303a2783ec1873c17b7a83dd515f7] ASoC: dt-bindings: Deprecate {hp,mic}-det-gpio
# good: [64207f8024899938f8e13c4649a060a19f18bff3] ASoC: sh: rz-ssi: Use SSIFCR_FIFO_RST macro
# good: [46854574fd76c711c890423f8ac60df4fb726559] spi: spi-ti-qspi: remove redundant assignment to variable ret
# good: [6061483d7141db3a805f8660eae23805af02d544] ASoC: codecs: wcd9335: remove unnecessary MODULE_ALIAS()
# good: [8cd4e1f087b6906bacbbf8b637cac4e479a9cb34] ASoC: amd: acp: drop bogus NULL check from i2s_irq_handler
# good: [667b5e803a94f1ce48ac85b3fef94891a8d40ccf] spi: spi-fsl-lpspi: support effective_speed_hz
# good: [a34b9d812d7ec95789b15ce84de5f03c6dd1137b] ASoC: rt1320: fix the range of patch code address
# good: [4649cbd97fdae5069e9a71cd7669b62b90e03669] ASoC: dt-bindings: mt6359: Update generic node name and dmic-mode
# good: [ac8775d7de5a8ccac225a398cbce9fb9fffdbb9f] ASoC: atmel: atmel_ssc_dai: Drop S24_LE support due to single channel limitation
# good: [9864c8af89eb14a2e5334f8e24bb82086182e894] ASoC: amd: acp: remove unused variable from acp platform driver
# good: [625de1881b5aee6a42a3130004e47dbd632429f8] spi: atmel-quadspi: Add cs_hold and cs_inactive setting support
# good: [3c44a715e389929b8243d6a0545992d78cff6cba] ASoC: atmel: mchp-spdifrx: Remove interface name from stream_name
# good: [8adff2ff73d8271c993549b106b26f301fa003cf] ASoC: constify snd_soc_component_driver struct
# good: [dc9004ea273a9141c16b90a687da70b77f5a640a] ASoC: codecs: Add NeoFidelity NTP8835 codec
# good: [1482c40b440fa58f956bc3e1ef3426e0cdbc09e0] spi: rockchip-sfc: Use dev_err_probe() in the probe path
# good: [cc3ae21f360bfa375fc3539e24e7adb0e643a9d4] ASoC: fsl_micfil: Enable micfil error interrupt
# good: [49a85851b14cf6630013d1b9bae2ac2345c9430b] regcache: Store values more directly in maple trees
# good: [36ec3f437227470568e5f460997f367f5446a34d] regulator: Add devres version of of_regulator_get_optional()
# good: [18be43aca2c0ec475037923a8086d0a29fcc9d16] regulator: qcom-smd: make smd_vreg_rpm static
# good: [04e800fc328e6eba9f4ec3df375f2b500802653a] ASoC: codecs: aw88399: Fix spelling mistake "unsupport" -> "unsupported"
# good: [6c30eee359127c31cd8c6b586c8c3ced9f50f74b] spi: spi_amd: Add HIDDMA basic read support
# good: [0809a9ccac4a2ffdfd1561bb551aec6099775545] spi: remove {devm_}spi_alloc_master/slave()
# good: [7a01e17e42fe944982acde1dd40bdea177372173] ASoC: stm: fix macro definition on STM_SAI_HAS_EXT_SYNC
git bisect start 'ceab669fdf7b7510b4e4997b33d6f66e433a96db' 'ad023864550daf9a5062e68f7925320146404919' 'db7e59e6a39a4d3d54ca8197c796557e6d480b0d' 'd0ccf760a405d243a49485be0a43bd5b66ed17e2' '1e5d0f106164d2089826c16bb521891d1d06d3ad' '602ff58ae4fe4289b0ca71cba9fb82f7de92cd64' 'e6d20a9b0f376fda3e3c3709a59cefa6c0021784' '6a646e6de58f4aedf5f6c7a4605a0393c4490ef1' '5337ff41d37d4171868bb7b34dade68e269743f0' 'f45a4399c1b582c6ddc179cc940aed73907b9453' '42d20a6a61b8fccbb57d80df1ccde7dd82d5bbd6' '36dbe4521a381fd4d2561a90200ae4a2a3efb222' '1b9971a4e01b80afbf061ad7cdf84ac6fbbbde8d' 'e92edcf8023d425c7abcf1d7abb5dcac53d106f5' '4de1cdb3c299bb98d70198c1fa20c71f0893835c' '83c062ae81e89f73e3ab85953111a8b3daaaf98e' '9cb86a9cf12504c8dd60b40a6a200856852c1813' 'c6631ceea573ae364e4fe913045f2aad10a10784' '45b3605089b41b81ba36b231fbb97e3037a51beb' 'f5a0ea8936a640d8229d5219515141fc496ec5d8' 'e0941775e6bdcf45e6e20b7ff3bb87dbb7d92fbb' 'c1789209701143b50cba3783fa800a23df30a088' 'a0aae96be5ffc5b456ca07bfe1385b721c20e184' '941584e2f3ddde26e4d71941ebc0836ece181594' 'b39eec95b84d5dc326c3d7c89e4e08b898dbc73c' '8658c4eb9d6b76311322c1b74b3d4e0dec3599d8' 'fceffbfe57af7d9941d08e1a995cccf558d08451' 'b1258105f9ce5203f48a47fd2f2cec8c38c41841' 'ba4c5fad598c07492844e514add3ccda467063b2' '4e9a2c91bff44336157eefd8d80b8ceb27918737' '47701a85af0c0d655e06dd23f6b8761848147450' 'f3a59ab98cfc18c7b2fb1d8164bedbb1569a7e76' 'e2fc05873905f2ee96b38a116ae86f45fe7d8e49' '5cd575a87f141e438b3e062533bf0c6cc9eba99a' 'e5553cb6612989d18229c2b03948d6b4ba5d45f2' '86ce355c1f9ab943bbe099ea7d0b8a3af2247f65' '846a8d3cf3bace9f235c38caf1d8d853c323dbd4' '0372abfcd81a4db94070d235e1ae3ff928efcab9' 'c6e86e19e778553dbedab617aafb25b6bbaf4cd9' 'eb6c65049a274c37f9b6fdf632843b609a0b8fa8' '56d3705e4b36bf454965e66d8264356a23135aa7' 'e58b3914ab8303a2783ec1873c17b7a83dd515f7' '64207f8024899938f8e13c4649a060a19f18bff3' '46854574fd76c711c890423f8ac60df4fb726559' '6061483d7141db3a805f8660eae23805af02d544' '8cd4e1f087b6906bacbbf8b637cac4e479a9cb34' '667b5e803a94f1ce48ac85b3fef94891a8d40ccf' 'a34b9d812d7ec95789b15ce84de5f03c6dd1137b' '4649cbd97fdae5069e9a71cd7669b62b90e03669' 'ac8775d7de5a8ccac225a398cbce9fb9fffdbb9f' '9864c8af89eb14a2e5334f8e24bb82086182e894' '625de1881b5aee6a42a3130004e47dbd632429f8' '3c44a715e389929b8243d6a0545992d78cff6cba' '8adff2ff73d8271c993549b106b26f301fa003cf' 'dc9004ea273a9141c16b90a687da70b77f5a640a' '1482c40b440fa58f956bc3e1ef3426e0cdbc09e0' 'cc3ae21f360bfa375fc3539e24e7adb0e643a9d4' '49a85851b14cf6630013d1b9bae2ac2345c9430b' '36ec3f437227470568e5f460997f367f5446a34d' '18be43aca2c0ec475037923a8086d0a29fcc9d16' '04e800fc328e6eba9f4ec3df375f2b500802653a' '6c30eee359127c31cd8c6b586c8c3ced9f50f74b' '0809a9ccac4a2ffdfd1561bb551aec6099775545' '7a01e17e42fe944982acde1dd40bdea177372173'
# bad: [ceab669fdf7b7510b4e4997b33d6f66e433a96db] Add linux-next specific files for 20241023
git bisect bad ceab669fdf7b7510b4e4997b33d6f66e433a96db
# good: [397886451c608b881fc990abac627839b5010e4c] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git
git bisect good 397886451c608b881fc990abac627839b5010e4c
# good: [9bbf38bc6804c62ec9b29548b739f0e5dbd11d6b] Merge branch 'for-mfd-next' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git
git bisect good 9bbf38bc6804c62ec9b29548b739f0e5dbd11d6b
# good: [1b1952166f2b15f8d4665beeb8cc5443cda6f17d] Merge branch 'char-misc-next' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
git bisect good 1b1952166f2b15f8d4665beeb8cc5443cda6f17d
# good: [5a5a05d1e3cc8cb7127b5acac0fe647f4524567b] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git
git bisect good 5a5a05d1e3cc8cb7127b5acac0fe647f4524567b
# bad: [5ee7b36c01a29f242bd4c29dc95406caa53d0f1a] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching
git bisect bad 5ee7b36c01a29f242bd4c29dc95406caa53d0f1a
# bad: [b0ec3aaca3890d7e345e3ca4d58fb1b93d56354c] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git
git bisect bad b0ec3aaca3890d7e345e3ca4d58fb1b93d56354c
# bad: [8ddbd3e4039f2df83797cfda217f240d4a191bad] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/andy/linux-gpio-intel.git
git bisect bad 8ddbd3e4039f2df83797cfda217f240d4a191bad
# good: [2707a028c9b9c54a6dff22c9dcfebf3083ea095e] gpio: mpc8xxx: use a helper variable to store the address of pdev->dev
git bisect good 2707a028c9b9c54a6dff22c9dcfebf3083ea095e
# good: [8c44447bd76109e33a69fccda89c84715fbd5658] gpio: cdev: prepare gpio_desc_to_lineinfo() for being called from atomic
git bisect good 8c44447bd76109e33a69fccda89c84715fbd5658
# bad: [3aba8402910bfab998d5cf6c84744de5db466936] gpio: grgpio: remove remove()
git bisect bad 3aba8402910bfab998d5cf6c84744de5db466936
# bad: [07c61d4da43fa3b34c152b28010d20be115a96db] gpiolib: notify user-space about in-kernel line state changes
git bisect bad 07c61d4da43fa3b34c152b28010d20be115a96db
# good: [40b7c49950bd56c984b1f6722f865b922879260e] gpio: cdev: put emitting the line state events on a workqueue
git bisect good 40b7c49950bd56c984b1f6722f865b922879260e
# good: [fcc8b637c542d1a0c19e5797ad72f9258e10464c] gpiolib: switch the line state notifier to atomic
git bisect good fcc8b637c542d1a0c19e5797ad72f9258e10464c
# first bad commit: [07c61d4da43fa3b34c152b28010d20be115a96db] gpiolib: notify user-space about in-kernel line state changes

On Wed, Oct 23, 2024 at 11:05 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Oct 18, 2024 at 11:10:16AM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > We currently only notify user-space about line config changes that are
> > made from user-space. Any kernel config changes are not signalled.
> >
> > Let's improve the situation by emitting the events closer to the source.
> > To that end let's call the relevant notifier chain from the functions
> > setting direction, gpiod_set_config(), gpiod_set_consumer_name() and
> > gpiod_toggle_active_low(). This covers all the options that we can
> > inform the user-space about. We ignore events which don't have
> > corresponding flags exported to user-space on purpose - otherwise the
> > user would see a config-changed event but the associated line-info would
> > remain unchanged.
>
> Today's -next is not booting on several of my platforms, including
> beaglebone-black, i.MX8MP-EVK and pine64plus.  Bisects are pointing at
> this commit, and i.MX8MP-EVK is actually giving some console output:
>
> [    2.502208] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> [    2.511036] Mem abort info:
>
> ...
>
> [    2.679934] Call trace:
> [    2.682379]  gpiod_direction_output+0x34/0x5c
> [    2.686745]  i2c_register_adapter+0x59c/0x670
> [    2.691111]  __i2c_add_numbered_adapter+0x58/0xa8
> [    2.695822]  i2c_add_adapter+0xa0/0xd0
> [    2.699578]  i2c_add_numbered_adapter+0x2c/0x38
> [    2.704117]  i2c_imx_probe+0x2d0/0x640
>
> which looks plausible given the change.
>
> Full boot log for i.MX8MP-EVK:
>
>    https://lava.sirena.org.uk/scheduler/job/887083
>
> Bisect log for that, the others look similar (the long run of good/bad
> tags at the start for random commits is my automation pulling test
> results it already knows about in the affected range to try to speed up
> the bisect):
>

Hi Mark!

Any chance you could post the output of

    scripts/faddr2line drivers/gpio/gpiolib.o gpiod_direction_output+0x34/0x5c

for that build?

Bart

On Thu, 24 Oct 2024 08:49:30 +0200, Bartosz Golaszewski <brgl@bgdev.pl> said:
> On Wed, Oct 23, 2024 at 11:05 PM Mark Brown <broonie@kernel.org> wrote:
>>
>> On Fri, Oct 18, 2024 at 11:10:16AM +0200, Bartosz Golaszewski wrote:
>> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> >
>> > We currently only notify user-space about line config changes that are
>> > made from user-space. Any kernel config changes are not signalled.
>> >
>> > Let's improve the situation by emitting the events closer to the source.
>> > To that end let's call the relevant notifier chain from the functions
>> > setting direction, gpiod_set_config(), gpiod_set_consumer_name() and
>> > gpiod_toggle_active_low(). This covers all the options that we can
>> > inform the user-space about. We ignore events which don't have
>> > corresponding flags exported to user-space on purpose - otherwise the
>> > user would see a config-changed event but the associated line-info would
>> > remain unchanged.
>>
>> Today's -next is not booting on several of my platforms, including
>> beaglebone-black, i.MX8MP-EVK and pine64plus.  Bisects are pointing at
>> this commit, and i.MX8MP-EVK is actually giving some console output:
>>
>> [    2.502208] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
>> [    2.511036] Mem abort info:
>>
>> ...
>>
>> [    2.679934] Call trace:
>> [    2.682379]  gpiod_direction_output+0x34/0x5c
>> [    2.686745]  i2c_register_adapter+0x59c/0x670
>> [    2.691111]  __i2c_add_numbered_adapter+0x58/0xa8
>> [    2.695822]  i2c_add_adapter+0xa0/0xd0
>> [    2.699578]  i2c_add_numbered_adapter+0x2c/0x38
>> [    2.704117]  i2c_imx_probe+0x2d0/0x640
>>
>> which looks plausible given the change.
>>
>> Full boot log for i.MX8MP-EVK:
>>
>>    https://lava.sirena.org.uk/scheduler/job/887083
>>
>> Bisect log for that, the others look similar (the long run of good/bad
>> tags at the start for random commits is my automation pulling test
>> results it already knows about in the affected range to try to speed up
>> the bisect):
>>
>
> Hi Mark!
>
> Any chance you could post the output of
>
>     scripts/faddr2line drivers/gpio/gpiolib.o gpiod_direction_output+0x34/0x5c
>
> for that build?
>
> Bart
>

While I can't really reproduce it, I've looked at what could be wrong and
figured that the NULL-pointer in question can possibly be the
line_state_notifier.

I realized that for some historical reasons we add the GPIO device to the
global list before it's fully set up - including initializing the notifier.
In some corner cases (devlinks borked?) this could lead to consumers requesting
GPIOs before their provider is ready.

Mark: could you try the following diff and let me know if it works?

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index ae758ba6dc3d..4258acac0162 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -987,45 +987,6 @@ int gpiochip_add_data_with_key(struct gpio_chip
*gc, void *data,

 	gdev->ngpio = gc->ngpio;
 	gdev->can_sleep = gc->can_sleep;
-
-	scoped_guard(mutex, &gpio_devices_lock) {
-		/*
-		 * TODO: this allocates a Linux GPIO number base in the global
-		 * GPIO numberspace for this chip. In the long run we want to
-		 * get *rid* of this numberspace and use only descriptors, but
-		 * it may be a pipe dream. It will not happen before we get rid
-		 * of the sysfs interface anyways.
-		 */
-		base = gc->base;
-		if (base < 0) {
-			base = gpiochip_find_base_unlocked(gc->ngpio);
-			if (base < 0) {
-				ret = base;
-				base = 0;
-				goto err_free_label;
-			}
-
-			/*
-			 * TODO: it should not be necessary to reflect the
-			 * assigned base outside of the GPIO subsystem. Go over
-			 * drivers and see if anyone makes use of this, else
-			 * drop this and assign a poison instead.
-			 */
-			gc->base = base;
-		} else {
-			dev_warn(&gdev->dev,
-				 "Static allocation of GPIO base is deprecated, use dynamic
allocation.\n");
-		}
-
-		gdev->base = base;
-
-		ret = gpiodev_add_to_list_unlocked(gdev);
-		if (ret) {
-			chip_err(gc, "GPIO integer space overlap, cannot add chip\n");
-			goto err_free_label;
-		}
-	}
-
 	ATOMIC_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
 	BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);

@@ -1103,6 +1064,45 @@ int gpiochip_add_data_with_key(struct gpio_chip
*gc, void *data,
 		if (ret)
 			goto err_remove_irqchip;
 	}
+
+	scoped_guard(mutex, &gpio_devices_lock) {
+		/*
+		 * TODO: this allocates a Linux GPIO number base in the global
+		 * GPIO numberspace for this chip. In the long run we want to
+		 * get *rid* of this numberspace and use only descriptors, but
+		 * it may be a pipe dream. It will not happen before we get rid
+		 * of the sysfs interface anyways.
+		 */
+		base = gc->base;
+		if (base < 0) {
+			base = gpiochip_find_base_unlocked(gc->ngpio);
+			if (base < 0) {
+				ret = base;
+				base = 0;
+				goto err_free_label;
+			}
+
+			/*
+			 * TODO: it should not be necessary to reflect the
+			 * assigned base outside of the GPIO subsystem. Go over
+			 * drivers and see if anyone makes use of this, else
+			 * drop this and assign a poison instead.
+			 */
+			gc->base = base;
+		} else {
+			dev_warn(&gdev->dev,
+				 "Static allocation of GPIO base is deprecated, use dynamic
allocation.\n");
+		}
+
+		gdev->base = base;
+
+		ret = gpiodev_add_to_list_unlocked(gdev);
+		if (ret) {
+			chip_err(gc, "GPIO integer space overlap, cannot add chip\n");
+			goto err_free_label;
+		}
+	}
+
 	return 0;

 err_remove_irqchip:

Thanks
Bartosz

Hi,

On 2024-10-24 10:15, Bartosz Golaszewski wrote:
> On Thu, 24 Oct 2024 08:49:30 +0200, Bartosz Golaszewski <brgl@bgdev.pl> said:
>> On Wed, Oct 23, 2024 at 11:05 PM Mark Brown <broonie@kernel.org> wrote:
>>>
>>> On Fri, Oct 18, 2024 at 11:10:16AM +0200, Bartosz Golaszewski wrote:
>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>
>>>> We currently only notify user-space about line config changes that are
>>>> made from user-space. Any kernel config changes are not signalled.
>>>>
>>>> Let's improve the situation by emitting the events closer to the source.
>>>> To that end let's call the relevant notifier chain from the functions
>>>> setting direction, gpiod_set_config(), gpiod_set_consumer_name() and
>>>> gpiod_toggle_active_low(). This covers all the options that we can
>>>> inform the user-space about. We ignore events which don't have
>>>> corresponding flags exported to user-space on purpose - otherwise the
>>>> user would see a config-changed event but the associated line-info would
>>>> remain unchanged.
>>>
>>> Today's -next is not booting on several of my platforms, including
>>> beaglebone-black, i.MX8MP-EVK and pine64plus.  Bisects are pointing at
>>> this commit, and i.MX8MP-EVK is actually giving some console output:
>>>
>>> [    2.502208] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
>>> [    2.511036] Mem abort info:
>>>
>>> ...
>>>
>>> [    2.679934] Call trace:
>>> [    2.682379]  gpiod_direction_output+0x34/0x5c
>>> [    2.686745]  i2c_register_adapter+0x59c/0x670
>>> [    2.691111]  __i2c_add_numbered_adapter+0x58/0xa8
>>> [    2.695822]  i2c_add_adapter+0xa0/0xd0
>>> [    2.699578]  i2c_add_numbered_adapter+0x2c/0x38
>>> [    2.704117]  i2c_imx_probe+0x2d0/0x640
>>>
>>> which looks plausible given the change.
>>>
>>> Full boot log for i.MX8MP-EVK:
>>>
>>>     https://lava.sirena.org.uk/scheduler/job/887083
>>>
>>> Bisect log for that, the others look similar (the long run of good/bad
>>> tags at the start for random commits is my automation pulling test
>>> results it already knows about in the affected range to try to speed up
>>> the bisect):
>>>
>>
>> Hi Mark!
>>
>> Any chance you could post the output of
>>
>>      scripts/faddr2line drivers/gpio/gpiolib.o gpiod_direction_output+0x34/0x5c
>>
>> for that build?
>>
>> Bart
>>
> 
> While I can't really reproduce it, I've looked at what could be wrong and
> figured that the NULL-pointer in question can possibly be the
> line_state_notifier.
> 
> I realized that for some historical reasons we add the GPIO device to the
> global list before it's fully set up - including initializing the notifier.
> In some corner cases (devlinks borked?) this could lead to consumers requesting
> GPIOs before their provider is ready.
> 
> Mark: could you try the following diff and let me know if it works?
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index ae758ba6dc3d..4258acac0162 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -987,45 +987,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> 
>   	gdev->ngpio = gc->ngpio;
>   	gdev->can_sleep = gc->can_sleep;
> -
> -	scoped_guard(mutex, &gpio_devices_lock) {
> -		/*
> -		 * TODO: this allocates a Linux GPIO number base in the global
> -		 * GPIO numberspace for this chip. In the long run we want to
> -		 * get *rid* of this numberspace and use only descriptors, but
> -		 * it may be a pipe dream. It will not happen before we get rid
> -		 * of the sysfs interface anyways.
> -		 */
> -		base = gc->base;
> -		if (base < 0) {
> -			base = gpiochip_find_base_unlocked(gc->ngpio);
> -			if (base < 0) {
> -				ret = base;
> -				base = 0;
> -				goto err_free_label;
> -			}
> -
> -			/*
> -			 * TODO: it should not be necessary to reflect the
> -			 * assigned base outside of the GPIO subsystem. Go over
> -			 * drivers and see if anyone makes use of this, else
> -			 * drop this and assign a poison instead.
> -			 */
> -			gc->base = base;
> -		} else {
> -			dev_warn(&gdev->dev,
> -				 "Static allocation of GPIO base is deprecated, use dynamic allocation.\n");
> -		}
> -
> -		gdev->base = base;
> -
> -		ret = gpiodev_add_to_list_unlocked(gdev);
> -		if (ret) {
> -			chip_err(gc, "GPIO integer space overlap, cannot add chip\n");
> -			goto err_free_label;
> -		}
> -	}
> -
>   	ATOMIC_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
>   	BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);
> 
> @@ -1103,6 +1064,45 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
>   		if (ret)
>   			goto err_remove_irqchip;
>   	}
> +
> +	scoped_guard(mutex, &gpio_devices_lock) {
> +		/*
> +		 * TODO: this allocates a Linux GPIO number base in the global
> +		 * GPIO numberspace for this chip. In the long run we want to
> +		 * get *rid* of this numberspace and use only descriptors, but
> +		 * it may be a pipe dream. It will not happen before we get rid
> +		 * of the sysfs interface anyways.
> +		 */
> +		base = gc->base;
> +		if (base < 0) {
> +			base = gpiochip_find_base_unlocked(gc->ngpio);
> +			if (base < 0) {
> +				ret = base;
> +				base = 0;
> +				goto err_free_label;
> +			}
> +
> +			/*
> +			 * TODO: it should not be necessary to reflect the
> +			 * assigned base outside of the GPIO subsystem. Go over
> +			 * drivers and see if anyone makes use of this, else
> +			 * drop this and assign a poison instead.
> +			 */
> +			gc->base = base;
> +		} else {
> +			dev_warn(&gdev->dev,
> +				 "Static allocation of GPIO base is deprecated, use dynamic allocation.\n");
> +		}
> +
> +		gdev->base = base;
> +
> +		ret = gpiodev_add_to_list_unlocked(gdev);
> +		if (ret) {
> +			chip_err(gc, "GPIO integer space overlap, cannot add chip\n");
> +			goto err_free_label;
> +		}
> +	}
> +
>   	return 0;
> 
>   err_remove_irqchip:
> 
> Thanks
> Bartosz
> 

I think I hit the same, or a similar bug, on my Edgerouter 6P (Cavium 
Octeon III):

CPU 3 Unable to handle kernel paging request at virtual address 
0000000000000000, epc == ffffffff817a40c8, ra == ffffffff817a40a4
Oops[#1]:
CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Not tainted 
6.12.0-rc4-next-20241023-00032-g01c8e35f8d89 #84
...
Call Trace:
gpiod_direction_output (drivers/gpio/gpiolib.c:4164 
drivers/gpio/gpiolib.c:2700 drivers/gpio/gpiolib.c:2694)
i2c_register_adapter (drivers/i2c/i2c-core-base.c:396 
drivers/i2c/i2c-core-base.c:422 drivers/i2c/i2c-core-base.c:434 
drivers/i2c/i2c-core-base.c:1574)
octeon_i2c_probe (drivers/i2c/busses/i2c-octeon-platdrv.c:247)

(the complete log is attached)

Unfortunately the oops remains after applying the diff and the call 
trace looks to be the same.

Please let me know if there's anything else you need.

Regards,
Klara Modin# bad: [30226ad165626fa1d00945758ecafcfbdb47aed0] sched/ext: fix fmt fmt__str inconsistency
git bisect start 'HEAD'
# status: waiting for good commit(s), bad commit known
# good: [c2ee9f594da826bea183ed14f2cc029c719bf4da] KVM: selftests: Fix build on on non-x86 architectures
git bisect good c2ee9f594da826bea183ed14f2cc029c719bf4da
# good: [1f64fb9c2040c018d0e045b785b3d11a3bc0bf61] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git
git bisect good 1f64fb9c2040c018d0e045b785b3d11a3bc0bf61
# good: [d72293fcdadf4ca9fe265c9057e2d4e4b8c3fa7f] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git
git bisect good d72293fcdadf4ca9fe265c9057e2d4e4b8c3fa7f
# good: [d9e7d56ac23ba13e1255fa114aa1b90993386a40] Merge branch 'char-misc-next' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
git bisect good d9e7d56ac23ba13e1255fa114aa1b90993386a40
# bad: [a37d9a1b19767866febad59765806042bc49ad7c] Merge branch 'gpio/for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git
git bisect bad a37d9a1b19767866febad59765806042bc49ad7c
# good: [1096ccc17707eb50f677a6457bf65527bdf13d51] Merge branch 'staging-next' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
git bisect good 1096ccc17707eb50f677a6457bf65527bdf13d51
# good: [c6b673dd67833f12a52eedb2d7bb2429d7d95a8d] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git
git bisect good c6b673dd67833f12a52eedb2d7bb2429d7d95a8d
# good: [3bd13ae04ccc20e3a312596f89a269b8b6416dca] gpio: menz127: simplify error path and remove remove()
git bisect good 3bd13ae04ccc20e3a312596f89a269b8b6416dca
# bad: [3aba8402910bfab998d5cf6c84744de5db466936] gpio: grgpio: remove remove()
git bisect bad 3aba8402910bfab998d5cf6c84744de5db466936
# good: [81625f362497509526a2f9ac53843ae30b4709cc] gpio: cdev: go back to storing debounce period in the GPIO descriptor
git bisect good 81625f362497509526a2f9ac53843ae30b4709cc
# good: [fcc8b637c542d1a0c19e5797ad72f9258e10464c] gpiolib: switch the line state notifier to atomic
git bisect good fcc8b637c542d1a0c19e5797ad72f9258e10464c
# bad: [bc40668def384256673bc18296865869c4a4187b] gpio: grgpio: drop Kconfig dependency on OF_GPIO
git bisect bad bc40668def384256673bc18296865869c4a4187b
# bad: [07c61d4da43fa3b34c152b28010d20be115a96db] gpiolib: notify user-space about in-kernel line state changes
git bisect bad 07c61d4da43fa3b34c152b28010d20be115a96db
# first bad commit: [07c61d4da43fa3b34c152b28010d20be115a96db] gpiolib: notify user-space about in-kernel line state changes

On Thu, 24 Oct 2024 13:34:11 +0200, Klara Modin <klarasmodin@gmail.com> said:
>
> I think I hit the same, or a similar bug, on my Edgerouter 6P (Cavium
> Octeon III):
>
> CPU 3 Unable to handle kernel paging request at virtual address
> 0000000000000000, epc == ffffffff817a40c8, ra == ffffffff817a40a4
> Oops[#1]:
> CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Not tainted
> 6.12.0-rc4-next-20241023-00032-g01c8e35f8d89 #84
> ...
> Call Trace:
> gpiod_direction_output (drivers/gpio/gpiolib.c:4164
> drivers/gpio/gpiolib.c:2700 drivers/gpio/gpiolib.c:2694)
> i2c_register_adapter (drivers/i2c/i2c-core-base.c:396
> drivers/i2c/i2c-core-base.c:422 drivers/i2c/i2c-core-base.c:434
> drivers/i2c/i2c-core-base.c:1574)
> octeon_i2c_probe (drivers/i2c/busses/i2c-octeon-platdrv.c:247)
>
> (the complete log is attached)
>
> Unfortunately the oops remains after applying the diff and the call
> trace looks to be the same.
>
> Please let me know if there's anything else you need.
>

Hmm so it's desc->gdev that is NULL... Could you try the following:

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index ae758ba6dc3d..c95c79ea2b49 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1026,6 +1026,9 @@ int gpiochip_add_data_with_key(struct gpio_chip
*gc, void *data,
 		}
 	}

+	for (desc_index = 0; desc_index < gc->ngpio; desc_index++)
+		gdev->descs[desc_index].gdev = gdev;
+
 	ATOMIC_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
 	BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);

@@ -1055,8 +1058,6 @@ int gpiochip_add_data_with_key(struct gpio_chip
*gc, void *data,
 	for (desc_index = 0; desc_index < gc->ngpio; desc_index++) {
 		struct gpio_desc *desc = &gdev->descs[desc_index];

-		desc->gdev = gdev;
-
 		if (gc->get_direction && gpiochip_line_is_valid(gc, desc_index)) {
 			assign_bit(FLAG_IS_OUT,
 				   &desc->flags, !gc->get_direction(gc, desc_index));

I'm wondering if this is not an earlier commit.

Bart

On 2024-10-24 13:45, Bartosz Golaszewski wrote:
> On Thu, 24 Oct 2024 13:34:11 +0200, Klara Modin <klarasmodin@gmail.com> said:
>>
>> I think I hit the same, or a similar bug, on my Edgerouter 6P (Cavium
>> Octeon III):
>>
>> CPU 3 Unable to handle kernel paging request at virtual address
>> 0000000000000000, epc == ffffffff817a40c8, ra == ffffffff817a40a4
>> Oops[#1]:
>> CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Not tainted
>> 6.12.0-rc4-next-20241023-00032-g01c8e35f8d89 #84
>> ...
>> Call Trace:
>> gpiod_direction_output (drivers/gpio/gpiolib.c:4164
>> drivers/gpio/gpiolib.c:2700 drivers/gpio/gpiolib.c:2694)
>> i2c_register_adapter (drivers/i2c/i2c-core-base.c:396
>> drivers/i2c/i2c-core-base.c:422 drivers/i2c/i2c-core-base.c:434
>> drivers/i2c/i2c-core-base.c:1574)
>> octeon_i2c_probe (drivers/i2c/busses/i2c-octeon-platdrv.c:247)
>>
>> (the complete log is attached)
>>
>> Unfortunately the oops remains after applying the diff and the call
>> trace looks to be the same.
>>
>> Please let me know if there's anything else you need.
>>
> 
> Hmm so it's desc->gdev that is NULL... Could you try the following:
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index ae758ba6dc3d..c95c79ea2b49 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1026,6 +1026,9 @@ int gpiochip_add_data_with_key(struct gpio_chip
> *gc, void *data,
>   		}
>   	}
> 
> +	for (desc_index = 0; desc_index < gc->ngpio; desc_index++)
> +		gdev->descs[desc_index].gdev = gdev;
> +
>   	ATOMIC_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
>   	BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);
> 
> @@ -1055,8 +1058,6 @@ int gpiochip_add_data_with_key(struct gpio_chip
> *gc, void *data,
>   	for (desc_index = 0; desc_index < gc->ngpio; desc_index++) {
>   		struct gpio_desc *desc = &gdev->descs[desc_index];
> 
> -		desc->gdev = gdev;
> -
>   		if (gc->get_direction && gpiochip_line_is_valid(gc, desc_index)) {
>   			assign_bit(FLAG_IS_OUT,
>   				   &desc->flags, !gc->get_direction(gc, desc_index));
> 
> I'm wondering if this is not an earlier commit.
> 
> Bart

That doesn't seem to change anything significantly:

Call Trace:
gpiod_direction_output (drivers/gpio/gpiolib.c:4165 
drivers/gpio/gpiolib.c:2701 drivers/gpio/gpiolib.c:2695)
i2c_register_adapter (drivers/i2c/i2c-core-base.c:396 
drivers/i2c/i2c-core-base.c:422 drivers/i2c/i2c-core-base.c:434 
drivers/i2c/i2c-core-base.c:1574)
octeon_i2c_probe (drivers/i2c/busses/i2c-octeon-platdrv.c:247)

On Thu, Oct 24, 2024 at 2:00 PM Klara Modin <klarasmodin@gmail.com> wrote:
>
> On 2024-10-24 13:45, Bartosz Golaszewski wrote:
> > On Thu, 24 Oct 2024 13:34:11 +0200, Klara Modin <klarasmodin@gmail.com> said:
> >>
> >> I think I hit the same, or a similar bug, on my Edgerouter 6P (Cavium
> >> Octeon III):
> >>
> >> CPU 3 Unable to handle kernel paging request at virtual address
> >> 0000000000000000, epc == ffffffff817a40c8, ra == ffffffff817a40a4
> >> Oops[#1]:
> >> CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Not tainted
> >> 6.12.0-rc4-next-20241023-00032-g01c8e35f8d89 #84
> >> ...
> >> Call Trace:
> >> gpiod_direction_output (drivers/gpio/gpiolib.c:4164
> >> drivers/gpio/gpiolib.c:2700 drivers/gpio/gpiolib.c:2694)
> >> i2c_register_adapter (drivers/i2c/i2c-core-base.c:396
> >> drivers/i2c/i2c-core-base.c:422 drivers/i2c/i2c-core-base.c:434
> >> drivers/i2c/i2c-core-base.c:1574)
> >> octeon_i2c_probe (drivers/i2c/busses/i2c-octeon-platdrv.c:247)
> >>
> >> (the complete log is attached)
> >>
> >> Unfortunately the oops remains after applying the diff and the call
> >> trace looks to be the same.
> >>
> >> Please let me know if there's anything else you need.
> >>
> >
> > Hmm so it's desc->gdev that is NULL... Could you try the following:
> >
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index ae758ba6dc3d..c95c79ea2b49 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -1026,6 +1026,9 @@ int gpiochip_add_data_with_key(struct gpio_chip
> > *gc, void *data,
> >               }
> >       }
> >
> > +     for (desc_index = 0; desc_index < gc->ngpio; desc_index++)
> > +             gdev->descs[desc_index].gdev = gdev;
> > +
> >       ATOMIC_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
> >       BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);
> >
> > @@ -1055,8 +1058,6 @@ int gpiochip_add_data_with_key(struct gpio_chip
> > *gc, void *data,
> >       for (desc_index = 0; desc_index < gc->ngpio; desc_index++) {
> >               struct gpio_desc *desc = &gdev->descs[desc_index];
> >
> > -             desc->gdev = gdev;
> > -
> >               if (gc->get_direction && gpiochip_line_is_valid(gc, desc_index)) {
> >                       assign_bit(FLAG_IS_OUT,
> >                                  &desc->flags, !gc->get_direction(gc, desc_index));
> >
> > I'm wondering if this is not an earlier commit.
> >
> > Bart
>
> That doesn't seem to change anything significantly:
>

I managed to reproduce it by using optional GPIOs. I sent out a fix
with both of you in Cc, please give it a try.

Bart