* [PATCH v2 0/4] fix the SpacemiT P1 Kconfig and resend the K1 I2C ILCR patch.
@ 2025-10-27 5:48 Troy Mitchell
2025-10-27 5:48 ` [PATCH v2 1/4] mfd: simple-mfd-i2c: remove select I2C_K1 Troy Mitchell
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Troy Mitchell @ 2025-10-27 5:48 UTC (permalink / raw)
To: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, Mark Brown
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc,
Troy Mitchell, kernel test robot
Since P1 Kconfig directly selects K1_I2C, after the I2C ILCR patch was
merged, the driver would fail [1] when COMMON_CLK was not selected.
This series fixes the P1 Kconfig and resends the I2C ILCR patch(This
patch has reverted by maintainer [2]). In addition, the Kconfig for
P1's two subdevices, regulator and RTC, has been updated to use
'depends on MFD_SPACEMIT_P1' instead of 'select'.
Link: https://lore.kernel.org/oe-kbuild-all/202510202150.2qXd8e7Y-lkp@intel.com/ [1]
Link: https://lore.kernel.org/all/sdhkjmi5l2m4ua4zqkwkecbihul5bc2dbmitudwfd57y66mdht@6ipjfyz7dtmx/ [2]
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
Troy Mitchell (4):
mfd: simple-mfd-i2c: remove select I2C_K1
i2c: spacemit: configure ILCR for accurate SCL frequency
rtc: spacemit: MFD_SPACEMIT_P1 as dependencies
regulator: spacemit: MFD_SPACEMIT_P1 as dependencies
drivers/i2c/busses/Kconfig | 2 +-
drivers/i2c/busses/i2c-k1.c | 159 ++++++++++++++++++++++++++++++++++++++++----
drivers/mfd/Kconfig | 1 -
drivers/regulator/Kconfig | 3 +-
drivers/rtc/Kconfig | 2 +-
5 files changed, 148 insertions(+), 19 deletions(-)
---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20251021-p1-kconfig-fix-6d2b59d03b8f
Best regards,
--
Troy Mitchell <troy.mitchell@linux.spacemit.com>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/4] mfd: simple-mfd-i2c: remove select I2C_K1
2025-10-27 5:48 [PATCH v2 0/4] fix the SpacemiT P1 Kconfig and resend the K1 I2C ILCR patch Troy Mitchell
@ 2025-10-27 5:48 ` Troy Mitchell
2025-11-06 16:04 ` (subset) " Lee Jones
2025-10-27 5:48 ` [PATCH v2 2/4] i2c: spacemit: configure ILCR for accurate SCL frequency Troy Mitchell
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Troy Mitchell @ 2025-10-27 5:48 UTC (permalink / raw)
To: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, Mark Brown
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc,
Troy Mitchell, kernel test robot
select will force a symbol to a specific value without considering
its dependencies. As a result, the i2c-k1 driver will fail to build
when OF or COMMON_CLK are disabled.
The reason for removing I2C_K1 instead of adding a depends on condition
is to keep the possibility for other SoCs to use this PMIC.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202510211523.sSEVqPUQ-lkp@intel.com/
Acked-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
Changelog in v2:
- nothing
Link to v1: https://lore.kernel.org/all/20251022-p1-kconfig-fix-v1-1-c142d51e1b08@linux.spacemit.com/
---
drivers/mfd/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 6cec1858947bf7ab5ee78beb730c95dabcb43a98..ea367c7e97f116d7585411fff5ba6bcd36882524 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1258,7 +1258,6 @@ config MFD_SPACEMIT_P1
tristate "SpacemiT P1 PMIC"
depends on ARCH_SPACEMIT || COMPILE_TEST
depends on I2C
- select I2C_K1
select MFD_SIMPLE_MFD_I2C
help
This option supports the I2C-based SpacemiT P1 PMIC, which
--
2.51.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/4] i2c: spacemit: configure ILCR for accurate SCL frequency
2025-10-27 5:48 [PATCH v2 0/4] fix the SpacemiT P1 Kconfig and resend the K1 I2C ILCR patch Troy Mitchell
2025-10-27 5:48 ` [PATCH v2 1/4] mfd: simple-mfd-i2c: remove select I2C_K1 Troy Mitchell
@ 2025-10-27 5:48 ` Troy Mitchell
2025-11-12 6:14 ` Troy Mitchell
2025-10-27 5:48 ` [PATCH v2 3/4] rtc: spacemit: MFD_SPACEMIT_P1 as dependencies Troy Mitchell
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Troy Mitchell @ 2025-10-27 5:48 UTC (permalink / raw)
To: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, Mark Brown
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc,
Troy Mitchell
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.
Reviewed-by: Yixun Lan <dlan@gentoo.org>
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
This patch was affected by the P1 Kconfig, which caused the maintainer
to revert it.
The current commit is a direct cherry-pick and reserves the original changelog.
This note is to clarify for anyone who sees the cover letter marked as v2
while the changelog entries reach v4.
---
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);
--
2.51.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/4] rtc: spacemit: MFD_SPACEMIT_P1 as dependencies
2025-10-27 5:48 [PATCH v2 0/4] fix the SpacemiT P1 Kconfig and resend the K1 I2C ILCR patch Troy Mitchell
2025-10-27 5:48 ` [PATCH v2 1/4] mfd: simple-mfd-i2c: remove select I2C_K1 Troy Mitchell
2025-10-27 5:48 ` [PATCH v2 2/4] i2c: spacemit: configure ILCR for accurate SCL frequency Troy Mitchell
@ 2025-10-27 5:48 ` Troy Mitchell
2025-10-27 5:48 ` [PATCH v2 4/4] regulator: " Troy Mitchell
2025-10-27 10:24 ` [PATCH v2 0/4] fix the SpacemiT P1 Kconfig and resend the K1 I2C ILCR patch Aurelien Jarno
4 siblings, 0 replies; 12+ messages in thread
From: Troy Mitchell @ 2025-10-27 5:48 UTC (permalink / raw)
To: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, Mark Brown
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc,
Troy Mitchell
RTC_DRV_SPACEMIT_P1 is a subdevice of P1 and should depend on
MFD_SPACEMIT_P1 rather than selecting it directly. Using 'select'
does not always respect the parent's dependencies, so 'depends on'
is the safer and more correct choice.
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
drivers/rtc/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 2933c41c77c88e60df721fe65b9c8afb995ae51e..1ea0123e386f2b140e1a63a182d1781f6a17e835 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -409,7 +409,7 @@ config RTC_DRV_MAX77686
config RTC_DRV_SPACEMIT_P1
tristate "SpacemiT P1 RTC"
depends on ARCH_SPACEMIT || COMPILE_TEST
- select MFD_SPACEMIT_P1
+ depends on MFD_SPACEMIT_P1
default ARCH_SPACEMIT
help
Enable support for the RTC function in the SpacemiT P1 PMIC.
--
2.51.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 4/4] regulator: spacemit: MFD_SPACEMIT_P1 as dependencies
2025-10-27 5:48 [PATCH v2 0/4] fix the SpacemiT P1 Kconfig and resend the K1 I2C ILCR patch Troy Mitchell
` (2 preceding siblings ...)
2025-10-27 5:48 ` [PATCH v2 3/4] rtc: spacemit: MFD_SPACEMIT_P1 as dependencies Troy Mitchell
@ 2025-10-27 5:48 ` Troy Mitchell
2025-10-27 12:08 ` Mark Brown
2025-10-27 10:24 ` [PATCH v2 0/4] fix the SpacemiT P1 Kconfig and resend the K1 I2C ILCR patch Aurelien Jarno
4 siblings, 1 reply; 12+ messages in thread
From: Troy Mitchell @ 2025-10-27 5:48 UTC (permalink / raw)
To: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, Mark Brown
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc,
Troy Mitchell
REGULATOR_SPACEMIT_P1 is a subdevice of P1 and should depend on
MFD_SPACEMIT_P1 rather than selecting it directly. Using 'select'
does not always respect the parent's dependencies, so 'depends on'
is the safer and more correct choice.
Since MFD_SPACEMIT_P1 already depends on I2C_K1, the dependency
in REGULATOR_SPACEMIT_P1 is now redundant.
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
drivers/regulator/Kconfig | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index d84f3d054c59d86d91d859808aa73a3b609d16d0..f5ee804077cfcb300ca5cf5d865b6684943cd749 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1455,8 +1455,7 @@ config REGULATOR_SLG51000
config REGULATOR_SPACEMIT_P1
tristate "SpacemiT P1 regulators"
depends on ARCH_SPACEMIT || COMPILE_TEST
- depends on I2C
- select MFD_SPACEMIT_P1
+ depends on MFD_SPACEMIT_P1
default ARCH_SPACEMIT
help
Enable support for regulators implemented by the SpacemiT P1
--
2.51.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/4] fix the SpacemiT P1 Kconfig and resend the K1 I2C ILCR patch.
2025-10-27 5:48 [PATCH v2 0/4] fix the SpacemiT P1 Kconfig and resend the K1 I2C ILCR patch Troy Mitchell
` (3 preceding siblings ...)
2025-10-27 5:48 ` [PATCH v2 4/4] regulator: " Troy Mitchell
@ 2025-10-27 10:24 ` Aurelien Jarno
2025-10-27 13:52 ` Troy Mitchell
4 siblings, 1 reply; 12+ messages in thread
From: Aurelien Jarno @ 2025-10-27 10:24 UTC (permalink / raw)
To: Troy Mitchell
Cc: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, Mark Brown, linux-kernel, linux-riscv, spacemit,
linux-i2c, linux-rtc, kernel test robot
Hi,
On 2025-10-27 13:48, Troy Mitchell wrote:
> Since P1 Kconfig directly selects K1_I2C, after the I2C ILCR patch was
> merged, the driver would fail [1] when COMMON_CLK was not selected.
>
> This series fixes the P1 Kconfig and resends the I2C ILCR patch(This
> patch has reverted by maintainer [2]). In addition, the Kconfig for
> P1's two subdevices, regulator and RTC, has been updated to use
> 'depends on MFD_SPACEMIT_P1' instead of 'select'.
>
> Link: https://lore.kernel.org/oe-kbuild-all/202510202150.2qXd8e7Y-lkp@intel.com/ [1]
> Link: https://lore.kernel.org/all/sdhkjmi5l2m4ua4zqkwkecbihul5bc2dbmitudwfd57y66mdht@6ipjfyz7dtmx/ [2]
>
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
I think this series misses a patch to add a default value for
MFD_SPACEMIT_P1. Otherwise it doesn't make sense to define a default
value for the ones depending on it (RTC_DRV_SPACEMIT_P1,
REGULATOR_SPACEMIT_P1).
Regards
Aurelien
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://aurel32.net
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] regulator: spacemit: MFD_SPACEMIT_P1 as dependencies
2025-10-27 5:48 ` [PATCH v2 4/4] regulator: " Troy Mitchell
@ 2025-10-27 12:08 ` Mark Brown
0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2025-10-27 12:08 UTC (permalink / raw)
To: Troy Mitchell
Cc: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, linux-kernel, linux-riscv, spacemit, linux-i2c,
linux-rtc
[-- Attachment #1.1: Type: text/plain, Size: 349 bytes --]
On Mon, Oct 27, 2025 at 01:48:08PM +0800, Troy Mitchell wrote:
> REGULATOR_SPACEMIT_P1 is a subdevice of P1 and should depend on
> MFD_SPACEMIT_P1 rather than selecting it directly. Using 'select'
> does not always respect the parent's dependencies, so 'depends on'
> is the safer and more correct choice.
Acked-by: Mark Brown <broonie@kernel.org>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/4] fix the SpacemiT P1 Kconfig and resend the K1 I2C ILCR patch.
2025-10-27 10:24 ` [PATCH v2 0/4] fix the SpacemiT P1 Kconfig and resend the K1 I2C ILCR patch Aurelien Jarno
@ 2025-10-27 13:52 ` Troy Mitchell
0 siblings, 0 replies; 12+ messages in thread
From: Troy Mitchell @ 2025-10-27 13:52 UTC (permalink / raw)
To: Troy Mitchell, Lee Jones, Yixun Lan, Alex Elder, Andi Shyti,
Alexandre Belloni, Liam Girdwood, Mark Brown, linux-kernel,
linux-riscv, spacemit, linux-i2c, linux-rtc, kernel test robot
On Mon, Oct 27, 2025 at 11:24:21AM +0100, Aurelien Jarno wrote:
> Hi,
>
> On 2025-10-27 13:48, Troy Mitchell wrote:
> > Since P1 Kconfig directly selects K1_I2C, after the I2C ILCR patch was
> > merged, the driver would fail [1] when COMMON_CLK was not selected.
> >
> > This series fixes the P1 Kconfig and resends the I2C ILCR patch(This
> > patch has reverted by maintainer [2]). In addition, the Kconfig for
> > P1's two subdevices, regulator and RTC, has been updated to use
> > 'depends on MFD_SPACEMIT_P1' instead of 'select'.
> >
> > Link: https://lore.kernel.org/oe-kbuild-all/202510202150.2qXd8e7Y-lkp@intel.com/ [1]
> > Link: https://lore.kernel.org/all/sdhkjmi5l2m4ua4zqkwkecbihul5bc2dbmitudwfd57y66mdht@6ipjfyz7dtmx/ [2]
> >
> > Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
>
> I think this series misses a patch to add a default value for
> MFD_SPACEMIT_P1. Otherwise it doesn't make sense to define a default
> value for the ones depending on it (RTC_DRV_SPACEMIT_P1,
> REGULATOR_SPACEMIT_P1).
Yes, I forgot that. I'll add it in the next version. Thanks!
- Troy
>
> Regards
> Aurelien
>
> --
> Aurelien Jarno GPG: 4096R/1DDD8C9B
> aurelien@aurel32.net http://aurel32.net
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: (subset) [PATCH v2 1/4] mfd: simple-mfd-i2c: remove select I2C_K1
2025-10-27 5:48 ` [PATCH v2 1/4] mfd: simple-mfd-i2c: remove select I2C_K1 Troy Mitchell
@ 2025-11-06 16:04 ` Lee Jones
2025-11-07 1:29 ` Troy Mitchell
0 siblings, 1 reply; 12+ messages in thread
From: Lee Jones @ 2025-11-06 16:04 UTC (permalink / raw)
To: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, Mark Brown, Troy Mitchell
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc,
kernel test robot
On Mon, 27 Oct 2025 13:48:05 +0800, Troy Mitchell wrote:
> select will force a symbol to a specific value without considering
> its dependencies. As a result, the i2c-k1 driver will fail to build
> when OF or COMMON_CLK are disabled.
>
> The reason for removing I2C_K1 instead of adding a depends on condition
> is to keep the possibility for other SoCs to use this PMIC.
>
> [...]
Applied, thanks!
[1/4] mfd: simple-mfd-i2c: remove select I2C_K1
commit: ecf6bc474ae97c404e2125b413eb0ef3627b03c5
--
Lee Jones [李琼斯]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: (subset) [PATCH v2 1/4] mfd: simple-mfd-i2c: remove select I2C_K1
2025-11-06 16:04 ` (subset) " Lee Jones
@ 2025-11-07 1:29 ` Troy Mitchell
2025-11-10 13:45 ` Lee Jones
0 siblings, 1 reply; 12+ messages in thread
From: Troy Mitchell @ 2025-11-07 1:29 UTC (permalink / raw)
To: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, Mark Brown, Troy Mitchell
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc,
kernel test robot
On Thu, Nov 06, 2025 at 04:04:21PM +0000, Lee Jones wrote:
> On Mon, 27 Oct 2025 13:48:05 +0800, Troy Mitchell wrote:
> > select will force a symbol to a specific value without considering
> > its dependencies. As a result, the i2c-k1 driver will fail to build
> > when OF or COMMON_CLK are disabled.
> >
> > The reason for removing I2C_K1 instead of adding a depends on condition
> > is to keep the possibility for other SoCs to use this PMIC.
> >
> > [...]
>
> Applied, thanks!
>
> [1/4] mfd: simple-mfd-i2c: remove select I2C_K1
> commit: ecf6bc474ae97c404e2125b413eb0ef3627b03c5
Hi Lee,
I think you didn't notice this reply [1]
(Maybe because he was replying to the cover letter).
As Aurelien mentioned, the current shutdown/reboot (and possibly the regulator
as well) intends to use the `default MFD_SPACEMIT_P1`.
So if there’s no `default m if ARCH_SPACEMIT`,
the default value in subdevices may not make much sense.
But don’t worry — to make things easier for you, I’ll send an additional
patch based on your branch (in this series).
How does that sound?
Link: https://lore.kernel.org/all/aP9IVckJT-k2_O4K@aurel32.net/ [1]
>
> --
> Lee Jones [李琼斯]
>
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: (subset) [PATCH v2 1/4] mfd: simple-mfd-i2c: remove select I2C_K1
2025-11-07 1:29 ` Troy Mitchell
@ 2025-11-10 13:45 ` Lee Jones
0 siblings, 0 replies; 12+ messages in thread
From: Lee Jones @ 2025-11-10 13:45 UTC (permalink / raw)
To: Troy Mitchell
Cc: Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, Mark Brown, linux-kernel, linux-riscv, spacemit,
linux-i2c, linux-rtc, kernel test robot
On Fri, 07 Nov 2025, Troy Mitchell wrote:
> On Thu, Nov 06, 2025 at 04:04:21PM +0000, Lee Jones wrote:
> > On Mon, 27 Oct 2025 13:48:05 +0800, Troy Mitchell wrote:
> > > select will force a symbol to a specific value without considering
> > > its dependencies. As a result, the i2c-k1 driver will fail to build
> > > when OF or COMMON_CLK are disabled.
> > >
> > > The reason for removing I2C_K1 instead of adding a depends on condition
> > > is to keep the possibility for other SoCs to use this PMIC.
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [1/4] mfd: simple-mfd-i2c: remove select I2C_K1
> > commit: ecf6bc474ae97c404e2125b413eb0ef3627b03c5
> Hi Lee,
>
> I think you didn't notice this reply [1]
> (Maybe because he was replying to the cover letter).
>
> As Aurelien mentioned, the current shutdown/reboot (and possibly the regulator
> as well) intends to use the `default MFD_SPACEMIT_P1`.
> So if there’s no `default m if ARCH_SPACEMIT`,
> the default value in subdevices may not make much sense.
>
> But don’t worry — to make things easier for you, I’ll send an additional
> patch based on your branch (in this series).
>
> How does that sound?
Go for it.
--
Lee Jones [李琼斯]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] i2c: spacemit: configure ILCR for accurate SCL frequency
2025-10-27 5:48 ` [PATCH v2 2/4] i2c: spacemit: configure ILCR for accurate SCL frequency Troy Mitchell
@ 2025-11-12 6:14 ` Troy Mitchell
0 siblings, 0 replies; 12+ messages in thread
From: Troy Mitchell @ 2025-11-12 6:14 UTC (permalink / raw)
To: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, Mark Brown
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc,
Troy Mitchell
On Mon, Oct 27, 2025 at 01:48:06PM +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.
>
> Reviewed-by: Yixun Lan <dlan@gentoo.org>
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
> This patch was affected by the P1 Kconfig, which caused the maintainer
> to revert it.
> The current commit is a direct cherry-pick and reserves the original changelog.
> This note is to clarify for anyone who sees the cover letter marked as v2
> while the changelog entries reach v4.
Hi Andi,
Since the issue affecting the I2C patch has been fixed [1],
I think it should be ready to be merged now. What do you think?
Link: https://lore.kernel.org/all/176244506110.1925720.10807118665958896958.b4-ty@kernel.org/ [1]
- Troy
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-11-12 6:16 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-27 5:48 [PATCH v2 0/4] fix the SpacemiT P1 Kconfig and resend the K1 I2C ILCR patch Troy Mitchell
2025-10-27 5:48 ` [PATCH v2 1/4] mfd: simple-mfd-i2c: remove select I2C_K1 Troy Mitchell
2025-11-06 16:04 ` (subset) " Lee Jones
2025-11-07 1:29 ` Troy Mitchell
2025-11-10 13:45 ` Lee Jones
2025-10-27 5:48 ` [PATCH v2 2/4] i2c: spacemit: configure ILCR for accurate SCL frequency Troy Mitchell
2025-11-12 6:14 ` Troy Mitchell
2025-10-27 5:48 ` [PATCH v2 3/4] rtc: spacemit: MFD_SPACEMIT_P1 as dependencies Troy Mitchell
2025-10-27 5:48 ` [PATCH v2 4/4] regulator: " Troy Mitchell
2025-10-27 12:08 ` Mark Brown
2025-10-27 10:24 ` [PATCH v2 0/4] fix the SpacemiT P1 Kconfig and resend the K1 I2C ILCR patch Aurelien Jarno
2025-10-27 13:52 ` Troy Mitchell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).