* 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
* Re: [PATCH] rtc: interface: Alarm race handling should not discard preceding error
From: Thorsten Leemhuis @ 2026-01-09 13:04 UTC (permalink / raw)
To: Esben Haabendal
Cc: alexandre.belloni@bootlin.com, Nick Bowler,
Anthony Pighin (Nokia), linux-rtc@vger.kernel.org,
Linux kernel regressions list
In-Reply-To: <87o6n3c5fh.fsf@geanix.com>
On 1/9/26 13:54, Esben Haabendal wrote:
> "Thorsten Leemhuis" <regressions@leemhuis.info> writes:
>
>> On 12/16/25 16:09, Nick Bowler wrote:
>>> On Tue, Dec 16, 2025 at 11:51:30AM +0100, Thorsten Leemhuis wrote:
>>>> Lo! FWIW, Nick Bowler (now CCed) reported that below patch fixes a
>>>> regression for him caused by the commit from Esben (now also CCed) the
>>>> Fixes: tag mentions. See "hwclock busted w/ M48T59 RTC (regression)" for
>>>> details:
>>>> https://lore.kernel.org/all/2t6bhs4udbu55ctbemkhlluchz2exrwown7kmu2gss6zukaxdm@ughygemahmem/
>>>> and
>>>>
>>>> Nick, could you maybe provide a tested-by tag here? Maybe that would
>>>> motivate someone to get this en route to mainline.
>>>>
>>>> Adding a "Cc: <stable@vger.kernel.org>" would be great, too, as Nick
>>>> encountered this on earlier series, as it was backported all the way to
>>>> 5.15.y
>>>
>>> It was backported to 5.10.y and 5.4.y too, but only after I had reported
>>> this regression back in October (and I guess 5.4.y is EOL now).
>>>
>>> Tested-by: Nick Bowler <nbowler@draconx.ca>
>>
>> Esben (author of the culprit) and Alexandre (who merged it): do you
>> still have reviewing/applying the patch at the start of this thread (
>> https://lore.kernel.org/all/BN0PR08MB6951415A751F236375A2945683D1A@BN0PR08MB6951.namprd08.prod.outlook.com/#t)
>> on your todo list?
>>
>> Sorry for nagging, would just be good to finally get this 6.18-rc1
>> regression fixed in mainline so the fix can be backported to stable,
>> too. And due to the holidays I thought a quick reminder might be wise.
>>
>> Or was this fixed somehow and I just missed it?
>
> Sorry for introducing this error.
Happens, no worries.
> The fix looks good to me. I have added my Reviewed-by to it.
Thx!
> Am I correct in that it also fixes the problem reported by Nick Bowler?
Yes, see the "Tested-by" quoted above.
Ciao, Thorsten
^ permalink raw reply
* Re: [PATCH] rtc: interface: Alarm race handling should not discard preceding error
From: Esben Haabendal @ 2026-01-09 12:54 UTC (permalink / raw)
To: Thorsten Leemhuis
Cc: alexandre.belloni@bootlin.com, Nick Bowler,
Anthony Pighin (Nokia), linux-rtc@vger.kernel.org,
Linux kernel regressions list
In-Reply-To: <45d14eb4-9495-4aa8-9382-8d756c0ae39e@leemhuis.info>
"Thorsten Leemhuis" <regressions@leemhuis.info> writes:
> On 12/16/25 16:09, Nick Bowler wrote:
>> On Tue, Dec 16, 2025 at 11:51:30AM +0100, Thorsten Leemhuis wrote:
>>> Lo! FWIW, Nick Bowler (now CCed) reported that below patch fixes a
>>> regression for him caused by the commit from Esben (now also CCed) the
>>> Fixes: tag mentions. See "hwclock busted w/ M48T59 RTC (regression)" for
>>> details:
>>> https://lore.kernel.org/all/2t6bhs4udbu55ctbemkhlluchz2exrwown7kmu2gss6zukaxdm@ughygemahmem/
>>> and
>>>
>>> Nick, could you maybe provide a tested-by tag here? Maybe that would
>>> motivate someone to get this en route to mainline.
>>>
>>> Adding a "Cc: <stable@vger.kernel.org>" would be great, too, as Nick
>>> encountered this on earlier series, as it was backported all the way to
>>> 5.15.y
>>
>> It was backported to 5.10.y and 5.4.y too, but only after I had reported
>> this regression back in October (and I guess 5.4.y is EOL now).
>>
>> Tested-by: Nick Bowler <nbowler@draconx.ca>
>
> Esben (author of the culprit) and Alexandre (who merged it): do you
> still have reviewing/applying the patch at the start of this thread (
> https://lore.kernel.org/all/BN0PR08MB6951415A751F236375A2945683D1A@BN0PR08MB6951.namprd08.prod.outlook.com/#t)
> on your todo list?
>
> Sorry for nagging, would just be good to finally get this 6.18-rc1
> regression fixed in mainline so the fix can be backported to stable,
> too. And due to the holidays I thought a quick reminder might be wise.
>
> Or was this fixed somehow and I just missed it?
Sorry for introducing this error. The fix looks good to me. I have added
my Reviewed-by to it.
Am I correct in that it also fixes the problem reported by Nick Bowler?
/Esben
^ permalink raw reply
* Re: [PATCH] rtc: interface: Alarm race handling should not discard preceding error
From: Esben Haabendal @ 2026-01-09 12:50 UTC (permalink / raw)
To: Anthony Pighin (Nokia)
Cc: linux-rtc@vger.kernel.org, alexandre.belloni@bootlin.com
In-Reply-To: <BN0PR08MB6951415A751F236375A2945683D1A@BN0PR08MB6951.namprd08.prod.outlook.com>
On Tuesday, November 25th, 2025 at 18:35, Anthony Pighin (Nokia) <anthony.pighin@nokia.com> wrote:
> Commit 795cda8338ea ("rtc: interface: Fix long-standing race when setting
> alarm") should not discard any errors from the preceding validations.
>
> Prior to that commit, if the alarm feature was disabled, or the
> set_alarm failed, a meaningful error code would be returned to the
> caller for further action.
>
> After, more often than not, the __rtc_read_time will cause a success
> return code instead, misleading the caller.
>
> An example of this is when timer_enqueue is called for a rtc-abx080x
> device. Since that driver does not clear the alarm feature bit, but
> instead relies on the set_alarm operation to return invalid, the discard
> of the return code causes very different behaviour; i.e.
> hwclock: select() to /dev/rtc0 to wait for clock tick timed out
>
> Fixes: 795cda8338ea ("rtc: interface: Fix long-standing race when setting alarm")
> Signed-off-by: Anthony Pighin anthony.pighin@nokia.com
>
> ---
> drivers/rtc/interface.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index b8b298efd9a9..1906f4884a83 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -457,7 +457,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
> * are in, we can return -ETIME to signal that the timer has already
> * expired, which is true in both cases.
> */
> - if ((scheduled - now) <= 1) {
> + if (!err && (scheduled - now) <= 1) {
> err = __rtc_read_time(rtc, &tm);
> if (err)
> return err;
> --
> 2.43.0
Sorry about the slow response. And thanks for the fix :)
Reviewed-by: Esben Haabendal <esben@geanix.com>
^ permalink raw reply
* Re: [PATCH] rtc: interface: Alarm race handling should not discard preceding error
From: Thorsten Leemhuis @ 2026-01-09 12:18 UTC (permalink / raw)
To: alexandre.belloni@bootlin.com, Esben Haabendal
Cc: Nick Bowler, Anthony Pighin (Nokia), linux-rtc@vger.kernel.org,
Linux kernel regressions list
In-Reply-To: <g3phtogna3a55vzah6olpxekdcmi322q5lzzwxwq5za4oi4plr@js34hr72bemi>
On 12/16/25 16:09, Nick Bowler wrote:
> On Tue, Dec 16, 2025 at 11:51:30AM +0100, Thorsten Leemhuis wrote:
>> Lo! FWIW, Nick Bowler (now CCed) reported that below patch fixes a
>> regression for him caused by the commit from Esben (now also CCed) the
>> Fixes: tag mentions. See "hwclock busted w/ M48T59 RTC (regression)" for
>> details:
>> https://lore.kernel.org/all/2t6bhs4udbu55ctbemkhlluchz2exrwown7kmu2gss6zukaxdm@ughygemahmem/
>> and
>>
>> Nick, could you maybe provide a tested-by tag here? Maybe that would
>> motivate someone to get this en route to mainline.
>>
>> Adding a "Cc: <stable@vger.kernel.org>" would be great, too, as Nick
>> encountered this on earlier series, as it was backported all the way to
>> 5.15.y
>
> It was backported to 5.10.y and 5.4.y too, but only after I had reported
> this regression back in October (and I guess 5.4.y is EOL now).
>
> Tested-by: Nick Bowler <nbowler@draconx.ca>
Esben (author of the culprit) and Alexandre (who merged it): do you
still have reviewing/applying the patch at the start of this thread (
https://lore.kernel.org/all/BN0PR08MB6951415A751F236375A2945683D1A@BN0PR08MB6951.namprd08.prod.outlook.com/#t)
on your todo list?
Sorry for nagging, would just be good to finally get this 6.18-rc1
regression fixed in mainline so the fix can be backported to stable,
too. And due to the holidays I thought a quick reminder might be wise.
Or was this fixed somehow and I just missed it?
Ciao, Thorsten
#regzbot ignore-activity
^ permalink raw reply
* Re: [PATCH RESEND v2 3/3] mfd: sec: drop now unused struct sec_pmic_dev::irq_data
From: Lee Jones @ 2026-01-09 10:45 UTC (permalink / raw)
To: André Draszik
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: <20251217-s5m-alarm-v2-3-b7bff003e94c@linaro.org>
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);
> }
>
> -int sec_irq_init(struct sec_pmic_dev *sec_pmic)
> +struct regmap_irq_chip_data *sec_irq_init(struct sec_pmic_dev *sec_pmic)
> {
> + struct regmap_irq_chip_data *sec_irq_chip_data;
> const struct regmap_irq_chip *sec_irq_chip;
> int ret;
>
> @@ -331,7 +334,7 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
> sec_irq_chip = &s5m8767_irq_chip;
> break;
> case S2DOS05:
> - return 0;
> + return NULL;
> case S2MPA01:
> sec_irq_chip = &s2mps14_irq_chip;
> break;
> @@ -356,30 +359,22 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
> sec_irq_chip = &s2mpu05_irq_chip;
> 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);
> }
>
> if (!sec_pmic->irq) {
> dev_warn(sec_pmic->dev,
> "No interrupt specified, no interrupts\n");
> - return 0;
> + return NULL;
> }
>
> ret = devm_regmap_add_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic,
> sec_pmic->irq, IRQF_ONESHOT,
> - 0, sec_irq_chip, &sec_pmic->irq_data);
> + 0, sec_irq_chip, &sec_irq_chip_data);
> if (ret)
> - return dev_err_probe(sec_pmic->dev, ret,
> - "Failed to add %s IRQ chip\n",
> - sec_irq_chip->name);
> + return dev_err_ptr_probe(sec_pmic->dev, ret, "Failed to add %s IRQ chip\n",
> + sec_irq_chip->name);
>
> - /*
> - * The rtc-s5m driver requests S2MPS14_IRQ_RTCA0 also for S2MPS11
> - * so the interrupt number must be consistent.
> - */
> - BUILD_BUG_ON(((enum s2mps14_irq)S2MPS11_IRQ_RTCA0) != S2MPS14_IRQ_RTCA0);
> -
> - return 0;
> + return sec_irq_chip_data;
> }
> diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
> index d785e101fe795a5d8f9cccf4ccc4232437e89416..c7c3c8cd8d5f99ef0cc3188e1c3b49031f4750f2 100644
> --- a/include/linux/mfd/samsung/core.h
> +++ b/include/linux/mfd/samsung/core.h
> @@ -69,7 +69,6 @@ struct sec_pmic_dev {
>
> int device_type;
> int irq;
> - struct regmap_irq_chip_data *irq_data;
> };
>
> struct sec_platform_data {
>
> --
> 2.52.0.351.gbe84eed79e-goog
>
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH RESEND v6 00/17] Support ROHM BD72720 PMIC
From: Lee Jones @ 2026-01-09 9:38 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sebastian Reichel, 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: <63bc889a-b97e-43c3-9f46-9ca444873b70@gmail.com>
Sebastian,
Any thoughts?
Matti,
If Sebastian doesn't reply, I can execute your plan next week.
On Fri, 09 Jan 2026, Matti Vaittinen wrote:
> On 08/01/2026 19:27, Lee Jones wrote:
> > On Mon, 15 Dec 2025, Matti Vaittinen wrote:
> >
> > > Resending the v6
> > >
> > > Series is same as v6 _except_ being rebased on v6.19-rc1 - and adding rb
> > > tags which were replied to v6.
> > >
> > > The ROHM BD72720 is a new power management IC for portable, battery
> > > powered devices. It integrates 10 BUCKs and 11 LDOs, RTC, charger, LEDs,
> > > GPIOs and a clock gate. To me the BD72720 seems like a successor to the
> > > BD71828 and BD71815 PMICs.
> > >
> > > This series depends on
> > > 5bff79dad20a ("power: supply: Add bd718(15/28/78) charger driver")
> > > which is in power-supply tree, for-next. Thus, the series is based on
> > > it.
> > >
> > > The testing since v4 has suffered some hardware-issues after I
> > > accidentally enabled charging while the PMIC's battery pin was connected
> > > to the I/O domain. Some heat was generated, not terribly lot smoke
> > > though...
> > >
> > > After the incident I've had occasional I2C failures. I, however, suspect
> > > the root cause is HW damage in I/O lines.
> > >
> > > Revision history:
> > > v6 resend:
> > > - Rebased on v6.19-rc1 and collected rb-tags from v6.
> > >
> > > v5 => v6:
> > > - MFD fixes as suggested by Lee
> > > - Styling mostly
> > > - New patch to Fix comment style for MFD driver
> > > More accurate changelog in individual patches
> > >
> > > v4 => v5:
> > > - dt-binding fixes as discussed in v4 reviews.
> > > - Drop rohm,vdr-battery.yaml and add vdr properties to battery.yaml
> > > - Drop 'rohm,' -vendor-prefix from vdr properties
> > > - Link to v4:
> > > https://lore.kernel.org/all/cover.1763022807.git.mazziesaccount@gmail.com/
> > > More accurate changelog in individual patches
> > >
> > > v3 => v4:
> > > - dt-binding fixes to the BD72720 MFD example and regulator bindings
> > > More accurate changelog in individual patches
> > >
> > > v2 => v3:
> > > - rebased to power-supply/for-next as dependencies are merged to there
> > > - plenty of dt-binding changes as suggested by reviewers
> > > - add new patch to better document existing 'trickle-charging' property
> > > More accurate changelog in individual patches
> > >
> > > RFCv1 => v2:
> > > - Drop RFC status
> > > - Use stacked regmaps to hide secondary map from the sub-drivers
> > > - Quite a few styling fixes and improvements as suggested by
> > > reviewers. More accurate changelog in individual patches.
> > > - Link to v1:
> > > https://lore.kernel.org/all/cover.1759824376.git.mazziesaccount@gmail.com/
> > >
> > > ---
> > >
> > > Matti Vaittinen (17):
> > > dt-bindings: regulator: ROHM BD72720
> > > dt-bindings: battery: Clarify trickle-charge
> > > dt-bindings: battery: Add trickle-charge upper limit
> > > dt-bindings: battery: Voltage drop properties
> > > dt-bindings: mfd: ROHM BD72720
> > > dt-bindings: leds: bd72720: Add BD72720
> > > mfd: rohm-bd71828: Use regmap_reg_range()
> > > mfd: rohm-bd71828: Use standard file header format
> > > mfd: rohm-bd71828: Support ROHM BD72720
> > > regulator: bd71828: rename IC specific entities
> > > regulator: bd71828: Support ROHM BD72720
> > > gpio: Support ROHM BD72720 gpios
> > > clk: clk-bd718x7: Support BD72720 clk gate
> > > rtc: bd70528: Support BD72720 rtc
> > > power: supply: bd71828: Support wider register addresses
> > > power: supply: bd71828-power: Support ROHM BD72720
> > > MAINTAINERS: Add ROHM BD72720 PMIC
> > >
> > > .../bindings/leds/rohm,bd71828-leds.yaml | 7 +-
> > > .../bindings/mfd/rohm,bd72720-pmic.yaml | 339 ++++++
> > > .../bindings/power/supply/battery.yaml | 33 +-
> > > .../regulator/rohm,bd72720-regulator.yaml | 148 +++
> > > MAINTAINERS | 2 +
> > > drivers/clk/Kconfig | 4 +-
> > > drivers/clk/clk-bd718x7.c | 10 +-
> > > drivers/gpio/Kconfig | 9 +
> > > drivers/gpio/Makefile | 1 +
> > > drivers/gpio/gpio-bd72720.c | 281 +++++
> > > drivers/mfd/Kconfig | 18 +-
> > > drivers/mfd/rohm-bd71828.c | 555 ++++++++-
> > > drivers/power/supply/bd71828-power.c | 160 ++-
> > > drivers/regulator/Kconfig | 8 +-
> > > drivers/regulator/bd71828-regulator.c | 1025 ++++++++++++++++-
> > > drivers/rtc/Kconfig | 3 +-
> > > drivers/rtc/rtc-bd70528.c | 21 +-
> > > include/linux/mfd/rohm-bd72720.h | 634 ++++++++++
> > > include/linux/mfd/rohm-generic.h | 1 +
> > > 19 files changed, 3127 insertions(+), 132 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd72720-pmic.yaml
> > > create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml
> > > create mode 100644 drivers/gpio/gpio-bd72720.c
> > > create mode 100644 include/linux/mfd/rohm-bd72720.h
> >
> > 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 :(
>
> 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 .
>
> Yours,
> -- Matti.
>
>
> ---
> Matti Vaittinen
> Linux kernel developer at ROHM Semiconductors
> Oulu Finland
>
> ~~ When things go utterly wrong vim users can always type :help! ~~
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH RESEND v6 00/17] Support ROHM BD72720 PMIC
From: Matti Vaittinen @ 2026-01-09 6:29 UTC (permalink / raw)
To: Lee Jones
Cc: Matti Vaittinen, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sebastian Reichel, 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: <20260108172735.GK302752@google.com>
On 08/01/2026 19:27, Lee Jones wrote:
> On Mon, 15 Dec 2025, Matti Vaittinen wrote:
>
>> Resending the v6
>>
>> Series is same as v6 _except_ being rebased on v6.19-rc1 - and adding rb
>> tags which were replied to v6.
>>
>> The ROHM BD72720 is a new power management IC for portable, battery
>> powered devices. It integrates 10 BUCKs and 11 LDOs, RTC, charger, LEDs,
>> GPIOs and a clock gate. To me the BD72720 seems like a successor to the
>> BD71828 and BD71815 PMICs.
>>
>> This series depends on
>> 5bff79dad20a ("power: supply: Add bd718(15/28/78) charger driver")
>> which is in power-supply tree, for-next. Thus, the series is based on
>> it.
>>
>> The testing since v4 has suffered some hardware-issues after I
>> accidentally enabled charging while the PMIC's battery pin was connected
>> to the I/O domain. Some heat was generated, not terribly lot smoke
>> though...
>>
>> After the incident I've had occasional I2C failures. I, however, suspect
>> the root cause is HW damage in I/O lines.
>>
>> Revision history:
>> v6 resend:
>> - Rebased on v6.19-rc1 and collected rb-tags from v6.
>>
>> v5 => v6:
>> - MFD fixes as suggested by Lee
>> - Styling mostly
>> - New patch to Fix comment style for MFD driver
>> More accurate changelog in individual patches
>>
>> v4 => v5:
>> - dt-binding fixes as discussed in v4 reviews.
>> - Drop rohm,vdr-battery.yaml and add vdr properties to battery.yaml
>> - Drop 'rohm,' -vendor-prefix from vdr properties
>> - Link to v4:
>> https://lore.kernel.org/all/cover.1763022807.git.mazziesaccount@gmail.com/
>> More accurate changelog in individual patches
>>
>> v3 => v4:
>> - dt-binding fixes to the BD72720 MFD example and regulator bindings
>> More accurate changelog in individual patches
>>
>> v2 => v3:
>> - rebased to power-supply/for-next as dependencies are merged to there
>> - plenty of dt-binding changes as suggested by reviewers
>> - add new patch to better document existing 'trickle-charging' property
>> More accurate changelog in individual patches
>>
>> RFCv1 => v2:
>> - Drop RFC status
>> - Use stacked regmaps to hide secondary map from the sub-drivers
>> - Quite a few styling fixes and improvements as suggested by
>> reviewers. More accurate changelog in individual patches.
>> - Link to v1:
>> https://lore.kernel.org/all/cover.1759824376.git.mazziesaccount@gmail.com/
>>
>> ---
>>
>> Matti Vaittinen (17):
>> dt-bindings: regulator: ROHM BD72720
>> dt-bindings: battery: Clarify trickle-charge
>> dt-bindings: battery: Add trickle-charge upper limit
>> dt-bindings: battery: Voltage drop properties
>> dt-bindings: mfd: ROHM BD72720
>> dt-bindings: leds: bd72720: Add BD72720
>> mfd: rohm-bd71828: Use regmap_reg_range()
>> mfd: rohm-bd71828: Use standard file header format
>> mfd: rohm-bd71828: Support ROHM BD72720
>> regulator: bd71828: rename IC specific entities
>> regulator: bd71828: Support ROHM BD72720
>> gpio: Support ROHM BD72720 gpios
>> clk: clk-bd718x7: Support BD72720 clk gate
>> rtc: bd70528: Support BD72720 rtc
>> power: supply: bd71828: Support wider register addresses
>> power: supply: bd71828-power: Support ROHM BD72720
>> MAINTAINERS: Add ROHM BD72720 PMIC
>>
>> .../bindings/leds/rohm,bd71828-leds.yaml | 7 +-
>> .../bindings/mfd/rohm,bd72720-pmic.yaml | 339 ++++++
>> .../bindings/power/supply/battery.yaml | 33 +-
>> .../regulator/rohm,bd72720-regulator.yaml | 148 +++
>> MAINTAINERS | 2 +
>> drivers/clk/Kconfig | 4 +-
>> drivers/clk/clk-bd718x7.c | 10 +-
>> drivers/gpio/Kconfig | 9 +
>> drivers/gpio/Makefile | 1 +
>> drivers/gpio/gpio-bd72720.c | 281 +++++
>> drivers/mfd/Kconfig | 18 +-
>> drivers/mfd/rohm-bd71828.c | 555 ++++++++-
>> drivers/power/supply/bd71828-power.c | 160 ++-
>> drivers/regulator/Kconfig | 8 +-
>> drivers/regulator/bd71828-regulator.c | 1025 ++++++++++++++++-
>> drivers/rtc/Kconfig | 3 +-
>> drivers/rtc/rtc-bd70528.c | 21 +-
>> include/linux/mfd/rohm-bd72720.h | 634 ++++++++++
>> include/linux/mfd/rohm-generic.h | 1 +
>> 19 files changed, 3127 insertions(+), 132 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd72720-pmic.yaml
>> create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml
>> create mode 100644 drivers/gpio/gpio-bd72720.c
>> create mode 100644 include/linux/mfd/rohm-bd72720.h
>
> 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 :(
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 .
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: [RFC PATCH v2 4/5] rust: add RTC core abstractions and data structures
From: Kari Argillander @ 2026-01-08 23:31 UTC (permalink / raw)
To: Ke Sun
Cc: Alexandre Belloni, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, linux-rtc, rust-for-linux,
Alvin Sun
In-Reply-To: <20260107143738.3021892-5-sunke@kylinos.cn>
On Wed, 7 Jan 2026 at 19:57, Ke Sun <sunke@kylinos.cn> wrote:
>
> Add Rust abstractions for RTC subsystem, including RtcDevice,
> RtcTime, RtcWkAlrm data structures, RtcOps trait for driver
> operations, and devm-managed device registration support.
>
> Signed-off-by: Ke Sun <sunke@kylinos.cn>
> ---
> rust/helpers/helpers.c | 1 +
> rust/helpers/rtc.c | 9 +
> rust/kernel/lib.rs | 2 +
> rust/kernel/rtc.rs | 985 +++++++++++++++++++++++++++++++++++++++++
All new files needs MAINTAINER entry at the end. I know this is RFC but
just saying.
> 4 files changed, 997 insertions(+)
> create mode 100644 rust/helpers/rtc.c
> create mode 100644 rust/kernel/rtc.rs
>
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 79c72762ad9c4..1a5c103fb24ba 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -48,6 +48,7 @@
> #include "rcu.c"
> #include "refcount.c"
> #include "regulator.c"
> +#include "rtc.c"
> #include "scatterlist.c"
> #include "security.c"
> #include "signal.c"
> diff --git a/rust/helpers/rtc.c b/rust/helpers/rtc.c
> new file mode 100644
> index 0000000000000..862cd61670bfc
> --- /dev/null
> +++ b/rust/helpers/rtc.c
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/rtc.h>
> +
> +int rust_helper_devm_rtc_register_device(struct rtc_device *rtc)
> +{
> + return __devm_rtc_register_device(THIS_MODULE, rtc);
> +}
> +
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 3e557335fc5fe..1390073e4ae27 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -135,6 +135,8 @@
> pub mod rbtree;
> pub mod regulator;
> pub mod revocable;
> +#[cfg(CONFIG_RTC_CLASS)]
> +pub mod rtc;
> pub mod scatterlist;
> pub mod security;
> pub mod seq_file;
> diff --git a/rust/kernel/rtc.rs b/rust/kernel/rtc.rs
> new file mode 100644
> index 0000000000000..0c662b94b96f4
> --- /dev/null
> +++ b/rust/kernel/rtc.rs
> @@ -0,0 +1,985 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! RTC (Real-Time Clock) device support.
> +//!
> +//! C headers: [`include/linux/rtc.h`](srctree/include/linux/rtc.h).
> +//!
> +//! Reference: <https://www.kernel.org/doc/html/latest/driver-api/rtc.html>
> +use crate::{
> + bindings,
> + bitmap::Bitmap,
> + device::{
> + self,
> + AsBusDevice, //
> + },
> + devres,
> + error::Error,
> + prelude::*,
> + seq_file::SeqFile,
> + sync::aref::{
> + ARef, //
> + AlwaysRefCounted,
> + },
> + types::{
> + ForeignOwnable,
> + Opaque, //
> + }, //
> +};
> +
> +use core::{
> + ffi::c_void,
> + marker::PhantomData,
> + ptr::NonNull, //
> +};
> +
> +/// RTC time structure.
> +///
> +/// Wraps the C `struct rtc_time` from `include/uapi/linux/rtc.h`.
> +#[repr(transparent)]
> +#[derive(Clone, Copy)]
> +pub struct RtcTime(pub bindings::rtc_time);
Probably Debug would be nice for this. As rtc_time is same as
`struct tm` I wonder should we just make struct tm and make RtcTime
type alias for that. That way we do not need to duplicate all the
things.
> +impl RtcTime {
> + /// Creates a new `RtcTime` from a time64_t value (seconds since 1970-01-01 00:00:00 UTC).
> + pub fn from_time64(time: i64) -> Self {
> + let mut tm = Self(pin_init::zeroed());
> + // SAFETY: `rtc_time64_to_tm` is a pure function that only writes to the provided
> + // `struct rtc_time` pointer. The pointer is valid because `tm.0` is a valid mutable
> + // reference, and the function does not retain any references to it.
> + unsafe {
> + bindings::rtc_time64_to_tm(time, &mut tm.0);
> + }
> + tm
> + }
> +
> + /// Converts this `RtcTime` to a time64_t value (seconds since 1970-01-01 00:00:00 UTC).
> + pub fn to_time64(&self) -> i64 {
> + // SAFETY: `rtc_tm_to_time64` is a pure function that only reads from the provided
> + // `struct rtc_time` pointer. The pointer is valid because `self.0` is a valid reference,
> + // and the function does not retain any references to it.
> + unsafe { bindings::rtc_tm_to_time64(core::ptr::from_ref(&self.0).cast_mut()) }
> + }
> +
> + /// Sets this `RtcTime` from a time64_t value (seconds since 1970-01-01 00:00:00 UTC).
> + pub fn set_from_time64(&mut self, time: i64) {
> + // SAFETY: `rtc_time64_to_tm` is a pure function that only writes to the provided
> + // `struct rtc_time` pointer. The pointer is valid because `self.0` is a valid mutable
> + // reference, and the function does not retain any references to it.
> + unsafe {
> + bindings::rtc_time64_to_tm(time, &mut self.0);
> + }
> + }
> +
> + /// Converts a time64_t value to an RTC time structure.
> + #[inline]
> + pub fn time64_to_tm(time: i64, tm: &mut Self) {
> + tm.set_from_time64(time);
> + }
> +
> + /// Converts an RTC time structure to a time64_t value (seconds since 1970-01-01 00:00:00 UTC).
> + #[inline]
> + pub fn tm_to_time64(tm: &Self) -> i64 {
> + tm.to_time64()
> + }
> +
> + /// Calculates the day of the year (0-365) from the date components.
> + #[inline]
> + pub fn year_days(&self) -> i32 {
> + // SAFETY: `rtc_year_days` is a pure function that only performs calculations based on
> + // the provided parameters. The values should be from valid RTC time structures and
> + // non-negative, but the function itself is safe to call with any values.
> + unsafe {
> + bindings::rtc_year_days(
> + self.0.tm_mday as u32,
> + self.0.tm_mon as u32,
> + self.0.tm_year as u32,
> + )
> + }
> + }
> +
> + /// Returns the seconds field (0-59).
> + #[inline]
> + pub fn tm_sec(&self) -> i32 {
> + self.0.tm_sec
> + }
> +
> + /// Sets the seconds field (0-59).
> + #[inline]
> + pub fn set_tm_sec(&mut self, sec: i32) {
> + self.0.tm_sec = sec;
> + }
> +
> + /// Returns the minutes field (0-59).
> + #[inline]
> + pub fn tm_min(&self) -> i32 {
> + self.0.tm_min
> + }
> +
> + /// Sets the minutes field (0-59).
> + #[inline]
> + pub fn set_tm_min(&mut self, min: i32) {
> + self.0.tm_min = min;
> + }
> +
> + /// Returns the hours field (0-23).
> + #[inline]
> + pub fn tm_hour(&self) -> i32 {
> + self.0.tm_hour
> + }
> +
> + /// Sets the hours field (0-23).
> + #[inline]
> + pub fn set_tm_hour(&mut self, hour: i32) {
> + self.0.tm_hour = hour;
> + }
> +
> + /// Returns the day of the month (1-31).
> + #[inline]
> + pub fn tm_mday(&self) -> i32 {
> + self.0.tm_mday
> + }
> +
> + /// Sets the day of the month (1-31).
> + #[inline]
> + pub fn set_tm_mday(&mut self, mday: i32) {
> + self.0.tm_mday = mday;
> + }
> +
> + /// Returns the month (0-11, where 0 is January).
> + #[inline]
> + pub fn tm_mon(&self) -> i32 {
> + self.0.tm_mon
> + }
> +
> + /// Sets the month (0-11, where 0 is January).
> + #[inline]
> + pub fn set_tm_mon(&mut self, mon: i32) {
> + self.0.tm_mon = mon;
> + }
> +
> + /// Returns the year (years since 1900).
> + #[inline]
> + pub fn tm_year(&self) -> i32 {
> + self.0.tm_year
> + }
> +
> + /// Sets the year (years since 1900).
> + #[inline]
> + pub fn set_tm_year(&mut self, year: i32) {
> + self.0.tm_year = year;
> + }
> +
> + /// Returns the day of the week (0-6, where 0 is Sunday).
> + #[inline]
> + pub fn tm_wday(&self) -> i32 {
> + self.0.tm_wday
> + }
> +
> + /// Sets the day of the week (0-6, where 0 is Sunday).
> + #[inline]
> + pub fn set_tm_wday(&mut self, wday: i32) {
> + self.0.tm_wday = wday;
> + }
> +
> + /// Returns the day of the year (0-365).
> + #[inline]
> + pub fn tm_yday(&self) -> i32 {
> + self.0.tm_yday
> + }
> +
> + /// Sets the day of the year (0-365).
> + #[inline]
> + pub fn set_tm_yday(&mut self, yday: i32) {
> + self.0.tm_yday = yday;
> + }
> +
> + /// Returns the daylight saving time flag.
> + #[inline]
> + pub fn tm_isdst(&self) -> i32 {
> + self.0.tm_isdst
> + }
> +
> + /// Sets the daylight saving time flag.
> + #[inline]
> + pub fn set_tm_isdst(&mut self, isdst: i32) {
> + self.0.tm_isdst = isdst;
> + }
> +}
> +
> +impl Default for RtcTime {
> + fn default() -> Self {
Could be const.
> + Self(pin_init::zeroed())
> + }
> +}
> +
> +/// RTC wake alarm structure.
> +///
> +/// Wraps the C `struct rtc_wkalrm` from `include/uapi/linux/rtc.h`.
> +#[repr(transparent)]
> +#[derive(Clone, Copy)]
> +pub struct RtcWkAlrm(pub bindings::rtc_wkalrm);
> +
> +impl Default for RtcWkAlrm {
> + fn default() -> Self {
Could be const.
> + Self(pin_init::zeroed())
> + }
> +}
> +
> +impl RtcWkAlrm {
> + /// Returns a reference to the alarm time.
> + #[inline]
> + pub fn get_time(&self) -> &RtcTime {
> + // SAFETY: `RtcTime` is `#[repr(transparent)]` over `bindings::rtc_time`, so the memory
> + // layout is identical. We can safely reinterpret the reference.
> + unsafe { &*(&raw const self.0.time).cast::<RtcTime>() }
> + }
> +
> + /// Returns a mutable reference to the alarm time.
> + #[inline]
> + pub fn get_time_mut(&mut self) -> &mut RtcTime {
> + // SAFETY: `RtcTime` is `#[repr(transparent)]` over `bindings::rtc_time`, so the memory
> + // layout is identical. We can safely reinterpret the reference.
> + unsafe { &mut *(&raw mut self.0.time).cast::<RtcTime>() }
> + }
> +
> + /// Sets the alarm time from a `RtcTime` value.
> + #[inline]
> + pub fn set_time(&mut self, time: RtcTime) {
> + self.0.time = time.0;
> + }
> +
> + /// Returns the enabled field.
> + #[inline]
> + pub fn enabled(&self) -> u8 {
> + self.0.enabled
> + }
> +
> + /// Sets the `enabled` field.
> + #[inline]
> + pub fn set_enabled(&mut self, enabled: u8) {
> + self.0.enabled = enabled;
> + }
> +
> + /// Returns the pending field.
> + #[inline]
> + pub fn pending(&self) -> u8 {
> + self.0.pending
> + }
> +
> + /// Sets the `pending` field.
> + #[inline]
> + pub fn set_pending(&mut self, pending: u8) {
> + self.0.pending = pending;
> + }
> +}
> +
> +/// RTC parameter structure.
> +///
> +/// Wraps the C `struct rtc_param` from `include/uapi/linux/rtc.h`.
> +#[repr(transparent)]
> +#[derive(Clone, Copy)]
> +pub struct RtcParam(pub bindings::rtc_param);
> +
> +impl Default for RtcParam {
> + fn default() -> Self {
Could be made const.
> + Self(pin_init::zeroed())
> + }
> +}
> +
> +/// A Rust wrapper for the C `struct rtc_device`.
> +///
> +/// This type provides safe access to RTC device operations. The underlying `rtc_device`
> +/// is managed by the kernel and remains valid for the lifetime of the `RtcDevice`.
> +///
> +/// # Invariants
> +///
> +/// A [`RtcDevice`] instance holds a pointer to a valid [`struct rtc_device`] that is
> +/// registered and managed by the kernel.
> +///
> +/// # Examples
> +///
> +/// ```rust
> +/// # use kernel::{
> +/// # prelude::*,
> +/// # rtc::RtcDevice, //
> +/// # };
> +/// // Example: Set the time range for the RTC device
> +/// // rtc.set_range_min(0);
> +/// // rtc.set_range_max(u64::MAX);
Currently this example is commented out.
> +/// // Ok(())
> +/// // }
> +/// ```
> +///
> +/// [`struct rtc_device`]: https://docs.kernel.org/driver-api/rtc.html
> +#[repr(transparent)]
> +pub struct RtcDevice<T: 'static = ()>(Opaque<bindings::rtc_device>, PhantomData<T>);
> +
> +impl<T: 'static> RtcDevice<T> {
> + /// Obtain the raw [`struct rtc_device`] pointer.
> + #[inline]
> + pub fn as_raw(&self) -> *mut bindings::rtc_device {
> + self.0.get()
> + }
> +
> + /// Set the minimum time range for the RTC device.
> + #[inline]
> + pub fn set_range_min(&self, min: i64) {
> + // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
> + // `struct rtc_device`, and we're only writing to the `range_min` field.
> + unsafe {
> + (*self.as_raw()).range_min = min;
> + }
Put ';' outside of unsafe that way this will be onliner. See also other places.
So
unsafe { (*self.as_raw()).range_min = min };
> + }
> +
> + /// Set the maximum time range for the RTC device.
> + #[inline]
> + pub fn set_range_max(&self, max: u64) {
> + // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
> + // `struct rtc_device`, and we're only writing to the `range_max` field.
> + unsafe {
> + (*self.as_raw()).range_max = max;
> + }
> + }
> +
> + /// Get the minimum time range for the RTC device.
> + #[inline]
> + pub fn range_min(&self) -> i64 {
> + // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
> + // `struct rtc_device`, and we're only reading the `range_min` field.
> + unsafe { (*self.as_raw()).range_min }
> + }
> +
> + /// Get the maximum time range for the RTC device.
> + #[inline]
> + pub fn range_max(&self) -> u64 {
> + // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
> + // `struct rtc_device`, and we're only reading the `range_max` field.
> + unsafe { (*self.as_raw()).range_max }
> + }
> +
> + /// Notify the RTC framework that an interrupt has occurred.
> + ///
> + /// Should be called from interrupt handlers. Schedules work to handle the interrupt
> + /// in process context.
> + #[inline]
> + pub fn update_irq(&self, num: usize, events: usize) {
> + // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
> + // `struct rtc_device`. The rtc_update_irq function handles NULL/ERR checks internally.
> + unsafe {
> + bindings::rtc_update_irq(self.as_raw(), num, events);
> + }
> + }
> +
> + /// Clear a feature bit in the RTC device.
> + #[inline]
> + pub fn clear_feature(&self, feature: u32) {
> + // SAFETY: By the type invariants, self.as_raw() is a valid pointer to a
> + // `struct rtc_device`, and features is a valid bitmap array with RTC_FEATURE_CNT bits.
> + let features_bitmap = unsafe {
> + Bitmap::from_raw_mut(
> + (*self.as_raw()).features.as_mut_ptr().cast::<usize>(),
> + bindings::RTC_FEATURE_CNT as usize,
> + )
> + };
> + features_bitmap.clear_bit(feature as usize);
> + }
> +}
> +
> +impl<T: 'static, Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for RtcDevice<T> {
> + fn as_ref(&self) -> &device::Device<Ctx> {
> + let raw = self.as_raw();
> + // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
> + // `struct rtc_device`.
> + let dev = unsafe { &raw mut (*raw).dev };
> +
> + // SAFETY: `dev` points to a valid `struct device`.
> + unsafe { device::Device::from_raw(dev) }
> + }
> +}
> +
> +// SAFETY: `RtcDevice` is a transparent wrapper of `struct rtc_device`.
> +// The offset is guaranteed to point to a valid device field inside `RtcDevice`.
> +unsafe impl<T: 'static, Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for RtcDevice<T> {
> + const OFFSET: usize = core::mem::offset_of!(bindings::rtc_device, dev);
> +}
> +
> +// SAFETY: Instances of `RtcDevice` are always reference-counted via the underlying `device`.
> +// The `struct rtc_device` contains a `struct device dev` as its first field, and the
> +// reference counting is managed through `get_device`/`put_device` on the `dev` field.
> +unsafe impl<T: 'static> AlwaysRefCounted for RtcDevice<T> {
> + fn inc_ref(&self) {
> + let dev: &device::Device = self.as_ref();
> + // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> + // `dev.as_raw()` is a valid pointer to a `struct device` with a non-zero refcount.
> + unsafe { bindings::get_device(dev.as_raw()) };
> + }
> +
> + unsafe fn dec_ref(obj: NonNull<Self>) {
> + let rtc: *mut bindings::rtc_device = obj.cast().as_ptr();
> +
> + // SAFETY: By the type invariant of `Self`, `rtc` is a pointer to a valid
> + // `struct rtc_device`. The `dev` field is the first field of `struct rtc_device`,
> + // so we can safely access it.
> + let dev = unsafe { &raw mut (*rtc).dev };
> +
> + // SAFETY: The safety requirements guarantee that the refcount is non-zero.
> + unsafe { bindings::put_device(dev) };
> + }
> +}
> +
> +// SAFETY: `RtcDevice` is reference-counted and can be released from any thread.
> +unsafe impl<T: 'static> Send for RtcDevice<T> {}
> +
> +// SAFETY: `RtcDevice` can be shared among threads because all immutable methods are
> +// protected by the synchronization in `struct rtc_device` (via `ops_lock` mutex).
> +unsafe impl<T: 'static> Sync for RtcDevice<T> {}
> +
> +impl<T: RtcOps> RtcDevice<T> {
> + /// Allocates a new RTC device managed by devres.
> + ///
> + /// This function allocates an RTC device and sets the driver data. The device will be
> + /// automatically freed when the parent device is removed.
> + pub fn new(
> + parent_dev: &device::Device,
> + init: impl PinInit<T, Error>,
> + ) -> Result<ARef<Self>> {
> + // SAFETY: `Device<Bound>` and `Device<CoreInternal>` have the same layout.
> + let dev_internal: &device::Device<device::CoreInternal> =
> + unsafe { &*core::ptr::from_ref(parent_dev).cast() };
> +
> + // Allocate RTC device.
> + // SAFETY: `devm_rtc_allocate_device` returns a pointer to a devm-managed rtc_device.
> + // We use `dev_internal.as_raw()` which is `pub(crate)`, but we can access it through
> + // the same device pointer.
> + let rtc: *mut bindings::rtc_device =
> + unsafe { bindings::devm_rtc_allocate_device(dev_internal.as_raw()) };
> + if rtc.is_null() {
> + return Err(ENOMEM);
> + }
You could construct NonNull with new() and convert None to ENOMEM.
That way you do not need to use new_unchecked.
> +
> + // Set the RTC device ops.
> + // SAFETY: We just allocated the RTC device, so it's safe to set the ops.
> + unsafe {
> + (*rtc).ops = Adapter::<T>::VTABLE.as_raw();
> + }
> +
> + // SAFETY: `rtc` is a valid pointer to a newly allocated rtc_device.
> + // `RtcDevice` is `#[repr(transparent)]` over `Opaque<rtc_device>`, so we can safely cast.
> + let rtc_device = unsafe { ARef::from_raw(NonNull::new_unchecked(rtc.cast::<Self>())) };
> + rtc_device.set_drvdata(init)?;
> + Ok(rtc_device)
> + }
> +
> + /// Store a pointer to the bound driver's private data.
> + pub fn set_drvdata(&self, data: impl PinInit<T, Error>) -> Result {
> + let data = KBox::pin_init(data, GFP_KERNEL)?;
> + let dev: &device::Device<device::Bound> = self.as_ref();
> +
> + // SAFETY: `self.as_raw()` is a valid pointer to a `struct rtc_device`.
> + unsafe { bindings::dev_set_drvdata(dev.as_raw(), data.into_foreign().cast()) };
> + Ok(())
> + }
> +
> + /// Borrow the driver's private data bound to this [`RtcDevice`].
> + pub fn drvdata(&self) -> Result<Pin<&T>> {
> + let dev: &device::Device<device::Bound> = self.as_ref();
> +
> + // SAFETY: `self.as_raw()` is a valid pointer to a `struct device`.
> + let ptr = unsafe { bindings::dev_get_drvdata(dev.as_raw()) };
> +
> + if ptr.is_null() {
> + return Err(ENOENT);
> + }
> +
> + // SAFETY: The caller ensures that `ptr` is valid and writable.
> + Ok(unsafe { Pin::<KBox<T>>::borrow(ptr.cast()) })
> + }
> +}
> +
> +impl<T: 'static> Drop for RtcDevice<T> {
> + fn drop(&mut self) {
> + let dev: &device::Device<device::Bound> = self.as_ref();
> + // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> + let ptr: *mut c_void = unsafe { bindings::dev_get_drvdata(dev.as_raw()) };
> +
> + // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> + unsafe { bindings::dev_set_drvdata(dev.as_raw(), core::ptr::null_mut()) };
> +
> + if !ptr.is_null() {
> + // SAFETY: `ptr` comes from a previous call to `into_foreign()`, and
> + // `dev_get_drvdata()` guarantees to return the same pointer given to
> + // `dev_set_drvdata()`.
> + unsafe { drop(Pin::<KBox<T>>::from_foreign(ptr.cast())) };
> + }
> + }
> +}
> +
> +/// A resource guard that ensures the RTC device is properly registered.
> +///
> +/// This struct is intended to be managed by the `devres` framework by transferring its ownership
> +/// via [`devres::register`]. This ties the lifetime of the RTC device registration
> +/// to the lifetime of the underlying device.
> +pub struct Registration<T: 'static> {
> + #[allow(dead_code)]
> + rtc_device: ARef<RtcDevice<T>>,
> +}
> +
> +impl<T: 'static> Registration<T> {
> + /// Registers an RTC device with the RTC subsystem.
> + ///
> + /// Transfers its ownership to the `devres` framework, which ties its lifetime
> + /// to the parent device.
> + /// On unbind of the parent device, the `devres` entry will be dropped, automatically
> + /// cleaning up the RTC device. This function should be called from the driver's `probe`.
> + pub fn register(dev: &device::Device<device::Bound>, rtc_device: ARef<RtcDevice<T>>) -> Result {
> + let rtc_dev: &device::Device = rtc_device.as_ref();
> + let rtc_parent = rtc_dev.parent().ok_or(EINVAL)?;
> + if dev.as_raw() != rtc_parent.as_raw() {
> + return Err(EINVAL);
> + }
> +
> + // Registers an RTC device with the RTC subsystem.
> + // SAFETY: The device will be automatically unregistered when the parent device
> + // is removed (devm cleanup). The helper function uses `THIS_MODULE` internally.
> + let err = unsafe { bindings::devm_rtc_register_device(rtc_device.as_raw()) };
> + if err != 0 {
> + return Err(Error::from_errno(err));
> + }
You can do
to_result(unsafe {
bindings::devm_rtc_register_device(rtc_device.as_raw()) })?;
> + let registration = Registration { rtc_device };
> +
> + devres::register(dev, registration, GFP_KERNEL)
> + }
> +}
> +
> +/// Options for creating an RTC device.
> +#[derive(Copy, Clone)]
> +pub struct RtcDeviceOptions {
> + /// The name of the RTC device.
> + pub name: &'static CStr,
> +}
> +
> +/// Trait implemented by RTC device operations.
> +///
> +/// This trait defines the operations that an RTC device driver must implement.
> +/// Most methods are optional and have default implementations that return an error.
> +#[vtable]
> +pub trait RtcOps: Sized + 'static {
> + /// Read the current time from the RTC.
> + ///
> + /// This is a required method and must be implemented.
> + fn read_time(_rtcdev: &RtcDevice<Self>, _tm: &mut RtcTime) -> Result {
> + Err(ENOTSUPP)
> + }
> +
> + /// Set the time in the RTC.
> + ///
> + /// This is a required method and must be implemented.
> + ///
> + /// Note: The parameter is `&mut` to match the C API signature, even though
> + /// it's conceptually read-only from the Rust side.
> + fn set_time(_rtcdev: &RtcDevice<Self>, _tm: &mut RtcTime) -> Result {
> + Err(ENOTSUPP)
> + }
> +
> + /// Read the alarm time from the RTC.
> + fn read_alarm(_rtcdev: &RtcDevice<Self>, _alarm: &mut RtcWkAlrm) -> Result {
> + Err(ENOTSUPP)
> + }
> +
> + /// Set the alarm time in the RTC.
> + ///
> + /// Note: The parameter is `&mut` to match the C API signature, even though
> + /// it's conceptually read-only from the Rust side.
> + fn set_alarm(_rtcdev: &RtcDevice<Self>, _alarm: &mut RtcWkAlrm) -> Result {
> + Err(ENOTSUPP)
> + }
> +
> + /// Enable or disable the alarm interrupt.
> + ///
> + /// `enabled` is non-zero to enable, zero to disable.
> + fn alarm_irq_enable(_rtcdev: &RtcDevice<Self>, _enabled: u32) -> Result {
> + Err(ENOTSUPP)
> + }
> +
> + /// Handle custom ioctl commands.
> + fn ioctl(_rtcdev: &RtcDevice<Self>, _cmd: u32, _arg: c_ulong) -> Result<c_int> {
> + Err(ENOTSUPP)
> + }
> +
> + /// Show information in /proc/driver/rtc.
> + fn proc(_rtcdev: &RtcDevice<Self>, _seq: &mut SeqFile) -> Result {
> + Err(ENOTSUPP)
> + }
> +
> + /// Read the time offset.
> + fn read_offset(_rtcdev: &RtcDevice<Self>, _offset: &mut i64) -> Result {
Do not just blindly copy C interface. This example would probably much
better be that we return offset. So Result<i64>. That is much more
idiomatic Rust.
> + Err(ENOTSUPP)
> + }
> +
> + /// Set the time offset.
> + fn set_offset(_rtcdev: &RtcDevice<Self>, _offset: i64) -> Result {
> + Err(ENOTSUPP)
> + }
Why not use
build_error!(VTABLE_DEFAULT_ERROR)
for these?
> + /// Get an RTC parameter.
> + fn param_get(_rtcdev: &RtcDevice<Self>, _param: &mut RtcParam) -> Result {
> + Err(ENOTSUPP)
> + }
> +
> + /// Set an RTC parameter.
> + ///
> + /// Note: The parameter is `&mut` to match the C API signature, even though
> + /// it's conceptually read-only from the Rust side.
> + fn param_set(_rtcdev: &RtcDevice<Self>, _param: &mut RtcParam) -> Result {
> + Err(ENOTSUPP)
> + }
> +}
> +
> +struct Adapter<T: RtcOps> {
> + _p: PhantomData<T>,
> +}
> +
> +impl<T: RtcOps> Adapter<T> {
> + const VTABLE: RtcOpsVTable = create_rtc_ops::<T>();
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `tm` must be a valid pointer to a `struct rtc_time`.
> + unsafe extern "C" fn read_time(
> + dev: *mut bindings::device,
> + tm: *mut bindings::rtc_time,
> + ) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `tm` is valid and writable.
> + // `RtcTime` is `#[repr(transparent)]` over `bindings::rtc_time`, so we can safely cast.
> + let rtc_tm = unsafe { &mut *tm.cast::<RtcTime>() };
> +
> + match T::read_time(rtc_dev, rtc_tm) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `tm` must be a valid pointer to a `struct rtc_time`.
> + unsafe extern "C" fn set_time(
> + dev: *mut bindings::device,
> + tm: *mut bindings::rtc_time,
> + ) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `tm` is valid and writable.
> + // `RtcTime` is `#[repr(transparent)]` over `bindings::rtc_time`, so we can safely cast.
> + let rtc_tm = unsafe { &mut *tm.cast::<RtcTime>() };
> +
> + match T::set_time(rtc_dev, rtc_tm) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `alarm` must be a valid pointer to a `struct rtc_wkalrm`.
> + unsafe extern "C" fn read_alarm(
> + dev: *mut bindings::device,
> + alarm: *mut bindings::rtc_wkalrm,
> + ) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `alarm` is valid and writable.
> + // `RtcWkAlrm` is `#[repr(transparent)]` over `bindings::rtc_wkalrm`, so we can safely cast.
> + let rtc_alarm = unsafe { &mut *alarm.cast::<RtcWkAlrm>() };
> +
> + match T::read_alarm(rtc_dev, rtc_alarm) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `alarm` must be a valid pointer to a `struct rtc_wkalrm`.
> + unsafe extern "C" fn set_alarm(
> + dev: *mut bindings::device,
> + alarm: *mut bindings::rtc_wkalrm,
> + ) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `alarm` is valid and writable.
> + // `RtcWkAlrm` is `#[repr(transparent)]` over `bindings::rtc_wkalrm`, so we can safely cast.
> + let rtc_alarm = unsafe { &mut *alarm.cast::<RtcWkAlrm>() };
> +
> + match T::set_alarm(rtc_dev, rtc_alarm) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + unsafe extern "C" fn alarm_irq_enable(dev: *mut bindings::device, enabled: c_uint) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> +
> + match T::alarm_irq_enable(rtc_dev, enabled) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + unsafe extern "C" fn ioctl(dev: *mut bindings::device, cmd: c_uint, arg: c_ulong) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> +
> + match T::ioctl(rtc_dev, cmd, arg) {
> + Ok(ret) => ret,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `seq` must be a valid pointer to a `struct seq_file`.
> + unsafe extern "C" fn proc(dev: *mut bindings::device, seq: *mut bindings::seq_file) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `seq` is valid and writable.
> + let seq_file = unsafe { &mut *seq.cast::<SeqFile>() };
> +
> + match T::proc(rtc_dev, seq_file) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `offset` must be a valid pointer to a `long`.
> + unsafe extern "C" fn read_offset(dev: *mut bindings::device, offset: *mut c_long) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `offset` is valid and writable.
> + let mut offset_val: i64 = unsafe { *offset.cast() };
> +
> + match T::read_offset(rtc_dev, &mut offset_val) {
> + Ok(()) => {
> + // SAFETY: The caller ensures that `offset` is valid and writable.
> + unsafe { *offset.cast() = offset_val as c_long };
> + 0
> + }
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + unsafe extern "C" fn set_offset(dev: *mut bindings::device, offset: c_long) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> +
> + match T::set_offset(rtc_dev, offset as i64) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `param` must be a valid pointer to a `struct rtc_param`.
> + unsafe extern "C" fn param_get(
> + dev: *mut bindings::device,
> + param: *mut bindings::rtc_param,
> + ) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `param` is valid and writable.
> + // `RtcParam` is `#[repr(transparent)]` over `bindings::rtc_param`, so we can safely cast.
> + let rtc_param = unsafe { &mut *param.cast::<RtcParam>() };
> +
> + match T::param_get(rtc_dev, rtc_param) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + ///
> + /// `dev` must be a valid pointer to the `struct device` embedded in a `struct rtc_device`.
> + /// `param` must be a valid pointer to a `struct rtc_param`.
> + unsafe extern "C" fn param_set(
> + dev: *mut bindings::device,
> + param: *mut bindings::rtc_param,
> + ) -> c_int {
> + // SAFETY: The caller ensures that `dev` is a valid pointer to a `struct device`.
> + let device_dev: &device::Device = unsafe { device::Device::from_raw(dev) };
> + // SAFETY: `dev` is embedded in a `struct rtc_device`, so we can use
> + // `AsBusDevice` to get it.
> + let rtc_dev = unsafe { RtcDevice::<T>::from_device(device_dev) };
> + // SAFETY: The caller ensures that `param` is valid and writable.
> + // `RtcParam` is `#[repr(transparent)]` over `bindings::rtc_param`, so we can safely cast.
> + let rtc_param = unsafe { &mut *param.cast::<RtcParam>() };
> +
> + match T::param_set(rtc_dev, rtc_param) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +}
> +
> +/// VTable structure wrapper for RTC operations.
> +/// Mirrors [`struct rtc_class_ops`](srctree/include/linux/rtc.h).
> +#[repr(transparent)]
> +pub struct RtcOpsVTable(bindings::rtc_class_ops);
> +
> +// SAFETY: RtcOpsVTable is Send. The vtable contains only function pointers,
> +// which are simple data types that can be safely moved across threads.
> +// The thread-safety of calling these functions is handled by the kernel's
> +// locking mechanisms.
> +unsafe impl Send for RtcOpsVTable {}
> +
> +// SAFETY: RtcOpsVTable is Sync. The vtable is immutable after it is created,
> +// so it can be safely referenced and accessed concurrently by multiple threads
> +// e.g. to read the function pointers.
> +unsafe impl Sync for RtcOpsVTable {}
> +
> +impl RtcOpsVTable {
> + /// Returns a raw pointer to the underlying `rtc_class_ops` struct.
> + pub(crate) const fn as_raw(&self) -> *const bindings::rtc_class_ops {
> + &self.0
> + }
> +}
> +
> +/// Creates an RTC operations vtable for a type `T` that implements `RtcOps`.
> +///
> +/// This is used to bridge Rust trait implementations to the C `struct rtc_class_ops`
> +/// expected by the kernel.
> +pub const fn create_rtc_ops<T: RtcOps>() -> RtcOpsVTable {
> + let mut ops: bindings::rtc_class_ops = pin_init::zeroed();
> +
> + ops.read_time = if T::HAS_READ_TIME {
> + Some(Adapter::<T>::read_time)
> + } else {
> + None
> + };
> + ops.set_time = if T::HAS_SET_TIME {
> + Some(Adapter::<T>::set_time)
> + } else {
> + None
> + };
> + ops.read_alarm = if T::HAS_READ_ALARM {
> + Some(Adapter::<T>::read_alarm)
> + } else {
> + None
> + };
> + ops.set_alarm = if T::HAS_SET_ALARM {
> + Some(Adapter::<T>::set_alarm)
> + } else {
> + None
> + };
> + ops.alarm_irq_enable = if T::HAS_ALARM_IRQ_ENABLE {
> + Some(Adapter::<T>::alarm_irq_enable)
> + } else {
> + None
> + };
> + ops.ioctl = if T::HAS_IOCTL {
> + Some(Adapter::<T>::ioctl)
> + } else {
> + None
> + };
> + ops.proc_ = if T::HAS_PROC {
> + Some(Adapter::<T>::proc)
> + } else {
> + None
> + };
> + ops.read_offset = if T::HAS_READ_OFFSET {
> + Some(Adapter::<T>::read_offset)
> + } else {
> + None
> + };
> + ops.set_offset = if T::HAS_SET_OFFSET {
> + Some(Adapter::<T>::set_offset)
> + } else {
> + None
> + };
> + ops.param_get = if T::HAS_PARAM_GET {
> + Some(Adapter::<T>::param_get)
> + } else {
> + None
> + };
> + ops.param_set = if T::HAS_PARAM_SET {
> + Some(Adapter::<T>::param_set)
> + } else {
> + None
> + };
> +
> + RtcOpsVTable(ops)
> +}
> +
> +/// Declares a kernel module that exposes a single RTC AMBA driver.
> +///
> +/// # Examples
> +///
> +///```ignore
> +/// kernel::module_rtc_amba_driver! {
> +/// type: MyDriver,
> +/// name: "Module name",
> +/// authors: ["Author name"],
> +/// description: "Description",
> +/// license: "GPL v2",
> +/// }
> +///```
> +#[macro_export]
> +macro_rules! module_rtc_amba_driver {
> + ($($user_args:tt)*) => {
> + $crate::module_amba_driver! {
> + $($user_args)*
> + imports_ns: ["RTC"],
> + }
> + };
> +}
This does not make lot of sense. Usually these macros are to make life
easier for most common cases. Amba RTC is not common case. I looked at
little bit and I do not even know what is common. Platform, I2c or SPI are
most used but for now just do not have macro at all unless someone asks
for it.
Looking forward for this series :)
Argillander
^ permalink raw reply
* [PATCH 03/27] rtc: ac100: convert from divider_round_rate() to divider_determine_rate()
From: Brian Masney @ 2026-01-08 21:16 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-clk, linux-kernel, Brian Masney, Alexandre Belloni,
linux-rtc
In-Reply-To: <20260108-clk-divider-round-rate-v1-0-535a3ed73bf3@redhat.com>
The divider_round_rate() function is now deprecated, so let's migrate
to divider_determine_rate() instead so that this deprecated API can be
removed.
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: linux-rtc@vger.kernel.org
---
drivers/rtc/rtc-ac100.c | 75 +++++++++++++++++++++++++------------------------
1 file changed, 38 insertions(+), 37 deletions(-)
diff --git a/drivers/rtc/rtc-ac100.c b/drivers/rtc/rtc-ac100.c
index 33626311fa781b5ce90dcc472f948dc933bbc897..16aca4431da8c029e6195d8a3c9fe75fa95d0bc0 100644
--- a/drivers/rtc/rtc-ac100.c
+++ b/drivers/rtc/rtc-ac100.c
@@ -140,42 +140,16 @@ static unsigned long ac100_clkout_recalc_rate(struct clk_hw *hw,
AC100_CLKOUT_DIV_WIDTH);
}
-static long ac100_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
- unsigned long prate)
-{
- unsigned long best_rate = 0, tmp_rate, tmp_prate;
- int i;
-
- if (prate == AC100_RTC_32K_RATE)
- return divider_round_rate(hw, rate, &prate, NULL,
- AC100_CLKOUT_DIV_WIDTH,
- CLK_DIVIDER_POWER_OF_TWO);
-
- for (i = 0; ac100_clkout_prediv[i].div; i++) {
- tmp_prate = DIV_ROUND_UP(prate, ac100_clkout_prediv[i].val);
- tmp_rate = divider_round_rate(hw, rate, &tmp_prate, NULL,
- AC100_CLKOUT_DIV_WIDTH,
- CLK_DIVIDER_POWER_OF_TWO);
-
- if (tmp_rate > rate)
- continue;
- if (rate - tmp_rate < best_rate - tmp_rate)
- best_rate = tmp_rate;
- }
-
- return best_rate;
-}
-
static int ac100_clkout_determine_rate(struct clk_hw *hw,
struct clk_rate_request *req)
{
- struct clk_hw *best_parent;
+ int i, ret, num_parents = clk_hw_get_num_parents(hw);
+ struct clk_hw *best_parent = NULL;
unsigned long best = 0;
- int i, num_parents = clk_hw_get_num_parents(hw);
for (i = 0; i < num_parents; i++) {
struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
- unsigned long tmp, prate;
+ unsigned long prate;
/*
* The clock has two parents, one is a fixed clock which is
@@ -199,13 +173,40 @@ static int ac100_clkout_determine_rate(struct clk_hw *hw,
prate = clk_hw_get_rate(parent);
- tmp = ac100_clkout_round_rate(hw, req->rate, prate);
-
- if (tmp > req->rate)
- continue;
- if (req->rate - tmp < req->rate - best) {
- best = tmp;
- best_parent = parent;
+ if (prate == AC100_RTC_32K_RATE) {
+ struct clk_rate_request div_req = *req;
+
+ div_req.best_parent_rate = prate;
+
+ ret = divider_determine_rate(hw, &div_req, NULL,
+ AC100_CLKOUT_DIV_WIDTH,
+ CLK_DIVIDER_POWER_OF_TWO);
+ if (ret != 0 || div_req.rate > req->rate)
+ continue;
+ else if (req->rate - div_req.rate < req->rate - best) {
+ best = div_req.rate;
+ best_parent = parent;
+ }
+ } else {
+ int j;
+
+ for (j = 0; ac100_clkout_prediv[j].div; j++) {
+ struct clk_rate_request div_req = *req;
+ unsigned long tmp_prate;
+
+ tmp_prate = DIV_ROUND_UP(prate, ac100_clkout_prediv[j].div);
+ div_req.best_parent_rate = tmp_prate;
+
+ ret = divider_determine_rate(hw, &div_req, NULL,
+ AC100_CLKOUT_DIV_WIDTH,
+ CLK_DIVIDER_POWER_OF_TWO);
+ if (ret != 0 || div_req.rate > req->rate)
+ continue;
+ else if (req->rate - div_req.rate < req->rate - best) {
+ best = div_req.rate;
+ best_parent = parent;
+ }
+ }
}
}
@@ -213,7 +214,7 @@ static int ac100_clkout_determine_rate(struct clk_hw *hw,
return -EINVAL;
req->best_parent_hw = best_parent;
- req->best_parent_rate = best;
+ req->best_parent_rate = clk_hw_get_rate(best_parent);
req->rate = best;
return 0;
--
2.52.0
^ permalink raw reply related
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