* [PATCH v3 0/3] Samsung mfd/rtc driver alarm IRQ simplification
From: André Draszik @ 2026-01-13 14:03 UTC (permalink / raw)
To: Krzysztof Kozlowski, Lee Jones, Alexandre Belloni
Cc: Peter Griffin, Tudor Ambarus, Will McVicker, Juan Yescas,
Douglas Anderson, kernel-team, Kaustabh Chakraborty, linux-kernel,
linux-samsung-soc, linux-rtc, André Draszik
Hi,
With the attached patches the Samsung s5m RTC driver is simplified a
little bit with regards to alarm IRQ acquisition.
The end result is that instead of having a list of IRQ numbers for each
variant (and a BUILD_BUG_ON() to ensure consistency), the RTC driver
queries the 'alarm' platform resource from the parent (mfd cell).
Additionally, we can drop a now-useless field from runtime data,
reducing memory consumption slightly.
The attached patches must be applied in-order as patch 2 without 1 will
fail at runtime, and patch 3 without 2 will fail at build time. I would
expect them all to go via the MFD tree. Alternatively, they could be
applied individually to the respective kernel trees during multiple
kernel release cycles, but that seems a needless complication and
delay.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
Changes in v3:
- make s2mpg1x_add_chained_irq_chip() specific to PMIC irq chip (Lee),
and rename to s2mpg1x_add_chained_pmic() (Andre)
- Link to v2: https://lore.kernel.org/r/20251120-s5m-alarm-v2-0-cc15f0e32161@linaro.org
Changes in v2:
- rebase on top of https://lore.kernel.org/r/20251114-s2mpg10-chained-irq-v1-1-34ddfa49c4cd@linaro.org
- return struct regmap_irq_chip_data * in sec_irq_init() (Lee)
- collect tags
- Link to v1: https://lore.kernel.org/r/20251114-s5m-alarm-v1-0-c9b3bebae65f@linaro.org
---
André Draszik (3):
mfd: sec: add rtc alarm IRQ as platform device resource
rtc: s5m: query platform device IRQ resource for alarm IRQ
mfd: sec: drop now unused struct sec_pmic_dev::irq_data
drivers/mfd/sec-common.c | 45 ++++++++++++++++++++--------
drivers/mfd/sec-core.h | 2 +-
drivers/mfd/sec-irq.c | 64 ++++++++++++++++++----------------------
drivers/rtc/rtc-s5m.c | 21 +++++--------
include/linux/mfd/samsung/core.h | 1 -
5 files changed, 71 insertions(+), 62 deletions(-)
---
base-commit: 0f853ca2a798ead9d24d39cad99b0966815c582a
change-id: 20251114-s5m-alarm-3de705ea53ce
Best regards,
--
André Draszik <andre.draszik@linaro.org>
^ permalink raw reply
* Re: [PATCH RESEND v2 3/3] mfd: sec: drop now unused struct sec_pmic_dev::irq_data
From: André Draszik @ 2026-01-13 13:48 UTC (permalink / raw)
To: Lee Jones
Cc: Krzysztof Kozlowski, Alexandre Belloni, Peter Griffin,
Tudor Ambarus, Will McVicker, Juan Yescas, Douglas Anderson,
kernel-team, Kaustabh Chakraborty, linux-kernel,
linux-samsung-soc, linux-rtc
In-Reply-To: <20260109104534.GF1118061@google.com>
Hi Lee,
Thanks for your review.
On Fri, 2026-01-09 at 10:45 +0000, Lee Jones wrote:
> On Wed, 17 Dec 2025, André Draszik wrote:
>
> > This was used only to allow the s5m RTC driver to deal with the alarm
> > IRQ. That driver now uses a different approach to acquire that IRQ, and
> > ::irq_data doesn't need to be kept around anymore.
> >
> > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > ---
> > drivers/mfd/sec-common.c | 9 +++---
> > drivers/mfd/sec-core.h | 2 +-
> > drivers/mfd/sec-irq.c | 63 ++++++++++++++++++----------------------
> > include/linux/mfd/samsung/core.h | 1 -
> > 4 files changed, 35 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
> > index 77370db52a7ba81234136b29f85892f4b197f429..0021f9ae8484fd0afc2e47c813a953c91fa38546 100644
> > --- a/drivers/mfd/sec-common.c
> > +++ b/drivers/mfd/sec-common.c
> > @@ -163,6 +163,7 @@ sec_pmic_parse_dt_pdata(struct device *dev)
> > int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
> > struct regmap *regmap, struct i2c_client *client)
> > {
> > + struct regmap_irq_chip_data *irq_data;
> > struct sec_platform_data *pdata;
> > const struct mfd_cell *sec_devs;
> > struct sec_pmic_dev *sec_pmic;
> > @@ -187,9 +188,9 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
> >
> > sec_pmic->pdata = pdata;
> >
> > - ret = sec_irq_init(sec_pmic);
> > - if (ret)
> > - return ret;
> > + irq_data = sec_irq_init(sec_pmic);
> > + if (IS_ERR(irq_data))
> > + return PTR_ERR(irq_data);
> >
> > pm_runtime_set_active(sec_pmic->dev);
> >
> > @@ -240,7 +241,7 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
> > sec_pmic->device_type);
> > }
> > ret = devm_mfd_add_devices(sec_pmic->dev, -1, sec_devs, num_sec_devs,
> > - NULL, 0, regmap_irq_get_domain(sec_pmic->irq_data));
> > + NULL, 0, regmap_irq_get_domain(irq_data));
> > if (ret)
> > return ret;
> >
> > diff --git a/drivers/mfd/sec-core.h b/drivers/mfd/sec-core.h
> > index 92c7558ab8b0de44a52e028eeb7998e38358cb4c..8d85c70c232612d1f7e5fb61b2acd25bf03a62e0 100644
> > --- a/drivers/mfd/sec-core.h
> > +++ b/drivers/mfd/sec-core.h
> > @@ -18,6 +18,6 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
> > struct regmap *regmap, struct i2c_client *client);
> > void sec_pmic_shutdown(struct device *dev);
> >
> > -int sec_irq_init(struct sec_pmic_dev *sec_pmic);
> > +struct regmap_irq_chip_data *sec_irq_init(struct sec_pmic_dev *sec_pmic);
> >
> > #endif /* __SEC_CORE_INT_H */
> > diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
> > index d992e41e716dcdc060421e1db8475523842a12be..96f53c3617da4cb54f650f9b98c0b934b823ceda 100644
> > --- a/drivers/mfd/sec-irq.c
> > +++ b/drivers/mfd/sec-irq.c
> > @@ -268,26 +268,28 @@ static const struct regmap_irq_chip s5m8767_irq_chip = {
> > .ack_base = S5M8767_REG_INT1,
> > };
> >
> > -static int s2mpg1x_add_chained_irq_chip(struct device *dev, struct regmap *regmap, int pirq,
> > - struct regmap_irq_chip_data *parent,
> > - const struct regmap_irq_chip *chip,
> > - struct regmap_irq_chip_data **data)
> > +static struct regmap_irq_chip_data *
> > +s2mpg1x_add_chained_irq_chip(struct device *dev, struct regmap *regmap, int pirq,
> > + struct regmap_irq_chip_data *parent,
> > + const struct regmap_irq_chip *chip)
> > {
> > + struct regmap_irq_chip_data *data;
> > int irq, ret;
> >
> > irq = regmap_irq_get_virq(parent, pirq);
> > if (irq < 0)
> > - return dev_err_probe(dev, irq, "Failed to get parent vIRQ(%d) for chip %s\n", pirq,
> > - chip->name);
> > + return dev_err_ptr_probe(dev, irq, "Failed to get parent vIRQ(%d) for chip %s\n",
> > + pirq, chip->name);
> >
> > - ret = devm_regmap_add_irq_chip(dev, regmap, irq, IRQF_ONESHOT | IRQF_SHARED, 0, chip, data);
> > + ret = devm_regmap_add_irq_chip(dev, regmap, irq, IRQF_ONESHOT | IRQF_SHARED, 0, chip,
> > + &data);
> > if (ret)
> > - return dev_err_probe(dev, ret, "Failed to add %s IRQ chip\n", chip->name);
> > + return dev_err_ptr_probe(dev, ret, "Failed to add %s IRQ chip\n", chip->name);
> >
> > - return 0;
> > + return data;
> > }
> >
> > -static int sec_irq_init_s2mpg1x(struct sec_pmic_dev *sec_pmic)
> > +static struct regmap_irq_chip_data *sec_irq_init_s2mpg1x(struct sec_pmic_dev *sec_pmic)
> > {
> > const struct regmap_irq_chip *irq_chip, *chained_irq_chip;
> > struct regmap_irq_chip_data *irq_data;
> > @@ -302,27 +304,28 @@ static int sec_irq_init_s2mpg1x(struct sec_pmic_dev *sec_pmic)
> > chained_pirq = S2MPG10_COMMON_IRQ_PMIC;
> > break;
> > default:
> > - return dev_err_probe(sec_pmic->dev, -EINVAL, "Unsupported device type %d\n",
> > - sec_pmic->device_type);
> > + return dev_err_ptr_probe(sec_pmic->dev, -EINVAL, "Unsupported device type %d\n",
> > + sec_pmic->device_type);
> > };
> >
> > regmap_common = dev_get_regmap(sec_pmic->dev, "common");
> > if (!regmap_common)
> > - return dev_err_probe(sec_pmic->dev, -EINVAL, "No 'common' regmap %d\n",
> > - sec_pmic->device_type);
> > + return dev_err_ptr_probe(sec_pmic->dev, -EINVAL, "No 'common' regmap %d\n",
> > + sec_pmic->device_type);
> >
> > ret = devm_regmap_add_irq_chip(sec_pmic->dev, regmap_common, sec_pmic->irq, IRQF_ONESHOT, 0,
> > irq_chip, &irq_data);
> > if (ret)
> > - return dev_err_probe(sec_pmic->dev, ret, "Failed to add %s IRQ chip\n",
> > - irq_chip->name);
> > + return dev_err_ptr_probe(sec_pmic->dev, ret, "Failed to add %s IRQ chip\n",
> > + irq_chip->name);
> >
> > return s2mpg1x_add_chained_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic, chained_pirq,
> > - irq_data, chained_irq_chip, &sec_pmic->irq_data);
> > + irq_data, chained_irq_chip);
>
> That's a shame.
>
> By keeping irq_data, you could have cleaned-up a bunch of these ugly
> calls by simply passing around sec_pmic or better yet dev (then extract
> sec_pmic from there).
>
> Thus:
>
> return s2mpg1x_add_chained_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic, chained_pirq,
> irq_data, chained_irq_chip, &sec_pmic->irq_data);
> irq_data, chained_irq_chip);
>
> Becomes:
>
> return s2mpg1x_add_chained_irq_chip(dev, chained_pirq, irq_data, chained_irq_chip);
s2mpg1x_add_chained_irq_chip() was modelled to be consistent with other
drivers doing a similar setup:
drivers/mfd/intel_soc_pmic_bxtwc.c
drivers/mfd/max77759.c
Maybe they're not good examples, but then maybe they also should be changed,
to ensure people don't copy from them.
As-is, it can easily be extended for additional chained chips, as the relevant
arguments are parameters and it can be used as a template for the benefit of
other (new) drivers (writers).
It slightly diverged from above two drivers in v2 since you requested for it
to 'return' 'struct regmap_irq_chip_data *' instead of having
'struct regmap_irq_chip_data **' an additional argument in v1, though.
With your proposed change it won't be able to serve as example code as
easily. I see benefit in having similar code for similar setup. Anyway,
I'll change it again.
Cheers,
Andre'
^ permalink raw reply
* Re: [PATCH v5 2/3] mfd: simple-mfd-i2c: add default value
From: Lee Jones @ 2026-01-13 12:42 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
In-Reply-To: <20260108-p1-kconfig-fix-v5-2-6fe19f460269@linux.spacemit.com>
On Thu, 08 Jan 2026, Troy Mitchell wrote:
> The default value of the P1 sub-device depends on the value
> of P1, so P1 should have a default value here.
>
> Acked-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
> Change log in v5:
> - nothing
> - Link to v4: https://lore.kernel.org/all/20251225-p1-kconfig-fix-v4-2-44b6728117c1@linux.spacemit.com/
>
> Change log in v4:
> - default m if ARCH_SPACEMIT instead of default ARCH_SPACEMIT
> - Link to v3: https://lore.kernel.org/all/20251118-p1-kconfig-fix-v3-4-8839c5ac5db3@linux.spacemit.com/
> ---
> drivers/mfd/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index aace5766b38aa5e46e32a8a7b42eea238159fbcf..c757bc365029dc794c658fc5b10084a0f29ac9b6 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1276,6 +1276,7 @@ config MFD_SPACEMIT_P1
> depends on ARCH_SPACEMIT || COMPILE_TEST
> depends on I2C
> select MFD_SIMPLE_MFD_I2C
> + default m if ARCH_SPACEMIT
> help
> This option supports the I2C-based SpacemiT P1 PMIC, which
> contains regulators, a power switch, GPIOs, an RTC, and more.
I already applied v4.
--
Lee Jones [李琼斯]
^ permalink raw reply
* [PATCH v2 11/16] rtc: pic32: update include to use pic32.h from platform_data
From: Brian Masney @ 2026-01-12 22:48 UTC (permalink / raw)
To: Thomas Bogendoerfer, Claudiu Beznea
Cc: linux-mips, linux-kernel, Brian Masney, Alexandre Belloni,
linux-rtc
In-Reply-To: <20260112-mips-pic32-header-move-v2-0-927d516b1ff9@redhat.com>
Use the linux/platform_data/pic32.h include instead of
asm/mach-pic32/pic32.h so that the asm variant can be dropped. This
is in preparation for allowing some drivers to be compiled on other
architectures with COMPILE_TEST enabled.
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: linux-rtc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
| 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
--git a/drivers/rtc/rtc-pic32.c b/drivers/rtc/rtc-pic32.c
index 52c11532bc3a3696359ca56349b42860aa90c966..3c7a38a4ac08eb0f5a44ae4e470c208a9d1dd599 100644
--- a/drivers/rtc/rtc-pic32.c
+++ b/drivers/rtc/rtc-pic32.c
@@ -15,8 +15,7 @@
#include <linux/clk.h>
#include <linux/rtc.h>
#include <linux/bcd.h>
-
-#include <asm/mach-pic32/pic32.h>
+#include <linux/platform_data/pic32.h>
#define PIC32_RTCCON 0x00
#define PIC32_RTCCON_ON BIT(15)
--
2.52.0
^ permalink raw reply related
* [PATCH v2 00/16] MIPS: move pic32.h header file from asm to platform_data
From: Brian Masney @ 2026-01-12 22:47 UTC (permalink / raw)
To: Thomas Bogendoerfer, Claudiu Beznea
Cc: linux-mips, linux-kernel, Brian Masney, Michael Turquette,
Stephen Boyd, linux-clk, Thomas Gleixner, Adrian Hunter,
Ulf Hansson, linux-mmc, Linus Walleij, linux-gpio,
Alexandre Belloni, linux-rtc, Greg Kroah-Hartman, Jiri Slaby,
linux-serial, Guenter Roeck, Wim Van Sebroeck, linux-watchdog
There are currently some pic32 MIPS drivers that are in tree, and are
only configured to be compiled on the MIPS pic32 platform. There's a
risk of breaking some of these drivers when migrating drivers away from
legacy APIs. It happened to me with a pic32 clk driver.
Let's go ahead and move the pic32.h from the asm to the platform_data
include directory in the tree. This will make it easier, and cleaner to
enable COMPILE_TEST for some of these pic32 drivers. To do this requires
updating some includes, which I do at the beginning of this series.
This series was compile tested on a centos-stream-10 arm64 host in two
different configurations:
- native arm64 build with COMPILE_TEST (via make allmodconfig)
- MIPS cross compile on arm64 with:
ARCH=mips CROSS_COMPILE=mips64-linux-gnu- make pic32mzda_defconfig
Note that there is a separate MIPS compile error in linux-next, and I
reported it at https://lore.kernel.org/all/aWVs2gVB418WiMVa@redhat.com/
I included a patch at the end that shows enabling COMPILE_TEST for a
pic32 clk driver.
Merge Strategy
==============
- Patches 1-15 can go through the MIPS tree.
- Patch 16 I can repost to Claudiu after patches 1-15 are in Linus's
tree after the next merge window. There is a separate patch set that
fixes a compiler error I unintentionally introduced via the clk tree.
https://lore.kernel.org/linux-clk/CABx5tq+eOocJ41X-GSgkGy6S+s+Am1yCS099wqP695NtwALTmg@mail.gmail.com/T/
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
Changes in v2:
- Fix native MIPS build by updating include files
- Link to v1: https://lore.kernel.org/r/20260109-mips-pic32-header-move-v1-0-99859c55783d@redhat.com
---
Brian Masney (16):
MIPS: pic32: include linux/io.h header on several files
MIPS: pic32: include linux/types.h on pic32.h
MIPS: pic32: drop unused include linux/io.h from pic32.h
MIPS: copy pic32.h header file from asm/mach-pic32/ to include/platform-data/
MAINTAINERS: add include/linux/platform_data/pic32.h to MIPS entry
MIPS: update include to use pic32.h from platform_data
clk: microchip: core: update include to use pic32.h from platform_data
irqchip/irq-pic32-evic: update include to use pic32.h from platform_data
mmc: sdhci-pic32: update include to use pic32.h from platform_data
pinctrl: pic32: update include to use pic32.h from platform_data
rtc: pic32: update include to use pic32.h from platform_data
serial: pic32_uart: update include to use pic32.h from platform_data
watchdog: pic32-dmt: update include to use pic32.h from platform_data
watchdog: pic32-wdt: update include to use pic32.h from platform_data
MIPS: drop unused pic32.h header
clk: microchip: core: allow driver to be compiled with COMPILE_TEST
MAINTAINERS | 1 +
arch/mips/pic32/common/reset.c | 3 ++-
arch/mips/pic32/pic32mzda/config.c | 3 +--
arch/mips/pic32/pic32mzda/early_clk.c | 3 ++-
arch/mips/pic32/pic32mzda/early_console.c | 3 ++-
drivers/clk/microchip/Kconfig | 2 +-
drivers/clk/microchip/clk-core.c | 6 +++++-
drivers/irqchip/irq-pic32-evic.c | 2 +-
drivers/mmc/host/sdhci-pic32.c | 2 +-
drivers/pinctrl/pinctrl-pic32.c | 3 +--
drivers/rtc/rtc-pic32.c | 3 +--
drivers/tty/serial/pic32_uart.c | 3 +--
drivers/watchdog/pic32-dmt.c | 3 +--
drivers/watchdog/pic32-wdt.c | 3 +--
.../mach-pic32 => include/linux/platform_data}/pic32.h | 17 ++++++++++-------
15 files changed, 31 insertions(+), 26 deletions(-)
---
base-commit: f417b7ffcbef7d76b0d8860518f50dae0e7e5eda
change-id: 20260109-mips-pic32-header-move-6ac9965aa70a
Best regards,
--
Brian Masney <bmasney@redhat.com>
^ permalink raw reply
* Re: [PATCH] rtc: max31335: use correct CONFIG symbol in IS_REACHABLE()
From: Nuno Sá @ 2026-01-12 13:23 UTC (permalink / raw)
To: Randy Dunlap, linux-kernel; +Cc: Antoniu Miclaus, Alexandre Belloni, linux-rtc
In-Reply-To: <20260108045432.2705691-1-rdunlap@infradead.org>
On Wed, 2026-01-07 at 20:54 -0800, Randy Dunlap wrote:
> IS_REACHABLE() is meant to be used with full symbol names from a kernel
> .config file, not the shortened symbols used in Kconfig files, so
> change HWMON to CONFIG_HWMON in 3 places.
>
> Fixes: dedaf03b99d6 ("rtc: max31335: add driver support")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> ---
Acked-by: Nuno Sá <nuno.sa@analog.com>
> Cc: Antoniu Miclaus <antoniu.miclaus@analog.com>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: linux-rtc@vger.kernel.org
>
> drivers/rtc/rtc-max31335.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> --- linux-next-20260107.orig/drivers/rtc/rtc-max31335.c
> +++ linux-next-20260107/drivers/rtc/rtc-max31335.c
> @@ -591,7 +591,7 @@ static struct nvmem_config max31335_nvme
> .size = MAX31335_RAM_SIZE,
> };
>
> -#if IS_REACHABLE(HWMON)
> +#if IS_REACHABLE(CONFIG_HWMON)
> static int max31335_read_temp(struct device *dev, enum hwmon_sensor_types type,
> u32 attr, int channel, long *val)
> {
> @@ -672,7 +672,7 @@ static int max31335_clkout_register(stru
> static int max31335_probe(struct i2c_client *client)
> {
> struct max31335_data *max31335;
> -#if IS_REACHABLE(HWMON)
> +#if IS_REACHABLE(CONFIG_HWMON)
> struct device *hwmon;
> #endif
> const struct chip_desc *match;
> @@ -727,7 +727,7 @@ static int max31335_probe(struct i2c_cli
> return dev_err_probe(&client->dev, ret,
> "cannot register rtc nvmem\n");
>
> -#if IS_REACHABLE(HWMON)
> +#if IS_REACHABLE(CONFIG_HWMON)
> if (max31335->chip->temp_reg) {
> hwmon = devm_hwmon_device_register_with_info(&client->dev, client->name,
> max31335,
> &max31335_chip_info, NULL);
^ permalink raw reply
* Re: [PATCH RESEND v6 00/17] Support ROHM BD72720 PMIC
From: Matti Vaittinen @ 2026-01-12 12:04 UTC (permalink / raw)
To: Sebastian Reichel, Lee Jones
Cc: Matti Vaittinen, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Michael Turquette,
Stephen Boyd, Linus Walleij, Bartosz Golaszewski,
Alexandre Belloni, linux-leds, devicetree, linux-kernel, linux-pm,
linux-clk, linux-gpio, linux-rtc, Andreas Kemnade
In-Reply-To: <aWRFs3CJvd37BaoH@venus>
On 12/01/2026 02:53, Sebastian Reichel wrote:
> Hi,
>
> On Fri, Jan 09, 2026 at 09:38:31AM +0000, Lee Jones wrote:
>> [...]
>>>> The MFD parts LGTM.
>>>
>>> Thanks Lee!
>>>
>>>> What Acks are you waiting on? What's the merge strategy?
>>>
>>> I think everything else has been acked by maintainers, except the
>>> power-supply parts. I think those have only been looked at by Andreas and
>>> Linus W. Haven't heard anything from Sebastian :(
>
> Yes, I'm lacking behind quite a bit, sorry for that.
>
>>> I would love to see the patches 1 - 14 and 17 to be merged (via MFD?). I
>>> could then re-spin the 15 and 16 to limited audience as I hope Sebastian had
>>> time to take a look at them. However, I don't think any of the other patches
>>> in the series depend on the last .
>
> Sounds good to me.
Ah. Since the 15/17:
"[PATCH RESEND v6 15/17] power: supply: bd71828: Support wider register
addresses"
was now acked by Sebastian, then it can also go via MFD?
Also, if it is Ok to address all the "dev_err() + return ERRNO" =>
"return dev_err_probe(,ERRNO,)" conversions in a follow-up, then I guess
the whole series, including 16/17 is good to go? If this is the case,
please just let me know and I'll send the follow-up. Otherwise, I will
re-spin the 16/17 and add a new patch for the remaining "dev_err() +
return ERRNO" => "return dev_err_probe(,ERRNO,)" case(s).
Yours,
-- Matti
--
---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* Re: [PATCH RESEND v6 16/17] power: supply: bd71828-power: Support ROHM BD72720
From: Matti Vaittinen @ 2026-01-12 6:44 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Matti Vaittinen, Lee Jones, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Michael Turquette, Stephen Boyd, Linus Walleij,
Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
Andreas Kemnade
In-Reply-To: <aWRERf70jg-IzqIx@venus>
On 12/01/2026 02:51, Sebastian Reichel wrote:
> Hi,
>
> On Mon, Dec 15, 2025 at 03:21:19PM +0200, Matti Vaittinen wrote:
>> From: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> The ROHM BD72720 is a power management IC with a charger and coulomb
>> counter block which is closely related to the charger / coulomb counter
>> found from the BD71815, BD71828, BD71879 which are all supported by the
>> bd71828-power driver. Due to the similarities it makes sense to support
>> also the BD72720 with the same driver.
>>
>> Add basic support for the charger logic on ROHM BD72720.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> ---
>> Revision history:
>> v2 => :
>> - No changes
>>
>> RFCv1 => v2:
>> - Support using 9-bit register addresses (offset of 0x100) with the
>> BD72720
>> - Simplify probe and IC data as we don't need two regmaps
>> - Drop two BD72720 specific functions as we no longer need different
>> regmap for it.
>>
>> Note: This patch depends on the series: "power: supply: add charger for
>> BD71828" by Andreas:
>> https://lore.kernel.org/all/20250918-bd71828-charger-v5-0-851164839c28@kemnade.info/
>
> That should be in v6.19?
Ah, right. As Andreas also stated, yes. This is no longer relevant. I
simply forgot to clean-up the note from this patch.
>> @@ -958,21 +1043,27 @@ static int bd71828_power_probe(struct platform_device *pdev)
>> struct power_supply_config ac_cfg = {};
>> struct power_supply_config bat_cfg = {};
>> int ret;
>> - struct regmap *regmap;
>> -
>> - regmap = dev_get_regmap(pdev->dev.parent, NULL);
>> - if (!regmap) {
>> - dev_err(&pdev->dev, "No parent regmap\n");
>> - return -EINVAL;
>> - }
>>
>> pwr = devm_kzalloc(&pdev->dev, sizeof(*pwr), GFP_KERNEL);
>> if (!pwr)
>> return -ENOMEM;
>>
>> - pwr->regmap = regmap;
>> - pwr->dev = &pdev->dev;
>> + /*
>> + * The BD72720 MFD device registers two regmaps. Power-supply driver
>> + * uses the "wrap-map", which provides access to both of the I2C slave
>> + * addresses used by the BD72720
>> + */
>> pwr->chip_type = platform_get_device_id(pdev)->driver_data;
>> + if (pwr->chip_type != ROHM_CHIP_TYPE_BD72720)
>> + pwr->regmap = dev_get_regmap(pdev->dev.parent, NULL);
>> + else
>> + pwr->regmap = dev_get_regmap(pdev->dev.parent, "wrap-map");
>> + if (!pwr->regmap) {
>> + dev_err(&pdev->dev, "No parent regmap\n");
>> + return -EINVAL;
>> + }
>
> return dev_err_probe(&pdev->dev, -EINVAL, "No parent regmap\n");
>
> Otherwise LGTM.
Thanks Sebastian! I appreciate the review!
This driver uses dev_err_probe() only in cases where the error to be
returned is not hard-coded. The design dates back to when I was first
introduced to the dev_err_probe() - and using pattern like:
ret = -EINVAL;
if (ret == -EPROBE_DEFER)
...
(which results from calling the dev_err_probe with hard-coded error)
felt very repulsive to me.
I have since quit resisting the 'use dev_err_probe() for all probe error
paths' -policy (since Resistance is futile - and because there are other
benefits besides the deferred probe handling) - but I suppose we should
clean-up also the other similar cases in this driver (I see at least one
other occurrence in the chip_type-check below). Is it Ok to merge this
as is, and do a follow-up patch to clean-up all the occurrences? If yes,
then this might go "as-is" via MFD, together with the other stuff, right?
I can also re-spin this with the print fixed and:
- add new patch to fix the other occurrence.
- meld the fix for existing print in this patch.
Just please let me know your preference.
>> switch (pwr->chip_type) {
>> case ROHM_CHIP_TYPE_BD71828:
>> @@ -985,6 +1076,12 @@ static int bd71828_power_probe(struct platform_device *pdev)
>> pwr->get_temp = bd71815_get_temp;
>> pwr->regs = &pwr_regs_bd71815;
>> break;
>> + case ROHM_CHIP_TYPE_BD72720:
>> + pwr->bat_inserted = bd71828_bat_inserted;
>> + pwr->regs = &pwr_regs_bd72720;
>> + pwr->get_temp = bd71828_get_temp;
>> + dev_dbg(pwr->dev, "Found ROHM BD72720\n");
>> + break;
>> default:
>> dev_err(pwr->dev, "Unknown PMIC\n");
>> return -EINVAL;
The other occurrence --^
>> @@ -1030,6 +1127,7 @@ static int bd71828_power_probe(struct platform_device *pdev)
>> static const struct platform_device_id bd71828_charger_id[] = {
>> { "bd71815-power", ROHM_CHIP_TYPE_BD71815 },
>> { "bd71828-power", ROHM_CHIP_TYPE_BD71828 },
>> + { "bd72720-power", ROHM_CHIP_TYPE_BD72720 },
>> { },
>> };
>> MODULE_DEVICE_TABLE(platform, bd71828_charger_id);
>> --
>> 2.52.0
Yours,
-- Matti
--
---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* Re: [PATCH RESEND v6 16/17] power: supply: bd71828-power: Support ROHM BD72720
From: Andreas Kemnade @ 2026-01-12 6:11 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Matti Vaittinen, Matti Vaittinen, Lee Jones, Pavel Machek,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Mark Brown, Michael Turquette, Stephen Boyd, Linus Walleij,
Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc
In-Reply-To: <aWRERf70jg-IzqIx@venus>
On Mon, 12 Jan 2026 01:51:58 +0100
Sebastian Reichel <sebastian.reichel@collabora.com> wrote:
> Hi,
>
> On Mon, Dec 15, 2025 at 03:21:19PM +0200, Matti Vaittinen wrote:
> > From: Matti Vaittinen <mazziesaccount@gmail.com>
> >
> > The ROHM BD72720 is a power management IC with a charger and coulomb
> > counter block which is closely related to the charger / coulomb counter
> > found from the BD71815, BD71828, BD71879 which are all supported by the
> > bd71828-power driver. Due to the similarities it makes sense to support
> > also the BD72720 with the same driver.
> >
> > Add basic support for the charger logic on ROHM BD72720.
> >
> > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> >
> > ---
> > Revision history:
> > v2 => :
> > - No changes
> >
> > RFCv1 => v2:
> > - Support using 9-bit register addresses (offset of 0x100) with the
> > BD72720
> > - Simplify probe and IC data as we don't need two regmaps
> > - Drop two BD72720 specific functions as we no longer need different
> > regmap for it.
> >
> > Note: This patch depends on the series: "power: supply: add charger for
> > BD71828" by Andreas:
> > https://lore.kernel.org/all/20250918-bd71828-charger-v5-0-851164839c28@kemnade.info/
>
> That should be in v6.19?
>
yes, it is.
Just this note survived...
Regards,
Andreas
^ permalink raw reply
* Re: [PATCH v5 1/3] regulator: spacemit: MFD_SPACEMIT_P1 as dependencies
From: Troy Mitchell @ 2026-01-12 1:51 UTC (permalink / raw)
To: Alexandre Belloni, Troy Mitchell
Cc: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Liam Girdwood,
Mark Brown, linux-kernel, linux-riscv, spacemit, linux-i2c,
linux-rtc
In-Reply-To: <20260109224147267169ba@mail.local>
On Sat Jan 10, 2026 at 6:41 AM CST, Alexandre Belloni wrote:
> On 08/01/2026 16:38:54+0800, Troy Mitchell wrote:
>> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
>> index d2335276cce5ffbd500bbaf251d1761a9116aee9..b51888a9a78f399a6af3294fc19f60792576332c 100644
>> --- a/drivers/regulator/Kconfig
>> +++ b/drivers/regulator/Kconfig
>> @@ -1496,9 +1496,8 @@ config REGULATOR_SLG51000
>> config REGULATOR_SPACEMIT_P1
>> tristate "SpacemiT P1 regulators"
>> depends on ARCH_SPACEMIT || COMPILE_TEST
>> - depends on I2C
>> - select MFD_SPACEMIT_P1
>> - default ARCH_SPACEMIT
>> + depends on MFD_SPACEMIT_P1
>> + default m if MFD_SPACEMIT_P1
>
> default MFD_SPACEMIT_P1 is certainly enough here.
Yes, Thanks!
I'll use it in the next version.
- Troy
^ permalink raw reply
* Re: [PATCH v4 3/3] rtc: spacemit: default module when MFD_SPACEMIT_P1 is enabled
From: Troy Mitchell @ 2026-01-12 1:49 UTC (permalink / raw)
To: Alex Elder, Alexandre Belloni
Cc: Troy Mitchell, Lee Jones, Yixun Lan, Andi Shyti, Liam Girdwood,
Mark Brown, linux-kernel, linux-riscv, spacemit, linux-i2c,
linux-rtc
In-Reply-To: <a383dbc8-5d14-4654-933d-5dfa73a23b12@riscstar.com>
On Mon Jan 12, 2026 at 3:55 AM CST, Alex Elder wrote:
> On 1/9/26 4:36 PM, Alexandre Belloni wrote:
>>> The purpose is to make the driver a module (not built-in)
>>> when "defconfig" is used (without the need for any Kconfig
>>> fragments to unselect things).
>>>
>>>
>>> In arch/riscv/configs/defconfig, we have this:
>>> CONFIG_ARCH_SPACEMIT=y
>>>
>>> In drivers/mfd/Kconfig b/drivers/mfd/Kconfig, we have this
>>> (added by patch 2 in this series):
>>> config MFD_SPACEMIT_P1
>>> default m if ARCH_SPACEMIT
>>>
>>> So when using defconfig (alone), MFD_SPACEMIT_P1 is set to m,
>>> to benefit non-SpacemiT RISC-V platforms.
>>>
>>> This patch is trying to do the same thing for the RTC,
>>> i.e. having RTC_DRV_SPACEMIT_P1 be defined as a module
>>> by default.
>>>
>>> I think you understand.
>> I'm sorry, I must be dumb but I don't understand. The current behaviour
>
> I think I'm the dumb one. I think I finally understand your
> point.
>
>> is that when MFD_SPACEMIT_P1 is m, then the default value for
>> RTC_DRV_SPACEMIT_P1 will be m. Since patch 2 makes it exactly that way
>> (MFD_SPACEMIT_P set to m), I don't get why it is necessary to mess with
>> the default of RTC_DRV_SPACEMIT_P1.
>
> Your point is that patch has no real effect, at least not
> on the scenario I was talking about.
>
> I.e., I was saying this mattered for using defconfig alone.
> But, as you point out, using defconfig alone gives us:
>
> ARCH_SPACMIT=y (in defconfig)
> MFD_SPACEMIT_P1=m (from patch 2)
>
> And then, *without* this patch:
> RTC_DRV_SPACEMIT_P1=MFD_SPACEMIT_P1
> meaning
> RTC_DRV_SPACEMIT_P1=m
>
> And therefore there's no need for this patch to set the
> default to m rather than MFD_SPACEMIT_P1.
>
Thanks Alex and Alexandre for the clarification. You're absolutely right -
the patch is indeed redundant since RTC_DRV_SPACEMIT_P1 already inherits
the correct default value (m) through its dependency on MFD_SPACEMIT_P1.
I'll drop this patch in the next version and review the rest of the series
for similar unnecessary default overrides.
- Troy
>
>
>> The current default behaviour of RTC_DRV_SPACEMIT_P1 seems to be the
>> correct one and the proper fix is then patch 2.
>
> Yes, now I understand. I'm sorry about my confusion.
>
> -Alex
^ permalink raw reply
* Re: [PATCH RESEND v6 00/17] Support ROHM BD72720 PMIC
From: Sebastian Reichel @ 2026-01-12 0:53 UTC (permalink / raw)
To: Lee Jones
Cc: Matti Vaittinen, Matti Vaittinen, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Michael Turquette, Stephen Boyd, Linus Walleij,
Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
Andreas Kemnade
In-Reply-To: <20260109093831.GB1118061@google.com>
[-- Attachment #1: Type: text/plain, Size: 799 bytes --]
Hi,
On Fri, Jan 09, 2026 at 09:38:31AM +0000, Lee Jones wrote:
> [...]
> > > The MFD parts LGTM.
> >
> > Thanks Lee!
> >
> > > What Acks are you waiting on? What's the merge strategy?
> >
> > I think everything else has been acked by maintainers, except the
> > power-supply parts. I think those have only been looked at by Andreas and
> > Linus W. Haven't heard anything from Sebastian :(
Yes, I'm lacking behind quite a bit, sorry for that.
> > I would love to see the patches 1 - 14 and 17 to be merged (via MFD?). I
> > could then re-spin the 15 and 16 to limited audience as I hope Sebastian had
> > time to take a look at them. However, I don't think any of the other patches
> > in the series depend on the last .
Sounds good to me.
Greetings,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH RESEND v6 16/17] power: supply: bd71828-power: Support ROHM BD72720
From: Sebastian Reichel @ 2026-01-12 0:51 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Lee Jones, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Michael Turquette, Stephen Boyd, Linus Walleij,
Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
Andreas Kemnade
In-Reply-To: <fb74c0cab3dfe534135d26dbbb9c66699678c2de.1765804226.git.mazziesaccount@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 12564 bytes --]
Hi,
On Mon, Dec 15, 2025 at 03:21:19PM +0200, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> The ROHM BD72720 is a power management IC with a charger and coulomb
> counter block which is closely related to the charger / coulomb counter
> found from the BD71815, BD71828, BD71879 which are all supported by the
> bd71828-power driver. Due to the similarities it makes sense to support
> also the BD72720 with the same driver.
>
> Add basic support for the charger logic on ROHM BD72720.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>
> ---
> Revision history:
> v2 => :
> - No changes
>
> RFCv1 => v2:
> - Support using 9-bit register addresses (offset of 0x100) with the
> BD72720
> - Simplify probe and IC data as we don't need two regmaps
> - Drop two BD72720 specific functions as we no longer need different
> regmap for it.
>
> Note: This patch depends on the series: "power: supply: add charger for
> BD71828" by Andreas:
> https://lore.kernel.org/all/20250918-bd71828-charger-v5-0-851164839c28@kemnade.info/
That should be in v6.19?
> NOTE: Fuel-gauging is not supported. You can find an unmaintained
> downstream reference-driver with a fuel-gauge example from:
> https://github.com/RohmSemiconductor/Linux-Kernel-PMIC-Drivers/releases/tag/bd72720-reference-driver-v1
> ---
> drivers/power/supply/bd71828-power.c | 134 +++++++++++++++++++++++----
> 1 file changed, 116 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/power/supply/bd71828-power.c b/drivers/power/supply/bd71828-power.c
> index ce73c0f48397..438e220a9cb7 100644
> --- a/drivers/power/supply/bd71828-power.c
> +++ b/drivers/power/supply/bd71828-power.c
> @@ -5,6 +5,7 @@
> #include <linux/kernel.h>
> #include <linux/mfd/rohm-bd71815.h>
> #include <linux/mfd/rohm-bd71828.h>
> +#include <linux/mfd/rohm-bd72720.h>
> #include <linux/module.h>
> #include <linux/mod_devicetable.h>
> #include <linux/platform_device.h>
> @@ -51,12 +52,14 @@ struct pwr_regs {
> unsigned int chg_state;
> unsigned int bat_temp;
> unsigned int dcin_stat;
> + unsigned int dcin_online_mask;
> unsigned int dcin_collapse_limit;
> unsigned int chg_set1;
> unsigned int chg_en;
> unsigned int vbat_alm_limit_u;
> unsigned int conf;
> unsigned int vdcin;
> + unsigned int vdcin_himask;
> };
>
> static const struct pwr_regs pwr_regs_bd71828 = {
> @@ -67,12 +70,14 @@ static const struct pwr_regs pwr_regs_bd71828 = {
> .chg_state = BD71828_REG_CHG_STATE,
> .bat_temp = BD71828_REG_BAT_TEMP,
> .dcin_stat = BD71828_REG_DCIN_STAT,
> + .dcin_online_mask = BD7182x_MASK_DCIN_DET,
> .dcin_collapse_limit = BD71828_REG_DCIN_CLPS,
> .chg_set1 = BD71828_REG_CHG_SET1,
> .chg_en = BD71828_REG_CHG_EN,
> .vbat_alm_limit_u = BD71828_REG_ALM_VBAT_LIMIT_U,
> .conf = BD71828_REG_CONF,
> .vdcin = BD71828_REG_VDCIN_U,
> + .vdcin_himask = BD7182x_MASK_VDCIN_U,
> };
>
> static const struct pwr_regs pwr_regs_bd71815 = {
> @@ -85,6 +90,7 @@ static const struct pwr_regs pwr_regs_bd71815 = {
> .chg_state = BD71815_REG_CHG_STATE,
> .bat_temp = BD71815_REG_BAT_TEMP,
> .dcin_stat = BD71815_REG_DCIN_STAT,
> + .dcin_online_mask = BD7182x_MASK_DCIN_DET,
> .dcin_collapse_limit = BD71815_REG_DCIN_CLPS,
> .chg_set1 = BD71815_REG_CHG_SET1,
> .chg_en = BD71815_REG_CHG_SET1,
> @@ -92,6 +98,31 @@ static const struct pwr_regs pwr_regs_bd71815 = {
> .conf = BD71815_REG_CONF,
>
> .vdcin = BD71815_REG_VM_DCIN_U,
> + .vdcin_himask = BD7182x_MASK_VDCIN_U,
> +};
> +
> +static struct pwr_regs pwr_regs_bd72720 = {
> + .vbat_avg = BD72720_REG_VM_SA_VBAT_U,
> + .ibat = BD72720_REG_CC_CURCD_U,
> + .ibat_avg = BD72720_REG_CC_SA_CURCD_U,
> + .btemp_vth = BD72720_REG_VM_BTMP_U,
> + /*
> + * Note, state 0x40 IMP_CHK. not documented
> + * on other variants but was still handled in
> + * existing code. No memory traces as to why.
> + */
> + .chg_state = BD72720_REG_CHG_STATE,
> + .bat_temp = BD72720_REG_CHG_BAT_TEMP_STAT,
> + .dcin_stat = BD72720_REG_INT_VBUS_SRC,
> + .dcin_online_mask = BD72720_MASK_DCIN_DET,
> + .dcin_collapse_limit = -1, /* Automatic. Setting not supported */
> + .chg_set1 = BD72720_REG_CHG_SET_1,
> + .chg_en = BD72720_REG_CHG_EN,
> + /* 15mV note in data-sheet */
> + .vbat_alm_limit_u = BD72720_REG_ALM_VBAT_TH_U,
> + .conf = BD72720_REG_CONF, /* o XSTB, only PON. Seprate slave addr */
> + .vdcin = BD72720_REG_VM_VBUS_U, /* 10 bits not 11 as with other ICs */
> + .vdcin_himask = BD72720_MASK_VDCIN_U,
> };
>
> struct bd71828_power {
> @@ -298,7 +329,7 @@ static int get_chg_online(struct bd71828_power *pwr, int *chg_online)
> dev_err(pwr->dev, "Failed to read DCIN status\n");
> return ret;
> }
> - *chg_online = ((r & BD7182x_MASK_DCIN_DET) != 0);
> + *chg_online = ((r & pwr->regs->dcin_online_mask) != 0);
>
> return 0;
> }
> @@ -329,8 +360,8 @@ static int bd71828_bat_inserted(struct bd71828_power *pwr)
> ret = val & BD7182x_MASK_CONF_PON;
>
> if (ret)
> - regmap_update_bits(pwr->regmap, pwr->regs->conf,
> - BD7182x_MASK_CONF_PON, 0);
> + if (regmap_update_bits(pwr->regmap, pwr->regs->conf, BD7182x_MASK_CONF_PON, 0))
> + dev_err(pwr->dev, "Failed to write CONF register\n");
>
> return ret;
> }
> @@ -358,11 +389,13 @@ static int bd71828_init_hardware(struct bd71828_power *pwr)
> int ret;
>
> /* TODO: Collapse limit should come from device-tree ? */
> - ret = regmap_write(pwr->regmap, pwr->regs->dcin_collapse_limit,
> - BD7182x_DCIN_COLLAPSE_DEFAULT);
> - if (ret) {
> - dev_err(pwr->dev, "Failed to write DCIN collapse limit\n");
> - return ret;
> + if (pwr->regs->dcin_collapse_limit != (unsigned int)-1) {
> + ret = regmap_write(pwr->regmap, pwr->regs->dcin_collapse_limit,
> + BD7182x_DCIN_COLLAPSE_DEFAULT);
> + if (ret) {
> + dev_err(pwr->dev, "Failed to write DCIN collapse limit\n");
> + return ret;
> + }
> }
>
> ret = pwr->bat_inserted(pwr);
> @@ -419,7 +452,7 @@ static int bd71828_charger_get_property(struct power_supply *psy,
> break;
> case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> ret = bd7182x_read16_himask(pwr, pwr->regs->vdcin,
> - BD7182x_MASK_VDCIN_U, &tmp);
> + pwr->regs->vdcin_himask, &tmp);
> if (ret)
> return ret;
>
> @@ -630,6 +663,9 @@ BD_ISR_AC(dcin_ovp_det, "DCIN OVER VOLTAGE", true)
> BD_ISR_DUMMY(dcin_mon_det, "DCIN voltage below threshold")
> BD_ISR_DUMMY(dcin_mon_res, "DCIN voltage above threshold")
>
> +BD_ISR_DUMMY(vbus_curr_limit, "VBUS current limited")
> +BD_ISR_DUMMY(vsys_ov_res, "VSYS over-voltage cleared")
> +BD_ISR_DUMMY(vsys_ov_det, "VSYS over-voltage")
> BD_ISR_DUMMY(vsys_uv_res, "VSYS under-voltage cleared")
> BD_ISR_DUMMY(vsys_uv_det, "VSYS under-voltage")
> BD_ISR_DUMMY(vsys_low_res, "'VSYS low' cleared")
> @@ -878,6 +914,51 @@ static int bd7182x_get_irqs(struct platform_device *pdev,
> BDIRQ("bd71828-temp-125-over", bd71828_temp_vf125_det),
> BDIRQ("bd71828-temp-125-under", bd71828_temp_vf125_res),
> };
> + static const struct bd7182x_irq_res bd72720_irqs[] = {
> + BDIRQ("bd72720_int_vbus_rmv", BD_ISR_NAME(dcin_removed)),
> + BDIRQ("bd72720_int_vbus_det", bd7182x_dcin_detected),
> + BDIRQ("bd72720_int_vbus_mon_res", BD_ISR_NAME(dcin_mon_res)),
> + BDIRQ("bd72720_int_vbus_mon_det", BD_ISR_NAME(dcin_mon_det)),
> + BDIRQ("bd72720_int_vsys_mon_res", BD_ISR_NAME(vsys_mon_res)),
> + BDIRQ("bd72720_int_vsys_mon_det", BD_ISR_NAME(vsys_mon_det)),
> + BDIRQ("bd72720_int_vsys_uv_res", BD_ISR_NAME(vsys_uv_res)),
> + BDIRQ("bd72720_int_vsys_uv_det", BD_ISR_NAME(vsys_uv_det)),
> + BDIRQ("bd72720_int_vsys_lo_res", BD_ISR_NAME(vsys_low_res)),
> + BDIRQ("bd72720_int_vsys_lo_det", BD_ISR_NAME(vsys_low_det)),
> + BDIRQ("bd72720_int_vsys_ov_res", BD_ISR_NAME(vsys_ov_res)),
> + BDIRQ("bd72720_int_vsys_ov_det", BD_ISR_NAME(vsys_ov_det)),
> + BDIRQ("bd72720_int_bat_ilim", BD_ISR_NAME(vbus_curr_limit)),
> + BDIRQ("bd72720_int_chg_done", bd718x7_chg_done),
> + BDIRQ("bd72720_int_extemp_tout", BD_ISR_NAME(chg_wdg_temp)),
> + BDIRQ("bd72720_int_chg_wdt_exp", BD_ISR_NAME(chg_wdg)),
> + BDIRQ("bd72720_int_bat_mnt_out", BD_ISR_NAME(rechg_res)),
> + BDIRQ("bd72720_int_bat_mnt_in", BD_ISR_NAME(rechg_det)),
> + BDIRQ("bd72720_int_chg_trns", BD_ISR_NAME(chg_state_changed)),
> +
> + BDIRQ("bd72720_int_vbat_mon_res", BD_ISR_NAME(bat_mon_res)),
> + BDIRQ("bd72720_int_vbat_mon_det", BD_ISR_NAME(bat_mon)),
> + BDIRQ("bd72720_int_vbat_sht_res", BD_ISR_NAME(bat_short_res)),
> + BDIRQ("bd72720_int_vbat_sht_det", BD_ISR_NAME(bat_short)),
> + BDIRQ("bd72720_int_vbat_lo_res", BD_ISR_NAME(bat_low_res)),
> + BDIRQ("bd72720_int_vbat_lo_det", BD_ISR_NAME(bat_low)),
> + BDIRQ("bd72720_int_vbat_ov_res", BD_ISR_NAME(bat_ov_res)),
> + BDIRQ("bd72720_int_vbat_ov_det", BD_ISR_NAME(bat_ov)),
> + BDIRQ("bd72720_int_bat_rmv", BD_ISR_NAME(bat_removed)),
> + BDIRQ("bd72720_int_bat_det", BD_ISR_NAME(bat_det)),
> + BDIRQ("bd72720_int_dbat_det", BD_ISR_NAME(bat_dead)),
> + BDIRQ("bd72720_int_bat_temp_trns", BD_ISR_NAME(temp_transit)),
> + BDIRQ("bd72720_int_lobtmp_res", BD_ISR_NAME(temp_bat_low_res)),
> + BDIRQ("bd72720_int_lobtmp_det", BD_ISR_NAME(temp_bat_low)),
> + BDIRQ("bd72720_int_ovbtmp_res", BD_ISR_NAME(temp_bat_hi_res)),
> + BDIRQ("bd72720_int_ovbtmp_det", BD_ISR_NAME(temp_bat_hi)),
> + BDIRQ("bd72720_int_ocur1_res", BD_ISR_NAME(bat_oc1_res)),
> + BDIRQ("bd72720_int_ocur1_det", BD_ISR_NAME(bat_oc1)),
> + BDIRQ("bd72720_int_ocur2_res", BD_ISR_NAME(bat_oc2_res)),
> + BDIRQ("bd72720_int_ocur2_det", BD_ISR_NAME(bat_oc2)),
> + BDIRQ("bd72720_int_ocur3_res", BD_ISR_NAME(bat_oc3_res)),
> + BDIRQ("bd72720_int_ocur3_det", BD_ISR_NAME(bat_oc3)),
> + BDIRQ("bd72720_int_cc_mon2_det", BD_ISR_NAME(bat_cc_mon)),
> + };
> int num_irqs;
> const struct bd7182x_irq_res *irqs;
>
> @@ -890,6 +971,10 @@ static int bd7182x_get_irqs(struct platform_device *pdev,
> irqs = &bd71815_irqs[0];
> num_irqs = ARRAY_SIZE(bd71815_irqs);
> break;
> + case ROHM_CHIP_TYPE_BD72720:
> + irqs = &bd72720_irqs[0];
> + num_irqs = ARRAY_SIZE(bd72720_irqs);
> + break;
> default:
> return -EINVAL;
> }
> @@ -958,21 +1043,27 @@ static int bd71828_power_probe(struct platform_device *pdev)
> struct power_supply_config ac_cfg = {};
> struct power_supply_config bat_cfg = {};
> int ret;
> - struct regmap *regmap;
> -
> - regmap = dev_get_regmap(pdev->dev.parent, NULL);
> - if (!regmap) {
> - dev_err(&pdev->dev, "No parent regmap\n");
> - return -EINVAL;
> - }
>
> pwr = devm_kzalloc(&pdev->dev, sizeof(*pwr), GFP_KERNEL);
> if (!pwr)
> return -ENOMEM;
>
> - pwr->regmap = regmap;
> - pwr->dev = &pdev->dev;
> + /*
> + * The BD72720 MFD device registers two regmaps. Power-supply driver
> + * uses the "wrap-map", which provides access to both of the I2C slave
> + * addresses used by the BD72720
> + */
> pwr->chip_type = platform_get_device_id(pdev)->driver_data;
> + if (pwr->chip_type != ROHM_CHIP_TYPE_BD72720)
> + pwr->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + else
> + pwr->regmap = dev_get_regmap(pdev->dev.parent, "wrap-map");
> + if (!pwr->regmap) {
> + dev_err(&pdev->dev, "No parent regmap\n");
> + return -EINVAL;
> + }
return dev_err_probe(&pdev->dev, -EINVAL, "No parent regmap\n");
Otherwise LGTM.
Greetings,
-- Sebastian
> +
> + pwr->dev = &pdev->dev;
>
> switch (pwr->chip_type) {
> case ROHM_CHIP_TYPE_BD71828:
> @@ -985,6 +1076,12 @@ static int bd71828_power_probe(struct platform_device *pdev)
> pwr->get_temp = bd71815_get_temp;
> pwr->regs = &pwr_regs_bd71815;
> break;
> + case ROHM_CHIP_TYPE_BD72720:
> + pwr->bat_inserted = bd71828_bat_inserted;
> + pwr->regs = &pwr_regs_bd72720;
> + pwr->get_temp = bd71828_get_temp;
> + dev_dbg(pwr->dev, "Found ROHM BD72720\n");
> + break;
> default:
> dev_err(pwr->dev, "Unknown PMIC\n");
> return -EINVAL;
> @@ -1030,6 +1127,7 @@ static int bd71828_power_probe(struct platform_device *pdev)
> static const struct platform_device_id bd71828_charger_id[] = {
> { "bd71815-power", ROHM_CHIP_TYPE_BD71815 },
> { "bd71828-power", ROHM_CHIP_TYPE_BD71828 },
> + { "bd72720-power", ROHM_CHIP_TYPE_BD72720 },
> { },
> };
> MODULE_DEVICE_TABLE(platform, bd71828_charger_id);
> --
> 2.52.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH RESEND v6 15/17] power: supply: bd71828: Support wider register addresses
From: Sebastian Reichel @ 2026-01-12 0:45 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Lee Jones, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Michael Turquette, Stephen Boyd, Linus Walleij,
Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
Andreas Kemnade
In-Reply-To: <57c87f7e2082a666f0adeafcd11f673c0af7d326.1765804226.git.mazziesaccount@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2825 bytes --]
Hi,
On Mon, Dec 15, 2025 at 03:21:03PM +0200, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> The BD71828 power-supply driver assumes register addresses to be 8-bit.
> The new BD72720 will use stacked register maps to hide paging which is
> done using secondary I2C slave address. This requires use of 9-bit
> register addresses in the power-supply driver (added offset 0x100 to
> the 8-bit hardware register addresses).
>
> The cost is slightly used memory consumption as the members in the
> struct pwr_regs will be changed from u8 to unsigned int, which means 3
> byte increase / member / instance.
> This is currently 14 members (expected to possibly be increased when
> adding new variants / new functionality which may introduce new
> registers, but not expected to grow much) and 2 instances (will be 3
> instances when BD72720 gets added).
>
> So, even if the number of registers grew to 50 it'd be 150 bytes /
> instance. Assuming we eventually supported 5 variants, it'd be
> 5 * 150 bytes, which stays very reasonable considering systems we are
> dealing with.
>
> As a side note, we can reduce the "wasted space / member / instance" from
> 3 bytes to 1 byte, by using u16 instead of the unsigned int if needed. I
> rather use unsigned int to be initially prepared for devices with 32 bit
> registers if there is no need to count bytes.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Greetings,
-- Sebastian
> Revision history:
> v2 => :
> - No changes
>
> RFCv1 => v2:
> - New patch
> ---
> drivers/power/supply/bd71828-power.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/power/supply/bd71828-power.c b/drivers/power/supply/bd71828-power.c
> index f667baedeb77..ce73c0f48397 100644
> --- a/drivers/power/supply/bd71828-power.c
> +++ b/drivers/power/supply/bd71828-power.c
> @@ -44,19 +44,19 @@
> #define VBAT_LOW_TH 0x00D4
>
> struct pwr_regs {
> - u8 vbat_avg;
> - u8 ibat;
> - u8 ibat_avg;
> - u8 btemp_vth;
> - u8 chg_state;
> - u8 bat_temp;
> - u8 dcin_stat;
> - u8 dcin_collapse_limit;
> - u8 chg_set1;
> - u8 chg_en;
> - u8 vbat_alm_limit_u;
> - u8 conf;
> - u8 vdcin;
> + unsigned int vbat_avg;
> + unsigned int ibat;
> + unsigned int ibat_avg;
> + unsigned int btemp_vth;
> + unsigned int chg_state;
> + unsigned int bat_temp;
> + unsigned int dcin_stat;
> + unsigned int dcin_collapse_limit;
> + unsigned int chg_set1;
> + unsigned int chg_en;
> + unsigned int vbat_alm_limit_u;
> + unsigned int conf;
> + unsigned int vdcin;
> };
>
> static const struct pwr_regs pwr_regs_bd71828 = {
> --
> 2.52.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v5 3/3] rtc: spacemit: default module when MFD_SPACEMIT_P1 is enabled
From: Alex Elder @ 2026-01-11 19:58 UTC (permalink / raw)
To: Alexandre Belloni, Troy Mitchell
Cc: Lee Jones, Yixun Lan, Andi Shyti, Liam Girdwood, Mark Brown,
linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc
In-Reply-To: <20260109223839b59ca7ce@mail.local>
On 1/9/26 4:38 PM, Alexandre Belloni wrote:
> On 08/01/2026 16:38:56+0800, Troy Mitchell wrote:
>> The RTC driver defaulted to the same value as MFD_SPACEMIT_P1, which
>> caused it to be built-in automatically whenever the PMIC support was
>> set to y.
>>
>> This is not always desirable, as the RTC function is not required on
>> all platforms using the SpacemiT P1 PMIC.
>>
>> Acked-by: Alex Elder <elder@riscstar.com>
>> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
>> ---
>> Change log in v5:
>> - add Alex's tag
>> - Link to v4: https://lore.kernel.org/all/20251225-p1-kconfig-fix-v4-3-44b6728117c1@linux.spacemit.com/
>
> See my reply on v4, this is still a NAK for me, I don't believe this
> change is necessary as soon as the default for MFD_SPACEMIT_P1 is m.
Yes I have reconsidered what Alexandre was saying and now I
agree with him, I don't believe this patch is required.
-Alex
>> ---
>> drivers/rtc/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
>> index 50dc779f7f983074df7882200c90f0df21d142f2..53866493e9bbaf35ff0de85cbfe43e8343eadc1e 100644
>> --- a/drivers/rtc/Kconfig
>> +++ b/drivers/rtc/Kconfig
>> @@ -410,7 +410,7 @@ config RTC_DRV_SPACEMIT_P1
>> tristate "SpacemiT P1 RTC"
>> depends on ARCH_SPACEMIT || COMPILE_TEST
>> depends on MFD_SPACEMIT_P1
>> - default MFD_SPACEMIT_P1
>> + default m if MFD_SPACEMIT_P1
>> help
>> Enable support for the RTC function in the SpacemiT P1 PMIC.
>> This driver can also be built as a module, which will be called
>>
>> --
>> 2.52.0
>>
^ permalink raw reply
* Re: [PATCH v4 3/3] rtc: spacemit: default module when MFD_SPACEMIT_P1 is enabled
From: Alex Elder @ 2026-01-11 19:55 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Troy Mitchell, Lee Jones, Yixun Lan, Andi Shyti, Liam Girdwood,
Mark Brown, linux-kernel, linux-riscv, spacemit, linux-i2c,
linux-rtc
In-Reply-To: <20260109223627b566d2b0@mail.local>
On 1/9/26 4:36 PM, Alexandre Belloni wrote:
>> The purpose is to make the driver a module (not built-in)
>> when "defconfig" is used (without the need for any Kconfig
>> fragments to unselect things).
>>
>>
>> In arch/riscv/configs/defconfig, we have this:
>> CONFIG_ARCH_SPACEMIT=y
>>
>> In drivers/mfd/Kconfig b/drivers/mfd/Kconfig, we have this
>> (added by patch 2 in this series):
>> config MFD_SPACEMIT_P1
>> default m if ARCH_SPACEMIT
>>
>> So when using defconfig (alone), MFD_SPACEMIT_P1 is set to m,
>> to benefit non-SpacemiT RISC-V platforms.
>>
>> This patch is trying to do the same thing for the RTC,
>> i.e. having RTC_DRV_SPACEMIT_P1 be defined as a module
>> by default.
>>
>> I think you understand.
> I'm sorry, I must be dumb but I don't understand. The current behaviour
I think I'm the dumb one. I think I finally understand your
point.
> is that when MFD_SPACEMIT_P1 is m, then the default value for
> RTC_DRV_SPACEMIT_P1 will be m. Since patch 2 makes it exactly that way
> (MFD_SPACEMIT_P set to m), I don't get why it is necessary to mess with
> the default of RTC_DRV_SPACEMIT_P1.
Your point is that patch has no real effect, at least not
on the scenario I was talking about.
I.e., I was saying this mattered for using defconfig alone.
But, as you point out, using defconfig alone gives us:
ARCH_SPACMIT=y (in defconfig)
MFD_SPACEMIT_P1=m (from patch 2)
And then, *without* this patch:
RTC_DRV_SPACEMIT_P1=MFD_SPACEMIT_P1
meaning
RTC_DRV_SPACEMIT_P1=m
And therefore there's no need for this patch to set the
default to m rather than MFD_SPACEMIT_P1.
> The current default behaviour of RTC_DRV_SPACEMIT_P1 seems to be the
> correct one and the proper fix is then patch 2.
Yes, now I understand. I'm sorry about my confusion.
-Alex
^ permalink raw reply
* Re: (subset) [PATCH 00/27] clk: remove deprecated API divider_round_rate() and friends
From: Bjorn Andersson @ 2026-01-10 19:11 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney
Cc: linux-clk, linux-kernel, Chen Wang, Inochi Amaoto, sophgo,
Chen-Yu Tsai, Maxime Ripard, Jernej Skrabec, Samuel Holland,
linux-arm-kernel, linux-sunxi, Alexandre Belloni, linux-rtc,
Andreas Färber, Manivannan Sadhasivam, linux-actions,
Keguang Zhang, linux-mips, Taichi Sugaya, Takao Orito,
Jacky Huang, Shan-Chun Hung, Vladimir Zapolskiy,
Piotr Wojtaszczyk, linux-arm-msm, Orson Zhai, Baolin Wang,
Chunyan Zhang, Maxime Coquelin, Alexandre Torgue, linux-stm32,
Michal Simek, Rob Clark, Dmitry Baryshkov, David Airlie,
Simona Vetter, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, dri-devel, freedreno, Vinod Koul, Neil Armstrong,
linux-phy
In-Reply-To: <20260108-clk-divider-round-rate-v1-0-535a3ed73bf3@redhat.com>
On Thu, 08 Jan 2026 16:16:18 -0500, Brian Masney wrote:
> Here's a series that gets rid of the deprecated APIs
> divider_round_rate(), divider_round_rate_parent(), and
> divider_ro_round_rate_parent() since these functions are just wrappers
> for the determine_rate variant.
>
> Note that when I converted some of these drivers from round_rate to
> determine_rate, this was mistakenly converted to the following in some
> cases:
>
> [...]
Applied, thanks!
[14/27] clk: qcom: alpha-pll: convert from divider_round_rate() to divider_determine_rate()
commit: e1f08613e113f02a3ec18c9a7964de97f940acbf
[15/27] clk: qcom: regmap-divider: convert from divider_ro_round_rate() to divider_ro_determine_rate()
commit: 35a48f41b63f67c490f3a2a89b042536be67cf0f
[16/27] clk: qcom: regmap-divider: convert from divider_round_rate() to divider_determine_rate()
commit: b2f36d675e09299d9aee395c6f83d8a95d4c9441
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: [PATCH v5 1/3] regulator: spacemit: MFD_SPACEMIT_P1 as dependencies
From: Alexandre Belloni @ 2026-01-09 22:41 UTC (permalink / raw)
To: Troy Mitchell
Cc: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Liam Girdwood,
Mark Brown, linux-kernel, linux-riscv, spacemit, linux-i2c,
linux-rtc
In-Reply-To: <20260108-p1-kconfig-fix-v5-1-6fe19f460269@linux.spacemit.com>
On 08/01/2026 16:38:54+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.
>
> Since MFD_SPACEMIT_P1 already depends on I2C_K1, the dependency
> in REGULATOR_SPACEMIT_P1 is now redundant.
>
> Additionally, the default value depends on MFD_SPACEMIT_P1 rather
> than ARCH_SPACEMIT.
>
> Acked-by: Mark Brown <broonie@kernel.org>
> Acked-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
> Change log in v5:
> - nothing
> - Link to v4: https://lore.kernel.org/all/20251225-p1-kconfig-fix-v4-1-44b6728117c1@linux.spacemit.com/
>
> Change log in v4:
> - default m if MFD_SPACEMIT_P1 instead of default MFD_SPACEMIT_P1
> Link to v3: https://lore.kernel.org/all/20251118-p1-kconfig-fix-v3-3-8839c5ac5db3@linux.spacemit.com/
>
> Changelog in v3:
> - modify commit message
> - change default value from ARCH_SPACEMIT to MFD_SPACEMIT_P1
> - Link to v2: https://lore.kernel.org/all/20251027-p1-kconfig-fix-v2-4-49688f30bae8@linux.spacemit.com/
> ---
> drivers/regulator/Kconfig | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index d2335276cce5ffbd500bbaf251d1761a9116aee9..b51888a9a78f399a6af3294fc19f60792576332c 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -1496,9 +1496,8 @@ config REGULATOR_SLG51000
> config REGULATOR_SPACEMIT_P1
> tristate "SpacemiT P1 regulators"
> depends on ARCH_SPACEMIT || COMPILE_TEST
> - depends on I2C
> - select MFD_SPACEMIT_P1
> - default ARCH_SPACEMIT
> + depends on MFD_SPACEMIT_P1
> + default m if MFD_SPACEMIT_P1
default MFD_SPACEMIT_P1 is certainly enough here.
^ permalink raw reply
* Re: [PATCH v5 3/3] rtc: spacemit: default module when MFD_SPACEMIT_P1 is enabled
From: Alexandre Belloni @ 2026-01-09 22:38 UTC (permalink / raw)
To: Troy Mitchell
Cc: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Liam Girdwood,
Mark Brown, linux-kernel, linux-riscv, spacemit, linux-i2c,
linux-rtc
In-Reply-To: <20260108-p1-kconfig-fix-v5-3-6fe19f460269@linux.spacemit.com>
On 08/01/2026 16:38:56+0800, Troy Mitchell wrote:
> The RTC driver defaulted to the same value as MFD_SPACEMIT_P1, which
> caused it to be built-in automatically whenever the PMIC support was
> set to y.
>
> This is not always desirable, as the RTC function is not required on
> all platforms using the SpacemiT P1 PMIC.
>
> Acked-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
> Change log in v5:
> - add Alex's tag
> - Link to v4: https://lore.kernel.org/all/20251225-p1-kconfig-fix-v4-3-44b6728117c1@linux.spacemit.com/
See my reply on v4, this is still a NAK for me, I don't believe this
change is necessary as soon as the default for MFD_SPACEMIT_P1 is m.
> ---
> drivers/rtc/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 50dc779f7f983074df7882200c90f0df21d142f2..53866493e9bbaf35ff0de85cbfe43e8343eadc1e 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -410,7 +410,7 @@ config RTC_DRV_SPACEMIT_P1
> tristate "SpacemiT P1 RTC"
> depends on ARCH_SPACEMIT || COMPILE_TEST
> depends on MFD_SPACEMIT_P1
> - default MFD_SPACEMIT_P1
> + default m if MFD_SPACEMIT_P1
> help
> Enable support for the RTC function in the SpacemiT P1 PMIC.
> This driver can also be built as a module, which will be called
>
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v4 3/3] rtc: spacemit: default module when MFD_SPACEMIT_P1 is enabled
From: Alexandre Belloni @ 2026-01-09 22:36 UTC (permalink / raw)
To: Alex Elder
Cc: Troy Mitchell, Lee Jones, Yixun Lan, Andi Shyti, Liam Girdwood,
Mark Brown, linux-kernel, linux-riscv, spacemit, linux-i2c,
linux-rtc
In-Reply-To: <6ca28183-1687-4ddc-8b3c-5e5be4255561@riscstar.com>
On 29/12/2025 19:46:59-0600, Alex Elder wrote:
> On 12/29/25 6:51 PM, Alexandre Belloni wrote:
> > On 29/12/2025 12:02:23-0600, Alex Elder wrote:
> > > On 12/25/25 10:53 AM, Alexandre Belloni wrote:
> > > > On 25/12/2025 15:46:33+0800, Troy Mitchell wrote:
> > > > > The RTC driver defaulted to the same value as MFD_SPACEMIT_P1, which
> > > > > caused it to be built-in automatically whenever the PMIC support was
> > > > > set to y.
> > > > >
> > > > > This is not always desirable, as the RTC function is not required on
> > > > > all platforms using the SpacemiT P1 PMIC.
> > > >
> > > > But then, can't people simply change the config? I don't feel like
> > > > this is an improvement.
> > >
> > > It's not an improvement for people who want to use the SpacemiT
> > > P1 PMIC, but it's an improvement for all the other RISC-V builds
> > > using "defconfig" that would rather have that support be modular
> > > to avoid needlessly consuming resources.
> >
> > But then, wouldn't MFD_SPACEMIT_P1 be simply not set or set to m ? So
> > this doesn't have any impact on other RISC-V builds while it makes
> > people using the SpacemiT P1 PMIC jump through hoops to be able to use
> > the RTC as this is a very uncommon way to set default values.
> >
> > My point is:
> > - other RISC-V platforms would simply not select MFD_SPACEMIT_P1 or
> > have MFD_SPACEMIT_P1 set to m
> > - having RTC_DRV_SPACEMIT_P1 built-in by default when MFD_SPACEMIT_P1
> > is built-in doesn't really hurt any SpacemiT P1 users but would be
> > the expectation of those using the RTC.
>
> The "hurt" isn't about P1 users, it's about everyone else.
>
> > - those wanting to optimise because they won't use the RTC, they can
> > already simply unselect RTC_DRV_SPACEMIT_P1 or set it to m.
>
>
> The purpose is to make the driver a module (not built-in)
> when "defconfig" is used (without the need for any Kconfig
> fragments to unselect things).
>
>
> In arch/riscv/configs/defconfig, we have this:
> CONFIG_ARCH_SPACEMIT=y
>
> In drivers/mfd/Kconfig b/drivers/mfd/Kconfig, we have this
> (added by patch 2 in this series):
> config MFD_SPACEMIT_P1
> default m if ARCH_SPACEMIT
>
> So when using defconfig (alone), MFD_SPACEMIT_P1 is set to m,
> to benefit non-SpacemiT RISC-V platforms.
>
> This patch is trying to do the same thing for the RTC,
> i.e. having RTC_DRV_SPACEMIT_P1 be defined as a module
> by default.
>
> I think you understand.
I'm sorry, I must be dumb but I don't understand. The current behaviour
is that when MFD_SPACEMIT_P1 is m, then the default value for
RTC_DRV_SPACEMIT_P1 will be m. Since patch 2 makes it exactly that way
(MFD_SPACEMIT_P set to m), I don't get why it is necessary to mess with
the default of RTC_DRV_SPACEMIT_P1.
The current default behaviour of RTC_DRV_SPACEMIT_P1 seems to be the
correct one and the proper fix is then patch 2.
^ permalink raw reply
* Re: [PATCH 08/13] rtc: pic32: update include to use pic32.h from platform_data
From: Alexandre Belloni @ 2026-01-09 17:42 UTC (permalink / raw)
To: Brian Masney
Cc: Thomas Bogendoerfer, Claudiu Beznea, linux-mips, linux-kernel,
linux-rtc
In-Reply-To: <20260109-mips-pic32-header-move-v1-8-99859c55783d@redhat.com>
On 09/01/2026 11:41:21-0500, Brian Masney wrote:
> Use the linux/platform_data/pic32.h include instead of
> asm/mach-pic32/pic32.h so that the asm variant can be dropped. This
> is in preparation for allowing some drivers to be compiled on other
> architectures with COMPILE_TEST enabled.
>
> Signed-off-by: Brian Masney <bmasney@redhat.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>
> ---
> To: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: linux-rtc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/rtc/rtc-pic32.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/rtc/rtc-pic32.c b/drivers/rtc/rtc-pic32.c
> index 52c11532bc3a3696359ca56349b42860aa90c966..3c7a38a4ac08eb0f5a44ae4e470c208a9d1dd599 100644
> --- a/drivers/rtc/rtc-pic32.c
> +++ b/drivers/rtc/rtc-pic32.c
> @@ -15,8 +15,7 @@
> #include <linux/clk.h>
> #include <linux/rtc.h>
> #include <linux/bcd.h>
> -
> -#include <asm/mach-pic32/pic32.h>
> +#include <linux/platform_data/pic32.h>
>
> #define PIC32_RTCCON 0x00
> #define PIC32_RTCCON_ON BIT(15)
>
> --
> 2.52.0
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 00/13] MIPS: move pic32.h header file from asm to platform_data
From: Brian Masney @ 2026-01-09 17:14 UTC (permalink / raw)
To: Thomas Bogendoerfer, Claudiu Beznea
Cc: linux-mips, linux-kernel, Michael Turquette, Stephen Boyd,
linux-clk, Thomas Gleixner, Adrian Hunter, Ulf Hansson, linux-mmc,
Linus Walleij, linux-gpio, Alexandre Belloni, linux-rtc,
Greg Kroah-Hartman, Jiri Slaby, linux-serial, Wim Van Sebroeck,
Guenter Roeck, linux-watchdog
In-Reply-To: <20260109-mips-pic32-header-move-v1-0-99859c55783d@redhat.com>
On Fri, Jan 09, 2026 at 11:41:13AM -0500, Brian Masney wrote:
> There are currently some pic32 MIPS drivers that are in tree, and are
> only configured to be compiled on the MIPS pic32 platform. There's a
> risk of breaking some of these drivers when migrating drivers away from
> legacy APIs. It happened to me with a pic32 clk driver.
>
> Let's go ahead and move the pic32.h from the asm to the platform_data
> include directory in the tree. This will make it easier, and cleaner to
> enable COMPILE_TEST for some of these pic32 drivers.
>
> I included a patch at the end that shows enabling COMPILE_TEST for a
> pic32 clk driver.
I didn't CC everyone on patch 1 to this series that copes pic32.h from
the MIPS ASM directory to linux/platform_data/pic32.h. It's available at
the following location if you want to see it:
https://lore.kernel.org/linux-mips/20260109-mips-pic32-header-move-v1-0-99859c55783d@redhat.com/T/#m1e0e50adfe2ea4bf430025660fada7b1468d0fbf
Patch 12 of this series is where I remove the asm variant of pic32.h.
Brian
^ permalink raw reply
* [PATCH 08/13] rtc: pic32: update include to use pic32.h from platform_data
From: Brian Masney @ 2026-01-09 16:41 UTC (permalink / raw)
To: Thomas Bogendoerfer, Claudiu Beznea
Cc: linux-mips, linux-kernel, Brian Masney, Alexandre Belloni,
linux-rtc
In-Reply-To: <20260109-mips-pic32-header-move-v1-0-99859c55783d@redhat.com>
Use the linux/platform_data/pic32.h include instead of
asm/mach-pic32/pic32.h so that the asm variant can be dropped. This
is in preparation for allowing some drivers to be compiled on other
architectures with COMPILE_TEST enabled.
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: linux-rtc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
| 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
--git a/drivers/rtc/rtc-pic32.c b/drivers/rtc/rtc-pic32.c
index 52c11532bc3a3696359ca56349b42860aa90c966..3c7a38a4ac08eb0f5a44ae4e470c208a9d1dd599 100644
--- a/drivers/rtc/rtc-pic32.c
+++ b/drivers/rtc/rtc-pic32.c
@@ -15,8 +15,7 @@
#include <linux/clk.h>
#include <linux/rtc.h>
#include <linux/bcd.h>
-
-#include <asm/mach-pic32/pic32.h>
+#include <linux/platform_data/pic32.h>
#define PIC32_RTCCON 0x00
#define PIC32_RTCCON_ON BIT(15)
--
2.52.0
^ permalink raw reply related
* [PATCH 00/13] MIPS: move pic32.h header file from asm to platform_data
From: Brian Masney @ 2026-01-09 16:41 UTC (permalink / raw)
To: Thomas Bogendoerfer, Claudiu Beznea
Cc: linux-mips, linux-kernel, Brian Masney, Michael Turquette,
Stephen Boyd, linux-clk, Thomas Gleixner, Adrian Hunter,
Ulf Hansson, linux-mmc, Linus Walleij, linux-gpio,
Alexandre Belloni, linux-rtc, Greg Kroah-Hartman, Jiri Slaby,
linux-serial, Wim Van Sebroeck, Guenter Roeck, linux-watchdog
There are currently some pic32 MIPS drivers that are in tree, and are
only configured to be compiled on the MIPS pic32 platform. There's a
risk of breaking some of these drivers when migrating drivers away from
legacy APIs. It happened to me with a pic32 clk driver.
Let's go ahead and move the pic32.h from the asm to the platform_data
include directory in the tree. This will make it easier, and cleaner to
enable COMPILE_TEST for some of these pic32 drivers.
I included a patch at the end that shows enabling COMPILE_TEST for a
pic32 clk driver.
Merge Strategy
==============
- Patches 1-12 can go through the MIPS tree.
- Patch 13 I can repost to Claudiu after patches 1-12 are in Linus's
tree after the next merge window. There is a separate patch set that
fixes a compiler error I unintentionally introduced via the clk tree.
https://lore.kernel.org/linux-clk/CABx5tq+eOocJ41X-GSgkGy6S+s+Am1yCS099wqP695NtwALTmg@mail.gmail.com/T/
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
Brian Masney (13):
MIPS: copy pic32.h header file from asm/mach-pic32/ to include/platform-data/
MAINTAINERS: add include/linux/platform_data/pic32.h to MIPS entry
MIPS: update include to use pic32.h from platform_data
clk: microchip: core: update include to use pic32.h from platform_data
irqchip/irq-pic32-evic: update include to use pic32.h from platform_data
mmc: sdhci-pic32: update include to use pic32.h from platform_data
pinctrl: pic32: update include to use pic32.h from platform_data
rtc: pic32: update include to use pic32.h from platform_data
serial: pic32_uart: update include to use pic32.h from platform_data
watchdog: pic32-dmt: update include to use pic32.h from platform_data
watchdog: pic32-wdt: update include to use pic32.h from platform_data
MIPS: drop unused pic32.h header
clk: microchip: core: allow driver to be compiled with COMPILE_TEST
MAINTAINERS | 1 +
arch/mips/pic32/common/reset.c | 2 +-
arch/mips/pic32/pic32mzda/config.c | 3 +--
arch/mips/pic32/pic32mzda/early_clk.c | 2 +-
arch/mips/pic32/pic32mzda/early_console.c | 2 +-
drivers/clk/microchip/Kconfig | 2 +-
drivers/clk/microchip/clk-core.c | 6 +++++-
drivers/irqchip/irq-pic32-evic.c | 2 +-
drivers/mmc/host/sdhci-pic32.c | 2 +-
drivers/pinctrl/pinctrl-pic32.c | 3 +--
drivers/rtc/rtc-pic32.c | 3 +--
drivers/tty/serial/pic32_uart.c | 3 +--
drivers/watchdog/pic32-dmt.c | 3 +--
drivers/watchdog/pic32-wdt.c | 3 +--
.../mach-pic32 => include/linux/platform_data}/pic32.h | 17 +++++++++--------
15 files changed, 27 insertions(+), 27 deletions(-)
---
base-commit: f417b7ffcbef7d76b0d8860518f50dae0e7e5eda
change-id: 20260109-mips-pic32-header-move-6ac9965aa70a
Best regards,
--
Brian Masney <bmasney@redhat.com>
^ permalink raw reply
* Re: (subset) [PATCH v4 2/3] mfd: simple-mfd-i2c: add default value
From: Lee Jones @ 2026-01-09 16:41 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
In-Reply-To: <20251225-p1-kconfig-fix-v4-2-44b6728117c1@linux.spacemit.com>
On Thu, 25 Dec 2025 15:46:32 +0800, Troy Mitchell wrote:
> The default value of the P1 sub-device depends on the value
> of P1, so P1 should have a default value here.
>
>
Applied, thanks!
[2/3] mfd: simple-mfd-i2c: add default value
commit: 77df11d1f1f962636e897f3fcf0977109aa74791
--
Lee Jones [李琼斯]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox