* [PATCH v3 6/8] clk: sunxi-ng: div: add read-only operation support
From: Jerome Brunet @ 2026-07-02 8:10 UTC (permalink / raw)
To: Junhui Liu, Alexandre Belloni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Michael Turquette, Stephen Boyd, Maxime Ripard
Cc: linux-rtc, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel, linux-clk, Jerome Brunet
In-Reply-To: <20260702-a733-rtc-v3-0-eb2580374de6@baylibre.com>
Add support for sunxi-ng read-only dividers. This will be
useful to the a733 oscillator detection logic.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/clk/sunxi-ng/ccu_div.c | 42 ++++++++++++++++++++++++++++++++++++++++++
drivers/clk/sunxi-ng/ccu_div.h | 1 +
drivers/clk/sunxi-ng/ccu_mux.c | 3 ++-
drivers/clk/sunxi-ng/ccu_mux.h | 4 ++++
4 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c
index 62d680ccb524..d1c8c7baa12d 100644
--- a/drivers/clk/sunxi-ng/ccu_div.c
+++ b/drivers/clk/sunxi-ng/ccu_div.c
@@ -84,6 +84,36 @@ static int ccu_div_determine_rate(struct clk_hw *hw,
req, ccu_div_determine_rate_helper, cd);
}
+static int ccu_rodiv_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ struct ccu_div *cd = hw_to_ccu_div(hw);
+ unsigned long val;
+ u32 reg;
+ int ret;
+
+ reg = readl(cd->common.base + cd->common.reg);
+ val = reg >> cd->div.shift;
+ val &= (1 << cd->div.width) - 1;
+
+ req->rate = ccu_mux_helper_unapply_prediv(&cd->common, &cd->mux, -1,
+ req->rate);
+
+ if (cd->common.features & CCU_FEATURE_FIXED_POSTDIV)
+ req->rate *= cd->fixed_post_div;
+
+ ret = divider_ro_determine_rate(hw, req, cd->div.table,
+ cd->div.width, cd->div.flags, val);
+
+ if (cd->common.features & CCU_FEATURE_FIXED_POSTDIV)
+ req->rate /= cd->fixed_post_div;
+
+ req->rate = ccu_mux_helper_apply_prediv(&cd->common, &cd->mux, -1,
+ req->rate);
+
+ return ret;
+}
+
static int ccu_div_set_rate(struct clk_hw *hw, unsigned long rate,
unsigned long parent_rate)
{
@@ -143,3 +173,15 @@ const struct clk_ops ccu_div_ops = {
.set_rate = ccu_div_set_rate,
};
EXPORT_SYMBOL_NS_GPL(ccu_div_ops, "SUNXI_CCU");
+
+const struct clk_ops ccu_rodiv_ops = {
+ .disable = ccu_div_disable,
+ .enable = ccu_div_enable,
+ .is_enabled = ccu_div_is_enabled,
+
+ .get_parent = ccu_div_get_parent,
+
+ .determine_rate = ccu_rodiv_determine_rate,
+ .recalc_rate = ccu_div_recalc_rate,
+};
+EXPORT_SYMBOL_NS_GPL(ccu_rodiv_ops, "SUNXI_CCU");
diff --git a/drivers/clk/sunxi-ng/ccu_div.h b/drivers/clk/sunxi-ng/ccu_div.h
index be00b3277e97..a30a92780a05 100644
--- a/drivers/clk/sunxi-ng/ccu_div.h
+++ b/drivers/clk/sunxi-ng/ccu_div.h
@@ -300,5 +300,6 @@ static inline struct ccu_div *hw_to_ccu_div(struct clk_hw *hw)
}
extern const struct clk_ops ccu_div_ops;
+extern const struct clk_ops ccu_rodiv_ops;
#endif /* _CCU_DIV_H_ */
diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_mux.c
index 766f27cff748..e2d6833a6d33 100644
--- a/drivers/clk/sunxi-ng/ccu_mux.c
+++ b/drivers/clk/sunxi-ng/ccu_mux.c
@@ -68,13 +68,14 @@ unsigned long ccu_mux_helper_apply_prediv(struct ccu_common *common,
}
EXPORT_SYMBOL_NS_GPL(ccu_mux_helper_apply_prediv, "SUNXI_CCU");
-static unsigned long ccu_mux_helper_unapply_prediv(struct ccu_common *common,
+unsigned long ccu_mux_helper_unapply_prediv(struct ccu_common *common,
struct ccu_mux_internal *cm,
int parent_index,
unsigned long parent_rate)
{
return parent_rate * ccu_mux_get_prediv(common, cm, parent_index);
}
+EXPORT_SYMBOL_NS_GPL(ccu_mux_helper_unapply_prediv, "SUNXI_CCU");
int ccu_mux_helper_determine_rate(struct ccu_common *common,
struct ccu_mux_internal *cm,
diff --git a/drivers/clk/sunxi-ng/ccu_mux.h b/drivers/clk/sunxi-ng/ccu_mux.h
index c94a4bde5d01..272a2c36a8f2 100644
--- a/drivers/clk/sunxi-ng/ccu_mux.h
+++ b/drivers/clk/sunxi-ng/ccu_mux.h
@@ -134,6 +134,10 @@ unsigned long ccu_mux_helper_apply_prediv(struct ccu_common *common,
struct ccu_mux_internal *cm,
int parent_index,
unsigned long parent_rate);
+unsigned long ccu_mux_helper_unapply_prediv(struct ccu_common *common,
+ struct ccu_mux_internal *cm,
+ int parent_index,
+ unsigned long parent_rate);
int ccu_mux_helper_determine_rate(struct ccu_common *common,
struct ccu_mux_internal *cm,
struct clk_rate_request *req,
--
2.47.3
^ permalink raw reply related
* [PATCH v3 5/8] clk: sunxi-ng: sun6i-rtc: Add feature bit for IOSC calibration
From: Jerome Brunet @ 2026-07-02 8:10 UTC (permalink / raw)
To: Junhui Liu, Alexandre Belloni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Michael Turquette, Stephen Boyd, Maxime Ripard
Cc: linux-rtc, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel, linux-clk, Jerome Brunet
In-Reply-To: <20260702-a733-rtc-v3-0-eb2580374de6@baylibre.com>
From: Junhui Liu <junhui.liu@pigmoral.tech>
The sun6i-rtc CCU driver currently uses a global static variable to
denote whether calibration is supported, which makes IOSC operations
tightly coupled to this file.
Convert this into a feature bit to decouple the logic. This allows the
IOSC clock code to be moved into a shared module for reuse by other SoCs.
Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/clk/sunxi-ng/ccu-sun6i-rtc.c | 17 +++++++++--------
drivers/clk/sunxi-ng/ccu_common.h | 1 +
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
index 0f528bfaed00..b24c8b196e66 100644
--- a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
+++ b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
@@ -52,8 +52,6 @@ struct sun6i_rtc_match_data {
u8 osc32k_fanout_nparents;
};
-static bool have_iosc_calibration;
-
static int ccu_iosc_enable(struct clk_hw *hw)
{
struct ccu_common *cm = hw_to_ccu_common(hw);
@@ -80,7 +78,7 @@ static unsigned long ccu_iosc_recalc_rate(struct clk_hw *hw,
{
struct ccu_common *cm = hw_to_ccu_common(hw);
- if (have_iosc_calibration) {
+ if (cm->features & CCU_FEATURE_IOSC_CALIBRATION) {
u32 reg = readl(cm->base + IOSC_CLK_CALI_REG);
/*
@@ -119,7 +117,7 @@ static int ccu_iosc_32k_prepare(struct clk_hw *hw)
struct ccu_common *cm = hw_to_ccu_common(hw);
u32 val;
- if (!have_iosc_calibration)
+ if (!(cm->features & CCU_FEATURE_IOSC_CALIBRATION))
return 0;
val = readl(cm->base + IOSC_CLK_CALI_REG);
@@ -134,7 +132,7 @@ static void ccu_iosc_32k_unprepare(struct clk_hw *hw)
struct ccu_common *cm = hw_to_ccu_common(hw);
u32 val;
- if (!have_iosc_calibration)
+ if (!(cm->features & CCU_FEATURE_IOSC_CALIBRATION))
return;
val = readl(cm->base + IOSC_CLK_CALI_REG);
@@ -148,7 +146,7 @@ static unsigned long ccu_iosc_32k_recalc_rate(struct clk_hw *hw,
struct ccu_common *cm = hw_to_ccu_common(hw);
u32 val;
- if (have_iosc_calibration) {
+ if (cm->features & CCU_FEATURE_IOSC_CALIBRATION) {
val = readl(cm->base + IOSC_CLK_CALI_REG);
/* Assume the calibrated 32k clock is accurate. */
@@ -167,7 +165,7 @@ static unsigned long ccu_iosc_32k_recalc_accuracy(struct clk_hw *hw,
struct ccu_common *cm = hw_to_ccu_common(hw);
u32 val;
- if (have_iosc_calibration) {
+ if (cm->features & CCU_FEATURE_IOSC_CALIBRATION) {
val = readl(cm->base + IOSC_CLK_CALI_REG);
/* Assume the calibrated 32k clock is accurate. */
@@ -358,7 +356,10 @@ int sun6i_rtc_ccu_probe(struct device *dev, void __iomem *reg)
return 0;
data = match->data;
- have_iosc_calibration = data->have_iosc_calibration;
+ if (data->have_iosc_calibration) {
+ iosc_clk.features |= CCU_FEATURE_IOSC_CALIBRATION;
+ iosc_32k_clk.features |= CCU_FEATURE_IOSC_CALIBRATION;
+ }
if (!data->have_ext_osc32k) {
/* ext-osc32k-gate is an orphan, so do not register it. */
diff --git a/drivers/clk/sunxi-ng/ccu_common.h b/drivers/clk/sunxi-ng/ccu_common.h
index bbec283b9d99..d9dc24ad5503 100644
--- a/drivers/clk/sunxi-ng/ccu_common.h
+++ b/drivers/clk/sunxi-ng/ccu_common.h
@@ -21,6 +21,7 @@
#define CCU_FEATURE_CLOSEST_RATE BIT(9)
#define CCU_FEATURE_DUAL_DIV BIT(10)
#define CCU_FEATURE_UPDATE_BIT BIT(11)
+#define CCU_FEATURE_IOSC_CALIBRATION BIT(12)
/* MMC timing mode switch bit */
#define CCU_MMC_NEW_TIMING_MODE BIT(30)
--
2.47.3
^ permalink raw reply related
* [PATCH v3 4/8] clk: sunxi-ng: sun6i-rtc: clean up DT usage
From: Jerome Brunet @ 2026-07-02 8:10 UTC (permalink / raw)
To: Junhui Liu, Alexandre Belloni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Michael Turquette, Stephen Boyd, Maxime Ripard
Cc: linux-rtc, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel, linux-clk, Jerome Brunet
In-Reply-To: <20260702-a733-rtc-v3-0-eb2580374de6@baylibre.com>
With sun6i-rtc compatible devices, the "ext-osc32k" clock input
is optional for the devices that support this input (r329 and onward).
Probably preparing for older SoC support, the driver does something funny
when parsing DT. It check if "ext-osc32k" is present in the clock-names and
if it is not, it uses the first clock as "ext-osc32k". This clock will
actually be the rtc bus clock so what the driver does is wrong.
At the moment, the driver does not support the older SoCs that would have
an external 32k clock provided on index #0 so just remove this quirk.
Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/clk/sunxi-ng/ccu-sun6i-rtc.c | 23 +++--------------------
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
index f6bfeba009e8..0f528bfaed00 100644
--- a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
+++ b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
@@ -191,10 +191,8 @@ static struct ccu_common iosc_32k_clk = {
CLK_GET_RATE_NOCACHE),
};
-static const struct clk_hw *ext_osc32k[] = { NULL }; /* updated during probe */
-
-static SUNXI_CCU_GATE_HWS(ext_osc32k_gate_clk, "ext-osc32k-gate",
- ext_osc32k, 0x0, BIT(4), 0);
+static SUNXI_CCU_GATE_FW(ext_osc32k_gate_clk, "ext-osc32k-gate",
+ "ext-osc32k", 0x0, BIT(4), 0);
static const struct clk_hw *osc32k_parents[] = {
&iosc_32k_clk.hw,
@@ -352,7 +350,6 @@ MODULE_DEVICE_TABLE(of, sun6i_rtc_ccu_match);
int sun6i_rtc_ccu_probe(struct device *dev, void __iomem *reg)
{
const struct sun6i_rtc_match_data *data;
- struct clk *ext_osc32k_clk = NULL;
const struct of_device_id *match;
/* This driver is only used for newer variants of the hardware. */
@@ -363,21 +360,7 @@ int sun6i_rtc_ccu_probe(struct device *dev, void __iomem *reg)
data = match->data;
have_iosc_calibration = data->have_iosc_calibration;
- if (data->have_ext_osc32k) {
- const char *fw_name;
-
- /* ext-osc32k was the only input clock in the old binding. */
- fw_name = of_property_present(dev->of_node, "clock-names")
- ? "ext-osc32k" : NULL;
- ext_osc32k_clk = devm_clk_get_optional(dev, fw_name);
- if (IS_ERR(ext_osc32k_clk))
- return PTR_ERR(ext_osc32k_clk);
- }
-
- if (ext_osc32k_clk) {
- /* Link ext-osc32k-gate to its parent. */
- *ext_osc32k = __clk_get_hw(ext_osc32k_clk);
- } else {
+ if (!data->have_ext_osc32k) {
/* ext-osc32k-gate is an orphan, so do not register it. */
sun6i_rtc_ccu_hw_clks.hws[CLK_EXT_OSC32K_GATE] = NULL;
osc32k_init_data.num_parents = 1;
--
2.47.3
^ permalink raw reply related
* [PATCH v3 3/8] clk: sunxi-ng: fix ccu probe clock unregister on error
From: Jerome Brunet @ 2026-07-02 8:10 UTC (permalink / raw)
To: Junhui Liu, Alexandre Belloni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Michael Turquette, Stephen Boyd, Maxime Ripard
Cc: linux-rtc, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel, linux-clk, Sashiko, Jerome Brunet
In-Reply-To: <20260702-a733-rtc-v3-0-eb2580374de6@baylibre.com>
When registering clocks with sunxi_ccu_probe(), the number of ccu_clocks
and the number of hw clocks might be different, eventhough they usually are
the same.
If they are different, it could lead to out-of-bound access or registered
clock left behind on error.
Use a different variable when iterating on hw clocks so every registered
clock, and only those, gets unregistered on error.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/20260629131254.7E34C1F00A3A@smtp.kernel.org
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/clk/sunxi-ng/ccu_common.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu_common.c b/drivers/clk/sunxi-ng/ccu_common.c
index 1c083b4d0b7e..43d8eca6abee 100644
--- a/drivers/clk/sunxi-ng/ccu_common.c
+++ b/drivers/clk/sunxi-ng/ccu_common.c
@@ -114,7 +114,7 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device *dev,
const struct sunxi_ccu_desc *desc)
{
struct ccu_reset *reset;
- int i, ret;
+ int i, j, ret;
ccu->desc = desc;
@@ -130,8 +130,8 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device *dev,
cclk->lock = &ccu->lock;
}
- for (i = 0; i < desc->hw_clks->num ; i++) {
- struct clk_hw *hw = desc->hw_clks->hws[i];
+ for (j = 0; j < desc->hw_clks->num ; j++) {
+ struct clk_hw *hw = desc->hw_clks->hws[j];
const char *name;
if (!hw)
@@ -143,7 +143,7 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device *dev,
else
ret = of_clk_hw_register(node, hw);
if (ret) {
- pr_err("Couldn't register clock %d - %s\n", i, name);
+ pr_err("Couldn't register clock %d - %s\n", j, name);
goto err_clk_unreg;
}
}
@@ -186,8 +186,8 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device *dev,
err_del_provider:
of_clk_del_provider(node);
err_clk_unreg:
- while (--i >= 0) {
- struct clk_hw *hw = desc->hw_clks->hws[i];
+ while (--j >= 0) {
+ struct clk_hw *hw = desc->hw_clks->hws[j];
if (!hw)
continue;
--
2.47.3
^ permalink raw reply related
* [PATCH v3 2/8] dt-bindings: rtc: sun6i: add sun60i-a733 support
From: Jerome Brunet @ 2026-07-02 8:10 UTC (permalink / raw)
To: Junhui Liu, Alexandre Belloni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Michael Turquette, Stephen Boyd, Maxime Ripard
Cc: linux-rtc, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel, linux-clk, Jerome Brunet
In-Reply-To: <20260702-a733-rtc-v3-0-eb2580374de6@baylibre.com>
Add a new rtc compatible for the sun60i-a733 SoC and new IDs for the
peripheral oscillator clock gates of this SoC.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml | 1 +
include/dt-bindings/clock/sun6i-rtc.h | 4 ++++
2 files changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml b/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
index 959a012c626f..f2b91186ed37 100644
--- a/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
@@ -33,6 +33,7 @@ properties:
- enum:
- allwinner,sun20i-d1-rtc
- allwinner,sun55i-a523-rtc
+ - allwinner,sun60i-a733-rtc
- const: allwinner,sun50i-r329-rtc
reg:
diff --git a/include/dt-bindings/clock/sun6i-rtc.h b/include/dt-bindings/clock/sun6i-rtc.h
index 3bd3aa3d57ce..5132a393ca4b 100644
--- a/include/dt-bindings/clock/sun6i-rtc.h
+++ b/include/dt-bindings/clock/sun6i-rtc.h
@@ -6,5 +6,9 @@
#define CLK_OSC32K 0
#define CLK_OSC32K_FANOUT 1
#define CLK_IOSC 2
+#define CLK_HOSC_UFS 8
+#define CLK_HOSC_HDMI 9
+#define CLK_HOSC_SERDES0 10
+#define CLK_HOSC_SERDES1 11
#endif /* _DT_BINDINGS_CLK_SUN6I_RTC_H_ */
--
2.47.3
^ permalink raw reply related
* [PATCH v3 1/8] dt-bindings: rtc: sun6i: no clock-output-names on h616/r329
From: Jerome Brunet @ 2026-07-02 8:10 UTC (permalink / raw)
To: Junhui Liu, Alexandre Belloni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Michael Turquette, Stephen Boyd, Maxime Ripard
Cc: linux-rtc, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel, linux-clk, Sashiko, Jerome Brunet
In-Reply-To: <20260702-a733-rtc-v3-0-eb2580374de6@baylibre.com>
On h616 and r329 chips, clock output names are never defined through DT and
are not meant to be. Just disallow the property for those chips.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: http://lore.kernel.org/r/20260629125305.0DF981F000E9@smtp.kernel.org
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
.../devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml b/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
index 9df5cdb6f63f..959a012c626f 100644
--- a/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml
@@ -175,6 +175,18 @@ allOf:
interrupts:
minItems: 2
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - allwinner,sun50i-h616-rtc
+ - allwinner,sun50i-r329-rtc
+
+ then:
+ properties:
+ clock-output-names: false
+
required:
- "#clock-cells"
- compatible
--
2.47.3
^ permalink raw reply related
* [PATCH v3 0/8] clk: sun6i-rtc: Add support for Allwinner A733 SoC
From: Jerome Brunet @ 2026-07-02 8:09 UTC (permalink / raw)
To: Junhui Liu, Alexandre Belloni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Michael Turquette, Stephen Boyd, Maxime Ripard
Cc: linux-rtc, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel, linux-clk, Sashiko, Jerome Brunet
Add support for the Allwinner A733 RTC and its internal Clock Control
Unit (CCU). Reuse the rtc-sun6i rtc driver while introducing a new
SoC-specific RTC CCU driver to handle the hardware's evolved clock
structure.
The A733 implementation supports hardware detection of three external
crystal frequencies (19.2MHz, 24MHz and 26MHz), which is represented in
the driver via read-only divider operations. Implement logic to derive a
normalized 32kHz reference from these DCXO sources using fixed
pre-dividers. Additionally, provide several new DCXO gate clocks for
peripherals, including SerDes, HDMI, and UFS.
This was tested on a Raxda Cubie A7A.
Changes in v3:
- Disallow clock-output-names DT property for h616/r329 chips
- Fix ccu probe helper to properly unregister clocks on error
- Implement .determine_rate for ccu divider RO ops
- Drop unused DCXO_CTRL_REG_EN define
- Link to v2: https://patch.msgid.link/20260629-a733-rtc-v2-0-7b72112784f8@baylibre.com
Changes in v2:
* Changed DT bindings as suggested. Those have changed significantly
since v1 so I did not pick up Rob's review trailer
* Support added in the existing RTC CCU driver rather than a separate driver
* Added DT parsing clean up of the existing driver
* Xtal detection exposed by RO divider rather than a MUX.
* Dropped conversion to aux device for now. This is not strictly related
the a733 support and will submitted again later on.
* Link to v1: https://lore.kernel.org/r/20260121-a733-rtc-v1-0-d359437f23a7@pigmoral.tech
---
Jerome Brunet (7):
dt-bindings: rtc: sun6i: no clock-output-names on h616/r329
dt-bindings: rtc: sun6i: add sun60i-a733 support
clk: sunxi-ng: fix ccu probe clock unregister on error
clk: sunxi-ng: sun6i-rtc: clean up DT usage
clk: sunxi-ng: div: add read-only operation support
clk: sunxi-ng: sun6i-rtc: split main oscillator div and gate.
clk: sunxi-ng: sun6i-rtc: add a733 support
Junhui Liu (1):
clk: sunxi-ng: sun6i-rtc: Add feature bit for IOSC calibration
.../bindings/rtc/allwinner,sun6i-a31-rtc.yaml | 13 +++
drivers/clk/sunxi-ng/ccu-sun6i-rtc.c | 123 +++++++++++++++------
drivers/clk/sunxi-ng/ccu-sun6i-rtc.h | 3 +-
drivers/clk/sunxi-ng/ccu_common.c | 12 +-
drivers/clk/sunxi-ng/ccu_common.h | 1 +
drivers/clk/sunxi-ng/ccu_div.c | 42 +++++++
drivers/clk/sunxi-ng/ccu_div.h | 1 +
drivers/clk/sunxi-ng/ccu_mux.c | 3 +-
drivers/clk/sunxi-ng/ccu_mux.h | 4 +
include/dt-bindings/clock/sun6i-rtc.h | 4 +
10 files changed, 163 insertions(+), 43 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20251226-a733-rtc-c5167df14e6e
Best regards,
--
Jerome
^ permalink raw reply
* Re: [PATCH 2/8] dt-bindings: mfd: ROHM BD73800 PMIC
From: Krzysztof Kozlowski @ 2026-07-02 8:05 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Matti Vaittinen, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Michael Turquette, Stephen Boyd, Brian Masney, Linus Walleij,
Bartosz Golaszewski, Alexandre Belloni, devicetree, linux-kernel,
linux-clk, linux-gpio, linux-rtc
In-Reply-To: <3e700a3fa7872a96257ff25a77670ec05cfd239c.1782909323.git.mazziesaccount@gmail.com>
On Wed, Jul 01, 2026 at 03:41:34PM +0300, Matti Vaittinen wrote:
> +description:
> + BD73800GW is a single-chip power management IC for battery-powered
> + portable devices. It integrates 8 buck converters, 4 LDOs and a current
> + sense amplifier with ADC. Also included is a Real Time Clock (RTC) and a
> + 32.768 kHz clock gate. Depending on the OTP configuration the BD73800
> + may also have interrupt controller and GPIOs.
> +
> + There are also different variants called BD71851 and BD71885 which are
> + almost identical from the software point of view.
> +
> +properties:
> + compatible:
> + oneOf:
> + - const: rohm,bd73800
> +
> + - items:
> + - const: rohm,bd71851
> + - const: rohm,bd73800
> +
> + - items:
> + - const: rohm,bd71885
items:
- enum:
-
-
- const: rohm,bd73800
> + - const: rohm,bd73800
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + # The GPIO1, CLKOUT (GPIO2), FAULT_B and EXTEN_OUT pins can be
> + # configured to interrupt pins by OTP.
> + interrupt-controller: true
> +
> + "#interrupt-cells":
> + const: 1
> + description:
> + The IRQ number. 0 is GPIO1, 1 CLKOUT (GPIO2), 2 FAULT_B and 3 EXTEN_OUT.
> + NOTE, A pin can operate as IRQ source only when the OTP
> + configuration for it has been set to GPI.
> +
> + gpio-controller: true
> +
> + "#gpio-cells":
> + const: 2
> +
> +# The GPIO1, CLKOUT, FAULT_B and EXTEN_OUT pins may be configured for a
Missing two spaces (indent) before the comment, although this should be
put into description. I understand it applies to each description - it
is fine to add to the first one. Descriptions might be used to generate
user-friendly representation of bindings (PDF). Comments won't, so
comments are only to explain the binding/schema syntax choices.
> +# specific purpose (like ADC input, 32.768 clk output, fault indicator or
> +# delivering power sequence to a companion PMIC when multiple PMICs are
> +# used) - but also to be either a GPO or GPI. (When used as a GPI the pin
> +# can also be used as an IRQ source). The pin purpose is determined by
> +# OTP (One Time Programmable memory), typically during device manufacturing.
> +# The OTP can't be read at runtime so device-tree should describe the pins.
> + rohm,pin-gpio1:
> + $ref: /schemas/types.yaml#/definitions/string
> + description:
> + Indicate if the GPIO1 pin has been set to GPI or GPO at manufacturing.
> + enum: [gpi, gpo]
> +
> + rohm,pin-clkout:
> + $ref: /schemas/types.yaml#/definitions/string
> + description:
> + Indicate if the CLKOUT pin has been set to GPI or GPO at manufacturing.
> + enum: [gpi, gpo]
> +
> + rohm,pin-fault-b:
> + $ref: /schemas/types.yaml#/definitions/string
> + description:
> + Indicate if the FAULT_B pin has been set to GPI or GPO at manufacturing.
> + enum: [gpi, gpo]
> +
> + rohm,pin-exten:
> + $ref: /schemas/types.yaml#/definitions/string
> + description:
> + Indicate if the EXTEN_OUT pin has been set to GPI or GPO at
> + manufacturing.
> + enum: [gpi, gpo]
> +
> + # The CLKOUT pin may have its purpose overridden by OTP configuration. It is
> + # possible the BD73800 does not output a clock signal. Hence the optional clk
> + # properties.
Same here
items:
- description: foo bar
> + clocks:
> + maxItems: 1
> +
> + "#clock-cells":
> + const: 0
> +
> + clock-output-names:
> + const: bd73800-32k-out
If this is fixed, then drop clock-output-names. Otherwise should be just
maxItems: 1
> +
> + rohm,clkout-open-drain:
> + description: clk32kout mode. Set to 1 for "open-drain" or 0 for "cmos".
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 1
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: rtc: Add sii,wakealarm-output-pin property for S35390A
From: Krzysztof Kozlowski @ 2026-07-02 6:09 UTC (permalink / raw)
To: Markus Probst, Alexandre Belloni
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Uwe Kleine-König, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-arm-kernel, linux-rtc, devicetree,
linux-kernel
In-Reply-To: <74b32ed0a700e3900c0f34d730b2b5b69eb2ca19.camel@posteo.de>
On 01/07/2026 19:08, Markus Probst wrote:
> On Wed, 2026-07-01 at 18:48 +0200, Alexandre Belloni wrote:
>> On 01/07/2026 16:43:07+0000, Markus Probst wrote:
>>> On Wed, 2026-07-01 at 17:14 +0200, Krzysztof Kozlowski wrote:
>>>> On 01/07/2026 15:25, Markus Probst wrote:
>>>>>>> +
>>>>>>> +maintainers:
>>>>>>> + - Alexandre Belloni <alexandre.belloni@bootlin.com>
>>>>>>
>>>>>> This should be someone caring about this hardware.
>>>>> He does have the majority of commits on this driver (excluding merge
>>>>> commits and commits not exclusive to this driver), although most of
>>>>> them are pretty tiny.
>>>>>
>>>>> Who would you suggest instead?
>>>>
>>>> Someone adding features for this driver, maybe driver maintainers. But
>>>> if Alexandre is fine, you can leave him.
>>>>
>>>>>>
>>>>>>> +
>>>>>>> +description:
>>>>>>> + The S-35390A is a CMOS 2-wire real-time clock IC which operates with the
>>>>>>> + very low current consumption in the wide range of operation voltage.
>>>>>>> +
>>>>>>> +allOf:
>>>>>>> + - $ref: rtc.yaml#
>>>>>>> +
>>>>>>> +properties:
>>>>>>> + compatible:
>>>>>>> + const: sii,s35390a
>>>>>>> +
>>>>>>> + reg:
>>>>>>> + maxItems: 1
>>>>>>> +
>>>>>>> + sii,wakealarm-output-pin:
>>>>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>>>>> + enum: [1, 2]
>>>>>>> + description: |
>>>>>>> + The output pin to wake up the system.
>>>>>>> + Default will use the output pin for interrupt signal 2.
>>>>>>> + <S35390A_OUTPUT_PIN_INT1> : Output pin for interrupt signal 1
>>>>>>> + <S35390A_OUTPUT_PIN_INT2> : Output pin for interrupt signal 2
>>>>>>
>>>>>> Does that mean device generates the interrupts?
>>>>> Yes.
>>>>>
>>>>
>>>>
>>>> Then I think you miss interrupts property.
>>> From what I can tell the line is used to generate a system wakeup
>>> event.
>>>
>>> There would be no obvious benefit of connecting it to an interrupt
>>> controller, so this property would be obsolete?
>>>
>>
>> Then you need proper wakeup-source support
> Wouldn't that break existing devicetrees?
How?
>
> The current driver allows to wake up the system, even without
> having wakeup-source set.
Anyway, wakeup-source is already there in rtc, so this would be done. I
don't get though, why there is no benefit of routing it to interrupt
controller (interrupt controllers do wake up the system). Additionally,
if you do not connect it to any interrupt, then how does it wake up the
system?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 5/8] regulator: bd71828: Support ROHM BD73800
From: Matti Vaittinen @ 2026-07-02 4:55 UTC (permalink / raw)
To: Mark Brown
Cc: Matti Vaittinen, Matti Vaittinen, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Michael Turquette, Stephen Boyd, Brian Masney, Linus Walleij,
Bartosz Golaszewski, Alexandre Belloni, devicetree, linux-kernel,
linux-clk, linux-gpio, linux-rtc
In-Reply-To: <bf744d4f-eabe-48e7-92e4-b147b8d79d91@sirena.org.uk>
On 01/07/2026 16:01, Mark Brown wrote:
> On Wed, Jul 01, 2026 at 03:42:35PM +0300, Matti Vaittinen wrote:
>> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
>> + nproot = of_get_child_by_name(nproot, "regulators");
>> + if (!nproot) {
>> + dev_err(dev, "failed to find regulators node\n");
>> + return -ENODEV;
>> + }
>> + for_each_child_of_node(nproot, np) {
>> + if (of_node_name_eq(np, LDO1_NODE_NAME))
>> + ldo1_use_high_range = of_property_read_bool(np,
>> + "rohm,ldo-range-high");
>> + if (of_node_name_eq(np, LDO3_NODE_NAME))
>> + ldo3_use_high_range = of_property_read_bool(np,
>> + "rohm,ldo-range-high");
>> + }
>
> Why do we iterate over all nodes rather than doing additional
> of_get_child_by_name()s?
This series has been sitting in my "TODO" -folder for quite a while - so
I am not anymore 100% sure as to why. I believe I've thought that the
of_get_child_by_name() does (internally) iterate all the child nodes, so
using it twice would cause code to loop through the nodes twice. So,
looping through all child nodes in a single loop probably felt like the
right thing to do. Furthermore, I've probably written the first version
before I found out the cleanup.h...
But yes. I think you're right. This can be made much leaner.
>> + if (ldo1_use_high_range) {
>> + d[BD73800_LDO1].desc.linear_ranges = bd73800_ldo13_high_volts;
>> + d[BD73800_LDO1].desc.n_linear_ranges =
>> + ARRAY_SIZE(bd73800_ldo13_high_volts);
>> + }
>> + if (ldo3_use_high_range) {
>> + d[BD73800_LDO3].desc.linear_ranges = bd73800_ldo13_high_volts;
>> + d[BD73800_LDO3].desc.n_linear_ranges =
>> + ARRAY_SIZE(bd73800_ldo13_high_volts);
>> + }
>
> You could just do these updates without the intermediate variables.
Yes.
Thanks for the suggestions! I will fix these for the v2, but it will
probably be out only during August.
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* Re: [PATCH 1/8] dt-bindings: regulator: ROHM BD73800 regulators
From: Matti Vaittinen @ 2026-07-02 4:45 UTC (permalink / raw)
To: Rob Herring
Cc: Matti Vaittinen, Matti Vaittinen, Lee Jones, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Michael Turquette,
Stephen Boyd, Brian Masney, Linus Walleij, Bartosz Golaszewski,
Alexandre Belloni, devicetree, linux-kernel, linux-clk,
linux-gpio, linux-rtc
In-Reply-To: <20260701192559.GA1313239-robh@kernel.org>
Hi Rob,
Thanks (again) for the review!
On 01/07/2026 22:25, Rob Herring wrote:
> On Wed, Jul 01, 2026 at 03:41:11PM +0300, Matti Vaittinen wrote:
>> From: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> Add bindings for the BUCKs and LDOs on ROHM BD73800. The PMIC state
>> specific voltages can be set in same fashion as with a few other ROHM
>> PMICs (for example with BD718[15,28,37,47,50,79]). Same properties are
>> recycled :)
>>
>> The LDOs 1 and 4 can use different voltage ranges depending on the OTP
>> configuration.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>> ---
>> .../regulator/rohm,bd73800-regulator.yaml | 119 ++++++++++++++++++
>> 1 file changed, 119 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml b/Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml
>> new file mode 100644
>> index 000000000000..c427a04098ec
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml
>
>
>> + rohm,dvs-run-voltage:
>> + description:
>> + PMIC default "RUN" state voltage in uV. 0 means disabled. See the
>> + explanation below for regulator specific details.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + minimum: 0
>> + maximum: 3500000
>
> [...]
>
>> + rohm,dvs-run-voltage:
>> + description:
>> + Set the default output state at PMIC's "RUN" state.
>> + 0 is disabled, 1 is enabled.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + minimum: 0
>> + maximum: 1
>
> Same property name with 2 different meanings. Not a good design pattern.
Hmm. They do actually have the same meaning. Setting the "RUN" -state
voltage. Values '0' and '1' have special meaning "disable" and "enable"
- also for BUCKs.
For LDOs on this PMIC, only the enable/disable configuration can be set
for each hardware-state as the LDO voltage is same for all hardware
states. Hence only subset of the property values (1/0) are supported for
the LDOs.
> Also, if these properties are copied from other schemas, don't duplicate
> them. Put them in a common schema and reference it here.
Ah. I think this is a great idea, and I should've thought that already a
few PMICs ago :) We have been reviewing and discussing these properties
with you since ... maybe 2018, as I've added new PMICs re-using them... ;)
Thanks! I'll rework this but v2 is likely to be out only at August.
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* Re: [PATCH 4/4] rtc: s35390a: convert to dev_err_probe()
From: Balakrishnan.S @ 2026-07-02 4:01 UTC (permalink / raw)
To: alexandre.belloni
Cc: amergnat, baolin.wang, zhang.lyra, orsonzhai, linux-kernel,
linux-rtc
In-Reply-To: <202607010857395be38657@mail.local>
Hi Alexandre Belloni,
On 01/07/26 2:27 pm, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hello,
>
> On 01/07/2026 07:26:29+0000, Balakrishnan.S@microchip.com wrote:
>> Hi Alexandre,
>>
>> Thanks for the review/feedback.
>>
>> On 30/06/26 10:40 pm, Alexandre Mergnat wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On Thu, 28 May 2026 09:16:47 +0530, Balakrishnan Sambath <balakrishnan.s@microchip.com> wrote:
>>>> diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
>>>> index a4678d7c6cf6..342fd2b568a3 100644
>>>> --- a/drivers/rtc/rtc-s35390a.c
>>>> +++ b/drivers/rtc/rtc-s35390a.c
>>>> @@ -479,10 +479,8 @@ static int s35390a_probe(struct i2c_client *client)
>>>> return PTR_ERR(rtc);
>>>>
>>>> err_read = s35390a_read_status(s35390a, &status1);
>>>> - if (err_read < 0) {
>>>> - dev_err(dev, "error resetting chip\n");
>>>> - return err_read;
>>>> - }
>>>> + if (err_read < 0)
>>>> + return dev_err_probe(dev, err_read, "error resetting chip\n");
>>>
>>> The devm_i2c_new_dummy_device() loop above this hunk still uses
>>> dev_err()+return PTR_ERR("Address %02x unavailable"). dev_err_probe() takes
>>> format args, so it converts cleanly:
>>>
>>> return dev_err_probe(dev, PTR_ERR(s35390a->client[i]),
>>> "Address %02x unavailable\n", client->addr + i);
>>>
>>> Worth converting for consistency with the rest of the probe.
>> Sure, I'll fix this too in next revision.
>>>
>>>> @@ -493,16 +491,12 @@ static int s35390a_probe(struct i2c_client *client)
>>>> /* disable alarm (and maybe test mode) */
>>>> buf = 0;
>>>> err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &buf, 1);
>>>> - if (err < 0) {
>>>> - dev_err(dev, "error disabling alarm");
>>>> - return err;
>>>> - }
>>>> + if (err < 0)
>>>> + return dev_err_probe(dev, err, "error disabling alarm");
>>>
>>> This message is missing its trailing newline (pre-existing). dev_err_probe()
>>> formats as "error %pe: %pV" and does not append "\n" itself, so the line
>>> runs into the next log message. Since you are touching this line, adding
>>> "\n" is a cheap fix even if the issue was here before your patch.
>>> I recommand to fix it ;)
>> Okay noted. Will fix it too.
>
> Honestly, my plan was to not apply those patches because once I do
> that, I'll get hundreds of those. There is no benefit to the change and
> I'll cite the dev_err_probe doc:
>
> * This helper implements common pattern present in probe functions for error
> * checking: print debug or error message depending if the error value is
> * -EPROBE_DEFER and propagate error upwards.
> * In case of -EPROBE_DEFER it sets also defer probe reason, which can be
> * checked later by reading devices_deferred debugfs attribute.
> * It replaces the following code sequence::
> *
> * if (err != -EPROBE_DEFER)
> * dev_err(dev, ...);
> * else
> * dev_dbg(dev, ...);
> * return err;
>
>
> We are not checking for EPROBE_DEFER in any of the drivers so there is
> no point in doing the change.
Okay thanks for the feedback. Lets drop this.
>
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
^ permalink raw reply
* Re: [PATCH 1/8] dt-bindings: regulator: ROHM BD73800 regulators
From: Rob Herring @ 2026-07-01 19:25 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Matti Vaittinen, Lee Jones, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Michael Turquette,
Stephen Boyd, Brian Masney, Linus Walleij, Bartosz Golaszewski,
Alexandre Belloni, devicetree, linux-kernel, linux-clk,
linux-gpio, linux-rtc
In-Reply-To: <67b42b5363533f11c22a6421417c3345f9872aec.1782909323.git.mazziesaccount@gmail.com>
On Wed, Jul 01, 2026 at 03:41:11PM +0300, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> Add bindings for the BUCKs and LDOs on ROHM BD73800. The PMIC state
> specific voltages can be set in same fashion as with a few other ROHM
> PMICs (for example with BD718[15,28,37,47,50,79]). Same properties are
> recycled :)
>
> The LDOs 1 and 4 can use different voltage ranges depending on the OTP
> configuration.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
> .../regulator/rohm,bd73800-regulator.yaml | 119 ++++++++++++++++++
> 1 file changed, 119 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml
>
> diff --git a/Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml b/Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml
> new file mode 100644
> index 000000000000..c427a04098ec
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml
> + rohm,dvs-run-voltage:
> + description:
> + PMIC default "RUN" state voltage in uV. 0 means disabled. See the
> + explanation below for regulator specific details.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 3500000
[...]
> + rohm,dvs-run-voltage:
> + description:
> + Set the default output state at PMIC's "RUN" state.
> + 0 is disabled, 1 is enabled.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 1
Same property name with 2 different meanings. Not a good design pattern.
Also, if these properties are copied from other schemas, don't duplicate
them. Put them in a common schema and reference it here.
Rob
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: rtc: Add sii,wakealarm-output-pin property for S35390A
From: Markus Probst @ 2026-07-01 17:34 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Uwe Kleine-König, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-arm-kernel, linux-rtc, devicetree,
linux-kernel
In-Reply-To: <202607011506045358209c@mail.local>
[-- Attachment #1: Type: text/plain, Size: 2564 bytes --]
On Wed, 2026-07-01 at 17:06 +0200, Alexandre Belloni wrote:
> On 30/06/2026 19:22:21+0000, Markus Probst wrote:
> > Synology NAS devices use the output pin for interrupt signal 1 to wake up
> > the system.
> >
> > Move devicetree bindings for sii,s35390a into its own file.
> > Add sii,wakealarm-output-pin property to enable the use of the output
> > pin for interrupt signal 1 for the wake alarm, which makes it possible to
> > set an wake alarm on Synology NAS devices.
> >
> > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > ---
> > .../devicetree/bindings/rtc/sii,s35390a.yaml | 54 ++++++++++++++++++++++
> > .../devicetree/bindings/rtc/trivial-rtc.yaml | 3 --
> > MAINTAINERS | 1 +
> > include/dt-bindings/rtc/s35390a.h | 9 ++++
> > 4 files changed, 64 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml b/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml
> > new file mode 100644
> > index 000000000000..31a578673870
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml
> > @@ -0,0 +1,54 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/rtc/sii,s35390a.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: S-35390A 2-WIRE REAL-TIME CLOCK
> > +
> > +maintainers:
> > + - Alexandre Belloni <alexandre.belloni@bootlin.com>
> > +
> > +description:
> > + The S-35390A is a CMOS 2-wire real-time clock IC which operates with the
> > + very low current consumption in the wide range of operation voltage.
> > +
> > +allOf:
> > + - $ref: rtc.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: sii,s35390a
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + sii,wakealarm-output-pin:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum: [1, 2]
> > + description: |
> > + The output pin to wake up the system.
> > + Default will use the output pin for interrupt signal 2.
> > + <S35390A_OUTPUT_PIN_INT1> : Output pin for interrupt signal 1
> > + <S35390A_OUTPUT_PIN_INT2> : Output pin for interrupt signal 2
> > +
>
> Ideally, we'd get a proper pinctrl driver part for this because what
> happens if you want interrupts on both pin or clock output on both pins
> or any combination of interrupts and clocks?
>
Yes.
Thanks
- Markus Probst
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: rtc: Add sii,wakealarm-output-pin property for S35390A
From: Markus Probst @ 2026-07-01 17:08 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Uwe Kleine-König, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-arm-kernel, linux-rtc, devicetree,
linux-kernel
In-Reply-To: <20260701164821b7492eac@mail.local>
[-- Attachment #1: Type: text/plain, Size: 2417 bytes --]
On Wed, 2026-07-01 at 18:48 +0200, Alexandre Belloni wrote:
> On 01/07/2026 16:43:07+0000, Markus Probst wrote:
> > On Wed, 2026-07-01 at 17:14 +0200, Krzysztof Kozlowski wrote:
> > > On 01/07/2026 15:25, Markus Probst wrote:
> > > > > > +
> > > > > > +maintainers:
> > > > > > + - Alexandre Belloni <alexandre.belloni@bootlin.com>
> > > > >
> > > > > This should be someone caring about this hardware.
> > > > He does have the majority of commits on this driver (excluding merge
> > > > commits and commits not exclusive to this driver), although most of
> > > > them are pretty tiny.
> > > >
> > > > Who would you suggest instead?
> > >
> > > Someone adding features for this driver, maybe driver maintainers. But
> > > if Alexandre is fine, you can leave him.
> > >
> > > > >
> > > > > > +
> > > > > > +description:
> > > > > > + The S-35390A is a CMOS 2-wire real-time clock IC which operates with the
> > > > > > + very low current consumption in the wide range of operation voltage.
> > > > > > +
> > > > > > +allOf:
> > > > > > + - $ref: rtc.yaml#
> > > > > > +
> > > > > > +properties:
> > > > > > + compatible:
> > > > > > + const: sii,s35390a
> > > > > > +
> > > > > > + reg:
> > > > > > + maxItems: 1
> > > > > > +
> > > > > > + sii,wakealarm-output-pin:
> > > > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > + enum: [1, 2]
> > > > > > + description: |
> > > > > > + The output pin to wake up the system.
> > > > > > + Default will use the output pin for interrupt signal 2.
> > > > > > + <S35390A_OUTPUT_PIN_INT1> : Output pin for interrupt signal 1
> > > > > > + <S35390A_OUTPUT_PIN_INT2> : Output pin for interrupt signal 2
> > > > >
> > > > > Does that mean device generates the interrupts?
> > > > Yes.
> > > >
> > >
> > >
> > > Then I think you miss interrupts property.
> > From what I can tell the line is used to generate a system wakeup
> > event.
> >
> > There would be no obvious benefit of connecting it to an interrupt
> > controller, so this property would be obsolete?
> >
>
> Then you need proper wakeup-source support
Wouldn't that break existing devicetrees?
The current driver allows to wake up the system, even without
having wakeup-source set.
As an example: arch/arm/boot/dts/marvell/kirkwood-ts219.dtsi
Thanks
- Markus Probst
>
>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: rtc: Add sii,wakealarm-output-pin property for S35390A
From: Alexandre Belloni @ 2026-07-01 16:48 UTC (permalink / raw)
To: Markus Probst
Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Uwe Kleine-König, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-arm-kernel, linux-rtc, devicetree,
linux-kernel
In-Reply-To: <d06dd0726aa3795ae99df5fa8a9c05d6e2001efd.camel@posteo.de>
On 01/07/2026 16:43:07+0000, Markus Probst wrote:
> On Wed, 2026-07-01 at 17:14 +0200, Krzysztof Kozlowski wrote:
> > On 01/07/2026 15:25, Markus Probst wrote:
> > > > > +
> > > > > +maintainers:
> > > > > + - Alexandre Belloni <alexandre.belloni@bootlin.com>
> > > >
> > > > This should be someone caring about this hardware.
> > > He does have the majority of commits on this driver (excluding merge
> > > commits and commits not exclusive to this driver), although most of
> > > them are pretty tiny.
> > >
> > > Who would you suggest instead?
> >
> > Someone adding features for this driver, maybe driver maintainers. But
> > if Alexandre is fine, you can leave him.
> >
> > > >
> > > > > +
> > > > > +description:
> > > > > + The S-35390A is a CMOS 2-wire real-time clock IC which operates with the
> > > > > + very low current consumption in the wide range of operation voltage.
> > > > > +
> > > > > +allOf:
> > > > > + - $ref: rtc.yaml#
> > > > > +
> > > > > +properties:
> > > > > + compatible:
> > > > > + const: sii,s35390a
> > > > > +
> > > > > + reg:
> > > > > + maxItems: 1
> > > > > +
> > > > > + sii,wakealarm-output-pin:
> > > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > > + enum: [1, 2]
> > > > > + description: |
> > > > > + The output pin to wake up the system.
> > > > > + Default will use the output pin for interrupt signal 2.
> > > > > + <S35390A_OUTPUT_PIN_INT1> : Output pin for interrupt signal 1
> > > > > + <S35390A_OUTPUT_PIN_INT2> : Output pin for interrupt signal 2
> > > >
> > > > Does that mean device generates the interrupts?
> > > Yes.
> > >
> >
> >
> > Then I think you miss interrupts property.
> From what I can tell the line is used to generate a system wakeup
> event.
>
> There would be no obvious benefit of connecting it to an interrupt
> controller, so this property would be obsolete?
>
Then you need proper wakeup-source support
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: rtc: Add sii,wakealarm-output-pin property for S35390A
From: Markus Probst @ 2026-07-01 16:43 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Uwe Kleine-König, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-arm-kernel, linux-rtc, devicetree,
linux-kernel
In-Reply-To: <7de66163-369e-4118-af51-6913b565fa4b@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1865 bytes --]
On Wed, 2026-07-01 at 17:14 +0200, Krzysztof Kozlowski wrote:
> On 01/07/2026 15:25, Markus Probst wrote:
> > > > +
> > > > +maintainers:
> > > > + - Alexandre Belloni <alexandre.belloni@bootlin.com>
> > >
> > > This should be someone caring about this hardware.
> > He does have the majority of commits on this driver (excluding merge
> > commits and commits not exclusive to this driver), although most of
> > them are pretty tiny.
> >
> > Who would you suggest instead?
>
> Someone adding features for this driver, maybe driver maintainers. But
> if Alexandre is fine, you can leave him.
>
> > >
> > > > +
> > > > +description:
> > > > + The S-35390A is a CMOS 2-wire real-time clock IC which operates with the
> > > > + very low current consumption in the wide range of operation voltage.
> > > > +
> > > > +allOf:
> > > > + - $ref: rtc.yaml#
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + const: sii,s35390a
> > > > +
> > > > + reg:
> > > > + maxItems: 1
> > > > +
> > > > + sii,wakealarm-output-pin:
> > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > + enum: [1, 2]
> > > > + description: |
> > > > + The output pin to wake up the system.
> > > > + Default will use the output pin for interrupt signal 2.
> > > > + <S35390A_OUTPUT_PIN_INT1> : Output pin for interrupt signal 1
> > > > + <S35390A_OUTPUT_PIN_INT2> : Output pin for interrupt signal 2
> > >
> > > Does that mean device generates the interrupts?
> > Yes.
> >
>
>
> Then I think you miss interrupts property.
From what I can tell the line is used to generate a system wakeup
event.
There would be no obvious benefit of connecting it to an interrupt
controller, so this property would be obsolete?
Thanks
- Markus Probst
>
> Best regards,
> Krzysztof
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply
* Re: [PATCH 1/8] dt-bindings: regulator: ROHM BD73800 regulators
From: Rob Herring (Arm) @ 2026-07-01 15:39 UTC (permalink / raw)
To: Matti Vaittinen
Cc: devicetree, linux-kernel, Mark Brown, Brian Masney,
Michael Turquette, linux-clk, Stephen Boyd, Bartosz Golaszewski,
Conor Dooley, Linus Walleij, linux-rtc, Krzysztof Kozlowski,
Alexandre Belloni, Matti Vaittinen, linux-gpio, Matti Vaittinen,
Lee Jones, Liam Girdwood
In-Reply-To: <67b42b5363533f11c22a6421417c3345f9872aec.1782909323.git.mazziesaccount@gmail.com>
On Wed, 01 Jul 2026 15:41:11 +0300, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> Add bindings for the BUCKs and LDOs on ROHM BD73800. The PMIC state
> specific voltages can be set in same fashion as with a few other ROHM
> PMICs (for example with BD718[15,28,37,47,50,79]). Same properties are
> recycled :)
>
> The LDOs 1 and 4 can use different voltage ranges depending on the OTP
> configuration.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
> .../regulator/rohm,bd73800-regulator.yaml | 119 ++++++++++++++++++
> 1 file changed, 119 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
Warning: Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml references a file that doesn't exist: Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml
Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml: Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml
See https://patchwork.kernel.org/project/devicetree/patch/67b42b5363533f11c22a6421417c3345f9872aec.1782909323.git.mazziesaccount@gmail.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: rtc: Add sii,wakealarm-output-pin property for S35390A
From: Krzysztof Kozlowski @ 2026-07-01 15:14 UTC (permalink / raw)
To: Markus Probst
Cc: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Uwe Kleine-König, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-arm-kernel, linux-rtc, devicetree,
linux-kernel
In-Reply-To: <45e8157be53c3d8827fcccece7f706968bc056d3.camel@posteo.de>
On 01/07/2026 15:25, Markus Probst wrote:
>>> +
>>> +maintainers:
>>> + - Alexandre Belloni <alexandre.belloni@bootlin.com>
>>
>> This should be someone caring about this hardware.
> He does have the majority of commits on this driver (excluding merge
> commits and commits not exclusive to this driver), although most of
> them are pretty tiny.
>
> Who would you suggest instead?
Someone adding features for this driver, maybe driver maintainers. But
if Alexandre is fine, you can leave him.
>>
>>> +
>>> +description:
>>> + The S-35390A is a CMOS 2-wire real-time clock IC which operates with the
>>> + very low current consumption in the wide range of operation voltage.
>>> +
>>> +allOf:
>>> + - $ref: rtc.yaml#
>>> +
>>> +properties:
>>> + compatible:
>>> + const: sii,s35390a
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + sii,wakealarm-output-pin:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + enum: [1, 2]
>>> + description: |
>>> + The output pin to wake up the system.
>>> + Default will use the output pin for interrupt signal 2.
>>> + <S35390A_OUTPUT_PIN_INT1> : Output pin for interrupt signal 1
>>> + <S35390A_OUTPUT_PIN_INT2> : Output pin for interrupt signal 2
>>
>> Does that mean device generates the interrupts?
> Yes.
>
Then I think you miss interrupts property.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: rtc: Add sii,wakealarm-output-pin property for S35390A
From: Alexandre Belloni @ 2026-07-01 15:11 UTC (permalink / raw)
To: Markus Probst
Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Uwe Kleine-König, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-arm-kernel, linux-rtc, devicetree,
linux-kernel
In-Reply-To: <45e8157be53c3d8827fcccece7f706968bc056d3.camel@posteo.de>
On 01/07/2026 13:25:41+0000, Markus Probst wrote:
> On Wed, 2026-07-01 at 09:35 +0200, Krzysztof Kozlowski wrote:
> > On Tue, Jun 30, 2026 at 07:22:21PM +0000, Markus Probst wrote:
> > > Synology NAS devices use the output pin for interrupt signal 1 to wake up
> > > the system.
> > >
> > > Move devicetree bindings for sii,s35390a into its own file.
> > > Add sii,wakealarm-output-pin property to enable the use of the output
> > > pin for interrupt signal 1 for the wake alarm, which makes it possible to
> > > set an wake alarm on Synology NAS devices.
> > >
> > > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > > ---
> > > .../devicetree/bindings/rtc/sii,s35390a.yaml | 54 ++++++++++++++++++++++
> > > .../devicetree/bindings/rtc/trivial-rtc.yaml | 3 --
> > > MAINTAINERS | 1 +
> > > include/dt-bindings/rtc/s35390a.h | 9 ++++
> > > 4 files changed, 64 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml b/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml
> > > new file mode 100644
> > > index 000000000000..31a578673870
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml
> > > @@ -0,0 +1,54 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/rtc/sii,s35390a.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: S-35390A 2-WIRE REAL-TIME CLOCK
> > > +
> > > +maintainers:
> > > + - Alexandre Belloni <alexandre.belloni@bootlin.com>
> >
> > This should be someone caring about this hardware.
> He does have the majority of commits on this driver (excluding merge
> commits and commits not exclusive to this driver), although most of
> them are pretty tiny.
>
> Who would you suggest instead?
I can take it but the point of Krzysztof is mainly that the ones working
on the driver don't necessarily have to be the DT bindings maintainers
as both are well separated.
I mostly did clean ups in the driver, Lorenz Brun submitted something
way more interesting.
> >
> > > +
> > > +description:
> > > + The S-35390A is a CMOS 2-wire real-time clock IC which operates with the
> > > + very low current consumption in the wide range of operation voltage.
> > > +
> > > +allOf:
> > > + - $ref: rtc.yaml#
> > > +
> > > +properties:
> > > + compatible:
> > > + const: sii,s35390a
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + sii,wakealarm-output-pin:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + enum: [1, 2]
> > > + description: |
> > > + The output pin to wake up the system.
> > > + Default will use the output pin for interrupt signal 2.
> > > + <S35390A_OUTPUT_PIN_INT1> : Output pin for interrupt signal 1
> > > + <S35390A_OUTPUT_PIN_INT2> : Output pin for interrupt signal 2
> >
> > Does that mean device generates the interrupts?
> Yes.
>
> Thanks
> - Markus Probst
>
> >
> > Best regards,
> > Krzysztof
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: rtc: Add sii,wakealarm-output-pin property for S35390A
From: Alexandre Belloni @ 2026-07-01 15:06 UTC (permalink / raw)
To: Markus Probst
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Uwe Kleine-König, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-arm-kernel, linux-rtc, devicetree,
linux-kernel
In-Reply-To: <20260630-rtc_s35390a_int1-v1-1-1b2239e16be2@posteo.de>
On 30/06/2026 19:22:21+0000, Markus Probst wrote:
> Synology NAS devices use the output pin for interrupt signal 1 to wake up
> the system.
>
> Move devicetree bindings for sii,s35390a into its own file.
> Add sii,wakealarm-output-pin property to enable the use of the output
> pin for interrupt signal 1 for the wake alarm, which makes it possible to
> set an wake alarm on Synology NAS devices.
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> .../devicetree/bindings/rtc/sii,s35390a.yaml | 54 ++++++++++++++++++++++
> .../devicetree/bindings/rtc/trivial-rtc.yaml | 3 --
> MAINTAINERS | 1 +
> include/dt-bindings/rtc/s35390a.h | 9 ++++
> 4 files changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml b/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml
> new file mode 100644
> index 000000000000..31a578673870
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/sii,s35390a.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: S-35390A 2-WIRE REAL-TIME CLOCK
> +
> +maintainers:
> + - Alexandre Belloni <alexandre.belloni@bootlin.com>
> +
> +description:
> + The S-35390A is a CMOS 2-wire real-time clock IC which operates with the
> + very low current consumption in the wide range of operation voltage.
> +
> +allOf:
> + - $ref: rtc.yaml#
> +
> +properties:
> + compatible:
> + const: sii,s35390a
> +
> + reg:
> + maxItems: 1
> +
> + sii,wakealarm-output-pin:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [1, 2]
> + description: |
> + The output pin to wake up the system.
> + Default will use the output pin for interrupt signal 2.
> + <S35390A_OUTPUT_PIN_INT1> : Output pin for interrupt signal 1
> + <S35390A_OUTPUT_PIN_INT2> : Output pin for interrupt signal 2
> +
Ideally, we'd get a proper pinctrl driver part for this because what
happens if you want interrupts on both pin or clock output on both pins
or any combination of interrupts and clocks?
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v2 10/10] rtc: rzn1: Add support for Renesas RZ/T2H and RZ/N2H SoCs
From: sashiko-bot @ 2026-07-01 14:54 UTC (permalink / raw)
To: Prabhakar
Cc: robh, wsa+renesas, linux-rtc, conor+dt, devicetree,
Alexandre Belloni
In-Reply-To: <20260701142953.2014895-11-prabhakar.mahadev-lad.rj@bp.renesas.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The driver does not enforce SCMP mode for the new RZ/T2H compatible, allowing an unsafe fallback to SUBU mode that accesses the unimplemented RTCA0SUBU register.
- [Medium] The driver clears the entire `RZN1_RTC_CTL1` register to disable interrupts, which inadvertently destroys the state of the RZ/T2H-specific 1 Hz output control bit.
--
commit a19c5bdab6f846898749c84cf9743ad18017cdd8
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
rtc: rzn1: Add support for Renesas RZ/T2H and RZ/N2H SoCs
Add support for the RTC block found on the Renesas RZ/T2H (R9A09G077)
and RZ/N2H (R9A09G087) SoCs by matching the new
renesas,r9a09g077-rtc compatible string.
> These SoCs integrate a closely related variant of the RZ/N1 RTC IP.
> Unlike the RZ/N1 implementation, the RZ/T2H and RZ/N2H variants lack
> the RTCA0SUBU register. However, this register is not accessed when
> operating in SCMP mode using the existing rzn1_rtc_ops_scmp
> configuration, allowing the current driver infrastructure to be reused
> without modification.
[Severity: High]
Does the current driver infrastructure guarantee SCMP mode is always used?
If the xtal clock is missing (since devm_clk_get_optional() is used) or
if its rate is exactly 32768 Hz, scmp_val remains 0:
drivers/rtc/rtc-rzn1.c:rzn1_rtc_probe() {
...
xtal = devm_clk_get_optional(dev, "xtal");
if (IS_ERR(xtal)) {
...
} else if (xtal) {
rate = clk_get_rate(xtal);
...
if (rate != 32768)
scmp_val = RZN1_RTC_CTL0_SLSB_SCMP;
}
...
This causes the driver to fall back to the SUBU ops:
if (scmp_val) {
writel(rate - 1, rtc->base + RZN1_RTC_SCMP);
rtc->rtcdev->ops = &rzn1_rtc_ops_scmp;
} else {
rtc->rtcdev->ops = &rzn1_rtc_ops_subu;
}
If this happens, the SUBU ops will access the missing RTCA0SUBU register,
which can trigger a synchronous external abort. Should the driver use OF match
data to identify variants lacking the SUBU register and explicitly prevent
assigning rzn1_rtc_ops_subu?
> The RZ/T2H RTC variant also supports a 1 Hz output signal on the
> RTCAT1HZ pin, controlled by the RTCA0CTL1[RTCA01HZE] bit. This bit is
> marked as reserved in the RZ/N1 hardware manual, making RZ/T2H a
> distinct RTC variant despite its overall compatibility with the RZ/N1
> implementation.
[Severity: Medium]
Will this 1 Hz output signal be inadvertently disabled during probe and removal?
During probe and removal, the driver unconditionally clears the entire
RZN1_RTC_CTL1 register to disable interrupts:
drivers/rtc/rtc-rzn1.c:rzn1_rtc_probe() {
...
/* Disable all interrupts */
writel(0, rtc->base + RZN1_RTC_CTL1);
...
If the bootloader or firmware previously configured the 1 Hz output signal,
won't this unconditional write to 0 destroy that state? Could the driver
perform a read-modify-write operation to clear only the interrupt enable bits
instead?
> diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> index e77b7e7bcd5bd..26a5cde41c6b2 100644
> --- a/drivers/rtc/rtc-rzn1.c
> +++ b/drivers/rtc/rtc-rzn1.c
> @@ -518,6 +518,7 @@ static void rzn1_rtc_remove(struct platform_device *pdev)
> }
>
> static const struct of_device_id rzn1_rtc_of_match[] = {
> + { .compatible = "renesas,r9a09g077-rtc" },
> { .compatible = "renesas,rzn1-rtc" },
> {},
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701142953.2014895-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=10
^ permalink raw reply
* Re: [PATCH v2 08/10] rtc: rzn1: Consistently use dev_err_probe()
From: sashiko-bot @ 2026-07-01 14:50 UTC (permalink / raw)
To: Prabhakar
Cc: wsa+renesas, linux-rtc, devicetree, conor+dt, robh,
Alexandre Belloni
In-Reply-To: <20260701142953.2014895-9-prabhakar.mahadev-lad.rj@bp.renesas.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Premature pm_runtime_put() in remove callback leads to a potential bus fault or kernel panic due to devres teardown ordering.
--
commit 20388d9c3bb9eb8111711742d828f2997e736384
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
rtc: rzn1: Consistently use dev_err_probe()
This patch updates rzn1_rtc_probe() to use dev_err_probe() when the alarm
interrupt request fails. This brings the error handling into alignment with
the rest of the probe function.
> diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> index 244d0fc70fdad..d990075839c13 100644
> --- a/drivers/rtc/rtc-rzn1.c
> +++ b/drivers/rtc/rtc-rzn1.c
> @@ -471,7 +471,7 @@ static int rzn1_rtc_probe(struct platform_device *pdev)
>
> ret = devm_request_irq(dev, irq, rzn1_rtc_alarm_irq, 0, "RZN1 RTC Alarm", rtc);
> if (ret) {
> - dev_err(dev, "RTC alarm interrupt not available\n");
> + dev_err_probe(dev, ret, "RTC alarm interrupt not available\n");
> goto dis_runtime_pm;
> }
[Severity: High]
This isn't a bug introduced by this patch, but since we are looking at driver
error handling and teardown paths, is there a potential use-after-suspend
issue during device removal?
Since the RTC is registered using devm_rtc_register_device() in
rzn1_rtc_probe(), it remains registered and accessible to user space until
the devres teardown runs, which happens after rzn1_rtc_remove() returns.
However, rzn1_rtc_remove() manually drops the runtime PM reference:
drivers/rtc/rtc-rzn1.c:rzn1_rtc_remove() {
...
/* Disable all interrupts */
writel(0, rtc->base + RZN1_RTC_CTL1);
pm_runtime_put(&pdev->dev);
}
If a user-space process accesses the RTC device (like reading the time via
ioctl) precisely during driver unbinding after pm_runtime_put() is called,
would this access hardware registers while the clock is gated and trigger
a kernel panic?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701142953.2014895-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=8
^ permalink raw reply
* Re: [PATCH v2 06/10] rtc: rzn1: Dynamically calculate synchronization delay based on clock rate
From: sashiko-bot @ 2026-07-01 14:49 UTC (permalink / raw)
To: Prabhakar
Cc: linux-rtc, conor+dt, Alexandre Belloni, robh, devicetree,
wsa+renesas
In-Reply-To: <20260701142953.2014895-7-prabhakar.mahadev-lad.rj@bp.renesas.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] Incorrect time conversion macro NSEC_PER_MSEC used instead of USEC_PER_SEC for microsecond calculation.
Pre-existing issues:
- [High] Race condition between PM runtime teardown and devres device unregistration can lead to accessing powered-down hardware.
--
commit f3a6e4b28e451318639b10e7927bf136a7d225d3
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
rtc: rzn1: Dynamically calculate synchronization delay based on clock rate
This patch replaces hardcoded hardware synchronization delays with a
calculated time window derived from the runtime clock rate. It stores this
synchronization time in the driver private structure to dynamically adjust
wait boundaries.
> diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> index 9969bcefede72..583d174954043 100644
> --- a/drivers/rtc/rtc-rzn1.c
> +++ b/drivers/rtc/rtc-rzn1.c
[ ... ]
> @@ -432,12 +433,20 @@ static int rzn1_rtc_probe(struct platform_device *pdev)
> scmp_val = RZN1_RTC_CTL0_SLSB_SCMP;
> }
>
> + /*
> + * The internal clock counter operates in synchronization with the
> + * RTC_PCLK clock. Calculate the duration of two RTC_PCLK clock
> + * cycles in microseconds required for operations to complete.
> + */
> + rtc->sync_time = DIV_ROUND_UP(2 * NSEC_PER_MSEC, rate);
[Severity: Low]
This isn't a bug, but should this macro be USEC_PER_SEC instead?
The comment mentions calculating the duration in microseconds. While
NSEC_PER_MSEC evaluates to 1,000,000 which produces the correct runtime
result for converting cycles per second into microseconds, using
USEC_PER_SEC would more accurately reflect the intended units and improve
readability.
[Severity: High]
This is a pre-existing issue, but is there a race condition between the
PM runtime teardown and devres cleanup?
In drivers/rtc/rtc-rzn1.c, the RTC device is registered via devres
during rzn1_rtc_probe():
ret = devm_rtc_register_device(rtc->rtcdev);
Devres runs its cleanup (which calls rtc_unregister_device()) strictly
after the driver's remove callback returns. However, in rzn1_rtc_remove(),
the driver manually drops the PM runtime reference:
pm_runtime_put(&pdev->dev);
This powers down the hardware while the RTC character device is still
registered and exposed to userspace. If a userspace process calls an ioctl,
such as RTC_RD_TIME, during this window, the driver's rzn1_rtc_read_time()
callback executes and attempts to access unclocked registers, which could
lead to a synchronous external abort or bus fault.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701142953.2014895-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=6
^ permalink raw reply
* Re: [PATCH v2 09/10] rtc: rzn1: use FIELD_PREP/FIELD_GET and GENMASK for register access
From: sashiko-bot @ 2026-07-01 14:48 UTC (permalink / raw)
To: Prabhakar
Cc: linux-rtc, conor+dt, robh, wsa+renesas, devicetree,
Alexandre Belloni
In-Reply-To: <20260701142953.2014895-10-prabhakar.mahadev-lad.rj@bp.renesas.com>
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Replace open-coded shift and mask operations with the bitfield API.
>
> Note that the weekday field is changed from an explicit 0x0f mask to
> an 8-bit field definition, matching the hardware manual. This does not
> change behaviour, as valid weekday values cannot exceed 7.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701142953.2014895-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=9
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox