All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Shubhrajyoti Datta" <shubhrajyoti.datta@amd.com>
Cc: robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v2 3/8] clk: clocking-wizard: Move clk_in1 acquisition before static-config check
Date: Thu, 09 Jul 2026 11:25:04 +0000	[thread overview]
Message-ID: <20260709112505.05C9F1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260709111403.1579159-4-shubhrajyoti.datta@amd.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unmet dependency on `s_axi_aclk` in static-config mode causes probe failure.
--

--- Patch [3]: [PATCH v2 3/8] clk: clocking-wizard: Move clk_in1 acquisition before static-config check ---
commit 2d23efc5eb24a84c2dc3c21068e9e68c699593d8
Author: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>

clk: clocking-wizard: Move clk_in1 acquisition before static-config check

The clk_in1 is the input clock for both the dynamic reconfig and the
static-config paths. Acquire clk_in1 for static-config as well. Output
clocks are registered as fixed-factor children of clk_in1.

> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index e082051221bef..ffc78c90bee6f 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> @@ -1178,6 +1178,11 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> +	clk_wzrd->clk_in1 = devm_clk_get(&pdev->dev, "clk_in1");
> +	if (IS_ERR(clk_wzrd->clk_in1))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->clk_in1),
> +				     "failed to get clk_in1\n");
> +

[Severity: High]
Does this probe sequence correctly handle devices using xlnx,static-config
without an s_axi_aclk?

Earlier in clk_wzrd_probe(), the driver unconditionally acquires s_axi_aclk:

drivers/clk/xilinx/clk-xlnx-clock-wizard.c:clk_wzrd_probe() {
    ...
    clk_wzrd->axi_clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
    if (IS_ERR(clk_wzrd->axi_clk))
        return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->axi_clk),
                             "s_axi_aclk not found\n");
    ...
}

If a device tree specifies xlnx,static-config and omits the s_axi_aclk clock,
wouldn't this unconditional acquisition return -ENOENT and cause the probe to
fail before it even reaches this new clk_in1 acquisition?

>  	if (!of_property_present(np, "xlnx,static-config")) {
>  		clk_wzrd->base = devm_platform_ioremap_resource(pdev, 0);
>  		if (IS_ERR(clk_wzrd->base))

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709111403.1579159-1-shubhrajyoti.datta@amd.com?part=3

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

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-09 11:13 [PATCH v2 0/8] clk: clocking-wizard: Add static-config clock provider support Shubhrajyoti Datta
2026-07-09 11:13 ` [PATCH v2 1/8] dt-bindings: clock: clocking-wizard: Add static-config mode support Shubhrajyoti Datta
2026-07-09 11:13 ` [PATCH v2 2/8] clk: clocking-wizard: Do not map the memory for static-config Shubhrajyoti Datta
2026-07-09 11:28   ` sashiko-bot
2026-07-09 11:13 ` [PATCH v2 3/8] clk: clocking-wizard: Move clk_in1 acquisition before static-config check Shubhrajyoti Datta
2026-07-09 11:25   ` sashiko-bot [this message]
2026-07-09 11:13 ` [PATCH v2 4/8] clk: clocking-wizard: Add static-config clock provider support Shubhrajyoti Datta
2026-07-09 11:26   ` sashiko-bot
2026-07-09 11:14 ` [PATCH v2 5/8] clk: clocking-wizard: Skip s_axi_aclk for static-config Shubhrajyoti Datta
2026-07-09 11:28   ` sashiko-bot
2026-07-09 11:14 ` [PATCH v2 6/8] clk: clocking-wizard: Use dev_err_probe() when mapping registers Shubhrajyoti Datta
2026-07-09 11:25   ` sashiko-bot
2026-07-09 11:14 ` [PATCH v2 7/8] clk: clocking-wizard: Fix division by zero and unbounded register write Shubhrajyoti Datta
2026-07-09 11:28   ` sashiko-bot
2026-07-09 11:14 ` [PATCH v2 8/8] clk: clocking-wizard: Use separate notifier_block for each clock Shubhrajyoti Datta
2026-07-09 11:20   ` sashiko-bot

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=20260709112505.05C9F1F000E9@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=shubhrajyoti.datta@amd.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.