Re: [PATCH v7 2/2] hwmon: add Altera SoC FPGA hardware monitoring driver

From: NG, TZE YEE

Date: Wed Jul 22 2026 - 03:42:36 EST


On 21/7/2026 11:08 am, Guenter Roeck wrote:
> On 7/20/26 19:18, tze.yee.ng@xxxxxxxxxx wrote:
>> From: Tze Yee Ng <tze.yee.ng@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>> Signed-off-by: Tze Yee Ng <tze.yee.ng@xxxxxxxxxx>
>> ---
> ...
>> +
>> +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