| 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
| ||
|
Message-ID: <d5fcd8b83f424bc05d8363210632bf8db4c376cb.camel@gmail.com>
Date: Tue, 20 Feb 2024 11:45:55 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: David Lechner <dlechner@...libre.com>, Mark Brown <broonie@...nel.org>
Cc: Martin Sperl <kernel@...tin.sperl.org>, David Jander
<david@...tonic.nl>, Jonathan Cameron <jic23@...nel.org>, Michael
Hennerich <michael.hennerich@...log.com>, Nuno Sá
<nuno.sa@...log.com>, Alain Volmat <alain.volmat@...s.st.com>, Maxime
Coquelin <mcoquelin.stm32@...il.com>, Alexandre Torgue
<alexandre.torgue@...s.st.com>, linux-spi@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-iio@...r.kernel.org, Julien
Stephan <jstephan@...libre.com>
Subject: Re: [PATCH v2 4/5] spi: axi-spi-engine: move message compile to
optimize_message
On Mon, 2024-02-19 at 16:33 -0600, David Lechner wrote:
> In the AXI SPI Engine driver, compiling the message is an expensive
> operation. Previously, it was done per message transfer in the
> prepare_message hook. This patch moves the message compile to the
> optimize_message hook so that it is only done once per message in
> cases where the peripheral driver calls spi_optimize_message().
>
> This can be a significant performance improvement for some peripherals.
> For example, the ad7380 driver saw a 13% improvement in throughput
> when using the AXI SPI Engine driver with this patch.
>
> Since we now need two message states, one for the optimization stage
> that doesn't change for the lifetime of the message and one that is
> reset on each transfer for managing the current transfer state, the old
> msg->state is split into msg->opt_state and spi_engine->msg_state. The
> latter is included in the driver struct now since there is only one
> current message at a time that can ever use it and it is in a hot path
> so avoiding allocating a new one on each message transfer saves a few
> cpu cycles and lets us get rid of the prepare_message callback.
>
> Signed-off-by: David Lechner <dlechner@...libre.com>
> ---
Reviewed-by: Nuno Sa <nuno.sa@...log.com>
>
> v2 changes: none
>
> drivers/spi/spi-axi-spi-engine.c | 40 +++++++++++++++++-----------------------
> 1 file changed, 17 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
> index ca66d202f0e2..6177c1a8d56e 100644
> --- a/drivers/spi/spi-axi-spi-engine.c
> +++ b/drivers/spi/spi-axi-spi-engine.c
> @@ -109,6 +109,7 @@ struct spi_engine {
> spinlock_t lock;
>
> void __iomem *base;
> + struct spi_engine_message_state msg_state;
> struct completion msg_complete;
> unsigned int int_enable;
> };
> @@ -499,17 +500,11 @@ static irqreturn_t spi_engine_irq(int irq, void *devid)
> return IRQ_HANDLED;
> }
>
> -static int spi_engine_prepare_message(struct spi_controller *host,
> - struct spi_message *msg)
> +static int spi_engine_optimize_message(struct spi_message *msg)
> {
> struct spi_engine_program p_dry, *p;
> - struct spi_engine_message_state *st;
> size_t size;
>
> - st = kzalloc(sizeof(*st), GFP_KERNEL);
> - if (!st)
> - return -ENOMEM;
> -
> spi_engine_precompile_message(msg);
>
> p_dry.length = 0;
> @@ -517,31 +512,22 @@ static int spi_engine_prepare_message(struct spi_controller
> *host,
>
> size = sizeof(*p->instructions) * (p_dry.length + 1);
> p = kzalloc(sizeof(*p) + size, GFP_KERNEL);
> - if (!p) {
> - kfree(st);
> + if (!p)
> return -ENOMEM;
> - }
>
> spi_engine_compile_message(msg, false, p);
>
> spi_engine_program_add_cmd(p, false, SPI_ENGINE_CMD_SYNC(
> AXI_SPI_ENGINE_CUR_MSG_SYNC_ID));
>
> - st->p = p;
> - st->cmd_buf = p->instructions;
> - st->cmd_length = p->length;
> - msg->state = st;
> + msg->opt_state = p;
>
> return 0;
> }
>
> -static int spi_engine_unprepare_message(struct spi_controller *host,
> - struct spi_message *msg)
> +static int spi_engine_unoptimize_message(struct spi_message *msg)
> {
> - struct spi_engine_message_state *st = msg->state;
> -
> - kfree(st->p);
> - kfree(st);
> + kfree(msg->opt_state);
>
> return 0;
> }
> @@ -550,10 +536,18 @@ static int spi_engine_transfer_one_message(struct
> spi_controller *host,
> struct spi_message *msg)
> {
> struct spi_engine *spi_engine = spi_controller_get_devdata(host);
> - struct spi_engine_message_state *st = msg->state;
> + struct spi_engine_message_state *st = &spi_engine->msg_state;
> + struct spi_engine_program *p = msg->opt_state;
> unsigned int int_enable = 0;
> unsigned long flags;
>
> + /* reinitialize message state for this transfer */
> + memset(st, 0, sizeof(*st));
> + st->p = p;
> + st->cmd_buf = p->instructions;
> + st->cmd_length = p->length;
> + msg->state = st;
> +
> reinit_completion(&spi_engine->msg_complete);
>
> spin_lock_irqsave(&spi_engine->lock, flags);
> @@ -658,8 +652,8 @@ static int spi_engine_probe(struct platform_device *pdev)
> host->bits_per_word_mask = SPI_BPW_RANGE_MASK(1, 32);
> host->max_speed_hz = clk_get_rate(spi_engine->ref_clk) / 2;
> host->transfer_one_message = spi_engine_transfer_one_message;
> - host->prepare_message = spi_engine_prepare_message;
> - host->unprepare_message = spi_engine_unprepare_message;
> + host->optimize_message = spi_engine_optimize_message;
> + host->unoptimize_message = spi_engine_unoptimize_message;
> host->num_chipselect = 8;
>
> if (host->max_speed_hz == 0)
>
Powered by blists - more mailing lists