| Message ID | 1454347206-23717-2-git-send-email-wei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 1 February 2016 at 17:20, Wei Huang <wei@redhat.com> wrote: > This patch removes float_high field of PL061State, which doesn't seem > to be used anywhere. > > Signed-off-by: Wei Huang <wei@redhat.com> > --- > hw/gpio/pl061.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c > index 342a70d..2c08e88 100644 > --- a/hw/gpio/pl061.c > +++ b/hw/gpio/pl061.c > @@ -56,7 +56,6 @@ typedef struct PL061State { > uint32_t slr; > uint32_t den; > uint32_t cr; > - uint32_t float_high; > uint32_t amsel; > qemu_irq irq; > qemu_irq out[8]; > @@ -88,7 +87,6 @@ static const VMStateDescription vmstate_pl061 = { > VMSTATE_UINT32(slr, PL061State), > VMSTATE_UINT32(den, PL061State), > VMSTATE_UINT32(cr, PL061State), > - VMSTATE_UINT32(float_high, PL061State), > VMSTATE_UINT32_V(amsel, PL061State, 2), > VMSTATE_END_OF_LIST() This would be a migration compatibility break, so at a minimum you need to bump the vmstate struct versions. thanks -- PMM
01.02.2016 21:01, Peter Maydell wrote: > On 1 February 2016 at 17:20, Wei Huang <wei@redhat.com> wrote: >> This patch removes float_high field of PL061State, which doesn't seem >> to be used anywhere. [] >> @@ -88,7 +87,6 @@ static const VMStateDescription vmstate_pl061 = { >> VMSTATE_UINT32(slr, PL061State), >> VMSTATE_UINT32(den, PL061State), >> VMSTATE_UINT32(cr, PL061State), >> - VMSTATE_UINT32(float_high, PL061State), >> VMSTATE_UINT32_V(amsel, PL061State, 2), >> VMSTATE_END_OF_LIST() > > This would be a migration compatibility break, so at a minimum > you need to bump the vmstate struct versions. Is it worth the effort to remove this field if it causes compatibility break? Maybe keep it around, it doesn't hurt? At the very least, we may rename it to "unused_float_high", or something, to indicate it is a known-unused? Thanks, /mjt
On 02/02/2016 01:03 AM, Michael Tokarev wrote: > 01.02.2016 21:01, Peter Maydell wrote: >> On 1 February 2016 at 17:20, Wei Huang <wei@redhat.com> wrote: >>> This patch removes float_high field of PL061State, which doesn't seem >>> to be used anywhere. > [] >>> @@ -88,7 +87,6 @@ static const VMStateDescription vmstate_pl061 = { >>> VMSTATE_UINT32(slr, PL061State), >>> VMSTATE_UINT32(den, PL061State), >>> VMSTATE_UINT32(cr, PL061State), >>> - VMSTATE_UINT32(float_high, PL061State), >>> VMSTATE_UINT32_V(amsel, PL061State, 2), >>> VMSTATE_END_OF_LIST() >> >> This would be a migration compatibility break, so at a minimum >> you need to bump the vmstate struct versions. > > Is it worth the effort to remove this field if it causes > compatibility break? Maybe keep it around, it doesn't hurt? It doesn't hurt. So either way is fine. I just happened to find it while reviewing the code. > At the very least, we may rename it to "unused_float_high", > or something, to indicate it is a known-unused? I don't think renaming solves any problem. Either we keep this variable as it is or remove it. > > Thanks, > > /mjt >
diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c index 342a70d..2c08e88 100644 --- a/hw/gpio/pl061.c +++ b/hw/gpio/pl061.c @@ -56,7 +56,6 @@ typedef struct PL061State { uint32_t slr; uint32_t den; uint32_t cr; - uint32_t float_high; uint32_t amsel; qemu_irq irq; qemu_irq out[8]; @@ -88,7 +87,6 @@ static const VMStateDescription vmstate_pl061 = { VMSTATE_UINT32(slr, PL061State), VMSTATE_UINT32(den, PL061State), VMSTATE_UINT32(cr, PL061State), - VMSTATE_UINT32(float_high, PL061State), VMSTATE_UINT32_V(amsel, PL061State, 2), VMSTATE_END_OF_LIST() }
This patch removes float_high field of PL061State, which doesn't seem to be used anywhere. Signed-off-by: Wei Huang <wei@redhat.com> --- hw/gpio/pl061.c | 2 -- 1 file changed, 2 deletions(-)