Re: [PATCH v2 2/4] hw/i2c: Fix checkpatch line over 80 chars warnings

[Cédric Le Goater]

On 4/17/24 16:20, Philippe Mathieu-Daudé wrote:

On 17/4/24 08:24, Cédric Le Goater wrote:

Hello,

On 4/16/24 20:47, Philippe Mathieu-Daudé wrote:

We are going to modify these lines, fix their style
in order to avoid checkpatch.pl warnings:

   WARNING: line over 80 characters

Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
  include/hw/i2c/i2c.h            |  11 ++-
  include/hw/nvram/eeprom_at24c.h |   6 +-
  hw/arm/aspeed.c                 | 140 +++++++++++++++++++-------------
  hw/nvram/eeprom_at24c.c         |   6 +-
  4 files changed, 98 insertions(+), 65 deletions(-)

-    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3), "dps310", 0x76);
-    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3), "max31785", 
0x52);
-    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), "tmp423", 0x4c);
-    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 5), "tmp423", 0x4c);
+    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3),
+                            "dps310", 0x76);
+    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3),
+                            "max31785", 0x52);
+    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4),
+                            "tmp423", 0x4c);
+    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 5),
+                            "tmp423", 0x4c);
      /* The Witherspoon expects a TMP275 but a TMP105 is compatible */
-    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), TYPE_TMP105,
-                     0x4a);
+    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9),
+                            TYPE_TMP105, 0x4a);
      /* The witherspoon board expects Epson RX8900 I2C RTC but a ds1338 is
       * good enough */
-    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 11), "ds1338", 0x32);
+    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 11),
+                            "ds1338", 0x32);

If the definitions were on a single line, they would be more
readable IMHO. So I would do the opposit change ...

An alternate solution could be to define an array of devices
at the machine class level, something like
   struct i2c_device [
           const char *type;
           uint8_t bus;
           uint8_t addr;
   } devices[] = { ... };

I agree this would be better, but this should be done separately
of this series. For now I propose not modifying hw/arm/aspeed.c
in this patch, and ignoring the checkpatch errors in the next
patch. What do you think?

sure, np.

Thanks,

C.