[PATCH v4] i2c: spacemit: configure ILCR for accurate SCL frequency

Troy Mitchell posted 1 patch 2 months ago
drivers/i2c/busses/Kconfig  |   2 +-
drivers/i2c/busses/i2c-k1.c | 159 ++++++++++++++++++++++++++++++++++++++++----
2 files changed, 146 insertions(+), 15 deletions(-)
[PATCH v4] i2c: spacemit: configure ILCR for accurate SCL frequency
Posted by Troy Mitchell 2 months ago
The SpacemiT I2C controller's SCL (Serial Clock Line) frequency for
master mode operations is determined by the ILCR (I2C Load Count Register).
Previously, the driver relied on the hardware's reset default
values for this register.

The hardware's default ILCR values (SLV=0x156, FLV=0x5d) yield SCL
frequencies lower than intended. For example, with the default
31.5 MHz input clock, these default settings result in an SCL
frequency of approximately 93 kHz (standard mode) when targeting 100 kHz,
and approximately 338 kHz (fast mode) when targeting 400 kHz.
These frequencies are below the 100 kHz/400 kHz nominal speeds.

This patch integrates the SCL frequency management into
the Common Clock Framework (CCF). Specifically, the ILCR register,
which acts as a frequency divider for the SCL clock, is now registered
as a managed clock (scl_clk) within the CCF.

This patch also cleans up unnecessary whitespace
in the included header files.

Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
Changelog in v4:
- initialize clk_init_data with {} so that init.flags is implicitly set to 0
- minor cleanup and style fixes for better readability
- remove unused spacemit_i2c_scl_clk_exclusive_put() cleanup callback
- replace clk_set_rate_exclusive()/clk_rate_exclusive_put() pair with clk_set_rate()
- simplify LCR LV field macros by using FIELD_GET/FIELD_MAX helpers
- Link to v3: https://lore.kernel.org/all/20250814-k1-i2c-ilcr-v3-1-317723e74bcd@linux.spacemit.com/

Changelog in v3:
- use MASK macro in `recalc_rate` function
- rename clock name
- Link to v2: https://lore.kernel.org/r/20250718-k1-i2c-ilcr-v2-1-b4c68f13dcb1@linux.spacemit.com

Changelog in v2:
- Align line breaks.
- Check `lv` in `clk_set_rate` function.
- Force fast mode when SCL frequency is illegal or unavailable.
- Change "linux/bits.h" to <linux/bits.h>
- Kconfig: Add dependency on CCF.
- Link to v1: https://lore.kernel.org/all/20250710-k1-i2c-ilcr-v1-1-188d1f460c7d@linux.spacemit.com/
---
 drivers/i2c/busses/Kconfig  |   2 +-
 drivers/i2c/busses/i2c-k1.c | 159 ++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 146 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index fd81e49638aaa161ae264a722e9e06adc7914cda..fedf5d31f9035b73a27a7f8a764bf5c26975d0e1 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -798,7 +798,7 @@ config I2C_JZ4780
 config I2C_K1
 	tristate "SpacemiT K1 I2C adapter"
 	depends on ARCH_SPACEMIT || COMPILE_TEST
-	depends on OF
+	depends on OF && COMMON_CLK
 	help
 	  This option enables support for the I2C interface on the SpacemiT K1
 	  platform.
diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
index 6b918770e612e098b8ad17418f420d87c94df166..e38a0ba71734ca602854c85672dcb61423453515 100644
--- a/drivers/i2c/busses/i2c-k1.c
+++ b/drivers/i2c/busses/i2c-k1.c
@@ -4,18 +4,21 @@
  */
 
 #include <linux/bitfield.h>
- #include <linux/clk.h>
- #include <linux/i2c.h>
- #include <linux/iopoll.h>
- #include <linux/module.h>
- #include <linux/of_address.h>
- #include <linux/platform_device.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/i2c.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
 
 /* spacemit i2c registers */
 #define SPACEMIT_ICR		 0x0		/* Control register */
 #define SPACEMIT_ISR		 0x4		/* Status register */
 #define SPACEMIT_IDBR		 0xc		/* Data buffer register */
 #define SPACEMIT_IRCR		 0x18		/* Reset cycle counter */
+#define SPACEMIT_ILCR		 0x10		/* Load Count Register */
 #define SPACEMIT_IBMR		 0x1c		/* Bus monitor register */
 
 /* SPACEMIT_ICR register fields */
@@ -87,6 +90,13 @@
 #define SPACEMIT_BMR_SDA         BIT(0)		/* SDA line level */
 #define SPACEMIT_BMR_SCL         BIT(1)		/* SCL line level */
 
+#define SPACEMIT_LCR_LV_STANDARD_SHIFT		0
+#define SPACEMIT_LCR_LV_FAST_SHIFT		9
+#define SPACEMIT_LCR_LV_STANDARD_MASK		GENMASK(8, 0)
+#define SPACEMIT_LCR_LV_FAST_MASK		GENMASK(17, 9)
+#define SPACEMIT_LCR_LV_STANDARD_MAX_VALUE	FIELD_MAX(SPACEMIT_LCR_LV_STANDARD_MASK)
+#define SPACEMIT_LCR_LV_FAST_MAX_VALUE		FIELD_MAX(SPACEMIT_LCR_LV_FAST_MASK)
+
 /* i2c bus recover timeout: us */
 #define SPACEMIT_I2C_BUS_BUSY_TIMEOUT		100000
 
@@ -104,11 +114,20 @@ enum spacemit_i2c_state {
 	SPACEMIT_STATE_WRITE,
 };
 
+enum spacemit_i2c_mode {
+	SPACEMIT_MODE_STANDARD,
+	SPACEMIT_MODE_FAST
+};
+
 /* i2c-spacemit driver's main struct */
 struct spacemit_i2c_dev {
 	struct device *dev;
 	struct i2c_adapter adapt;
 
+	struct clk_hw scl_clk_hw;
+	struct clk *scl_clk;
+	enum spacemit_i2c_mode mode;
+
 	/* hardware resources */
 	void __iomem *base;
 	int irq;
@@ -129,6 +148,79 @@ struct spacemit_i2c_dev {
 	u32 status;
 };
 
+static void spacemit_i2c_scl_clk_disable_unprepare(void *data)
+{
+	struct spacemit_i2c_dev *i2c = data;
+
+	clk_disable_unprepare(i2c->scl_clk);
+}
+
+static int spacemit_i2c_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+				     unsigned long parent_rate)
+{
+	struct spacemit_i2c_dev *i2c = container_of(hw, struct spacemit_i2c_dev, scl_clk_hw);
+	u32 lv, lcr, mask, shift, max_lv;
+
+	lv = DIV_ROUND_UP(parent_rate, rate);
+
+	if (i2c->mode == SPACEMIT_MODE_STANDARD) {
+		mask = SPACEMIT_LCR_LV_STANDARD_MASK;
+		shift = SPACEMIT_LCR_LV_STANDARD_SHIFT;
+		max_lv = SPACEMIT_LCR_LV_STANDARD_MAX_VALUE;
+	} else if (i2c->mode == SPACEMIT_MODE_FAST) {
+		mask = SPACEMIT_LCR_LV_FAST_MASK;
+		shift = SPACEMIT_LCR_LV_FAST_SHIFT;
+		max_lv = SPACEMIT_LCR_LV_FAST_MAX_VALUE;
+	}
+
+	if (!lv || lv > max_lv) {
+		dev_err(i2c->dev, "set scl clock failed: lv 0x%x", lv);
+		return -EINVAL;
+	}
+
+	lcr = readl(i2c->base + SPACEMIT_ILCR);
+	lcr &= ~mask;
+	lcr |= lv << shift;
+	writel(lcr, i2c->base + SPACEMIT_ILCR);
+
+	return 0;
+}
+
+static long spacemit_i2c_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+					unsigned long *parent_rate)
+{
+	u32 lv, freq;
+
+	lv = DIV_ROUND_UP(*parent_rate, rate);
+	freq = DIV_ROUND_UP(*parent_rate, lv);
+
+	return freq;
+}
+
+static unsigned long spacemit_i2c_clk_recalc_rate(struct clk_hw *hw,
+						  unsigned long parent_rate)
+{
+	struct spacemit_i2c_dev *i2c = container_of(hw, struct spacemit_i2c_dev, scl_clk_hw);
+	u32 lcr, lv = 0;
+
+	lcr = readl(i2c->base + SPACEMIT_ILCR);
+
+	if (i2c->mode == SPACEMIT_MODE_STANDARD)
+		lv = FIELD_GET(SPACEMIT_LCR_LV_STANDARD_MASK, lcr);
+	else if (i2c->mode == SPACEMIT_MODE_FAST)
+		lv = FIELD_GET(SPACEMIT_LCR_LV_FAST_MASK, lcr);
+	else
+		return 0;
+
+	return DIV_ROUND_UP(parent_rate, lv);
+}
+
+static const struct clk_ops spacemit_i2c_clk_ops = {
+	.set_rate = spacemit_i2c_clk_set_rate,
+	.round_rate = spacemit_i2c_clk_round_rate,
+	.recalc_rate = spacemit_i2c_clk_recalc_rate,
+};
+
 static void spacemit_i2c_enable(struct spacemit_i2c_dev *i2c)
 {
 	u32 val;
@@ -147,6 +239,26 @@ static void spacemit_i2c_disable(struct spacemit_i2c_dev *i2c)
 	writel(val, i2c->base + SPACEMIT_ICR);
 }
 
+static struct clk *spacemit_i2c_register_scl_clk(struct spacemit_i2c_dev *i2c,
+						 struct clk *parent)
+{
+	struct clk_init_data init = {};
+	char name[32];
+
+	snprintf(name, sizeof(name), "%s_scl_clk", dev_name(i2c->dev));
+
+	init.name = name;
+	init.ops = &spacemit_i2c_clk_ops;
+	init.parent_data = (struct clk_parent_data[]) {
+		{ .fw_name = "func" },
+	};
+	init.num_parents = 1;
+
+	i2c->scl_clk_hw.init = &init;
+
+	return devm_clk_register(i2c->dev, &i2c->scl_clk_hw);
+}
+
 static void spacemit_i2c_reset(struct spacemit_i2c_dev *i2c)
 {
 	writel(SPACEMIT_CR_UR, i2c->base + SPACEMIT_ICR);
@@ -246,7 +358,7 @@ static void spacemit_i2c_init(struct spacemit_i2c_dev *i2c)
 	 */
 	val |= SPACEMIT_CR_DRFIE;
 
-	if (i2c->clock_freq == SPACEMIT_I2C_MAX_FAST_MODE_FREQ)
+	if (i2c->mode == SPACEMIT_MODE_FAST)
 		val |= SPACEMIT_CR_MODE_FAST;
 
 	/* disable response to general call */
@@ -538,14 +650,15 @@ static int spacemit_i2c_probe(struct platform_device *pdev)
 		dev_warn(dev, "failed to read clock-frequency property: %d\n", ret);
 
 	/* For now, this driver doesn't support high-speed. */
-	if (!i2c->clock_freq || i2c->clock_freq > SPACEMIT_I2C_MAX_FAST_MODE_FREQ) {
-		dev_warn(dev, "unsupported clock frequency %u; using %u\n",
-			 i2c->clock_freq, SPACEMIT_I2C_MAX_FAST_MODE_FREQ);
+	if (i2c->clock_freq > SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ &&
+	    i2c->clock_freq <= SPACEMIT_I2C_MAX_FAST_MODE_FREQ) {
+		i2c->mode = SPACEMIT_MODE_FAST;
+	} else if (i2c->clock_freq && i2c->clock_freq <= SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ) {
+		i2c->mode = SPACEMIT_MODE_STANDARD;
+	} else {
+		dev_warn(i2c->dev, "invalid clock-frequency, fallback to fast mode");
+		i2c->mode = SPACEMIT_MODE_FAST;
 		i2c->clock_freq = SPACEMIT_I2C_MAX_FAST_MODE_FREQ;
-	} else if (i2c->clock_freq < SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ) {
-		dev_warn(dev, "unsupported clock frequency %u; using %u\n",
-			 i2c->clock_freq,  SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ);
-		i2c->clock_freq = SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ;
 	}
 
 	i2c->dev = &pdev->dev;
@@ -567,10 +680,28 @@ static int spacemit_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(clk))
 		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable func clock");
 
+	i2c->scl_clk = spacemit_i2c_register_scl_clk(i2c, clk);
+	if (IS_ERR(i2c->scl_clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(i2c->scl_clk),
+				     "failed to register scl clock\n");
+
 	clk = devm_clk_get_enabled(dev, "bus");
 	if (IS_ERR(clk))
 		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock");
 
+	ret = clk_set_rate(i2c->scl_clk, i2c->clock_freq);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to set rate for SCL clock");
+
+	ret = clk_prepare_enable(i2c->scl_clk);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to prepare and enable clock");
+
+	ret = devm_add_action_or_reset(dev, spacemit_i2c_scl_clk_disable_unprepare, i2c);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "failed to register cleanup action for clk disable and unprepare");
+
 	spacemit_i2c_reset(i2c);
 
 	i2c_set_adapdata(&i2c->adapt, i2c);

---
base-commit: 316cb41c2400fbc0584aca75be78556914b66272
change-id: 20251017-k1-i2c-ilcr-c928d50e407f

Best regards,
-- 
Troy Mitchell <troy.mitchell@linux.spacemit.com>
Re: [PATCH v4] i2c: spacemit: configure ILCR for accurate SCL frequency
Posted by Andi Shyti 1 month, 3 weeks ago
Hi Troy,

On Fri, Oct 17, 2025 at 03:27:39PM +0800, Troy Mitchell wrote:
> The SpacemiT I2C controller's SCL (Serial Clock Line) frequency for
> master mode operations is determined by the ILCR (I2C Load Count Register).
> Previously, the driver relied on the hardware's reset default
> values for this register.
> 
> The hardware's default ILCR values (SLV=0x156, FLV=0x5d) yield SCL
> frequencies lower than intended. For example, with the default
> 31.5 MHz input clock, these default settings result in an SCL
> frequency of approximately 93 kHz (standard mode) when targeting 100 kHz,
> and approximately 338 kHz (fast mode) when targeting 400 kHz.
> These frequencies are below the 100 kHz/400 kHz nominal speeds.
> 
> This patch integrates the SCL frequency management into
> the Common Clock Framework (CCF). Specifically, the ILCR register,
> which acts as a frequency divider for the SCL clock, is now registered
> as a managed clock (scl_clk) within the CCF.
> 
> This patch also cleans up unnecessary whitespace
> in the included header files.
> 
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>

merged to i2c/i2c-host.

Thanks Yixun for your reviews here.

Andi
Re: [PATCH v4] i2c: spacemit: configure ILCR for accurate SCL frequency
Posted by Andi Shyti 1 month, 3 weeks ago
Hi Troy,

On Mon, Oct 20, 2025 at 11:28:45AM +0200, Andi Shyti wrote:
> On Fri, Oct 17, 2025 at 03:27:39PM +0800, Troy Mitchell wrote:
> > The SpacemiT I2C controller's SCL (Serial Clock Line) frequency for
> > master mode operations is determined by the ILCR (I2C Load Count Register).
> > Previously, the driver relied on the hardware's reset default
> > values for this register.
> > 
> > The hardware's default ILCR values (SLV=0x156, FLV=0x5d) yield SCL
> > frequencies lower than intended. For example, with the default
> > 31.5 MHz input clock, these default settings result in an SCL
> > frequency of approximately 93 kHz (standard mode) when targeting 100 kHz,
> > and approximately 338 kHz (fast mode) when targeting 400 kHz.
> > These frequencies are below the 100 kHz/400 kHz nominal speeds.
> > 
> > This patch integrates the SCL frequency management into
> > the Common Clock Framework (CCF). Specifically, the ILCR register,
> > which acts as a frequency divider for the SCL clock, is now registered
> > as a managed clock (scl_clk) within the CCF.
> > 
> > This patch also cleans up unnecessary whitespace
> > in the included header files.
> > 
> > Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> 
> merged to i2c/i2c-host.

I'm sorry, because of the report from LKP(*) I reverted your
patch. I2C_K1 is selected by MFD_SPACEMIT_P1, so that we get the
following warning:

  WARNING: unmet direct dependencies detected for I2C_K1

and compile I2C_K1 without COMMON_CLK. Please, fix it and
resubmit the patch.

Thanks,
Andi

(*) https://lore.kernel.org/oe-kbuild-all/202510202150.2qXd8e7Y-lkp@intel.com/
Re: [PATCH v4] i2c: spacemit: configure ILCR for accurate SCL frequency
Posted by Troy Mitchell 1 month, 3 weeks ago
Hi Andi,
On Tue, Oct 21, 2025 at 06:03:34PM +0200, Andi Shyti wrote:
> Hi Troy,
> 
> On Mon, Oct 20, 2025 at 11:28:45AM +0200, Andi Shyti wrote:
> > On Fri, Oct 17, 2025 at 03:27:39PM +0800, Troy Mitchell wrote:
> > > The SpacemiT I2C controller's SCL (Serial Clock Line) frequency for
> > > master mode operations is determined by the ILCR (I2C Load Count Register).
> > > Previously, the driver relied on the hardware's reset default
> > > values for this register.
> > > 
> > > The hardware's default ILCR values (SLV=0x156, FLV=0x5d) yield SCL
> > > frequencies lower than intended. For example, with the default
> > > 31.5 MHz input clock, these default settings result in an SCL
> > > frequency of approximately 93 kHz (standard mode) when targeting 100 kHz,
> > > and approximately 338 kHz (fast mode) when targeting 400 kHz.
> > > These frequencies are below the 100 kHz/400 kHz nominal speeds.
> > > 
> > > This patch integrates the SCL frequency management into
> > > the Common Clock Framework (CCF). Specifically, the ILCR register,
> > > which acts as a frequency divider for the SCL clock, is now registered
> > > as a managed clock (scl_clk) within the CCF.
> > > 
> > > This patch also cleans up unnecessary whitespace
> > > in the included header files.
> > > 
> > > Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> > 
> > merged to i2c/i2c-host.
> 
> I'm sorry, because of the report from LKP(*) I reverted your
> patch. I2C_K1 is selected by MFD_SPACEMIT_P1, so that we get the
> following warning:
> 
>   WARNING: unmet direct dependencies detected for I2C_K1
> 
> and compile I2C_K1 without COMMON_CLK. Please, fix it and
> resubmit the patch.
Yes, I noticed that too. The issue is introduced by PMIC driver(P1)
instead of I2C.
Should I resubmit this patch right after sending the fix,
or wait until the fix gets merged first?

                                - Troy
> 
> Thanks,
> Andi
> 
> (*) https://lore.kernel.org/oe-kbuild-all/202510202150.2qXd8e7Y-lkp@intel.com/
>
Re: [PATCH v4] i2c: spacemit: configure ILCR for accurate SCL frequency
Posted by Andi Shyti 1 month, 3 weeks ago
Hi Troy,

On Wed, Oct 22, 2025 at 09:48:04AM +0800, Troy Mitchell wrote:
> On Tue, Oct 21, 2025 at 06:03:34PM +0200, Andi Shyti wrote:
> > On Mon, Oct 20, 2025 at 11:28:45AM +0200, Andi Shyti wrote:
> > > On Fri, Oct 17, 2025 at 03:27:39PM +0800, Troy Mitchell wrote:
> > > > The SpacemiT I2C controller's SCL (Serial Clock Line) frequency for
> > > > master mode operations is determined by the ILCR (I2C Load Count Register).
> > > > Previously, the driver relied on the hardware's reset default
> > > > values for this register.
> > > > 
> > > > The hardware's default ILCR values (SLV=0x156, FLV=0x5d) yield SCL
> > > > frequencies lower than intended. For example, with the default
> > > > 31.5 MHz input clock, these default settings result in an SCL
> > > > frequency of approximately 93 kHz (standard mode) when targeting 100 kHz,
> > > > and approximately 338 kHz (fast mode) when targeting 400 kHz.
> > > > These frequencies are below the 100 kHz/400 kHz nominal speeds.
> > > > 
> > > > This patch integrates the SCL frequency management into
> > > > the Common Clock Framework (CCF). Specifically, the ILCR register,
> > > > which acts as a frequency divider for the SCL clock, is now registered
> > > > as a managed clock (scl_clk) within the CCF.
> > > > 
> > > > This patch also cleans up unnecessary whitespace
> > > > in the included header files.
> > > > 
> > > > Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> > > 
> > > merged to i2c/i2c-host.
> > 
> > I'm sorry, because of the report from LKP(*) I reverted your
> > patch. I2C_K1 is selected by MFD_SPACEMIT_P1, so that we get the
> > following warning:
> > 
> >   WARNING: unmet direct dependencies detected for I2C_K1
> > 
> > and compile I2C_K1 without COMMON_CLK. Please, fix it and
> > resubmit the patch.
> Yes, I noticed that too. The issue is introduced by PMIC driver(P1)
> instead of I2C.
> Should I resubmit this patch right after sending the fix,
> or wait until the fix gets merged first?

I have reverted the patch. Please send them both in the same
series, where the first patch is the "fix" (or preparation to
this patch) and then this patch.

Thanks,
Andi
Re: [PATCH v4] i2c: spacemit: configure ILCR for accurate SCL frequency
Posted by Troy Mitchell 1 month, 3 weeks ago
On Thu, Oct 23, 2025 at 12:34:23PM +0200, Andi Shyti wrote:
> Hi Troy,
> 
> On Wed, Oct 22, 2025 at 09:48:04AM +0800, Troy Mitchell wrote:
> > On Tue, Oct 21, 2025 at 06:03:34PM +0200, Andi Shyti wrote:
> > > On Mon, Oct 20, 2025 at 11:28:45AM +0200, Andi Shyti wrote:
> > > > On Fri, Oct 17, 2025 at 03:27:39PM +0800, Troy Mitchell wrote:
> > > > > The SpacemiT I2C controller's SCL (Serial Clock Line) frequency for
> > > > > master mode operations is determined by the ILCR (I2C Load Count Register).
> > > > > Previously, the driver relied on the hardware's reset default
> > > > > values for this register.
> > > > > 
> > > > > The hardware's default ILCR values (SLV=0x156, FLV=0x5d) yield SCL
> > > > > frequencies lower than intended. For example, with the default
> > > > > 31.5 MHz input clock, these default settings result in an SCL
> > > > > frequency of approximately 93 kHz (standard mode) when targeting 100 kHz,
> > > > > and approximately 338 kHz (fast mode) when targeting 400 kHz.
> > > > > These frequencies are below the 100 kHz/400 kHz nominal speeds.
> > > > > 
> > > > > This patch integrates the SCL frequency management into
> > > > > the Common Clock Framework (CCF). Specifically, the ILCR register,
> > > > > which acts as a frequency divider for the SCL clock, is now registered
> > > > > as a managed clock (scl_clk) within the CCF.
> > > > > 
> > > > > This patch also cleans up unnecessary whitespace
> > > > > in the included header files.
> > > > > 
> > > > > Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> > > > 
> > > > merged to i2c/i2c-host.
> > > 
> > > I'm sorry, because of the report from LKP(*) I reverted your
> > > patch. I2C_K1 is selected by MFD_SPACEMIT_P1, so that we get the
> > > following warning:
> > > 
> > >   WARNING: unmet direct dependencies detected for I2C_K1
> > > 
> > > and compile I2C_K1 without COMMON_CLK. Please, fix it and
> > > resubmit the patch.
> > Yes, I noticed that too. The issue is introduced by PMIC driver(P1)
> > instead of I2C.
> > Should I resubmit this patch right after sending the fix,
> > or wait until the fix gets merged first?
> 
> I have reverted the patch. Please send them both in the same
> series, where the first patch is the "fix" (or preparation to
> this patch) and then this patch.
Oh, I have sent a sigle patch that fix the error..
I'll send v2 with this patch.

                          - Troy
> 
> Thanks,
> Andi
>
Re: [PATCH v4] i2c: spacemit: configure ILCR for accurate SCL frequency
Posted by Yixun Lan 1 month, 4 weeks ago
On 15:27 Fri 17 Oct     , Troy Mitchell wrote:
> The SpacemiT I2C controller's SCL (Serial Clock Line) frequency for
> master mode operations is determined by the ILCR (I2C Load Count Register).
> Previously, the driver relied on the hardware's reset default
> values for this register.
> 
> The hardware's default ILCR values (SLV=0x156, FLV=0x5d) yield SCL
> frequencies lower than intended. For example, with the default
> 31.5 MHz input clock, these default settings result in an SCL
> frequency of approximately 93 kHz (standard mode) when targeting 100 kHz,
> and approximately 338 kHz (fast mode) when targeting 400 kHz.
> These frequencies are below the 100 kHz/400 kHz nominal speeds.
> 
> This patch integrates the SCL frequency management into
> the Common Clock Framework (CCF). Specifically, the ILCR register,
> which acts as a frequency divider for the SCL clock, is now registered
> as a managed clock (scl_clk) within the CCF.
> 
> This patch also cleans up unnecessary whitespace
> in the included header files.
> 
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
Reviewed-by: Yixun Lan <dlan@gentoo.org>

> ---
> Changelog in v4:
> - initialize clk_init_data with {} so that init.flags is implicitly set to 0
> - minor cleanup and style fixes for better readability
> - remove unused spacemit_i2c_scl_clk_exclusive_put() cleanup callback
> - replace clk_set_rate_exclusive()/clk_rate_exclusive_put() pair with clk_set_rate()
> - simplify LCR LV field macros by using FIELD_GET/FIELD_MAX helpers
> - Link to v3: https://lore.kernel.org/all/20250814-k1-i2c-ilcr-v3-1-317723e74bcd@linux.spacemit.com/
..
-- 
Yixun Lan (dlan)
Re: [PATCH v4] i2c: spacemit: configure ILCR for accurate SCL frequency
Posted by Andi Shyti 1 month, 4 weeks ago
Hi,

On Fri, Oct 17, 2025 at 03:27:39PM +0800, Troy Mitchell wrote:
> The SpacemiT I2C controller's SCL (Serial Clock Line) frequency for
> master mode operations is determined by the ILCR (I2C Load Count Register).
> Previously, the driver relied on the hardware's reset default
> values for this register.
> 
> The hardware's default ILCR values (SLV=0x156, FLV=0x5d) yield SCL
> frequencies lower than intended. For example, with the default
> 31.5 MHz input clock, these default settings result in an SCL
> frequency of approximately 93 kHz (standard mode) when targeting 100 kHz,
> and approximately 338 kHz (fast mode) when targeting 400 kHz.
> These frequencies are below the 100 kHz/400 kHz nominal speeds.
> 
> This patch integrates the SCL frequency management into
> the Common Clock Framework (CCF). Specifically, the ILCR register,
> which acts as a frequency divider for the SCL clock, is now registered
> as a managed clock (scl_clk) within the CCF.
> 
> This patch also cleans up unnecessary whitespace
> in the included header files.
> 
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>

looks OK to me. Yixun, any comment here? Do you mind giving it an
ack?

Andi
Re: [PATCH v4] i2c: spacemit: configure ILCR for accurate SCL frequency
Posted by Yixun Lan 1 month, 4 weeks ago
Hi Andi,

On 00:32 Mon 20 Oct     , Andi Shyti wrote:
> Hi,
> 
> On Fri, Oct 17, 2025 at 03:27:39PM +0800, Troy Mitchell wrote:
> > The SpacemiT I2C controller's SCL (Serial Clock Line) frequency for
> > master mode operations is determined by the ILCR (I2C Load Count Register).
> > Previously, the driver relied on the hardware's reset default
> > values for this register.
> > 
> > The hardware's default ILCR values (SLV=0x156, FLV=0x5d) yield SCL
> > frequencies lower than intended. For example, with the default
> > 31.5 MHz input clock, these default settings result in an SCL
> > frequency of approximately 93 kHz (standard mode) when targeting 100 kHz,
> > and approximately 338 kHz (fast mode) when targeting 400 kHz.
> > These frequencies are below the 100 kHz/400 kHz nominal speeds.
> > 
> > This patch integrates the SCL frequency management into
> > the Common Clock Framework (CCF). Specifically, the ILCR register,
> > which acts as a frequency divider for the SCL clock, is now registered
> > as a managed clock (scl_clk) within the CCF.
> > 
> > This patch also cleans up unnecessary whitespace
> > in the included header files.
> > 
> > Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> 
> looks OK to me. Yixun, any comment here? Do you mind giving it an
> ack?
> 
Please go ahead, I gave a Reviewed-by, thanks

-- 
Yixun Lan (dlan)