Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Taniya Das" <taniya.das@oss.qualcomm.com>
Cc: robh@kernel.org, devicetree@vger.kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v3 4/6] clk: qcom: Add Nord display clock controller support
Date: Fri, 24 Jul 2026 11:11:53 +0000	[thread overview]
Message-ID: <20260724111154.5496E1F0429A@smtp.kernel.org> (raw)
In-Reply-To: <20260724-nords_mm_v1-v3-4-32b45232217f@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing `clk_alpha_pll_postdiv` object for `PLL1_OUT_EVEN`, resulting in incorrect parent aliasing.
--

--- Patch [4]: [PATCH v3 4/6] clk: qcom: Add Nord display clock controller support ---
commit 15bad88e0e41a7dac6339581fa90741a6515e990
Author: Taniya Das <taniya.das@oss.qualcomm.com>

clk: qcom: Add Nord display clock controller support

Add support for the display clock controllers (DISPCC) on the
Qualcomm Nord platform.

> diff --git a/drivers/clk/qcom/dispcc0-nord.c b/drivers/clk/qcom/dispcc0-nord.c
> new file mode 100644
> index 0000000000000..835aba1596136
> --- /dev/null
> +++ b/drivers/clk/qcom/dispcc0-nord.c

[ ... ]

> +static const struct parent_map disp_cc_0_parent_map_7[] = {
> +	{ P_BI_TCXO, 0 },
> +	{ P_MDSS_0_DISP_CC_PLL1_OUT_MAIN, 4 },
> +	{ P_MDSS_0_DISP_CC_PLL1_OUT_EVEN, 6 },
> +};
> +
> +static const struct clk_parent_data disp_cc_0_parent_data_7[] = {
> +	{ .index = DT_BI_TCXO },
> +	{ .hw = &mdss_0_disp_cc_pll1.clkr.hw },
> +	{ .hw = &mdss_0_disp_cc_pll1.clkr.hw },
> +};

[Severity: Medium]
Will this cause the Common Clock Framework to miscalculate the frequency
if the bootloader leaves the hardware configured to select the OUT_EVEN
parent?

Because both OUT_MAIN and OUT_EVEN alias directly to the main PLL hardware
object without a post-divider object, it appears the CCF will read the
undivided main PLL rate for both. This could result in reporting a
frequency higher than reality, potentially causing downstream display
pipeline underflows or glitches during splash screen handoff.

Does there need to be a separate clk_alpha_pll_postdiv object to
properly model the OUT_EVEN divider?

[ ... ]

> +static const struct parent_map disp_cc_0_parent_map_8[] = {
> +	{ P_BI_TCXO, 0 },
> +	{ P_MDSS_0_DISP_CC_PLL0_OUT_MAIN, 1 },
> +	{ P_MDSS_0_DISP_CC_PLL1_OUT_MAIN, 4 },
> +	{ P_MDSS_0_DISP_CC_PLL1_OUT_EVEN, 6 },
> +};
> +
> +static const struct clk_parent_data disp_cc_0_parent_data_8[] = {
> +	{ .index = DT_BI_TCXO },
> +	{ .hw = &mdss_0_disp_cc_pll0.clkr.hw },
> +	{ .hw = &mdss_0_disp_cc_pll1.clkr.hw },
> +	{ .hw = &mdss_0_disp_cc_pll1.clkr.hw },
> +};

[Severity: Medium]
This struct has the same aliasing of OUT_EVEN to the main PLL hardware as
disp_cc_0_parent_data_7 above.

Could this also miscalculate the mdss_0_disp_cc_mdss_mdp_clk_src frequency
if inherited from the bootloader?

(Note: This exact same pattern appears in drivers/clk/qcom/dispcc1-nord.c
for disp_cc_1_parent_data_7 and disp_cc_1_parent_data_8 as well).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-nords_mm_v1-v3-0-32b45232217f@oss.qualcomm.com?part=4

  reply	other threads:[~2026-07-24 11:11 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-24 10:55 [PATCH v3 0/6] clk: qcom: Add Nord multimedia clock controller support Taniya Das
2026-07-24 10:55 ` [PATCH v3 1/6] clk: qcom: gcc-nord: mark PCIe link clocks as critical Taniya Das
2026-07-24 10:55 ` [PATCH v3 2/6] clk: qcom: negcc-nord: keep GPU2 CFG clock enabled via critical CBCR Taniya Das
2026-07-24 10:55 ` [PATCH v3 3/6] dt-bindings: clock: qcom: Document Nord display clock controller Taniya Das
2026-07-24 10:55 ` [PATCH v3 4/6] clk: qcom: Add Nord display clock controller support Taniya Das
2026-07-24 11:11   ` sashiko-bot [this message]
2026-07-24 10:55 ` [PATCH v3 5/6] dt-bindings: clock: qcom: Document Nord GPU clock controllers Taniya Das
2026-07-24 10:55 ` [PATCH v3 6/6] clk: qcom: gpucc: Add Nord graphics clock controller support Taniya Das

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=20260724111154.5496E1F0429A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=taniya.das@oss.qualcomm.com \
    /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