Re: [U-Boot] [PATCH v3 04/11] spi: cadence_qspi: Use #define for bits instead of bit shifts

On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy phil.edworthy@renesas.com wrote:

Most of the code already uses #defines for the bit value, rather than the shift required to get the value. This changes the remaining code over.

Whislt at it, fix the names of the "Rd Data Capture" register defs.

Signed-off-by: Phil Edworthy phil.edworthy@renesas.com Acked-by: Marek Vasut marek.vasut@gmail.com

---

v3:

• Remove brackets that aren't needed.

v2:

• No change.

---

drivers/spi/cadence_qspi_apb.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index 0a2963d..b41f36b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -57,9 +57,9 @@

• Controller's configuration and status register (offset from QSPI_BASE)

****************************************************************************/ #define CQSPI_REG_CONFIG 0x00 -#define CQSPI_REG_CONFIG_CLK_POL_LSB 1 -#define CQSPI_REG_CONFIG_CLK_PHA_LSB 2 #define CQSPI_REG_CONFIG_ENABLE_MASK BIT(0) +#define CQSPI_REG_CONFIG_CLK_POL BIT(1) +#define CQSPI_REG_CONFIG_CLK_PHA BIT(2) #define CQSPI_REG_CONFIG_DIRECT_MASK BIT(7) #define CQSPI_REG_CONFIG_DECODE_MASK BIT(9) #define CQSPI_REG_CONFIG_XIP_IMM_MASK BIT(18) @@ -94,10 +94,10 @@ #define CQSPI_REG_DELAY_TSD2D_MASK 0xFF #define CQSPI_REG_DELAY_TSHSL_MASK 0xFF

-#define CQSPI_READLCAPTURE 0x10 -#define CQSPI_READLCAPTURE_BYPASS_LSB 0 -#define CQSPI_READLCAPTURE_DELAY_LSB 1 -#define CQSPI_READLCAPTURE_DELAY_MASK 0xF +#define CQSPI_REG_RD_DATA_CAPTURE 0x10 +#define CQSPI_REG_RD_DATA_CAPTURE_BYPASS BIT(0) +#define CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB 1 +#define CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK 0xF

#define CQSPI_REG_SIZE 0x14 #define CQSPI_REG_SIZE_ADDRESS_LSB 0 @@ -244,20 +244,20 @@ void cadence_qspi_apb_readdata_capture(void *reg_base, unsigned int reg; cadence_qspi_apb_controller_disable(reg_base);

•   reg = readl(reg_base + CQSPI_READLCAPTURE);
  

•   reg = readl(reg_base + CQSPI_REG_RD_DATA_CAPTURE);
  
    if (bypass)
  

•           reg |= (1 << CQSPI_READLCAPTURE_BYPASS_LSB);
  

•           reg |= CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
    else
  

•           reg &= ~(1 << CQSPI_READLCAPTURE_BYPASS_LSB);
  

•           reg &= ~CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
  
  

•   reg &= ~(CQSPI_READLCAPTURE_DELAY_MASK
  

•           << CQSPI_READLCAPTURE_DELAY_LSB);
  

•   reg &= ~(CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK
  

•           << CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB);
  
  

•   reg |= ((delay & CQSPI_READLCAPTURE_DELAY_MASK)
  

•           << CQSPI_READLCAPTURE_DELAY_LSB);
  

•   reg |= (delay & CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK)
  

•           << CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB;
  
  

•   writel(reg, reg_base + CQSPI_READLCAPTURE);
  

•   writel(reg, reg_base + CQSPI_REG_RD_DATA_CAPTURE);
  
    cadence_qspi_apb_controller_enable(reg_base);
    return;
  

@@ -301,11 +301,12 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base,

    cadence_qspi_apb_controller_disable(reg_base);
    reg = readl(reg_base + CQSPI_REG_CONFIG);

•   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_POL_LSB);
  

•   reg &= ~(1 << CQSPI_REG_CONFIG_CLK_PHA_LSB);
  

•   reg &= ~(CQSPI_REG_CONFIG_CLK_POL | CQSPI_REG_CONFIG_CLK_PHA);
  
  

•   reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
  

•   reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
  

•   if (clk_pol)
  

•           reg |= CQSPI_REG_CONFIG_CLK_POL;
  

•   if (clk_pha)
  

•           reg |= CQSPI_REG_CONFIG_CLK_PHA;
  

I've commented about this change [1]?

[1] https://www.mail-archive.com/u-boot@lists.denx.de/msg232120.html

thanks!