mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "NG, TZE YEE" <tze.yee.ng@altera.com>
To: Guenter Roeck <linux@roeck-us.net>,
	Dinh Nguyen <dinguyen@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>,
	"linux-hwmon@vger.kernel.org" <linux-hwmon@vger.kernel.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>
Subject: Re: [PATCH v7 2/2] hwmon: add Altera SoC FPGA hardware monitoring driver
Date: Wed, 22 Jul 2026 07:38:07 +0000	[thread overview]
Message-ID: <fc7b576c-6e83-45c9-aeb4-49f02113fdd0@altera.com> (raw)
In-Reply-To: <2c99fa33-515b-46f7-b2b8-b7a2b9dcbd84@roeck-us.net>

On 21/7/2026 11:08 am, Guenter Roeck wrote:
> On 7/20/26 19:18, tze.yee.ng@altera.com wrote:
>> From: Tze Yee Ng <tze.yee.ng@altera.com>
>>
>> Add a hardware monitor driver for Altera SoC FPGA devices using the
>> Stratix 10 service layer. Sensor channels are selected based on the
>> service layer compatible string.
>>
>> Signed-off-by: Nazim Amirul 
>> <muhammad.nazim.amirul.nazle.asmade@altera.com>
>> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
>> ---
> ...
>> +
>> +static int socfpga_hwmon_sync_read(struct device *dev,
>> +                   enum hwmon_sensor_types type,
>> +                   struct stratix10_svc_client_msg *msg)
>> +{
>> +    struct socfpga_hwmon_priv *priv = dev_get_drvdata(dev);
>> +    int ret;
>> +
>> +    reinit_completion(&priv->completion);
>> +
>> +    if (type == hwmon_temp)
>> +        priv->client.receive_cb = socfpga_hwmon_readtemp_cb;
>> +    else
>> +        priv->client.receive_cb = socfpga_hwmon_readvolt_cb;
>> +
>> +    ret = stratix10_svc_send(priv->chan, msg);
>> +    if (ret < 0)
>> +        goto status_done;
>> +
>> +    ret = wait_for_completion_timeout(&priv->completion, HWMON_TIMEOUT);
>> +    if (!ret) {
>> +        dev_err(priv->client.dev, "timeout waiting for SMC call\n");
>> +        /*
>> +         * stratix10_svc_done() stops the worker but does not flush
>> +         * svc_fifo. Wait for the outstanding callback before stopping
>> +         * so a leftover request cannot complete a later sensor read.
>> +         * Discard the late result and still report -ETIMEDOUT.
>> +         */
>> +        wait_for_completion(&priv->completion);
> 
> Just like Sashiko, I am puzzled and do not understand this code.
> 
> Guenter
> 

Hi Guenter,

Apologies, there shouldn't be another wait_for_completion if timeout. 
The new added wait_for_completion(&priv->completion) will be dropped in v8.

Regarding the FIFO concern: while svc_normal_to_secure_thread is
running it dequeues each request and issues the SMC before the client
timeout in the normal case, so there should be no leftover entry after
a successful or failed transaction that invoked receive_cb.

The remaining caveat is if the worker exits via kthread_should_stop()
(from stratix10_svc_done()) before dequeuing: svc_fifo is tied to the
channel, not the thread, and stratix10_svc_done() does not flush it, so
a queued request can remain and be processed by the next thread started
on a later stratix10_svc_send(). We plan to address that with a FIFO
flush in the SVC driver in a separate series.

Thanks,
Tze Yee



      reply	other threads:[~2026-07-22  7:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-21  2:17 [PATCH v7 0/2] hwmon: add Altera SoC FPGA hardware monitoring support tze.yee.ng
2026-07-21  2:18 ` [PATCH v7 1/2] firmware: stratix10-svc: add async HWMON read commands and register socfpga-hwmon device tze.yee.ng
2026-07-21  2:18 ` [PATCH v7 2/2] hwmon: add Altera SoC FPGA hardware monitoring driver tze.yee.ng
2026-07-21  3:08   ` Guenter Roeck
2026-07-22  7:38     ` NG, TZE YEE [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=fc7b576c-6e83-45c9-aeb4-49f02113fdd0@altera.com \
    --to=tze.yee.ng@altera.com \
    --cc=corbet@lwn.net \
    --cc=dinguyen@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=skhan@linuxfoundation.org \
    /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®