[U-Boot] [PATCH 1/2] spi: spi-mxc: implement clk control for ECSPI to fix SPI_MODE_3

[Jagan Teki]

On Mon, Feb 17, 2014 at 1:39 PM, Markus Niebel
<Markus.Niebel at tq-group.com> wrote:
> Hello,
>
>> Von: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>> Gesendet: Sonntag, 16. Februar 2014 17:58
>> An: Markus Niebel
>> Cc: u-boot at lists.denx.de; Markus Niebel
>> Betreff: Re: [U-Boot] [PATCH 1/2] spi: spi-mxc: implement clk control
>> for ECSPI to fix SPI_MODE_3
>>
>> On Mon, Feb 10, 2014 at 1:57 PM, Markus Niebel <list-09_u-boot at tqsc.de>
>> wrote:
>> > From: Markus Niebel <Markus.Niebel at tqs.de>
>> >
>> > SPI_MODE_3 requires clk high when inactive. The SCLK_CTL
>> > field of the config reg was not configured. Provide defines
>> > for missing fields in the ECSPI config reg and use them
>> > for SPI_CPOL
>> >
>> > Signed-off-by: Markus Niebel <Markus.Niebel at tqs.de>
>> > ---
>> >  arch/arm/include/asm/arch-mx5/imx-regs.h |    9 ++++++---
>> >  arch/arm/include/asm/arch-mx6/imx-regs.h |    9 ++++++---
>> >  drivers/spi/mxc_spi.c                    |    9 +++++++--
>> >  3 files changed, 19 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/arch/arm/include/asm/arch-mx5/imx-regs.h
>> b/arch/arm/include/asm/arch-mx5/imx-regs.h
>> > index 4955ccf..32386d3 100644
>> > --- a/arch/arm/include/asm/arch-mx5/imx-regs.h
>> > +++ b/arch/arm/include/asm/arch-mx5/imx-regs.h
>> > @@ -230,9 +230,12 @@
>> >  #define MXC_CSPICTRL_CHAN      18
>> >
>> >  /* Bit position inside CON register to be associated with SS */
>> > -#define MXC_CSPICON_POL                4
>> > -#define MXC_CSPICON_PHA                0
>> > -#define MXC_CSPICON_SSPOL      12
>> > +#define MXC_CSPICON_PHA                0  /* SCLK phase control */
>> > +#define MXC_CSPICON_POL                4  /* SCLK polarity */
>> > +#define MXC_CSPICON_SSCTL      8  /* SS wave form select */
>> > +#define MXC_CSPICON_SSPOL      12 /* SS polarity */
>> > +#define MXC_CSPICON_DCTL       16 /* inactive state of MOSI */
>> > +#define MXC_CSPICON_CTL                20 /* inactive state of SCLK
>> */
>> >  #define MXC_SPI_BASE_ADDRESSES \
>> >         CSPI1_BASE_ADDR, \
>> >         CSPI2_BASE_ADDR, \
>> > diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h
>> b/arch/arm/include/asm/arch-mx6/imx-regs.h
>> > index f2ad6e9..79842bf 100644
>> > --- a/arch/arm/include/asm/arch-mx6/imx-regs.h
>> > +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
>> > @@ -405,9 +405,12 @@ struct cspi_regs {
>> >  #define MXC_CSPICTRL_CHAN      18
>> >
>> >  /* Bit position inside CON register to be associated with SS */
>> > -#define MXC_CSPICON_POL                4
>> > -#define MXC_CSPICON_PHA                0
>> > -#define MXC_CSPICON_SSPOL      12
>> > +#define MXC_CSPICON_PHA                0  /* SCLK phase control */
>> > +#define MXC_CSPICON_POL                4  /* SCLK polarity */
>> > +#define MXC_CSPICON_SSCTL      8  /* SS wave form select */
>> > +#define MXC_CSPICON_SSPOL      12 /* SS polarity */
>> > +#define MXC_CSPICON_DCTL       16 /* inactive state of MOSI */
>> > +#define MXC_CSPICON_CTL                20 /* inactive state of SCLK
>> */
>>
>> Few of these macros not used in this patch, do you really require in
>> this change.?
>
> No, not really - but I saw this for other register related defines, too that
> bits that are actually not be used are defined - just for completeness
>
> If this is a coding style problem, I will send the patches without the unused
> Bit defines again

Better to avoid the not used stuff - then add it when ever it required.

>
>>
>> >  #ifdef CONFIG_MX6SL
>> >  #define MXC_SPI_BASE_ADDRESSES \
>> >         ECSPI1_BASE_ADDR, \
>> > diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
>> > index 95dd03f..f3f029d 100644
>> > --- a/drivers/spi/mxc_spi.c
>> > +++ b/drivers/spi/mxc_spi.c
>> > @@ -115,7 +115,8 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs,
>> unsigned int cs,
>> >  {
>> >         u32 clk_src = mxc_get_clock(MXC_CSPI_CLK);
>> >         s32 reg_ctrl, reg_config;
>> > -       u32 ss_pol = 0, sclkpol = 0, sclkpha = 0, pre_div = 0,
>> post_div = 0;
>> > +       u32 ss_pol = 0, sclkpol = 0, sclkpha = 0, sclkctl = 0;
>> > +       u32 pre_div = 0, post_div = 0;
>> >         struct cspi_regs *regs = (struct cspi_regs *)mxcs->base;
>> >
>> >         if (max_hz == 0) {
>> > @@ -164,8 +165,10 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave
>> *mxcs, unsigned int cs,
>> >         if (mode & SPI_CS_HIGH)
>> >                 ss_pol = 1;
>> >
>> > -       if (mode & SPI_CPOL)
>> > +       if (mode & SPI_CPOL) {
>> >                 sclkpol = 1;
>> > +               sclkctl = 1;
>> > +       }
>> >
>> >         if (mode & SPI_CPHA)
>> >                 sclkpha = 1;
>> > @@ -180,6 +183,8 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs,
>> unsigned int cs,
>> >                 (ss_pol << (cs + MXC_CSPICON_SSPOL));
>> >         reg_config = (reg_config & ~(1 << (cs + MXC_CSPICON_POL))) |
>> >                 (sclkpol << (cs + MXC_CSPICON_POL));
>> > +       reg_config = (reg_config & ~(1 << (cs + MXC_CSPICON_CTL))) |
>> > +               (sclkctl << (cs + MXC_CSPICON_CTL));
>> >         reg_config = (reg_config & ~(1 << (cs + MXC_CSPICON_PHA))) |
>> >                 (sclkpha << (cs + MXC_CSPICON_PHA));
>> >
>> > --
>> > 1.7.9.5
>> >
>> > _______________________________________________
>> > U-Boot mailing list
>> > U-Boot at lists.denx.de
>> > http://lists.denx.de/mailman/listinfo/u-boot
>>

thanks!
-- 
Jagan.