[U-Boot] [PATCH v2 4/8] spi: cadence_qspi: Use #define for bits instead of bit shifts

[Marek Vasut]

On 11/25/2016 03:38 PM, Phil Edworthy 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 at renesas.com>
> ---
>  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 3ae4b5a..cd46a15 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);

You can also drop the () around the whole expression here.

>  
> -	writel(reg, reg_base + CQSPI_READLCAPTURE);
> +	writel(reg, reg_base + CQSPI_REG_RD_DATA_CAPTURE);
>  
>  	cadence_qspi_apb_controller_enable(reg_base);
>  	return;
> @@ -302,11 +302,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;

Except the minor nit above,

Acked-by: Marek Vasut <marek.vasut at gmail.com>

>  	writel(reg, reg_base + CQSPI_REG_CONFIG);
>  
> 


-- 
Best regards,
Marek Vasut