Hi Marek, On 25 November 2016 15:00 Marek Vasut wrote: > 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.edwor...@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. Ok, will do.
> > > > - 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.va...@gmail.com> > > > writel(reg, reg_base + CQSPI_REG_CONFIG); > > > > > > > -- > Best regards, > Marek Vasut Thanks Phil _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
