* Re: [PATCH v3 8/8] clk: sunxi-ng: sun6i-rtc: add a733 support
From: sashiko-bot @ 2026-07-02 8:29 UTC (permalink / raw)
To: Jerome Brunet
Cc: conor+dt, robh, linux-sunxi, Alexandre Belloni, linux-rtc,
devicetree
In-Reply-To: <20260702-a733-rtc-v3-8-eb2580374de6@baylibre.com>
> Add support for the sun60i a733 CCU RTC.
>
> Compared to the a523, this SoC has a different input oscillator divider
> which auto-detects the oscillator rate and select a divider to provide
> a fixed 32768Hz clock. It also provides several phy reference clocks
> with dedicated clock gates.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-a733-rtc-v3-0-eb2580374de6@baylibre.com?part=8
^ permalink raw reply
* Re: [PATCH v3 6/8] clk: sunxi-ng: div: add read-only operation support
From: sashiko-bot @ 2026-07-02 8:26 UTC (permalink / raw)
To: Jerome Brunet
Cc: linux-rtc, Alexandre Belloni, conor+dt, linux-sunxi, devicetree,
robh
In-Reply-To: <20260702-a733-rtc-v3-6-eb2580374de6@baylibre.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The existing ccu_div_determine_rate_helper incorrectly asks the hardware parent for a non-predivided rate when CLK_SET_RATE_PARENT is set, resulting in unvalidated parent rate requests.
--
commit 20e3c89e1fcae37e3eaf3a9be6d0c4e28094d07d
Author: Jerome Brunet <jbrunet@baylibre.com>
clk: sunxi-ng: div: add read-only operation support
This commit adds support for sunxi-ng read-only dividers to be used
by the a733 oscillator detection logic.
> diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c
> index 62d680ccb524b..d1c8c7baa12dc 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);
> }
[Severity: High]
This is a pre-existing issue, but does ccu_div_determine_rate_helper
incorrectly ask the hardware parent for a non-predivided rate when
CLK_SET_RATE_PARENT is set?
When ccu_div_determine_rate is called on a clock with a pre-divider, it
delegates to ccu_mux_helper_determine_rate, which calls
ccu_div_determine_rate_helper. That helper then directly invokes
divider_determine_rate() which queries req->best_parent_hw for
req->rate * div.
Could this cause the clock core to query the upstream parent for a target
rate without accounting for the pre-divider? If the core subsequently sets
the parent rate to an unvalidated higher rate, it seems the parent PLL may
fail or clamp because it was asked for an achievable rate that it cannot
actually sustain when multiplied by the pre-divider.
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-a733-rtc-v3-0-eb2580374de6@baylibre.com?part=6
^ permalink raw reply
* Re: [PATCH 2/8] dt-bindings: mfd: ROHM BD73800 PMIC
From: Matti Vaittinen @ 2026-07-02 8:25 UTC (permalink / raw)
To: Krzysztof Kozlowski
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: <20260702-refreshing-terrific-tarsier-f56ada@quoll>
Hi dee Ho Krzysztof!
Thanks for the review (again)!
On 02/07/2026 11:05, Krzysztof Kozlowski wrote:
> 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
I suppose this will mean compatible is one of the enum values AND
rohm,bd73800 as a fallback, while accepting also the rohm,bd73800 alone.
(No need to reply, I will test it out). If so, then I'll do this for the
next version (which is probably out only at August).
>
>> + - 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.
This comment sounds like someone is crafting a tool which can convert
the bindings to human-readable spec? For someone as yaml-illiterate as
me, this really brings some hope :) Although, I would hope we saw some
"comment-key" which could be used to mark the comment to be included in
said human-readable format. Stuffing it in the 1.st description is Ok,
but still somewhat sub-optimal. Oh well, "Ok" will do - I'll add this to
the description when re-spinning.
I do appreciate your help and feedback (as always!).
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 v3 7/8] clk: sunxi-ng: sun6i-rtc: split main oscillator div and gate.
From: sashiko-bot @ 2026-07-02 8:23 UTC (permalink / raw)
To: Jerome Brunet
Cc: linux-rtc, devicetree, robh, linux-sunxi, Alexandre Belloni,
conor+dt
In-Reply-To: <20260702-a733-rtc-v3-7-eb2580374de6@baylibre.com>
> On the a733 the "osc24M-32k" clock has the same gate bits as the previously
> supported SoC but a different divider implementation.
>
> Instead of a fixed 750 divider, the divider is selected based on the
> rate of the oscillator. It can be seen as a simple read-only divider.
>
> To easily replace the divider part depending the SoC, split the divider
> and gate into two separate clock entities.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-a733-rtc-v3-0-eb2580374de6@baylibre.com?part=7
^ permalink raw reply
* Re: [PATCH v3 4/8] clk: sunxi-ng: sun6i-rtc: clean up DT usage
From: sashiko-bot @ 2026-07-02 8:21 UTC (permalink / raw)
To: Jerome Brunet
Cc: Alexandre Belloni, devicetree, linux-sunxi, robh, linux-rtc,
conor+dt
In-Reply-To: <20260702-a733-rtc-v3-4-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>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-a733-rtc-v3-0-eb2580374de6@baylibre.com?part=4
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: rtc: Add sii,wakealarm-output-pin property for S35390A
From: Krzysztof Kozlowski @ 2026-07-02 8:20 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Markus Probst, 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: <202607020817017cde494f@mail.local>
On 02/07/2026 10:17, Alexandre Belloni wrote:
> On 02/07/2026 08:09:55+0200, Krzysztof Kozlowski wrote:
>>>> 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?
>>
>
> Some systems are routing the interrupt output of the RTC directly to the
> PMIC and the PMIC is the one responsible for waking the rest of the
> system, including the SoC.
Yeah, then the PMIC has interrupt line to the SoC and the PMIC is the
wakeup-source, not RTC.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 5/8] clk: sunxi-ng: sun6i-rtc: Add feature bit for IOSC calibration
From: sashiko-bot @ 2026-07-02 8:19 UTC (permalink / raw)
To: Jerome Brunet
Cc: linux-sunxi, robh, conor+dt, Alexandre Belloni, devicetree,
linux-rtc
In-Reply-To: <20260702-a733-rtc-v3-5-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>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-a733-rtc-v3-0-eb2580374de6@baylibre.com?part=5
^ permalink raw reply
* Re: [PATCH v3 1/8] dt-bindings: rtc: sun6i: no clock-output-names on h616/r329
From: sashiko-bot @ 2026-07-02 8:17 UTC (permalink / raw)
To: Jerome Brunet
Cc: Alexandre Belloni, devicetree, linux-sunxi, linux-rtc, conor+dt,
robh
In-Reply-To: <20260702-a733-rtc-v3-1-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>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-a733-rtc-v3-0-eb2580374de6@baylibre.com?part=1
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: rtc: Add sii,wakealarm-output-pin property for S35390A
From: Alexandre Belloni @ 2026-07-02 8:17 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Markus Probst, 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: <187099d0-5e09-49c2-b67d-8bd4366ff1a0@kernel.org>
On 02/07/2026 08:09:55+0200, Krzysztof Kozlowski wrote:
> >> 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?
>
Some systems are routing the interrupt output of the RTC directly to the
PMIC and the PMIC is the one responsible for waking the rest of the
system, including the SoC.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v3 2/8] dt-bindings: rtc: sun6i: add sun60i-a733 support
From: sashiko-bot @ 2026-07-02 8:16 UTC (permalink / raw)
To: Jerome Brunet
Cc: linux-rtc, Alexandre Belloni, conor+dt, devicetree, linux-sunxi,
robh
In-Reply-To: <20260702-a733-rtc-v3-2-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>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-a733-rtc-v3-0-eb2580374de6@baylibre.com?part=2
^ permalink raw reply
* Re: [PATCH v3 3/8] clk: sunxi-ng: fix ccu probe clock unregister on error
From: sashiko-bot @ 2026-07-02 8:16 UTC (permalink / raw)
To: Jerome Brunet
Cc: robh, linux-sunxi, conor+dt, devicetree, linux-rtc,
Alexandre Belloni
In-Reply-To: <20260702-a733-rtc-v3-3-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>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-a733-rtc-v3-0-eb2580374de6@baylibre.com?part=3
^ permalink raw reply
* [PATCH v3 8/8] clk: sunxi-ng: sun6i-rtc: add 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 support for the sun60i a733 CCU RTC.
Compared to the a523, this SoC has a different input oscillator divider
which auto-detects the oscillator rate and select a divider to provide
a fixed 32768Hz clock. It also provides several phy reference clocks
with dedicated clock gates.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/clk/sunxi-ng/ccu-sun6i-rtc.c | 73 ++++++++++++++++++++++++++++++++++--
drivers/clk/sunxi-ng/ccu-sun6i-rtc.h | 2 +-
2 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
index 25dd87e78eb7..6b71bbd80255 100644
--- a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
+++ b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
@@ -44,9 +44,13 @@
#define DCXO_CTRL_REG 0x160
#define DCXO_CTRL_CLK16M_RC_EN BIT(0)
+#define DCXO_GATING_REG 0x16c
+
struct sun6i_rtc_match_data {
bool have_ext_osc32k : 1;
bool have_iosc_calibration : 1;
+ bool have_dcxo_status : 1;
+ bool have_phy_ref_gates : 1;
bool rtc_32k_single_parent : 1;
const struct clk_parent_data *osc32k_fanout_parents;
u8 osc32k_fanout_nparents;
@@ -213,7 +217,12 @@ static struct ccu_mux osc32k_clk = {
},
};
-/* This falls back to the global name for fwnodes without a named reference. */
+/*
+ * This falls back to the global name for fwnodes without a named reference.
+ * NOTE: osc24M name might be misleading the oscillator could also be a 26MHz
+ * or a 19.2MHz one starting with the a733. The original name is kept anyway
+ * in case anything is relying on it.
+ */
static const struct clk_parent_data osc24M[] = {
{ .fw_name = "hosc", .name = "osc24M" }
};
@@ -227,8 +236,28 @@ static struct clk_fixed_factor osc24M_32k_div_clk = {
0),
};
-static SUNXI_CCU_GATE_HW(osc24M_32k_clk, "osc24M-32k", &osc24M_32k_div_clk.hw,
- LOSC_OUT_GATING_REG, BIT(16), 0);
+static struct clk_div_table osc24M_32k_div_a733_table[] = {
+ { .val = 0, .div = 732 },
+ { .val = 1, .div = 586 },
+ { .val = 2, .div = 793 },
+ { .val = 3, .div = 732 },
+ { /* Sentinel */ },
+};
+
+static struct ccu_div osc24M_32k_div_a733_clk = {
+ .enable = BIT(1),
+ .div = _SUNXI_CCU_DIV_TABLE(14, 2, osc24M_32k_div_a733_table),
+ .common = {
+ .reg = DCXO_CTRL_REG,
+ .hw.init = CLK_HW_INIT_PARENTS_DATA("osc24M-32k-div",
+ osc24M,
+ &ccu_rodiv_ops,
+ 0),
+ },
+};
+
+static SUNXI_CCU_GATE(osc24M_32k_clk, "osc24M-32k", "osc24M-32k-div",
+ LOSC_OUT_GATING_REG, BIT(16), 0);
static const struct clk_hw *rtc_32k_parents[] = {
&osc32k_clk.common.hw,
@@ -267,6 +296,15 @@ static struct ccu_mux osc32k_fanout_clk = {
},
};
+static SUNXI_CCU_GATE_FW(hosc_serdes1_clk, "hosc-serdes1", "hosc",
+ DCXO_GATING_REG, BIT(5), 0);
+static SUNXI_CCU_GATE_FW(hosc_serdes0_clk, "hosc-serdes0", "hosc",
+ DCXO_GATING_REG, BIT(4), 0);
+static SUNXI_CCU_GATE_FW(hosc_hdmi_clk, "hosc-hdmi", "hosc",
+ DCXO_GATING_REG, BIT(1), 0);
+static SUNXI_CCU_GATE_FW(hosc_ufs_clk, "hosc-ufs", "hosc",
+ DCXO_GATING_REG, BIT(0), 0);
+
static struct ccu_common *sun6i_rtc_ccu_clks[] = {
&iosc_clk,
&iosc_32k_clk,
@@ -275,6 +313,11 @@ static struct ccu_common *sun6i_rtc_ccu_clks[] = {
&osc24M_32k_clk.common,
&rtc_32k_clk.common,
&osc32k_fanout_clk.common,
+ &osc24M_32k_div_a733_clk.common,
+ &hosc_serdes1_clk.common,
+ &hosc_serdes0_clk.common,
+ &hosc_hdmi_clk.common,
+ &hosc_ufs_clk.common,
};
static struct clk_hw_onecell_data sun6i_rtc_ccu_hw_clks = {
@@ -288,6 +331,10 @@ static struct clk_hw_onecell_data sun6i_rtc_ccu_hw_clks = {
[CLK_OSC24M_32K] = &osc24M_32k_clk.common.hw,
[CLK_RTC_32K] = &rtc_32k_clk.common.hw,
[CLK_OSC24M_32K_DIV] = &osc24M_32k_div_clk.hw,
+ [CLK_HOSC_UFS] = &hosc_ufs_clk.common.hw,
+ [CLK_HOSC_HDMI] = &hosc_hdmi_clk.common.hw,
+ [CLK_HOSC_SERDES0] = &hosc_serdes0_clk.common.hw,
+ [CLK_HOSC_SERDES1] = &hosc_serdes1_clk.common.hw,
},
};
@@ -330,6 +377,15 @@ static const struct sun6i_rtc_match_data sun55i_a523_rtc_ccu_data = {
.osc32k_fanout_nparents = ARRAY_SIZE(sun50i_r329_osc32k_fanout_parents),
};
+static const struct sun6i_rtc_match_data sun60i_a733_rtc_ccu_data = {
+ .have_ext_osc32k = true,
+ .have_iosc_calibration = true,
+ .have_dcxo_status = true,
+ .have_phy_ref_gates = true,
+ .osc32k_fanout_parents = sun50i_r329_osc32k_fanout_parents,
+ .osc32k_fanout_nparents = ARRAY_SIZE(sun50i_r329_osc32k_fanout_parents),
+};
+
static const struct of_device_id sun6i_rtc_ccu_match[] = {
{
.compatible = "allwinner,sun50i-h616-rtc",
@@ -343,6 +399,10 @@ static const struct of_device_id sun6i_rtc_ccu_match[] = {
.compatible = "allwinner,sun55i-a523-rtc",
.data = &sun55i_a523_rtc_ccu_data,
},
+ {
+ .compatible = "allwinner,sun60i-a733-rtc",
+ .data = &sun60i_a733_rtc_ccu_data,
+ },
{},
};
MODULE_DEVICE_TABLE(of, sun6i_rtc_ccu_match);
@@ -375,6 +435,13 @@ int sun6i_rtc_ccu_probe(struct device *dev, void __iomem *reg)
osc32k_fanout_init_data.parent_data = data->osc32k_fanout_parents;
osc32k_fanout_init_data.num_parents = data->osc32k_fanout_nparents;
+ if (data->have_dcxo_status)
+ sun6i_rtc_ccu_hw_clks.hws[CLK_OSC24M_32K_DIV] =
+ &osc24M_32k_div_a733_clk.common.hw;
+
+ if (!data->have_phy_ref_gates)
+ sun6i_rtc_ccu_hw_clks.num = CLK_OSC24M_32K_DIV + 1;
+
return devm_sunxi_ccu_probe(dev, reg, &sun6i_rtc_ccu_desc);
}
diff --git a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.h b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.h
index ab7b92b47f59..4f4f4cb00f1d 100644
--- a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.h
+++ b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.h
@@ -11,6 +11,6 @@
#define CLK_RTC_32K 6
#define CLK_OSC24M_32K_DIV 7
-#define CLK_NUMBER (CLK_OSC24M_32K_DIV + 1)
+#define CLK_NUMBER (CLK_HOSC_SERDES1 + 1)
#endif /* _CCU_SUN6I_RTC_H */
--
2.47.3
^ permalink raw reply related
* [PATCH v3 7/8] clk: sunxi-ng: sun6i-rtc: split main oscillator div and gate.
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>
On the a733 the "osc24M-32k" clock has the same gate bits as the previously
supported SoC but a different divider implementation.
Instead of a fixed 750 divider, the divider is selected based on the
rate of the oscillator. It can be seen as a simple read-only divider.
To easily replace the divider part depending the SoC, split the divider
and gate into two separate clock entities.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/clk/sunxi-ng/ccu-sun6i-rtc.c | 20 +++++++++++---------
drivers/clk/sunxi-ng/ccu-sun6i-rtc.h | 3 ++-
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
index b24c8b196e66..25dd87e78eb7 100644
--- a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
+++ b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
@@ -218,17 +218,18 @@ static const struct clk_parent_data osc24M[] = {
{ .fw_name = "hosc", .name = "osc24M" }
};
-static struct ccu_gate osc24M_32k_clk = {
- .enable = BIT(16),
- .common = {
- .reg = LOSC_OUT_GATING_REG,
- .prediv = 750,
- .features = CCU_FEATURE_ALL_PREDIV,
- .hw.init = CLK_HW_INIT_PARENTS_DATA("osc24M-32k", osc24M,
- &ccu_gate_ops, 0),
- },
+static struct clk_fixed_factor osc24M_32k_div_clk = {
+ .mult = 1,
+ .div = 750,
+ .hw.init = CLK_HW_INIT_PARENTS_DATA("osc24M-32k-div",
+ osc24M,
+ &clk_fixed_factor_ops,
+ 0),
};
+static SUNXI_CCU_GATE_HW(osc24M_32k_clk, "osc24M-32k", &osc24M_32k_div_clk.hw,
+ LOSC_OUT_GATING_REG, BIT(16), 0);
+
static const struct clk_hw *rtc_32k_parents[] = {
&osc32k_clk.common.hw,
&osc24M_32k_clk.common.hw
@@ -286,6 +287,7 @@ static struct clk_hw_onecell_data sun6i_rtc_ccu_hw_clks = {
[CLK_EXT_OSC32K_GATE] = &ext_osc32k_gate_clk.common.hw,
[CLK_OSC24M_32K] = &osc24M_32k_clk.common.hw,
[CLK_RTC_32K] = &rtc_32k_clk.common.hw,
+ [CLK_OSC24M_32K_DIV] = &osc24M_32k_div_clk.hw,
},
};
diff --git a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.h b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.h
index 9ae821fc2599..ab7b92b47f59 100644
--- a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.h
+++ b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.h
@@ -9,7 +9,8 @@
#define CLK_EXT_OSC32K_GATE 4
#define CLK_OSC24M_32K 5
#define CLK_RTC_32K 6
+#define CLK_OSC24M_32K_DIV 7
-#define CLK_NUMBER (CLK_RTC_32K + 1)
+#define CLK_NUMBER (CLK_OSC24M_32K_DIV + 1)
#endif /* _CCU_SUN6I_RTC_H */
--
2.47.3
^ permalink raw reply related
* [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
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