hw/arm/exynos: Minor cleanups

PATCH Superseded Krzysztof Kozlowski 1 participant 5 messages Mar 13, 2017 Arm Samsung Exynos

lore

Share & tools

lore

Krzysztof Kozlowski

author maintainer

2017-03-13 18:36 UTC #0 lore raw

v2 cover hw/arm/exynos: Minor cleanups

Hi,

Minor cleanups, no dependencies.

Changes since v1
================
1. Patch 1/3: Address Eric's comment.
2. Patch 2/3: Address Peter's comments about "const".
3. Patch 3/3: Use "unsigned int". I had impression that no consensus was reached during discussion. If that is wrong impression, let me know.

Best regards,
Krzysztof

Krzysztof Kozlowski (3):
hw/arm/exynos: Convert fprintf to error_report() hw/char/exynos4210_uart: Constify static array and few arguments hw/misc/exynos4210_pmu: Reorder local variables for readability

 hw/arm/exynos4_boards.c   |  6 +++---
 hw/char/exynos4210_uart.c |  8 ++++----
 hw/misc/exynos4210_pmu.c  |  4 ++--
 hw/timer/exynos4210_mct.c |  5 +++--
 hw/timer/exynos4210_pwm.c | 11 +++++------
 hw/timer/exynos4210_rtc.c | 16 +++++++---------
 6 files changed, 24 insertions(+), 26 deletions(-)

Signature · 1 line

2.9.3

Krzysztof Kozlowski

author maintainer

2017-03-13 18:36 UTC #1 lore raw

PATCH v2 1/3 hw/arm/exynos: Convert fprintf to error_report()

error_report() is preferred over fprintf() for logging errors. Also remove square brackets [] and additional new line characters in printed messages.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 hw/arm/exynos4_boards.c   |  6 +++---
 hw/timer/exynos4210_mct.c |  5 +++--
 hw/timer/exynos4210_pwm.c | 11 +++++------
 hw/timer/exynos4210_rtc.c | 16 +++++++---------
 4 files changed, 18 insertions(+), 20 deletions(-)

hw/arm/exynos4_boards.c +3 -3

index 0efa19405409..0a352815b86d 100644
@@ -22,6 +22,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "qemu-common.h"
 #include "cpu.h"
 #include "sysemu/sysemu.h"

@@ -101,9 +102,8 @@ static Exynos4210State *exynos4_boards_init_common(MachineState *machine,
     MachineClass *mc = MACHINE_GET_CLASS(machine);
 
     if (smp_cpus != EXYNOS4210_NCPUS && !qtest_enabled()) {
-        fprintf(stderr, "%s board supports only %d CPU cores. Ignoring smp_cpus"
-                " value.\n",
-                mc->name, EXYNOS4210_NCPUS);
+        error_report("%s board supports only %d CPU cores, ignoring smp_cpus value",
+                     mc->name, EXYNOS4210_NCPUS);
     }
 
     exynos4_board_binfo.ram_size = exynos4_board_ram_size[board_type];

hw/timer/exynos4210_mct.c +3 -2

index 6069116942a4..48041ab036a6 100644
@@ -53,6 +53,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "hw/sysbus.h"
 #include "qemu/timer.h"
 #include "qemu/main-loop.h"

@@ -1364,8 +1365,8 @@ static void exynos4210_mct_write(void *opaque, hwaddr offset,
     case L0_TCNTO: case L1_TCNTO:
     case L0_ICNTO: case L1_ICNTO:
     case L0_FRCNTO: case L1_FRCNTO:
-        fprintf(stderr, "\n[exynos4210.mct: write to RO register "
-                TARGET_FMT_plx "]\n\n", offset);
+        error_report("exynos4210.mct: write to RO register " TARGET_FMT_plx,
+                     offset);
         break;
 
     case L0_INT_CSTAT: case L1_INT_CSTAT:

hw/timer/exynos4210_pwm.c +5 -6

index f5765075c720..f8826e24e63d 100644
@@ -21,6 +21,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "hw/sysbus.h"
 #include "qemu/timer.h"
 #include "qemu-common.h"

@@ -252,9 +253,8 @@ static uint64_t exynos4210_pwm_read(void *opaque, hwaddr offset,
         break;
 
     default:
-        fprintf(stderr,
-                "[exynos4210.pwm: bad read offset " TARGET_FMT_plx "]\n",
-                offset);
+        error_report("exynos4210.pwm: bad read offset " TARGET_FMT_plx,
+                     offset);
         break;
     }
     return value;

@@ -343,9 +343,8 @@ static void exynos4210_pwm_write(void *opaque, hwaddr offset,
         break;
 
     default:
-        fprintf(stderr,
-                "[exynos4210.pwm: bad write offset " TARGET_FMT_plx "]\n",
-                offset);
+        error_report("exynos4210.pwm: bad write offset " TARGET_FMT_plx,
+                     offset);
         break;
 
     }

hw/timer/exynos4210_rtc.c +7 -9

index 1a648c5d9e67..f4548cd555f4 100644
@@ -26,6 +26,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "hw/sysbus.h"
 #include "qemu/timer.h"
 #include "qemu-common.h"

@@ -370,9 +371,8 @@ static uint64_t exynos4210_rtc_read(void *opaque, hwaddr offset,
         break;
 
     default:
-        fprintf(stderr,
-                "[exynos4210.rtc: bad read offset " TARGET_FMT_plx "]\n",
-                offset);
+        error_report("exynos4210.rtc: bad read offset " TARGET_FMT_plx,
+                     offset);
         break;
     }
     return value;

@@ -433,9 +433,8 @@ static void exynos4210_rtc_write(void *opaque, hwaddr offset,
         if (value > TICNT_THRESHOLD) {
             s->reg_ticcnt = value;
         } else {
-            fprintf(stderr,
-                    "[exynos4210.rtc: bad TICNT value %u ]\n",
-                    (uint32_t)value);
+            error_report("exynos4210.rtc: bad TICNT value %u",
+                         (uint32_t)value);
         }
         break;

@@ -500,9 +499,8 @@ static void exynos4210_rtc_write(void *opaque, hwaddr offset,
         break;
 
     default:
-        fprintf(stderr,
-                "[exynos4210.rtc: bad write offset " TARGET_FMT_plx "]\n",
-                offset);
+        error_report("exynos4210.rtc: bad write offset " TARGET_FMT_plx,
+                     offset);
         break;
 
     }

Signature · 1 line

2.9.3

Krzysztof Kozlowski

author maintainer

2017-03-13 18:39 UTC #2 lore raw

Re: [Qemu-arm] [PATCH v2 1/3] hw/arm/exynos: Convert fprintf to error_report()

Quoted 49 lines · Krzysztof Kozlowski

error_report() is preferred over fprintf() for logging errors. Also remove square brackets [] and additional new line characters in printed messages.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 hw/arm/exynos4_boards.c   |  6 +++---
 hw/timer/exynos4210_mct.c |  5 +++--
 hw/timer/exynos4210_pwm.c | 11 +++++------
 hw/timer/exynos4210_rtc.c | 16 +++++++---------
 4 files changed, 18 insertions(+), 20 deletions(-)

hw/arm/exynos4_boards.c +3 -3

index 0efa19405409..0a352815b86d 100644
@@ -22,6 +22,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "qemu-common.h"
 #include "cpu.h"
 #include "sysemu/sysemu.h"

@@ -101,9 +102,8 @@ static Exynos4210State *exynos4_boards_init_common(MachineState *machine,
     MachineClass *mc = MACHINE_GET_CLASS(machine);
 
     if (smp_cpus != EXYNOS4210_NCPUS && !qtest_enabled()) {
-        fprintf(stderr, "%s board supports only %d CPU cores. Ignoring smp_cpus"
-                " value.\n",
-                mc->name, EXYNOS4210_NCPUS);
+        error_report("%s board supports only %d CPU cores, ignoring smp_cpus value",
+                     mc->name, EXYNOS4210_NCPUS);
     }
 
     exynos4_board_binfo.ram_size = exynos4_board_ram_size[board_type];

hw/timer/exynos4210_mct.c +1 -0

index 6069116942a4..48041ab036a6 100644
@@ -53,6 +53,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "hw/sysbus.h"
 #include "qemu/timer.h"
 #include "qemu/main-loop.h"

hw/timer/exynos4210_mct.c static void exynos4210_mct_write(void *opaque, hwaddr offset, · line 1369

Quoted 7 lines

hw/timer/exynos4210_mct.c +2 -2

@@ -1364,8 +1365,8 @@ static void exynos4210_mct_write(void *opaque, hwaddr offset,
     case L0_TCNTO: case L1_TCNTO:
     case L0_ICNTO: case L1_ICNTO:
     case L0_FRCNTO: case L1_FRCNTO:
-        fprintf(stderr, "\n[exynos4210.mct: write to RO register "
-                TARGET_FMT_plx "]\n\n", offset);
+        error_report("exynos4210.mct: write to RO register " TARGET_FMT_plx,
+                     offset);

Ahh, I missed Philippe's suggestions here and other places. Sorry for the mess, I'll fix it and resend v3.

Best regards,
Krzysztof

Krzysztof Kozlowski

author maintainer

2017-03-13 18:36 UTC #3 lore raw

PATCH v2 2/3 hw/char/exynos4210_uart: Constify static array and few arguments

In reply to Krzysztof Kozlowski

The static array exynos4210_uart_regs with register values is not modified so it can be made const.

Few other functions accept driver or uart state as an argument but they do not change it and do not cast it so this can be made const for code safeness.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 hw/char/exynos4210_uart.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

hw/char/exynos4210_uart.c +4 -4

index b75f28d473bf..bff706ab3a68 100644
@@ -102,7 +102,7 @@ typedef struct Exynos4210UartReg {
     uint32_t            reset_value;
 } Exynos4210UartReg;
 
-static Exynos4210UartReg exynos4210_uart_regs[] = {
+static const Exynos4210UartReg exynos4210_uart_regs[] = {
     {"ULCON",    ULCON,    0x00000000},
     {"UCON",     UCON,     0x00003000},
     {"UFCON",    UFCON,    0x00000000},

@@ -220,7 +220,7 @@ static uint8_t fifo_retrieve(Exynos4210UartFIFO *q)
     return  ret;
 }
 
-static int fifo_elements_number(Exynos4210UartFIFO *q)
+static int fifo_elements_number(const Exynos4210UartFIFO *q)
 {
     if (q->sp < q->rp) {
         return q->size - q->rp + q->sp;

@@ -229,7 +229,7 @@ static int fifo_elements_number(Exynos4210UartFIFO *q)
     return q->sp - q->rp;
 }
 
-static int fifo_empty_elements_number(Exynos4210UartFIFO *q)
+static int fifo_empty_elements_number(const Exynos4210UartFIFO *q)
 {
     return q->size - fifo_elements_number(q);
 }

@@ -245,7 +245,7 @@ static void fifo_reset(Exynos4210UartFIFO *q)
     q->rp = 0;
 }
 
-static uint32_t exynos4210_uart_Tx_FIFO_trigger_level(Exynos4210UartState *s)
+static uint32_t exynos4210_uart_Tx_FIFO_trigger_level(const Exynos4210UartState *s)
 {
     uint32_t level = 0;
     uint32_t reg;

Signature · 1 line

2.9.3

Krzysztof Kozlowski

author maintainer

2017-03-13 18:36 UTC #4 lore raw

PATCH v2 3/3 hw/misc/exynos4210_pmu: Reorder local variables for readability

In reply to Krzysztof Kozlowski

Short declaration of 'i' was in the middle of declarations with assignments. Make it a little bit more readable. Additionally switch from "unsigned" to "unsigned int" as this pattern is more widely used. No functional change.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 hw/misc/exynos4210_pmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

hw/misc/exynos4210_pmu.c +2 -2

index e30dbc7d3d83..63a8ccd35559 100644
@@ -401,8 +401,8 @@ static uint64_t exynos4210_pmu_read(void *opaque, hwaddr offset,
                                     unsigned size)
 {
     Exynos4210PmuState *s = (Exynos4210PmuState *)opaque;
-    unsigned i;
     const Exynos4210PmuReg *reg_p = exynos4210_pmu_regs;
+    unsigned int i;
 
     for (i = 0; i < PMU_NUM_OF_REGISTERS; i++) {
         if (reg_p->offset == offset) {

@@ -420,8 +420,8 @@ static void exynos4210_pmu_write(void *opaque, hwaddr offset,
                                  uint64_t val, unsigned size)
 {
     Exynos4210PmuState *s = (Exynos4210PmuState *)opaque;
-    unsigned i;
     const Exynos4210PmuReg *reg_p = exynos4210_pmu_regs;
+    unsigned int i;
 
     for (i = 0; i < PMU_NUM_OF_REGISTERS; i++) {
         if (reg_p->offset == offset) {

Signature · 1 line

2.9.3