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

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

2016-12-02 Thread Jagan Teki

On Wed, Nov 30, 2016 at 1:02 PM, Phil Edworthy
 wrote:
> Hi Jagan,
>
> On 30 November 2016 04:59, Jagan Teki wrote:
>> On Tue, Nov 29, 2016 at 6:28 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 
>> > Acked-by: Marek Vasut 



>>
>> I've commented about this change [1]?
>>
>> [1] https://www.mail-archive.com/u-boot@lists.denx.de/msg232120.html
> Yes, I fixed that in a separate patch, see
> https://www.mail-archive.com/u-boot@lists.denx.de/msg232489.html
>
> I did it in a separate patch because this patch is purely about replacing the
> definitions of bit shifts with BIT(x).

Reviewed-by: Jagan Teki 

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-11-29 Thread Phil Edworthy

Hi Jagan,

On 30 November 2016 04:59, Jagan Teki wrote:
> On Tue, Nov 29, 2016 at 6:28 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 
> > Acked-by: Marek Vasut 
> > ---
> >  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)
> >
> **
> **/
> >  #defineCQSPI_REG_CONFIG0x00
> > -#defineCQSPI_REG_CONFIG_CLK_POL_LSB1
> > -#defineCQSPI_REG_CONFIG_CLK_PHA_LSB2
> >  #defineCQSPI_REG_CONFIG_ENABLE_MASKBIT(0)
> > +#defineCQSPI_REG_CONFIG_CLK_POLBIT(1)
> > +#defineCQSPI_REG_CONFIG_CLK_PHABIT(2)
> >  #defineCQSPI_REG_CONFIG_DIRECT_MASKBIT(7)
> >  #defineCQSPI_REG_CONFIG_DECODE_MASKBIT(9)
> >  #defineCQSPI_REG_CONFIG_XIP_IMM_MASK   BIT(18)
> > @@ -94,10 +94,10 @@
> >  #defineCQSPI_REG_DELAY_TSD2D_MASK  0xFF
> >  #defineCQSPI_REG_DELAY_TSHSL_MASK  0xFF
> >
> > -#defineCQSPI_READLCAPTURE  0x10
> > -#defineCQSPI_READLCAPTURE_BYPASS_LSB   0
> > -#defineCQSPI_READLCAPTURE_DELAY_LSB1
> > -#defineCQSPI_READLCAPTURE_DELAY_MASK   0xF
> > +#defineCQSPI_REG_RD_DATA_CAPTURE   0x10
> > +#defineCQSPI_REG_RD_DATA_CAPTURE_BYPASSBIT(0)
> > +#defineCQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB 1
> > +#defineCQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK0xF
> >
> >  #defineCQSPI_REG_SIZE  0x14
> >  #defineCQSPI_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
Yes, I fixed that in a separate patch, see
https://www.mail-archive.com/u-boot@lists.denx.de/msg232489.html

I did it in a separate patch because this patch is purely about replacing the
definitions of bit shifts with BIT(x).

> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com
> U-Boot, Linux | Upstream Maintainer
> Hyderabad, India.

Thanks
Phil
___

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

2016-11-29 Thread Jagan Teki

On Tue, Nov 29, 2016 at 6:28 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 
> Acked-by: Marek Vasut 
> ---
>  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)
>   
> /
>  #defineCQSPI_REG_CONFIG0x00
> -#defineCQSPI_REG_CONFIG_CLK_POL_LSB1
> -#defineCQSPI_REG_CONFIG_CLK_PHA_LSB2
>  #defineCQSPI_REG_CONFIG_ENABLE_MASKBIT(0)
> +#defineCQSPI_REG_CONFIG_CLK_POLBIT(1)
> +#defineCQSPI_REG_CONFIG_CLK_PHABIT(2)
>  #defineCQSPI_REG_CONFIG_DIRECT_MASKBIT(7)
>  #defineCQSPI_REG_CONFIG_DECODE_MASKBIT(9)
>  #defineCQSPI_REG_CONFIG_XIP_IMM_MASK   BIT(18)
> @@ -94,10 +94,10 @@
>  #defineCQSPI_REG_DELAY_TSD2D_MASK  0xFF
>  #defineCQSPI_REG_DELAY_TSHSL_MASK  0xFF
>
> -#defineCQSPI_READLCAPTURE  0x10
> -#defineCQSPI_READLCAPTURE_BYPASS_LSB   0
> -#defineCQSPI_READLCAPTURE_DELAY_LSB1
> -#defineCQSPI_READLCAPTURE_DELAY_MASK   0xF
> +#defineCQSPI_REG_RD_DATA_CAPTURE   0x10
> +#defineCQSPI_REG_RD_DATA_CAPTURE_BYPASSBIT(0)
> +#defineCQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB 1
> +#defineCQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK0xF
>
>  #defineCQSPI_REG_SIZE  0x14
>  #defineCQSPI_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!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-11-29 Thread Phil Edworthy

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 
Acked-by: Marek Vasut 
---
 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)
  /
 #defineCQSPI_REG_CONFIG0x00
-#defineCQSPI_REG_CONFIG_CLK_POL_LSB1
-#defineCQSPI_REG_CONFIG_CLK_PHA_LSB2
 #defineCQSPI_REG_CONFIG_ENABLE_MASKBIT(0)
+#defineCQSPI_REG_CONFIG_CLK_POLBIT(1)
+#defineCQSPI_REG_CONFIG_CLK_PHABIT(2)
 #defineCQSPI_REG_CONFIG_DIRECT_MASKBIT(7)
 #defineCQSPI_REG_CONFIG_DECODE_MASKBIT(9)
 #defineCQSPI_REG_CONFIG_XIP_IMM_MASK   BIT(18)
@@ -94,10 +94,10 @@
 #defineCQSPI_REG_DELAY_TSD2D_MASK  0xFF
 #defineCQSPI_REG_DELAY_TSHSL_MASK  0xFF
 
-#defineCQSPI_READLCAPTURE  0x10
-#defineCQSPI_READLCAPTURE_BYPASS_LSB   0
-#defineCQSPI_READLCAPTURE_DELAY_LSB1
-#defineCQSPI_READLCAPTURE_DELAY_MASK   0xF
+#defineCQSPI_REG_RD_DATA_CAPTURE   0x10
+#defineCQSPI_REG_RD_DATA_CAPTURE_BYPASSBIT(0)
+#defineCQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB 1
+#defineCQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK0xF
 
 #defineCQSPI_REG_SIZE  0x14
 #defineCQSPI_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;
 
writel(reg, reg_base + CQSPI_REG_CONFIG);
 
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

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

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

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

4 matches

Site Navigation

Mail list logo

Footer information