diff mbox

[U-Boot,V2] spi: mxs: Configure chipselect after block reset

Message ID 1377531923-5137-1-git-send-email-marex@denx.de
State Accepted
Delegated to: Jagannadha Sutradharudu Teki
Headers show

Commit Message

Marek Vasut Aug. 26, 2013, 3:45 p.m. UTC
The chipselect must be written into the CTRL0 register after the SSP
block is reset, otherwise the block will always use ChipSelect #0.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
Cc: Otavio Salvador <otavio@ossystems.com.br>
Cc: Stefano Babic <sbabic@denx.de>
---
 drivers/spi/mxs_spi.c |   12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

V2: Clean up a warning that "ssp_regs" are not used

Comments

Stefano Babic Aug. 26, 2013, 4:04 p.m. UTC | #1
On 26/08/2013 17:45, Marek Vasut wrote:
> The chipselect must be written into the CTRL0 register after the SSP
> block is reset, otherwise the block will always use ChipSelect #0.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
> Cc: Otavio Salvador <otavio@ossystems.com.br>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
>  drivers/spi/mxs_spi.c |   12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> V2: Clean up a warning that "ssp_regs" are not used
> 
> diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
> index 3cf7142..2b9f395 100644
> --- a/drivers/spi/mxs_spi.c
> +++ b/drivers/spi/mxs_spi.c
> @@ -56,8 +56,6 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>  				  unsigned int max_hz, unsigned int mode)
>  {
>  	struct mxs_spi_slave *mxs_slave;
> -	struct mxs_ssp_regs *ssp_regs;
> -	int reg;
>  
>  	if (!spi_cs_is_valid(bus, cs)) {
>  		printf("mxs_spi: invalid bus %d / chip select %d\n", bus, cs);
> @@ -74,13 +72,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>  	mxs_slave->max_khz = max_hz / 1000;
>  	mxs_slave->mode = mode;
>  	mxs_slave->regs = mxs_ssp_regs_by_bus(bus);
> -	ssp_regs = mxs_slave->regs;
>  
> -	reg = readl(&ssp_regs->hw_ssp_ctrl0);
> -	reg &= ~(MXS_SSP_CHIPSELECT_MASK);
> -	reg |= cs << MXS_SSP_CHIPSELECT_SHIFT;
> -
> -	writel(reg, &ssp_regs->hw_ssp_ctrl0);
>  	return &mxs_slave->slave;
>  
>  err_init:
> @@ -102,7 +94,9 @@ int spi_claim_bus(struct spi_slave *slave)
>  
>  	mxs_reset_block(&ssp_regs->hw_ssp_ctrl0_reg);
>  
> -	writel(SSP_CTRL0_BUS_WIDTH_ONE_BIT, &ssp_regs->hw_ssp_ctrl0);
> +	writel((slave->cs << MXS_SSP_CHIPSELECT_SHIFT) |
> +	       SSP_CTRL0_BUS_WIDTH_ONE_BIT,
> +	       &ssp_regs->hw_ssp_ctrl0);
>  
>  	reg = SSP_CTRL1_SSP_MODE_SPI | SSP_CTRL1_WORD_LENGTH_EIGHT_BITS;
>  	reg |= (mxs_slave->mode & SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;
> 

Acked-by: Stefano Babic <sbabic@denx.de>

Best regards,
Stefano Babic
Marek Vasut Aug. 27, 2013, 9:05 a.m. UTC | #2
Dear Stefano Babic,

> On 26/08/2013 17:45, Marek Vasut wrote:
> > The chipselect must be written into the CTRL0 register after the SSP
> > block is reset, otherwise the block will always use ChipSelect #0.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Fabio Estevam <fabio.estevam@freescale.com>
> > Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
> > Cc: Otavio Salvador <otavio@ossystems.com.br>
> > Cc: Stefano Babic <sbabic@denx.de>
> > ---
> > 
> >  drivers/spi/mxs_spi.c |   12 +++---------
> >  1 file changed, 3 insertions(+), 9 deletions(-)
> > 
> > V2: Clean up a warning that "ssp_regs" are not used
> > 
> > diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
> > index 3cf7142..2b9f395 100644
> > --- a/drivers/spi/mxs_spi.c
> > +++ b/drivers/spi/mxs_spi.c
> > @@ -56,8 +56,6 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
> > unsigned int cs,
> > 
> >  				  unsigned int max_hz, unsigned int mode)
> >  
> >  {
> >  
> >  	struct mxs_spi_slave *mxs_slave;
> > 
> > -	struct mxs_ssp_regs *ssp_regs;
> > -	int reg;
> > 
> >  	if (!spi_cs_is_valid(bus, cs)) {
> >  	
> >  		printf("mxs_spi: invalid bus %d / chip select %d\n", bus, cs);
> > 
> > @@ -74,13 +72,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
> > unsigned int cs,
> > 
> >  	mxs_slave->max_khz = max_hz / 1000;
> >  	mxs_slave->mode = mode;
> >  	mxs_slave->regs = mxs_ssp_regs_by_bus(bus);
> > 
> > -	ssp_regs = mxs_slave->regs;
> > 
> > -	reg = readl(&ssp_regs->hw_ssp_ctrl0);
> > -	reg &= ~(MXS_SSP_CHIPSELECT_MASK);
> > -	reg |= cs << MXS_SSP_CHIPSELECT_SHIFT;
> > -
> > -	writel(reg, &ssp_regs->hw_ssp_ctrl0);
> > 
> >  	return &mxs_slave->slave;
> >  
> >  err_init:
> > @@ -102,7 +94,9 @@ int spi_claim_bus(struct spi_slave *slave)
> > 
> >  	mxs_reset_block(&ssp_regs->hw_ssp_ctrl0_reg);
> > 
> > -	writel(SSP_CTRL0_BUS_WIDTH_ONE_BIT, &ssp_regs->hw_ssp_ctrl0);
> > +	writel((slave->cs << MXS_SSP_CHIPSELECT_SHIFT) |
> > +	       SSP_CTRL0_BUS_WIDTH_ONE_BIT,
> > +	       &ssp_regs->hw_ssp_ctrl0);
> > 
> >  	reg = SSP_CTRL1_SSP_MODE_SPI | SSP_CTRL1_WORD_LENGTH_EIGHT_BITS;
> >  	reg |= (mxs_slave->mode & SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;
> 
> Acked-by: Stefano Babic <sbabic@denx.de>

Can we push this via IMX tree ?

Best regards,
Marek Vasut
Jagan Teki Aug. 27, 2013, 9:08 a.m. UTC | #3
Hi,

On Tue, Aug 27, 2013 at 2:35 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Stefano Babic,
>
>> On 26/08/2013 17:45, Marek Vasut wrote:
>> > The chipselect must be written into the CTRL0 register after the SSP
>> > block is reset, otherwise the block will always use ChipSelect #0.
>> >
>> > Signed-off-by: Marek Vasut <marex@denx.de>
>> > Cc: Fabio Estevam <fabio.estevam@freescale.com>
>> > Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>> > Cc: Otavio Salvador <otavio@ossystems.com.br>
>> > Cc: Stefano Babic <sbabic@denx.de>
>> > ---
>> >
>> >  drivers/spi/mxs_spi.c |   12 +++---------
>> >  1 file changed, 3 insertions(+), 9 deletions(-)
>> >
>> > V2: Clean up a warning that "ssp_regs" are not used
>> >
>> > diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
>> > index 3cf7142..2b9f395 100644
>> > --- a/drivers/spi/mxs_spi.c
>> > +++ b/drivers/spi/mxs_spi.c
>> > @@ -56,8 +56,6 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
>> > unsigned int cs,
>> >
>> >                               unsigned int max_hz, unsigned int mode)
>> >
>> >  {
>> >
>> >     struct mxs_spi_slave *mxs_slave;
>> >
>> > -   struct mxs_ssp_regs *ssp_regs;
>> > -   int reg;
>> >
>> >     if (!spi_cs_is_valid(bus, cs)) {
>> >
>> >             printf("mxs_spi: invalid bus %d / chip select %d\n", bus, cs);
>> >
>> > @@ -74,13 +72,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
>> > unsigned int cs,
>> >
>> >     mxs_slave->max_khz = max_hz / 1000;
>> >     mxs_slave->mode = mode;
>> >     mxs_slave->regs = mxs_ssp_regs_by_bus(bus);
>> >
>> > -   ssp_regs = mxs_slave->regs;
>> >
>> > -   reg = readl(&ssp_regs->hw_ssp_ctrl0);
>> > -   reg &= ~(MXS_SSP_CHIPSELECT_MASK);
>> > -   reg |= cs << MXS_SSP_CHIPSELECT_SHIFT;
>> > -
>> > -   writel(reg, &ssp_regs->hw_ssp_ctrl0);
>> >
>> >     return &mxs_slave->slave;
>> >
>> >  err_init:
>> > @@ -102,7 +94,9 @@ int spi_claim_bus(struct spi_slave *slave)
>> >
>> >     mxs_reset_block(&ssp_regs->hw_ssp_ctrl0_reg);
>> >
>> > -   writel(SSP_CTRL0_BUS_WIDTH_ONE_BIT, &ssp_regs->hw_ssp_ctrl0);
>> > +   writel((slave->cs << MXS_SSP_CHIPSELECT_SHIFT) |
>> > +          SSP_CTRL0_BUS_WIDTH_ONE_BIT,
>> > +          &ssp_regs->hw_ssp_ctrl0);
>> >
>> >     reg = SSP_CTRL1_SSP_MODE_SPI | SSP_CTRL1_WORD_LENGTH_EIGHT_BITS;
>> >     reg |= (mxs_slave->mode & SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;
>>
>> Acked-by: Stefano Babic <sbabic@denx.de>
>
> Can we push this via IMX tree ?

I have a simple concern about commit head msg.
spi: mxs:
spi: mxs_spi:

I preferred the second one as rest are follow the same, just for unification.
Please send the v2, if your OK. then I will push it on my tree.

--
Thanks,
Jagan.
Stefano Babic Aug. 27, 2013, 9:10 a.m. UTC | #4
On 27/08/2013 11:05, Marek Vasut wrote:
> Dear Stefano Babic,
> 
>> On 26/08/2013 17:45, Marek Vasut wrote:
>>> The chipselect must be written into the CTRL0 register after the SSP
>>> block is reset, otherwise the block will always use ChipSelect #0.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Fabio Estevam <fabio.estevam@freescale.com>
>>> Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>>> Cc: Otavio Salvador <otavio@ossystems.com.br>
>>> Cc: Stefano Babic <sbabic@denx.de>
>>> ---
>>>
>>>  drivers/spi/mxs_spi.c |   12 +++---------
>>>  1 file changed, 3 insertions(+), 9 deletions(-)
>>>
>>> V2: Clean up a warning that "ssp_regs" are not used
>>>
>>> diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
>>> index 3cf7142..2b9f395 100644
>>> --- a/drivers/spi/mxs_spi.c
>>> +++ b/drivers/spi/mxs_spi.c
>>> @@ -56,8 +56,6 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
>>> unsigned int cs,
>>>
>>>  				  unsigned int max_hz, unsigned int mode)
>>>  
>>>  {
>>>  
>>>  	struct mxs_spi_slave *mxs_slave;
>>>
>>> -	struct mxs_ssp_regs *ssp_regs;
>>> -	int reg;
>>>
>>>  	if (!spi_cs_is_valid(bus, cs)) {
>>>  	
>>>  		printf("mxs_spi: invalid bus %d / chip select %d\n", bus, cs);
>>>
>>> @@ -74,13 +72,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
>>> unsigned int cs,
>>>
>>>  	mxs_slave->max_khz = max_hz / 1000;
>>>  	mxs_slave->mode = mode;
>>>  	mxs_slave->regs = mxs_ssp_regs_by_bus(bus);
>>>
>>> -	ssp_regs = mxs_slave->regs;
>>>
>>> -	reg = readl(&ssp_regs->hw_ssp_ctrl0);
>>> -	reg &= ~(MXS_SSP_CHIPSELECT_MASK);
>>> -	reg |= cs << MXS_SSP_CHIPSELECT_SHIFT;
>>> -
>>> -	writel(reg, &ssp_regs->hw_ssp_ctrl0);
>>>
>>>  	return &mxs_slave->slave;
>>>  
>>>  err_init:
>>> @@ -102,7 +94,9 @@ int spi_claim_bus(struct spi_slave *slave)
>>>
>>>  	mxs_reset_block(&ssp_regs->hw_ssp_ctrl0_reg);
>>>
>>> -	writel(SSP_CTRL0_BUS_WIDTH_ONE_BIT, &ssp_regs->hw_ssp_ctrl0);
>>> +	writel((slave->cs << MXS_SSP_CHIPSELECT_SHIFT) |
>>> +	       SSP_CTRL0_BUS_WIDTH_ONE_BIT,
>>> +	       &ssp_regs->hw_ssp_ctrl0);
>>>
>>>  	reg = SSP_CTRL1_SSP_MODE_SPI | SSP_CTRL1_WORD_LENGTH_EIGHT_BITS;
>>>  	reg |= (mxs_slave->mode & SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;
>>
>> Acked-by: Stefano Babic <sbabic@denx.de>
> 
> Can we push this via IMX tree ?

Maybe, but correctly this should be push via u-boot-spi.

Best regards,
Stefano Babic
Jagan Teki Aug. 27, 2013, 9:13 a.m. UTC | #5
On Tue, Aug 27, 2013 at 2:40 PM, Stefano Babic <sbabic@denx.de> wrote:
> On 27/08/2013 11:05, Marek Vasut wrote:
>> Dear Stefano Babic,
>>
>>> On 26/08/2013 17:45, Marek Vasut wrote:
>>>> The chipselect must be written into the CTRL0 register after the SSP
>>>> block is reset, otherwise the block will always use ChipSelect #0.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> Cc: Fabio Estevam <fabio.estevam@freescale.com>
>>>> Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>>>> Cc: Otavio Salvador <otavio@ossystems.com.br>
>>>> Cc: Stefano Babic <sbabic@denx.de>
>>>> ---
>>>>
>>>>  drivers/spi/mxs_spi.c |   12 +++---------
>>>>  1 file changed, 3 insertions(+), 9 deletions(-)
>>>>
>>>> V2: Clean up a warning that "ssp_regs" are not used
>>>>
>>>> diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
>>>> index 3cf7142..2b9f395 100644
>>>> --- a/drivers/spi/mxs_spi.c
>>>> +++ b/drivers/spi/mxs_spi.c
>>>> @@ -56,8 +56,6 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
>>>> unsigned int cs,
>>>>
>>>>                               unsigned int max_hz, unsigned int mode)
>>>>
>>>>  {
>>>>
>>>>     struct mxs_spi_slave *mxs_slave;
>>>>
>>>> -   struct mxs_ssp_regs *ssp_regs;
>>>> -   int reg;
>>>>
>>>>     if (!spi_cs_is_valid(bus, cs)) {
>>>>
>>>>             printf("mxs_spi: invalid bus %d / chip select %d\n", bus, cs);
>>>>
>>>> @@ -74,13 +72,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
>>>> unsigned int cs,
>>>>
>>>>     mxs_slave->max_khz = max_hz / 1000;
>>>>     mxs_slave->mode = mode;
>>>>     mxs_slave->regs = mxs_ssp_regs_by_bus(bus);
>>>>
>>>> -   ssp_regs = mxs_slave->regs;
>>>>
>>>> -   reg = readl(&ssp_regs->hw_ssp_ctrl0);
>>>> -   reg &= ~(MXS_SSP_CHIPSELECT_MASK);
>>>> -   reg |= cs << MXS_SSP_CHIPSELECT_SHIFT;
>>>> -
>>>> -   writel(reg, &ssp_regs->hw_ssp_ctrl0);
>>>>
>>>>     return &mxs_slave->slave;
>>>>
>>>>  err_init:
>>>> @@ -102,7 +94,9 @@ int spi_claim_bus(struct spi_slave *slave)
>>>>
>>>>     mxs_reset_block(&ssp_regs->hw_ssp_ctrl0_reg);
>>>>
>>>> -   writel(SSP_CTRL0_BUS_WIDTH_ONE_BIT, &ssp_regs->hw_ssp_ctrl0);
>>>> +   writel((slave->cs << MXS_SSP_CHIPSELECT_SHIFT) |
>>>> +          SSP_CTRL0_BUS_WIDTH_ONE_BIT,
>>>> +          &ssp_regs->hw_ssp_ctrl0);
>>>>
>>>>     reg = SSP_CTRL1_SSP_MODE_SPI | SSP_CTRL1_WORD_LENGTH_EIGHT_BITS;
>>>>     reg |= (mxs_slave->mode & SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;
>>>
>>> Acked-by: Stefano Babic <sbabic@denx.de>
>>
>> Can we push this via IMX tree ?
>
> Maybe, but correctly this should be push via u-boot-spi.

Yes, I sent a comment regarding commit head msg.
Will push it on my repo once i got the response.

--
Thanks,
Jagan.
Marek Vasut Aug. 27, 2013, 9:21 a.m. UTC | #6
Dear Jagan Teki,

> Hi,
> 
> On Tue, Aug 27, 2013 at 2:35 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Stefano Babic,
> > 
> >> On 26/08/2013 17:45, Marek Vasut wrote:
> >> > The chipselect must be written into the CTRL0 register after the SSP
> >> > block is reset, otherwise the block will always use ChipSelect #0.
> >> > 
> >> > Signed-off-by: Marek Vasut <marex@denx.de>
> >> > Cc: Fabio Estevam <fabio.estevam@freescale.com>
> >> > Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
> >> > Cc: Otavio Salvador <otavio@ossystems.com.br>
> >> > Cc: Stefano Babic <sbabic@denx.de>
> >> > ---
> >> > 
> >> >  drivers/spi/mxs_spi.c |   12 +++---------
> >> >  1 file changed, 3 insertions(+), 9 deletions(-)
> >> > 
> >> > V2: Clean up a warning that "ssp_regs" are not used
> >> > 
> >> > diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
> >> > index 3cf7142..2b9f395 100644
> >> > --- a/drivers/spi/mxs_spi.c
> >> > +++ b/drivers/spi/mxs_spi.c
> >> > @@ -56,8 +56,6 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
> >> > unsigned int cs,
> >> > 
> >> >                               unsigned int max_hz, unsigned int mode)
> >> >  
> >> >  {
> >> >  
> >> >     struct mxs_spi_slave *mxs_slave;
> >> > 
> >> > -   struct mxs_ssp_regs *ssp_regs;
> >> > -   int reg;
> >> > 
> >> >     if (!spi_cs_is_valid(bus, cs)) {
> >> >     
> >> >             printf("mxs_spi: invalid bus %d / chip select %d\n", bus,
> >> >             cs);
> >> > 
> >> > @@ -74,13 +72,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
> >> > unsigned int cs,
> >> > 
> >> >     mxs_slave->max_khz = max_hz / 1000;
> >> >     mxs_slave->mode = mode;
> >> >     mxs_slave->regs = mxs_ssp_regs_by_bus(bus);
> >> > 
> >> > -   ssp_regs = mxs_slave->regs;
> >> > 
> >> > -   reg = readl(&ssp_regs->hw_ssp_ctrl0);
> >> > -   reg &= ~(MXS_SSP_CHIPSELECT_MASK);
> >> > -   reg |= cs << MXS_SSP_CHIPSELECT_SHIFT;
> >> > -
> >> > -   writel(reg, &ssp_regs->hw_ssp_ctrl0);
> >> > 
> >> >     return &mxs_slave->slave;
> >> >  
> >> >  err_init:
> >> > @@ -102,7 +94,9 @@ int spi_claim_bus(struct spi_slave *slave)
> >> > 
> >> >     mxs_reset_block(&ssp_regs->hw_ssp_ctrl0_reg);
> >> > 
> >> > -   writel(SSP_CTRL0_BUS_WIDTH_ONE_BIT, &ssp_regs->hw_ssp_ctrl0);
> >> > +   writel((slave->cs << MXS_SSP_CHIPSELECT_SHIFT) |
> >> > +          SSP_CTRL0_BUS_WIDTH_ONE_BIT,
> >> > +          &ssp_regs->hw_ssp_ctrl0);
> >> > 
> >> >     reg = SSP_CTRL1_SSP_MODE_SPI | SSP_CTRL1_WORD_LENGTH_EIGHT_BITS;
> >> >     reg |= (mxs_slave->mode & SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;
> >> 
> >> Acked-by: Stefano Babic <sbabic@denx.de>
> > 
> > Can we push this via IMX tree ?
> 
> I have a simple concern about commit head msg.
> spi: mxs:
> spi: mxs_spi:
> 
> I preferred the second one as rest are follow the same, just for
> unification. Please send the v2, if your OK. then I will push it on my
> tree.

Can you fix that before pushing? I am fine either way.

Best regards,
Marek Vasut
Jagan Teki Aug. 27, 2013, 9:28 a.m. UTC | #7
Hi

On Tue, Aug 27, 2013 at 2:51 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Jagan Teki,
>
>> Hi,
>>
>> On Tue, Aug 27, 2013 at 2:35 PM, Marek Vasut <marex@denx.de> wrote:
>> > Dear Stefano Babic,
>> >
>> >> On 26/08/2013 17:45, Marek Vasut wrote:
>> >> > The chipselect must be written into the CTRL0 register after the SSP
>> >> > block is reset, otherwise the block will always use ChipSelect #0.
>> >> >
>> >> > Signed-off-by: Marek Vasut <marex@denx.de>
>> >> > Cc: Fabio Estevam <fabio.estevam@freescale.com>
>> >> > Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>> >> > Cc: Otavio Salvador <otavio@ossystems.com.br>
>> >> > Cc: Stefano Babic <sbabic@denx.de>
>> >> > ---
>> >> >
>> >> >  drivers/spi/mxs_spi.c |   12 +++---------
>> >> >  1 file changed, 3 insertions(+), 9 deletions(-)
>> >> >
>> >> > V2: Clean up a warning that "ssp_regs" are not used
>> >> >
>> >> > diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
>> >> > index 3cf7142..2b9f395 100644
>> >> > --- a/drivers/spi/mxs_spi.c
>> >> > +++ b/drivers/spi/mxs_spi.c
>> >> > @@ -56,8 +56,6 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
>> >> > unsigned int cs,
>> >> >
>> >> >                               unsigned int max_hz, unsigned int mode)
>> >> >
>> >> >  {
>> >> >
>> >> >     struct mxs_spi_slave *mxs_slave;
>> >> >
>> >> > -   struct mxs_ssp_regs *ssp_regs;
>> >> > -   int reg;
>> >> >
>> >> >     if (!spi_cs_is_valid(bus, cs)) {
>> >> >
>> >> >             printf("mxs_spi: invalid bus %d / chip select %d\n", bus,
>> >> >             cs);
>> >> >
>> >> > @@ -74,13 +72,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
>> >> > unsigned int cs,
>> >> >
>> >> >     mxs_slave->max_khz = max_hz / 1000;
>> >> >     mxs_slave->mode = mode;
>> >> >     mxs_slave->regs = mxs_ssp_regs_by_bus(bus);
>> >> >
>> >> > -   ssp_regs = mxs_slave->regs;
>> >> >
>> >> > -   reg = readl(&ssp_regs->hw_ssp_ctrl0);
>> >> > -   reg &= ~(MXS_SSP_CHIPSELECT_MASK);
>> >> > -   reg |= cs << MXS_SSP_CHIPSELECT_SHIFT;
>> >> > -
>> >> > -   writel(reg, &ssp_regs->hw_ssp_ctrl0);
>> >> >
>> >> >     return &mxs_slave->slave;
>> >> >
>> >> >  err_init:
>> >> > @@ -102,7 +94,9 @@ int spi_claim_bus(struct spi_slave *slave)
>> >> >
>> >> >     mxs_reset_block(&ssp_regs->hw_ssp_ctrl0_reg);
>> >> >
>> >> > -   writel(SSP_CTRL0_BUS_WIDTH_ONE_BIT, &ssp_regs->hw_ssp_ctrl0);
>> >> > +   writel((slave->cs << MXS_SSP_CHIPSELECT_SHIFT) |
>> >> > +          SSP_CTRL0_BUS_WIDTH_ONE_BIT,
>> >> > +          &ssp_regs->hw_ssp_ctrl0);
>> >> >
>> >> >     reg = SSP_CTRL1_SSP_MODE_SPI | SSP_CTRL1_WORD_LENGTH_EIGHT_BITS;
>> >> >     reg |= (mxs_slave->mode & SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;
>> >>
>> >> Acked-by: Stefano Babic <sbabic@denx.de>
>> >
>> > Can we push this via IMX tree ?
>>
>> I have a simple concern about commit head msg.
>> spi: mxs:
>> spi: mxs_spi:
>>
>> I preferred the second one as rest are follow the same, just for
>> unification. Please send the v2, if your OK. then I will push it on my
>> tree.
>
> Can you fix that before pushing? I am fine either way.

OK, sure I will do that.

--
Thanks,
Jagan.
Marek Vasut Aug. 27, 2013, 9:34 a.m. UTC | #8
Dear Jagan Teki,

> Hi
> 
> On Tue, Aug 27, 2013 at 2:51 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Jagan Teki,
> > 
> >> Hi,
> >> 
> >> On Tue, Aug 27, 2013 at 2:35 PM, Marek Vasut <marex@denx.de> wrote:
> >> > Dear Stefano Babic,
> >> > 
> >> >> On 26/08/2013 17:45, Marek Vasut wrote:
> >> >> > The chipselect must be written into the CTRL0 register after the
> >> >> > SSP block is reset, otherwise the block will always use ChipSelect
> >> >> > #0.
> >> >> > 
> >> >> > Signed-off-by: Marek Vasut <marex@denx.de>
> >> >> > Cc: Fabio Estevam <fabio.estevam@freescale.com>
> >> >> > Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
> >> >> > Cc: Otavio Salvador <otavio@ossystems.com.br>
> >> >> > Cc: Stefano Babic <sbabic@denx.de>
> >> >> > ---
> >> >> > 
> >> >> >  drivers/spi/mxs_spi.c |   12 +++---------
> >> >> >  1 file changed, 3 insertions(+), 9 deletions(-)
> >> >> > 
> >> >> > V2: Clean up a warning that "ssp_regs" are not used
> >> >> > 
> >> >> > diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
> >> >> > index 3cf7142..2b9f395 100644
> >> >> > --- a/drivers/spi/mxs_spi.c
> >> >> > +++ b/drivers/spi/mxs_spi.c
> >> >> > @@ -56,8 +56,6 @@ struct spi_slave *spi_setup_slave(unsigned int
> >> >> > bus, unsigned int cs,
> >> >> > 
> >> >> >                               unsigned int max_hz, unsigned int
> >> >> >                               mode)
> >> >> >  
> >> >> >  {
> >> >> >  
> >> >> >     struct mxs_spi_slave *mxs_slave;
> >> >> > 
> >> >> > -   struct mxs_ssp_regs *ssp_regs;
> >> >> > -   int reg;
> >> >> > 
> >> >> >     if (!spi_cs_is_valid(bus, cs)) {
> >> >> >     
> >> >> >             printf("mxs_spi: invalid bus %d / chip select %d\n",
> >> >> >             bus, cs);
> >> >> > 
> >> >> > @@ -74,13 +72,7 @@ struct spi_slave *spi_setup_slave(unsigned int
> >> >> > bus, unsigned int cs,
> >> >> > 
> >> >> >     mxs_slave->max_khz = max_hz / 1000;
> >> >> >     mxs_slave->mode = mode;
> >> >> >     mxs_slave->regs = mxs_ssp_regs_by_bus(bus);
> >> >> > 
> >> >> > -   ssp_regs = mxs_slave->regs;
> >> >> > 
> >> >> > -   reg = readl(&ssp_regs->hw_ssp_ctrl0);
> >> >> > -   reg &= ~(MXS_SSP_CHIPSELECT_MASK);
> >> >> > -   reg |= cs << MXS_SSP_CHIPSELECT_SHIFT;
> >> >> > -
> >> >> > -   writel(reg, &ssp_regs->hw_ssp_ctrl0);
> >> >> > 
> >> >> >     return &mxs_slave->slave;
> >> >> >  
> >> >> >  err_init:
> >> >> > @@ -102,7 +94,9 @@ int spi_claim_bus(struct spi_slave *slave)
> >> >> > 
> >> >> >     mxs_reset_block(&ssp_regs->hw_ssp_ctrl0_reg);
> >> >> > 
> >> >> > -   writel(SSP_CTRL0_BUS_WIDTH_ONE_BIT, &ssp_regs->hw_ssp_ctrl0);
> >> >> > +   writel((slave->cs << MXS_SSP_CHIPSELECT_SHIFT) |
> >> >> > +          SSP_CTRL0_BUS_WIDTH_ONE_BIT,
> >> >> > +          &ssp_regs->hw_ssp_ctrl0);
> >> >> > 
> >> >> >     reg = SSP_CTRL1_SSP_MODE_SPI |
> >> >> >     SSP_CTRL1_WORD_LENGTH_EIGHT_BITS; reg |= (mxs_slave->mode &
> >> >> >     SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;
> >> >> 
> >> >> Acked-by: Stefano Babic <sbabic@denx.de>
> >> > 
> >> > Can we push this via IMX tree ?
> >> 
> >> I have a simple concern about commit head msg.
> >> spi: mxs:
> >> spi: mxs_spi:
> >> 
> >> I preferred the second one as rest are follow the same, just for
> >> unification. Please send the v2, if your OK. then I will push it on my
> >> tree.
> > 
> > Can you fix that before pushing? I am fine either way.
> 
> OK, sure I will do that.

Thanks a lot!

Best regards,
Marek Vasut
Jagan Teki Aug. 27, 2013, 2:12 p.m. UTC | #9
On 27-08-2013 15:04, Marek Vasut wrote:
> Dear Jagan Teki,
>
>> Hi
>>
>> On Tue, Aug 27, 2013 at 2:51 PM, Marek Vasut <marex@denx.de> wrote:
>>> Dear Jagan Teki,
>>>
>>>> Hi,
>>>>
>>>> On Tue, Aug 27, 2013 at 2:35 PM, Marek Vasut <marex@denx.de> wrote:
>>>>> Dear Stefano Babic,
>>>>>
>>>>>> On 26/08/2013 17:45, Marek Vasut wrote:
>>>>>>> The chipselect must be written into the CTRL0 register after the
>>>>>>> SSP block is reset, otherwise the block will always use ChipSelect
>>>>>>> #0.
>>>>>>>
>>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>>>> Cc: Fabio Estevam <fabio.estevam@freescale.com>
>>>>>>> Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>>>>>>> Cc: Otavio Salvador <otavio@ossystems.com.br>
>>>>>>> Cc: Stefano Babic <sbabic@denx.de>
>>>>>>> ---
>>>>>>>
>>>>>>>   drivers/spi/mxs_spi.c |   12 +++---------
>>>>>>>   1 file changed, 3 insertions(+), 9 deletions(-)
>>>>>>>
>>>>>>> V2: Clean up a warning that "ssp_regs" are not used
>>>>>>>
>>>>>>> diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
>>>>>>> index 3cf7142..2b9f395 100644
>>>>>>> --- a/drivers/spi/mxs_spi.c
>>>>>>> +++ b/drivers/spi/mxs_spi.c
>>>>>>> @@ -56,8 +56,6 @@ struct spi_slave *spi_setup_slave(unsigned int
>>>>>>> bus, unsigned int cs,
>>>>>>>
>>>>>>>                                unsigned int max_hz, unsigned int
>>>>>>>                                mode)
>>>>>>>
>>>>>>>   {
>>>>>>>
>>>>>>>      struct mxs_spi_slave *mxs_slave;
>>>>>>>
>>>>>>> -   struct mxs_ssp_regs *ssp_regs;
>>>>>>> -   int reg;
>>>>>>>
>>>>>>>      if (!spi_cs_is_valid(bus, cs)) {
>>>>>>>
>>>>>>>              printf("mxs_spi: invalid bus %d / chip select %d\n",
>>>>>>>              bus, cs);
>>>>>>>
>>>>>>> @@ -74,13 +72,7 @@ struct spi_slave *spi_setup_slave(unsigned int
>>>>>>> bus, unsigned int cs,
>>>>>>>
>>>>>>>      mxs_slave->max_khz = max_hz / 1000;
>>>>>>>      mxs_slave->mode = mode;
>>>>>>>      mxs_slave->regs = mxs_ssp_regs_by_bus(bus);
>>>>>>>
>>>>>>> -   ssp_regs = mxs_slave->regs;
>>>>>>>
>>>>>>> -   reg = readl(&ssp_regs->hw_ssp_ctrl0);
>>>>>>> -   reg &= ~(MXS_SSP_CHIPSELECT_MASK);
>>>>>>> -   reg |= cs << MXS_SSP_CHIPSELECT_SHIFT;
>>>>>>> -
>>>>>>> -   writel(reg, &ssp_regs->hw_ssp_ctrl0);
>>>>>>>
>>>>>>>      return &mxs_slave->slave;
>>>>>>>
>>>>>>>   err_init:
>>>>>>> @@ -102,7 +94,9 @@ int spi_claim_bus(struct spi_slave *slave)
>>>>>>>
>>>>>>>      mxs_reset_block(&ssp_regs->hw_ssp_ctrl0_reg);
>>>>>>>
>>>>>>> -   writel(SSP_CTRL0_BUS_WIDTH_ONE_BIT, &ssp_regs->hw_ssp_ctrl0);
>>>>>>> +   writel((slave->cs << MXS_SSP_CHIPSELECT_SHIFT) |
>>>>>>> +          SSP_CTRL0_BUS_WIDTH_ONE_BIT,
>>>>>>> +          &ssp_regs->hw_ssp_ctrl0);
>>>>>>>
>>>>>>>      reg = SSP_CTRL1_SSP_MODE_SPI |
>>>>>>>      SSP_CTRL1_WORD_LENGTH_EIGHT_BITS; reg |= (mxs_slave->mode &
>>>>>>>      SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;
>>>>>>
>>>>>> Acked-by: Stefano Babic <sbabic@denx.de>
>>>>>
>>>>> Can we push this via IMX tree ?
>>>>
>>>> I have a simple concern about commit head msg.
>>>> spi: mxs:
>>>> spi: mxs_spi:
>>>>
>>>> I preferred the second one as rest are follow the same, just for
>>>> unification. Please send the v2, if your OK. then I will push it on my
>>>> tree.
>>>
>>> Can you fix that before pushing? I am fine either way.
>>
>> OK, sure I will do that.
>
> Thanks a lot!
>
> Best regards,
> Marek Vasut
>

Applied to u-boot-spi/master

--
Thanks,
Jagan.
diff mbox

Patch

diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
index 3cf7142..2b9f395 100644
--- a/drivers/spi/mxs_spi.c
+++ b/drivers/spi/mxs_spi.c
@@ -56,8 +56,6 @@  struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
 				  unsigned int max_hz, unsigned int mode)
 {
 	struct mxs_spi_slave *mxs_slave;
-	struct mxs_ssp_regs *ssp_regs;
-	int reg;
 
 	if (!spi_cs_is_valid(bus, cs)) {
 		printf("mxs_spi: invalid bus %d / chip select %d\n", bus, cs);
@@ -74,13 +72,7 @@  struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
 	mxs_slave->max_khz = max_hz / 1000;
 	mxs_slave->mode = mode;
 	mxs_slave->regs = mxs_ssp_regs_by_bus(bus);
-	ssp_regs = mxs_slave->regs;
 
-	reg = readl(&ssp_regs->hw_ssp_ctrl0);
-	reg &= ~(MXS_SSP_CHIPSELECT_MASK);
-	reg |= cs << MXS_SSP_CHIPSELECT_SHIFT;
-
-	writel(reg, &ssp_regs->hw_ssp_ctrl0);
 	return &mxs_slave->slave;
 
 err_init:
@@ -102,7 +94,9 @@  int spi_claim_bus(struct spi_slave *slave)
 
 	mxs_reset_block(&ssp_regs->hw_ssp_ctrl0_reg);
 
-	writel(SSP_CTRL0_BUS_WIDTH_ONE_BIT, &ssp_regs->hw_ssp_ctrl0);
+	writel((slave->cs << MXS_SSP_CHIPSELECT_SHIFT) |
+	       SSP_CTRL0_BUS_WIDTH_ONE_BIT,
+	       &ssp_regs->hw_ssp_ctrl0);
 
 	reg = SSP_CTRL1_SSP_MODE_SPI | SSP_CTRL1_WORD_LENGTH_EIGHT_BITS;
 	reg |= (mxs_slave->mode & SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;