| Message ID | 20200813222625.243136-2-ehabkost@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qom: Automated conversion of type checking boilerplate | expand |
On 8/14/20 12:25 AM, Eduardo Habkost wrote: > The PL1110 enum value name will conflict with the PL1110 type > cast checker, when we replace the existing macro with an inline > function. Rename it to PL1110_STOCK. typo s/PL1110/PL110/ in subject and description. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > hw/display/pl110.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/hw/display/pl110.c b/hw/display/pl110.c > index c2991a28d2..4664fde3f2 100644 > --- a/hw/display/pl110.c > +++ b/hw/display/pl110.c > @@ -42,7 +42,7 @@ enum pl110_bppmode > /* The Versatile/PB uses a slightly modified PL110 controller. */ > enum pl110_version > { > - PL110, > + PL110_STOCK, > PL110_VERSATILE, > PL111 For completeness I'd also rename PL111. What about: enum pl110_version { PL110_VERSION, PL110_VERSATILE_VERSION, PL111_VERSION } ? > }; > @@ -372,12 +372,12 @@ static uint64_t pl110_read(void *opaque, hwaddr offset, > case 5: /* LCDLPBASE */ > return s->lpbase; > case 6: /* LCDIMSC */ > - if (s->version != PL110) { > + if (s->version != PL110_STOCK) { > return s->cr; > } > return s->int_mask; > case 7: /* LCDControl */ > - if (s->version != PL110) { > + if (s->version != PL110_STOCK) { > return s->int_mask; > } > return s->cr; > @@ -437,7 +437,7 @@ static void pl110_write(void *opaque, hwaddr offset, > s->lpbase = val; > break; > case 6: /* LCDIMSC */ > - if (s->version != PL110) { > + if (s->version != PL110_STOCK) { > goto control; > } > imsc: > @@ -445,7 +445,7 @@ static void pl110_write(void *opaque, hwaddr offset, > pl110_update(s); > break; > case 7: /* LCDControl */ > - if (s->version != PL110) { > + if (s->version != PL110_STOCK) { > goto imsc; > } > control: > @@ -513,7 +513,7 @@ static void pl110_init(Object *obj) > { > PL110State *s = PL110(obj); > > - s->version = PL110; > + s->version = PL110_STOCK; > } > > static void pl110_versatile_init(Object *obj) >
CCing maintainer (pmaydell). On Fri, Aug 14, 2020 at 07:45:40PM +0200, Philippe Mathieu-Daudé wrote: > On 8/14/20 12:25 AM, Eduardo Habkost wrote: > > The PL1110 enum value name will conflict with the PL1110 type > > cast checker, when we replace the existing macro with an inline > > function. Rename it to PL1110_STOCK. > > typo s/PL1110/PL110/ in subject and description. Thanks for spotting that! Will be fixed in v2. > > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > hw/display/pl110.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/hw/display/pl110.c b/hw/display/pl110.c > > index c2991a28d2..4664fde3f2 100644 > > --- a/hw/display/pl110.c > > +++ b/hw/display/pl110.c > > @@ -42,7 +42,7 @@ enum pl110_bppmode > > /* The Versatile/PB uses a slightly modified PL110 controller. */ > > enum pl110_version > > { > > - PL110, > > + PL110_STOCK, > > PL110_VERSATILE, > > PL111 > > For completeness I'd also rename PL111. > > What about: > > enum pl110_version > { > PL110_VERSION, > PL110_VERSATILE_VERSION, > PL111_VERSION > } > > ? That would work too, although I'm more used to enum values to have a common prefix instead of a common suffix. Any objections to: enum pl110_version { VERSION_PL110, VERSION_PL110_VERSATILE, VERSION_PL111 } ?
Le mar. 18 août 2020 23:30, Eduardo Habkost <ehabkost@redhat.com> a écrit : > CCing maintainer (pmaydell). > > On Fri, Aug 14, 2020 at 07:45:40PM +0200, Philippe Mathieu-Daudé wrote: > > On 8/14/20 12:25 AM, Eduardo Habkost wrote: > > > The PL1110 enum value name will conflict with the PL1110 type > > > cast checker, when we replace the existing macro with an inline > > > function. Rename it to PL1110_STOCK. > > > > typo s/PL1110/PL110/ in subject and description. > > Thanks for spotting that! Will be fixed in v2. > > > > > > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > > --- > > > hw/display/pl110.c | 12 ++++++------ > > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > > > diff --git a/hw/display/pl110.c b/hw/display/pl110.c > > > index c2991a28d2..4664fde3f2 100644 > > > --- a/hw/display/pl110.c > > > +++ b/hw/display/pl110.c > > > @@ -42,7 +42,7 @@ enum pl110_bppmode > > > /* The Versatile/PB uses a slightly modified PL110 controller. */ > > > enum pl110_version > > > { > > > - PL110, > > > + PL110_STOCK, > > > PL110_VERSATILE, > > > PL111 > > > > For completeness I'd also rename PL111. > > > > What about: > > > > enum pl110_version > > { > > PL110_VERSION, > > PL110_VERSATILE_VERSION, > > PL111_VERSION > > } > > > > ? > > That would work too, although I'm more used to enum values to > have a common prefix instead of a common suffix. > > Any objections to: > > enum pl110_version > { > VERSION_PL110, > VERSION_PL110_VERSATILE, > VERSION_PL111 > } > > ? > Sounds good. > -- > Eduardo > >
diff --git a/hw/display/pl110.c b/hw/display/pl110.c index c2991a28d2..4664fde3f2 100644 --- a/hw/display/pl110.c +++ b/hw/display/pl110.c @@ -42,7 +42,7 @@ enum pl110_bppmode /* The Versatile/PB uses a slightly modified PL110 controller. */ enum pl110_version { - PL110, + PL110_STOCK, PL110_VERSATILE, PL111 }; @@ -372,12 +372,12 @@ static uint64_t pl110_read(void *opaque, hwaddr offset, case 5: /* LCDLPBASE */ return s->lpbase; case 6: /* LCDIMSC */ - if (s->version != PL110) { + if (s->version != PL110_STOCK) { return s->cr; } return s->int_mask; case 7: /* LCDControl */ - if (s->version != PL110) { + if (s->version != PL110_STOCK) { return s->int_mask; } return s->cr; @@ -437,7 +437,7 @@ static void pl110_write(void *opaque, hwaddr offset, s->lpbase = val; break; case 6: /* LCDIMSC */ - if (s->version != PL110) { + if (s->version != PL110_STOCK) { goto control; } imsc: @@ -445,7 +445,7 @@ static void pl110_write(void *opaque, hwaddr offset, pl110_update(s); break; case 7: /* LCDControl */ - if (s->version != PL110) { + if (s->version != PL110_STOCK) { goto imsc; } control: @@ -513,7 +513,7 @@ static void pl110_init(Object *obj) { PL110State *s = PL110(obj); - s->version = PL110; + s->version = PL110_STOCK; } static void pl110_versatile_init(Object *obj)
The PL1110 enum value name will conflict with the PL1110 type cast checker, when we replace the existing macro with an inline function. Rename it to PL1110_STOCK. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/display/pl110.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)