Re: [PATCH v3] spi: rockchip: initialize bus_num to -1

Re: [PATCH v3] spi: rockchip: initialize bus_num to -1

[Ahmad Fatoum]

Hi,

On 8/27/26 2:06 PM, support wrote:
> Applied, thanks!  

I have no idea what this is.

Before pointing a bot that can send email at the mailing list, please
start a (human) discussion about what you are think this accomplishes.

Also generally this style of mails is not acceptable. It confuses users
to say you apply patches (apply where?) and the formatting is broken.

Ahmad

> 
> 
> 
> 
> Thanks for the quick turnaround on this one. This bug class is more common than the MNT report suggests — any RK3588/RK3568 board 
> 
> 
> 
> design with a PMIC on one SPI controller and peripherals on another (which is the standard topology for RK806-based designs, 
> 
> 
> 
> e.g. our own boards with the PMIC on SPI2) hits the same bus_num=0 collision the moment a second controller is enabled. 
> 
> 
> 
> So far most of these designs simply never enabled two controllers in barebox, which is probably why it stayed latent this long. 
> 
> 
> 
> 
> One observation on the failure mode itself: the XFM_RO while(1) hang in rockchip_spi_pio is a separate latent hazard. Even with bus routing now correct, 
> 
> 
> 
> any future misrouted or wrong-device read can still wedge the bootloader with no timeout and no diagnostic output. 
> 
> 
> 
> Might be worth a follow-up hardening patch — a transfer timeout in the PIO loop would turn a silent hang into a loud error at the cost of a few lines.
> 
> 
> 
> We're tracking this fix for our RK35xx barebox evaluation (secure boot path for industrial designs), so it's on the regression checklist for the next release.
> 
> 
> 
>  --Boardcon Embedded Design — Rockchip-based industrial SBCs & SoMs
> 
> 
> 
> https://www.boardcon.com
> 
> 
> 
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH v3] spi: rockchip: initialize bus_num to -1

[Sascha Hauer]

On Mon, 24 Aug 2026 18:21:05 -0700, Stephano Cetola wrote:
> spi_register_controller() only runs the OF alias lookup when
> ctrl->bus_num is negative. This driver's kzalloc'd ctlr leaves
> bus_num at 0. The lookup gets skipped. Every Rockchip SPI controller
> ends up on bus 0.
> 
> Any board that enables two Rockchip SPI controllers at once hits this
> bug. Both end up on bus 0. Looking one up by bus number can then
> return the wrong controller.
> 
> [...]

Applied, thanks!

[1/1] spi: rockchip: initialize bus_num to -1
      https://git.pengutronix.de/cgit/barebox/commit/?id=df1055a2e0dd (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

[PATCH v3] spi: rockchip: initialize bus_num to -1

[Stephano Cetola]

spi_register_controller() only runs the OF alias lookup when
ctrl->bus_num is negative. This driver's kzalloc'd ctlr leaves
bus_num at 0. The lookup gets skipped. Every Rockchip SPI controller
ends up on bus 0.

Any board that enables two Rockchip SPI controllers at once hits this
bug. Both end up on bus 0. Looking one up by bus number can then
return the wrong controller.

Use -1 here to trigger the lookup from the device tree. No board board
file deliberately sets the id.

Fixes: 1efba64ba04 ("spi: add rockchip spi controller support")
Signed-off-by: Stephano Cetola <stephano@cetola.net>
---
We saw this on the MNT Pocket Reform (RK3588S). spi@feb20000 returns
the PMIC controller instead of the intended gpio-spi RP2040 bus. The
barebox spi command then does a write+read through the Rockchip SPI
driver, which programmes the hardware in XFM_RO (RX-only) mode for
the read phase. In that mode dummy TXDR writes don't generate clock
pulses, the RX FIFO never fills, and rockchip_spi_pio's while(1) loop
never exits.
---
Changes in v3:
- Initialize bus_num to -1 directly. Copying dev->id worked by
  coincidence, not by design.
- Rewrote the commit message to explain why -1 is correct, per
  Sascha's review.
- Link to v2: https://patch.msgid.link/20260822-send-spi-rockchip-busnum-v2-1-0f43376cb391@cetola.net

Changes in v2:
- Updated commit message for clarity.
- Link to v1: https://patch.msgid.link/20260822-send-spi-rockchip-busnum-v1-1-bf9c183786e4@cetola.net

To: Sascha Hauer <s.hauer@pengutronix.de>
To: "open list:BAREBOX" <barebox@lists.infradead.org>
---
 drivers/spi/spi-rockchip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 1e81e9393f..0ee31c1f55 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -514,7 +514,7 @@ static int rockchip_spi_probe(struct device *dev)
 		goto err_put_ctlr;
 	}
 
-//	ctlr->bus_num = pdev->id;
+	ctlr->bus_num = -1;
 //	ctlr->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP | SPI_LSB_FIRST;
 
 	/*

---
base-commit: 9bc1a26592a59919d2ee8c60c273d87d3d9f2e81
change-id: 20260821-send-spi-rockchip-busnum-a9b1116e14a5