[U-Boot] [PATCH v2 1/8] spi: cadence_qspi: Fix clearing of pol/pha bits

[Jagan Teki]

On Fri, Nov 25, 2016 at 8:08 PM, Phil Edworthy
<phil.edworthy at renesas.com> wrote:
> Or'ing together bit positions is clearly wrong.
>
> Signed-off-by: Phil Edworthy <phil.edworthy at renesas.com>
> ---
>  drivers/spi/cadence_qspi_apb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index e285d3c..2403e71 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -311,8 +311,8 @@ 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 | CQSPI_REG_CONFIG_CLK_PHA_LSB));
> +       reg &= ~(1 << CQSPI_REG_CONFIG_CLK_POL_LSB);
> +       reg &= ~(1 << CQSPI_REG_CONFIG_CLK_PHA_LSB);

OK, but see below

>
>         reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
>         reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);

This existing code look not easy to understand for me with doing & and
<< operations with numeric 0x1. what about this

Say for example POL and PHA on cadence has with bit 1 and 2
CQSPI_REG_CONFIG_CLK_POL_LSB   BIT(1)
CQSPI_REG_CONFIG_CLK_PHA_LSB   BIT(2)

reg &= ~(CQSPI_REG_CONFIG_CLK_PHA_LSB | CQSPI_REG_CONFIG_CLK_POL_LSB);

if (mode & SPI_CPHA)
     reg |= CQSPI_REG_CONFIG_CLK_PHA_LSB;
if (mode & SPI_CPOL)
     reg |= CQSPI_REG_CONFIG_CLK_POL_LSB;


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