public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Billy Tsai <billy_tsai@aspeedtech.com>,
	Linus Walleij <linusw@kernel.org>,
	Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>,
	Sasha Levin <sashal@kernel.org>,
	brgl@kernel.org, joel@jms.id.au, andrew@codeconstruct.com.au,
	linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org
Subject: [PATCH AUTOSEL 6.19-5.10] gpio: aspeed-sgpio: Change the macro to support deferred probe
Date: Fri, 13 Feb 2026 19:59:14 -0500	[thread overview]
Message-ID: <20260214010245.3671907-74-sashal@kernel.org> (raw)
In-Reply-To: <20260214010245.3671907-1-sashal@kernel.org>

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



  parent reply	other threads:[~2026-02-14  1:05 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260214010245.3671907-1-sashal@kernel.org>
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-5.15] drm/v3d: Set DMA segment size to avoid debug warnings Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-6.12] ASoC: fsl: imx-rpmsg: use snd_soc_find_dai_with_mutex() in probe Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-5.10] drm/atmel-hlcdc: fix use-after-free of drm_crtc_commit after release Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-5.15] spi: stm32: fix Overrun issue at < 8bpw Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-5.10] drm/atmel-hlcdc: fix memory leak from the atomic_destroy_state callback Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-6.1] media: rkisp1: Fix filter mode register configuration Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-5.15] drm/atmel-hlcdc: don't reject the commit if the src rect has fractional parts Sasha Levin
2026-02-14  0:59 ` Sasha Levin [this message]
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19] drm/atmel-hlcdc: destroy properly the plane state in the reset callback Sasha Levin
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-6.6] media: mediatek: vcodec: Don't try to decode 422/444 VP9 Sasha Levin
2026-02-14  1:00 ` [PATCH AUTOSEL 6.19-6.1] ASoC: sunxi: sun50i-dmic: Add missing check for devm_regmap_init_mmio Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260214010245.3671907-74-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=andrew@codeconstruct.com.au \
    --cc=bartosz.golaszewski@oss.qualcomm.com \
    --cc=billy_tsai@aspeedtech.com \
    --cc=brgl@kernel.org \
    --cc=joel@jms.id.au \
    --cc=linusw@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox