| Message ID | 1358777318-7579-3-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Commit | 1be97bf22447088adebf23b1ca508d4bb00f853c |
| Headers | show |
On Tue, Jan 22, 2013 at 12:08 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > The code for handling the default "unknown command state" case in > pflash_read in pflash_cfi01.c comments "reset state & treat it as > a read". However the code doesn't actually do this. Moving the > default case to the top of the switch so it can fall through into > the read case brings this file into line with pflash_cfi02 and > makes the code behave as the comments suggest. > > The pflash_cfi01 code has always had this bug -- it was presumably > introduced when the original author copied the cfi02 code and > rearranged the order of the switch statement without noticing > that the default case relied on the fall-through. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> Tested-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > hw/pflash_cfi01.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/hw/pflash_cfi01.c b/hw/pflash_cfi01.c > index 310c716..b30cac6 100644 > --- a/hw/pflash_cfi01.c > +++ b/hw/pflash_cfi01.c > @@ -122,6 +122,12 @@ static uint32_t pflash_read (pflash_t *pfl, hwaddr offset, > __func__, offset, pfl->cmd, width); > #endif > switch (pfl->cmd) { > + default: > + /* This should never happen : reset state & treat it as a read */ > + DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd); > + pfl->wcycle = 0; > + pfl->cmd = 0; > + /* fall through to read code */ > case 0x00: > /* Flash area read */ > p = pfl->storage; > @@ -197,11 +203,6 @@ static uint32_t pflash_read (pflash_t *pfl, hwaddr offset, > else > ret = pfl->cfi_table[boff]; > break; > - default: > - /* This should never happen : reset state & treat it as a read */ > - DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd); > - pfl->wcycle = 0; > - pfl->cmd = 0; > } > return ret; > } > -- > 1.7.9.5 > >
diff --git a/hw/pflash_cfi01.c b/hw/pflash_cfi01.c index 310c716..b30cac6 100644 --- a/hw/pflash_cfi01.c +++ b/hw/pflash_cfi01.c @@ -122,6 +122,12 @@ static uint32_t pflash_read (pflash_t *pfl, hwaddr offset, __func__, offset, pfl->cmd, width); #endif switch (pfl->cmd) { + default: + /* This should never happen : reset state & treat it as a read */ + DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd); + pfl->wcycle = 0; + pfl->cmd = 0; + /* fall through to read code */ case 0x00: /* Flash area read */ p = pfl->storage; @@ -197,11 +203,6 @@ static uint32_t pflash_read (pflash_t *pfl, hwaddr offset, else ret = pfl->cfi_table[boff]; break; - default: - /* This should never happen : reset state & treat it as a read */ - DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd); - pfl->wcycle = 0; - pfl->cmd = 0; } return ret; }
The code for handling the default "unknown command state" case in pflash_read in pflash_cfi01.c comments "reset state & treat it as a read". However the code doesn't actually do this. Moving the default case to the top of the switch so it can fall through into the read case brings this file into line with pflash_cfi02 and makes the code behave as the comments suggest. The pflash_cfi01 code has always had this bug -- it was presumably introduced when the original author copied the cfi02 code and rearranged the order of the switch statement without noticing that the default case relied on the fall-through. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/pflash_cfi01.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)