[PATCH v2 0/3] spi: axi-spi-engine: small cleanups

[PATCH v2 0/3] spi: axi-spi-engine: small cleanups

[David Lechner]

This series contains a few small cleanups to the axi-spi-engine driver,
mostly suggested from previous reviews.

---
Changes in v2:
- Picked up the Reviewed-by trailers.
- Modified second patch to reorder assignment of the length field.
- Link to v1: https://lore.kernel.org/r/20240301-mainline-axi-spi-engine-small-cleanups-v1-0-241dfd2a79f7@baylibre.com

---
David Lechner (3):
      spi: axi-spi-engine: remove p from struct spi_engine_message_state
      spi: axi-spi-engine: use __counted_by() attribute
      spi: axi-spi-engine: use struct_size() macro

 drivers/spi/spi-axi-spi-engine.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)
---
base-commit: bf790d87088a04d5f3a4659e04ff2a5a16eca294
change-id: 20240301-mainline-axi-spi-engine-small-cleanups-cd08b51cb6d4

Best regards,
-- 
David Lechner <dlechner@baylibre.com>

[PATCH v2 1/3] spi: axi-spi-engine: remove p from struct spi_engine_message_state

[David Lechner]

The program pointer p in struct spi_engine_message_state in the AXI SPI
Engine controller driver was assigned but never read so it can be
removed.

Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/spi/spi-axi-spi-engine.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index 6177c1a8d56e..d89f75170c9e 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -82,8 +82,6 @@ struct spi_engine_program {
  * struct spi_engine_message_state - SPI engine per-message state
  */
 struct spi_engine_message_state {
-	/** @p: Instructions for executing this message. */
-	struct spi_engine_program *p;
 	/** @cmd_length: Number of elements in cmd_buf array. */
 	unsigned cmd_length;
 	/** @cmd_buf: Array of commands not yet written to CMD FIFO. */
@@ -543,7 +541,6 @@ static int spi_engine_transfer_one_message(struct spi_controller *host,
 
 	/* 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;

-- 
2.43.2

[PATCH v2 2/3] spi: axi-spi-engine: use __counted_by() attribute

[David Lechner]

This adds the __counted_by() attribute to the flex array at the end of
struct spi_engine_program in the AXI SPI Engine controller driver.

The assignment of the length field has to be reordered to be before
the access to the flex array in order to avoid potential compiler
warnings/errors due to adding the __counted_by() attribute.

Suggested-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
v2 changes:
* Reordered assignment of length field.
---
 drivers/spi/spi-axi-spi-engine.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index d89f75170c9e..a8f626165f44 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -75,7 +75,7 @@
 
 struct spi_engine_program {
 	unsigned int length;
-	uint16_t instructions[];
+	uint16_t instructions[] __counted_by(length);
 };
 
 /**
@@ -115,9 +115,10 @@ struct spi_engine {
 static void spi_engine_program_add_cmd(struct spi_engine_program *p,
 	bool dry, uint16_t cmd)
 {
-	if (!dry)
-		p->instructions[p->length] = cmd;
 	p->length++;
+
+	if (!dry)
+		p->instructions[p->length - 1] = cmd;
 }
 
 static unsigned int spi_engine_get_config(struct spi_device *spi)

-- 
2.43.2

[PATCH v2 3/3] spi: axi-spi-engine: use struct_size() macro

[David Lechner]

This makes use of the struct_size() macro to calculate the size of the
struct axi_spi_engine when allocating it.

Suggested-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/spi/spi-axi-spi-engine.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index a8f626165f44..7cc219d78551 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -12,6 +12,7 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/module.h>
+#include <linux/overflow.h>
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
 
@@ -502,15 +503,13 @@ static irqreturn_t spi_engine_irq(int irq, void *devid)
 static int spi_engine_optimize_message(struct spi_message *msg)
 {
 	struct spi_engine_program p_dry, *p;
-	size_t size;
 
 	spi_engine_precompile_message(msg);
 
 	p_dry.length = 0;
 	spi_engine_compile_message(msg, true, &p_dry);
 
-	size = sizeof(*p->instructions) * (p_dry.length + 1);
-	p = kzalloc(sizeof(*p) + size, GFP_KERNEL);
+	p = kzalloc(struct_size(p, instructions, p_dry.length + 1), GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
 

-- 
2.43.2

Re: [PATCH v2 1/3] spi: axi-spi-engine: remove p from struct spi_engine_message_state

[Gustavo A. R. Silva]

On 04/03/24 10:04, David Lechner wrote:
> The program pointer p in struct spi_engine_message_state in the AXI SPI
> Engine controller driver was assigned but never read so it can be
> removed.
> 
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Lechner <dlechner@baylibre.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
--
Gustavo

> ---
>   drivers/spi/spi-axi-spi-engine.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
> index 6177c1a8d56e..d89f75170c9e 100644
> --- a/drivers/spi/spi-axi-spi-engine.c
> +++ b/drivers/spi/spi-axi-spi-engine.c
> @@ -82,8 +82,6 @@ struct spi_engine_program {
>    * struct spi_engine_message_state - SPI engine per-message state
>    */
>   struct spi_engine_message_state {
> -	/** @p: Instructions for executing this message. */
> -	struct spi_engine_program *p;
>   	/** @cmd_length: Number of elements in cmd_buf array. */
>   	unsigned cmd_length;
>   	/** @cmd_buf: Array of commands not yet written to CMD FIFO. */
> @@ -543,7 +541,6 @@ static int spi_engine_transfer_one_message(struct spi_controller *host,
>   
>   	/* 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;
> 

Re: [PATCH v2 2/3] spi: axi-spi-engine: use __counted_by() attribute

[Gustavo A. R. Silva]

On 04/03/24 10:04, David Lechner wrote:
> This adds the __counted_by() attribute to the flex array at the end of
> struct spi_engine_program in the AXI SPI Engine controller driver.
> 
> The assignment of the length field has to be reordered to be before
> the access to the flex array in order to avoid potential compiler
> warnings/errors due to adding the __counted_by() attribute.
> 
> Suggested-by: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: David Lechner <dlechner@baylibre.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
--
Gustavo

> ---
> v2 changes:
> * Reordered assignment of length field.
> ---
>   drivers/spi/spi-axi-spi-engine.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
> index d89f75170c9e..a8f626165f44 100644
> --- a/drivers/spi/spi-axi-spi-engine.c
> +++ b/drivers/spi/spi-axi-spi-engine.c
> @@ -75,7 +75,7 @@
>   
>   struct spi_engine_program {
>   	unsigned int length;
> -	uint16_t instructions[];
> +	uint16_t instructions[] __counted_by(length);
>   };
>   
>   /**
> @@ -115,9 +115,10 @@ struct spi_engine {
>   static void spi_engine_program_add_cmd(struct spi_engine_program *p,
>   	bool dry, uint16_t cmd)
>   {
> -	if (!dry)
> -		p->instructions[p->length] = cmd;
>   	p->length++;
> +
> +	if (!dry)
> +		p->instructions[p->length - 1] = cmd;
>   }
>   
>   static unsigned int spi_engine_get_config(struct spi_device *spi)
> 

Re: [PATCH v2 3/3] spi: axi-spi-engine: use struct_size() macro

[Gustavo A. R. Silva]

On 04/03/24 10:04, David Lechner wrote:
> This makes use of the struct_size() macro to calculate the size of the
> struct axi_spi_engine when allocating it.
> 
> Suggested-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Lechner <dlechner@baylibre.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
--
Gustavo

> ---
>   drivers/spi/spi-axi-spi-engine.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
> index a8f626165f44..7cc219d78551 100644
> --- a/drivers/spi/spi-axi-spi-engine.c
> +++ b/drivers/spi/spi-axi-spi-engine.c
> @@ -12,6 +12,7 @@
>   #include <linux/io.h>
>   #include <linux/of.h>
>   #include <linux/module.h>
> +#include <linux/overflow.h>
>   #include <linux/platform_device.h>
>   #include <linux/spi/spi.h>
>   
> @@ -502,15 +503,13 @@ static irqreturn_t spi_engine_irq(int irq, void *devid)
>   static int spi_engine_optimize_message(struct spi_message *msg)
>   {
>   	struct spi_engine_program p_dry, *p;
> -	size_t size;
>   
>   	spi_engine_precompile_message(msg);
>   
>   	p_dry.length = 0;
>   	spi_engine_compile_message(msg, true, &p_dry);
>   
> -	size = sizeof(*p->instructions) * (p_dry.length + 1);
> -	p = kzalloc(sizeof(*p) + size, GFP_KERNEL);
> +	p = kzalloc(struct_size(p, instructions, p_dry.length + 1), GFP_KERNEL);
>   	if (!p)
>   		return -ENOMEM;
>   
> 

Re: [PATCH v2 2/3] spi: axi-spi-engine: use __counted_by() attribute

[Kees Cook]

On Mon, Mar 04, 2024 at 10:04:24AM -0600, David Lechner wrote:
> This adds the __counted_by() attribute to the flex array at the end of
> struct spi_engine_program in the AXI SPI Engine controller driver.
> 
> The assignment of the length field has to be reordered to be before
> the access to the flex array in order to avoid potential compiler
> warnings/errors due to adding the __counted_by() attribute.
> 
> Suggested-by: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: David Lechner <dlechner@baylibre.com>

Looks good! Thanks for the respin.

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH v2 0/3] spi: axi-spi-engine: small cleanups

[Mark Brown]

On Mon, 04 Mar 2024 10:04:22 -0600, David Lechner wrote:
> This series contains a few small cleanups to the axi-spi-engine driver,
> mostly suggested from previous reviews.
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/3] spi: axi-spi-engine: remove p from struct spi_engine_message_state
      commit: 69d54ee2e5b0dab9350be2a7019c472b9b8d4c14
[2/3] spi: axi-spi-engine: use __counted_by() attribute
      commit: c8340ac1015471ec5af234beff535efe15f382e9
[3/3] spi: axi-spi-engine: use struct_size() macro
      commit: 5c708541301e695b611cb0b9c6d732bed9b5d904

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark