| Message ID | 20170321030320.8062-1-cyril.bur@au1.ibm.com |
|---|---|
| State | Rejected |
| Headers | show |
I think Stewart was informed out of band but I'll say it here. This patch should not be merged. This bug is known to exist by all current implementations and changing the behaviour would break more than it fixes. Cyril On Tue, 2017-03-21 at 14:03 +1100, Cyril Bur wrote: > The position to flush from should be relative to the window not to the > entire flash. > > "Args 0-1: Where within window as number of blocks" > > This is actually quite nice as skiboot will always want to flush the > entire window, so from zero to size. > > Fixes: 23fe769115c41e8859ce3d23dc75953bfb290f45 > Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> > --- > libflash/mbox-flash.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libflash/mbox-flash.c b/libflash/mbox-flash.c > index 7bf731d0..61de4ff8 100644 > --- a/libflash/mbox-flash.c > +++ b/libflash/mbox-flash.c > @@ -342,7 +342,7 @@ static int mbox_flash_write(struct blocklevel_device *bl, uint64_t pos, > * without flushing entitles the BMC to throw away the > * data > */ > - rc = mbox_flash_flush(mbox_flash, pos, size); > + rc = mbox_flash_flush(mbox_flash, 0, size); > if (rc) > return rc; >
On Fri, Mar 24, 2017 at 11:34 AM, Cyril Bur <cyrilbur@gmail.com> wrote: > I think Stewart was informed out of band but I'll say it here. > > This patch should not be merged. This bug is known to exist by all > current implementations and changing the behaviour would break more > than it fixes. > > Cyril > > On Tue, 2017-03-21 at 14:03 +1100, Cyril Bur wrote: >> The position to flush from should be relative to the window not to the >> entire flash. >> >> "Args 0-1: Where within window as number of blocks" >> >> This is actually quite nice as skiboot will always want to flush the >> entire window, so from zero to size. IS THIS V3??????????? >> >> Fixes: 23fe769115c41e8859ce3d23dc75953bfb290f45 >> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> >> --- >> libflash/mbox-flash.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libflash/mbox-flash.c b/libflash/mbox-flash.c >> index 7bf731d0..61de4ff8 100644 >> --- a/libflash/mbox-flash.c >> +++ b/libflash/mbox-flash.c >> @@ -342,7 +342,7 @@ static int mbox_flash_write(struct blocklevel_device *bl, uint64_t pos, >> * without flushing entitles the BMC to throw away the >> * data >> */ >> - rc = mbox_flash_flush(mbox_flash, pos, size); >> + rc = mbox_flash_flush(mbox_flash, 0, size); >> if (rc) >> return rc; >> > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
diff --git a/libflash/mbox-flash.c b/libflash/mbox-flash.c index 7bf731d0..61de4ff8 100644 --- a/libflash/mbox-flash.c +++ b/libflash/mbox-flash.c @@ -342,7 +342,7 @@ static int mbox_flash_write(struct blocklevel_device *bl, uint64_t pos, * without flushing entitles the BMC to throw away the * data */ - rc = mbox_flash_flush(mbox_flash, pos, size); + rc = mbox_flash_flush(mbox_flash, 0, size); if (rc) return rc;
The position to flush from should be relative to the window not to the entire flash. "Args 0-1: Where within window as number of blocks" This is actually quite nice as skiboot will always want to flush the entire window, so from zero to size. Fixes: 23fe769115c41e8859ce3d23dc75953bfb290f45 Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> --- libflash/mbox-flash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)