Re: [PATCH 1/2] SPI: S3C64XX: Correction for 16,32 bits bus width

[Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>]

On Fri, Sep 10, 2010 at 12:07:50AM +0900, Jassi Brar wrote:
> On Thu, Sep 9, 2010 at 11:51 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> > On Thu, Sep 09, 2010 at 04:18:57PM +0900, Jassi Brar wrote:
[...]
> > Nit: switch statements like this tend to obscure the fact that the same
> > function is called each time with a different value.  How about the
> > following:
> >
> >        width = 1;
> >        if (sdd->cur_bpw == 32)
> >                width = 4;
> >        if (sdd->cur_bpw == 16)
> >                width = 2;
> >        s3c2410_dma_config(sdd->tx_dmach, width);
> >
> > It's also more concise.
> 
> yuck! why didn't i do
>           s3c2410_dma_config(sdd->tx_dmach, sdd->cur_bpw / 8); ??

Heh, very true!

> >> @@ -610,6 +648,14 @@ static void handle_msg(struct s3c64xx_spi_driver_data *sdd,
> >>               bpw = xfer->bits_per_word ? : spi->bits_per_word;
> >>               speed = xfer->speed_hz ? : spi->max_speed_hz;
> >>
> >> +             if (bpw != 8 && xfer->len % (bpw / 8)) {
> >
> > The (bpw != 8) test is superfluous.
> 
>  xfer->len % (bpw / 8)    always evaluate to true(error hence) when bpw==8
> so we wanna avoid that case

When bpw == 8, the equation reduces to (len % 1), which will always
evaluate to 0 (false) for integer len.

g.


------------------------------------------------------------------------------
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