* [PATCH v4] i3c: master: dw-i3c-master: fix OD timing for first broadcast
@ 2026-07-31 4:23 ` tze.yee.ng
0 siblings, 0 replies; 5+ messages in thread
From: tze.yee.ng @ 2026-07-31 4:23 UTC (permalink / raw)
To: Alexandre Belloni, Frank Li, Adrian Ng Ho Yin, Felix Gu,
Adrian Hunter, Akhil R, Manikanta Guntupalli, Shubhrajyoti Datta,
linux-i3c, linux-kernel
From: Tze Yee Ng <tze.yee.ng@altera.com>
Implement ->set_speed() so the I3C core can switch open-drain timing for
the first broadcast address per spec: I3C_OPEN_DRAIN_SLOW_SPEED programs
tHIGH_INIT (200 ns) before RSTDAA, and I3C_OPEN_DRAIN_NORMAL_SPEED restores
normal OD timing afterward. Cache the normal OD register value during bus
init and use a separate od_hcnt for the slow path so SDR extended timing
remains derived from the normal PP hcnt.
For AMD_I3C_OD_PP_TIMING, cache AMD_I3C_OD_TIMING as the normal OD
baseline and stop rewriting OD timing in send_ccc_cmd()/runtime resume so
I3C_OPEN_DRAIN_SLOW_SPEED is preserved through RSTDAA.
Use PM_RUNTIME_ACQUIRE_AUTOSUSPEND() in set_speed(). Compute od_hcnt with
DIV_ROUND_UP_ULL() for 32-bit safety and clamp it to U8_MAX to match the
8-bit I3C_OD_HCNT field.
Fixes I2C devices with spike filters not being detected on mixed buses.
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
Changes in v4:
- Rebase on i3c/next.
- No functional changes.
Changes in v3:
- Use DIV_ROUND_UP_ULL(..., NSEC_PER_SEC) for od_hcnt to avoid
__udivdi3 linker failures on 32-bit platforms (Sashiko/Frank)
- Compute od_hcnt as u32 and clamp to U8_MAX to match the 8-bit
I3C_OD_HCNT field and avoid silent truncation (Sashiko/Frank)
Changes in v2:
- Recalculate od_hcnt with Zephyr-style formula (Sashiko)
- Fix AMD_I3C_OD_PP_TIMING interaction with set_speed() (Sashiko)
- Switch set_speed() to PM_RUNTIME_ACQUIRE_AUTOSUSPEND() (Frank)
---
drivers/i3c/master/dw-i3c-master.c | 82 +++++++++++++++++++++++-------
drivers/i3c/master/dw-i3c-master.h | 1 +
include/linux/i3c/master.h | 1 +
3 files changed, 67 insertions(+), 17 deletions(-)
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 3816a50a52cc..11ae517cf2e6 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -572,6 +572,13 @@ static unsigned long dw_i3c_master_get_core_rate(struct dw_i3c_master *master)
return core_rate_prop;
}
+static void amd_configure_od_pp_quirk(struct dw_i3c_master *master)
+{
+ master->i3c_od_timing = AMD_I3C_OD_TIMING;
+ master->i3c_od_timing_normal = AMD_I3C_OD_TIMING;
+ master->i3c_pp_timing = AMD_I3C_PP_TIMING;
+}
+
static int dw_i3c_clk_cfg(struct dw_i3c_master *master)
{
unsigned long core_rate, core_period;
@@ -610,6 +617,18 @@ static int dw_i3c_clk_cfg(struct dw_i3c_master *master)
scl_timing = SCL_I3C_TIMING_HCNT(hcnt) | SCL_I3C_TIMING_LCNT(lcnt);
writel(scl_timing, master->regs + SCL_I3C_OD_TIMING);
master->i3c_od_timing = scl_timing;
+ master->i3c_od_timing_normal = scl_timing;
+
+ /*
+ * AMD legacy platforms need fixed OD/PP timings. Cache them as the
+ * normal OD baseline so set_speed(NORMAL) restores AMD values, and
+ * set_speed(SLOW) can stretch HCNT while keeping the AMD LCNT.
+ */
+ if (master->quirks & AMD_I3C_OD_PP_TIMING) {
+ amd_configure_od_pp_quirk(master);
+ writel(master->i3c_pp_timing, master->regs + SCL_I3C_PP_TIMING);
+ writel(master->i3c_od_timing, master->regs + SCL_I3C_OD_TIMING);
+ }
lcnt = DIV_ROUND_UP(core_rate, I3C_BUS_SDR1_SCL_RATE) - hcnt;
scl_timing = SCL_EXT_LCNT_1(lcnt);
@@ -825,12 +844,6 @@ static int dw_i3c_ccc_get(struct dw_i3c_master *master, struct i3c_ccc_cmd *ccc)
return ret;
}
-static void amd_configure_od_pp_quirk(struct dw_i3c_master *master)
-{
- master->i3c_od_timing = AMD_I3C_OD_TIMING;
- master->i3c_pp_timing = AMD_I3C_PP_TIMING;
-}
-
static int dw_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
struct i3c_ccc_cmd *ccc)
{
@@ -840,13 +853,6 @@ static int dw_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
if (ccc->id == I3C_CCC_ENTDAA)
return -EINVAL;
- /* AMD platform specific OD and PP timings */
- if (master->quirks & AMD_I3C_OD_PP_TIMING) {
- amd_configure_od_pp_quirk(master);
- writel(master->i3c_pp_timing, master->regs + SCL_I3C_PP_TIMING);
- writel(master->i3c_od_timing, master->regs + SCL_I3C_OD_TIMING);
- }
-
ret = pm_runtime_resume_and_get(master->dev);
if (ret < 0) {
dev_err(master->dev,
@@ -1531,6 +1537,50 @@ static irqreturn_t dw_i3c_master_irq_handler(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static int dw_i3c_master_set_speed(struct i3c_master_controller *m,
+ enum i3c_open_drain_speed speed)
+{
+ struct dw_i3c_master *master = to_dw_i3c_master(m);
+ unsigned long core_rate;
+ u32 scl_timing, od_hcnt;
+ u8 lcnt;
+
+ PM_RUNTIME_ACQUIRE_AUTOSUSPEND(master->dev, pm);
+ if (PM_RUNTIME_ACQUIRE_ERR(&pm))
+ return -ENXIO;
+
+ switch (speed) {
+ case I3C_OPEN_DRAIN_SLOW_SPEED:
+ core_rate = clk_get_rate(master->core_clk);
+ if (!core_rate)
+ return -EINVAL;
+
+ lcnt = SCL_I3C_TIMING_LCNT(master->i3c_od_timing_normal);
+ od_hcnt = DIV_ROUND_UP_ULL((u64)I3C_BUS_THIGH_INIT_OD_MIN_NS *
+ core_rate, NSEC_PER_SEC) - 1;
+ if (od_hcnt < SCL_I3C_TIMING_CNT_MIN)
+ od_hcnt = SCL_I3C_TIMING_CNT_MIN;
+ else if (od_hcnt > U8_MAX)
+ od_hcnt = U8_MAX;
+ scl_timing = SCL_I3C_TIMING_HCNT(od_hcnt) |
+ SCL_I3C_TIMING_LCNT(lcnt);
+ writel(scl_timing, master->regs + SCL_I3C_OD_TIMING);
+ master->i3c_od_timing = scl_timing;
+ break;
+
+ case I3C_OPEN_DRAIN_NORMAL_SPEED:
+ writel(master->i3c_od_timing_normal,
+ master->regs + SCL_I3C_OD_TIMING);
+ master->i3c_od_timing = master->i3c_od_timing_normal;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int dw_i3c_master_set_dev_nack_retry(struct i3c_master_controller *m,
unsigned int dev_nack_retry_cnt)
{
@@ -1585,6 +1635,7 @@ static const struct i3c_master_controller_ops dw_mipi_i3c_ops = {
.recycle_ibi_slot = dw_i3c_master_recycle_ibi_slot,
.enable_hotjoin = dw_i3c_master_enable_hotjoin,
.disable_hotjoin = dw_i3c_master_disable_hotjoin,
+ .set_speed = dw_i3c_master_set_speed,
.set_dev_nack_retry = dw_i3c_master_set_dev_nack_retry,
};
@@ -1780,10 +1831,7 @@ static void dw_i3c_master_restore_addrs(struct dw_i3c_master *master)
static void dw_i3c_master_restore_timing_regs(struct dw_i3c_master *master)
{
- /* AMD platform specific OD and PP timings */
- if (master->quirks & AMD_I3C_OD_PP_TIMING)
- amd_configure_od_pp_quirk(master);
-
+ /* Preserve cached OD timing; it may be the SLOW setting from set_speed(). */
writel(master->i3c_pp_timing, master->regs + SCL_I3C_PP_TIMING);
writel(master->bus_free_timing, master->regs + BUS_FREE_TIMING);
writel(master->i3c_od_timing, master->regs + SCL_I3C_OD_TIMING);
diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h
index 28e9348f2153..17ad817d1f8e 100644
--- a/drivers/i3c/master/dw-i3c-master.h
+++ b/drivers/i3c/master/dw-i3c-master.h
@@ -46,6 +46,7 @@ struct dw_i3c_master {
u32 dev_addr;
u32 i3c_pp_timing;
u32 i3c_od_timing;
+ u32 i3c_od_timing_normal;
u32 ext_lcnt_timing;
u32 bus_free_timing;
u32 i2c_fm_timing;
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 2dc139a217bf..c3df1c187050 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -269,6 +269,7 @@ struct i3c_device {
#define I3C_BUS_THIGH_MIXED_MAX_NS 41
#define I3C_BUS_TIDLE_MIN_NS 200000
#define I3C_BUS_TLOW_OD_MIN_NS 200
+#define I3C_BUS_THIGH_INIT_OD_MIN_NS 200
/**
* enum i3c_bus_mode - I3C bus mode
--
2.43.7
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v4] i3c: master: dw-i3c-master: fix OD timing for first broadcast
@ 2026-07-31 4:23 ` tze.yee.ng
0 siblings, 0 replies; 5+ messages in thread
From: tze.yee.ng @ 2026-07-31 4:23 UTC (permalink / raw)
To: Alexandre Belloni, Frank Li, Adrian Ng Ho Yin, Felix Gu,
Adrian Hunter, Akhil R, Manikanta Guntupalli, Shubhrajyoti Datta,
linux-i3c, linux-kernel
From: Tze Yee Ng <tze.yee.ng@altera.com>
Implement ->set_speed() so the I3C core can switch open-drain timing for
the first broadcast address per spec: I3C_OPEN_DRAIN_SLOW_SPEED programs
tHIGH_INIT (200 ns) before RSTDAA, and I3C_OPEN_DRAIN_NORMAL_SPEED restores
normal OD timing afterward. Cache the normal OD register value during bus
init and use a separate od_hcnt for the slow path so SDR extended timing
remains derived from the normal PP hcnt.
For AMD_I3C_OD_PP_TIMING, cache AMD_I3C_OD_TIMING as the normal OD
baseline and stop rewriting OD timing in send_ccc_cmd()/runtime resume so
I3C_OPEN_DRAIN_SLOW_SPEED is preserved through RSTDAA.
Use PM_RUNTIME_ACQUIRE_AUTOSUSPEND() in set_speed(). Compute od_hcnt with
DIV_ROUND_UP_ULL() for 32-bit safety and clamp it to U8_MAX to match the
8-bit I3C_OD_HCNT field.
Fixes I2C devices with spike filters not being detected on mixed buses.
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
Changes in v4:
- Rebase on i3c/next.
- No functional changes.
Changes in v3:
- Use DIV_ROUND_UP_ULL(..., NSEC_PER_SEC) for od_hcnt to avoid
__udivdi3 linker failures on 32-bit platforms (Sashiko/Frank)
- Compute od_hcnt as u32 and clamp to U8_MAX to match the 8-bit
I3C_OD_HCNT field and avoid silent truncation (Sashiko/Frank)
Changes in v2:
- Recalculate od_hcnt with Zephyr-style formula (Sashiko)
- Fix AMD_I3C_OD_PP_TIMING interaction with set_speed() (Sashiko)
- Switch set_speed() to PM_RUNTIME_ACQUIRE_AUTOSUSPEND() (Frank)
---
drivers/i3c/master/dw-i3c-master.c | 82 +++++++++++++++++++++++-------
drivers/i3c/master/dw-i3c-master.h | 1 +
include/linux/i3c/master.h | 1 +
3 files changed, 67 insertions(+), 17 deletions(-)
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 3816a50a52cc..11ae517cf2e6 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -572,6 +572,13 @@ static unsigned long dw_i3c_master_get_core_rate(struct dw_i3c_master *master)
return core_rate_prop;
}
+static void amd_configure_od_pp_quirk(struct dw_i3c_master *master)
+{
+ master->i3c_od_timing = AMD_I3C_OD_TIMING;
+ master->i3c_od_timing_normal = AMD_I3C_OD_TIMING;
+ master->i3c_pp_timing = AMD_I3C_PP_TIMING;
+}
+
static int dw_i3c_clk_cfg(struct dw_i3c_master *master)
{
unsigned long core_rate, core_period;
@@ -610,6 +617,18 @@ static int dw_i3c_clk_cfg(struct dw_i3c_master *master)
scl_timing = SCL_I3C_TIMING_HCNT(hcnt) | SCL_I3C_TIMING_LCNT(lcnt);
writel(scl_timing, master->regs + SCL_I3C_OD_TIMING);
master->i3c_od_timing = scl_timing;
+ master->i3c_od_timing_normal = scl_timing;
+
+ /*
+ * AMD legacy platforms need fixed OD/PP timings. Cache them as the
+ * normal OD baseline so set_speed(NORMAL) restores AMD values, and
+ * set_speed(SLOW) can stretch HCNT while keeping the AMD LCNT.
+ */
+ if (master->quirks & AMD_I3C_OD_PP_TIMING) {
+ amd_configure_od_pp_quirk(master);
+ writel(master->i3c_pp_timing, master->regs + SCL_I3C_PP_TIMING);
+ writel(master->i3c_od_timing, master->regs + SCL_I3C_OD_TIMING);
+ }
lcnt = DIV_ROUND_UP(core_rate, I3C_BUS_SDR1_SCL_RATE) - hcnt;
scl_timing = SCL_EXT_LCNT_1(lcnt);
@@ -825,12 +844,6 @@ static int dw_i3c_ccc_get(struct dw_i3c_master *master, struct i3c_ccc_cmd *ccc)
return ret;
}
-static void amd_configure_od_pp_quirk(struct dw_i3c_master *master)
-{
- master->i3c_od_timing = AMD_I3C_OD_TIMING;
- master->i3c_pp_timing = AMD_I3C_PP_TIMING;
-}
-
static int dw_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
struct i3c_ccc_cmd *ccc)
{
@@ -840,13 +853,6 @@ static int dw_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
if (ccc->id == I3C_CCC_ENTDAA)
return -EINVAL;
- /* AMD platform specific OD and PP timings */
- if (master->quirks & AMD_I3C_OD_PP_TIMING) {
- amd_configure_od_pp_quirk(master);
- writel(master->i3c_pp_timing, master->regs + SCL_I3C_PP_TIMING);
- writel(master->i3c_od_timing, master->regs + SCL_I3C_OD_TIMING);
- }
-
ret = pm_runtime_resume_and_get(master->dev);
if (ret < 0) {
dev_err(master->dev,
@@ -1531,6 +1537,50 @@ static irqreturn_t dw_i3c_master_irq_handler(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static int dw_i3c_master_set_speed(struct i3c_master_controller *m,
+ enum i3c_open_drain_speed speed)
+{
+ struct dw_i3c_master *master = to_dw_i3c_master(m);
+ unsigned long core_rate;
+ u32 scl_timing, od_hcnt;
+ u8 lcnt;
+
+ PM_RUNTIME_ACQUIRE_AUTOSUSPEND(master->dev, pm);
+ if (PM_RUNTIME_ACQUIRE_ERR(&pm))
+ return -ENXIO;
+
+ switch (speed) {
+ case I3C_OPEN_DRAIN_SLOW_SPEED:
+ core_rate = clk_get_rate(master->core_clk);
+ if (!core_rate)
+ return -EINVAL;
+
+ lcnt = SCL_I3C_TIMING_LCNT(master->i3c_od_timing_normal);
+ od_hcnt = DIV_ROUND_UP_ULL((u64)I3C_BUS_THIGH_INIT_OD_MIN_NS *
+ core_rate, NSEC_PER_SEC) - 1;
+ if (od_hcnt < SCL_I3C_TIMING_CNT_MIN)
+ od_hcnt = SCL_I3C_TIMING_CNT_MIN;
+ else if (od_hcnt > U8_MAX)
+ od_hcnt = U8_MAX;
+ scl_timing = SCL_I3C_TIMING_HCNT(od_hcnt) |
+ SCL_I3C_TIMING_LCNT(lcnt);
+ writel(scl_timing, master->regs + SCL_I3C_OD_TIMING);
+ master->i3c_od_timing = scl_timing;
+ break;
+
+ case I3C_OPEN_DRAIN_NORMAL_SPEED:
+ writel(master->i3c_od_timing_normal,
+ master->regs + SCL_I3C_OD_TIMING);
+ master->i3c_od_timing = master->i3c_od_timing_normal;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int dw_i3c_master_set_dev_nack_retry(struct i3c_master_controller *m,
unsigned int dev_nack_retry_cnt)
{
@@ -1585,6 +1635,7 @@ static const struct i3c_master_controller_ops dw_mipi_i3c_ops = {
.recycle_ibi_slot = dw_i3c_master_recycle_ibi_slot,
.enable_hotjoin = dw_i3c_master_enable_hotjoin,
.disable_hotjoin = dw_i3c_master_disable_hotjoin,
+ .set_speed = dw_i3c_master_set_speed,
.set_dev_nack_retry = dw_i3c_master_set_dev_nack_retry,
};
@@ -1780,10 +1831,7 @@ static void dw_i3c_master_restore_addrs(struct dw_i3c_master *master)
static void dw_i3c_master_restore_timing_regs(struct dw_i3c_master *master)
{
- /* AMD platform specific OD and PP timings */
- if (master->quirks & AMD_I3C_OD_PP_TIMING)
- amd_configure_od_pp_quirk(master);
-
+ /* Preserve cached OD timing; it may be the SLOW setting from set_speed(). */
writel(master->i3c_pp_timing, master->regs + SCL_I3C_PP_TIMING);
writel(master->bus_free_timing, master->regs + BUS_FREE_TIMING);
writel(master->i3c_od_timing, master->regs + SCL_I3C_OD_TIMING);
diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h
index 28e9348f2153..17ad817d1f8e 100644
--- a/drivers/i3c/master/dw-i3c-master.h
+++ b/drivers/i3c/master/dw-i3c-master.h
@@ -46,6 +46,7 @@ struct dw_i3c_master {
u32 dev_addr;
u32 i3c_pp_timing;
u32 i3c_od_timing;
+ u32 i3c_od_timing_normal;
u32 ext_lcnt_timing;
u32 bus_free_timing;
u32 i2c_fm_timing;
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 2dc139a217bf..c3df1c187050 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -269,6 +269,7 @@ struct i3c_device {
#define I3C_BUS_THIGH_MIXED_MAX_NS 41
#define I3C_BUS_TIDLE_MIN_NS 200000
#define I3C_BUS_TLOW_OD_MIN_NS 200
+#define I3C_BUS_THIGH_INIT_OD_MIN_NS 200
/**
* enum i3c_bus_mode - I3C bus mode
--
2.43.7
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v4] i3c: master: dw-i3c-master: fix OD timing for first broadcast
2026-07-31 4:23 ` tze.yee.ng
(?)
@ 2026-07-31 4:35 ` sashiko-bot
2026-07-31 6:03 ` NG, TZE YEE
-1 siblings, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-07-31 4:35 UTC (permalink / raw)
To: tze.yee.ng; +Cc: Frank.Li, Alexandre Belloni, linux-i3c
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Initialization failure on ACPI/DT platforms due to missing core clock fallback in dw_i3c_master_set_speed().
--
--- Patch [1]: [PATCH v4] i3c: master: dw-i3c-master: fix OD timing for first broadcast ---
commit c4596518222ed00cb6c15e2963a846694b7f5d55
Author: Tze Yee Ng <tze.yee.ng@altera.com>
i3c: master: dw-i3c-master: fix OD timing for first broadcast
This commit implements the ->set_speed() callback to allow the I3C core to
switch open-drain timings for the first broadcast address. It caches the
normal OD register value during bus initialization and computes a separate
od_hcnt for the slow path.
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 3816a50a52cc5..11ae517cf2e60 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
[ ... ]
> @@ -1531,6 +1537,50 @@ static irqreturn_t dw_i3c_master_irq_handler(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static int dw_i3c_master_set_speed(struct i3c_master_controller *m,
> + enum i3c_open_drain_speed speed)
> +{
> + struct dw_i3c_master *master = to_dw_i3c_master(m);
> + unsigned long core_rate;
> + u32 scl_timing, od_hcnt;
> + u8 lcnt;
> +
> + PM_RUNTIME_ACQUIRE_AUTOSUSPEND(master->dev, pm);
> + if (PM_RUNTIME_ACQUIRE_ERR(&pm))
> + return -ENXIO;
> +
> + switch (speed) {
> + case I3C_OPEN_DRAIN_SLOW_SPEED:
> + core_rate = clk_get_rate(master->core_clk);
> + if (!core_rate)
> + return -EINVAL;
[Severity: High]
Will this cause platforms without a core clock provider to fail here?
The rest of the driver uses dw_i3c_master_get_core_rate() to fall back to
the "clock-frequency" device property if master->core_clk is NULL (such as
on ACPI/DT platforms). If clk_get_rate() is called directly here on those
platforms, it appears core_rate will be 0 and the callback will abort with
-EINVAL.
> +
> + lcnt = SCL_I3C_TIMING_LCNT(master->i3c_od_timing_normal);
> + od_hcnt = DIV_ROUND_UP_ULL((u64)I3C_BUS_THIGH_INIT_OD_MIN_NS *
> + core_rate, NSEC_PER_SEC) - 1;
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/c2d80c4fabd36aa00dc1fc507ff20211bba10c3f.1785471627.git.tze.yee.ng@altera.com?part=1
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v4] i3c: master: dw-i3c-master: fix OD timing for first broadcast
2026-07-31 4:35 ` sashiko-bot
@ 2026-07-31 6:03 ` NG, TZE YEE
0 siblings, 0 replies; 5+ messages in thread
From: NG, TZE YEE @ 2026-07-31 6:03 UTC (permalink / raw)
To: Alexandre Belloni, Frank Li, NG, ADRIAN HO YIN, Felix Gu,
Adrian Hunter, Akhil R, Manikanta Guntupalli, Shubhrajyoti Datta,
linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org
On 31/7/2026 12:35 pm, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Initialization failure on ACPI/DT platforms due to missing core clock fallback in dw_i3c_master_set_speed().
> --
>
> --- Patch [1]: [PATCH v4] i3c: master: dw-i3c-master: fix OD timing for first broadcast ---
> commit c4596518222ed00cb6c15e2963a846694b7f5d55
> Author: Tze Yee Ng <tze.yee.ng@altera.com>
>
> i3c: master: dw-i3c-master: fix OD timing for first broadcast
>
> This commit implements the ->set_speed() callback to allow the I3C core to
> switch open-drain timings for the first broadcast address. It caches the
> normal OD register value during bus initialization and computes a separate
> od_hcnt for the slow path.
>
>> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
>> index 3816a50a52cc5..11ae517cf2e60 100644
>> --- a/drivers/i3c/master/dw-i3c-master.c
>> +++ b/drivers/i3c/master/dw-i3c-master.c
>
> [ ... ]
>
>> @@ -1531,6 +1537,50 @@ static irqreturn_t dw_i3c_master_irq_handler(int irq, void *dev_id)
>> return IRQ_HANDLED;
>> }
>>
>> +static int dw_i3c_master_set_speed(struct i3c_master_controller *m,
>> + enum i3c_open_drain_speed speed)
>> +{
>> + struct dw_i3c_master *master = to_dw_i3c_master(m);
>> + unsigned long core_rate;
>> + u32 scl_timing, od_hcnt;
>> + u8 lcnt;
>> +
>> + PM_RUNTIME_ACQUIRE_AUTOSUSPEND(master->dev, pm);
>> + if (PM_RUNTIME_ACQUIRE_ERR(&pm))
>> + return -ENXIO;
>> +
>> + switch (speed) {
>> + case I3C_OPEN_DRAIN_SLOW_SPEED:
>> + core_rate = clk_get_rate(master->core_clk);
>> + if (!core_rate)
>> + return -EINVAL;
>
> [Severity: High]
> Will this cause platforms without a core clock provider to fail here?
>
> The rest of the driver uses dw_i3c_master_get_core_rate() to fall back to
> the "clock-frequency" device property if master->core_clk is NULL (such as
> on ACPI/DT platforms). If clk_get_rate() is called directly here on those
> platforms, it appears core_rate will be 0 and the callback will abort with
> -EINVAL.
>
Agreed. I will switch set_speed() to use dw_i3c_master_get_core_rate()
like the rest of the driver.
Thanks,
Tze Yee
>> +
>> + lcnt = SCL_I3C_TIMING_LCNT(master->i3c_od_timing_normal);
>> + od_hcnt = DIV_ROUND_UP_ULL((u64)I3C_BUS_THIGH_INIT_OD_MIN_NS *
>> + core_rate, NSEC_PER_SEC) - 1;
>
> [ ... ]
>
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v4] i3c: master: dw-i3c-master: fix OD timing for first broadcast
@ 2026-07-31 6:03 ` NG, TZE YEE
0 siblings, 0 replies; 5+ messages in thread
From: NG, TZE YEE @ 2026-07-31 6:03 UTC (permalink / raw)
To: Alexandre Belloni, Frank Li, NG, ADRIAN HO YIN, Felix Gu,
Adrian Hunter, Akhil R, Manikanta Guntupalli, Shubhrajyoti Datta,
linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org
On 31/7/2026 12:35 pm, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Initialization failure on ACPI/DT platforms due to missing core clock fallback in dw_i3c_master_set_speed().
> --
>
> --- Patch [1]: [PATCH v4] i3c: master: dw-i3c-master: fix OD timing for first broadcast ---
> commit c4596518222ed00cb6c15e2963a846694b7f5d55
> Author: Tze Yee Ng <tze.yee.ng@altera.com>
>
> i3c: master: dw-i3c-master: fix OD timing for first broadcast
>
> This commit implements the ->set_speed() callback to allow the I3C core to
> switch open-drain timings for the first broadcast address. It caches the
> normal OD register value during bus initialization and computes a separate
> od_hcnt for the slow path.
>
>> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
>> index 3816a50a52cc5..11ae517cf2e60 100644
>> --- a/drivers/i3c/master/dw-i3c-master.c
>> +++ b/drivers/i3c/master/dw-i3c-master.c
>
> [ ... ]
>
>> @@ -1531,6 +1537,50 @@ static irqreturn_t dw_i3c_master_irq_handler(int irq, void *dev_id)
>> return IRQ_HANDLED;
>> }
>>
>> +static int dw_i3c_master_set_speed(struct i3c_master_controller *m,
>> + enum i3c_open_drain_speed speed)
>> +{
>> + struct dw_i3c_master *master = to_dw_i3c_master(m);
>> + unsigned long core_rate;
>> + u32 scl_timing, od_hcnt;
>> + u8 lcnt;
>> +
>> + PM_RUNTIME_ACQUIRE_AUTOSUSPEND(master->dev, pm);
>> + if (PM_RUNTIME_ACQUIRE_ERR(&pm))
>> + return -ENXIO;
>> +
>> + switch (speed) {
>> + case I3C_OPEN_DRAIN_SLOW_SPEED:
>> + core_rate = clk_get_rate(master->core_clk);
>> + if (!core_rate)
>> + return -EINVAL;
>
> [Severity: High]
> Will this cause platforms without a core clock provider to fail here?
>
> The rest of the driver uses dw_i3c_master_get_core_rate() to fall back to
> the "clock-frequency" device property if master->core_clk is NULL (such as
> on ACPI/DT platforms). If clk_get_rate() is called directly here on those
> platforms, it appears core_rate will be 0 and the callback will abort with
> -EINVAL.
>
Agreed. I will switch set_speed() to use dw_i3c_master_get_core_rate()
like the rest of the driver.
Thanks,
Tze Yee
>> +
>> + lcnt = SCL_I3C_TIMING_LCNT(master->i3c_od_timing_normal);
>> + od_hcnt = DIV_ROUND_UP_ULL((u64)I3C_BUS_THIGH_INIT_OD_MIN_NS *
>> + core_rate, NSEC_PER_SEC) - 1;
>
> [ ... ]
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-31 6:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-31 4:23 [PATCH v4] i3c: master: dw-i3c-master: fix OD timing for first broadcast tze.yee.ng
2026-07-31 4:23 ` tze.yee.ng
2026-07-31 4:35 ` sashiko-bot
2026-07-31 6:03 ` NG, TZE YEE
2026-07-31 6:03 ` NG, TZE YEE
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.