ARM Sunxi Platform Development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jerome Brunet" <jbrunet@baylibre.com>
Cc: linux-sunxi@lists.linux.dev, conor+dt@kernel.org,
	robh@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v2 5/6] clk: sunxi-ng: sun6i-rtc: split main oscillator div and gate.
Date: Mon, 29 Jun 2026 13:12:54 +0000	[thread overview]
Message-ID: <20260629131254.7E34C1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260629-a733-rtc-v2-5-7b72112784f8@baylibre.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the error path in `sunxi_ccu_probe` unregisters clocks using the wrong loop limit, causing either memory leaks or out-of-bounds array reads depending on the struct array sizes of the specific CCU driver.
--

commit fe4850822f9e2e522aaac703e02ecb786fb47f26
Author: Jerome Brunet <jbrunet@baylibre.com>

clk: sunxi-ng: sun6i-rtc: split main oscillator div and gate.

This commit splits the oscillator divider and gate into two separate clock 
entities to support the A733 SoC. It introduces a new read-only divider 
based on the oscillator rate to replace the fixed 750 divider.

> diff --git a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
> index b24c8b196e664..25dd87e78eb7a 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c

[ ... ]

> @@ -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,
>  	},
>  };

[Severity: High]
This isn't a bug introduced by this patch, but does adding an extra clock
here expose an issue in the error handling path of sunxi_ccu_probe()?

In drivers/clk/sunxi-ng/ccu_common.c:sunxi_ccu_probe(), the err_clk_unreg
path unregisters clocks using a loop limit that appears incorrect:

err_clk_unreg:
	while (--i >= 0) {
		struct clk_hw *hw = desc->hw_clks->hws[i];

The variable 'i' incorrectly retains the value of 'desc->num_ccu_clks' from
the preceding loop, rather than using the correct bound 'desc->hw_clks->num'.

For drivers where num_ccu_clks < hw_clks->num (such as sun6i-rtc after this
patch), will this skip unregistering the extra clocks if probe fails? And for
drivers where num_ccu_clks > hw_clks->num, could this read past the end of
the hws array?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-a733-rtc-v2-0-7b72112784f8@baylibre.com?part=5

  reply	other threads:[~2026-06-29 13:12 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-29 12:42 [PATCH v2 0/6] clk: sun6i-rtc: Add support for Allwinner A733 SoC Jerome Brunet
2026-06-29 12:42 ` [PATCH v2 1/6] dt-bindings: rtc: sun6i: add sun60i-a733 support Jerome Brunet
2026-06-29 12:53   ` sashiko-bot
2026-06-29 15:05     ` Jerome Brunet
2026-06-29 15:16       ` Conor Dooley
2026-06-29 15:20         ` Chen-Yu Tsai
2026-06-29 15:27           ` Jerome Brunet
2026-06-29 15:40             ` Conor Dooley
2026-06-29 12:42 ` [PATCH v2 2/6] clk: sunxi-ng: sun6i-rtc: clean up DT usage Jerome Brunet
2026-06-29 15:37   ` Chen-Yu Tsai
2026-06-29 12:42 ` [PATCH v2 3/6] clk: sunxi-ng: sun6i-rtc: Add feature bit for IOSC calibration Jerome Brunet
2026-06-29 15:25   ` Chen-Yu Tsai
2026-06-29 12:42 ` [PATCH v2 4/6] clk: sunxi-ng: div: add read-only operation support Jerome Brunet
2026-06-29 13:03   ` sashiko-bot
2026-06-29 13:58     ` Jerome Brunet
2026-06-29 12:42 ` [PATCH v2 5/6] clk: sunxi-ng: sun6i-rtc: split main oscillator div and gate Jerome Brunet
2026-06-29 13:12   ` sashiko-bot [this message]
2026-06-29 14:15     ` Jerome Brunet
2026-06-29 12:42 ` [PATCH v2 6/6] clk: sunxi-ng: sun6i-rtc: add a733 support Jerome Brunet
2026-06-29 13:15   ` sashiko-bot
2026-06-29 14:24     ` Jerome Brunet
2026-06-29 15:15       ` Chen-Yu Tsai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260629131254.7E34C1F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jbrunet@baylibre.com \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox