* [PATCH 01/17] hw/arm/spitz: Detabify
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 8:49 ` Philippe Mathieu-Daudé
2020-06-29 19:30 ` Alistair Francis
2020-06-28 14:24 ` [PATCH 02/17] hw/arm/spitz: Create SpitzMachineClass abstract base class Peter Maydell
` (16 subsequent siblings)
17 siblings, 2 replies; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
The spitz board has been around a long time, and still has a fair number
of hard-coded tab characters in it. We're about to do some work on
this source file, so start out by expanding out the tabs.
This commit is a pure whitespace only change.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Couple of checkpatch errors due to the QUEUE_KEY macro which can
be ignored as this is just a detabify.
---
hw/arm/spitz.c | 156 ++++++++++++++++++++++++-------------------------
1 file changed, 78 insertions(+), 78 deletions(-)
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index fc18212e686..9eaedab79b5 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -34,25 +34,25 @@
#include "cpu.h"
#undef REG_FMT
-#define REG_FMT "0x%02lx"
+#define REG_FMT "0x%02lx"
/* Spitz Flash */
-#define FLASH_BASE 0x0c000000
-#define FLASH_ECCLPLB 0x00 /* Line parity 7 - 0 bit */
-#define FLASH_ECCLPUB 0x04 /* Line parity 15 - 8 bit */
-#define FLASH_ECCCP 0x08 /* Column parity 5 - 0 bit */
-#define FLASH_ECCCNTR 0x0c /* ECC byte counter */
-#define FLASH_ECCCLRR 0x10 /* Clear ECC */
-#define FLASH_FLASHIO 0x14 /* Flash I/O */
-#define FLASH_FLASHCTL 0x18 /* Flash Control */
+#define FLASH_BASE 0x0c000000
+#define FLASH_ECCLPLB 0x00 /* Line parity 7 - 0 bit */
+#define FLASH_ECCLPUB 0x04 /* Line parity 15 - 8 bit */
+#define FLASH_ECCCP 0x08 /* Column parity 5 - 0 bit */
+#define FLASH_ECCCNTR 0x0c /* ECC byte counter */
+#define FLASH_ECCCLRR 0x10 /* Clear ECC */
+#define FLASH_FLASHIO 0x14 /* Flash I/O */
+#define FLASH_FLASHCTL 0x18 /* Flash Control */
-#define FLASHCTL_CE0 (1 << 0)
-#define FLASHCTL_CLE (1 << 1)
-#define FLASHCTL_ALE (1 << 2)
-#define FLASHCTL_WP (1 << 3)
-#define FLASHCTL_CE1 (1 << 4)
-#define FLASHCTL_RYBY (1 << 5)
-#define FLASHCTL_NCE (FLASHCTL_CE0 | FLASHCTL_CE1)
+#define FLASHCTL_CE0 (1 << 0)
+#define FLASHCTL_CLE (1 << 1)
+#define FLASHCTL_ALE (1 << 2)
+#define FLASHCTL_WP (1 << 3)
+#define FLASHCTL_CE1 (1 << 4)
+#define FLASHCTL_RYBY (1 << 5)
+#define FLASHCTL_NCE (FLASHCTL_CE0 | FLASHCTL_CE1)
#define TYPE_SL_NAND "sl-nand"
#define SL_NAND(obj) OBJECT_CHECK(SLNANDState, (obj), TYPE_SL_NAND)
@@ -74,12 +74,12 @@ static uint64_t sl_read(void *opaque, hwaddr addr, unsigned size)
int ryby;
switch (addr) {
-#define BSHR(byte, from, to) ((s->ecc.lp[byte] >> (from - to)) & (1 << to))
+#define BSHR(byte, from, to) ((s->ecc.lp[byte] >> (from - to)) & (1 << to))
case FLASH_ECCLPLB:
return BSHR(0, 4, 0) | BSHR(0, 5, 2) | BSHR(0, 6, 4) | BSHR(0, 7, 6) |
BSHR(1, 4, 1) | BSHR(1, 5, 3) | BSHR(1, 6, 5) | BSHR(1, 7, 7);
-#define BSHL(byte, from, to) ((s->ecc.lp[byte] << (to - from)) & (1 << to))
+#define BSHL(byte, from, to) ((s->ecc.lp[byte] << (to - from)) & (1 << to))
case FLASH_ECCLPUB:
return BSHL(0, 0, 0) | BSHL(0, 1, 2) | BSHL(0, 2, 4) | BSHL(0, 3, 6) |
BSHL(1, 0, 1) | BSHL(1, 1, 3) | BSHL(1, 2, 5) | BSHL(1, 3, 7);
@@ -191,8 +191,8 @@ static void sl_nand_realize(DeviceState *dev, Error **errp)
/* Spitz Keyboard */
-#define SPITZ_KEY_STROBE_NUM 11
-#define SPITZ_KEY_SENSE_NUM 7
+#define SPITZ_KEY_STROBE_NUM 11
+#define SPITZ_KEY_SENSE_NUM 7
static const int spitz_gpio_key_sense[SPITZ_KEY_SENSE_NUM] = {
12, 17, 91, 34, 36, 38, 39
@@ -214,11 +214,11 @@ static int spitz_keymap[SPITZ_KEY_SENSE_NUM + 1][SPITZ_KEY_STROBE_NUM] = {
{ 0x52, 0x43, 0x01, 0x47, 0x49, -1 , -1 , -1 , -1 , -1 , -1 },
};
-#define SPITZ_GPIO_AK_INT 13 /* Remote control */
-#define SPITZ_GPIO_SYNC 16 /* Sync button */
-#define SPITZ_GPIO_ON_KEY 95 /* Power button */
-#define SPITZ_GPIO_SWA 97 /* Lid */
-#define SPITZ_GPIO_SWB 96 /* Tablet mode */
+#define SPITZ_GPIO_AK_INT 13 /* Remote control */
+#define SPITZ_GPIO_SYNC 16 /* Sync button */
+#define SPITZ_GPIO_ON_KEY 95 /* Power button */
+#define SPITZ_GPIO_SWA 97 /* Lid */
+#define SPITZ_GPIO_SWB 96 /* Tablet mode */
/* The special buttons are mapped to unused keys */
static const int spitz_gpiomap[5] = {
@@ -300,7 +300,7 @@ static void spitz_keyboard_keydown(SpitzKeyboardState *s, int keycode)
#define SPITZ_MOD_CTRL (1 << 8)
#define SPITZ_MOD_FN (1 << 9)
-#define QUEUE_KEY(c) s->fifo[(s->fifopos + s->fifolen ++) & 0xf] = c
+#define QUEUE_KEY(c) s->fifo[(s->fifopos + s->fifolen ++) & 0xf] = c
static void spitz_keyboard_handler(void *opaque, int keycode)
{
@@ -308,25 +308,25 @@ static void spitz_keyboard_handler(void *opaque, int keycode)
uint16_t code;
int mapcode;
switch (keycode) {
- case 0x2a: /* Left Shift */
+ case 0x2a: /* Left Shift */
s->modifiers |= 1;
break;
case 0xaa:
s->modifiers &= ~1;
break;
- case 0x36: /* Right Shift */
+ case 0x36: /* Right Shift */
s->modifiers |= 2;
break;
case 0xb6:
s->modifiers &= ~2;
break;
- case 0x1d: /* Control */
+ case 0x1d: /* Control */
s->modifiers |= 4;
break;
case 0x9d:
s->modifiers &= ~4;
break;
- case 0x38: /* Alt */
+ case 0x38: /* Alt */
s->modifiers |= 8;
break;
case 0xb8:
@@ -536,14 +536,14 @@ static void spitz_keyboard_realize(DeviceState *dev, Error **errp)
/* LCD backlight controller */
-#define LCDTG_RESCTL 0x00
-#define LCDTG_PHACTRL 0x01
-#define LCDTG_DUTYCTRL 0x02
-#define LCDTG_POWERREG0 0x03
-#define LCDTG_POWERREG1 0x04
-#define LCDTG_GPOR3 0x05
-#define LCDTG_PICTRL 0x06
-#define LCDTG_POLCTRL 0x07
+#define LCDTG_RESCTL 0x00
+#define LCDTG_PHACTRL 0x01
+#define LCDTG_DUTYCTRL 0x02
+#define LCDTG_POWERREG0 0x03
+#define LCDTG_POWERREG1 0x04
+#define LCDTG_GPOR3 0x05
+#define LCDTG_PICTRL 0x06
+#define LCDTG_POLCTRL 0x07
typedef struct {
SSISlave ssidev;
@@ -623,12 +623,12 @@ static void spitz_lcdtg_realize(SSISlave *dev, Error **errp)
/* SSP devices */
-#define CORGI_SSP_PORT 2
+#define CORGI_SSP_PORT 2
-#define SPITZ_GPIO_LCDCON_CS 53
-#define SPITZ_GPIO_ADS7846_CS 14
-#define SPITZ_GPIO_MAX1111_CS 20
-#define SPITZ_GPIO_TP_INT 11
+#define SPITZ_GPIO_LCDCON_CS 53
+#define SPITZ_GPIO_ADS7846_CS 14
+#define SPITZ_GPIO_MAX1111_CS 20
+#define SPITZ_GPIO_TP_INT 11
static DeviceState *max1111;
@@ -659,13 +659,13 @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level)
s->enable[line] = !level;
}
-#define MAX1111_BATT_VOLT 1
-#define MAX1111_BATT_TEMP 2
-#define MAX1111_ACIN_VOLT 3
+#define MAX1111_BATT_VOLT 1
+#define MAX1111_BATT_TEMP 2
+#define MAX1111_ACIN_VOLT 3
-#define SPITZ_BATTERY_TEMP 0xe0 /* About 2.9V */
-#define SPITZ_BATTERY_VOLT 0xd0 /* About 4.0V */
-#define SPITZ_CHARGEON_ACIN 0x80 /* About 5.0V */
+#define SPITZ_BATTERY_TEMP 0xe0 /* About 2.9V */
+#define SPITZ_BATTERY_VOLT 0xd0 /* About 4.0V */
+#define SPITZ_CHARGEON_ACIN 0x80 /* About 5.0V */
static void spitz_adc_temp_on(void *opaque, int line, int level)
{
@@ -735,11 +735,11 @@ static void spitz_microdrive_attach(PXA2xxState *cpu, int slot)
/* Wm8750 and Max7310 on I2C */
-#define AKITA_MAX_ADDR 0x18
-#define SPITZ_WM_ADDRL 0x1b
-#define SPITZ_WM_ADDRH 0x1a
+#define AKITA_MAX_ADDR 0x18
+#define SPITZ_WM_ADDRL 0x1b
+#define SPITZ_WM_ADDRH 0x1a
-#define SPITZ_GPIO_WM 5
+#define SPITZ_GPIO_WM 5
static void spitz_wm8750_addr(void *opaque, int line, int level)
{
@@ -806,20 +806,20 @@ static void spitz_out_switch(void *opaque, int line, int level)
}
}
-#define SPITZ_SCP_LED_GREEN 1
-#define SPITZ_SCP_JK_B 2
-#define SPITZ_SCP_CHRG_ON 3
-#define SPITZ_SCP_MUTE_L 4
-#define SPITZ_SCP_MUTE_R 5
-#define SPITZ_SCP_CF_POWER 6
-#define SPITZ_SCP_LED_ORANGE 7
-#define SPITZ_SCP_JK_A 8
-#define SPITZ_SCP_ADC_TEMP_ON 9
-#define SPITZ_SCP2_IR_ON 1
-#define SPITZ_SCP2_AKIN_PULLUP 2
-#define SPITZ_SCP2_BACKLIGHT_CONT 7
-#define SPITZ_SCP2_BACKLIGHT_ON 8
-#define SPITZ_SCP2_MIC_BIAS 9
+#define SPITZ_SCP_LED_GREEN 1
+#define SPITZ_SCP_JK_B 2
+#define SPITZ_SCP_CHRG_ON 3
+#define SPITZ_SCP_MUTE_L 4
+#define SPITZ_SCP_MUTE_R 5
+#define SPITZ_SCP_CF_POWER 6
+#define SPITZ_SCP_LED_ORANGE 7
+#define SPITZ_SCP_JK_A 8
+#define SPITZ_SCP_ADC_TEMP_ON 9
+#define SPITZ_SCP2_IR_ON 1
+#define SPITZ_SCP2_AKIN_PULLUP 2
+#define SPITZ_SCP2_BACKLIGHT_CONT 7
+#define SPITZ_SCP2_BACKLIGHT_ON 8
+#define SPITZ_SCP2_MIC_BIAS 9
static void spitz_scoop_gpio_setup(PXA2xxState *cpu,
DeviceState *scp0, DeviceState *scp1)
@@ -839,15 +839,15 @@ static void spitz_scoop_gpio_setup(PXA2xxState *cpu,
qdev_connect_gpio_out(scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
}
-#define SPITZ_GPIO_HSYNC 22
-#define SPITZ_GPIO_SD_DETECT 9
-#define SPITZ_GPIO_SD_WP 81
-#define SPITZ_GPIO_ON_RESET 89
-#define SPITZ_GPIO_BAT_COVER 90
-#define SPITZ_GPIO_CF1_IRQ 105
-#define SPITZ_GPIO_CF1_CD 94
-#define SPITZ_GPIO_CF2_IRQ 106
-#define SPITZ_GPIO_CF2_CD 93
+#define SPITZ_GPIO_HSYNC 22
+#define SPITZ_GPIO_SD_DETECT 9
+#define SPITZ_GPIO_SD_WP 81
+#define SPITZ_GPIO_ON_RESET 89
+#define SPITZ_GPIO_BAT_COVER 90
+#define SPITZ_GPIO_CF1_IRQ 105
+#define SPITZ_GPIO_CF1_CD 94
+#define SPITZ_GPIO_CF2_IRQ 106
+#define SPITZ_GPIO_CF2_CD 93
static int spitz_hsync;
@@ -907,8 +907,8 @@ static void spitz_gpio_setup(PXA2xxState *cpu, int slots)
/* Board init. */
enum spitz_model_e { spitz, akita, borzoi, terrier };
-#define SPITZ_RAM 0x04000000
-#define SPITZ_ROM 0x00800000
+#define SPITZ_RAM 0x04000000
+#define SPITZ_ROM 0x00800000
static struct arm_boot_info spitz_binfo = {
.loader_start = PXA2XX_SDRAM_BASE,
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 01/17] hw/arm/spitz: Detabify
2020-06-28 14:24 ` [PATCH 01/17] hw/arm/spitz: Detabify Peter Maydell
@ 2020-06-29 8:49 ` Philippe Mathieu-Daudé
2020-06-29 19:30 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-29 8:49 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis
On 6/28/20 4:24 PM, Peter Maydell wrote:
> The spitz board has been around a long time, and still has a fair number
> of hard-coded tab characters in it. We're about to do some work on
> this source file, so start out by expanding out the tabs.
>
> This commit is a pure whitespace only change.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Couple of checkpatch errors due to the QUEUE_KEY macro which can
> be ignored as this is just a detabify.
> ---
> hw/arm/spitz.c | 156 ++++++++++++++++++++++++-------------------------
> 1 file changed, 78 insertions(+), 78 deletions(-)
'git-diff -w' -> no change.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 01/17] hw/arm/spitz: Detabify
2020-06-28 14:24 ` [PATCH 01/17] hw/arm/spitz: Detabify Peter Maydell
2020-06-29 8:49 ` Philippe Mathieu-Daudé
@ 2020-06-29 19:30 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Alistair Francis @ 2020-06-29 19:30 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sun, Jun 28, 2020 at 7:24 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The spitz board has been around a long time, and still has a fair number
> of hard-coded tab characters in it. We're about to do some work on
> this source file, so start out by expanding out the tabs.
>
> This commit is a pure whitespace only change.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> Couple of checkpatch errors due to the QUEUE_KEY macro which can
> be ignored as this is just a detabify.
> ---
> hw/arm/spitz.c | 156 ++++++++++++++++++++++++-------------------------
> 1 file changed, 78 insertions(+), 78 deletions(-)
>
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index fc18212e686..9eaedab79b5 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -34,25 +34,25 @@
> #include "cpu.h"
>
> #undef REG_FMT
> -#define REG_FMT "0x%02lx"
> +#define REG_FMT "0x%02lx"
>
> /* Spitz Flash */
> -#define FLASH_BASE 0x0c000000
> -#define FLASH_ECCLPLB 0x00 /* Line parity 7 - 0 bit */
> -#define FLASH_ECCLPUB 0x04 /* Line parity 15 - 8 bit */
> -#define FLASH_ECCCP 0x08 /* Column parity 5 - 0 bit */
> -#define FLASH_ECCCNTR 0x0c /* ECC byte counter */
> -#define FLASH_ECCCLRR 0x10 /* Clear ECC */
> -#define FLASH_FLASHIO 0x14 /* Flash I/O */
> -#define FLASH_FLASHCTL 0x18 /* Flash Control */
> +#define FLASH_BASE 0x0c000000
> +#define FLASH_ECCLPLB 0x00 /* Line parity 7 - 0 bit */
> +#define FLASH_ECCLPUB 0x04 /* Line parity 15 - 8 bit */
> +#define FLASH_ECCCP 0x08 /* Column parity 5 - 0 bit */
> +#define FLASH_ECCCNTR 0x0c /* ECC byte counter */
> +#define FLASH_ECCCLRR 0x10 /* Clear ECC */
> +#define FLASH_FLASHIO 0x14 /* Flash I/O */
> +#define FLASH_FLASHCTL 0x18 /* Flash Control */
>
> -#define FLASHCTL_CE0 (1 << 0)
> -#define FLASHCTL_CLE (1 << 1)
> -#define FLASHCTL_ALE (1 << 2)
> -#define FLASHCTL_WP (1 << 3)
> -#define FLASHCTL_CE1 (1 << 4)
> -#define FLASHCTL_RYBY (1 << 5)
> -#define FLASHCTL_NCE (FLASHCTL_CE0 | FLASHCTL_CE1)
> +#define FLASHCTL_CE0 (1 << 0)
> +#define FLASHCTL_CLE (1 << 1)
> +#define FLASHCTL_ALE (1 << 2)
> +#define FLASHCTL_WP (1 << 3)
> +#define FLASHCTL_CE1 (1 << 4)
> +#define FLASHCTL_RYBY (1 << 5)
> +#define FLASHCTL_NCE (FLASHCTL_CE0 | FLASHCTL_CE1)
>
> #define TYPE_SL_NAND "sl-nand"
> #define SL_NAND(obj) OBJECT_CHECK(SLNANDState, (obj), TYPE_SL_NAND)
> @@ -74,12 +74,12 @@ static uint64_t sl_read(void *opaque, hwaddr addr, unsigned size)
> int ryby;
>
> switch (addr) {
> -#define BSHR(byte, from, to) ((s->ecc.lp[byte] >> (from - to)) & (1 << to))
> +#define BSHR(byte, from, to) ((s->ecc.lp[byte] >> (from - to)) & (1 << to))
> case FLASH_ECCLPLB:
> return BSHR(0, 4, 0) | BSHR(0, 5, 2) | BSHR(0, 6, 4) | BSHR(0, 7, 6) |
> BSHR(1, 4, 1) | BSHR(1, 5, 3) | BSHR(1, 6, 5) | BSHR(1, 7, 7);
>
> -#define BSHL(byte, from, to) ((s->ecc.lp[byte] << (to - from)) & (1 << to))
> +#define BSHL(byte, from, to) ((s->ecc.lp[byte] << (to - from)) & (1 << to))
> case FLASH_ECCLPUB:
> return BSHL(0, 0, 0) | BSHL(0, 1, 2) | BSHL(0, 2, 4) | BSHL(0, 3, 6) |
> BSHL(1, 0, 1) | BSHL(1, 1, 3) | BSHL(1, 2, 5) | BSHL(1, 3, 7);
> @@ -191,8 +191,8 @@ static void sl_nand_realize(DeviceState *dev, Error **errp)
>
> /* Spitz Keyboard */
>
> -#define SPITZ_KEY_STROBE_NUM 11
> -#define SPITZ_KEY_SENSE_NUM 7
> +#define SPITZ_KEY_STROBE_NUM 11
> +#define SPITZ_KEY_SENSE_NUM 7
>
> static const int spitz_gpio_key_sense[SPITZ_KEY_SENSE_NUM] = {
> 12, 17, 91, 34, 36, 38, 39
> @@ -214,11 +214,11 @@ static int spitz_keymap[SPITZ_KEY_SENSE_NUM + 1][SPITZ_KEY_STROBE_NUM] = {
> { 0x52, 0x43, 0x01, 0x47, 0x49, -1 , -1 , -1 , -1 , -1 , -1 },
> };
>
> -#define SPITZ_GPIO_AK_INT 13 /* Remote control */
> -#define SPITZ_GPIO_SYNC 16 /* Sync button */
> -#define SPITZ_GPIO_ON_KEY 95 /* Power button */
> -#define SPITZ_GPIO_SWA 97 /* Lid */
> -#define SPITZ_GPIO_SWB 96 /* Tablet mode */
> +#define SPITZ_GPIO_AK_INT 13 /* Remote control */
> +#define SPITZ_GPIO_SYNC 16 /* Sync button */
> +#define SPITZ_GPIO_ON_KEY 95 /* Power button */
> +#define SPITZ_GPIO_SWA 97 /* Lid */
> +#define SPITZ_GPIO_SWB 96 /* Tablet mode */
>
> /* The special buttons are mapped to unused keys */
> static const int spitz_gpiomap[5] = {
> @@ -300,7 +300,7 @@ static void spitz_keyboard_keydown(SpitzKeyboardState *s, int keycode)
> #define SPITZ_MOD_CTRL (1 << 8)
> #define SPITZ_MOD_FN (1 << 9)
>
> -#define QUEUE_KEY(c) s->fifo[(s->fifopos + s->fifolen ++) & 0xf] = c
> +#define QUEUE_KEY(c) s->fifo[(s->fifopos + s->fifolen ++) & 0xf] = c
>
> static void spitz_keyboard_handler(void *opaque, int keycode)
> {
> @@ -308,25 +308,25 @@ static void spitz_keyboard_handler(void *opaque, int keycode)
> uint16_t code;
> int mapcode;
> switch (keycode) {
> - case 0x2a: /* Left Shift */
> + case 0x2a: /* Left Shift */
> s->modifiers |= 1;
> break;
> case 0xaa:
> s->modifiers &= ~1;
> break;
> - case 0x36: /* Right Shift */
> + case 0x36: /* Right Shift */
> s->modifiers |= 2;
> break;
> case 0xb6:
> s->modifiers &= ~2;
> break;
> - case 0x1d: /* Control */
> + case 0x1d: /* Control */
> s->modifiers |= 4;
> break;
> case 0x9d:
> s->modifiers &= ~4;
> break;
> - case 0x38: /* Alt */
> + case 0x38: /* Alt */
> s->modifiers |= 8;
> break;
> case 0xb8:
> @@ -536,14 +536,14 @@ static void spitz_keyboard_realize(DeviceState *dev, Error **errp)
>
> /* LCD backlight controller */
>
> -#define LCDTG_RESCTL 0x00
> -#define LCDTG_PHACTRL 0x01
> -#define LCDTG_DUTYCTRL 0x02
> -#define LCDTG_POWERREG0 0x03
> -#define LCDTG_POWERREG1 0x04
> -#define LCDTG_GPOR3 0x05
> -#define LCDTG_PICTRL 0x06
> -#define LCDTG_POLCTRL 0x07
> +#define LCDTG_RESCTL 0x00
> +#define LCDTG_PHACTRL 0x01
> +#define LCDTG_DUTYCTRL 0x02
> +#define LCDTG_POWERREG0 0x03
> +#define LCDTG_POWERREG1 0x04
> +#define LCDTG_GPOR3 0x05
> +#define LCDTG_PICTRL 0x06
> +#define LCDTG_POLCTRL 0x07
>
> typedef struct {
> SSISlave ssidev;
> @@ -623,12 +623,12 @@ static void spitz_lcdtg_realize(SSISlave *dev, Error **errp)
>
> /* SSP devices */
>
> -#define CORGI_SSP_PORT 2
> +#define CORGI_SSP_PORT 2
>
> -#define SPITZ_GPIO_LCDCON_CS 53
> -#define SPITZ_GPIO_ADS7846_CS 14
> -#define SPITZ_GPIO_MAX1111_CS 20
> -#define SPITZ_GPIO_TP_INT 11
> +#define SPITZ_GPIO_LCDCON_CS 53
> +#define SPITZ_GPIO_ADS7846_CS 14
> +#define SPITZ_GPIO_MAX1111_CS 20
> +#define SPITZ_GPIO_TP_INT 11
>
> static DeviceState *max1111;
>
> @@ -659,13 +659,13 @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level)
> s->enable[line] = !level;
> }
>
> -#define MAX1111_BATT_VOLT 1
> -#define MAX1111_BATT_TEMP 2
> -#define MAX1111_ACIN_VOLT 3
> +#define MAX1111_BATT_VOLT 1
> +#define MAX1111_BATT_TEMP 2
> +#define MAX1111_ACIN_VOLT 3
>
> -#define SPITZ_BATTERY_TEMP 0xe0 /* About 2.9V */
> -#define SPITZ_BATTERY_VOLT 0xd0 /* About 4.0V */
> -#define SPITZ_CHARGEON_ACIN 0x80 /* About 5.0V */
> +#define SPITZ_BATTERY_TEMP 0xe0 /* About 2.9V */
> +#define SPITZ_BATTERY_VOLT 0xd0 /* About 4.0V */
> +#define SPITZ_CHARGEON_ACIN 0x80 /* About 5.0V */
>
> static void spitz_adc_temp_on(void *opaque, int line, int level)
> {
> @@ -735,11 +735,11 @@ static void spitz_microdrive_attach(PXA2xxState *cpu, int slot)
>
> /* Wm8750 and Max7310 on I2C */
>
> -#define AKITA_MAX_ADDR 0x18
> -#define SPITZ_WM_ADDRL 0x1b
> -#define SPITZ_WM_ADDRH 0x1a
> +#define AKITA_MAX_ADDR 0x18
> +#define SPITZ_WM_ADDRL 0x1b
> +#define SPITZ_WM_ADDRH 0x1a
>
> -#define SPITZ_GPIO_WM 5
> +#define SPITZ_GPIO_WM 5
>
> static void spitz_wm8750_addr(void *opaque, int line, int level)
> {
> @@ -806,20 +806,20 @@ static void spitz_out_switch(void *opaque, int line, int level)
> }
> }
>
> -#define SPITZ_SCP_LED_GREEN 1
> -#define SPITZ_SCP_JK_B 2
> -#define SPITZ_SCP_CHRG_ON 3
> -#define SPITZ_SCP_MUTE_L 4
> -#define SPITZ_SCP_MUTE_R 5
> -#define SPITZ_SCP_CF_POWER 6
> -#define SPITZ_SCP_LED_ORANGE 7
> -#define SPITZ_SCP_JK_A 8
> -#define SPITZ_SCP_ADC_TEMP_ON 9
> -#define SPITZ_SCP2_IR_ON 1
> -#define SPITZ_SCP2_AKIN_PULLUP 2
> -#define SPITZ_SCP2_BACKLIGHT_CONT 7
> -#define SPITZ_SCP2_BACKLIGHT_ON 8
> -#define SPITZ_SCP2_MIC_BIAS 9
> +#define SPITZ_SCP_LED_GREEN 1
> +#define SPITZ_SCP_JK_B 2
> +#define SPITZ_SCP_CHRG_ON 3
> +#define SPITZ_SCP_MUTE_L 4
> +#define SPITZ_SCP_MUTE_R 5
> +#define SPITZ_SCP_CF_POWER 6
> +#define SPITZ_SCP_LED_ORANGE 7
> +#define SPITZ_SCP_JK_A 8
> +#define SPITZ_SCP_ADC_TEMP_ON 9
> +#define SPITZ_SCP2_IR_ON 1
> +#define SPITZ_SCP2_AKIN_PULLUP 2
> +#define SPITZ_SCP2_BACKLIGHT_CONT 7
> +#define SPITZ_SCP2_BACKLIGHT_ON 8
> +#define SPITZ_SCP2_MIC_BIAS 9
>
> static void spitz_scoop_gpio_setup(PXA2xxState *cpu,
> DeviceState *scp0, DeviceState *scp1)
> @@ -839,15 +839,15 @@ static void spitz_scoop_gpio_setup(PXA2xxState *cpu,
> qdev_connect_gpio_out(scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
> }
>
> -#define SPITZ_GPIO_HSYNC 22
> -#define SPITZ_GPIO_SD_DETECT 9
> -#define SPITZ_GPIO_SD_WP 81
> -#define SPITZ_GPIO_ON_RESET 89
> -#define SPITZ_GPIO_BAT_COVER 90
> -#define SPITZ_GPIO_CF1_IRQ 105
> -#define SPITZ_GPIO_CF1_CD 94
> -#define SPITZ_GPIO_CF2_IRQ 106
> -#define SPITZ_GPIO_CF2_CD 93
> +#define SPITZ_GPIO_HSYNC 22
> +#define SPITZ_GPIO_SD_DETECT 9
> +#define SPITZ_GPIO_SD_WP 81
> +#define SPITZ_GPIO_ON_RESET 89
> +#define SPITZ_GPIO_BAT_COVER 90
> +#define SPITZ_GPIO_CF1_IRQ 105
> +#define SPITZ_GPIO_CF1_CD 94
> +#define SPITZ_GPIO_CF2_IRQ 106
> +#define SPITZ_GPIO_CF2_CD 93
>
> static int spitz_hsync;
>
> @@ -907,8 +907,8 @@ static void spitz_gpio_setup(PXA2xxState *cpu, int slots)
> /* Board init. */
> enum spitz_model_e { spitz, akita, borzoi, terrier };
>
> -#define SPITZ_RAM 0x04000000
> -#define SPITZ_ROM 0x00800000
> +#define SPITZ_RAM 0x04000000
> +#define SPITZ_ROM 0x00800000
>
> static struct arm_boot_info spitz_binfo = {
> .loader_start = PXA2XX_SDRAM_BASE,
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 02/17] hw/arm/spitz: Create SpitzMachineClass abstract base class
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
2020-06-28 14:24 ` [PATCH 01/17] hw/arm/spitz: Detabify Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 8:55 ` Philippe Mathieu-Daudé
2020-06-28 14:24 ` [PATCH 03/17] hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState Peter Maydell
` (15 subsequent siblings)
17 siblings, 1 reply; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
For the four Spitz-family machines (akita, borzoi, spitz, terrier)
create a proper abstract class SpitzMachineClass which encapsulates
the common behaviour, rather than having them all derive directly
from TYPE_MACHINE:
* instead of each machine class setting mc->init to a wrapper
function which calls spitz_common_init() with parameters,
put that data in the SpitzMachineClass and make spitz_common_init
the SpitzMachineClass machine-init function
* move the settings of mc->block_default_type and
mc->ignore_memory_transaction_failures into the SpitzMachineClass
class init rather than repeating them in each machine's class init
(The motivation is that we're going to want to keep some state in
the SpitzMachineState so we can connect GPIOs between devices created
in one sub-function of the machine init to devices created in a
different sub-function.)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/spitz.c | 91 ++++++++++++++++++++++++++++++--------------------
1 file changed, 55 insertions(+), 36 deletions(-)
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 9eaedab79b5..c70e912a33d 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -33,6 +33,26 @@
#include "exec/address-spaces.h"
#include "cpu.h"
+enum spitz_model_e { spitz, akita, borzoi, terrier };
+
+typedef struct {
+ MachineClass parent;
+ enum spitz_model_e model;
+ int arm_id;
+} SpitzMachineClass;
+
+typedef struct {
+ MachineState parent;
+} SpitzMachineState;
+
+#define TYPE_SPITZ_MACHINE "spitz-common"
+#define SPITZ_MACHINE(obj) \
+ OBJECT_CHECK(SpitzMachineState, obj, TYPE_SPITZ_MACHINE)
+#define SPITZ_MACHINE_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(SpitzMachineClass, obj, TYPE_SPITZ_MACHINE)
+#define SPITZ_MACHINE_CLASS(klass) \
+ OBJECT_CLASS_CHECK(SpitzMachineClass, klass, TYPE_SPITZ_MACHINE)
+
#undef REG_FMT
#define REG_FMT "0x%02lx"
@@ -905,8 +925,6 @@ static void spitz_gpio_setup(PXA2xxState *cpu, int slots)
}
/* Board init. */
-enum spitz_model_e { spitz, akita, borzoi, terrier };
-
#define SPITZ_RAM 0x04000000
#define SPITZ_ROM 0x00800000
@@ -915,9 +933,10 @@ static struct arm_boot_info spitz_binfo = {
.ram_size = 0x04000000,
};
-static void spitz_common_init(MachineState *machine,
- enum spitz_model_e model, int arm_id)
+static void spitz_common_init(MachineState *machine)
{
+ SpitzMachineClass *smc = SPITZ_MACHINE_GET_CLASS(machine);
+ enum spitz_model_e model = smc->model;
PXA2xxState *mpu;
DeviceState *scp0, *scp1 = NULL;
MemoryRegion *address_space_mem = get_system_memory();
@@ -958,100 +977,100 @@ static void spitz_common_init(MachineState *machine,
/* A 4.0 GB microdrive is permanently sitting in CF slot 0. */
spitz_microdrive_attach(mpu, 0);
- spitz_binfo.board_id = arm_id;
+ spitz_binfo.board_id = smc->arm_id;
arm_load_kernel(mpu->cpu, machine, &spitz_binfo);
sl_bootparam_write(SL_PXA_PARAM_BASE);
}
-static void spitz_init(MachineState *machine)
+static void spitz_common_class_init(ObjectClass *oc, void *data)
{
- spitz_common_init(machine, spitz, 0x2c9);
+ MachineClass *mc = MACHINE_CLASS(oc);
+
+ mc->block_default_type = IF_IDE;
+ mc->ignore_memory_transaction_failures = true;
+ mc->init = spitz_common_init;
}
-static void borzoi_init(MachineState *machine)
-{
- spitz_common_init(machine, borzoi, 0x33f);
-}
-
-static void akita_init(MachineState *machine)
-{
- spitz_common_init(machine, akita, 0x2e8);
-}
-
-static void terrier_init(MachineState *machine)
-{
- spitz_common_init(machine, terrier, 0x33f);
-}
+static const TypeInfo spitz_common_info = {
+ .name = TYPE_SPITZ_MACHINE,
+ .parent = TYPE_MACHINE,
+ .abstract = true,
+ .instance_size = sizeof(SpitzMachineState),
+ .class_size = sizeof(SpitzMachineClass),
+ .class_init = spitz_common_class_init,
+};
static void akitapda_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
+ SpitzMachineClass *smc = SPITZ_MACHINE_CLASS(oc);
mc->desc = "Sharp SL-C1000 (Akita) PDA (PXA270)";
- mc->init = akita_init;
- mc->ignore_memory_transaction_failures = true;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("pxa270-c0");
+ smc->model = akita;
+ smc->arm_id = 0x2e8;
}
static const TypeInfo akitapda_type = {
.name = MACHINE_TYPE_NAME("akita"),
- .parent = TYPE_MACHINE,
+ .parent = TYPE_SPITZ_MACHINE,
.class_init = akitapda_class_init,
};
static void spitzpda_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
+ SpitzMachineClass *smc = SPITZ_MACHINE_CLASS(oc);
mc->desc = "Sharp SL-C3000 (Spitz) PDA (PXA270)";
- mc->init = spitz_init;
- mc->block_default_type = IF_IDE;
- mc->ignore_memory_transaction_failures = true;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("pxa270-c0");
+ smc->model = spitz;
+ smc->arm_id = 0x2c9;
}
static const TypeInfo spitzpda_type = {
.name = MACHINE_TYPE_NAME("spitz"),
- .parent = TYPE_MACHINE,
+ .parent = TYPE_SPITZ_MACHINE,
.class_init = spitzpda_class_init,
};
static void borzoipda_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
+ SpitzMachineClass *smc = SPITZ_MACHINE_CLASS(oc);
mc->desc = "Sharp SL-C3100 (Borzoi) PDA (PXA270)";
- mc->init = borzoi_init;
- mc->block_default_type = IF_IDE;
- mc->ignore_memory_transaction_failures = true;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("pxa270-c0");
+ smc->model = borzoi;
+ smc->arm_id = 0x33f;
}
static const TypeInfo borzoipda_type = {
.name = MACHINE_TYPE_NAME("borzoi"),
- .parent = TYPE_MACHINE,
+ .parent = TYPE_SPITZ_MACHINE,
.class_init = borzoipda_class_init,
};
static void terrierpda_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
+ SpitzMachineClass *smc = SPITZ_MACHINE_CLASS(oc);
mc->desc = "Sharp SL-C3200 (Terrier) PDA (PXA270)";
- mc->init = terrier_init;
- mc->block_default_type = IF_IDE;
- mc->ignore_memory_transaction_failures = true;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("pxa270-c5");
+ smc->model = terrier;
+ smc->arm_id = 0x33f;
}
static const TypeInfo terrierpda_type = {
.name = MACHINE_TYPE_NAME("terrier"),
- .parent = TYPE_MACHINE,
+ .parent = TYPE_SPITZ_MACHINE,
.class_init = terrierpda_class_init,
};
static void spitz_machine_init(void)
{
+ type_register_static(&spitz_common_info);
type_register_static(&akitapda_type);
type_register_static(&spitzpda_type);
type_register_static(&borzoipda_type);
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 02/17] hw/arm/spitz: Create SpitzMachineClass abstract base class
2020-06-28 14:24 ` [PATCH 02/17] hw/arm/spitz: Create SpitzMachineClass abstract base class Peter Maydell
@ 2020-06-29 8:55 ` Philippe Mathieu-Daudé
2020-06-29 14:03 ` Peter Maydell
0 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-29 8:55 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis
On 6/28/20 4:24 PM, Peter Maydell wrote:
> For the four Spitz-family machines (akita, borzoi, spitz, terrier)
> create a proper abstract class SpitzMachineClass which encapsulates
> the common behaviour, rather than having them all derive directly
> from TYPE_MACHINE:
> * instead of each machine class setting mc->init to a wrapper
> function which calls spitz_common_init() with parameters,
> put that data in the SpitzMachineClass and make spitz_common_init
> the SpitzMachineClass machine-init function
> * move the settings of mc->block_default_type and
> mc->ignore_memory_transaction_failures into the SpitzMachineClass
> class init rather than repeating them in each machine's class init
>
> (The motivation is that we're going to want to keep some state in
> the SpitzMachineState so we can connect GPIOs between devices created
> in one sub-function of the machine init to devices created in a
> different sub-function.)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/arm/spitz.c | 91 ++++++++++++++++++++++++++++++--------------------
> 1 file changed, 55 insertions(+), 36 deletions(-)
>
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index 9eaedab79b5..c70e912a33d 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -33,6 +33,26 @@
> #include "exec/address-spaces.h"
> #include "cpu.h"
>
> +enum spitz_model_e { spitz, akita, borzoi, terrier };
> +
> +typedef struct {
> + MachineClass parent;
> + enum spitz_model_e model;
Nitpick, I'd drop the not very useful typedef and use
directly ...:
enum { spitz, akita, borzoi, terrier } model
> + int arm_id;
> +} SpitzMachineClass;
> +
> +typedef struct {
> + MachineState parent;
> +} SpitzMachineState;
> +
> +#define TYPE_SPITZ_MACHINE "spitz-common"
> +#define SPITZ_MACHINE(obj) \
> + OBJECT_CHECK(SpitzMachineState, obj, TYPE_SPITZ_MACHINE)
> +#define SPITZ_MACHINE_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(SpitzMachineClass, obj, TYPE_SPITZ_MACHINE)
> +#define SPITZ_MACHINE_CLASS(klass) \
> + OBJECT_CLASS_CHECK(SpitzMachineClass, klass, TYPE_SPITZ_MACHINE)
> +
> #undef REG_FMT
> #define REG_FMT "0x%02lx"
>
> @@ -905,8 +925,6 @@ static void spitz_gpio_setup(PXA2xxState *cpu, int slots)
> }
>
> /* Board init. */
> -enum spitz_model_e { spitz, akita, borzoi, terrier };
> -
> #define SPITZ_RAM 0x04000000
> #define SPITZ_ROM 0x00800000
>
> @@ -915,9 +933,10 @@ static struct arm_boot_info spitz_binfo = {
> .ram_size = 0x04000000,
> };
>
> -static void spitz_common_init(MachineState *machine,
> - enum spitz_model_e model, int arm_id)
> +static void spitz_common_init(MachineState *machine)
> {
> + SpitzMachineClass *smc = SPITZ_MACHINE_GET_CLASS(machine);
> + enum spitz_model_e model = smc->model;
... and use 'smc->model' in place.
Patch easier to review with 'git-diff -W' [*].
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
[*] Content of my .gitattributes:
$ cat .gitattributes
*.c diff=c
*.cpp diff=cpp
*.m text diff=objc
*.h diff=c
*.py diff=python
*.json text
*.pl text diff=perl
*.sh text eol=lf
*.xml text
*.yml text
*.bz2 binary
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 02/17] hw/arm/spitz: Create SpitzMachineClass abstract base class
2020-06-29 8:55 ` Philippe Mathieu-Daudé
@ 2020-06-29 14:03 ` Peter Maydell
0 siblings, 0 replies; 50+ messages in thread
From: Peter Maydell @ 2020-06-29 14:03 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-arm, QEMU Developers, Alistair Francis
On Mon, 29 Jun 2020 at 09:55, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 6/28/20 4:24 PM, Peter Maydell wrote:
> > For the four Spitz-family machines (akita, borzoi, spitz, terrier)
> > create a proper abstract class SpitzMachineClass which encapsulates
> > the common behaviour, rather than having them all derive directly
> > from TYPE_MACHINE:
> > * instead of each machine class setting mc->init to a wrapper
> > function which calls spitz_common_init() with parameters,
> > put that data in the SpitzMachineClass and make spitz_common_init
> > the SpitzMachineClass machine-init function
> > * move the settings of mc->block_default_type and
> > mc->ignore_memory_transaction_failures into the SpitzMachineClass
> > class init rather than repeating them in each machine's class init
> >
> > (The motivation is that we're going to want to keep some state in
> > the SpitzMachineState so we can connect GPIOs between devices created
> > in one sub-function of the machine init to devices created in a
> > different sub-function.)
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > hw/arm/spitz.c | 91 ++++++++++++++++++++++++++++++--------------------
> > 1 file changed, 55 insertions(+), 36 deletions(-)
> >
> > diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> > index 9eaedab79b5..c70e912a33d 100644
> > --- a/hw/arm/spitz.c
> > +++ b/hw/arm/spitz.c
> > @@ -33,6 +33,26 @@
> > #include "exec/address-spaces.h"
> > #include "cpu.h"
> >
> > +enum spitz_model_e { spitz, akita, borzoi, terrier };
> > +
> > +typedef struct {
> > + MachineClass parent;
> > + enum spitz_model_e model;
>
> Nitpick, I'd drop the not very useful typedef and use
> directly ...:
>
> enum { spitz, akita, borzoi, terrier } model
This is just code motion, I didn't want to mess with the
existing way this enum was defined.
> > -static void spitz_common_init(MachineState *machine,
> > - enum spitz_model_e model, int arm_id)
> > +static void spitz_common_init(MachineState *machine)
> > {
> > + SpitzMachineClass *smc = SPITZ_MACHINE_GET_CLASS(machine);
> > + enum spitz_model_e model = smc->model;
>
> ... and use 'smc->model' in place.
Similarly here I used smc->arm_id in-place because there
was only one user, but went for model = smc->model because
there were multiple users and it would have put extra
changed lines into the patch that I didn't think were
necessary.
thanks
-- PMM
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 03/17] hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
2020-06-28 14:24 ` [PATCH 01/17] hw/arm/spitz: Detabify Peter Maydell
2020-06-28 14:24 ` [PATCH 02/17] hw/arm/spitz: Create SpitzMachineClass abstract base class Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 19:36 ` Alistair Francis
2020-06-28 14:24 ` [PATCH 04/17] hw/arm/spitz: Keep pointers to scp0, scp1 " Peter Maydell
` (14 subsequent siblings)
17 siblings, 1 reply; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
Keep pointers to the MPU and the SSI devices in SpitzMachineState.
We're going to want to make GPIO connections between some of the
SSI devices and the SCPs, so we want to keep hold of a pointer to
those; putting the MPU into the struct allows us to pass just
one thing to spitz_ssp_attach() rather than two.
We have to retain the setting of the global "max1111" variable
for the moment as it is used in spitz_adc_temp_on(); later in
this series of commits we will be able to remove it.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/spitz.c | 50 ++++++++++++++++++++++++++++----------------------
1 file changed, 28 insertions(+), 22 deletions(-)
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index c70e912a33d..f48e966c047 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -43,6 +43,11 @@ typedef struct {
typedef struct {
MachineState parent;
+ PXA2xxState *mpu;
+ DeviceState *mux;
+ DeviceState *lcdtg;
+ DeviceState *ads7846;
+ DeviceState *max1111;
} SpitzMachineState;
#define TYPE_SPITZ_MACHINE "spitz-common"
@@ -709,34 +714,33 @@ static void corgi_ssp_realize(SSISlave *d, Error **errp)
s->bus[2] = ssi_create_bus(dev, "ssi2");
}
-static void spitz_ssp_attach(PXA2xxState *cpu)
+static void spitz_ssp_attach(SpitzMachineState *sms)
{
- DeviceState *mux;
- DeviceState *dev;
void *bus;
- mux = ssi_create_slave(cpu->ssp[CORGI_SSP_PORT - 1], "corgi-ssp");
+ sms->mux = ssi_create_slave(sms->mpu->ssp[CORGI_SSP_PORT - 1], "corgi-ssp");
- bus = qdev_get_child_bus(mux, "ssi0");
- ssi_create_slave(bus, "spitz-lcdtg");
+ bus = qdev_get_child_bus(sms->mux, "ssi0");
+ sms->lcdtg = ssi_create_slave(bus, "spitz-lcdtg");
- bus = qdev_get_child_bus(mux, "ssi1");
- dev = ssi_create_slave(bus, "ads7846");
- qdev_connect_gpio_out(dev, 0,
- qdev_get_gpio_in(cpu->gpio, SPITZ_GPIO_TP_INT));
+ bus = qdev_get_child_bus(sms->mux, "ssi1");
+ sms->ads7846 = ssi_create_slave(bus, "ads7846");
+ qdev_connect_gpio_out(sms->ads7846, 0,
+ qdev_get_gpio_in(sms->mpu->gpio, SPITZ_GPIO_TP_INT));
- bus = qdev_get_child_bus(mux, "ssi2");
- max1111 = ssi_create_slave(bus, "max1111");
- max111x_set_input(max1111, MAX1111_BATT_VOLT, SPITZ_BATTERY_VOLT);
- max111x_set_input(max1111, MAX1111_BATT_TEMP, 0);
- max111x_set_input(max1111, MAX1111_ACIN_VOLT, SPITZ_CHARGEON_ACIN);
+ bus = qdev_get_child_bus(sms->mux, "ssi2");
+ sms->max1111 = ssi_create_slave(bus, "max1111");
+ max1111 = sms->max1111;
+ max111x_set_input(sms->max1111, MAX1111_BATT_VOLT, SPITZ_BATTERY_VOLT);
+ max111x_set_input(sms->max1111, MAX1111_BATT_TEMP, 0);
+ max111x_set_input(sms->max1111, MAX1111_ACIN_VOLT, SPITZ_CHARGEON_ACIN);
- qdev_connect_gpio_out(cpu->gpio, SPITZ_GPIO_LCDCON_CS,
- qdev_get_gpio_in(mux, 0));
- qdev_connect_gpio_out(cpu->gpio, SPITZ_GPIO_ADS7846_CS,
- qdev_get_gpio_in(mux, 1));
- qdev_connect_gpio_out(cpu->gpio, SPITZ_GPIO_MAX1111_CS,
- qdev_get_gpio_in(mux, 2));
+ qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_LCDCON_CS,
+ qdev_get_gpio_in(sms->mux, 0));
+ qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_ADS7846_CS,
+ qdev_get_gpio_in(sms->mux, 1));
+ qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_MAX1111_CS,
+ qdev_get_gpio_in(sms->mux, 2));
}
/* CF Microdrive */
@@ -936,6 +940,7 @@ static struct arm_boot_info spitz_binfo = {
static void spitz_common_init(MachineState *machine)
{
SpitzMachineClass *smc = SPITZ_MACHINE_GET_CLASS(machine);
+ SpitzMachineState *sms = SPITZ_MACHINE(machine);
enum spitz_model_e model = smc->model;
PXA2xxState *mpu;
DeviceState *scp0, *scp1 = NULL;
@@ -945,6 +950,7 @@ static void spitz_common_init(MachineState *machine)
/* Setup CPU & memory */
mpu = pxa270_init(address_space_mem, spitz_binfo.ram_size,
machine->cpu_type);
+ sms->mpu = mpu;
sl_flash_register(mpu, (model == spitz) ? FLASH_128M : FLASH_1024M);
@@ -954,7 +960,7 @@ static void spitz_common_init(MachineState *machine)
/* Setup peripherals */
spitz_keyboard_register(mpu);
- spitz_ssp_attach(mpu);
+ spitz_ssp_attach(sms);
scp0 = sysbus_create_simple("scoop", 0x10800000, NULL);
if (model != akita) {
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 03/17] hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState
2020-06-28 14:24 ` [PATCH 03/17] hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState Peter Maydell
@ 2020-06-29 19:36 ` Alistair Francis
0 siblings, 0 replies; 50+ messages in thread
From: Alistair Francis @ 2020-06-29 19:36 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sun, Jun 28, 2020 at 7:24 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Keep pointers to the MPU and the SSI devices in SpitzMachineState.
> We're going to want to make GPIO connections between some of the
> SSI devices and the SCPs, so we want to keep hold of a pointer to
> those; putting the MPU into the struct allows us to pass just
> one thing to spitz_ssp_attach() rather than two.
>
> We have to retain the setting of the global "max1111" variable
> for the moment as it is used in spitz_adc_temp_on(); later in
> this series of commits we will be able to remove it.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/arm/spitz.c | 50 ++++++++++++++++++++++++++++----------------------
> 1 file changed, 28 insertions(+), 22 deletions(-)
>
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index c70e912a33d..f48e966c047 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -43,6 +43,11 @@ typedef struct {
>
> typedef struct {
> MachineState parent;
> + PXA2xxState *mpu;
> + DeviceState *mux;
> + DeviceState *lcdtg;
> + DeviceState *ads7846;
> + DeviceState *max1111;
> } SpitzMachineState;
>
> #define TYPE_SPITZ_MACHINE "spitz-common"
> @@ -709,34 +714,33 @@ static void corgi_ssp_realize(SSISlave *d, Error **errp)
> s->bus[2] = ssi_create_bus(dev, "ssi2");
> }
>
> -static void spitz_ssp_attach(PXA2xxState *cpu)
> +static void spitz_ssp_attach(SpitzMachineState *sms)
> {
> - DeviceState *mux;
> - DeviceState *dev;
> void *bus;
>
> - mux = ssi_create_slave(cpu->ssp[CORGI_SSP_PORT - 1], "corgi-ssp");
> + sms->mux = ssi_create_slave(sms->mpu->ssp[CORGI_SSP_PORT - 1], "corgi-ssp");
>
> - bus = qdev_get_child_bus(mux, "ssi0");
> - ssi_create_slave(bus, "spitz-lcdtg");
> + bus = qdev_get_child_bus(sms->mux, "ssi0");
> + sms->lcdtg = ssi_create_slave(bus, "spitz-lcdtg");
>
> - bus = qdev_get_child_bus(mux, "ssi1");
> - dev = ssi_create_slave(bus, "ads7846");
> - qdev_connect_gpio_out(dev, 0,
> - qdev_get_gpio_in(cpu->gpio, SPITZ_GPIO_TP_INT));
> + bus = qdev_get_child_bus(sms->mux, "ssi1");
> + sms->ads7846 = ssi_create_slave(bus, "ads7846");
> + qdev_connect_gpio_out(sms->ads7846, 0,
> + qdev_get_gpio_in(sms->mpu->gpio, SPITZ_GPIO_TP_INT));
>
> - bus = qdev_get_child_bus(mux, "ssi2");
> - max1111 = ssi_create_slave(bus, "max1111");
> - max111x_set_input(max1111, MAX1111_BATT_VOLT, SPITZ_BATTERY_VOLT);
> - max111x_set_input(max1111, MAX1111_BATT_TEMP, 0);
> - max111x_set_input(max1111, MAX1111_ACIN_VOLT, SPITZ_CHARGEON_ACIN);
> + bus = qdev_get_child_bus(sms->mux, "ssi2");
> + sms->max1111 = ssi_create_slave(bus, "max1111");
> + max1111 = sms->max1111;
> + max111x_set_input(sms->max1111, MAX1111_BATT_VOLT, SPITZ_BATTERY_VOLT);
> + max111x_set_input(sms->max1111, MAX1111_BATT_TEMP, 0);
> + max111x_set_input(sms->max1111, MAX1111_ACIN_VOLT, SPITZ_CHARGEON_ACIN);
>
> - qdev_connect_gpio_out(cpu->gpio, SPITZ_GPIO_LCDCON_CS,
> - qdev_get_gpio_in(mux, 0));
> - qdev_connect_gpio_out(cpu->gpio, SPITZ_GPIO_ADS7846_CS,
> - qdev_get_gpio_in(mux, 1));
> - qdev_connect_gpio_out(cpu->gpio, SPITZ_GPIO_MAX1111_CS,
> - qdev_get_gpio_in(mux, 2));
> + qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_LCDCON_CS,
> + qdev_get_gpio_in(sms->mux, 0));
> + qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_ADS7846_CS,
> + qdev_get_gpio_in(sms->mux, 1));
> + qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_MAX1111_CS,
> + qdev_get_gpio_in(sms->mux, 2));
> }
>
> /* CF Microdrive */
> @@ -936,6 +940,7 @@ static struct arm_boot_info spitz_binfo = {
> static void spitz_common_init(MachineState *machine)
> {
> SpitzMachineClass *smc = SPITZ_MACHINE_GET_CLASS(machine);
> + SpitzMachineState *sms = SPITZ_MACHINE(machine);
> enum spitz_model_e model = smc->model;
> PXA2xxState *mpu;
> DeviceState *scp0, *scp1 = NULL;
> @@ -945,6 +950,7 @@ static void spitz_common_init(MachineState *machine)
> /* Setup CPU & memory */
> mpu = pxa270_init(address_space_mem, spitz_binfo.ram_size,
> machine->cpu_type);
> + sms->mpu = mpu;
>
> sl_flash_register(mpu, (model == spitz) ? FLASH_128M : FLASH_1024M);
>
> @@ -954,7 +960,7 @@ static void spitz_common_init(MachineState *machine)
> /* Setup peripherals */
> spitz_keyboard_register(mpu);
>
> - spitz_ssp_attach(mpu);
> + spitz_ssp_attach(sms);
>
> scp0 = sysbus_create_simple("scoop", 0x10800000, NULL);
> if (model != akita) {
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 04/17] hw/arm/spitz: Keep pointers to scp0, scp1 in SpitzMachineState
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (2 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 03/17] hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 19:38 ` Alistair Francis
2020-06-28 14:24 ` [PATCH 05/17] hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals Peter Maydell
` (13 subsequent siblings)
17 siblings, 1 reply; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
Keep pointers to scp0, scp1 in SpitzMachineState, and just pass
that to spitz_scoop_gpio_setup().
(We'll want to use some of the other fields in SpitzMachineState
in that function in the next commit.)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/spitz.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index f48e966c047..69bc2b3fa10 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -48,6 +48,8 @@ typedef struct {
DeviceState *lcdtg;
DeviceState *ads7846;
DeviceState *max1111;
+ DeviceState *scp0;
+ DeviceState *scp1;
} SpitzMachineState;
#define TYPE_SPITZ_MACHINE "spitz-common"
@@ -845,22 +847,23 @@ static void spitz_out_switch(void *opaque, int line, int level)
#define SPITZ_SCP2_BACKLIGHT_ON 8
#define SPITZ_SCP2_MIC_BIAS 9
-static void spitz_scoop_gpio_setup(PXA2xxState *cpu,
- DeviceState *scp0, DeviceState *scp1)
+static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
{
- qemu_irq *outsignals = qemu_allocate_irqs(spitz_out_switch, cpu, 8);
+ qemu_irq *outsignals = qemu_allocate_irqs(spitz_out_switch, sms->mpu, 8);
- qdev_connect_gpio_out(scp0, SPITZ_SCP_CHRG_ON, outsignals[0]);
- qdev_connect_gpio_out(scp0, SPITZ_SCP_JK_B, outsignals[1]);
- qdev_connect_gpio_out(scp0, SPITZ_SCP_LED_GREEN, outsignals[2]);
- qdev_connect_gpio_out(scp0, SPITZ_SCP_LED_ORANGE, outsignals[3]);
+ qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_CHRG_ON, outsignals[0]);
+ qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_JK_B, outsignals[1]);
+ qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_GREEN, outsignals[2]);
+ qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_ORANGE, outsignals[3]);
- if (scp1) {
- qdev_connect_gpio_out(scp1, SPITZ_SCP2_BACKLIGHT_CONT, outsignals[4]);
- qdev_connect_gpio_out(scp1, SPITZ_SCP2_BACKLIGHT_ON, outsignals[5]);
+ if (sms->scp1) {
+ qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_CONT,
+ outsignals[4]);
+ qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_ON,
+ outsignals[5]);
}
- qdev_connect_gpio_out(scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
+ qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
}
#define SPITZ_GPIO_HSYNC 22
@@ -943,7 +946,6 @@ static void spitz_common_init(MachineState *machine)
SpitzMachineState *sms = SPITZ_MACHINE(machine);
enum spitz_model_e model = smc->model;
PXA2xxState *mpu;
- DeviceState *scp0, *scp1 = NULL;
MemoryRegion *address_space_mem = get_system_memory();
MemoryRegion *rom = g_new(MemoryRegion, 1);
@@ -962,12 +964,14 @@ static void spitz_common_init(MachineState *machine)
spitz_ssp_attach(sms);
- scp0 = sysbus_create_simple("scoop", 0x10800000, NULL);
+ sms->scp0 = sysbus_create_simple("scoop", 0x10800000, NULL);
if (model != akita) {
- scp1 = sysbus_create_simple("scoop", 0x08800040, NULL);
+ sms->scp1 = sysbus_create_simple("scoop", 0x08800040, NULL);
+ } else {
+ sms->scp1 = NULL;
}
- spitz_scoop_gpio_setup(mpu, scp0, scp1);
+ spitz_scoop_gpio_setup(sms);
spitz_gpio_setup(mpu, (model == akita) ? 1 : 2);
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 04/17] hw/arm/spitz: Keep pointers to scp0, scp1 in SpitzMachineState
2020-06-28 14:24 ` [PATCH 04/17] hw/arm/spitz: Keep pointers to scp0, scp1 " Peter Maydell
@ 2020-06-29 19:38 ` Alistair Francis
0 siblings, 0 replies; 50+ messages in thread
From: Alistair Francis @ 2020-06-29 19:38 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sun, Jun 28, 2020 at 7:27 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Keep pointers to scp0, scp1 in SpitzMachineState, and just pass
> that to spitz_scoop_gpio_setup().
>
> (We'll want to use some of the other fields in SpitzMachineState
> in that function in the next commit.)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/arm/spitz.c | 34 +++++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index f48e966c047..69bc2b3fa10 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -48,6 +48,8 @@ typedef struct {
> DeviceState *lcdtg;
> DeviceState *ads7846;
> DeviceState *max1111;
> + DeviceState *scp0;
> + DeviceState *scp1;
> } SpitzMachineState;
>
> #define TYPE_SPITZ_MACHINE "spitz-common"
> @@ -845,22 +847,23 @@ static void spitz_out_switch(void *opaque, int line, int level)
> #define SPITZ_SCP2_BACKLIGHT_ON 8
> #define SPITZ_SCP2_MIC_BIAS 9
>
> -static void spitz_scoop_gpio_setup(PXA2xxState *cpu,
> - DeviceState *scp0, DeviceState *scp1)
> +static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
> {
> - qemu_irq *outsignals = qemu_allocate_irqs(spitz_out_switch, cpu, 8);
> + qemu_irq *outsignals = qemu_allocate_irqs(spitz_out_switch, sms->mpu, 8);
>
> - qdev_connect_gpio_out(scp0, SPITZ_SCP_CHRG_ON, outsignals[0]);
> - qdev_connect_gpio_out(scp0, SPITZ_SCP_JK_B, outsignals[1]);
> - qdev_connect_gpio_out(scp0, SPITZ_SCP_LED_GREEN, outsignals[2]);
> - qdev_connect_gpio_out(scp0, SPITZ_SCP_LED_ORANGE, outsignals[3]);
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_CHRG_ON, outsignals[0]);
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_JK_B, outsignals[1]);
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_GREEN, outsignals[2]);
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_ORANGE, outsignals[3]);
>
> - if (scp1) {
> - qdev_connect_gpio_out(scp1, SPITZ_SCP2_BACKLIGHT_CONT, outsignals[4]);
> - qdev_connect_gpio_out(scp1, SPITZ_SCP2_BACKLIGHT_ON, outsignals[5]);
> + if (sms->scp1) {
> + qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_CONT,
> + outsignals[4]);
> + qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_ON,
> + outsignals[5]);
> }
>
> - qdev_connect_gpio_out(scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
> }
>
> #define SPITZ_GPIO_HSYNC 22
> @@ -943,7 +946,6 @@ static void spitz_common_init(MachineState *machine)
> SpitzMachineState *sms = SPITZ_MACHINE(machine);
> enum spitz_model_e model = smc->model;
> PXA2xxState *mpu;
> - DeviceState *scp0, *scp1 = NULL;
> MemoryRegion *address_space_mem = get_system_memory();
> MemoryRegion *rom = g_new(MemoryRegion, 1);
>
> @@ -962,12 +964,14 @@ static void spitz_common_init(MachineState *machine)
>
> spitz_ssp_attach(sms);
>
> - scp0 = sysbus_create_simple("scoop", 0x10800000, NULL);
> + sms->scp0 = sysbus_create_simple("scoop", 0x10800000, NULL);
> if (model != akita) {
> - scp1 = sysbus_create_simple("scoop", 0x08800040, NULL);
> + sms->scp1 = sysbus_create_simple("scoop", 0x08800040, NULL);
> + } else {
> + sms->scp1 = NULL;
> }
>
> - spitz_scoop_gpio_setup(mpu, scp0, scp1);
> + spitz_scoop_gpio_setup(sms);
>
> spitz_gpio_setup(mpu, (model == akita) ? 1 : 2);
>
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 05/17] hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (3 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 04/17] hw/arm/spitz: Keep pointers to scp0, scp1 " Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-30 20:45 ` Alistair Francis
2020-06-28 14:24 ` [PATCH 06/17] hw/misc/max111x: provide QOM properties for setting initial values Peter Maydell
` (12 subsequent siblings)
17 siblings, 1 reply; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
Currently the Spitz board uses a nasty hack for the GPIO lines
that pass "bit5" and "power" information to the LCD controller:
the lcdtg realize function sets a global variable to point to
the instance it just realized, and then the functions spitz_bl_power()
and spitz_bl_bit5() use that to find the device they are changing
the internal state of. There is a comment reading:
FIXME: Implement GPIO properly and remove this hack.
which was added in 2009.
Implement GPIO properly and remove this hack.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/spitz.c | 28 ++++++++++++----------------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 69bc2b3fa10..11e413723f4 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -586,12 +586,9 @@ static void spitz_bl_update(SpitzLCDTG *s)
zaurus_printf("LCD Backlight now off\n");
}
-/* FIXME: Implement GPIO properly and remove this hack. */
-static SpitzLCDTG *spitz_lcdtg;
-
static inline void spitz_bl_bit5(void *opaque, int line, int level)
{
- SpitzLCDTG *s = spitz_lcdtg;
+ SpitzLCDTG *s = opaque;
int prev = s->bl_intensity;
if (level)
@@ -605,7 +602,7 @@ static inline void spitz_bl_bit5(void *opaque, int line, int level)
static inline void spitz_bl_power(void *opaque, int line, int level)
{
- SpitzLCDTG *s = spitz_lcdtg;
+ SpitzLCDTG *s = opaque;
s->bl_power = !!level;
spitz_bl_update(s);
}
@@ -639,13 +636,16 @@ static uint32_t spitz_lcdtg_transfer(SSISlave *dev, uint32_t value)
return 0;
}
-static void spitz_lcdtg_realize(SSISlave *dev, Error **errp)
+static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp)
{
- SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, dev);
+ SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, ssi);
+ DeviceState *dev = DEVICE(s);
- spitz_lcdtg = s;
s->bl_power = 0;
s->bl_intensity = 0x20;
+
+ qdev_init_gpio_in_named(dev, spitz_bl_bit5, "bl_bit5", 1);
+ qdev_init_gpio_in_named(dev, spitz_bl_power, "bl_power", 1);
}
/* SSP devices */
@@ -820,15 +820,11 @@ static void spitz_out_switch(void *opaque, int line, int level)
case 3:
zaurus_printf("Orange LED %s.\n", level ? "on" : "off");
break;
- case 4:
- spitz_bl_bit5(opaque, line, level);
- break;
- case 5:
- spitz_bl_power(opaque, line, level);
- break;
case 6:
spitz_adc_temp_on(opaque, line, level);
break;
+ default:
+ g_assert_not_reached();
}
}
@@ -858,9 +854,9 @@ static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
if (sms->scp1) {
qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_CONT,
- outsignals[4]);
+ qdev_get_gpio_in_named(sms->lcdtg, "bl_bit5", 0));
qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_ON,
- outsignals[5]);
+ qdev_get_gpio_in_named(sms->lcdtg, "bl_power", 0));
}
qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 05/17] hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals
2020-06-28 14:24 ` [PATCH 05/17] hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals Peter Maydell
@ 2020-06-30 20:45 ` Alistair Francis
0 siblings, 0 replies; 50+ messages in thread
From: Alistair Francis @ 2020-06-30 20:45 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sun, Jun 28, 2020 at 7:29 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Currently the Spitz board uses a nasty hack for the GPIO lines
> that pass "bit5" and "power" information to the LCD controller:
> the lcdtg realize function sets a global variable to point to
> the instance it just realized, and then the functions spitz_bl_power()
> and spitz_bl_bit5() use that to find the device they are changing
> the internal state of. There is a comment reading:
> FIXME: Implement GPIO properly and remove this hack.
> which was added in 2009.
>
> Implement GPIO properly and remove this hack.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/arm/spitz.c | 28 ++++++++++++----------------
> 1 file changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index 69bc2b3fa10..11e413723f4 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -586,12 +586,9 @@ static void spitz_bl_update(SpitzLCDTG *s)
> zaurus_printf("LCD Backlight now off\n");
> }
>
> -/* FIXME: Implement GPIO properly and remove this hack. */
> -static SpitzLCDTG *spitz_lcdtg;
> -
> static inline void spitz_bl_bit5(void *opaque, int line, int level)
> {
> - SpitzLCDTG *s = spitz_lcdtg;
> + SpitzLCDTG *s = opaque;
> int prev = s->bl_intensity;
>
> if (level)
> @@ -605,7 +602,7 @@ static inline void spitz_bl_bit5(void *opaque, int line, int level)
>
> static inline void spitz_bl_power(void *opaque, int line, int level)
> {
> - SpitzLCDTG *s = spitz_lcdtg;
> + SpitzLCDTG *s = opaque;
> s->bl_power = !!level;
> spitz_bl_update(s);
> }
> @@ -639,13 +636,16 @@ static uint32_t spitz_lcdtg_transfer(SSISlave *dev, uint32_t value)
> return 0;
> }
>
> -static void spitz_lcdtg_realize(SSISlave *dev, Error **errp)
> +static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp)
> {
> - SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, dev);
> + SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, ssi);
> + DeviceState *dev = DEVICE(s);
>
> - spitz_lcdtg = s;
> s->bl_power = 0;
> s->bl_intensity = 0x20;
> +
> + qdev_init_gpio_in_named(dev, spitz_bl_bit5, "bl_bit5", 1);
> + qdev_init_gpio_in_named(dev, spitz_bl_power, "bl_power", 1);
> }
>
> /* SSP devices */
> @@ -820,15 +820,11 @@ static void spitz_out_switch(void *opaque, int line, int level)
> case 3:
> zaurus_printf("Orange LED %s.\n", level ? "on" : "off");
> break;
> - case 4:
> - spitz_bl_bit5(opaque, line, level);
> - break;
> - case 5:
> - spitz_bl_power(opaque, line, level);
> - break;
> case 6:
> spitz_adc_temp_on(opaque, line, level);
> break;
> + default:
> + g_assert_not_reached();
> }
> }
>
> @@ -858,9 +854,9 @@ static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
>
> if (sms->scp1) {
> qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_CONT,
> - outsignals[4]);
> + qdev_get_gpio_in_named(sms->lcdtg, "bl_bit5", 0));
> qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_ON,
> - outsignals[5]);
> + qdev_get_gpio_in_named(sms->lcdtg, "bl_power", 0));
> }
>
> qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 06/17] hw/misc/max111x: provide QOM properties for setting initial values
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (4 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 05/17] hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 9:01 ` Philippe Mathieu-Daudé
2020-06-30 20:51 ` Alistair Francis
2020-06-28 14:24 ` [PATCH 07/17] hw/misc/max111x: Don't use vmstate_register() Peter Maydell
` (11 subsequent siblings)
17 siblings, 2 replies; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
Add some QOM properties to the max111x ADC device to allow the
initial values to be configured. Currently this is done by
board code calling max111x_set_input() after it creates the
device, which doesn't work on system reset.
This requires us to implement a reset method for this device,
so while we're doing that make sure we reset the other parts
of the device state.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/misc/max111x.c | 57 ++++++++++++++++++++++++++++++++++++++---------
1 file changed, 47 insertions(+), 10 deletions(-)
diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
index 2b87bdee5b7..d0e5534e4f5 100644
--- a/hw/misc/max111x.c
+++ b/hw/misc/max111x.c
@@ -15,11 +15,15 @@
#include "hw/ssi/ssi.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
+#include "hw/qdev-properties.h"
typedef struct {
SSISlave parent_obj;
qemu_irq interrupt;
+ /* Values of inputs at system reset (settable by QOM property) */
+ uint8_t reset_input[8];
+
uint8_t tb1, rb2, rb3;
int cycle;
@@ -135,16 +139,6 @@ static int max111x_init(SSISlave *d, int inputs)
qdev_init_gpio_out(dev, &s->interrupt, 1);
s->inputs = inputs;
- /* TODO: add a user interface for setting these */
- s->input[0] = 0xf0;
- s->input[1] = 0xe0;
- s->input[2] = 0xd0;
- s->input[3] = 0xc0;
- s->input[4] = 0xb0;
- s->input[5] = 0xa0;
- s->input[6] = 0x90;
- s->input[7] = 0x80;
- s->com = 0;
vmstate_register(VMSTATE_IF(dev), VMSTATE_INSTANCE_ID_ANY,
&vmstate_max111x, s);
@@ -168,11 +162,50 @@ void max111x_set_input(DeviceState *dev, int line, uint8_t value)
s->input[line] = value;
}
+static void max111x_reset(DeviceState *dev)
+{
+ MAX111xState *s = MAX_111X(dev);
+ int i;
+
+ for (i = 0; i < s->inputs; i++) {
+ s->input[i] = s->reset_input[i];
+ }
+ s->com = 0;
+ s->tb1 = 0;
+ s->rb2 = 0;
+ s->rb3 = 0;
+ s->cycle = 0;
+}
+
+static Property max1110_properties[] = {
+ /* Reset values for ADC inputs */
+ DEFINE_PROP_UINT8("input0", MAX111xState, reset_input[0], 0xf0),
+ DEFINE_PROP_UINT8("input1", MAX111xState, reset_input[1], 0xe0),
+ DEFINE_PROP_UINT8("input2", MAX111xState, reset_input[2], 0xd0),
+ DEFINE_PROP_UINT8("input3", MAX111xState, reset_input[3], 0xc0),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static Property max1111_properties[] = {
+ /* Reset values for ADC inputs */
+ DEFINE_PROP_UINT8("input0", MAX111xState, reset_input[0], 0xf0),
+ DEFINE_PROP_UINT8("input1", MAX111xState, reset_input[1], 0xe0),
+ DEFINE_PROP_UINT8("input2", MAX111xState, reset_input[2], 0xd0),
+ DEFINE_PROP_UINT8("input3", MAX111xState, reset_input[3], 0xc0),
+ DEFINE_PROP_UINT8("input4", MAX111xState, reset_input[4], 0xb0),
+ DEFINE_PROP_UINT8("input5", MAX111xState, reset_input[5], 0xa0),
+ DEFINE_PROP_UINT8("input6", MAX111xState, reset_input[6], 0x90),
+ DEFINE_PROP_UINT8("input7", MAX111xState, reset_input[7], 0x80),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
static void max111x_class_init(ObjectClass *klass, void *data)
{
SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
k->transfer = max111x_transfer;
+ dc->reset = max111x_reset;
}
static const TypeInfo max111x_info = {
@@ -186,8 +219,10 @@ static const TypeInfo max111x_info = {
static void max1110_class_init(ObjectClass *klass, void *data)
{
SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
k->realize = max1110_realize;
+ device_class_set_props(dc, max1110_properties);
}
static const TypeInfo max1110_info = {
@@ -199,8 +234,10 @@ static const TypeInfo max1110_info = {
static void max1111_class_init(ObjectClass *klass, void *data)
{
SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
k->realize = max1111_realize;
+ device_class_set_props(dc, max1111_properties);
}
static const TypeInfo max1111_info = {
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 06/17] hw/misc/max111x: provide QOM properties for setting initial values
2020-06-28 14:24 ` [PATCH 06/17] hw/misc/max111x: provide QOM properties for setting initial values Peter Maydell
@ 2020-06-29 9:01 ` Philippe Mathieu-Daudé
2020-06-30 20:51 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-29 9:01 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis
On 6/28/20 4:24 PM, Peter Maydell wrote:
> Add some QOM properties to the max111x ADC device to allow the
> initial values to be configured. Currently this is done by
> board code calling max111x_set_input() after it creates the
> device, which doesn't work on system reset.
>
> This requires us to implement a reset method for this device,
> so while we're doing that make sure we reset the other parts
> of the device state.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/misc/max111x.c | 57 ++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 47 insertions(+), 10 deletions(-)
>
> diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
> index 2b87bdee5b7..d0e5534e4f5 100644
> --- a/hw/misc/max111x.c
> +++ b/hw/misc/max111x.c
> @@ -15,11 +15,15 @@
> #include "hw/ssi/ssi.h"
> #include "migration/vmstate.h"
> #include "qemu/module.h"
> +#include "hw/qdev-properties.h"
>
> typedef struct {
> SSISlave parent_obj;
>
> qemu_irq interrupt;
> + /* Values of inputs at system reset (settable by QOM property) */
> + uint8_t reset_input[8];
> +
> uint8_t tb1, rb2, rb3;
> int cycle;
>
An eventual improvement is to make 'inputs' a class property:
MAX111xClass {
SSISlaveClass parent_obj;
unsigned input_count;
}
"eventual" because from a QOM point it is cleaner, but we'd have
to add more boiler-plate casts, so code less clear.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 06/17] hw/misc/max111x: provide QOM properties for setting initial values
2020-06-28 14:24 ` [PATCH 06/17] hw/misc/max111x: provide QOM properties for setting initial values Peter Maydell
2020-06-29 9:01 ` Philippe Mathieu-Daudé
@ 2020-06-30 20:51 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Alistair Francis @ 2020-06-30 20:51 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sun, Jun 28, 2020 at 7:29 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Add some QOM properties to the max111x ADC device to allow the
> initial values to be configured. Currently this is done by
> board code calling max111x_set_input() after it creates the
> device, which doesn't work on system reset.
>
> This requires us to implement a reset method for this device,
> so while we're doing that make sure we reset the other parts
> of the device state.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/misc/max111x.c | 57 ++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 47 insertions(+), 10 deletions(-)
>
> diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
> index 2b87bdee5b7..d0e5534e4f5 100644
> --- a/hw/misc/max111x.c
> +++ b/hw/misc/max111x.c
> @@ -15,11 +15,15 @@
> #include "hw/ssi/ssi.h"
> #include "migration/vmstate.h"
> #include "qemu/module.h"
> +#include "hw/qdev-properties.h"
>
> typedef struct {
> SSISlave parent_obj;
>
> qemu_irq interrupt;
> + /* Values of inputs at system reset (settable by QOM property) */
> + uint8_t reset_input[8];
> +
> uint8_t tb1, rb2, rb3;
> int cycle;
>
> @@ -135,16 +139,6 @@ static int max111x_init(SSISlave *d, int inputs)
> qdev_init_gpio_out(dev, &s->interrupt, 1);
>
> s->inputs = inputs;
> - /* TODO: add a user interface for setting these */
> - s->input[0] = 0xf0;
> - s->input[1] = 0xe0;
> - s->input[2] = 0xd0;
> - s->input[3] = 0xc0;
> - s->input[4] = 0xb0;
> - s->input[5] = 0xa0;
> - s->input[6] = 0x90;
> - s->input[7] = 0x80;
> - s->com = 0;
>
> vmstate_register(VMSTATE_IF(dev), VMSTATE_INSTANCE_ID_ANY,
> &vmstate_max111x, s);
> @@ -168,11 +162,50 @@ void max111x_set_input(DeviceState *dev, int line, uint8_t value)
> s->input[line] = value;
> }
>
> +static void max111x_reset(DeviceState *dev)
> +{
> + MAX111xState *s = MAX_111X(dev);
> + int i;
> +
> + for (i = 0; i < s->inputs; i++) {
> + s->input[i] = s->reset_input[i];
> + }
> + s->com = 0;
> + s->tb1 = 0;
> + s->rb2 = 0;
> + s->rb3 = 0;
> + s->cycle = 0;
> +}
> +
> +static Property max1110_properties[] = {
> + /* Reset values for ADC inputs */
> + DEFINE_PROP_UINT8("input0", MAX111xState, reset_input[0], 0xf0),
> + DEFINE_PROP_UINT8("input1", MAX111xState, reset_input[1], 0xe0),
> + DEFINE_PROP_UINT8("input2", MAX111xState, reset_input[2], 0xd0),
> + DEFINE_PROP_UINT8("input3", MAX111xState, reset_input[3], 0xc0),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static Property max1111_properties[] = {
> + /* Reset values for ADC inputs */
> + DEFINE_PROP_UINT8("input0", MAX111xState, reset_input[0], 0xf0),
> + DEFINE_PROP_UINT8("input1", MAX111xState, reset_input[1], 0xe0),
> + DEFINE_PROP_UINT8("input2", MAX111xState, reset_input[2], 0xd0),
> + DEFINE_PROP_UINT8("input3", MAX111xState, reset_input[3], 0xc0),
> + DEFINE_PROP_UINT8("input4", MAX111xState, reset_input[4], 0xb0),
> + DEFINE_PROP_UINT8("input5", MAX111xState, reset_input[5], 0xa0),
> + DEFINE_PROP_UINT8("input6", MAX111xState, reset_input[6], 0x90),
> + DEFINE_PROP_UINT8("input7", MAX111xState, reset_input[7], 0x80),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void max111x_class_init(ObjectClass *klass, void *data)
> {
> SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->transfer = max111x_transfer;
> + dc->reset = max111x_reset;
> }
>
> static const TypeInfo max111x_info = {
> @@ -186,8 +219,10 @@ static const TypeInfo max111x_info = {
> static void max1110_class_init(ObjectClass *klass, void *data)
> {
> SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->realize = max1110_realize;
> + device_class_set_props(dc, max1110_properties);
> }
>
> static const TypeInfo max1110_info = {
> @@ -199,8 +234,10 @@ static const TypeInfo max1110_info = {
> static void max1111_class_init(ObjectClass *klass, void *data)
> {
> SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->realize = max1111_realize;
> + device_class_set_props(dc, max1111_properties);
> }
>
> static const TypeInfo max1111_info = {
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 07/17] hw/misc/max111x: Don't use vmstate_register()
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (5 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 06/17] hw/misc/max111x: provide QOM properties for setting initial values Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 9:01 ` Philippe Mathieu-Daudé
2020-06-30 20:52 ` Alistair Francis
2020-06-28 14:24 ` [PATCH 08/17] ssi: Add ssi_realize_and_unref() Peter Maydell
` (10 subsequent siblings)
17 siblings, 2 replies; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
The max111x is a proper qdev device; we can use dc->vmsd rather than
directly calling vmstate_register().
It's possible that this is a migration compat break, but the only
boards that use this device are the spitz-family ('akita', 'borzoi',
'spitz', 'terrier').
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/misc/max111x.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
index d0e5534e4f5..abddfa3c660 100644
--- a/hw/misc/max111x.c
+++ b/hw/misc/max111x.c
@@ -140,8 +140,6 @@ static int max111x_init(SSISlave *d, int inputs)
s->inputs = inputs;
- vmstate_register(VMSTATE_IF(dev), VMSTATE_INSTANCE_ID_ANY,
- &vmstate_max111x, s);
return 0;
}
@@ -206,6 +204,7 @@ static void max111x_class_init(ObjectClass *klass, void *data)
k->transfer = max111x_transfer;
dc->reset = max111x_reset;
+ dc->vmsd = &vmstate_max111x;
}
static const TypeInfo max111x_info = {
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 07/17] hw/misc/max111x: Don't use vmstate_register()
2020-06-28 14:24 ` [PATCH 07/17] hw/misc/max111x: Don't use vmstate_register() Peter Maydell
@ 2020-06-29 9:01 ` Philippe Mathieu-Daudé
2020-06-30 20:52 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-29 9:01 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis
On 6/28/20 4:24 PM, Peter Maydell wrote:
> The max111x is a proper qdev device; we can use dc->vmsd rather than
> directly calling vmstate_register().
>
> It's possible that this is a migration compat break, but the only
> boards that use this device are the spitz-family ('akita', 'borzoi',
> 'spitz', 'terrier').
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/misc/max111x.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
> index d0e5534e4f5..abddfa3c660 100644
> --- a/hw/misc/max111x.c
> +++ b/hw/misc/max111x.c
> @@ -140,8 +140,6 @@ static int max111x_init(SSISlave *d, int inputs)
>
> s->inputs = inputs;
>
> - vmstate_register(VMSTATE_IF(dev), VMSTATE_INSTANCE_ID_ANY,
> - &vmstate_max111x, s);
> return 0;
> }
>
> @@ -206,6 +204,7 @@ static void max111x_class_init(ObjectClass *klass, void *data)
>
> k->transfer = max111x_transfer;
> dc->reset = max111x_reset;
> + dc->vmsd = &vmstate_max111x;
> }
>
> static const TypeInfo max111x_info = {
>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 07/17] hw/misc/max111x: Don't use vmstate_register()
2020-06-28 14:24 ` [PATCH 07/17] hw/misc/max111x: Don't use vmstate_register() Peter Maydell
2020-06-29 9:01 ` Philippe Mathieu-Daudé
@ 2020-06-30 20:52 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Alistair Francis @ 2020-06-30 20:52 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sun, Jun 28, 2020 at 7:25 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The max111x is a proper qdev device; we can use dc->vmsd rather than
> directly calling vmstate_register().
>
> It's possible that this is a migration compat break, but the only
> boards that use this device are the spitz-family ('akita', 'borzoi',
> 'spitz', 'terrier').
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/misc/max111x.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
> index d0e5534e4f5..abddfa3c660 100644
> --- a/hw/misc/max111x.c
> +++ b/hw/misc/max111x.c
> @@ -140,8 +140,6 @@ static int max111x_init(SSISlave *d, int inputs)
>
> s->inputs = inputs;
>
> - vmstate_register(VMSTATE_IF(dev), VMSTATE_INSTANCE_ID_ANY,
> - &vmstate_max111x, s);
> return 0;
> }
>
> @@ -206,6 +204,7 @@ static void max111x_class_init(ObjectClass *klass, void *data)
>
> k->transfer = max111x_transfer;
> dc->reset = max111x_reset;
> + dc->vmsd = &vmstate_max111x;
> }
>
> static const TypeInfo max111x_info = {
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 08/17] ssi: Add ssi_realize_and_unref()
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (6 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 07/17] hw/misc/max111x: Don't use vmstate_register() Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 9:02 ` Philippe Mathieu-Daudé
2020-06-30 20:55 ` Alistair Francis
2020-06-28 14:24 ` [PATCH 09/17] hw/arm/spitz: Use max111x properties to set initial values Peter Maydell
` (9 subsequent siblings)
17 siblings, 2 replies; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
Add an ssi_realize_and_unref(), for the benefit of callers
who want to be able to create an SSI device, set QOM properties
on it, and then do the realize-and-unref afterwards.
The API works on the same principle as the recently added
qdev_realize_and_undef(), sysbus_realize_and_undef(), etc.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/ssi/ssi.h | 26 ++++++++++++++++++++++++++
hw/ssi/ssi.c | 7 ++++++-
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
index 93f2b8b0beb..4be5325e654 100644
--- a/include/hw/ssi/ssi.h
+++ b/include/hw/ssi/ssi.h
@@ -79,6 +79,32 @@ extern const VMStateDescription vmstate_ssi_slave;
}
DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
+/**
+ * ssi_realize_and_unref: realize and unref an SSI slave device
+ * @dev: SSI slave device to realize
+ * @bus: SSI bus to put it on
+ * @errp: error pointer
+ *
+ * Call 'realize' on @dev, put it on the specified @bus, and drop the
+ * reference to it. Errors are reported via @errp and by returning
+ * false.
+ *
+ * This function is useful if you have created @dev via qdev_new()
+ * (which takes a reference to the device it returns to you), so that
+ * you can set properties on it before realizing it. If you don't need
+ * to set properties then ssi_create_slave() is probably better (as it
+ * does the create, init and realize in one step).
+ *
+ * If you are embedding the SSI slave into another QOM device and
+ * initialized it via some variant on object_initialize_child() then
+ * do not use this function, because that family of functions arrange
+ * for the only reference to the child device to be held by the parent
+ * via the child<> property, and so the reference-count-drop done here
+ * would be incorrect. (Instead you would want ssi_realize(), which
+ * doesn't currently exist but would be trivial to create if we had
+ * any code that wanted it.)
+ */
+bool ssi_realize_and_unref(DeviceState *dev, SSIBus *bus, Error **errp);
/* Master interface. */
SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c
index 67b48c31cd6..a35d7ebb266 100644
--- a/hw/ssi/ssi.c
+++ b/hw/ssi/ssi.c
@@ -90,11 +90,16 @@ static const TypeInfo ssi_slave_info = {
.abstract = true,
};
+bool ssi_realize_and_unref(DeviceState *dev, SSIBus *bus, Error **errp)
+{
+ return qdev_realize_and_unref(dev, &bus->parent_obj, errp);
+}
+
DeviceState *ssi_create_slave(SSIBus *bus, const char *name)
{
DeviceState *dev = qdev_new(name);
- qdev_realize_and_unref(dev, &bus->parent_obj, &error_fatal);
+ ssi_realize_and_unref(dev, bus, &error_fatal);
return dev;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 08/17] ssi: Add ssi_realize_and_unref()
2020-06-28 14:24 ` [PATCH 08/17] ssi: Add ssi_realize_and_unref() Peter Maydell
@ 2020-06-29 9:02 ` Philippe Mathieu-Daudé
2020-06-30 20:55 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-29 9:02 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis, Markus Armbruster
On 6/28/20 4:24 PM, Peter Maydell wrote:
> Add an ssi_realize_and_unref(), for the benefit of callers
> who want to be able to create an SSI device, set QOM properties
> on it, and then do the realize-and-unref afterwards.
>
> The API works on the same principle as the recently added
> qdev_realize_and_undef(), sysbus_realize_and_undef(), etc.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> include/hw/ssi/ssi.h | 26 ++++++++++++++++++++++++++
> hw/ssi/ssi.c | 7 ++++++-
> 2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
> index 93f2b8b0beb..4be5325e654 100644
> --- a/include/hw/ssi/ssi.h
> +++ b/include/hw/ssi/ssi.h
> @@ -79,6 +79,32 @@ extern const VMStateDescription vmstate_ssi_slave;
> }
>
> DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
> +/**
> + * ssi_realize_and_unref: realize and unref an SSI slave device
> + * @dev: SSI slave device to realize
> + * @bus: SSI bus to put it on
> + * @errp: error pointer
> + *
> + * Call 'realize' on @dev, put it on the specified @bus, and drop the
> + * reference to it. Errors are reported via @errp and by returning
> + * false.
> + *
> + * This function is useful if you have created @dev via qdev_new()
> + * (which takes a reference to the device it returns to you), so that
> + * you can set properties on it before realizing it. If you don't need
> + * to set properties then ssi_create_slave() is probably better (as it
> + * does the create, init and realize in one step).
> + *
> + * If you are embedding the SSI slave into another QOM device and
> + * initialized it via some variant on object_initialize_child() then
> + * do not use this function, because that family of functions arrange
> + * for the only reference to the child device to be held by the parent
> + * via the child<> property, and so the reference-count-drop done here
> + * would be incorrect. (Instead you would want ssi_realize(), which
> + * doesn't currently exist but would be trivial to create if we had
> + * any code that wanted it.)
> + */
> +bool ssi_realize_and_unref(DeviceState *dev, SSIBus *bus, Error **errp);
>
> /* Master interface. */
> SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
> diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c
> index 67b48c31cd6..a35d7ebb266 100644
> --- a/hw/ssi/ssi.c
> +++ b/hw/ssi/ssi.c
> @@ -90,11 +90,16 @@ static const TypeInfo ssi_slave_info = {
> .abstract = true,
> };
>
> +bool ssi_realize_and_unref(DeviceState *dev, SSIBus *bus, Error **errp)
> +{
> + return qdev_realize_and_unref(dev, &bus->parent_obj, errp);
> +}
> +
> DeviceState *ssi_create_slave(SSIBus *bus, const char *name)
> {
> DeviceState *dev = qdev_new(name);
>
> - qdev_realize_and_unref(dev, &bus->parent_obj, &error_fatal);
> + ssi_realize_and_unref(dev, bus, &error_fatal);
> return dev;
> }
>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 08/17] ssi: Add ssi_realize_and_unref()
2020-06-28 14:24 ` [PATCH 08/17] ssi: Add ssi_realize_and_unref() Peter Maydell
2020-06-29 9:02 ` Philippe Mathieu-Daudé
@ 2020-06-30 20:55 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Alistair Francis @ 2020-06-30 20:55 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sun, Jun 28, 2020 at 7:27 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Add an ssi_realize_and_unref(), for the benefit of callers
> who want to be able to create an SSI device, set QOM properties
> on it, and then do the realize-and-unref afterwards.
>
> The API works on the same principle as the recently added
> qdev_realize_and_undef(), sysbus_realize_and_undef(), etc.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/hw/ssi/ssi.h | 26 ++++++++++++++++++++++++++
> hw/ssi/ssi.c | 7 ++++++-
> 2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
> index 93f2b8b0beb..4be5325e654 100644
> --- a/include/hw/ssi/ssi.h
> +++ b/include/hw/ssi/ssi.h
> @@ -79,6 +79,32 @@ extern const VMStateDescription vmstate_ssi_slave;
> }
>
> DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
> +/**
> + * ssi_realize_and_unref: realize and unref an SSI slave device
> + * @dev: SSI slave device to realize
> + * @bus: SSI bus to put it on
> + * @errp: error pointer
> + *
> + * Call 'realize' on @dev, put it on the specified @bus, and drop the
> + * reference to it. Errors are reported via @errp and by returning
> + * false.
> + *
> + * This function is useful if you have created @dev via qdev_new()
> + * (which takes a reference to the device it returns to you), so that
> + * you can set properties on it before realizing it. If you don't need
> + * to set properties then ssi_create_slave() is probably better (as it
> + * does the create, init and realize in one step).
> + *
> + * If you are embedding the SSI slave into another QOM device and
> + * initialized it via some variant on object_initialize_child() then
> + * do not use this function, because that family of functions arrange
> + * for the only reference to the child device to be held by the parent
> + * via the child<> property, and so the reference-count-drop done here
> + * would be incorrect. (Instead you would want ssi_realize(), which
> + * doesn't currently exist but would be trivial to create if we had
> + * any code that wanted it.)
> + */
> +bool ssi_realize_and_unref(DeviceState *dev, SSIBus *bus, Error **errp);
>
> /* Master interface. */
> SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
> diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c
> index 67b48c31cd6..a35d7ebb266 100644
> --- a/hw/ssi/ssi.c
> +++ b/hw/ssi/ssi.c
> @@ -90,11 +90,16 @@ static const TypeInfo ssi_slave_info = {
> .abstract = true,
> };
>
> +bool ssi_realize_and_unref(DeviceState *dev, SSIBus *bus, Error **errp)
> +{
> + return qdev_realize_and_unref(dev, &bus->parent_obj, errp);
> +}
> +
> DeviceState *ssi_create_slave(SSIBus *bus, const char *name)
> {
> DeviceState *dev = qdev_new(name);
>
> - qdev_realize_and_unref(dev, &bus->parent_obj, &error_fatal);
> + ssi_realize_and_unref(dev, bus, &error_fatal);
> return dev;
> }
>
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 09/17] hw/arm/spitz: Use max111x properties to set initial values
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (7 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 08/17] ssi: Add ssi_realize_and_unref() Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 9:09 ` Philippe Mathieu-Daudé
2020-06-28 14:24 ` [PATCH 10/17] hw/misc/max111x: Use GPIO lines rather than max111x_set_input() Peter Maydell
` (8 subsequent siblings)
17 siblings, 1 reply; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
Use the new max111x qdev properties to set the initial input
values rather than calling max111x_set_input(); this means that
on system reset the inputs will correctly return to their initial
values.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/spitz.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 11e413723f4..93a25edcb5b 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -731,11 +731,14 @@ static void spitz_ssp_attach(SpitzMachineState *sms)
qdev_get_gpio_in(sms->mpu->gpio, SPITZ_GPIO_TP_INT));
bus = qdev_get_child_bus(sms->mux, "ssi2");
- sms->max1111 = ssi_create_slave(bus, "max1111");
+ sms->max1111 = qdev_new("max1111");
max1111 = sms->max1111;
- max111x_set_input(sms->max1111, MAX1111_BATT_VOLT, SPITZ_BATTERY_VOLT);
- max111x_set_input(sms->max1111, MAX1111_BATT_TEMP, 0);
- max111x_set_input(sms->max1111, MAX1111_ACIN_VOLT, SPITZ_CHARGEON_ACIN);
+ qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */,
+ SPITZ_BATTERY_VOLT);
+ qdev_prop_set_uint8(sms->max1111, "input2" /* BATT_TEMP */, 0);
+ qdev_prop_set_uint8(sms->max1111, "input3" /* ACIN_VOLT */,
+ SPITZ_CHARGEON_ACIN);
+ ssi_realize_and_unref(sms->max1111, bus, &error_fatal);
qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_LCDCON_CS,
qdev_get_gpio_in(sms->mux, 0));
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 09/17] hw/arm/spitz: Use max111x properties to set initial values
2020-06-28 14:24 ` [PATCH 09/17] hw/arm/spitz: Use max111x properties to set initial values Peter Maydell
@ 2020-06-29 9:09 ` Philippe Mathieu-Daudé
2020-06-29 14:05 ` Peter Maydell
0 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-29 9:09 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis
On 6/28/20 4:24 PM, Peter Maydell wrote:
> Use the new max111x qdev properties to set the initial input
> values rather than calling max111x_set_input(); this means that
> on system reset the inputs will correctly return to their initial
> values.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/arm/spitz.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index 11e413723f4..93a25edcb5b 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -731,11 +731,14 @@ static void spitz_ssp_attach(SpitzMachineState *sms)
> qdev_get_gpio_in(sms->mpu->gpio, SPITZ_GPIO_TP_INT));
>
> bus = qdev_get_child_bus(sms->mux, "ssi2");
> - sms->max1111 = ssi_create_slave(bus, "max1111");
> + sms->max1111 = qdev_new("max1111");
> max1111 = sms->max1111;
> - max111x_set_input(sms->max1111, MAX1111_BATT_VOLT, SPITZ_BATTERY_VOLT);
> - max111x_set_input(sms->max1111, MAX1111_BATT_TEMP, 0);
> - max111x_set_input(sms->max1111, MAX1111_ACIN_VOLT, SPITZ_CHARGEON_ACIN);
> + qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */,
> + SPITZ_BATTERY_VOLT);
> + qdev_prop_set_uint8(sms->max1111, "input2" /* BATT_TEMP */, 0);
> + qdev_prop_set_uint8(sms->max1111, "input3" /* ACIN_VOLT */,
> + SPITZ_CHARGEON_ACIN);
Actually for arrays it would be nice to use:
DEFINE_PROP_ARRAY("input", MAX111xState, nr_inputs, reset_input,
qdev_prop_uint8, uint8_t),
Then something like:
qdev_prop_set_uint8_indexed(sms->max1111, "input", 2 /*BATT_TEMP*/, 0);
Anyway,
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> + ssi_realize_and_unref(sms->max1111, bus, &error_fatal);
>
> qdev_connect_gpio_out(sms->mpu->gpio, SPITZ_GPIO_LCDCON_CS,
> qdev_get_gpio_in(sms->mux, 0));
>
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 09/17] hw/arm/spitz: Use max111x properties to set initial values
2020-06-29 9:09 ` Philippe Mathieu-Daudé
@ 2020-06-29 14:05 ` Peter Maydell
0 siblings, 0 replies; 50+ messages in thread
From: Peter Maydell @ 2020-06-29 14:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-arm, QEMU Developers, Alistair Francis
On Mon, 29 Jun 2020 at 10:09, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 6/28/20 4:24 PM, Peter Maydell wrote:
> > Use the new max111x qdev properties to set the initial input
> > values rather than calling max111x_set_input(); this means that
> > on system reset the inputs will correctly return to their initial
> > values.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> Actually for arrays it would be nice to use:
>
> DEFINE_PROP_ARRAY("input", MAX111xState, nr_inputs, reset_input,
> qdev_prop_uint8, uint8_t),
DEFINE_PROP_ARRAY defines a variable length property array;
what we want for max111x is fixed-length.
thanks
-- PMM
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 10/17] hw/misc/max111x: Use GPIO lines rather than max111x_set_input()
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (8 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 09/17] hw/arm/spitz: Use max111x properties to set initial values Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-07-01 0:37 ` Alistair Francis
2020-06-28 14:24 ` [PATCH 11/17] hw/misc/max111x: Create header file for documentation, TYPE_ macros Peter Maydell
` (7 subsequent siblings)
17 siblings, 1 reply; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
The max111x ADC device model allows other code to set the level on
the 8 ADC inputs using the max111x_set_input() function. Replace
this with generic qdev GPIO inputs, which also allow inputs to be set
to arbitrary values.
Using GPIO lines will make it easier for board code to wire things
up, so that if device A wants to set the ADC input it doesn't need to
have a direct pointer to the max111x but can just set that value on
its output GPIO, which is then wired up by the board to the
appropriate max111x input.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/ssi/ssi.h | 3 ---
hw/arm/spitz.c | 9 +++++----
hw/misc/max111x.c | 16 +++++++++-------
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
index 4be5325e654..5fd411f2e4e 100644
--- a/include/hw/ssi/ssi.h
+++ b/include/hw/ssi/ssi.h
@@ -111,7 +111,4 @@ SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
uint32_t ssi_transfer(SSIBus *bus, uint32_t val);
-/* max111x.c */
-void max111x_set_input(DeviceState *dev, int line, uint8_t value);
-
#endif
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 93a25edcb5b..fa592aad6d6 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -696,13 +696,14 @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level)
static void spitz_adc_temp_on(void *opaque, int line, int level)
{
+ int batt_temp;
+
if (!max1111)
return;
- if (level)
- max111x_set_input(max1111, MAX1111_BATT_TEMP, SPITZ_BATTERY_TEMP);
- else
- max111x_set_input(max1111, MAX1111_BATT_TEMP, 0);
+ batt_temp = level ? SPITZ_BATTERY_TEMP : 0;
+
+ qemu_set_irq(qdev_get_gpio_in(max1111, MAX1111_BATT_TEMP), batt_temp);
}
static void corgi_ssp_realize(SSISlave *d, Error **errp)
diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
index abddfa3c660..3a5cb838445 100644
--- a/hw/misc/max111x.c
+++ b/hw/misc/max111x.c
@@ -131,12 +131,21 @@ static const VMStateDescription vmstate_max111x = {
}
};
+static void max111x_input_set(void *opaque, int line, int value)
+{
+ MAX111xState *s = MAX_111X(opaque);
+
+ assert(line >= 0 && line < s->inputs);
+ s->input[line] = value;
+}
+
static int max111x_init(SSISlave *d, int inputs)
{
DeviceState *dev = DEVICE(d);
MAX111xState *s = MAX_111X(dev);
qdev_init_gpio_out(dev, &s->interrupt, 1);
+ qdev_init_gpio_in(dev, max111x_input_set, inputs);
s->inputs = inputs;
@@ -153,13 +162,6 @@ static void max1111_realize(SSISlave *dev, Error **errp)
max111x_init(dev, 4);
}
-void max111x_set_input(DeviceState *dev, int line, uint8_t value)
-{
- MAX111xState *s = MAX_111X(dev);
- assert(line >= 0 && line < s->inputs);
- s->input[line] = value;
-}
-
static void max111x_reset(DeviceState *dev)
{
MAX111xState *s = MAX_111X(dev);
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 10/17] hw/misc/max111x: Use GPIO lines rather than max111x_set_input()
2020-06-28 14:24 ` [PATCH 10/17] hw/misc/max111x: Use GPIO lines rather than max111x_set_input() Peter Maydell
@ 2020-07-01 0:37 ` Alistair Francis
0 siblings, 0 replies; 50+ messages in thread
From: Alistair Francis @ 2020-07-01 0:37 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sun, Jun 28, 2020 at 7:28 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The max111x ADC device model allows other code to set the level on
> the 8 ADC inputs using the max111x_set_input() function. Replace
> this with generic qdev GPIO inputs, which also allow inputs to be set
> to arbitrary values.
>
> Using GPIO lines will make it easier for board code to wire things
> up, so that if device A wants to set the ADC input it doesn't need to
> have a direct pointer to the max111x but can just set that value on
> its output GPIO, which is then wired up by the board to the
> appropriate max111x input.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/hw/ssi/ssi.h | 3 ---
> hw/arm/spitz.c | 9 +++++----
> hw/misc/max111x.c | 16 +++++++++-------
> 3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
> index 4be5325e654..5fd411f2e4e 100644
> --- a/include/hw/ssi/ssi.h
> +++ b/include/hw/ssi/ssi.h
> @@ -111,7 +111,4 @@ SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
>
> uint32_t ssi_transfer(SSIBus *bus, uint32_t val);
>
> -/* max111x.c */
> -void max111x_set_input(DeviceState *dev, int line, uint8_t value);
> -
> #endif
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index 93a25edcb5b..fa592aad6d6 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -696,13 +696,14 @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level)
>
> static void spitz_adc_temp_on(void *opaque, int line, int level)
> {
> + int batt_temp;
> +
> if (!max1111)
> return;
>
> - if (level)
> - max111x_set_input(max1111, MAX1111_BATT_TEMP, SPITZ_BATTERY_TEMP);
> - else
> - max111x_set_input(max1111, MAX1111_BATT_TEMP, 0);
> + batt_temp = level ? SPITZ_BATTERY_TEMP : 0;
> +
> + qemu_set_irq(qdev_get_gpio_in(max1111, MAX1111_BATT_TEMP), batt_temp);
> }
>
> static void corgi_ssp_realize(SSISlave *d, Error **errp)
> diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
> index abddfa3c660..3a5cb838445 100644
> --- a/hw/misc/max111x.c
> +++ b/hw/misc/max111x.c
> @@ -131,12 +131,21 @@ static const VMStateDescription vmstate_max111x = {
> }
> };
>
> +static void max111x_input_set(void *opaque, int line, int value)
> +{
> + MAX111xState *s = MAX_111X(opaque);
> +
> + assert(line >= 0 && line < s->inputs);
> + s->input[line] = value;
> +}
> +
> static int max111x_init(SSISlave *d, int inputs)
> {
> DeviceState *dev = DEVICE(d);
> MAX111xState *s = MAX_111X(dev);
>
> qdev_init_gpio_out(dev, &s->interrupt, 1);
> + qdev_init_gpio_in(dev, max111x_input_set, inputs);
>
> s->inputs = inputs;
>
> @@ -153,13 +162,6 @@ static void max1111_realize(SSISlave *dev, Error **errp)
> max111x_init(dev, 4);
> }
>
> -void max111x_set_input(DeviceState *dev, int line, uint8_t value)
> -{
> - MAX111xState *s = MAX_111X(dev);
> - assert(line >= 0 && line < s->inputs);
> - s->input[line] = value;
> -}
> -
> static void max111x_reset(DeviceState *dev)
> {
> MAX111xState *s = MAX_111X(dev);
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 11/17] hw/misc/max111x: Create header file for documentation, TYPE_ macros
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (9 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 10/17] hw/misc/max111x: Use GPIO lines rather than max111x_set_input() Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 8:29 ` Philippe Mathieu-Daudé
2020-06-28 14:24 ` [PATCH 12/17] hw/arm/spitz: Encapsulate misc GPIO handling in a device Peter Maydell
` (6 subsequent siblings)
17 siblings, 1 reply; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
Create a header file for the hw/misc/max111x device, in the
usual modern style for QOM devices:
* definition of the TYPE_ constants and macros
* definition of the device's state struct so that it can
be embedded in other structs if desired
* documentation of the interface
This allows us to use TYPE_MAX_1111 in the spitz.c code rather
than the string "max1111".
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
To be honest the main driver here is that I wanted a header
file to put the documentation comment in :-)
---
include/hw/misc/max111x.h | 57 +++++++++++++++++++++++++++++++++++++++
hw/arm/spitz.c | 3 ++-
hw/misc/max111x.c | 25 +----------------
MAINTAINERS | 1 +
4 files changed, 61 insertions(+), 25 deletions(-)
create mode 100644 include/hw/misc/max111x.h
diff --git a/include/hw/misc/max111x.h b/include/hw/misc/max111x.h
new file mode 100644
index 00000000000..337ba63d588
--- /dev/null
+++ b/include/hw/misc/max111x.h
@@ -0,0 +1,57 @@
+/*
+ * Maxim MAX1110/1111 ADC chip emulation.
+ *
+ * Copyright (c) 2006 Openedhand Ltd.
+ * Written by Andrzej Zaborowski <balrog@zabor.org>
+ *
+ * This code is licensed under the GNU GPLv2.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#ifndef HW_MISC_MAX111X_H
+#define HW_MISC_MAX111X_H
+
+#include "hw/ssi/ssi.h"
+#include "hw/irq.h"
+
+/*
+ * This is a model of the Maxim MAX1110/1111 ADC chip, which for QEMU
+ * is an SSI slave device. It has either 4 (max1110) or 8 (max1111)
+ * 8-bit ADC channels.
+ *
+ * QEMU interface:
+ * + GPIO inputs 0..3 (for max1110) or 0..7 (for max1111): set the value
+ * of each ADC input, as an unsigned 8-bit value
+ * + GPIO output 0: interrupt line
+ * + Properties "input0" to "input3" (max1110) or "input0" to "input7"
+ * (max1111): initial reset values for ADC inputs.
+ *
+ * Known bugs:
+ * + the interrupt line is not correctly implemented, and will never
+ * be lowered once it has been asserted.
+ */
+typedef struct {
+ SSISlave parent_obj;
+
+ qemu_irq interrupt;
+ /* Values of inputs at system reset (settable by QOM property) */
+ uint8_t reset_input[8];
+
+ uint8_t tb1, rb2, rb3;
+ int cycle;
+
+ uint8_t input[8];
+ int inputs, com;
+} MAX111xState;
+
+#define TYPE_MAX_111X "max111x"
+
+#define MAX_111X(obj) \
+ OBJECT_CHECK(MAX111xState, (obj), TYPE_MAX_111X)
+
+#define TYPE_MAX_1110 "max1110"
+#define TYPE_MAX_1111 "max1111"
+
+#endif
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index fa592aad6d6..1400a56729d 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -29,6 +29,7 @@
#include "audio/audio.h"
#include "hw/boards.h"
#include "hw/sysbus.h"
+#include "hw/misc/max111x.h"
#include "migration/vmstate.h"
#include "exec/address-spaces.h"
#include "cpu.h"
@@ -732,7 +733,7 @@ static void spitz_ssp_attach(SpitzMachineState *sms)
qdev_get_gpio_in(sms->mpu->gpio, SPITZ_GPIO_TP_INT));
bus = qdev_get_child_bus(sms->mux, "ssi2");
- sms->max1111 = qdev_new("max1111");
+ sms->max1111 = qdev_new(TYPE_MAX_1111);
max1111 = sms->max1111;
qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */,
SPITZ_BATTERY_VOLT);
diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
index 3a5cb838445..d41cfd92a8d 100644
--- a/hw/misc/max111x.c
+++ b/hw/misc/max111x.c
@@ -11,34 +11,11 @@
*/
#include "qemu/osdep.h"
-#include "hw/irq.h"
-#include "hw/ssi/ssi.h"
+#include "hw/misc/max111x.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
#include "hw/qdev-properties.h"
-typedef struct {
- SSISlave parent_obj;
-
- qemu_irq interrupt;
- /* Values of inputs at system reset (settable by QOM property) */
- uint8_t reset_input[8];
-
- uint8_t tb1, rb2, rb3;
- int cycle;
-
- uint8_t input[8];
- int inputs, com;
-} MAX111xState;
-
-#define TYPE_MAX_111X "max111x"
-
-#define MAX_111X(obj) \
- OBJECT_CHECK(MAX111xState, (obj), TYPE_MAX_111X)
-
-#define TYPE_MAX_1110 "max1110"
-#define TYPE_MAX_1111 "max1111"
-
/* Control-byte bitfields */
#define CB_PD0 (1 << 0)
#define CB_PD1 (1 << 1)
diff --git a/MAINTAINERS b/MAINTAINERS
index 1b40446c739..550844d1514 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -787,6 +787,7 @@ F: hw/gpio/max7310.c
F: hw/gpio/zaurus.c
F: hw/misc/mst_fpga.c
F: hw/misc/max111x.c
+F: include/hw/misc/max111x.h
F: include/hw/arm/pxa.h
F: include/hw/arm/sharpsl.h
F: include/hw/display/tc6393xb.h
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 11/17] hw/misc/max111x: Create header file for documentation, TYPE_ macros
2020-06-28 14:24 ` [PATCH 11/17] hw/misc/max111x: Create header file for documentation, TYPE_ macros Peter Maydell
@ 2020-06-29 8:29 ` Philippe Mathieu-Daudé
2020-06-29 12:07 ` Peter Maydell
0 siblings, 1 reply; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-29 8:29 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis
On 6/28/20 4:24 PM, Peter Maydell wrote:
> Create a header file for the hw/misc/max111x device, in the
> usual modern style for QOM devices:
> * definition of the TYPE_ constants and macros
> * definition of the device's state struct so that it can
> be embedded in other structs if desired
> * documentation of the interface
>
> This allows us to use TYPE_MAX_1111 in the spitz.c code rather
> than the string "max1111".
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> To be honest the main driver here is that I wanted a header
> file to put the documentation comment in :-)
> ---
> include/hw/misc/max111x.h | 57 +++++++++++++++++++++++++++++++++++++++
> hw/arm/spitz.c | 3 ++-
> hw/misc/max111x.c | 25 +----------------
> MAINTAINERS | 1 +
> 4 files changed, 61 insertions(+), 25 deletions(-)
> create mode 100644 include/hw/misc/max111x.h
>
> diff --git a/include/hw/misc/max111x.h b/include/hw/misc/max111x.h
> new file mode 100644
> index 00000000000..337ba63d588
> --- /dev/null
> +++ b/include/hw/misc/max111x.h
> @@ -0,0 +1,57 @@
> +/*
> + * Maxim MAX1110/1111 ADC chip emulation.
> + *
> + * Copyright (c) 2006 Openedhand Ltd.
> + * Written by Andrzej Zaborowski <balrog@zabor.org>
> + *
> + * This code is licensed under the GNU GPLv2.
> + *
> + * Contributions after 2012-01-13 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + */
> +
> +#ifndef HW_MISC_MAX111X_H
> +#define HW_MISC_MAX111X_H
> +
> +#include "hw/ssi/ssi.h"
> +#include "hw/irq.h"
"hw/irq.h" not needed as qemu_irq is forward-declared in
"qemu/typedefs.h".
> +
> +/*
> + * This is a model of the Maxim MAX1110/1111 ADC chip, which for QEMU
> + * is an SSI slave device. It has either 4 (max1110) or 8 (max1111)
> + * 8-bit ADC channels.
> + *
> + * QEMU interface:
> + * + GPIO inputs 0..3 (for max1110) or 0..7 (for max1111): set the value
> + * of each ADC input, as an unsigned 8-bit value
> + * + GPIO output 0: interrupt line
> + * + Properties "input0" to "input3" (max1110) or "input0" to "input7"
> + * (max1111): initial reset values for ADC inputs.
> + *
> + * Known bugs:
> + * + the interrupt line is not correctly implemented, and will never
> + * be lowered once it has been asserted.
> + */
> +typedef struct {
> + SSISlave parent_obj;
> +
> + qemu_irq interrupt;
> + /* Values of inputs at system reset (settable by QOM property) */
> + uint8_t reset_input[8];
> +
> + uint8_t tb1, rb2, rb3;
> + int cycle;
> +
> + uint8_t input[8];
> + int inputs, com;
> +} MAX111xState;
> +
> +#define TYPE_MAX_111X "max111x"
> +
> +#define MAX_111X(obj) \
> + OBJECT_CHECK(MAX111xState, (obj), TYPE_MAX_111X)
Nitpick, we can keep MAX_111X() + MAX111xState in "hw/misc/max111x.c"
until we get a consumer.
> +
> +#define TYPE_MAX_1110 "max1110"
> +#define TYPE_MAX_1111 "max1111"
> +
> +#endif
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index fa592aad6d6..1400a56729d 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -29,6 +29,7 @@
> #include "audio/audio.h"
> #include "hw/boards.h"
> #include "hw/sysbus.h"
> +#include "hw/misc/max111x.h"
> #include "migration/vmstate.h"
> #include "exec/address-spaces.h"
> #include "cpu.h"
> @@ -732,7 +733,7 @@ static void spitz_ssp_attach(SpitzMachineState *sms)
> qdev_get_gpio_in(sms->mpu->gpio, SPITZ_GPIO_TP_INT));
>
> bus = qdev_get_child_bus(sms->mux, "ssi2");
> - sms->max1111 = qdev_new("max1111");
> + sms->max1111 = qdev_new(TYPE_MAX_1111);
> max1111 = sms->max1111;
> qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */,
> SPITZ_BATTERY_VOLT);
> diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
> index 3a5cb838445..d41cfd92a8d 100644
> --- a/hw/misc/max111x.c
> +++ b/hw/misc/max111x.c
> @@ -11,34 +11,11 @@
> */
>
> #include "qemu/osdep.h"
> -#include "hw/irq.h"
You still need "hw/irq.h" for qemu_irq_raise().
> -#include "hw/ssi/ssi.h"
> +#include "hw/misc/max111x.h"
> #include "migration/vmstate.h"
> #include "qemu/module.h"
> #include "hw/qdev-properties.h"
>
> -typedef struct {
> - SSISlave parent_obj;
> -
> - qemu_irq interrupt;
> - /* Values of inputs at system reset (settable by QOM property) */
> - uint8_t reset_input[8];
> -
> - uint8_t tb1, rb2, rb3;
> - int cycle;
> -
> - uint8_t input[8];
> - int inputs, com;
> -} MAX111xState;
> -
> -#define TYPE_MAX_111X "max111x"
> -
> -#define MAX_111X(obj) \
> - OBJECT_CHECK(MAX111xState, (obj), TYPE_MAX_111X)
> -
> -#define TYPE_MAX_1110 "max1110"
> -#define TYPE_MAX_1111 "max1111"
> -
> /* Control-byte bitfields */
> #define CB_PD0 (1 << 0)
> #define CB_PD1 (1 << 1)
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1b40446c739..550844d1514 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -787,6 +787,7 @@ F: hw/gpio/max7310.c
> F: hw/gpio/zaurus.c
> F: hw/misc/mst_fpga.c
> F: hw/misc/max111x.c
> +F: include/hw/misc/max111x.h
> F: include/hw/arm/pxa.h
> F: include/hw/arm/sharpsl.h
> F: include/hw/display/tc6393xb.h
>
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 11/17] hw/misc/max111x: Create header file for documentation, TYPE_ macros
2020-06-29 8:29 ` Philippe Mathieu-Daudé
@ 2020-06-29 12:07 ` Peter Maydell
2020-06-29 14:57 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 50+ messages in thread
From: Peter Maydell @ 2020-06-29 12:07 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-arm, QEMU Developers, Alistair Francis
On Mon, 29 Jun 2020 at 09:29, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 6/28/20 4:24 PM, Peter Maydell wrote:
> > Create a header file for the hw/misc/max111x device, in the
> > usual modern style for QOM devices:
> > * definition of the TYPE_ constants and macros
> > * definition of the device's state struct so that it can
> > be embedded in other structs if desired
> > * documentation of the interface
> >
> > This allows us to use TYPE_MAX_1111 in the spitz.c code rather
> > than the string "max1111".
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> Nitpick, we can keep MAX_111X() + MAX111xState in "hw/misc/max111x.c"
> until we get a consumer.
This is deliberate, as noted in the commit message.
thanks
-- PMM
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 11/17] hw/misc/max111x: Create header file for documentation, TYPE_ macros
2020-06-29 12:07 ` Peter Maydell
@ 2020-06-29 14:57 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-29 14:57 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers, Alistair Francis
On 6/29/20 2:07 PM, Peter Maydell wrote:
> On Mon, 29 Jun 2020 at 09:29, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> On 6/28/20 4:24 PM, Peter Maydell wrote:
>>> Create a header file for the hw/misc/max111x device, in the
>>> usual modern style for QOM devices:
>>> * definition of the TYPE_ constants and macros
>>> * definition of the device's state struct so that it can
>>> be embedded in other structs if desired
Ah, fine.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> * documentation of the interface
>>>
>>> This allows us to use TYPE_MAX_1111 in the spitz.c code rather
>>> than the string "max1111".
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>
>> Nitpick, we can keep MAX_111X() + MAX111xState in "hw/misc/max111x.c"
>> until we get a consumer.
>
> This is deliberate, as noted in the commit message.
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 12/17] hw/arm/spitz: Encapsulate misc GPIO handling in a device
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (10 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 11/17] hw/misc/max111x: Create header file for documentation, TYPE_ macros Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 9:12 ` Philippe Mathieu-Daudé
2020-07-02 17:39 ` Alistair Francis
2020-06-28 14:24 ` [PATCH 13/17] hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses Peter Maydell
` (5 subsequent siblings)
17 siblings, 2 replies; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
Currently we have a free-floating set of IRQs and a function
spitz_out_switch() which handle some miscellaneous GPIO lines for the
spitz board. Encapsulate this behaviour in a simple QOM device.
At this point we can finally remove the 'max1111' global, because the
ADC battery-temperature value is now handled by the misc-gpio device
writing the value to its outbound "adc-temp" GPIO, which the board
code wires up to the appropriate inbound GPIO line on the max1111.
This commit also fixes Coverity issue CID 1421913 (which pointed out
that the 'outsignals' in spitz_scoop_gpio_setup() were leaked),
because it removes the use of the qemu_allocate_irqs() API from this
code entirely.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/spitz.c | 129 +++++++++++++++++++++++++++++++++----------------
1 file changed, 87 insertions(+), 42 deletions(-)
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 1400a56729d..bab9968ccee 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -51,6 +51,7 @@ typedef struct {
DeviceState *max1111;
DeviceState *scp0;
DeviceState *scp1;
+ DeviceState *misc_gpio;
} SpitzMachineState;
#define TYPE_SPITZ_MACHINE "spitz-common"
@@ -658,8 +659,6 @@ static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp)
#define SPITZ_GPIO_MAX1111_CS 20
#define SPITZ_GPIO_TP_INT 11
-static DeviceState *max1111;
-
/* "Demux" the signal based on current chipselect */
typedef struct {
SSISlave ssidev;
@@ -695,18 +694,6 @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level)
#define SPITZ_BATTERY_VOLT 0xd0 /* About 4.0V */
#define SPITZ_CHARGEON_ACIN 0x80 /* About 5.0V */
-static void spitz_adc_temp_on(void *opaque, int line, int level)
-{
- int batt_temp;
-
- if (!max1111)
- return;
-
- batt_temp = level ? SPITZ_BATTERY_TEMP : 0;
-
- qemu_set_irq(qdev_get_gpio_in(max1111, MAX1111_BATT_TEMP), batt_temp);
-}
-
static void corgi_ssp_realize(SSISlave *d, Error **errp)
{
DeviceState *dev = DEVICE(d);
@@ -734,7 +721,6 @@ static void spitz_ssp_attach(SpitzMachineState *sms)
bus = qdev_get_child_bus(sms->mux, "ssi2");
sms->max1111 = qdev_new(TYPE_MAX_1111);
- max1111 = sms->max1111;
qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */,
SPITZ_BATTERY_VOLT);
qdev_prop_set_uint8(sms->max1111, "input2" /* BATT_TEMP */, 0);
@@ -810,27 +796,66 @@ static void spitz_akita_i2c_setup(PXA2xxState *cpu)
/* Other peripherals */
-static void spitz_out_switch(void *opaque, int line, int level)
+/*
+ * Encapsulation of some miscellaneous GPIO line behaviour for the Spitz boards.
+ *
+ * QEMU interface:
+ * + named GPIO inputs "green-led", "orange-led", "charging", "discharging":
+ * these currently just print messages that the line has been signalled
+ * + named GPIO input "adc-temp-on": set to cause the battery-temperature
+ * value to be passed to the max111x ADC
+ * + named GPIO output "adc-temp": the ADC value, to be wired up to the max111x
+ */
+#define TYPE_SPITZ_MISC_GPIO "spitz-misc-gpio"
+#define SPITZ_MISC_GPIO(obj) \
+ OBJECT_CHECK(SpitzMiscGPIOState, (obj), TYPE_SPITZ_MISC_GPIO)
+
+typedef struct SpitzMiscGPIOState {
+ SysBusDevice parent_obj;
+
+ qemu_irq adc_value;
+} SpitzMiscGPIOState;
+
+static void spitz_misc_charging(void *opaque, int n, int level)
{
- switch (line) {
- case 0:
- zaurus_printf("Charging %s.\n", level ? "off" : "on");
- break;
- case 1:
- zaurus_printf("Discharging %s.\n", level ? "on" : "off");
- break;
- case 2:
- zaurus_printf("Green LED %s.\n", level ? "on" : "off");
- break;
- case 3:
- zaurus_printf("Orange LED %s.\n", level ? "on" : "off");
- break;
- case 6:
- spitz_adc_temp_on(opaque, line, level);
- break;
- default:
- g_assert_not_reached();
- }
+ zaurus_printf("Charging %s.\n", level ? "off" : "on");
+}
+
+static void spitz_misc_discharging(void *opaque, int n, int level)
+{
+ zaurus_printf("Discharging %s.\n", level ? "off" : "on");
+}
+
+static void spitz_misc_green_led(void *opaque, int n, int level)
+{
+ zaurus_printf("Green LED %s.\n", level ? "off" : "on");
+}
+
+static void spitz_misc_orange_led(void *opaque, int n, int level)
+{
+ zaurus_printf("Orange LED %s.\n", level ? "off" : "on");
+}
+
+static void spitz_misc_adc_temp(void *opaque, int n, int level)
+{
+ SpitzMiscGPIOState *s = SPITZ_MISC_GPIO(opaque);
+ int batt_temp = level ? SPITZ_BATTERY_TEMP : 0;
+
+ qemu_set_irq(s->adc_value, batt_temp);
+}
+
+static void spitz_misc_gpio_init(Object *obj)
+{
+ SpitzMiscGPIOState *s = SPITZ_MISC_GPIO(obj);
+ DeviceState *dev = DEVICE(obj);
+
+ qdev_init_gpio_in_named(dev, spitz_misc_charging, "charging", 1);
+ qdev_init_gpio_in_named(dev, spitz_misc_discharging, "discharging", 1);
+ qdev_init_gpio_in_named(dev, spitz_misc_green_led, "green-led", 1);
+ qdev_init_gpio_in_named(dev, spitz_misc_orange_led, "orange-led", 1);
+ qdev_init_gpio_in_named(dev, spitz_misc_adc_temp, "adc-temp-on", 1);
+
+ qdev_init_gpio_out_named(dev, &s->adc_value, "adc-temp", 1);
}
#define SPITZ_SCP_LED_GREEN 1
@@ -850,12 +875,22 @@ static void spitz_out_switch(void *opaque, int line, int level)
static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
{
- qemu_irq *outsignals = qemu_allocate_irqs(spitz_out_switch, sms->mpu, 8);
+ DeviceState *miscdev = sysbus_create_simple(TYPE_SPITZ_MISC_GPIO, -1, NULL);
- qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_CHRG_ON, outsignals[0]);
- qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_JK_B, outsignals[1]);
- qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_GREEN, outsignals[2]);
- qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_ORANGE, outsignals[3]);
+ sms->misc_gpio = miscdev;
+
+ qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_CHRG_ON,
+ qdev_get_gpio_in_named(miscdev, "charging", 0));
+ qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_JK_B,
+ qdev_get_gpio_in_named(miscdev, "discharging", 0));
+ qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_GREEN,
+ qdev_get_gpio_in_named(miscdev, "green-led", 0));
+ qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_ORANGE,
+ qdev_get_gpio_in_named(miscdev, "orange-led", 0));
+ qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON,
+ qdev_get_gpio_in_named(miscdev, "adc-temp-on", 0));
+ qdev_connect_gpio_out_named(miscdev, "adc-temp", 0,
+ qdev_get_gpio_in(sms->max1111, MAX1111_BATT_TEMP));
if (sms->scp1) {
qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_CONT,
@@ -863,8 +898,6 @@ static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_ON,
qdev_get_gpio_in_named(sms->lcdtg, "bl_power", 0));
}
-
- qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
}
#define SPITZ_GPIO_HSYNC 22
@@ -1217,12 +1250,24 @@ static const TypeInfo spitz_lcdtg_info = {
.class_init = spitz_lcdtg_class_init,
};
+static const TypeInfo spitz_misc_gpio_info = {
+ .name = TYPE_SPITZ_MISC_GPIO,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(SpitzMiscGPIOState),
+ .instance_init = spitz_misc_gpio_init,
+ /*
+ * No class_init required: device has no internal state so does not
+ * need to set up reset or vmstate, and does not have a realize method.
+ */
+};
+
static void spitz_register_types(void)
{
type_register_static(&corgi_ssp_info);
type_register_static(&spitz_lcdtg_info);
type_register_static(&spitz_keyboard_info);
type_register_static(&sl_nand_info);
+ type_register_static(&spitz_misc_gpio_info);
}
type_init(spitz_register_types)
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 12/17] hw/arm/spitz: Encapsulate misc GPIO handling in a device
2020-06-28 14:24 ` [PATCH 12/17] hw/arm/spitz: Encapsulate misc GPIO handling in a device Peter Maydell
@ 2020-06-29 9:12 ` Philippe Mathieu-Daudé
2020-07-02 17:39 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-29 9:12 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis
On 6/28/20 4:24 PM, Peter Maydell wrote:
> Currently we have a free-floating set of IRQs and a function
> spitz_out_switch() which handle some miscellaneous GPIO lines for the
> spitz board. Encapsulate this behaviour in a simple QOM device.
>
> At this point we can finally remove the 'max1111' global, because the
> ADC battery-temperature value is now handled by the misc-gpio device
> writing the value to its outbound "adc-temp" GPIO, which the board
> code wires up to the appropriate inbound GPIO line on the max1111.
>
> This commit also fixes Coverity issue CID 1421913 (which pointed out
> that the 'outsignals' in spitz_scoop_gpio_setup() were leaked),
> because it removes the use of the qemu_allocate_irqs() API from this
> code entirely.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/arm/spitz.c | 129 +++++++++++++++++++++++++++++++++----------------
> 1 file changed, 87 insertions(+), 42 deletions(-)
>
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index 1400a56729d..bab9968ccee 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -51,6 +51,7 @@ typedef struct {
> DeviceState *max1111;
> DeviceState *scp0;
> DeviceState *scp1;
> + DeviceState *misc_gpio;
> } SpitzMachineState;
>
> #define TYPE_SPITZ_MACHINE "spitz-common"
> @@ -658,8 +659,6 @@ static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp)
> #define SPITZ_GPIO_MAX1111_CS 20
> #define SPITZ_GPIO_TP_INT 11
>
> -static DeviceState *max1111;
> -
> /* "Demux" the signal based on current chipselect */
> typedef struct {
> SSISlave ssidev;
> @@ -695,18 +694,6 @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level)
> #define SPITZ_BATTERY_VOLT 0xd0 /* About 4.0V */
> #define SPITZ_CHARGEON_ACIN 0x80 /* About 5.0V */
>
> -static void spitz_adc_temp_on(void *opaque, int line, int level)
> -{
> - int batt_temp;
> -
> - if (!max1111)
> - return;
> -
> - batt_temp = level ? SPITZ_BATTERY_TEMP : 0;
> -
> - qemu_set_irq(qdev_get_gpio_in(max1111, MAX1111_BATT_TEMP), batt_temp);
> -}
> -
> static void corgi_ssp_realize(SSISlave *d, Error **errp)
> {
> DeviceState *dev = DEVICE(d);
> @@ -734,7 +721,6 @@ static void spitz_ssp_attach(SpitzMachineState *sms)
>
> bus = qdev_get_child_bus(sms->mux, "ssi2");
> sms->max1111 = qdev_new(TYPE_MAX_1111);
> - max1111 = sms->max1111;
> qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */,
> SPITZ_BATTERY_VOLT);
> qdev_prop_set_uint8(sms->max1111, "input2" /* BATT_TEMP */, 0);
> @@ -810,27 +796,66 @@ static void spitz_akita_i2c_setup(PXA2xxState *cpu)
>
> /* Other peripherals */
>
> -static void spitz_out_switch(void *opaque, int line, int level)
> +/*
> + * Encapsulation of some miscellaneous GPIO line behaviour for the Spitz boards.
> + *
> + * QEMU interface:
> + * + named GPIO inputs "green-led", "orange-led", "charging", "discharging":
> + * these currently just print messages that the line has been signalled
> + * + named GPIO input "adc-temp-on": set to cause the battery-temperature
> + * value to be passed to the max111x ADC
> + * + named GPIO output "adc-temp": the ADC value, to be wired up to the max111x
> + */
> +#define TYPE_SPITZ_MISC_GPIO "spitz-misc-gpio"
> +#define SPITZ_MISC_GPIO(obj) \
> + OBJECT_CHECK(SpitzMiscGPIOState, (obj), TYPE_SPITZ_MISC_GPIO)
> +
> +typedef struct SpitzMiscGPIOState {
> + SysBusDevice parent_obj;
> +
> + qemu_irq adc_value;
> +} SpitzMiscGPIOState;
> +
> +static void spitz_misc_charging(void *opaque, int n, int level)
> {
> - switch (line) {
> - case 0:
> - zaurus_printf("Charging %s.\n", level ? "off" : "on");
> - break;
> - case 1:
> - zaurus_printf("Discharging %s.\n", level ? "on" : "off");
> - break;
> - case 2:
> - zaurus_printf("Green LED %s.\n", level ? "on" : "off");
> - break;
> - case 3:
> - zaurus_printf("Orange LED %s.\n", level ? "on" : "off");
> - break;
> - case 6:
> - spitz_adc_temp_on(opaque, line, level);
> - break;
> - default:
> - g_assert_not_reached();
> - }
> + zaurus_printf("Charging %s.\n", level ? "off" : "on");
> +}
> +
> +static void spitz_misc_discharging(void *opaque, int n, int level)
> +{
> + zaurus_printf("Discharging %s.\n", level ? "off" : "on");
> +}
> +
> +static void spitz_misc_green_led(void *opaque, int n, int level)
> +{
> + zaurus_printf("Green LED %s.\n", level ? "off" : "on");
> +}
> +
> +static void spitz_misc_orange_led(void *opaque, int n, int level)
> +{
> + zaurus_printf("Orange LED %s.\n", level ? "off" : "on");
> +}
> +
> +static void spitz_misc_adc_temp(void *opaque, int n, int level)
> +{
> + SpitzMiscGPIOState *s = SPITZ_MISC_GPIO(opaque);
> + int batt_temp = level ? SPITZ_BATTERY_TEMP : 0;
> +
> + qemu_set_irq(s->adc_value, batt_temp);
> +}
> +
> +static void spitz_misc_gpio_init(Object *obj)
> +{
> + SpitzMiscGPIOState *s = SPITZ_MISC_GPIO(obj);
> + DeviceState *dev = DEVICE(obj);
> +
> + qdev_init_gpio_in_named(dev, spitz_misc_charging, "charging", 1);
> + qdev_init_gpio_in_named(dev, spitz_misc_discharging, "discharging", 1);
> + qdev_init_gpio_in_named(dev, spitz_misc_green_led, "green-led", 1);
> + qdev_init_gpio_in_named(dev, spitz_misc_orange_led, "orange-led", 1);
> + qdev_init_gpio_in_named(dev, spitz_misc_adc_temp, "adc-temp-on", 1);
> +
> + qdev_init_gpio_out_named(dev, &s->adc_value, "adc-temp", 1);
> }
>
> #define SPITZ_SCP_LED_GREEN 1
> @@ -850,12 +875,22 @@ static void spitz_out_switch(void *opaque, int line, int level)
>
> static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
> {
> - qemu_irq *outsignals = qemu_allocate_irqs(spitz_out_switch, sms->mpu, 8);
> + DeviceState *miscdev = sysbus_create_simple(TYPE_SPITZ_MISC_GPIO, -1, NULL);
>
> - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_CHRG_ON, outsignals[0]);
> - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_JK_B, outsignals[1]);
> - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_GREEN, outsignals[2]);
> - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_ORANGE, outsignals[3]);
> + sms->misc_gpio = miscdev;
> +
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_CHRG_ON,
> + qdev_get_gpio_in_named(miscdev, "charging", 0));
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_JK_B,
> + qdev_get_gpio_in_named(miscdev, "discharging", 0));
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_GREEN,
> + qdev_get_gpio_in_named(miscdev, "green-led", 0));
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_ORANGE,
> + qdev_get_gpio_in_named(miscdev, "orange-led", 0));
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON,
> + qdev_get_gpio_in_named(miscdev, "adc-temp-on", 0));
> + qdev_connect_gpio_out_named(miscdev, "adc-temp", 0,
> + qdev_get_gpio_in(sms->max1111, MAX1111_BATT_TEMP));
>
> if (sms->scp1) {
> qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_CONT,
> @@ -863,8 +898,6 @@ static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
> qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_ON,
> qdev_get_gpio_in_named(sms->lcdtg, "bl_power", 0));
> }
> -
> - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
> }
>
> #define SPITZ_GPIO_HSYNC 22
> @@ -1217,12 +1250,24 @@ static const TypeInfo spitz_lcdtg_info = {
> .class_init = spitz_lcdtg_class_init,
> };
>
> +static const TypeInfo spitz_misc_gpio_info = {
> + .name = TYPE_SPITZ_MISC_GPIO,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(SpitzMiscGPIOState),
> + .instance_init = spitz_misc_gpio_init,
> + /*
> + * No class_init required: device has no internal state so does not
> + * need to set up reset or vmstate, and does not have a realize method.
> + */
> +};
> +
> static void spitz_register_types(void)
> {
> type_register_static(&corgi_ssp_info);
> type_register_static(&spitz_lcdtg_info);
> type_register_static(&spitz_keyboard_info);
> type_register_static(&sl_nand_info);
> + type_register_static(&spitz_misc_gpio_info);
> }
>
> type_init(spitz_register_types)
>
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 12/17] hw/arm/spitz: Encapsulate misc GPIO handling in a device
2020-06-28 14:24 ` [PATCH 12/17] hw/arm/spitz: Encapsulate misc GPIO handling in a device Peter Maydell
2020-06-29 9:12 ` Philippe Mathieu-Daudé
@ 2020-07-02 17:39 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Alistair Francis @ 2020-07-02 17:39 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sun, Jun 28, 2020 at 7:33 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Currently we have a free-floating set of IRQs and a function
> spitz_out_switch() which handle some miscellaneous GPIO lines for the
> spitz board. Encapsulate this behaviour in a simple QOM device.
>
> At this point we can finally remove the 'max1111' global, because the
> ADC battery-temperature value is now handled by the misc-gpio device
> writing the value to its outbound "adc-temp" GPIO, which the board
> code wires up to the appropriate inbound GPIO line on the max1111.
>
> This commit also fixes Coverity issue CID 1421913 (which pointed out
> that the 'outsignals' in spitz_scoop_gpio_setup() were leaked),
> because it removes the use of the qemu_allocate_irqs() API from this
> code entirely.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/arm/spitz.c | 129 +++++++++++++++++++++++++++++++++----------------
> 1 file changed, 87 insertions(+), 42 deletions(-)
>
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index 1400a56729d..bab9968ccee 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -51,6 +51,7 @@ typedef struct {
> DeviceState *max1111;
> DeviceState *scp0;
> DeviceState *scp1;
> + DeviceState *misc_gpio;
> } SpitzMachineState;
>
> #define TYPE_SPITZ_MACHINE "spitz-common"
> @@ -658,8 +659,6 @@ static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp)
> #define SPITZ_GPIO_MAX1111_CS 20
> #define SPITZ_GPIO_TP_INT 11
>
> -static DeviceState *max1111;
> -
> /* "Demux" the signal based on current chipselect */
> typedef struct {
> SSISlave ssidev;
> @@ -695,18 +694,6 @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level)
> #define SPITZ_BATTERY_VOLT 0xd0 /* About 4.0V */
> #define SPITZ_CHARGEON_ACIN 0x80 /* About 5.0V */
>
> -static void spitz_adc_temp_on(void *opaque, int line, int level)
> -{
> - int batt_temp;
> -
> - if (!max1111)
> - return;
> -
> - batt_temp = level ? SPITZ_BATTERY_TEMP : 0;
> -
> - qemu_set_irq(qdev_get_gpio_in(max1111, MAX1111_BATT_TEMP), batt_temp);
> -}
> -
> static void corgi_ssp_realize(SSISlave *d, Error **errp)
> {
> DeviceState *dev = DEVICE(d);
> @@ -734,7 +721,6 @@ static void spitz_ssp_attach(SpitzMachineState *sms)
>
> bus = qdev_get_child_bus(sms->mux, "ssi2");
> sms->max1111 = qdev_new(TYPE_MAX_1111);
> - max1111 = sms->max1111;
> qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */,
> SPITZ_BATTERY_VOLT);
> qdev_prop_set_uint8(sms->max1111, "input2" /* BATT_TEMP */, 0);
> @@ -810,27 +796,66 @@ static void spitz_akita_i2c_setup(PXA2xxState *cpu)
>
> /* Other peripherals */
>
> -static void spitz_out_switch(void *opaque, int line, int level)
> +/*
> + * Encapsulation of some miscellaneous GPIO line behaviour for the Spitz boards.
> + *
> + * QEMU interface:
> + * + named GPIO inputs "green-led", "orange-led", "charging", "discharging":
> + * these currently just print messages that the line has been signalled
> + * + named GPIO input "adc-temp-on": set to cause the battery-temperature
> + * value to be passed to the max111x ADC
> + * + named GPIO output "adc-temp": the ADC value, to be wired up to the max111x
> + */
> +#define TYPE_SPITZ_MISC_GPIO "spitz-misc-gpio"
> +#define SPITZ_MISC_GPIO(obj) \
> + OBJECT_CHECK(SpitzMiscGPIOState, (obj), TYPE_SPITZ_MISC_GPIO)
> +
> +typedef struct SpitzMiscGPIOState {
> + SysBusDevice parent_obj;
> +
> + qemu_irq adc_value;
> +} SpitzMiscGPIOState;
> +
> +static void spitz_misc_charging(void *opaque, int n, int level)
> {
> - switch (line) {
> - case 0:
> - zaurus_printf("Charging %s.\n", level ? "off" : "on");
> - break;
> - case 1:
> - zaurus_printf("Discharging %s.\n", level ? "on" : "off");
> - break;
> - case 2:
> - zaurus_printf("Green LED %s.\n", level ? "on" : "off");
> - break;
> - case 3:
> - zaurus_printf("Orange LED %s.\n", level ? "on" : "off");
> - break;
> - case 6:
> - spitz_adc_temp_on(opaque, line, level);
> - break;
> - default:
> - g_assert_not_reached();
> - }
> + zaurus_printf("Charging %s.\n", level ? "off" : "on");
> +}
> +
> +static void spitz_misc_discharging(void *opaque, int n, int level)
> +{
> + zaurus_printf("Discharging %s.\n", level ? "off" : "on");
> +}
> +
> +static void spitz_misc_green_led(void *opaque, int n, int level)
> +{
> + zaurus_printf("Green LED %s.\n", level ? "off" : "on");
> +}
> +
> +static void spitz_misc_orange_led(void *opaque, int n, int level)
> +{
> + zaurus_printf("Orange LED %s.\n", level ? "off" : "on");
> +}
> +
> +static void spitz_misc_adc_temp(void *opaque, int n, int level)
> +{
> + SpitzMiscGPIOState *s = SPITZ_MISC_GPIO(opaque);
> + int batt_temp = level ? SPITZ_BATTERY_TEMP : 0;
> +
> + qemu_set_irq(s->adc_value, batt_temp);
> +}
> +
> +static void spitz_misc_gpio_init(Object *obj)
> +{
> + SpitzMiscGPIOState *s = SPITZ_MISC_GPIO(obj);
> + DeviceState *dev = DEVICE(obj);
> +
> + qdev_init_gpio_in_named(dev, spitz_misc_charging, "charging", 1);
> + qdev_init_gpio_in_named(dev, spitz_misc_discharging, "discharging", 1);
> + qdev_init_gpio_in_named(dev, spitz_misc_green_led, "green-led", 1);
> + qdev_init_gpio_in_named(dev, spitz_misc_orange_led, "orange-led", 1);
> + qdev_init_gpio_in_named(dev, spitz_misc_adc_temp, "adc-temp-on", 1);
> +
> + qdev_init_gpio_out_named(dev, &s->adc_value, "adc-temp", 1);
> }
>
> #define SPITZ_SCP_LED_GREEN 1
> @@ -850,12 +875,22 @@ static void spitz_out_switch(void *opaque, int line, int level)
>
> static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
> {
> - qemu_irq *outsignals = qemu_allocate_irqs(spitz_out_switch, sms->mpu, 8);
> + DeviceState *miscdev = sysbus_create_simple(TYPE_SPITZ_MISC_GPIO, -1, NULL);
>
> - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_CHRG_ON, outsignals[0]);
> - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_JK_B, outsignals[1]);
> - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_GREEN, outsignals[2]);
> - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_ORANGE, outsignals[3]);
> + sms->misc_gpio = miscdev;
> +
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_CHRG_ON,
> + qdev_get_gpio_in_named(miscdev, "charging", 0));
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_JK_B,
> + qdev_get_gpio_in_named(miscdev, "discharging", 0));
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_GREEN,
> + qdev_get_gpio_in_named(miscdev, "green-led", 0));
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_LED_ORANGE,
> + qdev_get_gpio_in_named(miscdev, "orange-led", 0));
> + qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON,
> + qdev_get_gpio_in_named(miscdev, "adc-temp-on", 0));
> + qdev_connect_gpio_out_named(miscdev, "adc-temp", 0,
> + qdev_get_gpio_in(sms->max1111, MAX1111_BATT_TEMP));
>
> if (sms->scp1) {
> qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_CONT,
> @@ -863,8 +898,6 @@ static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
> qdev_connect_gpio_out(sms->scp1, SPITZ_SCP2_BACKLIGHT_ON,
> qdev_get_gpio_in_named(sms->lcdtg, "bl_power", 0));
> }
> -
> - qdev_connect_gpio_out(sms->scp0, SPITZ_SCP_ADC_TEMP_ON, outsignals[6]);
> }
>
> #define SPITZ_GPIO_HSYNC 22
> @@ -1217,12 +1250,24 @@ static const TypeInfo spitz_lcdtg_info = {
> .class_init = spitz_lcdtg_class_init,
> };
>
> +static const TypeInfo spitz_misc_gpio_info = {
> + .name = TYPE_SPITZ_MISC_GPIO,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(SpitzMiscGPIOState),
> + .instance_init = spitz_misc_gpio_init,
> + /*
> + * No class_init required: device has no internal state so does not
> + * need to set up reset or vmstate, and does not have a realize method.
> + */
> +};
> +
> static void spitz_register_types(void)
> {
> type_register_static(&corgi_ssp_info);
> type_register_static(&spitz_lcdtg_info);
> type_register_static(&spitz_keyboard_info);
> type_register_static(&sl_nand_info);
> + type_register_static(&spitz_misc_gpio_info);
> }
>
> type_init(spitz_register_types)
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 13/17] hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (11 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 12/17] hw/arm/spitz: Encapsulate misc GPIO handling in a device Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 9:13 ` Philippe Mathieu-Daudé
2020-07-01 0:38 ` Alistair Francis
2020-06-28 14:24 ` [PATCH 14/17] hw/arm/spitz: " Peter Maydell
` (4 subsequent siblings)
17 siblings, 2 replies; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
Instead of logging guest accesses to invalid register offsets in this
device using zaurus_printf() (which just prints to stderr), use the
usual qemu_log_mask(LOG_GUEST_ERROR,...).
Since this was the only use of the zaurus_printf() macro outside
spitz.c, we can move the definition of that macro from sharpsl.h
to spitz.c.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/arm/sharpsl.h | 3 ---
hw/arm/spitz.c | 3 +++
hw/gpio/zaurus.c | 12 +++++++-----
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/hw/arm/sharpsl.h b/include/hw/arm/sharpsl.h
index 89e168fbff3..e986b28c527 100644
--- a/include/hw/arm/sharpsl.h
+++ b/include/hw/arm/sharpsl.h
@@ -9,9 +9,6 @@
#include "exec/hwaddr.h"
-#define zaurus_printf(format, ...) \
- fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__)
-
/* zaurus.c */
#define SL_PXA_PARAM_BASE 0xa0000a00
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index bab9968ccee..6eb46869157 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -62,6 +62,9 @@ typedef struct {
#define SPITZ_MACHINE_CLASS(klass) \
OBJECT_CLASS_CHECK(SpitzMachineClass, klass, TYPE_SPITZ_MACHINE)
+#define zaurus_printf(format, ...) \
+ fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__)
+
#undef REG_FMT
#define REG_FMT "0x%02lx"
diff --git a/hw/gpio/zaurus.c b/hw/gpio/zaurus.c
index 9a12c683420..258e9264930 100644
--- a/hw/gpio/zaurus.c
+++ b/hw/gpio/zaurus.c
@@ -22,9 +22,7 @@
#include "hw/sysbus.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
-
-#undef REG_FMT
-#define REG_FMT "0x%02lx"
+#include "qemu/log.h"
/* SCOOP devices */
@@ -104,7 +102,9 @@ static uint64_t scoop_read(void *opaque, hwaddr addr,
case SCOOP_GPRR:
return s->gpio_level;
default:
- zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "scoop_read: bad register offset 0x%02" HWADDR_PRIx "\n",
+ addr);
}
return 0;
@@ -150,7 +150,9 @@ static void scoop_write(void *opaque, hwaddr addr,
scoop_gpio_handler_update(s);
break;
default:
- zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "scoop_write: bad register offset 0x%02" HWADDR_PRIx "\n",
+ addr);
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 13/17] hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses
2020-06-28 14:24 ` [PATCH 13/17] hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses Peter Maydell
@ 2020-06-29 9:13 ` Philippe Mathieu-Daudé
2020-07-01 0:38 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-29 9:13 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis
On 6/28/20 4:24 PM, Peter Maydell wrote:
> Instead of logging guest accesses to invalid register offsets in this
> device using zaurus_printf() (which just prints to stderr), use the
> usual qemu_log_mask(LOG_GUEST_ERROR,...).
>
> Since this was the only use of the zaurus_printf() macro outside
> spitz.c, we can move the definition of that macro from sharpsl.h
> to spitz.c.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/arm/sharpsl.h | 3 ---
> hw/arm/spitz.c | 3 +++
> hw/gpio/zaurus.c | 12 +++++++-----
> 3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/arm/sharpsl.h b/include/hw/arm/sharpsl.h
> index 89e168fbff3..e986b28c527 100644
> --- a/include/hw/arm/sharpsl.h
> +++ b/include/hw/arm/sharpsl.h
> @@ -9,9 +9,6 @@
>
> #include "exec/hwaddr.h"
>
> -#define zaurus_printf(format, ...) \
> - fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__)
> -
> /* zaurus.c */
>
> #define SL_PXA_PARAM_BASE 0xa0000a00
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index bab9968ccee..6eb46869157 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -62,6 +62,9 @@ typedef struct {
> #define SPITZ_MACHINE_CLASS(klass) \
> OBJECT_CLASS_CHECK(SpitzMachineClass, klass, TYPE_SPITZ_MACHINE)
>
> +#define zaurus_printf(format, ...) \
> + fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__)
> +
> #undef REG_FMT
> #define REG_FMT "0x%02lx"
>
> diff --git a/hw/gpio/zaurus.c b/hw/gpio/zaurus.c
> index 9a12c683420..258e9264930 100644
> --- a/hw/gpio/zaurus.c
> +++ b/hw/gpio/zaurus.c
> @@ -22,9 +22,7 @@
> #include "hw/sysbus.h"
> #include "migration/vmstate.h"
> #include "qemu/module.h"
> -
> -#undef REG_FMT
> -#define REG_FMT "0x%02lx"
> +#include "qemu/log.h"
>
> /* SCOOP devices */
>
> @@ -104,7 +102,9 @@ static uint64_t scoop_read(void *opaque, hwaddr addr,
> case SCOOP_GPRR:
> return s->gpio_level;
> default:
> - zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "scoop_read: bad register offset 0x%02" HWADDR_PRIx "\n",
> + addr);
> }
>
> return 0;
> @@ -150,7 +150,9 @@ static void scoop_write(void *opaque, hwaddr addr,
> scoop_gpio_handler_update(s);
> break;
> default:
> - zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "scoop_write: bad register offset 0x%02" HWADDR_PRIx "\n",
> + addr);
> }
> }
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 13/17] hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses
2020-06-28 14:24 ` [PATCH 13/17] hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses Peter Maydell
2020-06-29 9:13 ` Philippe Mathieu-Daudé
@ 2020-07-01 0:38 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Alistair Francis @ 2020-07-01 0:38 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sun, Jun 28, 2020 at 7:35 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Instead of logging guest accesses to invalid register offsets in this
> device using zaurus_printf() (which just prints to stderr), use the
> usual qemu_log_mask(LOG_GUEST_ERROR,...).
>
> Since this was the only use of the zaurus_printf() macro outside
> spitz.c, we can move the definition of that macro from sharpsl.h
> to spitz.c.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/hw/arm/sharpsl.h | 3 ---
> hw/arm/spitz.c | 3 +++
> hw/gpio/zaurus.c | 12 +++++++-----
> 3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/arm/sharpsl.h b/include/hw/arm/sharpsl.h
> index 89e168fbff3..e986b28c527 100644
> --- a/include/hw/arm/sharpsl.h
> +++ b/include/hw/arm/sharpsl.h
> @@ -9,9 +9,6 @@
>
> #include "exec/hwaddr.h"
>
> -#define zaurus_printf(format, ...) \
> - fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__)
> -
> /* zaurus.c */
>
> #define SL_PXA_PARAM_BASE 0xa0000a00
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index bab9968ccee..6eb46869157 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -62,6 +62,9 @@ typedef struct {
> #define SPITZ_MACHINE_CLASS(klass) \
> OBJECT_CLASS_CHECK(SpitzMachineClass, klass, TYPE_SPITZ_MACHINE)
>
> +#define zaurus_printf(format, ...) \
> + fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__)
> +
> #undef REG_FMT
> #define REG_FMT "0x%02lx"
>
> diff --git a/hw/gpio/zaurus.c b/hw/gpio/zaurus.c
> index 9a12c683420..258e9264930 100644
> --- a/hw/gpio/zaurus.c
> +++ b/hw/gpio/zaurus.c
> @@ -22,9 +22,7 @@
> #include "hw/sysbus.h"
> #include "migration/vmstate.h"
> #include "qemu/module.h"
> -
> -#undef REG_FMT
> -#define REG_FMT "0x%02lx"
> +#include "qemu/log.h"
>
> /* SCOOP devices */
>
> @@ -104,7 +102,9 @@ static uint64_t scoop_read(void *opaque, hwaddr addr,
> case SCOOP_GPRR:
> return s->gpio_level;
> default:
> - zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "scoop_read: bad register offset 0x%02" HWADDR_PRIx "\n",
> + addr);
> }
>
> return 0;
> @@ -150,7 +150,9 @@ static void scoop_write(void *opaque, hwaddr addr,
> scoop_gpio_handler_update(s);
> break;
> default:
> - zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "scoop_write: bad register offset 0x%02" HWADDR_PRIx "\n",
> + addr);
> }
> }
>
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 14/17] hw/arm/spitz: Use LOG_GUEST_ERROR for bad guest register accesses
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (12 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 13/17] hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 9:14 ` Philippe Mathieu-Daudé
2020-07-01 0:52 ` Alistair Francis
2020-06-28 14:24 ` [PATCH 15/17] hw/arm/pxa2xx_pic: " Peter Maydell
` (3 subsequent siblings)
17 siblings, 2 replies; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
Instead of logging guest accesses to invalid register offsets in the
Spitz flash device with zaurus_printf() (which just prints to stderr),
use the usual qemu_log_mask(LOG_GUEST_ERROR,...).
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/spitz.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 6eb46869157..49eae3fce4e 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -23,6 +23,7 @@
#include "hw/ssi/ssi.h"
#include "hw/block/flash.h"
#include "qemu/timer.h"
+#include "qemu/log.h"
#include "hw/arm/sharpsl.h"
#include "ui/console.h"
#include "hw/audio/wm8750.h"
@@ -65,9 +66,6 @@ typedef struct {
#define zaurus_printf(format, ...) \
fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__)
-#undef REG_FMT
-#define REG_FMT "0x%02lx"
-
/* Spitz Flash */
#define FLASH_BASE 0x0c000000
#define FLASH_ECCLPLB 0x00 /* Line parity 7 - 0 bit */
@@ -137,7 +135,9 @@ static uint64_t sl_read(void *opaque, hwaddr addr, unsigned size)
return ecc_digest(&s->ecc, nand_getio(s->nand));
default:
- zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "sl_read: bad register offset 0x%02" HWADDR_PRIx "\n",
+ addr);
}
return 0;
}
@@ -168,7 +168,9 @@ static void sl_write(void *opaque, hwaddr addr,
break;
default:
- zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "sl_write: bad register offset 0x%02" HWADDR_PRIx "\n",
+ addr);
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 14/17] hw/arm/spitz: Use LOG_GUEST_ERROR for bad guest register accesses
2020-06-28 14:24 ` [PATCH 14/17] hw/arm/spitz: " Peter Maydell
@ 2020-06-29 9:14 ` Philippe Mathieu-Daudé
2020-07-01 0:52 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-29 9:14 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis
On 6/28/20 4:24 PM, Peter Maydell wrote:
> Instead of logging guest accesses to invalid register offsets in the
> Spitz flash device with zaurus_printf() (which just prints to stderr),
> use the usual qemu_log_mask(LOG_GUEST_ERROR,...).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/arm/spitz.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index 6eb46869157..49eae3fce4e 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -23,6 +23,7 @@
> #include "hw/ssi/ssi.h"
> #include "hw/block/flash.h"
> #include "qemu/timer.h"
> +#include "qemu/log.h"
> #include "hw/arm/sharpsl.h"
> #include "ui/console.h"
> #include "hw/audio/wm8750.h"
> @@ -65,9 +66,6 @@ typedef struct {
> #define zaurus_printf(format, ...) \
> fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__)
>
> -#undef REG_FMT
> -#define REG_FMT "0x%02lx"
> -
> /* Spitz Flash */
> #define FLASH_BASE 0x0c000000
> #define FLASH_ECCLPLB 0x00 /* Line parity 7 - 0 bit */
> @@ -137,7 +135,9 @@ static uint64_t sl_read(void *opaque, hwaddr addr, unsigned size)
> return ecc_digest(&s->ecc, nand_getio(s->nand));
>
> default:
> - zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "sl_read: bad register offset 0x%02" HWADDR_PRIx "\n",
> + addr);
> }
> return 0;
> }
> @@ -168,7 +168,9 @@ static void sl_write(void *opaque, hwaddr addr,
> break;
>
> default:
> - zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "sl_write: bad register offset 0x%02" HWADDR_PRIx "\n",
> + addr);
> }
> }
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 14/17] hw/arm/spitz: Use LOG_GUEST_ERROR for bad guest register accesses
2020-06-28 14:24 ` [PATCH 14/17] hw/arm/spitz: " Peter Maydell
2020-06-29 9:14 ` Philippe Mathieu-Daudé
@ 2020-07-01 0:52 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Alistair Francis @ 2020-07-01 0:52 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sun, Jun 28, 2020 at 7:33 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Instead of logging guest accesses to invalid register offsets in the
> Spitz flash device with zaurus_printf() (which just prints to stderr),
> use the usual qemu_log_mask(LOG_GUEST_ERROR,...).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/arm/spitz.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index 6eb46869157..49eae3fce4e 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -23,6 +23,7 @@
> #include "hw/ssi/ssi.h"
> #include "hw/block/flash.h"
> #include "qemu/timer.h"
> +#include "qemu/log.h"
> #include "hw/arm/sharpsl.h"
> #include "ui/console.h"
> #include "hw/audio/wm8750.h"
> @@ -65,9 +66,6 @@ typedef struct {
> #define zaurus_printf(format, ...) \
> fprintf(stderr, "%s: " format, __func__, ##__VA_ARGS__)
>
> -#undef REG_FMT
> -#define REG_FMT "0x%02lx"
> -
> /* Spitz Flash */
> #define FLASH_BASE 0x0c000000
> #define FLASH_ECCLPLB 0x00 /* Line parity 7 - 0 bit */
> @@ -137,7 +135,9 @@ static uint64_t sl_read(void *opaque, hwaddr addr, unsigned size)
> return ecc_digest(&s->ecc, nand_getio(s->nand));
>
> default:
> - zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "sl_read: bad register offset 0x%02" HWADDR_PRIx "\n",
> + addr);
> }
> return 0;
> }
> @@ -168,7 +168,9 @@ static void sl_write(void *opaque, hwaddr addr,
> break;
>
> default:
> - zaurus_printf("Bad register offset " REG_FMT "\n", (unsigned long)addr);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "sl_write: bad register offset 0x%02" HWADDR_PRIx "\n",
> + addr);
> }
> }
>
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 15/17] hw/arm/pxa2xx_pic: Use LOG_GUEST_ERROR for bad guest register accesses
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (13 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 14/17] hw/arm/spitz: " Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 9:14 ` Philippe Mathieu-Daudé
2020-07-01 0:52 ` Alistair Francis
2020-06-28 14:24 ` [PATCH 16/17] hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg Peter Maydell
` (2 subsequent siblings)
17 siblings, 2 replies; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
Instead of using printf() for logging guest accesses to invalid
register offsets in the pxa2xx PIC device, use the usual
qemu_log_mask(LOG_GUEST_ERROR,...).
This was the only user of the REG_FMT macro in pxa.h, so we can
remove that.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/arm/pxa.h | 1 -
hw/arm/pxa2xx_pic.c | 9 +++++++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/include/hw/arm/pxa.h b/include/hw/arm/pxa.h
index f6dfb5c0cf0..8843e5f9107 100644
--- a/include/hw/arm/pxa.h
+++ b/include/hw/arm/pxa.h
@@ -184,7 +184,6 @@ struct PXA2xxI2SState {
};
# define PA_FMT "0x%08lx"
-# define REG_FMT "0x" TARGET_FMT_plx
PXA2xxState *pxa270_init(MemoryRegion *address_space, unsigned int sdram_size,
const char *revision);
diff --git a/hw/arm/pxa2xx_pic.c b/hw/arm/pxa2xx_pic.c
index 105c5e63f2f..ceee6aa48db 100644
--- a/hw/arm/pxa2xx_pic.c
+++ b/hw/arm/pxa2xx_pic.c
@@ -11,6 +11,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "qemu/module.h"
+#include "qemu/log.h"
#include "cpu.h"
#include "hw/arm/pxa.h"
#include "hw/sysbus.h"
@@ -166,7 +167,9 @@ static uint64_t pxa2xx_pic_mem_read(void *opaque, hwaddr offset,
case ICHP: /* Highest Priority register */
return pxa2xx_pic_highest(s);
default:
- printf("%s: Bad register offset " REG_FMT "\n", __func__, offset);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "pxa2xx_pic_mem_read: bad register offset 0x%" HWADDR_PRIx
+ "\n", offset);
return 0;
}
}
@@ -199,7 +202,9 @@ static void pxa2xx_pic_mem_write(void *opaque, hwaddr offset,
s->priority[32 + ((offset - IPR32) >> 2)] = value & 0x8000003f;
break;
default:
- printf("%s: Bad register offset " REG_FMT "\n", __func__, offset);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "pxa2xx_pic_mem_write: bad register offset 0x%"
+ HWADDR_PRIx "\n", offset);
return;
}
pxa2xx_pic_update(opaque);
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 15/17] hw/arm/pxa2xx_pic: Use LOG_GUEST_ERROR for bad guest register accesses
2020-06-28 14:24 ` [PATCH 15/17] hw/arm/pxa2xx_pic: " Peter Maydell
@ 2020-06-29 9:14 ` Philippe Mathieu-Daudé
2020-07-01 0:52 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-29 9:14 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis
On 6/28/20 4:24 PM, Peter Maydell wrote:
> Instead of using printf() for logging guest accesses to invalid
> register offsets in the pxa2xx PIC device, use the usual
> qemu_log_mask(LOG_GUEST_ERROR,...).
>
> This was the only user of the REG_FMT macro in pxa.h, so we can
> remove that.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/arm/pxa.h | 1 -
> hw/arm/pxa2xx_pic.c | 9 +++++++--
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/arm/pxa.h b/include/hw/arm/pxa.h
> index f6dfb5c0cf0..8843e5f9107 100644
> --- a/include/hw/arm/pxa.h
> +++ b/include/hw/arm/pxa.h
> @@ -184,7 +184,6 @@ struct PXA2xxI2SState {
> };
>
> # define PA_FMT "0x%08lx"
> -# define REG_FMT "0x" TARGET_FMT_plx
>
> PXA2xxState *pxa270_init(MemoryRegion *address_space, unsigned int sdram_size,
> const char *revision);
> diff --git a/hw/arm/pxa2xx_pic.c b/hw/arm/pxa2xx_pic.c
> index 105c5e63f2f..ceee6aa48db 100644
> --- a/hw/arm/pxa2xx_pic.c
> +++ b/hw/arm/pxa2xx_pic.c
> @@ -11,6 +11,7 @@
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "qemu/module.h"
> +#include "qemu/log.h"
> #include "cpu.h"
> #include "hw/arm/pxa.h"
> #include "hw/sysbus.h"
> @@ -166,7 +167,9 @@ static uint64_t pxa2xx_pic_mem_read(void *opaque, hwaddr offset,
> case ICHP: /* Highest Priority register */
> return pxa2xx_pic_highest(s);
> default:
> - printf("%s: Bad register offset " REG_FMT "\n", __func__, offset);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pxa2xx_pic_mem_read: bad register offset 0x%" HWADDR_PRIx
> + "\n", offset);
> return 0;
> }
> }
> @@ -199,7 +202,9 @@ static void pxa2xx_pic_mem_write(void *opaque, hwaddr offset,
> s->priority[32 + ((offset - IPR32) >> 2)] = value & 0x8000003f;
> break;
> default:
> - printf("%s: Bad register offset " REG_FMT "\n", __func__, offset);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pxa2xx_pic_mem_write: bad register offset 0x%"
> + HWADDR_PRIx "\n", offset);
> return;
> }
> pxa2xx_pic_update(opaque);
>
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 15/17] hw/arm/pxa2xx_pic: Use LOG_GUEST_ERROR for bad guest register accesses
2020-06-28 14:24 ` [PATCH 15/17] hw/arm/pxa2xx_pic: " Peter Maydell
2020-06-29 9:14 ` Philippe Mathieu-Daudé
@ 2020-07-01 0:52 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Alistair Francis @ 2020-07-01 0:52 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sun, Jun 28, 2020 at 7:29 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Instead of using printf() for logging guest accesses to invalid
> register offsets in the pxa2xx PIC device, use the usual
> qemu_log_mask(LOG_GUEST_ERROR,...).
>
> This was the only user of the REG_FMT macro in pxa.h, so we can
> remove that.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/hw/arm/pxa.h | 1 -
> hw/arm/pxa2xx_pic.c | 9 +++++++--
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/arm/pxa.h b/include/hw/arm/pxa.h
> index f6dfb5c0cf0..8843e5f9107 100644
> --- a/include/hw/arm/pxa.h
> +++ b/include/hw/arm/pxa.h
> @@ -184,7 +184,6 @@ struct PXA2xxI2SState {
> };
>
> # define PA_FMT "0x%08lx"
> -# define REG_FMT "0x" TARGET_FMT_plx
>
> PXA2xxState *pxa270_init(MemoryRegion *address_space, unsigned int sdram_size,
> const char *revision);
> diff --git a/hw/arm/pxa2xx_pic.c b/hw/arm/pxa2xx_pic.c
> index 105c5e63f2f..ceee6aa48db 100644
> --- a/hw/arm/pxa2xx_pic.c
> +++ b/hw/arm/pxa2xx_pic.c
> @@ -11,6 +11,7 @@
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "qemu/module.h"
> +#include "qemu/log.h"
> #include "cpu.h"
> #include "hw/arm/pxa.h"
> #include "hw/sysbus.h"
> @@ -166,7 +167,9 @@ static uint64_t pxa2xx_pic_mem_read(void *opaque, hwaddr offset,
> case ICHP: /* Highest Priority register */
> return pxa2xx_pic_highest(s);
> default:
> - printf("%s: Bad register offset " REG_FMT "\n", __func__, offset);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pxa2xx_pic_mem_read: bad register offset 0x%" HWADDR_PRIx
> + "\n", offset);
> return 0;
> }
> }
> @@ -199,7 +202,9 @@ static void pxa2xx_pic_mem_write(void *opaque, hwaddr offset,
> s->priority[32 + ((offset - IPR32) >> 2)] = value & 0x8000003f;
> break;
> default:
> - printf("%s: Bad register offset " REG_FMT "\n", __func__, offset);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pxa2xx_pic_mem_write: bad register offset 0x%"
> + HWADDR_PRIx "\n", offset);
> return;
> }
> pxa2xx_pic_update(opaque);
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 16/17] hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (14 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 15/17] hw/arm/pxa2xx_pic: " Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 9:17 ` Philippe Mathieu-Daudé
2020-07-02 17:51 ` Alistair Francis
2020-06-28 14:24 ` [PATCH 17/17] Replace uses of FROM_SSI_SLAVE() macro with QOM casts Peter Maydell
2020-06-28 14:43 ` [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups no-reply
17 siblings, 2 replies; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
The QOM types "spitz-lcdtg" and "corgi-ssp" are missing the
usual QOM TYPE and casting macros; provide and use them.
In particular, we can safely use the QOM cast macros instead of
FROM_SSI_SLAVE() because in both cases the 'ssidev' field of
the instance state struct is the first field in it.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/spitz.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 49eae3fce4e..f020aff9747 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -579,6 +579,9 @@ static void spitz_keyboard_realize(DeviceState *dev, Error **errp)
#define LCDTG_PICTRL 0x06
#define LCDTG_POLCTRL 0x07
+#define TYPE_SPITZ_LCDTG "spitz-lcdtg"
+#define SPITZ_LCDTG(obj) OBJECT_CHECK(SpitzLCDTG, (obj), TYPE_SPITZ_LCDTG)
+
typedef struct {
SSISlave ssidev;
uint32_t bl_intensity;
@@ -616,7 +619,7 @@ static inline void spitz_bl_power(void *opaque, int line, int level)
static uint32_t spitz_lcdtg_transfer(SSISlave *dev, uint32_t value)
{
- SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, dev);
+ SpitzLCDTG *s = SPITZ_LCDTG(dev);
int addr;
addr = value >> 5;
value &= 0x1f;
@@ -645,7 +648,7 @@ static uint32_t spitz_lcdtg_transfer(SSISlave *dev, uint32_t value)
static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp)
{
- SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, ssi);
+ SpitzLCDTG *s = SPITZ_LCDTG(ssi);
DeviceState *dev = DEVICE(s);
s->bl_power = 0;
@@ -664,6 +667,9 @@ static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp)
#define SPITZ_GPIO_MAX1111_CS 20
#define SPITZ_GPIO_TP_INT 11
+#define TYPE_CORGI_SSP "corgi-ssp"
+#define CORGI_SSP(obj) OBJECT_CHECK(CorgiSSPState, (obj), TYPE_CORGI_SSP)
+
/* "Demux" the signal based on current chipselect */
typedef struct {
SSISlave ssidev;
@@ -673,7 +679,7 @@ typedef struct {
static uint32_t corgi_ssp_transfer(SSISlave *dev, uint32_t value)
{
- CorgiSSPState *s = FROM_SSI_SLAVE(CorgiSSPState, dev);
+ CorgiSSPState *s = CORGI_SSP(dev);
int i;
for (i = 0; i < 3; i++) {
@@ -702,7 +708,7 @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level)
static void corgi_ssp_realize(SSISlave *d, Error **errp)
{
DeviceState *dev = DEVICE(d);
- CorgiSSPState *s = FROM_SSI_SLAVE(CorgiSSPState, d);
+ CorgiSSPState *s = CORGI_SSP(d);
qdev_init_gpio_in(dev, corgi_ssp_gpio_cs, 3);
s->bus[0] = ssi_create_bus(dev, "ssi0");
@@ -714,10 +720,11 @@ static void spitz_ssp_attach(SpitzMachineState *sms)
{
void *bus;
- sms->mux = ssi_create_slave(sms->mpu->ssp[CORGI_SSP_PORT - 1], "corgi-ssp");
+ sms->mux = ssi_create_slave(sms->mpu->ssp[CORGI_SSP_PORT - 1],
+ TYPE_CORGI_SSP);
bus = qdev_get_child_bus(sms->mux, "ssi0");
- sms->lcdtg = ssi_create_slave(bus, "spitz-lcdtg");
+ sms->lcdtg = ssi_create_slave(bus, TYPE_SPITZ_LCDTG);
bus = qdev_get_child_bus(sms->mux, "ssi1");
sms->ads7846 = ssi_create_slave(bus, "ads7846");
@@ -1220,7 +1227,7 @@ static void corgi_ssp_class_init(ObjectClass *klass, void *data)
}
static const TypeInfo corgi_ssp_info = {
- .name = "corgi-ssp",
+ .name = TYPE_CORGI_SSP,
.parent = TYPE_SSI_SLAVE,
.instance_size = sizeof(CorgiSSPState),
.class_init = corgi_ssp_class_init,
@@ -1249,7 +1256,7 @@ static void spitz_lcdtg_class_init(ObjectClass *klass, void *data)
}
static const TypeInfo spitz_lcdtg_info = {
- .name = "spitz-lcdtg",
+ .name = TYPE_SPITZ_LCDTG,
.parent = TYPE_SSI_SLAVE,
.instance_size = sizeof(SpitzLCDTG),
.class_init = spitz_lcdtg_class_init,
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 16/17] hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg
2020-06-28 14:24 ` [PATCH 16/17] hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg Peter Maydell
@ 2020-06-29 9:17 ` Philippe Mathieu-Daudé
2020-07-02 17:51 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-29 9:17 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis
On 6/28/20 4:24 PM, Peter Maydell wrote:
> The QOM types "spitz-lcdtg" and "corgi-ssp" are missing the
> usual QOM TYPE and casting macros; provide and use them.
>
> In particular, we can safely use the QOM cast macros instead of
> FROM_SSI_SLAVE() because in both cases the 'ssidev' field of
> the instance state struct is the first field in it.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/arm/spitz.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 16/17] hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg
2020-06-28 14:24 ` [PATCH 16/17] hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg Peter Maydell
2020-06-29 9:17 ` Philippe Mathieu-Daudé
@ 2020-07-02 17:51 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Alistair Francis @ 2020-07-02 17:51 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sun, Jun 28, 2020 at 7:35 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The QOM types "spitz-lcdtg" and "corgi-ssp" are missing the
> usual QOM TYPE and casting macros; provide and use them.
>
> In particular, we can safely use the QOM cast macros instead of
> FROM_SSI_SLAVE() because in both cases the 'ssidev' field of
> the instance state struct is the first field in it.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/arm/spitz.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index 49eae3fce4e..f020aff9747 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -579,6 +579,9 @@ static void spitz_keyboard_realize(DeviceState *dev, Error **errp)
> #define LCDTG_PICTRL 0x06
> #define LCDTG_POLCTRL 0x07
>
> +#define TYPE_SPITZ_LCDTG "spitz-lcdtg"
> +#define SPITZ_LCDTG(obj) OBJECT_CHECK(SpitzLCDTG, (obj), TYPE_SPITZ_LCDTG)
> +
> typedef struct {
> SSISlave ssidev;
> uint32_t bl_intensity;
> @@ -616,7 +619,7 @@ static inline void spitz_bl_power(void *opaque, int line, int level)
>
> static uint32_t spitz_lcdtg_transfer(SSISlave *dev, uint32_t value)
> {
> - SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, dev);
> + SpitzLCDTG *s = SPITZ_LCDTG(dev);
> int addr;
> addr = value >> 5;
> value &= 0x1f;
> @@ -645,7 +648,7 @@ static uint32_t spitz_lcdtg_transfer(SSISlave *dev, uint32_t value)
>
> static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp)
> {
> - SpitzLCDTG *s = FROM_SSI_SLAVE(SpitzLCDTG, ssi);
> + SpitzLCDTG *s = SPITZ_LCDTG(ssi);
> DeviceState *dev = DEVICE(s);
>
> s->bl_power = 0;
> @@ -664,6 +667,9 @@ static void spitz_lcdtg_realize(SSISlave *ssi, Error **errp)
> #define SPITZ_GPIO_MAX1111_CS 20
> #define SPITZ_GPIO_TP_INT 11
>
> +#define TYPE_CORGI_SSP "corgi-ssp"
> +#define CORGI_SSP(obj) OBJECT_CHECK(CorgiSSPState, (obj), TYPE_CORGI_SSP)
> +
> /* "Demux" the signal based on current chipselect */
> typedef struct {
> SSISlave ssidev;
> @@ -673,7 +679,7 @@ typedef struct {
>
> static uint32_t corgi_ssp_transfer(SSISlave *dev, uint32_t value)
> {
> - CorgiSSPState *s = FROM_SSI_SLAVE(CorgiSSPState, dev);
> + CorgiSSPState *s = CORGI_SSP(dev);
> int i;
>
> for (i = 0; i < 3; i++) {
> @@ -702,7 +708,7 @@ static void corgi_ssp_gpio_cs(void *opaque, int line, int level)
> static void corgi_ssp_realize(SSISlave *d, Error **errp)
> {
> DeviceState *dev = DEVICE(d);
> - CorgiSSPState *s = FROM_SSI_SLAVE(CorgiSSPState, d);
> + CorgiSSPState *s = CORGI_SSP(d);
>
> qdev_init_gpio_in(dev, corgi_ssp_gpio_cs, 3);
> s->bus[0] = ssi_create_bus(dev, "ssi0");
> @@ -714,10 +720,11 @@ static void spitz_ssp_attach(SpitzMachineState *sms)
> {
> void *bus;
>
> - sms->mux = ssi_create_slave(sms->mpu->ssp[CORGI_SSP_PORT - 1], "corgi-ssp");
> + sms->mux = ssi_create_slave(sms->mpu->ssp[CORGI_SSP_PORT - 1],
> + TYPE_CORGI_SSP);
>
> bus = qdev_get_child_bus(sms->mux, "ssi0");
> - sms->lcdtg = ssi_create_slave(bus, "spitz-lcdtg");
> + sms->lcdtg = ssi_create_slave(bus, TYPE_SPITZ_LCDTG);
>
> bus = qdev_get_child_bus(sms->mux, "ssi1");
> sms->ads7846 = ssi_create_slave(bus, "ads7846");
> @@ -1220,7 +1227,7 @@ static void corgi_ssp_class_init(ObjectClass *klass, void *data)
> }
>
> static const TypeInfo corgi_ssp_info = {
> - .name = "corgi-ssp",
> + .name = TYPE_CORGI_SSP,
> .parent = TYPE_SSI_SLAVE,
> .instance_size = sizeof(CorgiSSPState),
> .class_init = corgi_ssp_class_init,
> @@ -1249,7 +1256,7 @@ static void spitz_lcdtg_class_init(ObjectClass *klass, void *data)
> }
>
> static const TypeInfo spitz_lcdtg_info = {
> - .name = "spitz-lcdtg",
> + .name = TYPE_SPITZ_LCDTG,
> .parent = TYPE_SSI_SLAVE,
> .instance_size = sizeof(SpitzLCDTG),
> .class_init = spitz_lcdtg_class_init,
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 17/17] Replace uses of FROM_SSI_SLAVE() macro with QOM casts
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (15 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 16/17] hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg Peter Maydell
@ 2020-06-28 14:24 ` Peter Maydell
2020-06-29 9:18 ` Philippe Mathieu-Daudé
2020-07-02 18:23 ` Alistair Francis
2020-06-28 14:43 ` [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups no-reply
17 siblings, 2 replies; 50+ messages in thread
From: Peter Maydell @ 2020-06-28 14:24 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé, Alistair Francis
The FROM_SSI_SLAVE() macro predates QOM and is used as a typesafe way
to cast from an SSISlave* to the instance struct of a subtype of
TYPE_SSI_SLAVE. Switch to using the QOM cast macros instead, which
have the same effect (by writing the QOM macros if the types were
previously missing them.)
(The FROM_SSI_SLAVE() macro allows the SSISlave member of the
subtype's struct to be anywhere as long as it is named "ssidev",
whereas a QOM cast macro insists that it is the first thing in the
subtype's struct. This is true for all the types we convert here.)
This removes all the uses of FROM_SSI_SLAVE() so we can delete the
definition.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
include/hw/ssi/ssi.h | 2 --
hw/arm/z2.c | 11 +++++++----
hw/display/ads7846.c | 9 ++++++---
hw/display/ssd0323.c | 10 +++++++---
hw/sd/ssi-sd.c | 4 ++--
5 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
index 5fd411f2e4e..eac168aa1db 100644
--- a/include/hw/ssi/ssi.h
+++ b/include/hw/ssi/ssi.h
@@ -66,8 +66,6 @@ struct SSISlave {
bool cs;
};
-#define FROM_SSI_SLAVE(type, dev) DO_UPCAST(type, ssidev, dev)
-
extern const VMStateDescription vmstate_ssi_slave;
#define VMSTATE_SSI_SLAVE(_field, _state) { \
diff --git a/hw/arm/z2.c b/hw/arm/z2.c
index a0f40959904..e1f22f58681 100644
--- a/hw/arm/z2.c
+++ b/hw/arm/z2.c
@@ -111,9 +111,12 @@ typedef struct {
int pos;
} ZipitLCD;
+#define TYPE_ZIPIT_LCD "zipit-lcd"
+#define ZIPIT_LCD(obj) OBJECT_CHECK(ZipitLCD, (obj), TYPE_ZIPIT_LCD)
+
static uint32_t zipit_lcd_transfer(SSISlave *dev, uint32_t value)
{
- ZipitLCD *z = FROM_SSI_SLAVE(ZipitLCD, dev);
+ ZipitLCD *z = ZIPIT_LCD(dev);
uint16_t val;
if (z->selected) {
z->buf[z->pos] = value & 0xff;
@@ -153,7 +156,7 @@ static void z2_lcd_cs(void *opaque, int line, int level)
static void zipit_lcd_realize(SSISlave *dev, Error **errp)
{
- ZipitLCD *z = FROM_SSI_SLAVE(ZipitLCD, dev);
+ ZipitLCD *z = ZIPIT_LCD(dev);
z->selected = 0;
z->enabled = 0;
z->pos = 0;
@@ -185,7 +188,7 @@ static void zipit_lcd_class_init(ObjectClass *klass, void *data)
}
static const TypeInfo zipit_lcd_info = {
- .name = "zipit-lcd",
+ .name = TYPE_ZIPIT_LCD,
.parent = TYPE_SSI_SLAVE,
.instance_size = sizeof(ZipitLCD),
.class_init = zipit_lcd_class_init,
@@ -325,7 +328,7 @@ static void z2_init(MachineState *machine)
type_register_static(&zipit_lcd_info);
type_register_static(&aer915_info);
- z2_lcd = ssi_create_slave(mpu->ssp[1], "zipit-lcd");
+ z2_lcd = ssi_create_slave(mpu->ssp[1], TYPE_ZIPIT_LCD);
bus = pxa2xx_i2c_bus(mpu->i2c[0]);
i2c_create_slave(bus, TYPE_AER915, 0x55);
wm = i2c_create_slave(bus, TYPE_WM8750, 0x1b);
diff --git a/hw/display/ads7846.c b/hw/display/ads7846.c
index 9228b40b1af..56bf82fe079 100644
--- a/hw/display/ads7846.c
+++ b/hw/display/ads7846.c
@@ -29,6 +29,9 @@ typedef struct {
int output;
} ADS7846State;
+#define TYPE_ADS7846 "ads7846"
+#define ADS7846(obj) OBJECT_CHECK(ADS7846State, (obj), TYPE_ADS7846)
+
/* Control-byte bitfields */
#define CB_PD0 (1 << 0)
#define CB_PD1 (1 << 1)
@@ -61,7 +64,7 @@ static void ads7846_int_update(ADS7846State *s)
static uint32_t ads7846_transfer(SSISlave *dev, uint32_t value)
{
- ADS7846State *s = FROM_SSI_SLAVE(ADS7846State, dev);
+ ADS7846State *s = ADS7846(dev);
switch (s->cycle ++) {
case 0:
@@ -139,7 +142,7 @@ static const VMStateDescription vmstate_ads7846 = {
static void ads7846_realize(SSISlave *d, Error **errp)
{
DeviceState *dev = DEVICE(d);
- ADS7846State *s = FROM_SSI_SLAVE(ADS7846State, d);
+ ADS7846State *s = ADS7846(d);
qdev_init_gpio_out(dev, &s->interrupt, 1);
@@ -166,7 +169,7 @@ static void ads7846_class_init(ObjectClass *klass, void *data)
}
static const TypeInfo ads7846_info = {
- .name = "ads7846",
+ .name = TYPE_ADS7846,
.parent = TYPE_SSI_SLAVE,
.instance_size = sizeof(ADS7846State),
.class_init = ads7846_class_init,
diff --git a/hw/display/ssd0323.c b/hw/display/ssd0323.c
index c3bdb18742c..32d27f008ae 100644
--- a/hw/display/ssd0323.c
+++ b/hw/display/ssd0323.c
@@ -66,9 +66,13 @@ typedef struct {
uint8_t framebuffer[128 * 80 / 2];
} ssd0323_state;
+#define TYPE_SSD0323 "ssd0323"
+#define SSD0323(obj) OBJECT_CHECK(ssd0323_state, (obj), TYPE_SSD0323)
+
+
static uint32_t ssd0323_transfer(SSISlave *dev, uint32_t data)
{
- ssd0323_state *s = FROM_SSI_SLAVE(ssd0323_state, dev);
+ ssd0323_state *s = SSD0323(dev);
switch (s->mode) {
case SSD0323_DATA:
@@ -346,7 +350,7 @@ static const GraphicHwOps ssd0323_ops = {
static void ssd0323_realize(SSISlave *d, Error **errp)
{
DeviceState *dev = DEVICE(d);
- ssd0323_state *s = FROM_SSI_SLAVE(ssd0323_state, d);
+ ssd0323_state *s = SSD0323(d);
s->col_end = 63;
s->row_end = 79;
@@ -368,7 +372,7 @@ static void ssd0323_class_init(ObjectClass *klass, void *data)
}
static const TypeInfo ssd0323_info = {
- .name = "ssd0323",
+ .name = TYPE_SSD0323,
.parent = TYPE_SSI_SLAVE,
.instance_size = sizeof(ssd0323_state),
.class_init = ssd0323_class_init,
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 25cec2ddeaa..25cdf4c966d 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -74,7 +74,7 @@ typedef struct {
static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val)
{
- ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, dev);
+ ssi_sd_state *s = SSI_SD(dev);
/* Special case: allow CMD12 (STOP TRANSMISSION) while reading data. */
if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
@@ -241,7 +241,7 @@ static const VMStateDescription vmstate_ssi_sd = {
static void ssi_sd_realize(SSISlave *d, Error **errp)
{
- ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, d);
+ ssi_sd_state *s = SSI_SD(d);
DeviceState *carddev;
DriveInfo *dinfo;
Error *err = NULL;
--
2.20.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH 17/17] Replace uses of FROM_SSI_SLAVE() macro with QOM casts
2020-06-28 14:24 ` [PATCH 17/17] Replace uses of FROM_SSI_SLAVE() macro with QOM casts Peter Maydell
@ 2020-06-29 9:18 ` Philippe Mathieu-Daudé
2020-07-02 18:23 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-29 9:18 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis
On 6/28/20 4:24 PM, Peter Maydell wrote:
> The FROM_SSI_SLAVE() macro predates QOM and is used as a typesafe way
> to cast from an SSISlave* to the instance struct of a subtype of
> TYPE_SSI_SLAVE. Switch to using the QOM cast macros instead, which
> have the same effect (by writing the QOM macros if the types were
> previously missing them.)
>
> (The FROM_SSI_SLAVE() macro allows the SSISlave member of the
> subtype's struct to be anywhere as long as it is named "ssidev",
> whereas a QOM cast macro insists that it is the first thing in the
> subtype's struct. This is true for all the types we convert here.)
>
> This removes all the uses of FROM_SSI_SLAVE() so we can delete the
> definition.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> include/hw/ssi/ssi.h | 2 --
> hw/arm/z2.c | 11 +++++++----
> hw/display/ads7846.c | 9 ++++++---
> hw/display/ssd0323.c | 10 +++++++---
> hw/sd/ssi-sd.c | 4 ++--
> 5 files changed, 22 insertions(+), 14 deletions(-)
>
[...]
> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> index 25cec2ddeaa..25cdf4c966d 100644
> --- a/hw/sd/ssi-sd.c
> +++ b/hw/sd/ssi-sd.c
> @@ -74,7 +74,7 @@ typedef struct {
>
> static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val)
> {
> - ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, dev);
> + ssi_sd_state *s = SSI_SD(dev);
>
> /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data. */
> if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
> @@ -241,7 +241,7 @@ static const VMStateDescription vmstate_ssi_sd = {
>
> static void ssi_sd_realize(SSISlave *d, Error **errp)
> {
> - ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, d);
> + ssi_sd_state *s = SSI_SD(d);
> DeviceState *carddev;
> DriveInfo *dinfo;
> Error *err = NULL;
>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 17/17] Replace uses of FROM_SSI_SLAVE() macro with QOM casts
2020-06-28 14:24 ` [PATCH 17/17] Replace uses of FROM_SSI_SLAVE() macro with QOM casts Peter Maydell
2020-06-29 9:18 ` Philippe Mathieu-Daudé
@ 2020-07-02 18:23 ` Alistair Francis
1 sibling, 0 replies; 50+ messages in thread
From: Alistair Francis @ 2020-07-02 18:23 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sun, Jun 28, 2020 at 7:37 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The FROM_SSI_SLAVE() macro predates QOM and is used as a typesafe way
> to cast from an SSISlave* to the instance struct of a subtype of
> TYPE_SSI_SLAVE. Switch to using the QOM cast macros instead, which
> have the same effect (by writing the QOM macros if the types were
> previously missing them.)
>
> (The FROM_SSI_SLAVE() macro allows the SSISlave member of the
> subtype's struct to be anywhere as long as it is named "ssidev",
> whereas a QOM cast macro insists that it is the first thing in the
> subtype's struct. This is true for all the types we convert here.)
>
> This removes all the uses of FROM_SSI_SLAVE() so we can delete the
> definition.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/hw/ssi/ssi.h | 2 --
> hw/arm/z2.c | 11 +++++++----
> hw/display/ads7846.c | 9 ++++++---
> hw/display/ssd0323.c | 10 +++++++---
> hw/sd/ssi-sd.c | 4 ++--
> 5 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
> index 5fd411f2e4e..eac168aa1db 100644
> --- a/include/hw/ssi/ssi.h
> +++ b/include/hw/ssi/ssi.h
> @@ -66,8 +66,6 @@ struct SSISlave {
> bool cs;
> };
>
> -#define FROM_SSI_SLAVE(type, dev) DO_UPCAST(type, ssidev, dev)
> -
> extern const VMStateDescription vmstate_ssi_slave;
>
> #define VMSTATE_SSI_SLAVE(_field, _state) { \
> diff --git a/hw/arm/z2.c b/hw/arm/z2.c
> index a0f40959904..e1f22f58681 100644
> --- a/hw/arm/z2.c
> +++ b/hw/arm/z2.c
> @@ -111,9 +111,12 @@ typedef struct {
> int pos;
> } ZipitLCD;
>
> +#define TYPE_ZIPIT_LCD "zipit-lcd"
> +#define ZIPIT_LCD(obj) OBJECT_CHECK(ZipitLCD, (obj), TYPE_ZIPIT_LCD)
> +
> static uint32_t zipit_lcd_transfer(SSISlave *dev, uint32_t value)
> {
> - ZipitLCD *z = FROM_SSI_SLAVE(ZipitLCD, dev);
> + ZipitLCD *z = ZIPIT_LCD(dev);
> uint16_t val;
> if (z->selected) {
> z->buf[z->pos] = value & 0xff;
> @@ -153,7 +156,7 @@ static void z2_lcd_cs(void *opaque, int line, int level)
>
> static void zipit_lcd_realize(SSISlave *dev, Error **errp)
> {
> - ZipitLCD *z = FROM_SSI_SLAVE(ZipitLCD, dev);
> + ZipitLCD *z = ZIPIT_LCD(dev);
> z->selected = 0;
> z->enabled = 0;
> z->pos = 0;
> @@ -185,7 +188,7 @@ static void zipit_lcd_class_init(ObjectClass *klass, void *data)
> }
>
> static const TypeInfo zipit_lcd_info = {
> - .name = "zipit-lcd",
> + .name = TYPE_ZIPIT_LCD,
> .parent = TYPE_SSI_SLAVE,
> .instance_size = sizeof(ZipitLCD),
> .class_init = zipit_lcd_class_init,
> @@ -325,7 +328,7 @@ static void z2_init(MachineState *machine)
>
> type_register_static(&zipit_lcd_info);
> type_register_static(&aer915_info);
> - z2_lcd = ssi_create_slave(mpu->ssp[1], "zipit-lcd");
> + z2_lcd = ssi_create_slave(mpu->ssp[1], TYPE_ZIPIT_LCD);
> bus = pxa2xx_i2c_bus(mpu->i2c[0]);
> i2c_create_slave(bus, TYPE_AER915, 0x55);
> wm = i2c_create_slave(bus, TYPE_WM8750, 0x1b);
> diff --git a/hw/display/ads7846.c b/hw/display/ads7846.c
> index 9228b40b1af..56bf82fe079 100644
> --- a/hw/display/ads7846.c
> +++ b/hw/display/ads7846.c
> @@ -29,6 +29,9 @@ typedef struct {
> int output;
> } ADS7846State;
>
> +#define TYPE_ADS7846 "ads7846"
> +#define ADS7846(obj) OBJECT_CHECK(ADS7846State, (obj), TYPE_ADS7846)
> +
> /* Control-byte bitfields */
> #define CB_PD0 (1 << 0)
> #define CB_PD1 (1 << 1)
> @@ -61,7 +64,7 @@ static void ads7846_int_update(ADS7846State *s)
>
> static uint32_t ads7846_transfer(SSISlave *dev, uint32_t value)
> {
> - ADS7846State *s = FROM_SSI_SLAVE(ADS7846State, dev);
> + ADS7846State *s = ADS7846(dev);
>
> switch (s->cycle ++) {
> case 0:
> @@ -139,7 +142,7 @@ static const VMStateDescription vmstate_ads7846 = {
> static void ads7846_realize(SSISlave *d, Error **errp)
> {
> DeviceState *dev = DEVICE(d);
> - ADS7846State *s = FROM_SSI_SLAVE(ADS7846State, d);
> + ADS7846State *s = ADS7846(d);
>
> qdev_init_gpio_out(dev, &s->interrupt, 1);
>
> @@ -166,7 +169,7 @@ static void ads7846_class_init(ObjectClass *klass, void *data)
> }
>
> static const TypeInfo ads7846_info = {
> - .name = "ads7846",
> + .name = TYPE_ADS7846,
> .parent = TYPE_SSI_SLAVE,
> .instance_size = sizeof(ADS7846State),
> .class_init = ads7846_class_init,
> diff --git a/hw/display/ssd0323.c b/hw/display/ssd0323.c
> index c3bdb18742c..32d27f008ae 100644
> --- a/hw/display/ssd0323.c
> +++ b/hw/display/ssd0323.c
> @@ -66,9 +66,13 @@ typedef struct {
> uint8_t framebuffer[128 * 80 / 2];
> } ssd0323_state;
>
> +#define TYPE_SSD0323 "ssd0323"
> +#define SSD0323(obj) OBJECT_CHECK(ssd0323_state, (obj), TYPE_SSD0323)
> +
> +
> static uint32_t ssd0323_transfer(SSISlave *dev, uint32_t data)
> {
> - ssd0323_state *s = FROM_SSI_SLAVE(ssd0323_state, dev);
> + ssd0323_state *s = SSD0323(dev);
>
> switch (s->mode) {
> case SSD0323_DATA:
> @@ -346,7 +350,7 @@ static const GraphicHwOps ssd0323_ops = {
> static void ssd0323_realize(SSISlave *d, Error **errp)
> {
> DeviceState *dev = DEVICE(d);
> - ssd0323_state *s = FROM_SSI_SLAVE(ssd0323_state, d);
> + ssd0323_state *s = SSD0323(d);
>
> s->col_end = 63;
> s->row_end = 79;
> @@ -368,7 +372,7 @@ static void ssd0323_class_init(ObjectClass *klass, void *data)
> }
>
> static const TypeInfo ssd0323_info = {
> - .name = "ssd0323",
> + .name = TYPE_SSD0323,
> .parent = TYPE_SSI_SLAVE,
> .instance_size = sizeof(ssd0323_state),
> .class_init = ssd0323_class_init,
> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> index 25cec2ddeaa..25cdf4c966d 100644
> --- a/hw/sd/ssi-sd.c
> +++ b/hw/sd/ssi-sd.c
> @@ -74,7 +74,7 @@ typedef struct {
>
> static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val)
> {
> - ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, dev);
> + ssi_sd_state *s = SSI_SD(dev);
>
> /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data. */
> if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
> @@ -241,7 +241,7 @@ static const VMStateDescription vmstate_ssi_sd = {
>
> static void ssi_sd_realize(SSISlave *d, Error **errp)
> {
> - ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, d);
> + ssi_sd_state *s = SSI_SD(d);
> DeviceState *carddev;
> DriveInfo *dinfo;
> Error *err = NULL;
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups
2020-06-28 14:24 [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups Peter Maydell
` (16 preceding siblings ...)
2020-06-28 14:24 ` [PATCH 17/17] Replace uses of FROM_SSI_SLAVE() macro with QOM casts Peter Maydell
@ 2020-06-28 14:43 ` no-reply
17 siblings, 0 replies; 50+ messages in thread
From: no-reply @ 2020-06-28 14:43 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-arm, philmd, qemu-devel, alistair
Patchew URL: https://patchew.org/QEMU/20200628142429.17111-1-peter.maydell@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH 00/17] spitz: fix hacks, fix CID 1421913, various cleanups
Type: series
Message-id: 20200628142429.17111-1-peter.maydell@linaro.org
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
From https://github.com/patchew-project/qemu
553cf5d..e765115 master -> master
From https://github.com/patchew-project/qemu
* [new tag] patchew/20200628142429.17111-1-peter.maydell@linaro.org -> patchew/20200628142429.17111-1-peter.maydell@linaro.org
Switched to a new branch 'test'
108665c Replace uses of FROM_SSI_SLAVE() macro with QOM casts
6c99757 hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg
98348e5 hw/arm/pxa2xx_pic: Use LOG_GUEST_ERROR for bad guest register accesses
0c5e2a2 hw/arm/spitz: Use LOG_GUEST_ERROR for bad guest register accesses
7856c52 hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses
45b5766 hw/arm/spitz: Encapsulate misc GPIO handling in a device
7c67922 hw/misc/max111x: Create header file for documentation, TYPE_ macros
952c610 hw/misc/max111x: Use GPIO lines rather than max111x_set_input()
8febdff hw/arm/spitz: Use max111x properties to set initial values
9a1ed3c ssi: Add ssi_realize_and_unref()
efa4918 hw/misc/max111x: Don't use vmstate_register()
ccc835d hw/misc/max111x: provide QOM properties for setting initial values
9e5c852 hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals
0ec3ef7 hw/arm/spitz: Keep pointers to scp0, scp1 in SpitzMachineState
7be6379 hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState
8059a4e hw/arm/spitz: Create SpitzMachineClass abstract base class
736d97f hw/arm/spitz: Detabify
=== OUTPUT BEGIN ===
1/17 Checking commit 736d97fd84fb (hw/arm/spitz: Detabify)
ERROR: space prohibited before that '++' (ctx:WxB)
#110: FILE: hw/arm/spitz.c:303:
+#define QUEUE_KEY(c) s->fifo[(s->fifopos + s->fifolen ++) & 0xf] = c
^
ERROR: Macros with complex values should be enclosed in parenthesis
#110: FILE: hw/arm/spitz.c:303:
+#define QUEUE_KEY(c) s->fifo[(s->fifopos + s->fifolen ++) & 0xf] = c
total: 2 errors, 0 warnings, 259 lines checked
Patch 1/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/17 Checking commit 8059a4e54d2b (hw/arm/spitz: Create SpitzMachineClass abstract base class)
3/17 Checking commit 7be6379c8f45 (hw/arm/spitz: Keep pointers to MPU and SSI devices in SpitzMachineState)
4/17 Checking commit 0ec3ef7a8701 (hw/arm/spitz: Keep pointers to scp0, scp1 in SpitzMachineState)
5/17 Checking commit 9e5c852ff9ad (hw/arm/spitz: Implement inbound GPIO lines for bit5 and power signals)
WARNING: line over 80 characters
#94: FILE: hw/arm/spitz.c:859:
+ qdev_get_gpio_in_named(sms->lcdtg, "bl_power", 0));
total: 0 errors, 1 warnings, 68 lines checked
Patch 5/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/17 Checking commit ccc835da8b9e (hw/misc/max111x: provide QOM properties for setting initial values)
7/17 Checking commit efa4918480f8 (hw/misc/max111x: Don't use vmstate_register())
8/17 Checking commit 9a1ed3c4a634 (ssi: Add ssi_realize_and_unref())
9/17 Checking commit 8febdffc3c42 (hw/arm/spitz: Use max111x properties to set initial values)
WARNING: Block comments use a leading /* on a separate line
#29: FILE: hw/arm/spitz.c:736:
+ qdev_prop_set_uint8(sms->max1111, "input1" /* BATT_VOLT */,
WARNING: Block comments use a leading /* on a separate line
#31: FILE: hw/arm/spitz.c:738:
+ qdev_prop_set_uint8(sms->max1111, "input2" /* BATT_TEMP */, 0);
WARNING: Block comments use a leading /* on a separate line
#32: FILE: hw/arm/spitz.c:739:
+ qdev_prop_set_uint8(sms->max1111, "input3" /* ACIN_VOLT */,
total: 0 errors, 3 warnings, 18 lines checked
Patch 9/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/17 Checking commit 952c610248da (hw/misc/max111x: Use GPIO lines rather than max111x_set_input())
11/17 Checking commit 7c6792207add (hw/misc/max111x: Create header file for documentation, TYPE_ macros)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#94:
new file mode 100644
total: 0 errors, 1 warnings, 114 lines checked
Patch 11/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/17 Checking commit 45b57669dce5 (hw/arm/spitz: Encapsulate misc GPIO handling in a device)
WARNING: line over 80 characters
#183: FILE: hw/arm/spitz.c:893:
+ qdev_get_gpio_in(sms->max1111, MAX1111_BATT_TEMP));
total: 0 errors, 1 warnings, 185 lines checked
Patch 12/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/17 Checking commit 7856c52c4f9e (hw/gpio/zaurus.c: Use LOG_GUEST_ERROR for bad guest register accesses)
14/17 Checking commit 0c5e2a25d7ce (hw/arm/spitz: Use LOG_GUEST_ERROR for bad guest register accesses)
15/17 Checking commit 98348e514804 (hw/arm/pxa2xx_pic: Use LOG_GUEST_ERROR for bad guest register accesses)
16/17 Checking commit 6c9975774bb7 (hw/arm/spitz: Provide usual QOM macros for corgi-ssp and spitz-lcdtg)
17/17 Checking commit 108665c37c6d (Replace uses of FROM_SSI_SLAVE() macro with QOM casts)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20200628142429.17111-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 50+ messages in thread