On Mon, 2024-07-22 at 16:57 -0500, David Lechner wrote:
> This implements SPI offload support for the AXI SPI Engine. Currently,
> the hardware only supports triggering offload transfers with a hardware
> trigger so attempting to use an offload message in the regular SPI
> message queue will fail. Also, only allows streaming rx data to an
> external sink, so attempts to use a rx_buf in the offload message will
> fail.
>
> Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx>
> ---
>
...
I'm likely missing something but you already have:
priv = &spi_engine->offload_priv[args[0]];
which seems that from FW you already got the offload index you need. Can't we
just save that index in struct spi_device and use that directly in the other
operations? Saving the trouble to save the id string and having to always call
spi_engine_get_offload()?
> +
>
...
> +}
> +
> +static void spi_engine_offload_unprepare(struct spi_device *spi, const char
> *id)
> +{
> + struct spi_controller *host = spi->controller;
> + struct spi_engine *spi_engine = spi_controller_get_devdata(host);
> + struct spi_engine_offload *priv;
> + unsigned int offload_num;
> +
> + priv = spi_engine_get_offload(spi, id, &offload_num);
> + if (IS_ERR(priv)) {
> + dev_warn(&spi->dev, "failed match offload in unprepare\n");
> + return;
> + }
> +
> + writel_relaxed(1, spi_engine->base +
> SPI_ENGINE_REG_OFFLOAD_RESET(offload_num));
> + writel_relaxed(0, spi_engine->base +
> SPI_ENGINE_REG_OFFLOAD_RESET(offload_num));
> +
> + priv->prepared = false;
> +}
> +
> +static int spi_engine_hw_trigger_mode_enable(struct spi_device *spi,
> + const char *id)
> +{
> + struct spi_controller *host = spi->controller;
> + struct spi_engine *spi_engine = spi_controller_get_devdata(host);
> + struct spi_engine_offload *priv;
> + unsigned int offload_num, reg;
> +
> + priv = spi_engine_get_offload(spi, id, &offload_num);
> + if (IS_ERR(priv))
> + return PTR_ERR(priv);
> +
> + reg = readl_relaxed(spi_engine->base +
> + SPI_ENGINE_REG_OFFLOAD_CTRL(offload_num));
> + reg |= SPI_ENGINE_OFFLOAD_CTRL_ENABLE;
> + writel_relaxed(reg, spi_engine->base +
> + SPI_ENGINE_REG_OFFLOAD_CTRL(offload_num));
> +
> + return 0;
> +}
> +
> +static void spi_engine_hw_trigger_mode_disable(struct spi_device *spi,
> + const char *id)
> +{
> + struct spi_controller *host = spi->controller;
> + struct spi_engine *spi_engine = spi_controller_get_devdata(host);
> + struct spi_engine_offload *priv;
> + unsigned int offload_num, reg;
> +
> + priv = spi_engine_get_offload(spi, id, &offload_num);
> + if (IS_ERR(priv)) {
> + dev_warn(&spi->dev, "failed match offload in disable\n");
> + return;
> + }
> +
> + reg = readl_relaxed(spi_engine->base +
> + SPI_ENGINE_REG_OFFLOAD_CTRL(offload_num));
> + reg &= ~SPI_ENGINE_OFFLOAD_CTRL_ENABLE;
> + writel_relaxed(reg, spi_engine->base +
> + SPI_ENGINE_REG_OFFLOAD_CTRL(offload_num));
> +}
> +
I would expect for the enable/disable() operations to act on the trigger. In
this case to enable/disable the clock...
- Nuno Sá
