[PATCH 0/3] hw/arm/nseries: Minor housekeeping

[PATCH 0/3] hw/arm/nseries: Minor housekeeping

[Philippe Mathieu-Daudé]

Constify read-only arrays and remove some -Wextra warnings.

Philippe Mathieu-Daudé (3):
  hw/input/tsc2xxx: Constify set_transform()'s MouseTransformInfo arg
  hw/arm/nseries: Constify various read-only arrays
  hw/arm/nseries: Silent -Wmissing-field-initializers warning

 hw/arm/nseries.c           | 28 +++++++++++++---------------
 hw/input/tsc2005.c         |  2 +-
 hw/input/tsc210x.c         |  3 +--
 include/hw/input/tsc2xxx.h |  4 ++--
 4 files changed, 17 insertions(+), 20 deletions(-)

-- 
2.38.1

[PATCH 1/3] hw/input/tsc2xxx: Constify set_transform()'s MouseTransformInfo arg

[Philippe Mathieu-Daudé]

The pointed MouseTransformInfo structure is accessed read-only.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/input/tsc2005.c         | 2 +-
 hw/input/tsc210x.c         | 3 +--
 include/hw/input/tsc2xxx.h | 4 ++--
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/input/tsc2005.c b/hw/input/tsc2005.c
index 14698ce109..555b677173 100644
--- a/hw/input/tsc2005.c
+++ b/hw/input/tsc2005.c
@@ -523,7 +523,7 @@ void *tsc2005_init(qemu_irq pintdav)
  * from the touchscreen.  Assuming 12-bit precision was used during
  * tslib calibration.
  */
-void tsc2005_set_transform(void *opaque, MouseTransformInfo *info)
+void tsc2005_set_transform(void *opaque, const MouseTransformInfo *info)
 {
     TSC2005State *s = (TSC2005State *) opaque;
 
diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c
index df7313db5d..fdd5ff87d9 100644
--- a/hw/input/tsc210x.c
+++ b/hw/input/tsc210x.c
@@ -1176,8 +1176,7 @@ I2SCodec *tsc210x_codec(uWireSlave *chip)
  * from the touchscreen.  Assuming 12-bit precision was used during
  * tslib calibration.
  */
-void tsc210x_set_transform(uWireSlave *chip,
-                MouseTransformInfo *info)
+void tsc210x_set_transform(uWireSlave *chip, const MouseTransformInfo *info)
 {
     TSC210xState *s = (TSC210xState *) chip->opaque;
 #if 0
diff --git a/include/hw/input/tsc2xxx.h b/include/hw/input/tsc2xxx.h
index 5b76ebc177..00eca17674 100644
--- a/include/hw/input/tsc2xxx.h
+++ b/include/hw/input/tsc2xxx.h
@@ -30,12 +30,12 @@ uWireSlave *tsc2102_init(qemu_irq pint);
 uWireSlave *tsc2301_init(qemu_irq penirq, qemu_irq kbirq, qemu_irq dav);
 I2SCodec *tsc210x_codec(uWireSlave *chip);
 uint32_t tsc210x_txrx(void *opaque, uint32_t value, int len);
-void tsc210x_set_transform(uWireSlave *chip, MouseTransformInfo *info);
+void tsc210x_set_transform(uWireSlave *chip, const MouseTransformInfo *info);
 void tsc210x_key_event(uWireSlave *chip, int key, int down);
 
 /* tsc2005.c */
 void *tsc2005_init(qemu_irq pintdav);
 uint32_t tsc2005_txrx(void *opaque, uint32_t value, int len);
-void tsc2005_set_transform(void *opaque, MouseTransformInfo *info);
+void tsc2005_set_transform(void *opaque, const MouseTransformInfo *info);
 
 #endif
-- 
2.38.1

Re: [PATCH 1/3] hw/input/tsc2xxx: Constify set_transform()'s MouseTransformInfo arg

[Richard Henderson]

On 12/20/22 06:25, Philippe Mathieu-Daudé wrote:
> The pointed MouseTransformInfo structure is accessed read-only.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/input/tsc2005.c         | 2 +-
>   hw/input/tsc210x.c         | 3 +--
>   include/hw/input/tsc2xxx.h | 4 ++--
>   3 files changed, 4 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 2/3] hw/arm/nseries: Constify various read-only arrays

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/nseries.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index b151113c27..2ff29f52e2 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -230,13 +230,13 @@ static void n8x0_i2c_setup(struct n800_s *s)
 }
 
 /* Touchscreen and keypad controller */
-static MouseTransformInfo n800_pointercal = {
+static const MouseTransformInfo n800_pointercal = {
     .x = 800,
     .y = 480,
     .a = { 14560, -68, -3455208, -39, -9621, 35152972, 65536 },
 };
 
-static MouseTransformInfo n810_pointercal = {
+static const MouseTransformInfo n810_pointercal = {
     .x = 800,
     .y = 480,
     .a = { 15041, 148, -4731056, 171, -10238, 35933380, 65536 },
@@ -334,7 +334,7 @@ static void n810_key_event(void *opaque, int keycode)
 
 #define M	0
 
-static int n810_keys[0x80] = {
+static const int n810_keys[0x80] = {
     [0x01] = 16,	/* Q */
     [0x02] = 37,	/* K */
     [0x03] = 24,	/* O */
@@ -810,7 +810,7 @@ static void n8x0_usb_setup(struct n800_s *s)
 /* Setup done before the main bootloader starts by some early setup code
  * - used when we want to run the main bootloader in emulation.  This
  * isn't documented.  */
-static uint32_t n800_pinout[104] = {
+static const uint32_t n800_pinout[104] = {
     0x080f00d8, 0x00d40808, 0x03080808, 0x080800d0,
     0x00dc0808, 0x0b0f0f00, 0x080800b4, 0x00c00808,
     0x08080808, 0x180800c4, 0x00b80000, 0x08080808,
@@ -1060,7 +1060,7 @@ static void n8x0_boot_init(void *opaque)
 #define OMAP_TAG_CBUS		0x4e03
 #define OMAP_TAG_EM_ASIC_BB5	0x4e04
 
-static struct omap_gpiosw_info_s {
+static const struct omap_gpiosw_info_s {
     const char *name;
     int line;
     int type;
@@ -1102,7 +1102,7 @@ static struct omap_gpiosw_info_s {
     { NULL }
 };
 
-static struct omap_partition_info_s {
+static const struct omap_partition_info_s {
     uint32_t offset;
     uint32_t size;
     int mask;
@@ -1125,15 +1125,15 @@ static struct omap_partition_info_s {
     { 0, 0, 0, NULL }
 };
 
-static uint8_t n8x0_bd_addr[6] = { N8X0_BD_ADDR };
+static const uint8_t n8x0_bd_addr[6] = { N8X0_BD_ADDR };
 
 static int n8x0_atag_setup(void *p, int model)
 {
     uint8_t *b;
     uint16_t *w;
     uint32_t *l;
-    struct omap_gpiosw_info_s *gpiosw;
-    struct omap_partition_info_s *partition;
+    const struct omap_gpiosw_info_s *gpiosw;
+    const struct omap_partition_info_s *partition;
     const char *tag;
 
     w = p;
-- 
2.38.1

Re: [PATCH 2/3] hw/arm/nseries: Constify various read-only arrays

[Richard Henderson]

On 12/20/22 06:25, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/arm/nseries.c | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 3/3] hw/arm/nseries: Silent -Wmissing-field-initializers warning

[Philippe Mathieu-Daudé]

Silent when compiling with -Wextra:

  ../hw/arm/nseries.c:1081:12: warning: missing field 'line' initializer [-Wmissing-field-initializers]
      { NULL }
             ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/nseries.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index 2ff29f52e2..c9df063a08 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -1078,7 +1078,7 @@ static const struct omap_gpiosw_info_s {
         "headphone", N8X0_HEADPHONE_GPIO,
         OMAP_GPIOSW_TYPE_CONNECTION | OMAP_GPIOSW_INVERTED,
     },
-    { NULL }
+    { /* end of list */ }
 }, n810_gpiosw_info[] = {
     {
         "gps_reset", N810_GPS_RESET_GPIO,
@@ -1099,7 +1099,7 @@ static const struct omap_gpiosw_info_s {
         "slide", N810_SLIDE_GPIO,
         OMAP_GPIOSW_TYPE_COVER | OMAP_GPIOSW_INVERTED,
     },
-    { NULL }
+    { /* end of list */ }
 };
 
 static const struct omap_partition_info_s {
@@ -1113,16 +1113,14 @@ static const struct omap_partition_info_s {
     { 0x00080000, 0x00200000, 0x0, "kernel" },
     { 0x00280000, 0x00200000, 0x3, "initfs" },
     { 0x00480000, 0x0fb80000, 0x3, "rootfs" },
-
-    { 0, 0, 0, NULL }
+    { /* end of list */ }
 }, n810_part_info[] = {
     { 0x00000000, 0x00020000, 0x3, "bootloader" },
     { 0x00020000, 0x00060000, 0x0, "config" },
     { 0x00080000, 0x00220000, 0x0, "kernel" },
     { 0x002a0000, 0x00400000, 0x0, "initfs" },
     { 0x006a0000, 0x0f960000, 0x0, "rootfs" },
-
-    { 0, 0, 0, NULL }
+    { /* end of list */ }
 };
 
 static const uint8_t n8x0_bd_addr[6] = { N8X0_BD_ADDR };
-- 
2.38.1

Re: [PATCH 3/3] hw/arm/nseries: Silent -Wmissing-field-initializers warning

[Richard Henderson]

On 12/20/22 06:25, Philippe Mathieu-Daudé wrote:
> Silent when compiling with -Wextra:
> 
>    ../hw/arm/nseries.c:1081:12: warning: missing field 'line' initializer [-Wmissing-field-initializers]
>        { NULL }
>               ^
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/arm/nseries.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)

Not a fan of this specific warning.


r~

Re: [PATCH 3/3] hw/arm/nseries: Silent -Wmissing-field-initializers warning

[Philippe Mathieu-Daudé]

On 21/12/22 02:01, Richard Henderson wrote:
> On 12/20/22 06:25, Philippe Mathieu-Daudé wrote:
>> Silent when compiling with -Wextra:
>>
>>    ../hw/arm/nseries.c:1081:12: warning: missing field 'line' 
>> initializer [-Wmissing-field-initializers]
>>        { NULL }
>>               ^
>>
>> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
>> ---
>>   hw/arm/nseries.c | 10 ++++------
>>   1 file changed, 4 insertions(+), 6 deletions(-)
> 
> Not a fan of this specific warning.

Me neither, and I don't think we can enforce it on QEMU codebase.

However I find '{ /* end of list */ }' cleaner/clearer than assigning
the first field.

Re: [PATCH 3/3] hw/arm/nseries: Silent -Wmissing-field-initializers warning

[Richard Henderson]

On 12/20/22 23:16, Philippe Mathieu-Daudé wrote:
> However I find '{ /* end of list */ }' cleaner/clearer than assigning
> the first field.

I agree with that, for those cases we don't already have a separate sentinal define.


r~

Re: [PATCH 0/3] hw/arm/nseries: Minor housekeeping

[Peter Maydell]

On Tue, 20 Dec 2022 at 14:25, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Constify read-only arrays and remove some -Wextra warnings.
>
> Philippe Mathieu-Daudé (3):
>   hw/input/tsc2xxx: Constify set_transform()'s MouseTransformInfo arg
>   hw/arm/nseries: Constify various read-only arrays
>   hw/arm/nseries: Silent -Wmissing-field-initializers warning
>
>  hw/arm/nseries.c           | 28 +++++++++++++---------------
>  hw/input/tsc2005.c         |  2 +-
>  hw/input/tsc210x.c         |  3 +--
>  include/hw/input/tsc2xxx.h |  4 ++--
>  4 files changed, 17 insertions(+), 20 deletions(-)




Applied to target-arm.next, thanks.

-- PMM