Re: [PATCH] spi/dw_spi: clean the cs_control code

[Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

On Wed, 1 Sep 2010 23:07:57 +0800
Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:

> Hi Feng,
> 
> On Wed, Sep 01, 2010 at 01:35:59PM +0800, Feng Tang wrote:
> > commit 052dc7c45 introduced cs_control code,
> 
> Be friendly to mere-mortals.  You should quote the patch title
> "spi/dw_spi: conditional transfer mode changes" in addition to the
> sha1 id.

Got it, thanks, will add it in V2

> > @@ -544,13 +540,13 @@ static void pump_transfers(unsigned long data)
> >  	 */
> >  	if (dws->cs_control) {
> >  		if (dws->rx && dws->tx)
> > -			chip->tmode = 0x00;
> > +			chip->tmode = SPI_TMOD_TR;
> >  		else if (dws->rx)
> > -			chip->tmode = 0x02;
> > +			chip->tmode = SPI_TMOD_RO;
> >  		else
> > -			chip->tmode = 0x01;
> > +			chip->tmode = SPI_TMOD_TO;
> >  
> > -		cr0 &= ~(0x3 << SPI_MODE_OFFSET);
> > +		cr0 &= ~SPI_TMOD_MASK;
> >  		cr0 |= (chip->tmode << SPI_TMOD_OFFSET);
> 
> Changing these values isn't mentioned in the patch description.  I
> assume this is not the bug fix because the #defines are the same
> values.

OK, it's a little confusing, there are 2 OFFSET here:
	SPI_MODE_OFFSET (for spi mode 0/1/2 etc setting)
	SPI_TMOD_OFFSET (for tx only/rx only/duplex)

So simple fix should be
-		cr0 &= ~(0x3 << SPI_MODE_OFFSET); 
+		cr0 &= ~(0x3 << SPI_TMOD_OFFSET);
while my new introduced SPI_TMOD_MASK make things complex :)

Thanks,
Feng

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd