Re: [PATCH] firmware: arm_scmi: add timeout in do_xfer_with_response()

[Cristian Marussi <cristian.marussi@arm.com>]

On Sat, Apr 12, 2025 at 01:39:45PM +0300, Matthew Bystrin wrote:
> Sudeep,
> 

Hi Matthew,

> Thanks for taking your time.
> 
> Sudeep Holla, Apr 09, 2025 at 14:12:
> > The start update should retain as soon as Platform uC acks the request.
> > And 2 notifications can be sent out for update procedure started and
> > completed. I don't see any issue there. What is the semantics you are
> > talking about ?
> 
> I'm going to refer to section 4.1.1 from the spec, where stated following about
> delayed responses,
> 
> "Messages sent to indicate completion of the work that is associated with an
> asynchronous command" 
> 
> Compared to notifications,
> 
> "These messages provide notifications of events taking place in the platform.
> Events might include changes in power state, performance state, or other
> platform status"
> 
> So before I implemented mentioned driver I had red this two and had chosen
> delayed responses, because it had seemed more appropriate. Details below.
> 
> > Even delayed response as some timeout so I would rather prefer to use
> > notifications
> 
> Hmm, I see.
> 
> > in your usecase as it is completely async.
> 
> Just to emphasize, according to the spec I don't think that delayed responses
> and events have different degree of asynchrony. The difference is in the
> initiator of 'messaging'. Events are sent by platform to indicate its' state and
> delayed responses are sent to indicate status of previously requested operation.
> 

Delayed reponses are certainly better than notification for completion
of agent initiated actions BUT this does not exclude the usage instead
of a sync-command to start the operation and a notification to signal
its completion...depends really on the case.

The classic example of a needed async-cmd is reading a sensor that takes
a long time due to its own physical nature...

AFAIU, in this case you have an async operation whose completion time is
considerably longer (so you aim to configure a specific timeout for that
specific command) BUT it is also bound to the payload itself that you
are trying to load AND/OR to other platform specific HW charactristics
(like how slow are your flashes in this HW releases...): this means that
while the sensor slowness is stable and predictable, and the timeout can
be fixed a-priori, in this case you risk to have in the future anyway to
have to refine and tune this ad-hoc custom timeout....while you'd have
none of this issue by simply waiting for a notification (ofc you could
have to set a large timeout on your side anyway while waiting for
notifs...)

...unless you plan to dynamically tune the async-cmd timeout at runtime
based on the known payload size (that means more commands to query the
soon-to-be-flahsed payload) but anyway this does NOT solve the fact that
the platform characteristics can influence the length of the operation.

Thanks,
Cristian