From: "Junhui Liu" <junhui.liu@pigmoral.tech>
To: "Troy Mitchell" <troy.mitchell@linux.spacemit.com>,
"Junhui Liu" <junhui.liu@pigmoral.tech>
Cc: "Heiko Schocher" <hs@nabladev.com>, <u-boot-spacemit@groups.io>,
<u-boot@lists.u-boot-project.org>,
"Huan Zhou" <pericycle.cc@gmail.com>,
"Guodong Xu" <guodong.xu@oss.qualcomm.com>,
"Tom Rini" <trini@konsulko.com>,
"Raymond Mao" <raymond.mao@riscstar.com>,
"Lukasz Majewski" <lukma@denx.de>, "Yixun Lan" <dlan@kernel.org>
Subject: Re: [PATCH v2 1/4] i2c: k1: fix wrong bus speed setting
Date: Mon, 17 Aug 2026 22:25:30 +0800 [thread overview]
Message-ID: <DKRADSUN4GN9.299FCK1VIS6EH@pigmoral.tech> (raw)
In-Reply-To: <DKR3Q808Z4E0.27KFK7LNAM3K7@linux.spacemit.com>
Hi Troy,
Thanks for the review.
On Mon Aug 17, 2026 at 5:12 PM CST, Troy Mitchell wrote:
>> The controller bus mode should be selected according to the requested
>> I2C bus speed. However, the driver currently passes the functional clock
>> rate to k1_i2c_set_bus_speed(), so the selected mode does not reflect
>> the requested bus speed.
>>
>> Fix this by reading the clock-frequency property from the Device Tree,
>> defaulting to standard speed, and drop the unused clk_rate field.
>
> `clock-frequency` is the requested SCL rate, but this patch only changes the
> value passed to `k1_i2c_set_bus_speed()`.
>
>> @@ -496,10 +496,13 @@ static int k1_i2c_probe(struct udevice *bus)
>> debug("%s: failed to enable clock\n", __func__);
>> return ret;
>> }
>> - priv->clk_rate = clk_get_rate(&priv->clk);
>>
>> priv->base = (void *)devfdt_get_addr_ptr(bus);
>> - k1_i2c_set_bus_speed(bus, priv->clk_rate);
>> +
>> + speed = dev_read_u32_default(bus, "clock-frequency",
>> + I2C_SPEED_STANDARD_RATE);
>> + k1_i2c_set_bus_speed(bus, speed);
>
> `k1_i2c_set_bus_speed()` only changes `ICR_MODE_MASK`. It does not program
> the ILCR divider or initialize IWCR, so the actual SCL rate still depends on
> the register reset values and may not match the Device Tree.
Yes. This patch only aims to fix the ICR_MODE selection. On the K1 board
I tested, the register reset defaults are enough for both the PMIC and
the EEPROM to work under U-Boot.
>
> Please calculate and program ILCR from the functional clock rate and the
> requested SCL rate, initialize IWCR as required by the hardware, and reject
> unsupported rates. The functional clock handle or rate therefore needs to
> remain available to `k1_i2c_set_bus_speed()`.
That's a fair point. The current driver is aligned with the mainline
Linux K1 I2C driver. It does not include your later work to program
ILCR/IWCR:
https://lore.kernel.org/linux-riscv/20260508-k1-i2c-ilcr-v7-1-8c2dde5c3ed5@linux.spacemit.com/
Porting that change to U-Boot is not a small amount of work. I'd prefer
to keep that as a follow-up series rather than mix it into this ICR_MODE
fix.
>
> - Troy
--
Best regards,
Junhui Liu
next prev parent reply other threads:[~2026-08-17 14:26 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-15 3:40 [PATCH v2 0/4] i2c/clk: spacemit: k1: fix I2C bus configuration and clock handling Junhui Liu
2026-08-15 3:40 ` [PATCH v2 1/4] i2c: k1: fix wrong bus speed setting Junhui Liu
2026-08-17 8:29 ` Yixun Lan
2026-08-17 9:12 ` Troy Mitchell
2026-08-17 14:25 ` Junhui Liu [this message]
2026-08-18 1:03 ` Troy Mitchell
2026-08-15 3:40 ` [PATCH v2 2/4] clk: spacemit: k1: add TWSI bus clocks to SPL Junhui Liu
2026-08-15 3:40 ` [PATCH v2 3/4] i2c: k1: enable both functional and bus clocks Junhui Liu
2026-08-17 4:50 ` Heiko Schocher via U-Boot
2026-08-17 8:31 ` Yixun Lan
2026-08-17 8:56 ` Troy Mitchell
2026-08-17 9:18 ` Yixun Lan
2026-08-18 1:03 ` Troy Mitchell
2026-08-18 6:21 ` Yao Zi
2026-08-18 6:26 ` Yao Zi
2026-08-15 3:40 ` [PATCH v2 4/4] clk: spacemit: k1: remove bus gate from TWSI functional clocks Junhui Liu
2026-08-17 8:55 ` Troy Mitchell
2026-08-18 6:53 ` [PATCH v2 0/4] i2c/clk: spacemit: k1: fix I2C bus configuration and clock handling Yao Zi
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=DKRADSUN4GN9.299FCK1VIS6EH@pigmoral.tech \
--to=junhui.liu@pigmoral.tech \
--cc=dlan@kernel.org \
--cc=guodong.xu@oss.qualcomm.com \
--cc=hs@nabladev.com \
--cc=lukma@denx.de \
--cc=pericycle.cc@gmail.com \
--cc=raymond.mao@riscstar.com \
--cc=trini@konsulko.com \
--cc=troy.mitchell@linux.spacemit.com \
--cc=u-boot-spacemit@groups.io \
--cc=u-boot@lists.u-boot-project.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