diff mbox series

[06/10] spi: rzv2m-csi: Squash timing settings into one statement

Message ID 20230715010407.1751715-7-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 
  Register CLKSEL hosts the configuration for both clock polarity
and data phase, and both values can be set in one write operation.

Squash the clock polarity and data phase register writes into
one statement, for efficiency.

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

Comments

Andy Shevchenko July 15, 2023, 7:54 a.m. UTC | #1 
On Sat, Jul 15, 2023 at 4:04 AM Fabrizio Castro
<fabrizio.castro.jz@renesas.com> wrote:
>
> Register CLKSEL hosts the configuration for both clock polarity
> and data phase, and both values can be set in one write operation.
>
> Squash the clock polarity and data phase register writes into
> one statement, for efficiency.

...

>         /* Setup clock polarity and phase timing */
> -       rzv2m_csi_reg_write_bit(csi, CSI_CLKSEL, CSI_CLKSEL_CKP,
> -                               !(spi->mode & SPI_CPOL));
> -       rzv2m_csi_reg_write_bit(csi, CSI_CLKSEL, CSI_CLKSEL_DAP,
> -                               !(spi->mode & SPI_CPHA));
> +       rzv2m_csi_reg_write_bit(csi, CSI_CLKSEL, CSI_CLKSEL_MODE,
> +                               ~spi->mode & SPI_MODE_X_MASK);

I think this now regresses due to the absence of parentheses.
Fabrizio Castro July 17, 2023, 10:44 a.m. UTC | #2 
Hi Andy,

Thanks for your reply.

> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> Subject: Re: [PATCH 06/10] spi: rzv2m-csi: Squash timing settings into
> one statement
> 
> On Sat, Jul 15, 2023 at 4:04 AM Fabrizio Castro
> <fabrizio.castro.jz@renesas.com> wrote:
> >
> > Register CLKSEL hosts the configuration for both clock polarity
> > and data phase, and both values can be set in one write operation.
> >
> > Squash the clock polarity and data phase register writes into
> > one statement, for efficiency.
> 
> ...
> 
> >         /* Setup clock polarity and phase timing */
> > -       rzv2m_csi_reg_write_bit(csi, CSI_CLKSEL, CSI_CLKSEL_CKP,
> > -                               !(spi->mode & SPI_CPOL));
> > -       rzv2m_csi_reg_write_bit(csi, CSI_CLKSEL, CSI_CLKSEL_DAP,
> > -                               !(spi->mode & SPI_CPHA));
> > +       rzv2m_csi_reg_write_bit(csi, CSI_CLKSEL, CSI_CLKSEL_MODE,
> > +                               ~spi->mode & SPI_MODE_X_MASK);
> 
> I think this now regresses due to the absence of parentheses.

This looks okay to me. CSI_CLKSEL_CKP needs to contain the inverted value
of SPI_CPOL, and CSI_CLKSEL_DAP needs to contain the inverted value of
SPI_CPHA, and that happens with both use cases.

Thanks,
Fab

> 
> --
> With Best Regards,
> Andy Shevchenko
Andy Shevchenko July 17, 2023, 11:15 a.m. UTC | #3 
On Mon, Jul 17, 2023 at 1:44 PM Fabrizio Castro
<fabrizio.castro.jz@renesas.com> wrote:
> > From: Andy Shevchenko <andy.shevchenko@gmail.com>
> > On Sat, Jul 15, 2023 at 4:04 AM Fabrizio Castro
> > <fabrizio.castro.jz@renesas.com> wrote:

...

> > >         /* Setup clock polarity and phase timing */
> > > -       rzv2m_csi_reg_write_bit(csi, CSI_CLKSEL, CSI_CLKSEL_CKP,
> > > -                               !(spi->mode & SPI_CPOL));
> > > -       rzv2m_csi_reg_write_bit(csi, CSI_CLKSEL, CSI_CLKSEL_DAP,
> > > -                               !(spi->mode & SPI_CPHA));
> > > +       rzv2m_csi_reg_write_bit(csi, CSI_CLKSEL, CSI_CLKSEL_MODE,
> > > +                               ~spi->mode & SPI_MODE_X_MASK);
> >
> > I think this now regresses due to the absence of parentheses.
>
> This looks okay to me. CSI_CLKSEL_CKP needs to contain the inverted value
> of SPI_CPOL, and CSI_CLKSEL_DAP needs to contain the inverted value of
> SPI_CPHA, and that happens with both use cases.

Ah, this is interchangeable since we will get the same bits in the end, indeed.
diff mbox series

Patch

diff --git a/drivers/spi/spi-rzv2m-csi.c b/drivers/spi/spi-rzv2m-csi.c
index 038f1486b7d7..faf5898bc3e0 100644
--- a/drivers/spi/spi-rzv2m-csi.c
+++ b/drivers/spi/spi-rzv2m-csi.c
@@ -38,6 +38,7 @@ 
 /* CSI_CLKSEL */
 #define CSI_CLKSEL_CKP		BIT(17)
 #define CSI_CLKSEL_DAP		BIT(16)
+#define CSI_CLKSEL_MODE		(CSI_CLKSEL_CKP|CSI_CLKSEL_DAP)
 #define CSI_CLKSEL_SLAVE	BIT(15)
 #define CSI_CLKSEL_CKS		GENMASK(14, 1)
 
@@ -408,10 +409,8 @@  static int rzv2m_csi_setup(struct spi_device *spi)
 	writel(CSI_MODE_SETUP, csi->base + CSI_MODE);
 
 	/* Setup clock polarity and phase timing */
-	rzv2m_csi_reg_write_bit(csi, CSI_CLKSEL, CSI_CLKSEL_CKP,
-				!(spi->mode & SPI_CPOL));
-	rzv2m_csi_reg_write_bit(csi, CSI_CLKSEL, CSI_CLKSEL_DAP,
-				!(spi->mode & SPI_CPHA));
+	rzv2m_csi_reg_write_bit(csi, CSI_CLKSEL, CSI_CLKSEL_MODE,
+				~spi->mode & SPI_MODE_X_MASK);
 
 	/* Setup serial data order */
 	rzv2m_csi_reg_write_bit(csi, CSI_MODE, CSI_MODE_DIR,