linux-kernel - Re: [PATCH RFC v3 6/9] spi: axi-spi-engine: implement offload support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5b246e7628ea189be5f8430dac4cffde723b7907.camel@gmail.com>
Date: Tue, 23 Jul 2024 10:01:32 +0200
From: Nuno Sá <noname.nuno@...il.com>
To: David Lechner <dlechner@...libre.com>, Mark Brown <broonie@...nel.org>, 
 Jonathan Cameron
	 <jic23@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
	 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Nuno
 Sá
	 <nuno.sa@...log.com>
Cc: Michael Hennerich <Michael.Hennerich@...log.com>, Lars-Peter Clausen
	 <lars@...afoo.de>, David Jander <david@...tonic.nl>, Martin Sperl
	 <kernel@...tin.sperl.org>, linux-spi@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-iio@...r.kernel.org
Subject: Re: [PATCH RFC v3 6/9] spi: axi-spi-engine: implement offload
 support

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@...libre.com>
> ---
> 

...


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á