Re: [PATCH v5 0/8] Add SCMI Virtio & Clock atomic support

From: Michael S. Tsirkin
Date: Fri Mar 04 2022 - 11:14:11 EST


On Thu, Feb 17, 2022 at 01:12:26PM +0000, Cristian Marussi wrote:
> Hi,
>
> This small series is the tail-subset of the previous V8 series about atomic
> support in SCMI [1], whose 8-patches head-subset has now been queued on
> [2]; as such, it is based on [2] on top of tag scmi-updates-5.17:
>
> commit 94d0cd1da14a ("firmware: arm_scmi: Add new parameter to
> mark_txdone")
>
> Patch [1/8] substitute virtio-scmi ready flag and lock with a reference
> counter to keep track of vio channels lifetime while removing the need of
> a wide spinlocked section (that would cause issues with introduction of
> virtio polling support)
>
> Patch [2/8] adds a few helpers to handle the TX free_list and a dedicated
> spinlock to reduce the reliance on the main one.
>
> Patch [3/8] adds polling mode to SCMI VirtIO transport in order to support
> atomic operations on such transport.
>
> Patches [4,5/8] introduce a new optional SCMI binding, atomic-threshold-us,
> to configure a platform specific time threshold used in the following
> patches to select with a finer grain which SCMI resources should be
> eligible for atomic operations when requested.
>
> Patch [6/8] exposes new SCMI Clock protocol operations to allow an SCMI
> user to request atomic mode on clock enable commands.
>
> Patch [7/8] adds support to SCMI Clock protocol for a new clock attributes
> field which advertises typical enable latency for a specific resource.
>
> Finally patch [8/8] add support for atomic operations to the SCMI clock
> driver; the underlying logic here is that we register with the Clock
> framework atomic-capable clock resources if and only if the backing SCMI
> transport is capable of atomic operations AND the specific clock resource
> has been advertised by the SCMI platform as having:
>
> clock_enable_latency <= atomic-threshold-us
>
> The idea is to avoid costly atomic busy-waiting for resources that have
> been advertised as 'slow' to operate upon. (i.e. a PLL vs a gating clock)
>
> To ease testing the whole series can be find at [3].
>
> Any feedback/testing welcome as usual.
>
> Thanks,
> Cristian


SCMI specific stuff so I don't have anything to add here.

By the way, it does not look like anything regarding SCMI atomic support
is needed in the virtio spec - is it true the interface with the device
is unaffected?

> [1]: https://lore.kernel.org/linux-arm-kernel/20211220195646.44498-1-cristian.marussi@xxxxxxx/
> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git/tag/?h=scmi-updates-5.17
> [3]: https://gitlab.arm.com/linux-arm/linux-cm/-/commits/scmi_atomic_clk_virtio_V5/
>
> ---
> v4 --> v5
> - dt_bindings: fixed example and removed dtschema warnings/errors
> - dt_bindings: added 'default: 0' clause
> - introduced vio_msg refcounts and helpers to avoid premature reuse of
> freed messages when both poling and IRQ path are active on a buffer
> - better handling of timed out polled messages on late replies using
> new VIO_MSG_POLL_TIMEOUT state
> - fixed comments on locks
> - removed unneeded virtqueue re-enable when fail to acquire channel in
> complete_cb
>
> V3 --> V4
> - renamed optional DT property to atomic-threshold-us
>
> V2 --> V3
> - split out virtio_ring RFC patch into a distinct series
> - calling virtqueue_broke_device when cleaning up channel
> - removed RFC tags from CLK related patches
>
> V1 --> V2
> - added vio channel refcount support patch
> - reviewed free_list support and usage
> - added virtio_ring RFC patch
> - shrinked spinlocked section within virtio_poll_done to exclude
> virtqueue_poll call
> - removed poll_lock
> - use vio channel refcount acquire/release logic when polling
> - using new free_list accessors
> - added new dedicated pending_lock to access pending_cmds_list
> - fixed a few comments
>
> Cristian Marussi (8):
> firmware: arm_scmi: Add a virtio channel refcount
> firmware: arm_scmi: Review virtio free_list handling
> firmware: arm_scmi: Add atomic mode support to virtio transport
> dt-bindings: firmware: arm,scmi: Add atomic-threshold-us optional
> property
> firmware: arm_scmi: Support optional system wide atomic-threshold-us
> firmware: arm_scmi: Add atomic support to clock protocol
> firmware: arm_scmi: Add support for clock_enable_latency
> clk: scmi: Support atomic clock enable/disable API
>
> .../bindings/firmware/arm,scmi.yaml | 10 +
> drivers/clk/clk-scmi.c | 71 ++-
> drivers/firmware/arm_scmi/Kconfig | 15 +
> drivers/firmware/arm_scmi/clock.c | 34 +-
> drivers/firmware/arm_scmi/driver.c | 33 +-
> drivers/firmware/arm_scmi/virtio.c | 591 +++++++++++++++---
> include/linux/scmi_protocol.h | 9 +-
> 7 files changed, 655 insertions(+), 108 deletions(-)
>
> --
> 2.17.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>