diff mbox series

[08/10] spi: rzv2m-csi: Improve data types and alignment

Message ID 20230715010407.1751715-9-fabrizio.castro.jz@renesas.com
State New
Headers
Series spi: rzv2m-csi: Code refactoring |

Commit Message

Fabrizio Castro July 15, 2023, 1:04 a.m. UTC 
  "unsigned int" is more appropriate than "int" for the members
of "struct rzv2m_csi_priv".
Also, members "bytes_per_word" and "errors" introduce gaps
in the structure.
Adjust "struct rzv2m_csi_priv" and its members usage accordingly.
While at it, remove the unnecessary casting of "data" to
"struct rzv2m_csi_priv*" in function "rzv2m_csi_irq_handler".

Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 drivers/spi/spi-rzv2m-csi.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Comments

Geert Uytterhoeven July 17, 2023, 9:44 a.m. UTC | #1 
Hi Fabrizio,

Thanks for your patch!

On Sat, Jul 15, 2023 at 3:04 AM Fabrizio Castro
<fabrizio.castro.jz@renesas.com> wrote:
> "unsigned int" is more appropriate than "int" for the members
> of "struct rzv2m_csi_priv".

Agreed.

> Also, members "bytes_per_word" and "errors" introduce gaps
> in the structure.

While enlarging the types does get rid of the gaps, that was not the
intent of my comment ;-)
You can reorder fields to avoid gaps, and reduce the size of the structure.

> Adjust "struct rzv2m_csi_priv" and its members usage accordingly.
> While at it, remove the unnecessary casting of "data" to
> "struct rzv2m_csi_priv*" in function "rzv2m_csi_irq_handler".
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>

> --- a/drivers/spi/spi-rzv2m-csi.c
> +++ b/drivers/spi/spi-rzv2m-csi.c

> @@ -88,14 +88,14 @@ struct rzv2m_csi_priv {
>         struct spi_controller *controller;
>         const void *txbuf;
>         void *rxbuf;
> -       int buffer_len;
> -       int bytes_sent;
> -       int bytes_received;
> -       int bytes_to_transfer;
> -       int words_to_transfer;
> -       unsigned char bytes_per_word;
> +       unsigned int buffer_len;
> +       unsigned int bytes_sent;
> +       unsigned int bytes_received;
> +       unsigned int bytes_to_transfer;
> +       unsigned int words_to_transfer;
> +       unsigned int bytes_per_word;

bytes_per_word is calculated from spi_transfer.bits_per_word,
so u8 was fine.

>         wait_queue_head_t wait;
> -       u8 errors;
> +       u32 errors;

u8 was sufficiently large to hold all possible values.

>         u32 status;
>  };
>

Anyway, the code should work fine, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/drivers/spi/spi-rzv2m-csi.c b/drivers/spi/spi-rzv2m-csi.c
index d0d6b183ffaf..1e5ed1089f42 100644
--- a/drivers/spi/spi-rzv2m-csi.c
+++ b/drivers/spi/spi-rzv2m-csi.c
@@ -63,8 +63,8 @@ 
 /* CSI_FIFOTRG */
 #define CSI_FIFOTRG_R_TRG       GENMASK(2, 0)
 
-#define CSI_FIFO_SIZE_BYTES	32
-#define CSI_FIFO_HALF_SIZE	16
+#define CSI_FIFO_SIZE_BYTES	32U
+#define CSI_FIFO_HALF_SIZE	16U
 #define CSI_EN_DIS_TIMEOUT_US	100
 /*
  * Clock "csiclk" gets divided by 2 * CSI_CLKSEL_CKS in order to generate the
@@ -88,14 +88,14 @@  struct rzv2m_csi_priv {
 	struct spi_controller *controller;
 	const void *txbuf;
 	void *rxbuf;
-	int buffer_len;
-	int bytes_sent;
-	int bytes_received;
-	int bytes_to_transfer;
-	int words_to_transfer;
-	unsigned char bytes_per_word;
+	unsigned int buffer_len;
+	unsigned int bytes_sent;
+	unsigned int bytes_received;
+	unsigned int bytes_to_transfer;
+	unsigned int words_to_transfer;
+	unsigned int bytes_per_word;
 	wait_queue_head_t wait;
-	u8 errors;
+	u32 errors;
 	u32 status;
 };
 
@@ -193,9 +193,9 @@  static int rzv2m_csi_read_rxfifo(struct rzv2m_csi_priv *csi)
 
 static inline void rzv2m_csi_calc_current_transfer(struct rzv2m_csi_priv *csi)
 {
-	int bytes_transferred = max_t(int, csi->bytes_received, csi->bytes_sent);
-	int bytes_remaining = csi->buffer_len - bytes_transferred;
-	int to_transfer;
+	unsigned int bytes_transferred = max(csi->bytes_received, csi->bytes_sent);
+	unsigned int bytes_remaining = csi->buffer_len - bytes_transferred;
+	unsigned int to_transfer;
 
 	if (csi->txbuf)
 		/*
@@ -203,9 +203,9 @@  static inline void rzv2m_csi_calc_current_transfer(struct rzv2m_csi_priv *csi)
 		 * hard to raise an overflow error (which is only possible
 		 * when IP transmits and receives at the same time).
 		 */
-		to_transfer = min_t(int, CSI_FIFO_HALF_SIZE, bytes_remaining);
+		to_transfer = min(CSI_FIFO_HALF_SIZE, bytes_remaining);
 	else
-		to_transfer = min_t(int, CSI_FIFO_SIZE_BYTES, bytes_remaining);
+		to_transfer = min(CSI_FIFO_SIZE_BYTES, bytes_remaining);
 
 	if (csi->bytes_per_word == 2)
 		to_transfer >>= 1;
@@ -325,7 +325,7 @@  static inline int rzv2m_csi_wait_for_rx_ready(struct rzv2m_csi_priv *csi)
 
 static irqreturn_t rzv2m_csi_irq_handler(int irq, void *data)
 {
-	struct rzv2m_csi_priv *csi = (struct rzv2m_csi_priv *)data;
+	struct rzv2m_csi_priv *csi = data;
 
 	csi->status = readl(csi->base + CSI_INT);
 	rzv2m_csi_disable_irqs(csi, csi->status);