Skip to content

spi: uclass: skip cs lookup if bus not probed - #93

Open
jiez wants to merge 1 commit into
adi-u-boot-2025.10.yfrom
92-sf-probe-error-on-sc598-som-ezkit
Open

spi: uclass: skip cs lookup if bus not probed#93
jiez wants to merge 1 commit into
adi-u-boot-2025.10.yfrom
92-sf-probe-error-on-sc598-som-ezkit

Conversation

@jiez

@jiez jiez commented May 5, 2026

Copy link
Copy Markdown

spi_find_bus_and_cs() uses uclass_find_device_by_seq() which locates the bus without probing it. Calling spi_find_chip_select() on an unprobed bus causes drivers that validate chip selects using private data (initialized only at probe time) to spuriously reject valid CS numbers. Since no SPI device can exist on an unprobed bus, return -ENODEV early instead.

U-Boot SPL 2025.10 (May 04 2026 - 12:40:02 -0400)
ADI Boot Mode: 0x1 (QSPI Master)
Trying to boot from BOOTROM


U-Boot 2025.10 (May 04 2026 - 12:40:02 -0400)

CPU:   ADSP ADSP-SC598-0.0 (QSPI Master boot)
Model: ADI SC598-SOM-EZKIT
DRAM:  224 MiB
ADP558x Detected: Rev 4, Rev ID 4
Core:  139 devices, 23 uclasses, devicetree: embed
MMC:   mmc@310C7000: 0
Loading Environment from nowhere... OK
In:    serial@0x31003000
Out:   serial@0x31003000
Err:   serial@0x31003000
Net:   eth0: eth0
=> sf probe
adi_spi3 spi2: invalid chipselect 1
adi_spi3 spi2: Invalid cs 1 (err=-22)
SF: Detected is25lp01g with page size 256 Bytes, erase size 4 KiB, total 128 MiB
=>

@jiez
jiez requested review from a team, ozan956 and pamolloy May 5, 2026 01:43
@jiez jiez linked an issue May 5, 2026 that may be closed by this pull request

@CalebEthridgeADI CalebEthridgeADI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on a Rev E sc598.

@ozan956 ozan956 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spi_find_bus_and_cs() is documented as a lookup helper that finds the bus and slave without activating either side, but this change now returns -ENODEV because the bus is not active yet.

Also the commit message is misleading. A bound SPI child device can already exist on an unprobed bus: the SPI uclass scans child nodes at bind time via .post_bind = dm_scan_fdt_dev, and child SPI plat is populated in spi_child_post_bind(). So this patch can hide a valid, already-bound slave device simply because the parent bus is still inactive.

It also does not fix the underlying issue generally. The real problem here is that spi_find_chip_select() may call ->cs_info() on an unprobed bus, but this patch only papers over one caller, spi_find_bus_and_cs(). Other paths such as spi_cs_info() / spi_cs_is_valid() still go through the same unprobed-bus flow.

If the goal is to address cs_info() relying on probe-time private data, I would expect the fix either in spi_find_chip_select() or in the driver-side cs_info() handling, not as a behavioral change in spi_find_bus_and_cs().

Move chip select validation from spi_find_chip_select() into
spi_child_pre_probe(), where the bus is guaranteed to be active and
driver private data is initialized. This avoids false CS rejections
when spi_find_chip_select() is called on an unprobed bus, and
simplifies the lookup function.

Signed-off-by: Jie Zhang <jie.zhang@analog.com>
@jiez
jiez force-pushed the 92-sf-probe-error-on-sc598-som-ezkit branch from 0abc067 to a3262e0 Compare May 8, 2026 03:41
@jiez

jiez commented May 8, 2026

Copy link
Copy Markdown
Author

spi_find_bus_and_cs() is documented as a lookup helper that finds the bus and slave without activating either side, but this change now returns -ENODEV because the bus is not active yet.

Also the commit message is kinda misleading. A bound SPI child device can already exist on an unprobed bus. The SPI uclass scans child nodes at bind time via .post_bind = dm_scan_fdt_dev, and child SPI plat is populated in spi_child_post_bind(). So this patch can hide a valid, already-bound slave device simply because the parent bus is still inactive.

It also does not fix the underlying issue generally. The real problem here is that spi_find_chip_select() may call ->cs_info() on an unprobed bus, but this patch only papers over one caller, spi_find_bus_and_cs(). Other paths such as spi_cs_info() / spi_cs_is_valid() still go through the same unprobed-bus flow.

If the goal is to address cs_info() relying on probe-time private data, I would expect the fix either in spi_find_chip_select() or in the driver-side cs_info() handling, not as a behavioral change in spi_find_bus_and_cs().

Thanks for your review!

I agree with your comments. I pushed a new commit, which moves cs validation from spi_find_chip_select() to spi_child_pre_probe(), i.e. move cs validation to probe phase. spi_find_chip_select() now does not validate cs.

@CalebEthridgeADI
CalebEthridgeADI self-requested a review May 8, 2026 12:30
@pamolloy pamolloy added this to ADSP Jun 9, 2026
@pamolloy

pamolloy commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

I haven't had a chance to read this in detail, but it looks like there are still issues. See review-inline.txt from Claude Sonnet 4.6 including Ozan's comment. Given that review and looking at other implementations in U-Boot and Linux Claude suggested the following:

  For adi_spi3.c:
  static int adi_spi_cs_info(struct udevice *bus, uint cs,
                             struct spi_cs_info *info)
  {
        // Hardware supports CS 1-7 (1-indexed)
        // This matches the default in DT: dev_read_u32_default(bus, "num-cs", 7)
        if (cs == 0 || cs > MAX_CTRL_CS) {
                return -EINVAL;
        }

        return 0;
  }

@pamolloy pamolloy moved this to In Development in ADSP Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Development

Development

Successfully merging this pull request may close these issues.

sf probe error on SC598-SOM-EZKIT

4 participants