Re: [RFC PATCH v1 2/8] misc/arm-cla: Add launch operation helpers

["Arnd Bergmann" <arnd@arndb.de>]

On Fri, Jul 17, 2026, at 12:47, Ryan Roberts wrote:
> From: Jean-Philippe Brucker <jpb@kernel.org>
>
> CLA commands are issued by writing optional payload registers,
> programming the LAUNCH register and polling LRESP until the hardware
> accepts or rejects the operation.
>
> Add a common launch helper that performs this sequence on the CLA's
> local CPU, waits for LRESP completion and translates launch response
> codes into Linux errors.
>
> Build accelerator reset and register read and write support on top of
> it. The register read and write helpers split larger accesses into
> multiple launch operations when an access crosses an eight-register
> window.

I'm a bit confused by the MMIO register access ordering, if this is
not a normal AXI attached device with a DMA master, I think it
would make sense to better document what it is.

> +static inline u64 cla_reg_read(struct cla_dev *dev, off_t reg)
> +{
> +	return readq_relaxed(dev->regs + reg);
> +}
> +
> +static inline void cla_reg_write(struct cla_dev *dev, off_t reg, u64 
> val)
> +{
> +	return writeq_relaxed(val, dev->regs + reg);
> +}

For regular devices that have a DMA master, you cannot use
the relaxed operations by default since they do not serialize
against DMA transfers.

To do this properly, you'd have to define separate cla_reg_read()
and cla_reg_read_relaxed() helpers and then use them as needed,
ideally with a comment for each relaxed instance to explain why
that one is both performance critical and safe.

If for some reason this accelerator is not a DMA master (e.g.
because it is implemented through CPU microcode and accesses
the memory through the CPU's own load/store unit), that should
be documented here to explain that you are relying on
implementation defined behavior outside of the normal driver
and memory model.

> +	/*
> +	 * No barrier needed because accesses use Device-nGnRE, within the 
> same
> +	 * memory-mapped peripheral, so accesses arrive at the endpoint in
> +	 * program order.
> +	 */

This comment in turn looks completely useless, as that is true
for any MMIO device. The only barriers that you'd normally need
here on sane architectures (not Alpha) are to serialize MMIO
against DMA.

> +
> +	if (launch->data_mode == CLA_DATA_OUT)
> +		for (i = 0; i < launch->ndata_m1 + 1; i++)
> +			launch->data[i] = cla_reg_read(dev, CLA_REG_DATA(i));

Instead of the open-coded loop, maybe this can be built
on top of __iowrite32_copy()

> +/**
> + * cla_op_wait_lresp - Wait for any LAUNCH op to complete.
> +int cla_op_wait_lresp(struct cla_dev *dev, u64 *lresp)
> +{
> +	return readq_relaxed_poll_timeout_atomic(dev->regs + CLA_REG_LRESP,
> +			*lresp, FIELD_GET(CLA_LRESP_PENDING, *lresp) == 0,
> +			CLA_LRESP_DELAY_US, CLA_LRESP_TIMEOUT_US);

Similarly, the readq_relaxed_poll_timeout_atomic() specifically
does not wait for DMA, so you may need separate helpers for
devices that can do DMA and readq_poll_timeout_atomic() vs devices
that never access memory and can use the relaxed version.

You may also need a non-atomic version, as blocking the CPU
for 100µs is not great for realtime workloads.

    Arnd