Page MenuHomeFreeBSD

scmi: Add SCMI message tracking and centralize tx/rx logic
ClosedPublic

Authored by cristian.marussi_arm.com on Dec 13 2023, 6:37 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 4, 10:26 PM
Unknown Object (File)
Mon, Nov 4, 10:26 PM
Unknown Object (File)
Mon, Nov 4, 10:26 PM
Unknown Object (File)
Mon, Nov 4, 10:26 PM
Unknown Object (File)
Mon, Nov 4, 10:26 PM
Unknown Object (File)
Mon, Nov 4, 10:26 PM
Unknown Object (File)
Mon, Nov 4, 10:10 PM
Unknown Object (File)
Fri, Nov 1, 11:52 AM
Subscribers

Details

Summary

In order to be able to support also new, more parallel, SCMI transports
that by nature can allow multiple concurrent commands to be in-flight,
pending a reply, we must be able to use the sequence number provided in
the SCMI messages to track the message status, matching commands and
replies while keeping track of timeouts and duplicates.

Add the needed message tracking machinery in the core SCMI stack and
move the residual common tx/rx logic from the specific transports to
the core SCMI stack, while adding one more interface to let the
transports customize ther behaviour.

Tested on: ARM Morello Board
Sponsored by: Arm Ltd

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Could you use unr(9), the kernel unit number allocator, to allocate the tokens?

Could you use unr(9), the kernel unit number allocator, to allocate the tokens?

Thanks for the suggestion, I'll have a better look at unr(9).
In general the SCMI stack tries to allocate sequence numbers in a monotonically increasing pattern (till they wraps at 1023) and unr(9) seems
to always try to allocate the lowest possible free number (if not asked for a specific number).
I have to check (with myself O_o) if this is a remnant from the Linux world and can be dropped or it does make still sense also
in the freeBSD SCMI stack.

sys/dev/firmware/arm/scmi.c
187–188

malloc with M_WAITOK can't fail so doesn't need a null check.

Removed check on malloc return value

malloc M_WAITOK cannot fail remove the useless check

sys/dev/firmware/arm/scmi.c
194

Could this be moved before the malloc? That way if it fails we don't need to free anything.

228
sys/dev/firmware/arm/scmi.c
194

I dont think it can be moved, because the SCMI_TRANSPORT_INIT is the per-transport specific initialization that finally sets up and made the underlying transport ready to go (i.e.by enabling vqueue or enabling mbox irqs...) and as such it relies on the core transport stuff to have been already setup....if some unexpected spurious traffic from platform arrives after SCMI_TRANSPORT_INIT, an attempt will be made to handle it, and it will need trs-stuff (like the hashtable).

What I think instead is wrong looking at this now, it is the sc->trs = trs; line down below...it should really happen before SCMI_TRANSPORT_INIT to be really ready for this limit-case.

I'll fix that in the next respin and repost related patches if needed by rebasing

Moved sc->trs allocation earlier

In scmi_transport_init initialize sc->trs before calling into per-transport init.

Fixed if (token)

Proper checking for non zero.

This revision is now accepted and ready to land.Mar 8 2024, 2:47 PM