Messages in this thread | | | From | Peter Hilber <> | Subject | Re: [PATCH v6 00/17] Introduce SCMI transport based on VirtIO | Date | Thu, 15 Jul 2021 18:35:38 +0200 |
| |
On 12.07.21 16:18, Cristian Marussi wrote: > Hi all, >
Hi Cristian,
thanks for your update. Please find some additional comments in this reply and the following.
Best regards,
Peter
> While reworking this series starting from the work done up to V3 by > OpenSynergy, I am keeping the original autorship and list distribution > unchanged. > > The main aim of this rework, as said, is to simplify where possible the > SCMI VirtIO support added in V3 by adding at first some new general > mechanisms in the SCMI Transport layer. > > Indeed, after some initial small fixes, patches 05/06/07/08 add such new > additional mechanisms to the SCMI core to ease implementation of more > complex transports like virtio, while also addressing a few general issues > already potentially affecting existing transports. > > In terms of rework I dropped original V3 patches 05/06/07/08/12 as no more > needed, and modified where needed the remaining original patches to take > advantage of the above mentioned new SCMI transport features. > > DT bindings patch has been ported on top of freshly YAML converted arm,scmi > bindings. > > Moreover, since V5 I dropped support for polling mode from the virtio-scmi > transport, since it is an optional general mechanism provided by the core > to allow transports lacking a completion IRQ to work and it seemed a > needless addition/complication in the context of virtio transport. >
Just for correctness, in my understanding polling is not completely optional ATM. Polling would be required by scmi_cpufreq_fast_switch(). But that requirement might be irrelevant for now.
> Additionally, in V5 I could also simplify a bit the virtio transport > probing sequence starting from the observation that, by the VirtIO spec, > in fact, only one single SCMI VirtIO device can possibly exist on a system. >
I wouldn't say that the virtio spec restricts the # of virtio-scmi devices to one. But I do think the one device limitation in the kernel is acceptable.
> The series has been tested using an emulated fake SCMI device and also a > proper SCP-fw stack running through QEMU vhost-users, with the SCMI stack > compiled, in both cases, as builtin and as a loadable module, running tests > against mocked SCMI Sensors using HWMON and IIO interfaces to check the > functionality of notifications and sync/async commands. > > Virtio-scmi support has been exercised in the following testing scenario > on a JUNO board: > > - normal sync/async command transfers > - notifications > - concurrent delivery of correlated response and delayed responses > - out-of-order delivery of delayed responses before related responses > - unexpected delayed response delivery for sync commands > - late delivery of timed-out responses and delayed responses > > Some basic regression testing against mailbox transport has been performed > for commands and notifications too. > > No sensible overhead in total handling time of commands and notifications > has been observed, even though this series do indeed add a considerable > amount of code to execute on TX path. > More test and measurements could be needed in these regards. > > This series is based on top of v5.14-rc1. > > Any feedback/testing is welcome :D > > Thanks, > Cristian > --- > V5 --> V6: > - removed delegated xfers and its usage > - add and use *priv optional parameter in scmi_rx_callback() > - made .poll_done and .clear_channel ops optional > > V4 --> V5: > - removed msg raw_payload helpers > - reworked msg helpers to use xfer->priv reference > - simplified SCMI device probe sequence (one static device) > - added new SCMI Kconfig layout > - removed SCMI virtio polling support > > V3 --> V4: > - using new delegated xfers support and monotonically increasing tokens > in virtio transport > - ported SCMI virtio transport DT bindings to YAML format > - added virtio-scmi polling support > - added delegated xfers support > > Cristian Marussi (11): > firmware: arm_scmi: Avoid padding in sensor message structure > firmware: arm_scmi: Fix max pending messages boundary check > firmware: arm_scmi: Add support for type handling in common functions > firmware: arm_scmi: Remove scmi_dump_header_dbg() helper > firmware: arm_scmi: Add transport optional init/exit support > firmware: arm_scmi: Introduce monotonically increasing tokens > firmware: arm_scmi: Handle concurrent and out-of-order messages > firmware: arm_scmi: Add priv parameter to scmi_rx_callback > firmware: arm_scmi: Make .clear_channel optional > firmware: arm_scmi: Make polling mode optional > firmware: arm_scmi: Make SCMI transports configurable > > Igor Skalkin (4): > firmware: arm_scmi: Make shmem support optional for transports > firmware: arm_scmi: Add method to override max message number > dt-bindings: arm: Add virtio transport for SCMI > firmware: arm_scmi: Add virtio transport > > Peter Hilber (2): > firmware: arm_scmi: Add message passing abstractions for transports > firmware: arm_scmi: Add optional link_supplier() transport op > > .../bindings/firmware/arm,scmi.yaml | 8 +- > MAINTAINERS | 1 + > drivers/firmware/Kconfig | 34 +- > drivers/firmware/arm_scmi/Kconfig | 97 +++ > drivers/firmware/arm_scmi/Makefile | 8 +- > drivers/firmware/arm_scmi/common.h | 94 ++- > drivers/firmware/arm_scmi/driver.c | 651 +++++++++++++++--- > drivers/firmware/arm_scmi/mailbox.c | 2 +- > drivers/firmware/arm_scmi/msg.c | 113 +++ > drivers/firmware/arm_scmi/sensors.c | 6 +- > drivers/firmware/arm_scmi/smc.c | 3 +- > drivers/firmware/arm_scmi/virtio.c | 491 +++++++++++++ > include/uapi/linux/virtio_ids.h | 1 + > include/uapi/linux/virtio_scmi.h | 24 + > 14 files changed, 1389 insertions(+), 144 deletions(-) > create mode 100644 drivers/firmware/arm_scmi/Kconfig > create mode 100644 drivers/firmware/arm_scmi/msg.c > create mode 100644 drivers/firmware/arm_scmi/virtio.c > create mode 100644 include/uapi/linux/virtio_scmi.h >
| |