* [PATCH v5 01/16] dt-bindings: regulator: ROHM BD72720
From: Matti Vaittinen @ 2025-11-20 8:19 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sebastian Reichel, Liam Girdwood, Mark Brown,
Michael Turquette, Stephen Boyd, Matti Vaittinen, Linus Walleij,
Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
Andreas Kemnade
In-Reply-To: <cover.1763625920.git.mazziesaccount@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 7242 bytes --]
From: Matti Vaittinen <mazziesaccount@gmail.com>
The ROHM BD72720 is a new PMIC with 10 BUCk and 11 LDO regulators.
The BD72720 is designed to support using the BUCK10 as a supply for
the LDOs 1 to 4. When the BUCK10 is used for this, it can be set to a
LDON_HEAD mode. In this mode, the BUCK10 voltage can't be controlled by
software, but the voltage is adjusted by PMIC to match the LDO1 .. LDO4
voltages with a given offset. Offset can be 50mV .. 300mV and is
changeable at 50mV steps.
Add 'ldon-head-microvolt' property to denote a board which is designed
to utilize the LDON_HEAD mode.
All other properties are already existing.
Add dt-binding doc for ROHM BD72720 regulators to make it usable.
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
Revision history:
v4 =>
- No changes
v3 => v4:
- Drop type from ldon-head
- Fix the name patterns for regulator nodes and names
v2 => v3:
- drop unnecessary descriptions
- use microvolts for the 'ldon-head' dt-property
RFCv1 => v2:
- No changes
---
.../regulator/rohm,bd72720-regulator.yaml | 148 ++++++++++++++++++
1 file changed, 148 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml
diff --git a/Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml b/Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml
new file mode 100644
index 000000000000..5518082129bd
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/rohm,bd72720-regulator.yaml
@@ -0,0 +1,148 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/rohm,bd72720-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD72720 Power Management Integrated Circuit regulators
+
+maintainers:
+ - Matti Vaittinen <mazziesaccount@gmail.com>
+
+description: |
+ This module is part of the ROHM BD72720 MFD device. For more details
+ see Documentation/devicetree/bindings/mfd/rohm,bd72720-pmic.yaml.
+
+ The regulator controller is represented as a sub-node of the PMIC node
+ on the device tree.
+
+ Regulator nodes should be named to BUCK_<number> and LDO_<number>.
+ The valid names for BD72720 regulator nodes are
+ buck1, buck2, buck3, buck4, buck5, buck6, buck7, buck8, buck9, buck10
+ ldo1, ldo2, ldo3, ldo4, ldo5, ldo6, ldo7, ldo8, ldo9, ldo10, ldo11
+
+patternProperties:
+ "^ldo([1-9]|1[0-1])$":
+ type: object
+ description:
+ Properties for single LDO regulator.
+ $ref: regulator.yaml#
+
+ properties:
+ regulator-name:
+ pattern: "^ldo([1-9]|1[0-1])$"
+
+ rohm,dvs-run-voltage:
+ description:
+ PMIC default "RUN" state voltage in uV. See below table for
+ LDOs which support this. 0 means disabled.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 3300000
+
+ rohm,dvs-idle-voltage:
+ description:
+ PMIC default "IDLE" state voltage in uV. See below table for
+ LDOs which support this. 0 means disabled.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 3300000
+
+ rohm,dvs-suspend-voltage:
+ description:
+ PMIC default "SUSPEND" state voltage in uV. See below table for
+ LDOs which support this. 0 means disabled.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 3300000
+
+ rohm,dvs-lpsr-voltage:
+ description:
+ PMIC default "deep-idle" state voltage in uV. See below table for
+ LDOs which support this. 0 means disabled.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 3300000
+
+ # Supported default DVS states:
+ # ldo | run | idle | suspend | lpsr
+ # --------------------------------------------------------------
+ # 1, 2, 3, and 4 | supported | supported | supported | supported
+ # --------------------------------------------------------------
+ # 5 - 11 | supported (*)
+ # --------------------------------------------------------------
+ #
+ # (*) All states use same voltage but have own enable / disable
+ # settings. Voltage 0 can be specified for a state to make
+ # regulator disabled on that state.
+
+ unevaluatedProperties: false
+
+ "^buck([1-9]|10)$":
+ type: object
+ description:
+ Properties for single BUCK regulator.
+ $ref: regulator.yaml#
+
+ properties:
+ regulator-name:
+ pattern: "^buck([1-9]|10)$"
+
+ rohm,ldon-head-microvolt:
+ description:
+ Set this on boards where BUCK10 is used to supply LDOs 1-4. The bucki
+ voltage will be changed by the PMIC to follow the LDO output voltages
+ with the offset voltage given here. This will improve the LDO efficiency.
+ minimum: 50000
+ maximum: 300000
+
+ rohm,dvs-run-voltage:
+ description:
+ PMIC default "RUN" state voltage in uV. See below table for
+ bucks which support this. 0 means disabled.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 3300000
+
+ rohm,dvs-idle-voltage:
+ description:
+ PMIC default "IDLE" state voltage in uV. See below table for
+ bucks which support this. 0 means disabled.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 3300000
+
+ rohm,dvs-suspend-voltage:
+ description:
+ PMIC default "SUSPEND" state voltage in uV. See below table for
+ bucks which support this. 0 means disabled.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 3300000
+
+ rohm,dvs-lpsr-voltage:
+ description:
+ PMIC default "deep-idle" state voltage in uV. See below table for
+ bucks which support this. 0 means disabled.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 3300000
+
+ # Supported default DVS states:
+ # buck | run | idle | suspend | lpsr
+ # --------------------------------------------------------------
+ # 1, 2, 3, and 4 | supported | supported | supported | supported
+ # --------------------------------------------------------------
+ # 5 - 10 | supported (*)
+ # --------------------------------------------------------------
+ #
+ # (*) All states use same voltage but have own enable / disable
+ # settings. Voltage 0 can be specified for a state to make
+ # regulator disabled on that state.
+
+ required:
+ - regulator-name
+
+ unevaluatedProperties: false
+
+additionalProperties: false
--
2.51.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related
* [PATCH v5 00/16] Support ROHM BD72720 PMIC
From: Matti Vaittinen @ 2025-11-20 8:19 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sebastian Reichel, Liam Girdwood, Mark Brown,
Michael Turquette, Stephen Boyd, Matti Vaittinen, Linus Walleij,
Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
Andreas Kemnade
[-- Attachment #1: Type: text/plain, Size: 4269 bytes --]
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 of v4 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 since changes in this revision
have been made to dt-bindings. It's still fair to note that though, as
my testing was impacted.
Revision history:
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 (16):
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: 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 | 546 ++++++++-
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, 3120 insertions(+), 130 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
base-commit: 8e8856396b54bea5c00a7ae88d87c6254aef2d94
--
2.51.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v3 5/5] rtc: pcf85363: add watchdog support with configurable step size
From: Guenter Roeck @ 2025-11-19 15:37 UTC (permalink / raw)
To: Lakshay Piplani, alexandre.belloni, linux-rtc, linux-kernel, robh,
krzk+dt, conor+dt, devicetree, wim, linux-watchdog
Cc: vikash.bansal, priyanka.jain, shashank.rebbapragada
In-Reply-To: <20251119083336.2241142-5-lakshay.piplani@nxp.com>
On 11/19/25 00:33, Lakshay Piplani wrote:
> Add watchdog timer support to PCF85263/PCF85363 using the linux watchdog
> subsystem. The driver programs the hardware watchdog timeout based on
> the requested period.
>
> Also use rtc_add_group() instead of sysfs_create_group() to register
> timestamp attributes under the RTC class device (/sys/class/rtc/rtcX).
>
> Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
> ---
> V2 -> V3:
> - Split into separate patches as suggested:
> - Battery switch-over detection.
> - Timestamp recording for TS pin and battery switch-over events.
> - Offset calibration.
> - Watchdog timer (to be reviewed by watchdog maintainers).
> - Dropped Alarm2 support
> - Switched to rtc_add_group() for sysfs attributes
> - Removed failure paths after RTC device registration as per subsystem guidelines.
> V1 -> V2:
> - Watchdog related changes due to removal of vendor specific properties
> from device tree
> * remove vendor DT knobs (enable/timeout/stepsize/repeat)
> * use watchdog_init_timeout (with 10s default)
> * derive clock_sel from final timeout
> * default, repeat=true (repeat mode)
> - Fixed uninitalised warning on 'ret' (reported by kernel test robot)
> - Use dev_dbg instead of dev_info for debug related print messages
> - Minor cleanup and comments
>
> drivers/rtc/rtc-pcf85363.c | 168 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 160 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
> index 3d733375187b..34d4c2e16774 100644
> --- a/drivers/rtc/rtc-pcf85363.c
> +++ b/drivers/rtc/rtc-pcf85363.c
> @@ -5,6 +5,10 @@
> * Driver for NXP PCF85363 real-time clock.
> *
> * Copyright (C) 2017 Eric Nelson
> + *
> + * Copyright 2025 NXP
> + * Added support for timestamps, battery switch-over,
> + * watchdog, offset calibration.
> */
> #include <linux/module.h>
> #include <linux/i2c.h>
> @@ -17,6 +21,8 @@
> #include <linux/device.h>
> #include <linux/of.h>
> #include <linux/regmap.h>
> +#include <linux/rtc.h>
> +#include <linux/watchdog.h>
>
> /*
> * Date/Time registers
> @@ -127,6 +133,18 @@
> #define OFFSET_MAXIMUM 127
> #define OFFSET_MASK 0xFF
>
> +#define WD_MODE_REPEAT BIT(7)
> +#define WD_TIMEOUT_MASK GENMASK(6, 2)
> +#define WD_TIMEOUT_SHIFT 2
> +#define WD_CLKSEL_MASK GENMASK(1, 0)
> +#define WD_CLKSEL_0_25HZ 0x00
> +#define WD_CLKSEL_1HZ 0x01
> +#define WD_CLKSEL_4HZ 0x02
> +#define WD_CLKSEL_16HZ 0x03
> +
> +#define WD_TIMEOUT_MIN 1
> +#define WD_TIMEOUT_MAX 0x1F
> +
> struct pcf85363 {
> struct rtc_device *rtc;
> struct regmap *regmap;
> @@ -138,6 +156,15 @@ struct pcf85x63_config {
> unsigned int num_nvram;
> };
>
> +struct pcf85363_watchdog {
> + struct watchdog_device wdd;
> + struct regmap *regmap;
> + struct device *dev;
> + u8 timeout_val;
> + u8 clock_sel;
> + bool repeat;
> +};
> +
> static int pcf85363_load_capacitance(struct pcf85363 *pcf85363, struct device_node *node)
> {
> u32 load = 7000;
> @@ -323,12 +350,13 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
> return IRQ_NONE;
>
> if (flags) {
> - dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s%s%s%s\n",
> + dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s%s%s%s%s\n",
> flags, (flags & FLAGS_A1F) ? " [A1F]" : "",
> (flags & FLAGS_TSR1F) ? " [TSR1F]" : "",
> (flags & FLAGS_TSR2F) ? " [TSR2F]" : "",
> (flags & FLAGS_TSR3F) ? " [TSR3F]" : "",
> - (flags & FLAGS_BSF) ? " [BSF]" : "");
> + (flags & FLAGS_BSF) ? " [BSF]" : "",
> + (flags & FLAGS_WDF) ? " [WDF]" : "");
> }
>
> if (flags & FLAGS_A1F) {
> @@ -360,6 +388,11 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
> handled = true;
> }
>
> + if (flags & FLAGS_WDF) {
> + regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_WDF, 0);
> + handled = true;
> + }
> +
> return handled ? IRQ_HANDLED : IRQ_NONE;
> }
>
> @@ -503,6 +536,123 @@ static const struct pcf85x63_config pcf_85363_config = {
> .num_nvram = 2
> };
>
> +/*
> + * This function sets the watchdog control register based on the timeout,
> + * clock selection and repeat mode settings. It prepares the value to
> + * write into the watchdog control register (CTRL_WDOG).
> + */
> +static int pcf85363_wdt_reload(struct pcf85363_watchdog *wd)
> +{
> + u8 val;
> +
> + val = (wd->repeat ? WD_MODE_REPEAT : 0) |
> + ((wd->timeout_val & WD_TIMEOUT_MAX) << WD_TIMEOUT_SHIFT) |
> + (wd->clock_sel & WD_CLKSEL_MASK);
> +
> + return regmap_write(wd->regmap, CTRL_WDOG, val);
> +}
> +
> +static int pcf85363_wdt_start(struct watchdog_device *wdd)
> +{
> + struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
> +
> + return pcf85363_wdt_reload(wd);
> +}
> +
> +static int pcf85363_wdt_stop(struct watchdog_device *wdd)
> +{
> + struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
> +
> + return regmap_write(wd->regmap, CTRL_WDOG, 0);
> +}
> +
> +static int pcf85363_wdt_ping(struct watchdog_device *wdd)
> +{
> + struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
> +
> + regmap_update_bits(wd->regmap, CTRL_FLAGS, FLAGS_WDF, 0);
> +
> + return pcf85363_wdt_reload(wd);
> +}
> +
> +static int pcf85363_wdt_set_timeout(struct watchdog_device *wdd,
> + unsigned int timeout)
> +{
> + struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
> +
> + wd->timeout_val = clamp(timeout, WD_TIMEOUT_MIN, WD_TIMEOUT_MAX);
> + wdd->timeout = wd->timeout_val;
> +
> + return pcf85363_wdt_reload(wd);
> +}
> +
> +static const struct watchdog_info pcf85363_wdt_info = {
> + .identity = "PCF85363 Watchdog",
> + .options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
> +};
> +
> +static const struct watchdog_ops pcf85363_wdt_ops = {
> + .owner = THIS_MODULE,
> + .start = pcf85363_wdt_start,
> + .stop = pcf85363_wdt_stop,
> + .ping = pcf85363_wdt_ping,
> + .set_timeout = pcf85363_wdt_set_timeout,
> +};
> +
> +static int pcf85363_watchdog_init(struct device *dev, struct regmap *regmap)
> +{
> + struct pcf85363_watchdog *wd;
> + unsigned int timeout_sec;
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_WATCHDOG))
> + return 0;
> +
> + wd = devm_kzalloc(dev, sizeof(*wd), GFP_KERNEL);
> + if (!wd)
> + return -ENOMEM;
> +
> + wd->regmap = regmap;
> + wd->dev = dev;
> +
> + wd->wdd.info = &pcf85363_wdt_info;
> + wd->wdd.ops = &pcf85363_wdt_ops;
> + wd->wdd.min_timeout = WD_TIMEOUT_MIN;
> + wd->wdd.max_timeout = WD_TIMEOUT_MAX;
> + wd->wdd.parent = dev;
> + wd->wdd.status = WATCHDOG_NOWAYOUT_INIT_STATUS;
> +
> + ret = watchdog_init_timeout(&wd->wdd, 10, dev);
Calling watchdog_init_timeout() with a value other than 0 means that
a parameter from devicetree won't be accepted. Calling it with a fixed
value is usually pointless unless the value is out of the valid range,
which by itself would be pointless.
watchdog_init_timeout() is normally called to pass and validate a module
parameter or to pick a timeout from devicetree. Calling it with a constant
value other than 0 is unnecessary.
> + if (ret)
> + wd->wdd.timeout = clamp(10U, WD_TIMEOUT_MIN, WD_TIMEOUT_MAX);
So if 10 seconds is invalid, 10 is clamped to [1, 31] and applied directly.
That is an odd and complicated way of setting the timeout to 10 seconds.
If you don't want a timeout value from devicetree to be accepted, just make this
wd->wdd.timeout = 10;
and do not call watchdog_init_timeout() in the first place.
> +
> + timeout_sec = wd->wdd.timeout;
> +
> + if (timeout_sec <= 2)
> + wd->clock_sel = WD_CLKSEL_16HZ;
> + else if (timeout_sec <= 8)
> + wd->clock_sel = WD_CLKSEL_4HZ;
> + else if (timeout_sec <= 16)
> + wd->clock_sel = WD_CLKSEL_1HZ;
> + else
> + wd->clock_sel = WD_CLKSEL_0_25HZ;
> +
This seems an odd location for this code. What if the timeout changes
later on to one of the other values ?
Also, the timeout is set to a fixed value of 10. That means the above
can be simplified to
wd->clock_sel = WD_CLKSEL_1HZ;
... and that in turn means that the variable is pointless, and that
WD_CLKSEL_1HZ could be used as constant instead.
Why all that complexity ? Am I missing something ? I am quite concerned that
I may be missing trees in the forest, meaning that the real problems are hiding
behind the noise.
Guenter
> + wd->repeat = true;
What is the purpose of this variable ? It is always set to true.
You might as well drop it.
> +
> + ret = regmap_update_bits(regmap, CTRL_FLAGS, FLAGS_WDF, 0);
> + if (ret) {
> + dev_err(dev, "failed to clear WDF:%d\n", ret);
> + return ret;
> + }
> +
> + watchdog_set_drvdata(&wd->wdd, wd);
> +
> + dev_dbg(dev, "pcf85363 watchdog registered (timeout=%us, clk_sel=%u)\n",
> + timeout_sec, wd->clock_sel);
> +
> + return devm_watchdog_register_device(dev, &wd->wdd);
> +}
> +
> /*
> * Reads 6 bytes of timestamp data starting at the given base register,
> * converts them from BCD to binary, and formats the result into a
> @@ -684,20 +834,22 @@ static int pcf85363_probe(struct i2c_client *client)
> PIN_IO_TSPM | PIN_IO_TSIM,
> PIN_IO_TSPM | PIN_IO_TSIM);
>
> + ret = pcf85363_watchdog_init(dev, pcf85363->regmap);
> + if (ret)
> + dev_err_probe(dev, ret, "Watchdog init failed\n");
> +
> if (irq_a > 0 || wakeup_source)
> device_init_wakeup(dev, true);
>
> dev_set_drvdata(&pcf85363->rtc->dev, pcf85363);
>
> - ret = devm_rtc_register_device(pcf85363->rtc);
> -
> + ret = rtc_add_group(pcf85363->rtc, &pcf85363_attr_group);
> if (ret)
> - return dev_err_probe(dev, ret, "RTC registration failed\n");
> -
> - ret = sysfs_create_group(&pcf85363->rtc->dev.kobj, &pcf85363_attr_group);
> + return ret;
>
> + ret = devm_rtc_register_device(pcf85363->rtc);
> if (ret)
> - return dev_err_probe(dev, ret, "Timestamp sysfs creation failed\n");
> + return dev_err_probe(dev, ret, "RTC registration failed\n");
>
It is not entirely obvious how those changes are related to adding watchdog support
to this driver.
> for (i = 0; i < config->num_nvram; i++) {
> nvmem_cfg[i].priv = pcf85363;
^ permalink raw reply
* RE: [EXT] Re: [PATCH v3 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config
From: Lakshay Piplani @ 2025-11-19 11:24 UTC (permalink / raw)
To: Krzysztof Kozlowski, alexandre.belloni@bootlin.com,
linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org,
robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
devicetree@vger.kernel.org, wim@linux-watchdog.org,
linux@roeck-us.net, linux-watchdog@vger.kernel.org
Cc: Vikash Bansal, Priyanka Jain, Shashank Rebbapragada
In-Reply-To: <0bc4d069-29bf-4d22-9a38-f9f6222fc07d@kernel.org>
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Wednesday, November 19, 2025 3:53 PM
> To: Lakshay Piplani <lakshay.piplani@nxp.com>;
> alexandre.belloni@bootlin.com; linux-rtc@vger.kernel.org; linux-
> kernel@vger.kernel.org; robh@kernel.org; krzk+dt@kernel.org;
> conor+dt@kernel.org; devicetree@vger.kernel.org; wim@linux-watchdog.org;
> linux@roeck-us.net; linux-watchdog@vger.kernel.org
> Cc: Vikash Bansal <vikash.bansal@nxp.com>; Priyanka Jain
> <priyanka.jain@nxp.com>; Shashank Rebbapragada
> <shashank.rebbapragada@nxp.com>
> Subject: [EXT] Re: [PATCH v3 1/5] dt-bindings: rtc: nxp,pcf85363: add
> timestamp mode config
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> On 19/11/2025 09:33, Lakshay Piplani wrote:
> > NXP PCF85263/PCF85363 provides three timestamp registers (TSR1-TSR3)
> > which latch the current time when a selected event occurs. Add a
> > vendor specific property, nxp,timestamp-mode, to select the event
> > source for each register.
> >
> > Also introduce a new header 'pcf85363-tsr.h' to expose macros for
> > timestamp mode fields, improving readability of device tree file.
> >
> > Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
> > ---
> > V2 -> V3:
> > - No changes in v3
> > - Added Reviewed-by: Rob Herring <robh@kernel.org>
>
> I don't see it. Please start using b4, so such trivialities won't affect the process.
>
> Best regards,
> Krzysztof
Hi Krzysztof,
Thanks for pointing that out.
I mistakenly mentioned the Reviewed-by tag in the changelog but forgot to include it in the actual commit. I’ll make sure to add it in the v4 DT patch and start using b4 for preparing future revisions to avoid such issues.
Best regards,
Lakshay
^ permalink raw reply
* Re: [PATCH v3 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config
From: Krzysztof Kozlowski @ 2025-11-19 10:22 UTC (permalink / raw)
To: Lakshay Piplani, alexandre.belloni, linux-rtc, linux-kernel, robh,
krzk+dt, conor+dt, devicetree, wim, linux, linux-watchdog
Cc: vikash.bansal, priyanka.jain, shashank.rebbapragada
In-Reply-To: <20251119083336.2241142-1-lakshay.piplani@nxp.com>
On 19/11/2025 09:33, Lakshay Piplani wrote:
> NXP PCF85263/PCF85363 provides three timestamp registers (TSR1-TSR3)
> which latch the current time when a selected event occurs. Add a
> vendor specific property, nxp,timestamp-mode, to select the event
> source for each register.
>
> Also introduce a new header 'pcf85363-tsr.h' to expose
> macros for timestamp mode fields, improving readability
> of device tree file.
>
> Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
> ---
> V2 -> V3:
> - No changes in v3
> - Added Reviewed-by: Rob Herring <robh@kernel.org>
I don't see it. Please start using b4, so such trivialities won't affect
the process.
Best regards,
Krzysztof
^ permalink raw reply
* Wiadomość z księgowości
From: Marek Poradecki @ 2025-11-19 9:01 UTC (permalink / raw)
To: linux-rtc
Dzień dobry,
pomagamy przedsiębiorcom wprowadzić model wymiany walut, który minimalizuje wahania kosztów przy rozliczeniach międzynarodowych.
Kiedyv możemy umówić się na 15-minutową rozmowę, aby zaprezentować, jak taki model mógłby działać w Państwa firmie - z gwarancją indywidualnych kursów i pełnym uproszczeniem płatności? Proszę o propozycję dogodnego terminu.
Pozdrawiam
Marek Poradecki
^ permalink raw reply
* [PATCH v3 4/5] rtc: pcf85363: add oscillator offset calibration support
From: Lakshay Piplani @ 2025-11-19 8:33 UTC (permalink / raw)
To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
conor+dt, devicetree, wim, linux, linux-watchdog
Cc: vikash.bansal, priyanka.jain, shashank.rebbapragada,
Lakshay Piplani
In-Reply-To: <20251119083336.2241142-1-lakshay.piplani@nxp.com>
Expose the oscillator offset register of PCF85263/PCF85363 through the
rtc_class_ops read_offset and set_offset callbacks, allowing userspace
to apply frequency correction for drift compensation.
The correction mode defaults to normal mode (OFFM = 0), where each step
introduces an offset of approximately 2.170 ppm and corrections occur
every 4 hours.
Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
V2 -> V3:
- Split into separate patches as suggested:
- Battery switch-over detection.
- Timestamp recording for TS pin and battery switch-over events.
- Offset calibration.
- Watchdog timer (to be reviewed by watchdog maintainers).
- Dropped Alarm2 support
- Switched to rtc_add_group() for sysfs attributes
- Removed failure paths after RTC device registration as per subsystem guidelines.
V1 -> V2:
- Watchdog related changes due to removal of vendor specific properties
from device tree
* remove vendor DT knobs (enable/timeout/stepsize/repeat)
* use watchdog_init_timeout (with 10s default)
* derive clock_sel from final timeout
* default, repeat=true (repeat mode)
- Fixed uninitalised warning on 'ret' (reported by kernel test robot)
- Use dev_dbg instead of dev_info for debug related print messages
- Minor cleanup and comments
drivers/rtc/rtc-pcf85363.c | 46 ++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index a8b4f48d9894..3d733375187b 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -122,6 +122,11 @@
#define TSR2_SHIFT 2
#define TSR3_SHIFT 6
+#define OFFSET_SIGN_BIT 7
+#define OFFSET_MINIMUM -128
+#define OFFSET_MAXIMUM 127
+#define OFFSET_MASK 0xFF
+
struct pcf85363 {
struct rtc_device *rtc;
struct regmap *regmap;
@@ -358,6 +363,45 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
return handled ? IRQ_HANDLED : IRQ_NONE;
}
+/*
+ * Read the current RTC offset from the CTRL_OFFSET
+ * register. This value is an 8-bit signed 2's complement
+ * value that corrects osciallator drift.
+ */
+static int pcf85363_read_offset(struct device *dev, long *offset)
+{
+ struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(pcf85363->regmap, CTRL_OFFSET, &val);
+
+ if (ret)
+ return ret;
+
+ *offset = sign_extend32(val & OFFSET_MASK, OFFSET_SIGN_BIT);
+
+ return 0;
+}
+
+/*
+ * Write an oscillator offset correction value to
+ * the CTRL_OFFSET register. The valid range is
+ * -128 to 127 (8-bit signed), typically used to fine
+ * tune accuracy.
+ */
+static int pcf85363_set_offset(struct device *dev, long offset)
+{
+ struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+
+ if (offset < OFFSET_MINIMUM || offset > OFFSET_MAXIMUM) {
+ dev_warn(dev, "Offset out of range: %ld\n", offset);
+ return -ERANGE;
+ }
+
+ return regmap_write(pcf85363->regmap, CTRL_OFFSET, offset & OFFSET_MASK);
+}
+
static int pcf85363_rtc_ioctl(struct device *dev,
unsigned int cmd, unsigned long arg)
{
@@ -395,6 +439,8 @@ static const struct rtc_class_ops rtc_ops = {
.read_alarm = pcf85363_rtc_read_alarm,
.set_alarm = pcf85363_rtc_set_alarm,
.alarm_irq_enable = pcf85363_rtc_alarm_irq_enable,
+ .read_offset = pcf85363_read_offset,
+ .set_offset = pcf85363_set_offset,
};
static int pcf85363_nvram_read(void *priv, unsigned int offset, void *val,
--
2.25.1
^ permalink raw reply related
* [PATCH v3 3/5] rtc: pcf85363: add timestamp support with configurable timestamp mode
From: Lakshay Piplani @ 2025-11-19 8:33 UTC (permalink / raw)
To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
conor+dt, devicetree, wim, linux, linux-watchdog
Cc: vikash.bansal, priyanka.jain, shashank.rebbapragada,
Lakshay Piplani
In-Reply-To: <20251119083336.2241142-1-lakshay.piplani@nxp.com>
Add support for the timestamp capture registers available on PCF85263 and
PCF85363. The registers latch the current time when selected events occur,
such as TS pin activation or battery switch-over.
The capture source can be configured via the nxp,timestamp-mode device
tree property, and latched values are exported through read-only sysfs
attributes.
Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
V2 -> V3:
- Split into separate patches as suggested:
- Battery switch-over detection.
- Timestamp recording for TS pin and battery switch-over events.
- Offset calibration.
- Watchdog timer (to be reviewed by watchdog maintainers).
- Dropped Alarm2 support
- Switched to rtc_add_group() for sysfs attributes
- Removed failure paths after RTC device registration as per subsystem guidelines.
V1 -> V2:
- Watchdog related changes due to removal of vendor specific properties
from device tree
* remove vendor DT knobs (enable/timeout/stepsize/repeat)
* use watchdog_init_timeout (with 10s default)
* derive clock_sel from final timeout
* default, repeat=true (repeat mode)
- Fixed uninitalised warning on 'ret' (reported by kernel test robot)
- Use dev_dbg instead of dev_info for debug related print messages
- Minor cleanup and comments
drivers/rtc/rtc-pcf85363.c | 210 +++++++++++++++++++++++++++++++------
1 file changed, 176 insertions(+), 34 deletions(-)
diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index c03d5a65c5f7..a8b4f48d9894 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -101,19 +101,31 @@
#define PIN_IO_INTA_OUT 2
#define PIN_IO_INTA_HIZ 3
+#define PIN_IO_TSPM GENMASK(3, 2)
+#define PIN_IO_TSIM BIT(4)
+
#define OSC_CAP_SEL GENMASK(1, 0)
#define OSC_CAP_6000 0x01
#define OSC_CAP_12500 0x02
#define STOP_EN_STOP BIT(0)
+#define RTCM_BIT BIT(4)
#define RESET_CPR 0xa4
#define NVRAM_SIZE 0x40
+#define TSR1_MASK 0x03
+#define TSR2_MASK 0x07
+#define TSR3_MASK 0x03
+#define TSR1_SHIFT 0
+#define TSR2_SHIFT 2
+#define TSR3_SHIFT 6
+
struct pcf85363 {
struct rtc_device *rtc;
struct regmap *regmap;
+ u8 ts_valid_flags;
};
struct pcf85x63_config {
@@ -306,8 +318,11 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
return IRQ_NONE;
if (flags) {
- dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s\n",
+ dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s%s%s%s\n",
flags, (flags & FLAGS_A1F) ? " [A1F]" : "",
+ (flags & FLAGS_TSR1F) ? " [TSR1F]" : "",
+ (flags & FLAGS_TSR2F) ? " [TSR2F]" : "",
+ (flags & FLAGS_TSR3F) ? " [TSR3F]" : "",
(flags & FLAGS_BSF) ? " [BSF]" : "");
}
@@ -317,6 +332,24 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
handled = true;
}
+ if (flags & FLAGS_TSR1F) {
+ regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_TSR1F, 0);
+ pcf85363->ts_valid_flags |= FLAGS_TSR1F;
+ handled = true;
+ }
+
+ if (flags & FLAGS_TSR2F) {
+ regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_TSR2F, 0);
+ pcf85363->ts_valid_flags |= FLAGS_TSR2F;
+ handled = true;
+ }
+
+ if (flags & FLAGS_TSR3F) {
+ regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_TSR3F, 0);
+ pcf85363->ts_valid_flags |= FLAGS_TSR3F;
+ handled = true;
+ }
+
if (flags & FLAGS_BSF) {
regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_BSF, 0);
handled = true;
@@ -424,11 +457,94 @@ static const struct pcf85x63_config pcf_85363_config = {
.num_nvram = 2
};
+/*
+ * Reads 6 bytes of timestamp data starting at the given base register,
+ * converts them from BCD to binary, and formats the result into a
+ * human-readable string in "YYYY-MM-DD HH:MM:SS" format.
+ */
+static int pcf85363_read_timestamp(struct pcf85363 *pcf85363, u8 base_reg, char *buf)
+{
+ struct rtc_time tm;
+ u8 regs[6];
+ int ret;
+
+ ret = regmap_bulk_read(pcf85363->regmap, base_reg, regs, sizeof(regs));
+
+ if (ret)
+ return ret;
+
+ tm.tm_sec = bcd2bin(regs[0]);
+ tm.tm_min = bcd2bin(regs[1]);
+ tm.tm_hour = bcd2bin(regs[2]);
+ tm.tm_mday = bcd2bin(regs[3]);
+ tm.tm_mon = bcd2bin(regs[4]) - 1;
+ tm.tm_year = bcd2bin(regs[5]) + 100;
+
+ return sysfs_emit(buf, "%04d-%02d-%02d %02d:%02d:%02d\n",
+ tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
+ tm.tm_hour, tm.tm_min, tm.tm_sec);
+}
+
+/*
+ * Checks whether a specific timestamp flag is set. If so, reads and
+ * returns the formatted timestamp. Otherwise, returns "00-00-00 00:00:00".
+ */
+
+static ssize_t pcf85363_timestamp_show(struct device *dev, char *buf,
+ u8 timestamp_flag, u8 base_reg)
+{
+ struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+
+ if (!(pcf85363->ts_valid_flags & timestamp_flag))
+ return sysfs_emit(buf, "00-00-00 00:00:00\n");
+
+ return pcf85363_read_timestamp(pcf85363, base_reg, buf);
+}
+
+static ssize_t timestamp1_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return pcf85363_timestamp_show(dev, buf, FLAGS_TSR1F, DT_TIMESTAMP1);
+}
+static DEVICE_ATTR_RO(timestamp1);
+
+static ssize_t timestamp2_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return pcf85363_timestamp_show(dev, buf, FLAGS_TSR2F, DT_TIMESTAMP2);
+}
+static DEVICE_ATTR_RO(timestamp2);
+
+static ssize_t timestamp3_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return pcf85363_timestamp_show(dev, buf, FLAGS_TSR3F, DT_TIMESTAMP3);
+}
+static DEVICE_ATTR_RO(timestamp3);
+
+static struct attribute *pcf85363_attrs[] = {
+ &dev_attr_timestamp1.attr,
+ &dev_attr_timestamp2.attr,
+ &dev_attr_timestamp3.attr,
+ NULL,
+};
+
+static const struct attribute_group pcf85363_attr_group = {
+ .attrs = pcf85363_attrs,
+};
+
static int pcf85363_probe(struct i2c_client *client)
{
- struct pcf85363 *pcf85363;
const struct pcf85x63_config *config = &pcf_85363_config;
const void *data = of_device_get_match_data(&client->dev);
+ struct device *dev = &client->dev;
+ struct pcf85363 *pcf85363;
+ int irq_a = client->irq;
+ bool wakeup_source;
+ int ret, i, err;
+ u32 tsr_mode[3];
+ u8 val;
+
static struct nvmem_config nvmem_cfg[] = {
{
.name = "pcf85x63-",
@@ -446,25 +562,43 @@ static int pcf85363_probe(struct i2c_client *client)
.reg_write = pcf85363_nvram_write,
},
};
- int ret, i, err;
- bool wakeup_source;
if (data)
config = data;
- pcf85363 = devm_kzalloc(&client->dev, sizeof(struct pcf85363),
- GFP_KERNEL);
+ pcf85363 = devm_kzalloc(&client->dev, sizeof(*pcf85363), GFP_KERNEL);
if (!pcf85363)
return -ENOMEM;
+ pcf85363->ts_valid_flags = 0;
+
pcf85363->regmap = devm_regmap_init_i2c(client, &config->regmap);
- if (IS_ERR(pcf85363->regmap)) {
- dev_err(&client->dev, "regmap allocation failed\n");
- return PTR_ERR(pcf85363->regmap);
- }
+ if (IS_ERR(pcf85363->regmap))
+ return dev_err_probe(dev, PTR_ERR(pcf85363->regmap), "regmap init failed\n");
i2c_set_clientdata(client, pcf85363);
+ ret = regmap_update_bits(pcf85363->regmap, CTRL_FUNCTION, RTCM_BIT, 0);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to enable RTC mode\n");
+
+ if (!device_property_read_u32_array(dev, "nxp,timestamp-mode", tsr_mode, 3)) {
+ tsr_mode[0] &= TSR1_MASK;
+ tsr_mode[1] &= TSR2_MASK;
+ tsr_mode[2] &= TSR3_MASK;
+
+ val = (tsr_mode[2] << TSR3_SHIFT) |
+ (tsr_mode[1] << TSR2_SHIFT) |
+ (tsr_mode[0] << TSR1_SHIFT);
+
+ ret = regmap_write(pcf85363->regmap, DT_TS_MODE, val);
+ if (ret)
+ dev_warn(dev, "Failed to write timestamp mode register\n");
+
+ dev_dbg(dev, "Timestamp mode set: TSR1=0x%x TSR2=0x%x TSR3=0x%x\n",
+ tsr_mode[0], tsr_mode[1], tsr_mode[2]);
+ }
+
pcf85363->rtc = devm_rtc_allocate_device(&client->dev);
if (IS_ERR(pcf85363->rtc))
return PTR_ERR(pcf85363->rtc);
@@ -478,39 +612,47 @@ static int pcf85363_probe(struct i2c_client *client)
pcf85363->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
pcf85363->rtc->range_max = RTC_TIMESTAMP_END_2099;
- wakeup_source = device_property_read_bool(&client->dev,
- "wakeup-source");
- if (client->irq > 0 || wakeup_source) {
- regmap_write(pcf85363->regmap, CTRL_FLAGS, 0);
- regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
- PIN_IO_INTAPM, PIN_IO_INTA_OUT);
- }
+ wakeup_source = device_property_read_bool(dev, "wakeup-source");
+
+ ret = regmap_write(pcf85363->regmap, CTRL_FLAGS, 0x00);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to clear CTRL_FLAGS\n");
- if (client->irq > 0) {
- unsigned long irqflags = IRQF_TRIGGER_LOW;
+ if (irq_a > 0) {
+ regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO, PIN_IO_INTAPM, PIN_IO_INTA_OUT);
+ ret = devm_request_threaded_irq(dev, irq_a, NULL,
+ pcf85363_rtc_handle_irq,
+ IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+ "pcf85363-inta", client);
- if (dev_fwnode(&client->dev))
- irqflags = 0;
- ret = devm_request_threaded_irq(&client->dev, client->irq,
- NULL, pcf85363_rtc_handle_irq,
- irqflags | IRQF_ONESHOT,
- "pcf85363", client);
if (ret) {
- dev_warn(&client->dev,
- "unable to request IRQ, alarms disabled\n");
- client->irq = 0;
+ dev_err_probe(dev, ret, "INTA IRQ request failed\n");
+ irq_a = 0;
+ } else {
+ regmap_write(pcf85363->regmap, CTRL_INTA_EN, INT_BSIE
+ | INT_TSRIE);
}
}
- if (client->irq > 0 || wakeup_source) {
- device_init_wakeup(&client->dev, true);
- set_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
- } else {
- clear_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
- }
+ regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
+ PIN_IO_TSPM | PIN_IO_TSIM,
+ PIN_IO_TSPM | PIN_IO_TSIM);
+
+ if (irq_a > 0 || wakeup_source)
+ device_init_wakeup(dev, true);
+
+ dev_set_drvdata(&pcf85363->rtc->dev, pcf85363);
ret = devm_rtc_register_device(pcf85363->rtc);
+ if (ret)
+ return dev_err_probe(dev, ret, "RTC registration failed\n");
+
+ ret = sysfs_create_group(&pcf85363->rtc->dev.kobj, &pcf85363_attr_group);
+
+ if (ret)
+ return dev_err_probe(dev, ret, "Timestamp sysfs creation failed\n");
+
for (i = 0; i < config->num_nvram; i++) {
nvmem_cfg[i].priv = pcf85363;
devm_rtc_nvmem_register(pcf85363->rtc, &nvmem_cfg[i]);
--
2.25.1
^ permalink raw reply related
* [PATCH v3 2/5] rtc: pcf85363: support reporting battery switch-over via RTC_VL
From: Lakshay Piplani @ 2025-11-19 8:33 UTC (permalink / raw)
To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
conor+dt, devicetree, wim, linux, linux-watchdog
Cc: vikash.bansal, priyanka.jain, shashank.rebbapragada,
Lakshay Piplani
In-Reply-To: <20251119083336.2241142-1-lakshay.piplani@nxp.com>
Add battery switch-over reporting for PCF85263/PCF85363 using the standard
RTC_VL_* ioctl interface. When the backup supply takes over, the BSF flag
is exposed to userspace through RTC_VL_READ and can be cleared using
RTC_VL_CLR.
This allows applications to detect loss of main power without relying on
non-standard interfaces.
Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
V2 -> V3:
- Split into separate patches as suggested:
- Battery switch-over detection.
- Timestamp recording for TS pin and battery switch-over events.
- Offset calibration.
- Watchdog timer (to be reviewed by watchdog maintainers).
- Dropped Alarm2 support
- Switched to rtc_add_group() for sysfs attributes
- Removed failure paths after RTC device registration as per subsystem guidelines.
V1 -> V2:
- Watchdog related changes due to removal of vendor specific properties
from device tree
* remove vendor DT knobs (enable/timeout/stepsize/repeat)
* use watchdog_init_timeout (with 10s default)
* derive clock_sel from final timeout
* default, repeat=true (repeat mode)
- Fixed uninitalised warning on 'ret' (reported by kernel test robot)
- Use dev_dbg instead of dev_info for debug related print messages
- Minor cleanup and commentsi
drivers/rtc/rtc-pcf85363.c | 49 ++++++++++++++++++++++++++++++++++++--
1 file changed, 47 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index 540042b9eec8..c03d5a65c5f7 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -14,6 +14,7 @@
#include <linux/err.h>
#include <linux/errno.h>
#include <linux/bcd.h>
+#include <linux/device.h>
#include <linux/of.h>
#include <linux/regmap.h>
@@ -295,23 +296,67 @@ static int pcf85363_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
{
struct pcf85363 *pcf85363 = i2c_get_clientdata(dev_id);
+ bool handled = false;
unsigned int flags;
int err;
err = regmap_read(pcf85363->regmap, CTRL_FLAGS, &flags);
+
if (err)
return IRQ_NONE;
+ if (flags) {
+ dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s\n",
+ flags, (flags & FLAGS_A1F) ? " [A1F]" : "",
+ (flags & FLAGS_BSF) ? " [BSF]" : "");
+ }
+
if (flags & FLAGS_A1F) {
rtc_update_irq(pcf85363->rtc, 1, RTC_IRQF | RTC_AF);
regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_A1F, 0);
- return IRQ_HANDLED;
+ handled = true;
}
- return IRQ_NONE;
+ if (flags & FLAGS_BSF) {
+ regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_BSF, 0);
+ handled = true;
+ }
+
+ return handled ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static int pcf85363_rtc_ioctl(struct device *dev,
+ unsigned int cmd, unsigned long arg)
+{
+ struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+ unsigned int val;
+ int ret;
+
+ switch (cmd) {
+ case RTC_VL_READ: {
+ u32 status = 0;
+
+ ret = regmap_read(pcf85363->regmap, CTRL_FLAGS, &val);
+
+ if (ret)
+ return ret;
+
+ if (val & FLAGS_BSF)
+ status |= RTC_VL_BACKUP_SWITCH;
+
+ return put_user(status, (u32 __user *)arg);
+ }
+
+ case RTC_VL_CLR:
+ return regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_BSF, 0);
+
+ default:
+ return -ENOIOCTLCMD;
+ }
}
static const struct rtc_class_ops rtc_ops = {
+ .ioctl = pcf85363_rtc_ioctl,
.read_time = pcf85363_rtc_read_time,
.set_time = pcf85363_rtc_set_time,
.read_alarm = pcf85363_rtc_read_alarm,
--
2.25.1
^ permalink raw reply related
* [PATCH v3 5/5] rtc: pcf85363: add watchdog support with configurable step size
From: Lakshay Piplani @ 2025-11-19 8:33 UTC (permalink / raw)
To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
conor+dt, devicetree, wim, linux, linux-watchdog
Cc: vikash.bansal, priyanka.jain, shashank.rebbapragada,
Lakshay Piplani
In-Reply-To: <20251119083336.2241142-1-lakshay.piplani@nxp.com>
Add watchdog timer support to PCF85263/PCF85363 using the linux watchdog
subsystem. The driver programs the hardware watchdog timeout based on
the requested period.
Also use rtc_add_group() instead of sysfs_create_group() to register
timestamp attributes under the RTC class device (/sys/class/rtc/rtcX).
Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
V2 -> V3:
- Split into separate patches as suggested:
- Battery switch-over detection.
- Timestamp recording for TS pin and battery switch-over events.
- Offset calibration.
- Watchdog timer (to be reviewed by watchdog maintainers).
- Dropped Alarm2 support
- Switched to rtc_add_group() for sysfs attributes
- Removed failure paths after RTC device registration as per subsystem guidelines.
V1 -> V2:
- Watchdog related changes due to removal of vendor specific properties
from device tree
* remove vendor DT knobs (enable/timeout/stepsize/repeat)
* use watchdog_init_timeout (with 10s default)
* derive clock_sel from final timeout
* default, repeat=true (repeat mode)
- Fixed uninitalised warning on 'ret' (reported by kernel test robot)
- Use dev_dbg instead of dev_info for debug related print messages
- Minor cleanup and comments
drivers/rtc/rtc-pcf85363.c | 168 +++++++++++++++++++++++++++++++++++--
1 file changed, 160 insertions(+), 8 deletions(-)
diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index 3d733375187b..34d4c2e16774 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -5,6 +5,10 @@
* Driver for NXP PCF85363 real-time clock.
*
* Copyright (C) 2017 Eric Nelson
+ *
+ * Copyright 2025 NXP
+ * Added support for timestamps, battery switch-over,
+ * watchdog, offset calibration.
*/
#include <linux/module.h>
#include <linux/i2c.h>
@@ -17,6 +21,8 @@
#include <linux/device.h>
#include <linux/of.h>
#include <linux/regmap.h>
+#include <linux/rtc.h>
+#include <linux/watchdog.h>
/*
* Date/Time registers
@@ -127,6 +133,18 @@
#define OFFSET_MAXIMUM 127
#define OFFSET_MASK 0xFF
+#define WD_MODE_REPEAT BIT(7)
+#define WD_TIMEOUT_MASK GENMASK(6, 2)
+#define WD_TIMEOUT_SHIFT 2
+#define WD_CLKSEL_MASK GENMASK(1, 0)
+#define WD_CLKSEL_0_25HZ 0x00
+#define WD_CLKSEL_1HZ 0x01
+#define WD_CLKSEL_4HZ 0x02
+#define WD_CLKSEL_16HZ 0x03
+
+#define WD_TIMEOUT_MIN 1
+#define WD_TIMEOUT_MAX 0x1F
+
struct pcf85363 {
struct rtc_device *rtc;
struct regmap *regmap;
@@ -138,6 +156,15 @@ struct pcf85x63_config {
unsigned int num_nvram;
};
+struct pcf85363_watchdog {
+ struct watchdog_device wdd;
+ struct regmap *regmap;
+ struct device *dev;
+ u8 timeout_val;
+ u8 clock_sel;
+ bool repeat;
+};
+
static int pcf85363_load_capacitance(struct pcf85363 *pcf85363, struct device_node *node)
{
u32 load = 7000;
@@ -323,12 +350,13 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
return IRQ_NONE;
if (flags) {
- dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s%s%s%s\n",
+ dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s%s%s%s%s\n",
flags, (flags & FLAGS_A1F) ? " [A1F]" : "",
(flags & FLAGS_TSR1F) ? " [TSR1F]" : "",
(flags & FLAGS_TSR2F) ? " [TSR2F]" : "",
(flags & FLAGS_TSR3F) ? " [TSR3F]" : "",
- (flags & FLAGS_BSF) ? " [BSF]" : "");
+ (flags & FLAGS_BSF) ? " [BSF]" : "",
+ (flags & FLAGS_WDF) ? " [WDF]" : "");
}
if (flags & FLAGS_A1F) {
@@ -360,6 +388,11 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
handled = true;
}
+ if (flags & FLAGS_WDF) {
+ regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_WDF, 0);
+ handled = true;
+ }
+
return handled ? IRQ_HANDLED : IRQ_NONE;
}
@@ -503,6 +536,123 @@ static const struct pcf85x63_config pcf_85363_config = {
.num_nvram = 2
};
+/*
+ * This function sets the watchdog control register based on the timeout,
+ * clock selection and repeat mode settings. It prepares the value to
+ * write into the watchdog control register (CTRL_WDOG).
+ */
+static int pcf85363_wdt_reload(struct pcf85363_watchdog *wd)
+{
+ u8 val;
+
+ val = (wd->repeat ? WD_MODE_REPEAT : 0) |
+ ((wd->timeout_val & WD_TIMEOUT_MAX) << WD_TIMEOUT_SHIFT) |
+ (wd->clock_sel & WD_CLKSEL_MASK);
+
+ return regmap_write(wd->regmap, CTRL_WDOG, val);
+}
+
+static int pcf85363_wdt_start(struct watchdog_device *wdd)
+{
+ struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+
+ return pcf85363_wdt_reload(wd);
+}
+
+static int pcf85363_wdt_stop(struct watchdog_device *wdd)
+{
+ struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+
+ return regmap_write(wd->regmap, CTRL_WDOG, 0);
+}
+
+static int pcf85363_wdt_ping(struct watchdog_device *wdd)
+{
+ struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+
+ regmap_update_bits(wd->regmap, CTRL_FLAGS, FLAGS_WDF, 0);
+
+ return pcf85363_wdt_reload(wd);
+}
+
+static int pcf85363_wdt_set_timeout(struct watchdog_device *wdd,
+ unsigned int timeout)
+{
+ struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+
+ wd->timeout_val = clamp(timeout, WD_TIMEOUT_MIN, WD_TIMEOUT_MAX);
+ wdd->timeout = wd->timeout_val;
+
+ return pcf85363_wdt_reload(wd);
+}
+
+static const struct watchdog_info pcf85363_wdt_info = {
+ .identity = "PCF85363 Watchdog",
+ .options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
+};
+
+static const struct watchdog_ops pcf85363_wdt_ops = {
+ .owner = THIS_MODULE,
+ .start = pcf85363_wdt_start,
+ .stop = pcf85363_wdt_stop,
+ .ping = pcf85363_wdt_ping,
+ .set_timeout = pcf85363_wdt_set_timeout,
+};
+
+static int pcf85363_watchdog_init(struct device *dev, struct regmap *regmap)
+{
+ struct pcf85363_watchdog *wd;
+ unsigned int timeout_sec;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_WATCHDOG))
+ return 0;
+
+ wd = devm_kzalloc(dev, sizeof(*wd), GFP_KERNEL);
+ if (!wd)
+ return -ENOMEM;
+
+ wd->regmap = regmap;
+ wd->dev = dev;
+
+ wd->wdd.info = &pcf85363_wdt_info;
+ wd->wdd.ops = &pcf85363_wdt_ops;
+ wd->wdd.min_timeout = WD_TIMEOUT_MIN;
+ wd->wdd.max_timeout = WD_TIMEOUT_MAX;
+ wd->wdd.parent = dev;
+ wd->wdd.status = WATCHDOG_NOWAYOUT_INIT_STATUS;
+
+ ret = watchdog_init_timeout(&wd->wdd, 10, dev);
+ if (ret)
+ wd->wdd.timeout = clamp(10U, WD_TIMEOUT_MIN, WD_TIMEOUT_MAX);
+
+ timeout_sec = wd->wdd.timeout;
+
+ if (timeout_sec <= 2)
+ wd->clock_sel = WD_CLKSEL_16HZ;
+ else if (timeout_sec <= 8)
+ wd->clock_sel = WD_CLKSEL_4HZ;
+ else if (timeout_sec <= 16)
+ wd->clock_sel = WD_CLKSEL_1HZ;
+ else
+ wd->clock_sel = WD_CLKSEL_0_25HZ;
+
+ wd->repeat = true;
+
+ ret = regmap_update_bits(regmap, CTRL_FLAGS, FLAGS_WDF, 0);
+ if (ret) {
+ dev_err(dev, "failed to clear WDF:%d\n", ret);
+ return ret;
+ }
+
+ watchdog_set_drvdata(&wd->wdd, wd);
+
+ dev_dbg(dev, "pcf85363 watchdog registered (timeout=%us, clk_sel=%u)\n",
+ timeout_sec, wd->clock_sel);
+
+ return devm_watchdog_register_device(dev, &wd->wdd);
+}
+
/*
* Reads 6 bytes of timestamp data starting at the given base register,
* converts them from BCD to binary, and formats the result into a
@@ -684,20 +834,22 @@ static int pcf85363_probe(struct i2c_client *client)
PIN_IO_TSPM | PIN_IO_TSIM,
PIN_IO_TSPM | PIN_IO_TSIM);
+ ret = pcf85363_watchdog_init(dev, pcf85363->regmap);
+ if (ret)
+ dev_err_probe(dev, ret, "Watchdog init failed\n");
+
if (irq_a > 0 || wakeup_source)
device_init_wakeup(dev, true);
dev_set_drvdata(&pcf85363->rtc->dev, pcf85363);
- ret = devm_rtc_register_device(pcf85363->rtc);
-
+ ret = rtc_add_group(pcf85363->rtc, &pcf85363_attr_group);
if (ret)
- return dev_err_probe(dev, ret, "RTC registration failed\n");
-
- ret = sysfs_create_group(&pcf85363->rtc->dev.kobj, &pcf85363_attr_group);
+ return ret;
+ ret = devm_rtc_register_device(pcf85363->rtc);
if (ret)
- return dev_err_probe(dev, ret, "Timestamp sysfs creation failed\n");
+ return dev_err_probe(dev, ret, "RTC registration failed\n");
for (i = 0; i < config->num_nvram; i++) {
nvmem_cfg[i].priv = pcf85363;
--
2.25.1
^ permalink raw reply related
* [PATCH v3 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config
From: Lakshay Piplani @ 2025-11-19 8:33 UTC (permalink / raw)
To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
conor+dt, devicetree, wim, linux, linux-watchdog
Cc: vikash.bansal, priyanka.jain, shashank.rebbapragada,
Lakshay Piplani
NXP PCF85263/PCF85363 provides three timestamp registers (TSR1-TSR3)
which latch the current time when a selected event occurs. Add a
vendor specific property, nxp,timestamp-mode, to select the event
source for each register.
Also introduce a new header 'pcf85363-tsr.h' to expose
macros for timestamp mode fields, improving readability
of device tree file.
Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
V2 -> V3:
- No changes in v3
- Added Reviewed-by: Rob Herring <robh@kernel.org>
V1 -> V2:
- Addressed review comments from Rob Herring:
* use $ref: /schemas/types.yaml#/definitions/uint32-array
* tuple form with exactly 3 items (TSR1/TSR2/TSR3), per items decimal enums
* define 'nxp,timestamp-mode' clearly
* drop watchdog related vendor properties
* remove watchdog related vendor properties from i2c example
.../devicetree/bindings/rtc/nxp,pcf85363.yaml | 23 ++++++++++++++-
include/dt-bindings/rtc/pcf85363-tsr.h | 28 +++++++++++++++++++
2 files changed, 50 insertions(+), 1 deletion(-)
create mode 100644 include/dt-bindings/rtc/pcf85363-tsr.h
diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85363.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf85363.yaml
index 52aa3e2091e9..cf9c155162d6 100644
--- a/Documentation/devicetree/bindings/rtc/nxp,pcf85363.yaml
+++ b/Documentation/devicetree/bindings/rtc/nxp,pcf85363.yaml
@@ -4,7 +4,7 @@
$id: http://devicetree.org/schemas/rtc/nxp,pcf85363.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: Philips PCF85263/PCF85363 Real Time Clock
+title: NXP PCF85263/PCF85363 Real Time Clock
maintainers:
- Alexandre Belloni <alexandre.belloni@bootlin.com>
@@ -39,6 +39,24 @@ properties:
start-year: true
wakeup-source: true
+ nxp,timestamp-mode:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ items:
+ - enum: [0, 1, 2] # TSR1: NONE, FE, LE
+ description: TSR1 mode
+ - enum: [0, 1, 2, 3, 4, 5] # TSR2: NONE, FB, LB, LV, FE, LE
+ description: TSR2 mode
+ - enum: [0, 1, 2, 3] # TSR3: NONE, FB, LB, LV
+ description: TSR3 mode
+ description: |
+ Defines timestamp modes for TSR1, TSR2, and TSR3.
+ Use macros from <dt-bindings/rtc/pcf85363-tsr.h>.
+
+ Each value corresponds to a mode constant:
+ - TSR1: NONE, FE, LE
+ - TSR2: NONE, FB, LB, LV, FE, LE
+ - TSR3: NONE, FB, LB, LV
+
required:
- compatible
- reg
@@ -47,6 +65,7 @@ additionalProperties: false
examples:
- |
+ #include <dt-bindings/rtc/pcf85363-tsr.h>
i2c {
#address-cells = <1>;
#size-cells = <0>;
@@ -56,5 +75,7 @@ examples:
reg = <0x51>;
#clock-cells = <0>;
quartz-load-femtofarads = <12500>;
+ wakeup-source;
+ nxp,timestamp-mode = <PCF85363_TSR1_FE PCF85363_TSR2_LB PCF85363_TSR3_LV>;
};
};
diff --git a/include/dt-bindings/rtc/pcf85363-tsr.h b/include/dt-bindings/rtc/pcf85363-tsr.h
new file mode 100644
index 000000000000..1fb5b9b3601e
--- /dev/null
+++ b/include/dt-bindings/rtc/pcf85363-tsr.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * Copyright 2025 NXP
+ */
+
+#ifndef _DT_BINDINGS_RTC_PCF85363_TSR_H
+#define _DT_BINDINGS_RTC_PCF85363_TSR_H
+
+/* TSR1 modes */
+#define PCF85363_TSR1_NONE 0x00
+#define PCF85363_TSR1_FE 0x01
+#define PCF85363_TSR1_LE 0x02
+
+/* TSR2 modes */
+#define PCF85363_TSR2_NONE 0x00
+#define PCF85363_TSR2_FB 0x01
+#define PCF85363_TSR2_LB 0x02
+#define PCF85363_TSR2_LV 0x03
+#define PCF85363_TSR2_FE 0x04
+#define PCF85363_TSR2_LE 0x05
+
+/* TSR3 modes */
+#define PCF85363_TSR3_NONE 0x00
+#define PCF85363_TSR3_FB 0x01
+#define PCF85363_TSR3_LB 0x02
+#define PCF85363_TSR3_LV 0x03
+
+#endif /* _DT_BINDINGS_RTC_PCF85363_TSR_H */
--
2.25.1
^ permalink raw reply related
* Re: [PATCH next] rtc: atcrtc100: Fix signedness bug in probe()
From: Dan Carpenter @ 2025-11-19 7:13 UTC (permalink / raw)
To: CL Wang; +Cc: Alexandre Belloni, linux-rtc, linux-kernel, kernel-janitors
In-Reply-To: <aR0s653C4g2auavf@swlinux02>
On Wed, Nov 19, 2025 at 10:35:23AM +0800, CL Wang wrote:
> On Tue, Nov 18, 2025 at 01:48:56PM +0300, Dan Carpenter wrote:
>
> Hi Dan,
>
> Thank you for pointing out the issue and for providing the fix.
>
> You're absolutely correct that using an unsigned type for alarm_irq
> prevents proper error handling when platform_get_irq() returns a
> negative value. I will apply your patch and also review other return
> value checks to ensure there are no similar issues elsewhere.
>
> Thank you again for your detailed review and suggestions.
>
You're welcome. These are just Smatch warnings. Nothing super
major.
https://github.com/error27/smatch
https://github.com/error27/smatch/blob/master/Documentation/smatch.rst
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH next] rtc: atcrtc100: Fix signedness bug in probe()
From: CL Wang @ 2025-11-19 2:35 UTC (permalink / raw)
To: Dan Carpenter
Cc: Alexandre Belloni, linux-rtc, linux-kernel, kernel-janitors,
cl634
In-Reply-To: <aRxPGBEX8hbY6sjV@stanley.mountain>
On Tue, Nov 18, 2025 at 01:48:56PM +0300, Dan Carpenter wrote:
Hi Dan,
Thank you for pointing out the issue and for providing the fix.
You're absolutely correct that using an unsigned type for alarm_irq
prevents proper error handling when platform_get_irq() returns a
negative value. I will apply your patch and also review other return
value checks to ensure there are no similar issues elsewhere.
Thank you again for your detailed review and suggestions.
Best regards,
CL
>
> The "atcrtc_dev->alarm_irq" variable is an unsigned int but it needs to
> be signed for the error handling to work. Use the "ret" variable
> instead.
>
> Fixes: 7adca706fe16 ("rtc: atcrtc100: Add ATCRTC100 RTC driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> drivers/rtc/rtc-atcrtc100.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/rtc/rtc-atcrtc100.c b/drivers/rtc/rtc-atcrtc100.c
> index 51933ae1a2fa..9808fc2c5a49 100644
> --- a/drivers/rtc/rtc-atcrtc100.c
> +++ b/drivers/rtc/rtc-atcrtc100.c
> @@ -296,10 +296,12 @@ static int atcrtc_probe(struct platform_device *pdev)
> "Failed to initialize RTC: unsupported hardware ID 0x%x\n",
> rtc_id);
>
> - atcrtc_dev->alarm_irq = platform_get_irq(pdev, 1);
> - if (atcrtc_dev->alarm_irq < 0)
> - return dev_err_probe(&pdev->dev, atcrtc_dev->alarm_irq,
> + ret = platform_get_irq(pdev, 1);
> + if (ret < 0)
> + return dev_err_probe(&pdev->dev, ret,
> "Failed to get IRQ for alarm\n");
> + atcrtc_dev->alarm_irq = ret;
> +
> ret = devm_request_irq(&pdev->dev,
> atcrtc_dev->alarm_irq,
> atcrtc_alarm_isr,
> --
> 2.51.0
>
^ permalink raw reply
* Re: [PATCH v4 05/16] dt-bindings: mfd: ROHM BD72720
From: Linus Walleij @ 2025-11-18 23:06 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Lee Jones, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sebastian Reichel,
Liam Girdwood, Mark Brown, Michael Turquette, Stephen Boyd,
Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
Andreas Kemnade
In-Reply-To: <ec2cb44d9d00f5edaed2fbe17fd9ddbed914ff37.1763022807.git.mazziesaccount@gmail.com>
On Thu, Nov 13, 2025 at 9:52 AM Matti Vaittinen
<matti.vaittinen@linux.dev> wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> The ROHM BD72720 is a power management IC integrating regulators, GPIOs,
> charger, LEDs, RTC and a clock gate.
>
> Add dt-binding doc for ROHM BD72720.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
This is looking good!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v6 2/2] rtc: Add NXP PCF85053 driver support
From: kernel test robot @ 2025-11-18 20:58 UTC (permalink / raw)
To: Lakshay Piplani, alexandre.belloni, linux-rtc, linux-kernel, robh,
krzk+dt, conor+dt, devicetree
Cc: oe-kbuild-all, pankit.garg, vikash.bansal, priyanka.jain,
shashank.rebbapragada, Lakshay Piplani, Daniel Aguirre
In-Reply-To: <20251113054243.4045820-2-lakshay.piplani@nxp.com>
Hi Lakshay,
kernel test robot noticed the following build errors:
[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on linus/master v6.18-rc6 next-20251118]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Lakshay-Piplani/rtc-Add-NXP-PCF85053-driver-support/20251113-134432
base: https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
patch link: https://lore.kernel.org/r/20251113054243.4045820-2-lakshay.piplani%40nxp.com
patch subject: [PATCH v6 2/2] rtc: Add NXP PCF85053 driver support
config: sparc64-randconfig-001-20251119 (https://download.01.org/0day-ci/archive/20251119/202511190432.xfs91BVq-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251119/202511190432.xfs91BVq-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511190432.xfs91BVq-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
drivers/rtc/rtc-pcf85053.c: In function 'pcf85053_probe':
>> drivers/rtc/rtc-pcf85053.c:616:14: error: implicit declaration of function 'i2c_check_functionality' [-Wimplicit-function-declaration]
616 | if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/rtc/rtc-pcf85053.c: At top level:
>> drivers/rtc/rtc-pcf85053.c:729:1: warning: data definition has no type or storage class
729 | module_i2c_driver(pcf85053_driver);
| ^~~~~~~~~~~~~~~~~
>> drivers/rtc/rtc-pcf85053.c:729:1: error: type defaults to 'int' in declaration of 'module_i2c_driver' [-Wimplicit-int]
>> drivers/rtc/rtc-pcf85053.c:729:1: error: parameter names (without types) in function declaration [-Wdeclaration-missing-parameter-type]
>> drivers/rtc/rtc-pcf85053.c:720:26: warning: 'pcf85053_driver' defined but not used [-Wunused-variable]
720 | static struct i2c_driver pcf85053_driver = {
| ^~~~~~~~~~~~~~~
vim +/i2c_check_functionality +616 drivers/rtc/rtc-pcf85053.c
608
609 static int pcf85053_probe(struct i2c_client *client)
610 {
611 struct pcf85053 *pcf85053;
612 const struct pcf85053_config *config;
613 const char *iface = NULL;
614 int err;
615
> 616 if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
617 I2C_FUNC_SMBUS_BYTE |
618 I2C_FUNC_SMBUS_BLOCK_DATA))
619 return -ENODEV;
620
621 pcf85053 = devm_kzalloc(&client->dev, sizeof(struct pcf85053),
622 GFP_KERNEL);
623 if (!pcf85053)
624 return -ENOMEM;
625
626 config = i2c_get_match_data(client);
627 if (!config)
628 return -ENODEV;
629
630 pcf85053->regmap = devm_regmap_init_i2c(client, &config->regmap);
631 if (IS_ERR(pcf85053->regmap))
632 return PTR_ERR(pcf85053->regmap);
633
634 i2c_set_clientdata(client, pcf85053);
635
636 pcf85053->client = client;
637 device_set_wakeup_capable(&client->dev, 1);
638
639 pcf85053->is_primary = true;
640
641 if (of_property_read_string(client->dev.of_node, "nxp,interface", &iface))
642 return dev_err_probe(&client->dev, -EINVAL,
643 "Missing mandatory property: nxp,interface\n");
644 if (!strcmp(iface, "primary"))
645 pcf85053->is_primary = true;
646 else if (!strcmp(iface, "secondary"))
647 pcf85053->is_primary = false;
648 else
649 return dev_err_probe(&client->dev, -EINVAL,
650 "Invalid value for nxp,interface: %s\n", iface);
651
652 if (pcf85053->is_primary) {
653 unsigned int ctrl;
654 int err;
655
656 err = regmap_read(pcf85053->regmap, PCF85053_REG_CTRL, &ctrl);
657 if (err)
658 return err;
659
660 if (of_property_read_bool(client->dev.of_node, "nxp,write-access")) {
661 if (!(ctrl & PCF85053_BIT_TWO)) {
662 err = regmap_update_bits(pcf85053->regmap, PCF85053_REG_CTRL,
663 PCF85053_BIT_TWO, PCF85053_BIT_TWO);
664 if (err)
665 return err;
666 }
667 dev_dbg(&client->dev, "Ownership set: TWO=1 (primary writes)\n");
668 } else {
669 /* TWO (Time Write Ownership) bit defaults to 0 (Secondary) */
670 dev_dbg(&client->dev, "Default ownership set: TWO=0 (secondary writes)\n");
671 }
672 }
673
674 pcf85053->rtc = devm_rtc_allocate_device(&client->dev);
675 if (IS_ERR(pcf85053->rtc))
676 return PTR_ERR(pcf85053->rtc);
677
678 pcf85053->rtc->ops = &pcf85053_rtc_ops;
679 pcf85053->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
680 pcf85053->rtc->range_max = RTC_TIMESTAMP_END_2099;
681 clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, pcf85053->rtc->features);
682 clear_bit(RTC_FEATURE_ALARM, pcf85053->rtc->features);
683
684 if (config->has_alarms && client->irq > 0) {
685 err = devm_request_threaded_irq(&client->dev, client->irq,
686 NULL, pcf85053_irq,
687 IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
688 "pcf85053", client);
689 if (err) {
690 dev_err(&client->dev, "unable to request IRQ %d\n", client->irq);
691 } else {
692 set_bit(RTC_FEATURE_ALARM, pcf85053->rtc->features);
693 device_init_wakeup(&client->dev, true);
694 err = dev_pm_set_wake_irq(&client->dev, client->irq);
695 if (err)
696 dev_err(&client->dev, "failed to enable irq wake\n");
697 }
698 }
699
700 #ifdef CONFIG_COMMON_CLK
701 /* register clk in common clk framework */
702 pcf85053_clkout_register_clk(pcf85053);
703 #endif
704
705 return devm_rtc_register_device(pcf85053->rtc);
706 }
707
708 static const struct i2c_device_id pcf85053_id[] = {
709 { "pcf85053", .driver_data = (kernel_ulong_t)&config_pcf85053 },
710 { }
711 };
712 MODULE_DEVICE_TABLE(i2c, pcf85053_id);
713
714 static const struct of_device_id pcf85053_of_match[] = {
715 { .compatible = "nxp,pcf85053", .data = &config_pcf85053 },
716 {}
717 };
718 MODULE_DEVICE_TABLE(of, pcf85053_of_match);
719
> 720 static struct i2c_driver pcf85053_driver = {
721 .driver = {
722 .name = "rtc-pcf85053",
723 .of_match_table = of_match_ptr(pcf85053_of_match),
724 },
725 .probe = pcf85053_probe,
726 .id_table = pcf85053_id,
727 };
728
> 729 module_i2c_driver(pcf85053_driver);
730
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v3 4/4] mfd: simple-mfd-i2c: add default value
From: Alex Elder @ 2025-11-18 17:32 UTC (permalink / raw)
To: Troy Mitchell, Lee Jones, Yixun Lan, Andi Shyti,
Alexandre Belloni, Liam Girdwood, Mark Brown
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc
In-Reply-To: <20251118-p1-kconfig-fix-v3-4-8839c5ac5db3@linux.spacemit.com>
On 11/18/25 12:08 AM, 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.
>
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
> drivers/mfd/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 6cec1858947bf7ab5ee78beb730c95dabcb43a98..b0f109b3acc40b074e4d0178e123437495853496 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1260,6 +1260,7 @@ config MFD_SPACEMIT_P1
> depends on I2C
> select I2C_K1
> select MFD_SIMPLE_MFD_I2C
> + default ARCH_SPACEMIT
> help
> This option supports the I2C-based SpacemiT P1 PMIC, which
> contains regulators, a power switch, GPIOs, an RTC, and more.
>
I agree with Emil on making this be default m if possible.
Acked-by: Alex Elder <elder@riscstar.com>
Thank you for these fixes.
^ permalink raw reply
* Re: [PATCH v3 3/4] regulator: spacemit: MFD_SPACEMIT_P1 as dependencies
From: Alex Elder @ 2025-11-18 17:32 UTC (permalink / raw)
To: Troy Mitchell, Lee Jones, Yixun Lan, Andi Shyti,
Alexandre Belloni, Liam Girdwood, Mark Brown
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc
In-Reply-To: <20251118-p1-kconfig-fix-v3-3-8839c5ac5db3@linux.spacemit.com>
On 11/18/25 12:08 AM, 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>
> Signed-off-by: Troy Mitchell <troy.mitchell@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 d84f3d054c59d86d91d859808aa73a3b609d16d0..e2cbbb90500189a1c4282511b8d7141301cae1f0 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -1455,9 +1455,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 MFD_SPACEMIT_P1
If possible:
default m if MFD_SPACEMIT_P1
Acked-by: Alex Elder <elder@riscstar.com>
> help
> Enable support for regulators implemented by the SpacemiT P1
> power controller. The P1 implements 6 high-efficiency buck
>
^ permalink raw reply
* Re: [PATCH v3 2/4] rtc: spacemit: MFD_SPACEMIT_P1 as dependencies
From: Alex Elder @ 2025-11-18 17:32 UTC (permalink / raw)
To: Troy Mitchell, Lee Jones, Yixun Lan, Andi Shyti,
Alexandre Belloni, Liam Girdwood, Mark Brown
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc
In-Reply-To: <20251118-p1-kconfig-fix-v3-2-8839c5ac5db3@linux.spacemit.com>
On 11/18/25 12:08 AM, Troy Mitchell wrote:
> RTC_DRV_SPACEMIT_P1 is a subdevice of P1 and should depend on
> MFD_SPACEMIT_P1 rather than selecting it directly. Using 'select'
> does not always respect the parent's dependencies, so 'depends on'
> is the safer and more correct choice.
In particular, it looks like it doesn't depend on I2C...
> Additionally, the default value depends on MFD_SPACEMIT_P1 rather
> than ARCH_SPACEMIT.
>
> Signed-off-by: Troy Mitchell <troy.mitchell@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-3-49688f30bae8@linux.spacemit.com/
> ---
> drivers/rtc/Kconfig | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 2933c41c77c88e60df721fe65b9c8afb995ae51e..b392e6d096ed077e841a2e68b70d8b80d9ad1cde 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -409,8 +409,8 @@ config RTC_DRV_MAX77686
> config RTC_DRV_SPACEMIT_P1
> tristate "SpacemiT P1 RTC"
> depends on ARCH_SPACEMIT || COMPILE_TEST
> - select MFD_SPACEMIT_P1
> - default ARCH_SPACEMIT
> + depends on MFD_SPACEMIT_P1
> + default MFD_SPACEMIT_P1
If possible, this should maybe be:
default m if MFD_SPACEMIT_P1
In any case, this looks like an improvement.
Acked-by: Alex Elder <elder@riscstar.com>
> 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
>
^ permalink raw reply
* Re: [PATCH v3 1/4] i2c: spacemit: configure ILCR for accurate SCL frequency
From: Alex Elder @ 2025-11-18 17:32 UTC (permalink / raw)
To: Troy Mitchell, Lee Jones, Yixun Lan, Andi Shyti,
Alexandre Belloni, Liam Girdwood, Mark Brown
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc
In-Reply-To: <20251118-p1-kconfig-fix-v3-1-8839c5ac5db3@linux.spacemit.com>
On 11/18/25 12:08 AM, Troy Mitchell wrote:
> The SpacemiT I2C controller's SCL (Serial Clock Line) frequency for
> master mode operations is determined by the ILCR (I2C Load Count Register).
> Previously, the driver relied on the hardware's reset default
> values for this register.
>
> The hardware's default ILCR values (SLV=0x156, FLV=0x5d) yield SCL
> frequencies lower than intended. For example, with the default
> 31.5 MHz input clock, these default settings result in an SCL
> frequency of approximately 93 kHz (standard mode) when targeting 100 kHz,
> and approximately 338 kHz (fast mode) when targeting 400 kHz.
> These frequencies are below the 100 kHz/400 kHz nominal speeds.
>
> This patch integrates the SCL frequency management into
> the Common Clock Framework (CCF). Specifically, the ILCR register,
> which acts as a frequency divider for the SCL clock, is now registered
> as a managed clock (scl_clk) within the CCF.
>
> This patch also cleans up unnecessary whitespace
> in the included header files.
I have a few comments below. Sorry I didn't comment on
earlier versions. > Reviewed-by: Yixun Lan <dlan@gentoo.org>
> Link: https://lore.kernel.org/all/176244506110.1925720.10807118665958896958.b4-ty@kernel.org/ [1]
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
> This patch was affected by the P1 Kconfig, which caused the maintainer
> to revert it.
> The current commit is a direct cherry-pick and reserves the original changelog.
> This note is to clarify for anyone who sees the cover letter marked as v2
> while the changelog entries reach v4.
> ---
> Changelog in v4:
> - initialize clk_init_data with {} so that init.flags is implicitly set to 0
> - minor cleanup and style fixes for better readability
> - remove unused spacemit_i2c_scl_clk_exclusive_put() cleanup callback
> - replace clk_set_rate_exclusive()/clk_rate_exclusive_put() pair with clk_set_rate()
> - simplify LCR LV field macros by using FIELD_GET/FIELD_MAX helpers
> - Link to v3: https://lore.kernel.org/all/20250814-k1-i2c-ilcr-v3-1-317723e74bcd@linux.spacemit.com/
>
> Changelog in v3:
> - use MASK macro in `recalc_rate` function
> - rename clock name
> - Link to v2: https://lore.kernel.org/r/20250718-k1-i2c-ilcr-v2-1-b4c68f13dcb1@linux.spacemit.com
>
> Changelog in v2:
> - Align line breaks.
> - Check `lv` in `clk_set_rate` function.
> - Force fast mode when SCL frequency is illegal or unavailable.
> - Change "linux/bits.h" to <linux/bits.h>
> - Kconfig: Add dependency on CCF.
> - Link to v1: https://lore.kernel.org/all/20250710-k1-i2c-ilcr-v1-1-188d1f460c7d@linux.spacemit.com/
> ---
> drivers/i2c/busses/Kconfig | 2 +-
> drivers/i2c/busses/i2c-k1.c | 159 ++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 146 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index fd81e49638aaa161ae264a722e9e06adc7914cda..fedf5d31f9035b73a27a7f8a764bf5c26975d0e1 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -798,7 +798,7 @@ config I2C_JZ4780
> config I2C_K1
> tristate "SpacemiT K1 I2C adapter"
> depends on ARCH_SPACEMIT || COMPILE_TEST
> - depends on OF
> + depends on OF && COMMON_CLK
> help
> This option enables support for the I2C interface on the SpacemiT K1
> platform.
> diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
> index 6b918770e612e098b8ad17418f420d87c94df166..e38a0ba71734ca602854c85672dcb61423453515 100644
> --- a/drivers/i2c/busses/i2c-k1.c
> +++ b/drivers/i2c/busses/i2c-k1.c
> @@ -4,18 +4,21 @@
> */
>
> #include <linux/bitfield.h>
> - #include <linux/clk.h>
> - #include <linux/i2c.h>
> - #include <linux/iopoll.h>
> - #include <linux/module.h>
> - #include <linux/of_address.h>
> - #include <linux/platform_device.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/i2c.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
>
> /* spacemit i2c registers */
> #define SPACEMIT_ICR 0x0 /* Control register */
> #define SPACEMIT_ISR 0x4 /* Status register */
> #define SPACEMIT_IDBR 0xc /* Data buffer register */
> #define SPACEMIT_IRCR 0x18 /* Reset cycle counter */
> +#define SPACEMIT_ILCR 0x10 /* Load Count Register */
> #define SPACEMIT_IBMR 0x1c /* Bus monitor register */
>
> /* SPACEMIT_ICR register fields */
> @@ -87,6 +90,13 @@
> #define SPACEMIT_BMR_SDA BIT(0) /* SDA line level */
> #define SPACEMIT_BMR_SCL BIT(1) /* SCL line level */
>
> +#define SPACEMIT_LCR_LV_STANDARD_SHIFT 0
> +#define SPACEMIT_LCR_LV_FAST_SHIFT 9
Why do you need these SHIFT symbols? Just use the masks
and FIELD_GET() related macros. I'll provide examples below.
> +#define SPACEMIT_LCR_LV_STANDARD_MASK GENMASK(8, 0)
> +#define SPACEMIT_LCR_LV_FAST_MASK GENMASK(17, 9)
> +#define SPACEMIT_LCR_LV_STANDARD_MAX_VALUE FIELD_MAX(SPACEMIT_LCR_LV_STANDARD_MASK)
> +#define SPACEMIT_LCR_LV_FAST_MAX_VALUE FIELD_MAX(SPACEMIT_LCR_LV_FAST_MASK)
> +
> /* i2c bus recover timeout: us */
> #define SPACEMIT_I2C_BUS_BUSY_TIMEOUT 100000
>
> @@ -104,11 +114,20 @@ enum spacemit_i2c_state {
> SPACEMIT_STATE_WRITE,
> };
>
> +enum spacemit_i2c_mode {
> + SPACEMIT_MODE_STANDARD,
> + SPACEMIT_MODE_FAST
> +};
Will there ever be more than two i2c modes? If not, this
could simply be a Boolean.
> +
> /* i2c-spacemit driver's main struct */
> struct spacemit_i2c_dev {
> struct device *dev;
> struct i2c_adapter adapt;
>
> + struct clk_hw scl_clk_hw;
> + struct clk *scl_clk;
> + enum spacemit_i2c_mode mode;
Perhaps this could be:
bool fast_mode;
> +
> /* hardware resources */
> void __iomem *base;
> int irq;
> @@ -129,6 +148,79 @@ struct spacemit_i2c_dev {
> u32 status;
> };
>
> +static void spacemit_i2c_scl_clk_disable_unprepare(void *data)
> +{
> + struct spacemit_i2c_dev *i2c = data;
> +
> + clk_disable_unprepare(i2c->scl_clk);
> +}
> +
> +static int spacemit_i2c_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct spacemit_i2c_dev *i2c = container_of(hw, struct spacemit_i2c_dev, scl_clk_hw);
> + u32 lv, lcr, mask, shift, max_lv;
> +
> + lv = DIV_ROUND_UP(parent_rate, rate);
Would DIV_ROUND_CLOSEST() give a more accurate value?
> +
> + if (i2c->mode == SPACEMIT_MODE_STANDARD) {
> + mask = SPACEMIT_LCR_LV_STANDARD_MASK;
> + shift = SPACEMIT_LCR_LV_STANDARD_SHIFT;
> + max_lv = SPACEMIT_LCR_LV_STANDARD_MAX_VALUE;
> + } else if (i2c->mode == SPACEMIT_MODE_FAST) {
> + mask = SPACEMIT_LCR_LV_FAST_MASK;
> + shift = SPACEMIT_LCR_LV_FAST_SHIFT;
> + max_lv = SPACEMIT_LCR_LV_FAST_MAX_VALUE;
> + }
> +
> + if (!lv || lv > max_lv) {
> + dev_err(i2c->dev, "set scl clock failed: lv 0x%x", lv);
> + return -EINVAL;
> + }
> +
> + lcr = readl(i2c->base + SPACEMIT_ILCR);
> + lcr &= ~mask;
> + lcr |= lv << shift;
> + writel(lcr, i2c->base + SPACEMIT_ILCR);
FIELD_MODIFY(mask, &lcr, lv);
I suppose this might give you trouble because the mask isn't
constant at compile time, but anyway I think something like
this is simpler:
lv = DIV_ROUND_CLOSEST(parent_rate, rate);
lcr = readl(i2c->base + SPACEMIT_ILCR);
if (i2c->fast_mode)
FIELD_MODIFY(SPACEMIT_LCR_LV_FAST_MASK, &lcr, lv);
else
FIELD_MODIFY(SPACEMIT_LCR_LV_STANDARD_MASK, &lcr, lv);
writel(lcr, i2c->base + SPACEMIT_ILCR);
Note: I've never used FIELD_MODIFY(), but it looks like this is
how it's supposed to be used.
> + return 0;
> +}
> +
> +static long spacemit_i2c_clk_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + u32 lv, freq;
> +
> + lv = DIV_ROUND_UP(*parent_rate, rate);
> + freq = DIV_ROUND_UP(*parent_rate, lv);
Consider whether DIV_ROUND_CLOSEST() (in one or both of
these) provides a rate that is as close as possible to the
requested rate.
> +
> + return freq;
> +}
> +
> +static unsigned long spacemit_i2c_clk_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct spacemit_i2c_dev *i2c = container_of(hw, struct spacemit_i2c_dev, scl_clk_hw);
> + u32 lcr, lv = 0;
> +
> + lcr = readl(i2c->base + SPACEMIT_ILCR);
> +
> + if (i2c->mode == SPACEMIT_MODE_STANDARD)
> + lv = FIELD_GET(SPACEMIT_LCR_LV_STANDARD_MASK, lcr);
> + else if (i2c->mode == SPACEMIT_MODE_FAST)
> + lv = FIELD_GET(SPACEMIT_LCR_LV_FAST_MASK, lcr);
> + else
> + return 0;
You shouldn't need the last else here. You can probably tell
by inspection that it will always be one mode or the other.
And a Boolean reinforces that.
> +
> + return DIV_ROUND_UP(parent_rate, lv);
> +}
> +
> +static const struct clk_ops spacemit_i2c_clk_ops = {
> + .set_rate = spacemit_i2c_clk_set_rate,
> + .round_rate = spacemit_i2c_clk_round_rate,
> + .recalc_rate = spacemit_i2c_clk_recalc_rate,
> +};
> +
> static void spacemit_i2c_enable(struct spacemit_i2c_dev *i2c)
> {
> u32 val;
> @@ -147,6 +239,26 @@ static void spacemit_i2c_disable(struct spacemit_i2c_dev *i2c)
> writel(val, i2c->base + SPACEMIT_ICR);
> }
>
> +static struct clk *spacemit_i2c_register_scl_clk(struct spacemit_i2c_dev *i2c,
> + struct clk *parent)
> +{
> + struct clk_init_data init = {};
> + char name[32];
> +
> + snprintf(name, sizeof(name), "%s_scl_clk", dev_name(i2c->dev));
What if dev_name(i2c->dev) is longer than 24? You should
be checking the return value here.
> +
> + init.name = name;
> + init.ops = &spacemit_i2c_clk_ops;
> + init.parent_data = (struct clk_parent_data[]) {
> + { .fw_name = "func" },
> + };
> + init.num_parents = 1;
> +
> + i2c->scl_clk_hw.init = &init;
> +
> + return devm_clk_register(i2c->dev, &i2c->scl_clk_hw);
> +}
> +
> static void spacemit_i2c_reset(struct spacemit_i2c_dev *i2c)
> {
> writel(SPACEMIT_CR_UR, i2c->base + SPACEMIT_ICR);
> @@ -246,7 +358,7 @@ static void spacemit_i2c_init(struct spacemit_i2c_dev *i2c)
> */
> val |= SPACEMIT_CR_DRFIE;
>
> - if (i2c->clock_freq == SPACEMIT_I2C_MAX_FAST_MODE_FREQ)
> + if (i2c->mode == SPACEMIT_MODE_FAST)
> val |= SPACEMIT_CR_MODE_FAST;
>
> /* disable response to general call */
> @@ -538,14 +650,15 @@ static int spacemit_i2c_probe(struct platform_device *pdev)
> dev_warn(dev, "failed to read clock-frequency property: %d\n", ret);
I don't think there's any need to warn when the "clock-frequency"
property is not found. It's an optional property, and the default
is specified in the binding to be 400 KHz.
> /* For now, this driver doesn't support high-speed. */
> - if (!i2c->clock_freq || i2c->clock_freq > SPACEMIT_I2C_MAX_FAST_MODE_FREQ) {
> - dev_warn(dev, "unsupported clock frequency %u; using %u\n",
> - i2c->clock_freq, SPACEMIT_I2C_MAX_FAST_MODE_FREQ);
> + if (i2c->clock_freq > SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ &&
> + i2c->clock_freq <= SPACEMIT_I2C_MAX_FAST_MODE_FREQ) {
> + i2c->mode = SPACEMIT_MODE_FAST;
> + } else if (i2c->clock_freq && i2c->clock_freq <= SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ) {
> + i2c->mode = SPACEMIT_MODE_STANDARD;
So this I2C driver will literally support *either* the standard
speed *or* the high speed frequency. Is this a necessary
restriction? If it is, perhaps the DT binding should be
clear that the speeds should be one of those supported
(because anything else will result in using a supported
speed, not the one provided).
(Sorry, these last two comments are not about your patch,
but about the driver that's already accepted.)
-Alex
> + } else {
> + dev_warn(i2c->dev, "invalid clock-frequency, fallback to fast mode");
> + i2c->mode = SPACEMIT_MODE_FAST;
> i2c->clock_freq = SPACEMIT_I2C_MAX_FAST_MODE_FREQ;
> - } else if (i2c->clock_freq < SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ) {
> - dev_warn(dev, "unsupported clock frequency %u; using %u\n",
> - i2c->clock_freq, SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ);
> - i2c->clock_freq = SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ;
> }
>
> i2c->dev = &pdev->dev;
> @@ -567,10 +680,28 @@ static int spacemit_i2c_probe(struct platform_device *pdev)
> if (IS_ERR(clk))
> return dev_err_probe(dev, PTR_ERR(clk), "failed to enable func clock");
>
> + i2c->scl_clk = spacemit_i2c_register_scl_clk(i2c, clk);
> + if (IS_ERR(i2c->scl_clk))
> + return dev_err_probe(&pdev->dev, PTR_ERR(i2c->scl_clk),
> + "failed to register scl clock\n");
> +
> clk = devm_clk_get_enabled(dev, "bus");
> if (IS_ERR(clk))
> return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock");
>
> + ret = clk_set_rate(i2c->scl_clk, i2c->clock_freq);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "failed to set rate for SCL clock");
> +
> + ret = clk_prepare_enable(i2c->scl_clk);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "failed to prepare and enable clock");
> +
> + ret = devm_add_action_or_reset(dev, spacemit_i2c_scl_clk_disable_unprepare, i2c);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to register cleanup action for clk disable and unprepare");
> +
> spacemit_i2c_reset(i2c);
>
> i2c_set_adapdata(&i2c->adapt, i2c);
>
^ permalink raw reply
* Re: [PATCH v3 4/4] mfd: simple-mfd-i2c: add default value
From: Emil Renner Berthing @ 2025-11-18 16:16 UTC (permalink / raw)
To: Alex Elder, Alexandre Belloni, Andi Shyti, Lee Jones,
Liam Girdwood, Mark Brown, Troy Mitchell, Yixun Lan
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc
In-Reply-To: <20251118-p1-kconfig-fix-v3-4-8839c5ac5db3@linux.spacemit.com>
Quoting Troy Mitchell (2025-11-18 07:08:08)
> The default value of the P1 sub-device depends on the value
> of P1, so P1 should have a default value here.
>
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
> drivers/mfd/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 6cec1858947bf7ab5ee78beb730c95dabcb43a98..b0f109b3acc40b074e4d0178e123437495853496 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1260,6 +1260,7 @@ config MFD_SPACEMIT_P1
> depends on I2C
> select I2C_K1
> select MFD_SIMPLE_MFD_I2C
> + default ARCH_SPACEMIT
Can this not be default m if ARCH_SPACEMIT?
/Emil
^ permalink raw reply
* [PATCH next] rtc: atcrtc100: Fix signedness bug in probe()
From: Dan Carpenter @ 2025-11-18 10:48 UTC (permalink / raw)
To: CL Wang; +Cc: Alexandre Belloni, linux-rtc, linux-kernel, kernel-janitors
The "atcrtc_dev->alarm_irq" variable is an unsigned int but it needs to
be signed for the error handling to work. Use the "ret" variable
instead.
Fixes: 7adca706fe16 ("rtc: atcrtc100: Add ATCRTC100 RTC driver")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
drivers/rtc/rtc-atcrtc100.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/rtc/rtc-atcrtc100.c b/drivers/rtc/rtc-atcrtc100.c
index 51933ae1a2fa..9808fc2c5a49 100644
--- a/drivers/rtc/rtc-atcrtc100.c
+++ b/drivers/rtc/rtc-atcrtc100.c
@@ -296,10 +296,12 @@ static int atcrtc_probe(struct platform_device *pdev)
"Failed to initialize RTC: unsupported hardware ID 0x%x\n",
rtc_id);
- atcrtc_dev->alarm_irq = platform_get_irq(pdev, 1);
- if (atcrtc_dev->alarm_irq < 0)
- return dev_err_probe(&pdev->dev, atcrtc_dev->alarm_irq,
+ ret = platform_get_irq(pdev, 1);
+ if (ret < 0)
+ return dev_err_probe(&pdev->dev, ret,
"Failed to get IRQ for alarm\n");
+ atcrtc_dev->alarm_irq = ret;
+
ret = devm_request_irq(&pdev->dev,
atcrtc_dev->alarm_irq,
atcrtc_alarm_isr,
--
2.51.0
^ permalink raw reply related
* [PATCH v3 4/4] mfd: simple-mfd-i2c: add default value
From: Troy Mitchell @ 2025-11-18 6:08 UTC (permalink / raw)
To: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, Mark Brown
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc,
Troy Mitchell
In-Reply-To: <20251118-p1-kconfig-fix-v3-0-8839c5ac5db3@linux.spacemit.com>
The default value of the P1 sub-device depends on the value
of P1, so P1 should have a default value here.
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
drivers/mfd/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 6cec1858947bf7ab5ee78beb730c95dabcb43a98..b0f109b3acc40b074e4d0178e123437495853496 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1260,6 +1260,7 @@ config MFD_SPACEMIT_P1
depends on I2C
select I2C_K1
select MFD_SIMPLE_MFD_I2C
+ default ARCH_SPACEMIT
help
This option supports the I2C-based SpacemiT P1 PMIC, which
contains regulators, a power switch, GPIOs, an RTC, and more.
--
2.51.2
^ permalink raw reply related
* [PATCH v3 3/4] regulator: spacemit: MFD_SPACEMIT_P1 as dependencies
From: Troy Mitchell @ 2025-11-18 6:08 UTC (permalink / raw)
To: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, Mark Brown
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc,
Troy Mitchell
In-Reply-To: <20251118-p1-kconfig-fix-v3-0-8839c5ac5db3@linux.spacemit.com>
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>
Signed-off-by: Troy Mitchell <troy.mitchell@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 d84f3d054c59d86d91d859808aa73a3b609d16d0..e2cbbb90500189a1c4282511b8d7141301cae1f0 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1455,9 +1455,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 MFD_SPACEMIT_P1
help
Enable support for regulators implemented by the SpacemiT P1
power controller. The P1 implements 6 high-efficiency buck
--
2.51.2
^ permalink raw reply related
* [PATCH v3 2/4] rtc: spacemit: MFD_SPACEMIT_P1 as dependencies
From: Troy Mitchell @ 2025-11-18 6:08 UTC (permalink / raw)
To: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, Mark Brown
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc,
Troy Mitchell
In-Reply-To: <20251118-p1-kconfig-fix-v3-0-8839c5ac5db3@linux.spacemit.com>
RTC_DRV_SPACEMIT_P1 is a subdevice of P1 and should depend on
MFD_SPACEMIT_P1 rather than selecting it directly. Using 'select'
does not always respect the parent's dependencies, so 'depends on'
is the safer and more correct choice.
Additionally, the default value depends on MFD_SPACEMIT_P1 rather
than ARCH_SPACEMIT.
Signed-off-by: Troy Mitchell <troy.mitchell@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-3-49688f30bae8@linux.spacemit.com/
---
drivers/rtc/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 2933c41c77c88e60df721fe65b9c8afb995ae51e..b392e6d096ed077e841a2e68b70d8b80d9ad1cde 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -409,8 +409,8 @@ config RTC_DRV_MAX77686
config RTC_DRV_SPACEMIT_P1
tristate "SpacemiT P1 RTC"
depends on ARCH_SPACEMIT || COMPILE_TEST
- select MFD_SPACEMIT_P1
- default ARCH_SPACEMIT
+ depends on MFD_SPACEMIT_P1
+ default 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.51.2
^ permalink raw reply related
* [PATCH v3 1/4] i2c: spacemit: configure ILCR for accurate SCL frequency
From: Troy Mitchell @ 2025-11-18 6:08 UTC (permalink / raw)
To: Lee Jones, Yixun Lan, Alex Elder, Andi Shyti, Alexandre Belloni,
Liam Girdwood, Mark Brown
Cc: linux-kernel, linux-riscv, spacemit, linux-i2c, linux-rtc,
Troy Mitchell
In-Reply-To: <20251118-p1-kconfig-fix-v3-0-8839c5ac5db3@linux.spacemit.com>
The SpacemiT I2C controller's SCL (Serial Clock Line) frequency for
master mode operations is determined by the ILCR (I2C Load Count Register).
Previously, the driver relied on the hardware's reset default
values for this register.
The hardware's default ILCR values (SLV=0x156, FLV=0x5d) yield SCL
frequencies lower than intended. For example, with the default
31.5 MHz input clock, these default settings result in an SCL
frequency of approximately 93 kHz (standard mode) when targeting 100 kHz,
and approximately 338 kHz (fast mode) when targeting 400 kHz.
These frequencies are below the 100 kHz/400 kHz nominal speeds.
This patch integrates the SCL frequency management into
the Common Clock Framework (CCF). Specifically, the ILCR register,
which acts as a frequency divider for the SCL clock, is now registered
as a managed clock (scl_clk) within the CCF.
This patch also cleans up unnecessary whitespace
in the included header files.
Reviewed-by: Yixun Lan <dlan@gentoo.org>
Link: https://lore.kernel.org/all/176244506110.1925720.10807118665958896958.b4-ty@kernel.org/ [1]
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
This patch was affected by the P1 Kconfig, which caused the maintainer
to revert it.
The current commit is a direct cherry-pick and reserves the original changelog.
This note is to clarify for anyone who sees the cover letter marked as v2
while the changelog entries reach v4.
---
Changelog in v4:
- initialize clk_init_data with {} so that init.flags is implicitly set to 0
- minor cleanup and style fixes for better readability
- remove unused spacemit_i2c_scl_clk_exclusive_put() cleanup callback
- replace clk_set_rate_exclusive()/clk_rate_exclusive_put() pair with clk_set_rate()
- simplify LCR LV field macros by using FIELD_GET/FIELD_MAX helpers
- Link to v3: https://lore.kernel.org/all/20250814-k1-i2c-ilcr-v3-1-317723e74bcd@linux.spacemit.com/
Changelog in v3:
- use MASK macro in `recalc_rate` function
- rename clock name
- Link to v2: https://lore.kernel.org/r/20250718-k1-i2c-ilcr-v2-1-b4c68f13dcb1@linux.spacemit.com
Changelog in v2:
- Align line breaks.
- Check `lv` in `clk_set_rate` function.
- Force fast mode when SCL frequency is illegal or unavailable.
- Change "linux/bits.h" to <linux/bits.h>
- Kconfig: Add dependency on CCF.
- Link to v1: https://lore.kernel.org/all/20250710-k1-i2c-ilcr-v1-1-188d1f460c7d@linux.spacemit.com/
---
drivers/i2c/busses/Kconfig | 2 +-
drivers/i2c/busses/i2c-k1.c | 159 ++++++++++++++++++++++++++++++++++++++++----
2 files changed, 146 insertions(+), 15 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index fd81e49638aaa161ae264a722e9e06adc7914cda..fedf5d31f9035b73a27a7f8a764bf5c26975d0e1 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -798,7 +798,7 @@ config I2C_JZ4780
config I2C_K1
tristate "SpacemiT K1 I2C adapter"
depends on ARCH_SPACEMIT || COMPILE_TEST
- depends on OF
+ depends on OF && COMMON_CLK
help
This option enables support for the I2C interface on the SpacemiT K1
platform.
diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
index 6b918770e612e098b8ad17418f420d87c94df166..e38a0ba71734ca602854c85672dcb61423453515 100644
--- a/drivers/i2c/busses/i2c-k1.c
+++ b/drivers/i2c/busses/i2c-k1.c
@@ -4,18 +4,21 @@
*/
#include <linux/bitfield.h>
- #include <linux/clk.h>
- #include <linux/i2c.h>
- #include <linux/iopoll.h>
- #include <linux/module.h>
- #include <linux/of_address.h>
- #include <linux/platform_device.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/i2c.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
/* spacemit i2c registers */
#define SPACEMIT_ICR 0x0 /* Control register */
#define SPACEMIT_ISR 0x4 /* Status register */
#define SPACEMIT_IDBR 0xc /* Data buffer register */
#define SPACEMIT_IRCR 0x18 /* Reset cycle counter */
+#define SPACEMIT_ILCR 0x10 /* Load Count Register */
#define SPACEMIT_IBMR 0x1c /* Bus monitor register */
/* SPACEMIT_ICR register fields */
@@ -87,6 +90,13 @@
#define SPACEMIT_BMR_SDA BIT(0) /* SDA line level */
#define SPACEMIT_BMR_SCL BIT(1) /* SCL line level */
+#define SPACEMIT_LCR_LV_STANDARD_SHIFT 0
+#define SPACEMIT_LCR_LV_FAST_SHIFT 9
+#define SPACEMIT_LCR_LV_STANDARD_MASK GENMASK(8, 0)
+#define SPACEMIT_LCR_LV_FAST_MASK GENMASK(17, 9)
+#define SPACEMIT_LCR_LV_STANDARD_MAX_VALUE FIELD_MAX(SPACEMIT_LCR_LV_STANDARD_MASK)
+#define SPACEMIT_LCR_LV_FAST_MAX_VALUE FIELD_MAX(SPACEMIT_LCR_LV_FAST_MASK)
+
/* i2c bus recover timeout: us */
#define SPACEMIT_I2C_BUS_BUSY_TIMEOUT 100000
@@ -104,11 +114,20 @@ enum spacemit_i2c_state {
SPACEMIT_STATE_WRITE,
};
+enum spacemit_i2c_mode {
+ SPACEMIT_MODE_STANDARD,
+ SPACEMIT_MODE_FAST
+};
+
/* i2c-spacemit driver's main struct */
struct spacemit_i2c_dev {
struct device *dev;
struct i2c_adapter adapt;
+ struct clk_hw scl_clk_hw;
+ struct clk *scl_clk;
+ enum spacemit_i2c_mode mode;
+
/* hardware resources */
void __iomem *base;
int irq;
@@ -129,6 +148,79 @@ struct spacemit_i2c_dev {
u32 status;
};
+static void spacemit_i2c_scl_clk_disable_unprepare(void *data)
+{
+ struct spacemit_i2c_dev *i2c = data;
+
+ clk_disable_unprepare(i2c->scl_clk);
+}
+
+static int spacemit_i2c_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct spacemit_i2c_dev *i2c = container_of(hw, struct spacemit_i2c_dev, scl_clk_hw);
+ u32 lv, lcr, mask, shift, max_lv;
+
+ lv = DIV_ROUND_UP(parent_rate, rate);
+
+ if (i2c->mode == SPACEMIT_MODE_STANDARD) {
+ mask = SPACEMIT_LCR_LV_STANDARD_MASK;
+ shift = SPACEMIT_LCR_LV_STANDARD_SHIFT;
+ max_lv = SPACEMIT_LCR_LV_STANDARD_MAX_VALUE;
+ } else if (i2c->mode == SPACEMIT_MODE_FAST) {
+ mask = SPACEMIT_LCR_LV_FAST_MASK;
+ shift = SPACEMIT_LCR_LV_FAST_SHIFT;
+ max_lv = SPACEMIT_LCR_LV_FAST_MAX_VALUE;
+ }
+
+ if (!lv || lv > max_lv) {
+ dev_err(i2c->dev, "set scl clock failed: lv 0x%x", lv);
+ return -EINVAL;
+ }
+
+ lcr = readl(i2c->base + SPACEMIT_ILCR);
+ lcr &= ~mask;
+ lcr |= lv << shift;
+ writel(lcr, i2c->base + SPACEMIT_ILCR);
+
+ return 0;
+}
+
+static long spacemit_i2c_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ u32 lv, freq;
+
+ lv = DIV_ROUND_UP(*parent_rate, rate);
+ freq = DIV_ROUND_UP(*parent_rate, lv);
+
+ return freq;
+}
+
+static unsigned long spacemit_i2c_clk_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct spacemit_i2c_dev *i2c = container_of(hw, struct spacemit_i2c_dev, scl_clk_hw);
+ u32 lcr, lv = 0;
+
+ lcr = readl(i2c->base + SPACEMIT_ILCR);
+
+ if (i2c->mode == SPACEMIT_MODE_STANDARD)
+ lv = FIELD_GET(SPACEMIT_LCR_LV_STANDARD_MASK, lcr);
+ else if (i2c->mode == SPACEMIT_MODE_FAST)
+ lv = FIELD_GET(SPACEMIT_LCR_LV_FAST_MASK, lcr);
+ else
+ return 0;
+
+ return DIV_ROUND_UP(parent_rate, lv);
+}
+
+static const struct clk_ops spacemit_i2c_clk_ops = {
+ .set_rate = spacemit_i2c_clk_set_rate,
+ .round_rate = spacemit_i2c_clk_round_rate,
+ .recalc_rate = spacemit_i2c_clk_recalc_rate,
+};
+
static void spacemit_i2c_enable(struct spacemit_i2c_dev *i2c)
{
u32 val;
@@ -147,6 +239,26 @@ static void spacemit_i2c_disable(struct spacemit_i2c_dev *i2c)
writel(val, i2c->base + SPACEMIT_ICR);
}
+static struct clk *spacemit_i2c_register_scl_clk(struct spacemit_i2c_dev *i2c,
+ struct clk *parent)
+{
+ struct clk_init_data init = {};
+ char name[32];
+
+ snprintf(name, sizeof(name), "%s_scl_clk", dev_name(i2c->dev));
+
+ init.name = name;
+ init.ops = &spacemit_i2c_clk_ops;
+ init.parent_data = (struct clk_parent_data[]) {
+ { .fw_name = "func" },
+ };
+ init.num_parents = 1;
+
+ i2c->scl_clk_hw.init = &init;
+
+ return devm_clk_register(i2c->dev, &i2c->scl_clk_hw);
+}
+
static void spacemit_i2c_reset(struct spacemit_i2c_dev *i2c)
{
writel(SPACEMIT_CR_UR, i2c->base + SPACEMIT_ICR);
@@ -246,7 +358,7 @@ static void spacemit_i2c_init(struct spacemit_i2c_dev *i2c)
*/
val |= SPACEMIT_CR_DRFIE;
- if (i2c->clock_freq == SPACEMIT_I2C_MAX_FAST_MODE_FREQ)
+ if (i2c->mode == SPACEMIT_MODE_FAST)
val |= SPACEMIT_CR_MODE_FAST;
/* disable response to general call */
@@ -538,14 +650,15 @@ static int spacemit_i2c_probe(struct platform_device *pdev)
dev_warn(dev, "failed to read clock-frequency property: %d\n", ret);
/* For now, this driver doesn't support high-speed. */
- if (!i2c->clock_freq || i2c->clock_freq > SPACEMIT_I2C_MAX_FAST_MODE_FREQ) {
- dev_warn(dev, "unsupported clock frequency %u; using %u\n",
- i2c->clock_freq, SPACEMIT_I2C_MAX_FAST_MODE_FREQ);
+ if (i2c->clock_freq > SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ &&
+ i2c->clock_freq <= SPACEMIT_I2C_MAX_FAST_MODE_FREQ) {
+ i2c->mode = SPACEMIT_MODE_FAST;
+ } else if (i2c->clock_freq && i2c->clock_freq <= SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ) {
+ i2c->mode = SPACEMIT_MODE_STANDARD;
+ } else {
+ dev_warn(i2c->dev, "invalid clock-frequency, fallback to fast mode");
+ i2c->mode = SPACEMIT_MODE_FAST;
i2c->clock_freq = SPACEMIT_I2C_MAX_FAST_MODE_FREQ;
- } else if (i2c->clock_freq < SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ) {
- dev_warn(dev, "unsupported clock frequency %u; using %u\n",
- i2c->clock_freq, SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ);
- i2c->clock_freq = SPACEMIT_I2C_MAX_STANDARD_MODE_FREQ;
}
i2c->dev = &pdev->dev;
@@ -567,10 +680,28 @@ static int spacemit_i2c_probe(struct platform_device *pdev)
if (IS_ERR(clk))
return dev_err_probe(dev, PTR_ERR(clk), "failed to enable func clock");
+ i2c->scl_clk = spacemit_i2c_register_scl_clk(i2c, clk);
+ if (IS_ERR(i2c->scl_clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(i2c->scl_clk),
+ "failed to register scl clock\n");
+
clk = devm_clk_get_enabled(dev, "bus");
if (IS_ERR(clk))
return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock");
+ ret = clk_set_rate(i2c->scl_clk, i2c->clock_freq);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret, "failed to set rate for SCL clock");
+
+ ret = clk_prepare_enable(i2c->scl_clk);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret, "failed to prepare and enable clock");
+
+ ret = devm_add_action_or_reset(dev, spacemit_i2c_scl_clk_disable_unprepare, i2c);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to register cleanup action for clk disable and unprepare");
+
spacemit_i2c_reset(i2c);
i2c_set_adapdata(&i2c->adapt, i2c);
--
2.51.2
^ 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