public inbox for linux-gpio@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.19-5.10] gpio: aspeed-sgpio: Change the macro to support deferred probe
       [not found] <20260214010245.3671907-1-sashal@kernel.org>
@ 2026-02-14  0:59 ` Sasha Levin
  2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-6.18] gpio: pca953x: Add support for TCAL6408 TCAL6416 Sasha Levin
  1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2026-02-14  0:59 UTC (permalink / raw)
  To: patches, stable
  Cc: Billy Tsai, Linus Walleij, Bartosz Golaszewski, Sasha Levin, brgl,
	joel, andrew, linux-gpio, linux-arm-kernel, linux-aspeed

From: Billy Tsai <billy_tsai@aspeedtech.com>

[ Upstream commit e18533b023ec7a33488bcf33140ce69bbba2894f ]

Use module_platform_driver() to replace module_platform_driver_probe().
The former utilizes platform_driver_register(), which allows the driver to
defer probing when it doesn't acquire the necessary resources due to probe
order. In contrast, the latter uses __platform_driver_probe(), which
includes the comment "Note that this is incompatible with deferred
probing." Since our SGPIO driver requires access to the clock resource, the
former is more suitable.

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
Link: https://lore.kernel.org/r/20260123-upstream_sgpio-v2-1-69cfd1631400@aspeedtech.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis of gpio: aspeed-sgpio: Change the macro to support deferred
probe

### Commit Message Analysis

The commit changes `module_platform_driver_probe()` to
`module_platform_driver()` in the Aspeed SGPIO driver. The motivation is
to support deferred probing — when the clock resource isn't available
yet at probe time due to probe ordering, the driver should be able to
defer and retry later. With `module_platform_driver_probe()` (which uses
`__platform_driver_probe()`), deferred probing is explicitly not
supported.

### Code Change Analysis

The changes are:

1. **Remove `__init` annotation** from `aspeed_sgpio_probe()` —
   necessary because with `module_platform_driver()`, the probe function
   can be called after init (during deferred probe), so it can't be in
   `.init` section.

2. **Move `.probe` into the `platform_driver` struct** — from being
   passed as a second argument to `module_platform_driver_probe()` to
   being set as `.probe = aspeed_sgpio_probe` in the struct.

3. **Replace `module_platform_driver_probe()` with
   `module_platform_driver()`** — the actual behavioral change.

The probe function itself is completely unchanged in logic.

### Bug Classification

This fixes a **real probe failure bug**. When the Aspeed SGPIO driver is
compiled and the clock resource it depends on isn't yet available
(common on device-tree platforms where probe order isn't guaranteed),
the driver will fail to probe and **never retry**. This means the SGPIO
hardware is permanently non-functional until reboot, and even then only
if probe order happens to be favorable.

This is a real-world issue on Aspeed BMC platforms (AST2400, AST2500,
AST2600) which are widely used in server management. GPIO functionality
being unavailable due to probe ordering is a tangible user-facing bug.

### Scope and Risk Assessment

- **Lines changed**: Very small — removal of `__init`, moving the
  `.probe` assignment, macro swap
- **Files touched**: 1 file
- **Risk**: Very low. The probe function logic is completely unchanged.
  The only behavioral difference is that:
  1. The probe function is no longer in `.init` section (minor memory
     impact, but standard practice)
  2. Deferred probing is now supported (fixes the bug)
  3. The probe function pointer is stored in the driver struct rather
     than being patched in at registration time

This is a well-understood pattern — many drivers have made this exact
same transition over the years. The `module_platform_driver_probe()` to
`module_platform_driver()` conversion is one of the most common and
safest changes in the kernel.

### Stable Criteria Check

1. **Obviously correct and tested**: Yes — reviewed by Linus Walleij
   (GPIO subsystem co-maintainer), standard pattern
2. **Fixes a real bug**: Yes — probe failure when clock resource isn't
   available due to ordering
3. **Important issue**: Moderate — makes hardware permanently non-
   functional on affected platforms
4. **Small and contained**: Yes — minimal changes to one file
5. **No new features**: Correct — deferred probing is an existing kernel
   mechanism, this just enables the driver to participate in it
6. **Applies cleanly**: Should apply cleanly to any stable tree that has
   this driver

### User Impact

Aspeed BMC platforms are common in enterprise server management
(OpenBMC). If SGPIO fails to probe due to clock resource ordering, GPIO
pins managed by the SGPIO controller become unavailable, which can
affect system monitoring, power control, and other BMC functions. This
is a real-world issue that affects actual deployments.

### Concerns

- The commit doesn't have a `Fixes:` tag, which is expected for commits
  under review
- No explicit mention of user reports, but the commit author is from
  Aspeed Technology, indicating this is a known issue in their platform
- Minor: probe function no longer freed after init, but this is
  negligible memory impact

### Decision

This is a small, well-understood fix for a real probe failure bug on
Aspeed BMC platforms. The pattern change (`module_platform_driver_probe`
→ `module_platform_driver`) is one of the safest and most common
conversions in the kernel. It fixes a real-world issue where hardware
becomes non-functional due to probe ordering. The risk is minimal and
the benefit is clear.

**YES**

 drivers/gpio/gpio-aspeed-sgpio.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-aspeed-sgpio.c b/drivers/gpio/gpio-aspeed-sgpio.c
index 7622f9e9f54af..318cd0e397416 100644
--- a/drivers/gpio/gpio-aspeed-sgpio.c
+++ b/drivers/gpio/gpio-aspeed-sgpio.c
@@ -516,7 +516,7 @@ static const struct of_device_id aspeed_sgpio_of_table[] = {
 
 MODULE_DEVICE_TABLE(of, aspeed_sgpio_of_table);
 
-static int __init aspeed_sgpio_probe(struct platform_device *pdev)
+static int aspeed_sgpio_probe(struct platform_device *pdev)
 {
 	u32 nr_gpios, sgpio_freq, sgpio_clk_div, gpio_cnt_regval, pin_mask;
 	const struct aspeed_sgpio_pdata *pdata;
@@ -611,11 +611,12 @@ static int __init aspeed_sgpio_probe(struct platform_device *pdev)
 }
 
 static struct platform_driver aspeed_sgpio_driver = {
+	.probe = aspeed_sgpio_probe,
 	.driver = {
 		.name = KBUILD_MODNAME,
 		.of_match_table = aspeed_sgpio_of_table,
 	},
 };
 
-module_platform_driver_probe(aspeed_sgpio_driver, aspeed_sgpio_probe);
+module_platform_driver(aspeed_sgpio_driver);
 MODULE_DESCRIPTION("Aspeed Serial GPIO Driver");
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH AUTOSEL 6.19-6.18] gpio: pca953x: Add support for TCAL6408 TCAL6416
       [not found] <20260214010245.3671907-1-sashal@kernel.org>
  2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-5.10] gpio: aspeed-sgpio: Change the macro to support deferred probe Sasha Levin
@ 2026-02-14  0:59 ` Sasha Levin
  1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2026-02-14  0:59 UTC (permalink / raw)
  To: patches, stable
  Cc: Jan Remmet, Bartosz Golaszewski, Sasha Levin, linusw, brgl,
	linux-gpio

From: Jan Remmet <j.remmet@phytec.de>

[ Upstream commit a30a9cb9bca4296d25f253619883e7013b6be158 ]

TCAL6408 and TCAL6416 supports latchable inputs and maskable interrupt.
Tested on a TCAL6416, checked datasheets for the TCAL6408.

They use the same programming model ad the NXP PCAL64xx, but
support a lower supply power (1.08V to 3.6V) compared to PCAL
(1.65V to 5.5V)

Datasheet: https://www.ti.com/lit/ds/symlink/tcal6408.pdf
Datasheet: https://www.ti.com/lit/ds/symlink/tcal6416.pdf

Signed-off-by: Jan Remmet <j.remmet@phytec.de>
Link: https://lore.kernel.org/r/20251216-wip-jremmet-tcal6416rtw-v2-3-6516d98a9836@phytec.de
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Good - there's no fallback compatible mechanism here. Each chip gets its
own compatible string.

## Complete Analysis

### 1. Commit Message Analysis

The commit message is clear and well-structured:
- Explicitly states the chips use "the same programming model as the NXP
  PCAL64xx"
- Only difference is supply voltage range (1.08V-3.6V vs 1.65V-5.5V)
- References both datasheets for verification
- States it was "Tested on a TCAL6416" — author has real hardware
- The author (from phytec.de, an embedded hardware company) has a clear
  use case

### 2. Code Change Analysis

The changes are **purely table additions**:

**I2C device ID table** (`pca953x_id[]`): Adds 2 entries:
- `{ "tcal6408", 8 | PCA953X_TYPE | PCA_LATCH_INT, }` — 8-bit GPIO
  expander with PCAL features
- `{ "tcal6416", 16 | PCA953X_TYPE | PCA_LATCH_INT, }` — 16-bit GPIO
  expander with PCAL features

These use the exact same type flags (`PCA953X_TYPE | PCA_LATCH_INT`) as
the already-supported `pcal6408` and `pcal6416`, confirming identical
register layout handling.

**OF match table** (`pca953x_dt_ids[]`): Adds 2 entries:
- `{ .compatible = "ti,tcal6408", .data = OF_953X(8, PCA_LATCH_INT), }`
- `{ .compatible = "ti,tcal6416", .data = OF_953X(16, PCA_LATCH_INT), }`

**Kconfig**: Updates help text to list the new chip names (purely
cosmetic).

**No new code paths**, no new functions, no new registers, no new logic.
The `PCA_LATCH_INT` flag (`PCA_PCAL | PCA_INT`) already enables all the
extended PCAL register handling throughout the driver (input latching,
interrupt masking, pull-up/down config, interrupt status).

### 3. Classification

This is a textbook **new device ID** addition to an existing driver. The
analysis guidelines explicitly call this out:

> NEW DEVICE IDs (Very Common): Adding PCI IDs, USB IDs, ACPI IDs, etc.
to existing drivers. These are trivial one-line additions that enable
hardware support. Rule: The driver must already exist in stable; only
the ID is new.

The `pca953x` driver exists in all stable trees (it's been in the kernel
for over 15 years). The TCAL chips use the exact same PCAL register
model that's already fully supported.

### 4. Scope and Risk Assessment

- **Lines changed**: ~10 lines across 2 files (Kconfig help text +
  device tables)
- **Files touched**: 2 (`Kconfig`, `gpio-pca953x.c`)
- **Risk**: Essentially zero. Table entries can only affect systems with
  these specific chips. No existing functionality is modified.
- **Merge conflicts**: The entries are added at the end of their
  respective groups, so conflicts are unlikely unless another chip was
  added in the same spot.

### 5. User Impact

- Users with TCAL6408 or TCAL6416 on their boards (embedded systems,
  industrial boards) cannot use these chips without this patch
- Without this patch, the hardware is completely non-functional for
  these users
- Phytec (the author's company) ships embedded modules that likely use
  these chips

### 6. Dependencies

- The DT binding YAML update (patch 1-2 of the series) is needed for
  `dt_binding_check` but is NOT needed for the driver to function. A
  device tree using `compatible = "ti,tcal6408"` or `compatible =
  "ti,tcal6416"` will be matched by the driver with just this patch.
- No code dependencies on other patches.

### 7. Precedent

The exact same pattern has been used many times for this driver. For
example:
- `82466bb622e92` "gpio: pca953x: Add support for TI TCA9535 variant" —
  same pattern, same situation (lower voltage TI variant of NXP chip)
- `6c99a046edfac` "gpio: pca953x: Add support for TI TCA6418"
- `a9e49635e263b` "gpio: pca953xx: Add support for pca6408"

### Summary

This is a pure device ID addition to a well-established, widely-used
GPIO expander driver. The TCAL6408/TCAL6416 are Texas Instruments chips
that are register-compatible with the already-supported NXP
PCAL6408/PCAL6416. The change adds only table entries (I2C device IDs
and OF compatible strings) and zero new code logic. It enables real
hardware for real users (tested on actual hardware), with effectively
zero risk of regression to existing users. This is precisely the kind of
"device ID addition" that stable kernel guidelines explicitly allow as
an exception.

**YES**

 drivers/gpio/Kconfig        | 4 ++--
 drivers/gpio/gpio-pca953x.c | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index bd185482a7fdf..3439e025ba1c6 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1193,11 +1193,11 @@ config GPIO_PCA953X
 
 	  8 bits:       max7310, max7315, pca6107, pca9534, pca9538, pca9554,
 	                pca9556, pca9557, pca9574, tca6408, tca9554, xra1202,
-			pcal6408, pcal9554b, tca9538
+			pcal6408, pcal9554b, tca9538, tcal6408
 
 	  16 bits:      max7312, max7313, pca9535, pca9539, pca9555, pca9575,
 	                tca6416, pca6416, pcal6416, pcal9535, pcal9555a, max7318,
-			tca9539
+			tca9539, tcal6416
 
 	  18 bits:	tca6418
 
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index f93a3dbb2daaf..52e96cc5f67bb 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -126,6 +126,9 @@ static const struct i2c_device_id pca953x_id[] = {
 	{ "tca9539", 16 | PCA953X_TYPE | PCA_INT, },
 	{ "tca9554", 8  | PCA953X_TYPE | PCA_INT, },
 	{ "xra1202", 8  | PCA953X_TYPE },
+
+	{ "tcal6408", 8  | PCA953X_TYPE | PCA_LATCH_INT, },
+	{ "tcal6416", 16 | PCA953X_TYPE | PCA_LATCH_INT, },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, pca953x_id);
@@ -1469,6 +1472,9 @@ static const struct of_device_id pca953x_dt_ids[] = {
 	{ .compatible = "ti,tca9538", .data = OF_953X( 8, PCA_INT), },
 	{ .compatible = "ti,tca9539", .data = OF_953X(16, PCA_INT), },
 
+	{ .compatible = "ti,tcal6408", .data = OF_953X( 8, PCA_LATCH_INT), },
+	{ .compatible = "ti,tcal6416", .data = OF_953X(16, PCA_LATCH_INT), },
+
 	{ .compatible = "onnn,cat9554", .data = OF_953X( 8, PCA_INT), },
 	{ .compatible = "onnn,pca9654", .data = OF_953X( 8, PCA_INT), },
 	{ .compatible = "onnn,pca9655", .data = OF_953X(16, PCA_INT), },
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-02-14  1:06 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260214010245.3671907-1-sashal@kernel.org>
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-5.10] gpio: aspeed-sgpio: Change the macro to support deferred probe Sasha Levin
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-6.18] gpio: pca953x: Add support for TCAL6408 TCAL6416 Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox