From: Xu Yilun <yilun.xu@linux.intel.com>
To: Pengpeng Hou <hppiscas@163.com>
Cc: Moritz Fischer <mdf@kernel.org>, Xu Yilun <yilun.xu@intel.com>,
Tom Rix <trix@redhat.com>,
Alan Tull <atull@opensource.altera.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fpga: socfpga-a10: Propagate DCLK completion timeouts
Date: Mon, 7 Sep 2026 00:02:30 +0800 [thread overview]
Message-ID: <ap2OljjMRZehVISA@yilunxu-OptiPlex-7050> (raw)
In-Reply-To: <20260906033900.84849-1-hppiscas@163.com>
On Sun, Sep 06, 2026 at 11:39:00AM +0800, Pengpeng Hou wrote:
> socfpga_a10_fpga_generate_dclks() waits for the generated DCLK count to
> complete but discards the poll result. Configuration can therefore continue
> after the hardware did not acknowledge a required DCLK sequence.
The process can fail on multiple operations, breaking out only on
xx_generate_dclks() doesn't make much sense. I think if the final checks
can be the last safety net, then it is fine.
Do you find any real issue at runtime?
>
> Return the poll result and stop write initialization on failure. During
> write completion, retain the existing cleanup sequence and return the
> earlier PR error in preference to a later DCLK error.
>
> The issue was found by our static-analysis tool and manually reviewed.
>
> Fixes: acbb910ae04b ("fpga-manager: Add Socfpga Arria10 support")
> Assisted-by: gpt 5
> Signed-off-by: Pengpeng Hou <hppiscas@163.com>
> ---
> drivers/fpga/socfpga-a10.c | 26 ++++++++++++++++++--------
> 1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c
> index 0165a3c86932..f707c2bdc189 100644
> --- a/drivers/fpga/socfpga-a10.c
> +++ b/drivers/fpga/socfpga-a10.c
> @@ -124,10 +124,11 @@ static void socfpga_a10_fpga_set_cfg_width(struct a10_fpga_priv *priv,
> A10_FPGAMGR_IMGCFG_CTL_02_CFGWIDTH, width);
> }
>
> -static void socfpga_a10_fpga_generate_dclks(struct a10_fpga_priv *priv,
> - u32 count)
> +static int socfpga_a10_fpga_generate_dclks(struct a10_fpga_priv *priv,
> + u32 count)
> {
> u32 val;
> + int ret;
>
> /* Clear any existing DONE status. */
> regmap_write(priv->regmap, A10_FPGAMGR_DCLKSTAT_OFST,
> @@ -137,12 +138,15 @@ static void socfpga_a10_fpga_generate_dclks(struct a10_fpga_priv *priv,
> regmap_write(priv->regmap, A10_FPGAMGR_DCLKCNT_OFST, count);
>
> /* wait till the dclkcnt done */
> - regmap_read_poll_timeout(priv->regmap, A10_FPGAMGR_DCLKSTAT_OFST, val,
> - val, 1, 100);
> + ret = regmap_read_poll_timeout(priv->regmap,
> + A10_FPGAMGR_DCLKSTAT_OFST, val,
> + val, 1, 100);
>
> /* Clear DONE status. */
> regmap_write(priv->regmap, A10_FPGAMGR_DCLKSTAT_OFST,
> A10_FPGAMGR_DCLKSTAT_DCLKDONE);
> +
> + return ret;
> }
>
> #define RBF_ENCRYPTION_MODE_OFFSET 69
> @@ -334,7 +338,9 @@ static int socfpga_a10_fpga_write_init(struct fpga_manager *mgr,
> A10_FPGAMGR_IMGCFG_CTL_01_S2F_NENABLE_CONFIG, 0);
>
> /* Send some clocks to clear out any errors */
> - socfpga_a10_fpga_generate_dclks(priv, 256);
> + ret = socfpga_a10_fpga_generate_dclks(priv, 256);
> + if (ret)
> + return ret;
>
> /* Assert pr_request */
> regmap_update_bits(priv->regmap, A10_FPGAMGR_IMGCFG_CTL_01_OFST,
> @@ -342,7 +348,9 @@ static int socfpga_a10_fpga_write_init(struct fpga_manager *mgr,
> A10_FPGAMGR_IMGCFG_CTL_01_S2F_PR_REQUEST);
>
> /* Provide 2048 DCLKs before starting the config data streaming. */
> - socfpga_a10_fpga_generate_dclks(priv, 0x7ff);
> + ret = socfpga_a10_fpga_generate_dclks(priv, 0x7ff);
> + if (ret)
> + return ret;
>
> /* Wait for pr_ready */
> return socfpga_a10_fpga_wait_for_pr_ready(priv);
> @@ -393,7 +401,7 @@ static int socfpga_a10_fpga_write_complete(struct fpga_manager *mgr,
> {
> struct a10_fpga_priv *priv = mgr->priv;
> u32 reg;
> - int ret;
> + int dclk_ret, ret;
>
> /* Wait for pr_done */
> ret = socfpga_a10_fpga_wait_for_pr_done(priv);
> @@ -403,7 +411,7 @@ static int socfpga_a10_fpga_write_complete(struct fpga_manager *mgr,
> A10_FPGAMGR_IMGCFG_CTL_01_S2F_PR_REQUEST, 0);
>
> /* Send some clocks to clear out any errors */
> - socfpga_a10_fpga_generate_dclks(priv, 256);
> + dclk_ret = socfpga_a10_fpga_generate_dclks(priv, 256);
>
> /* Disable s2f dclk and data */
> regmap_update_bits(priv->regmap, A10_FPGAMGR_IMGCFG_CTL_02_OFST,
> @@ -422,6 +430,8 @@ static int socfpga_a10_fpga_write_complete(struct fpga_manager *mgr,
> /* Return any errors regarding pr_done or pr_error */
> if (ret)
> return ret;
> + if (dclk_ret)
> + return dclk_ret;
>
> /* Final check */
> reg = socfpga_a10_fpga_read_stat(priv);
> --
> 2.50.1 (Apple Git-155)
>
>
prev parent reply other threads:[~2026-09-06 16:02 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-06 3:39 Pengpeng Hou
2026-09-06 16:02 ` Xu Yilun [this message]
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=ap2OljjMRZehVISA@yilunxu-OptiPlex-7050 \
--to=yilun.xu@linux.intel.com \
--cc=atull@opensource.altera.com \
--cc=gregkh@linuxfoundation.org \
--cc=hppiscas@163.com \
--cc=linux-fpga@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mdf@kernel.org \
--cc=trix@redhat.com \
--cc=yilun.xu@intel.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
all inboxes | Powered by JetHome®