[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v11 3/5] msf2: Add Smartfusion2 SPI controller
From: |
sundeep subbaraya |
Subject: |
Re: [Qemu-devel] [PATCH v11 3/5] msf2: Add Smartfusion2 SPI controller |
Date: |
Mon, 16 Oct 2017 15:13:53 +0530 |
Hi Peter,
On Tue, Oct 10, 2017 at 6:24 PM, Peter Maydell <address@hidden>
wrote:
> On 20 September 2017 at 21:17, Philippe Mathieu-Daudé <address@hidden>
> wrote:
> > From: Subbaraya Sundeep <address@hidden>
> >
> > Modelled Microsemi's Smartfusion2 SPI controller.
> >
> > Signed-off-by: Subbaraya Sundeep <address@hidden>
> > Reviewed-by: Alistair Francis <address@hidden>
> > Tested-by: Philippe Mathieu-Daudé <address@hidden>
>
> > +#define FRAMESZ_MASK 0x1F
>
> > +static void set_fifodepth(MSSSpiState *s)
> > +{
> > + unsigned int size = s->regs[R_SPI_DFSIZE] & FRAMESZ_MASK;
> > +
> > + if (size <= 8) {
> > + s->fifo_depth = 32;
> > + } else if (size <= 16) {
> > + s->fifo_depth = 16;
> > + } else if (size <= 32) {
> > + s->fifo_depth = 8;
> > + } else {
> > + s->fifo_depth = 4;
> > + }
> > +}
>
> Hi. Coverity points out (CID 1381483) that the "else" case here
> is dead code, because the FRAMESZ_MASK of 0x1F means that size
> cannot be 32 or more.
>
> Paolo kindly checked up with the spec at
> https://www.eecs.umich.edu/courses/eecs373/readings/Actel_SmartFusion_MSS_
> UserGuide.pdf
> which says that this register's field is bits [5:0] which
> would imply an 0x3f mask is needed. On the other hand it also
> says that "maximum value is 32", so what is the else clause
> doing anyway?
>
I will remove the else, change mask to 0x3F and add check for max 32 in
spi_write:
case R_SPI_DFSIZE:
if (s->enabled || (value & FRAMESZ_MASK) > 32) {
break;
}
s->regs[R_SPI_DFSIZE] = value;
break;
Thanks for pointing out.
Sundeep
>
> thanks
> -- PMM
>