From: Jens Kehne jens.kehne@agilent.com
[ Upstream commit 9ac4890ac39352ccea132109e32911495574c3ec ]
We observed the initial probe of the da9063 failing in da9063_get_device_type in about 30% of boots on a Xilinx ZynqMP based board. The problem originates in da9063_i2c_blockreg_read, which uses a single bus transaction to turn the register page and then read a register. On the bus, this should translate to a write to register 0, followed by a read to the target register, separated by a repeated start. However, we found that after the write to register 0, the controller sometimes continues directly with the register address of the read request, without sending the chip address or a repeated start in between, which makes the read request invalid.
To fix this, separate turning the page and reading the register into two separate transactions. This brings the initialization code in line with the rest of the driver, which uses register maps (which to my knowledge do not use repeated starts after turning the page). This has been included in our kernel for several months and was recently included in a shipped product. For us, it reliably fixes the issue, and we have not observed any new issues.
While the underlying problem is probably with the i2c controller or its driver, I still propose a change here in the interest of robustness: First, I'm not sure this issue can be fixed on the controller side, since there are other issues related to repeated start which can't (AR# 60695, AR# 61664). Second, similar problems might exist with other controllers.
Signed-off-by: Jens Kehne jens.kehne@agilent.com Link: https://lore.kernel.org/r/20250804133754.3496718-1-jens.kehne@agilent.com Signed-off-by: Lee Jones lee@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- What it fixes - Addresses intermittent probe failures (~30% boots) during early chip identification reads on some I2C controllers (e.g., ZynqMP) where a combined transaction with a repeated start is mishandled, causing the read to be invalid. This is a user‑visible reliability bug in device bring‑up, not a feature change.
- Precise change - Adds a separate 1‑message transfer for page selection, then a 2‑message transfer for register address + read: - New `da9063_page_sel_msgs` for the standalone page select transfer: drivers/mfd/da9063-i2c.c:40–43 - Adjusts `da9063_paged_read_msgs` to start at the register select phase: drivers/mfd/da9063-i2c.c:45–49 - First transaction (page switch): set up `xfer[DA9063_PAGE_SEL_MSG]` then call `i2c_transfer(..., DA9063_PAGE_SEL_CNT)` with robust error checks: drivers/mfd/da9063-i2c.c:71–86 - Second transaction (register select + read): set up `xfer[DA9063_PAGED_READ_MSG_REG_SEL]` and `xfer[DA9063_PAGED_READ_MSG_DATA]`, then `i2c_transfer(..., DA9063_PAGED_READ_MSG_CNT)`: drivers/mfd/da9063-i2c.c:88–111 - The only user of this helper is the early device/variant read in `da9063_get_device_type()`: drivers/mfd/da9063-i2c.c:125–131
- Why this is safer - Many I2C controllers have quirks with repeated starts across a write‑then‑read sequence targeting different internal device behaviors (here, page register write followed by a read). Splitting the sequence issues a STOP between page switch and read address selection, avoiding controller bugs while remaining compliant with the device’s register paging model. - Matches how the rest of the driver operates via regmap, which uses page windows and does not rely on a repeated start immediately after page switching (see `selector_reg = DA9063_REG_PAGE_CON` and range config): drivers/mfd/da9063-i2c.c:341–351
- Scope and risk - Change is small, self‑contained to one function used only during probe; no architectural changes. - Adds clearer error logging for page switch failures without altering normal data paths. - Potential concern (atomicity between page set and read) is minimal here: this path runs during probe, before other clients of the device exist; typical single‑master systems are unaffected. Even on multi‑master, the benefit of avoiding known controller bugs outweighs the negligible race risk at probe time.
- Backport considerations - Older stable trees (e.g., v6.10, v6.6, v6.1) still use the single `i2c_transfer` with 3 messages for this path; applying this patch is straightforward and does not depend on other recent infrastructure changes. The change only introduces a new small enum and splits the existing message sequence with added error checks. - No ABI changes, no feature additions, no cross‑subsystem dependencies.
- Stable policy fit - Fixes a real, observed boot‑time reliability bug. - Minimal, localized patch with low regression risk. - No new features or behavior changes beyond making the transaction sequence more robust. - Although there’s no explicit “Cc: stable”, the nature and scope make it an appropriate stable backport.
drivers/mfd/da9063-i2c.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/mfd/da9063-i2c.c b/drivers/mfd/da9063-i2c.c index c6235cd0dbdc4..1ec9ab56442df 100644 --- a/drivers/mfd/da9063-i2c.c +++ b/drivers/mfd/da9063-i2c.c @@ -37,9 +37,13 @@ enum da9063_page_sel_buf_fmt { DA9063_PAGE_SEL_BUF_SIZE, };
+enum da9063_page_sel_msgs { + DA9063_PAGE_SEL_MSG = 0, + DA9063_PAGE_SEL_CNT, +}; + enum da9063_paged_read_msgs { - DA9063_PAGED_READ_MSG_PAGE_SEL = 0, - DA9063_PAGED_READ_MSG_REG_SEL, + DA9063_PAGED_READ_MSG_REG_SEL = 0, DA9063_PAGED_READ_MSG_DATA, DA9063_PAGED_READ_MSG_CNT, }; @@ -65,10 +69,21 @@ static int da9063_i2c_blockreg_read(struct i2c_client *client, u16 addr, (page_num << DA9063_I2C_PAGE_SEL_SHIFT) & DA9063_REG_PAGE_MASK;
/* Write reg address, page selection */ - xfer[DA9063_PAGED_READ_MSG_PAGE_SEL].addr = client->addr; - xfer[DA9063_PAGED_READ_MSG_PAGE_SEL].flags = 0; - xfer[DA9063_PAGED_READ_MSG_PAGE_SEL].len = DA9063_PAGE_SEL_BUF_SIZE; - xfer[DA9063_PAGED_READ_MSG_PAGE_SEL].buf = page_sel_buf; + xfer[DA9063_PAGE_SEL_MSG].addr = client->addr; + xfer[DA9063_PAGE_SEL_MSG].flags = 0; + xfer[DA9063_PAGE_SEL_MSG].len = DA9063_PAGE_SEL_BUF_SIZE; + xfer[DA9063_PAGE_SEL_MSG].buf = page_sel_buf; + + ret = i2c_transfer(client->adapter, xfer, DA9063_PAGE_SEL_CNT); + if (ret < 0) { + dev_err(&client->dev, "Page switch failed: %d\n", ret); + return ret; + } + + if (ret != DA9063_PAGE_SEL_CNT) { + dev_err(&client->dev, "Page switch failed to complete\n"); + return -EIO; + }
/* Select register address */ xfer[DA9063_PAGED_READ_MSG_REG_SEL].addr = client->addr;