Hi Peter, On Tue, Oct 10, 2017 at 6:24 PM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 20 September 2017 at 21:17, Philippe Mathieu-Daudé <f4...@amsat.org> > wrote: > > From: Subbaraya Sundeep <sundeep.l...@gmail.com> > > > > Modelled Microsemi's Smartfusion2 SPI controller. > > > > Signed-off-by: Subbaraya Sundeep <sundeep.l...@gmail.com> > > Reviewed-by: Alistair Francis <alistair.fran...@xilinx.com> > > Tested-by: Philippe Mathieu-Daudé <f4...@amsat.org> > > > +#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 >