* [PATCH v4 4/7] hw/adc: Make adci[*] R/W in NPCM7XX ADC
2021-11-03 22:01 [PATCH v4 3/7] hw/adc: Fix CONV bit in NPCM7XX ADC CON register Hao Wu
@ 2021-11-03 22:01 ` Hao Wu
2021-11-03 22:01 ` [PATCH v4 5/7] blockdev: Add a new IF type IF_OTHER Hao Wu
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Hao Wu @ 2021-11-03 22:01 UTC (permalink / raw)
To: peter.maydell
Cc: richard.henderson, qemu-arm, qemu-devel, wuhaotsh, venture,
Avi.Fishman, kfting, hskinnemoen, f4bug, bin.meng, qemu-block,
armbru, thuth, Titus Rwantare
Our sensor test requires both reading and writing from a sensor's
QOM property. So we need to make the input of ADC module R/W instead
of write only for that to work.
Signed-off-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Titus Rwantare <titusr@google.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/adc/npcm7xx_adc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/adc/npcm7xx_adc.c b/hw/adc/npcm7xx_adc.c
index 47fb9e5f74..bc6f3f55e6 100644
--- a/hw/adc/npcm7xx_adc.c
+++ b/hw/adc/npcm7xx_adc.c
@@ -242,7 +242,7 @@ static void npcm7xx_adc_init(Object *obj)
for (i = 0; i < NPCM7XX_ADC_NUM_INPUTS; ++i) {
object_property_add_uint32_ptr(obj, "adci[*]",
- &s->adci[i], OBJ_PROP_FLAG_WRITE);
+ &s->adci[i], OBJ_PROP_FLAG_READWRITE);
}
object_property_add_uint32_ptr(obj, "vref",
&s->vref, OBJ_PROP_FLAG_WRITE);
--
2.33.1.1089.g2158813163f-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 5/7] blockdev: Add a new IF type IF_OTHER
2021-11-03 22:01 [PATCH v4 3/7] hw/adc: Fix CONV bit in NPCM7XX ADC CON register Hao Wu
2021-11-03 22:01 ` [PATCH v4 4/7] hw/adc: Make adci[*] R/W in NPCM7XX ADC Hao Wu
@ 2021-11-03 22:01 ` Hao Wu
2021-11-06 6:47 ` Markus Armbruster
2021-11-03 22:01 ` [PATCH v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards Hao Wu
2021-11-03 22:01 ` [PATCH v4 7/7] hw/arm: quanta-gbs-bmc add i2c devices Hao Wu
3 siblings, 1 reply; 9+ messages in thread
From: Hao Wu @ 2021-11-03 22:01 UTC (permalink / raw)
To: peter.maydell
Cc: richard.henderson, qemu-arm, qemu-devel, wuhaotsh, venture,
Avi.Fishman, kfting, hskinnemoen, f4bug, bin.meng, qemu-block,
armbru, thuth
This type is used to represent block devs that are not suitable to
be represented by other existing types.
Signed-of-by: Hao Wu <wuhaotsh@google.com>
---
blockdev.c | 3 ++-
include/sysemu/blockdev.h | 1 +
meson | 2 +-
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index b35072644e..c26cbcc422 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -80,6 +80,7 @@ static const char *const if_name[IF_COUNT] = {
[IF_MTD] = "mtd",
[IF_SD] = "sd",
[IF_VIRTIO] = "virtio",
+ [IF_OTHER] = "other",
[IF_XEN] = "xen",
};
@@ -739,7 +740,7 @@ QemuOptsList qemu_legacy_drive_opts = {
},{
.name = "if",
.type = QEMU_OPT_STRING,
- .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
+ .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio, other)",
},{
.name = "file",
.type = QEMU_OPT_STRING,
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 32c2d6023c..bce6aab573 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -24,6 +24,7 @@ typedef enum {
*/
IF_NONE = 0,
IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
+ IF_OTHER,
IF_COUNT
} BlockInterfaceType;
diff --git a/meson b/meson
index b25d94e7c7..776acd2a80 160000
--- a/meson
+++ b/meson
@@ -1 +1 @@
-Subproject commit b25d94e7c77fda05a7fdfe8afe562cf9760d69da
+Subproject commit 776acd2a805c9b42b4f0375150977df42130317f
--
2.33.1.1089.g2158813163f-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4 5/7] blockdev: Add a new IF type IF_OTHER
2021-11-03 22:01 ` [PATCH v4 5/7] blockdev: Add a new IF type IF_OTHER Hao Wu
@ 2021-11-06 6:47 ` Markus Armbruster
0 siblings, 0 replies; 9+ messages in thread
From: Markus Armbruster @ 2021-11-06 6:47 UTC (permalink / raw)
To: Hao Wu
Cc: peter.maydell, thuth, qemu-block, venture, bin.meng,
richard.henderson, qemu-devel, hskinnemoen, kfting, qemu-arm,
Avi.Fishman, f4bug
Hao Wu <wuhaotsh@google.com> writes:
> This type is used to represent block devs that are not suitable to
> be represented by other existing types.
Hinting at intended use wouldn't hurt: ", such as <mumble>"
>
> Signed-of-by: Hao Wu <wuhaotsh@google.com>
> ---
> blockdev.c | 3 ++-
> include/sysemu/blockdev.h | 1 +
> meson | 2 +-
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index b35072644e..c26cbcc422 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -80,6 +80,7 @@ static const char *const if_name[IF_COUNT] = {
> [IF_MTD] = "mtd",
> [IF_SD] = "sd",
> [IF_VIRTIO] = "virtio",
> + [IF_OTHER] = "other",
> [IF_XEN] = "xen",
> };
>
> @@ -739,7 +740,7 @@ QemuOptsList qemu_legacy_drive_opts = {
> },{
> .name = "if",
> .type = QEMU_OPT_STRING,
> - .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
> + .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio, other)",
> },{
> .name = "file",
> .type = QEMU_OPT_STRING,
> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> index 32c2d6023c..bce6aab573 100644
> --- a/include/sysemu/blockdev.h
> +++ b/include/sysemu/blockdev.h
> @@ -24,6 +24,7 @@ typedef enum {
> */
> IF_NONE = 0,
> IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
> + IF_OTHER,
> IF_COUNT
> } BlockInterfaceType;
>
> diff --git a/meson b/meson
> index b25d94e7c7..776acd2a80 160000
> --- a/meson
> +++ b/meson
> @@ -1 +1 @@
> -Subproject commit b25d94e7c77fda05a7fdfe8afe562cf9760d69da
> +Subproject commit 776acd2a805c9b42b4f0375150977df42130317f
Oopsie :)
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards
2021-11-03 22:01 [PATCH v4 3/7] hw/adc: Fix CONV bit in NPCM7XX ADC CON register Hao Wu
2021-11-03 22:01 ` [PATCH v4 4/7] hw/adc: Make adci[*] R/W in NPCM7XX ADC Hao Wu
2021-11-03 22:01 ` [PATCH v4 5/7] blockdev: Add a new IF type IF_OTHER Hao Wu
@ 2021-11-03 22:01 ` Hao Wu
2021-11-06 6:53 ` Markus Armbruster
2021-11-15 13:19 ` Kevin Wolf
2021-11-03 22:01 ` [PATCH v4 7/7] hw/arm: quanta-gbs-bmc add i2c devices Hao Wu
3 siblings, 2 replies; 9+ messages in thread
From: Hao Wu @ 2021-11-03 22:01 UTC (permalink / raw)
To: peter.maydell
Cc: richard.henderson, qemu-arm, qemu-devel, wuhaotsh, venture,
Avi.Fishman, kfting, hskinnemoen, f4bug, bin.meng, qemu-block,
armbru, thuth
We made 3 changes to the at24c_eeprom_init function in
npcm7xx_boards.c:
1. We allow the function to take a I2CBus* as parameter. This allows
us to attach an EEPROM device behind an I2C mux which is not
possible with the old method.
2. We make at24c EEPROMs are backed by drives so that we can
specify the content of the EEPROMs.
3. Instead of using i2c address as unit number, This patch assigns
unique unit numbers for each eeproms in each board. This avoids
conflict in providing multiple eeprom contents with the same address.
In the old method if we specify two drives with the same unit number,
the following error will occur: `Device with id 'none85' exists`.
Signed-off-by: Hao Wu <wuhaotsh@google.com>
---
hw/arm/npcm7xx_boards.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
index dec7d16ae5..9121e081fa 100644
--- a/hw/arm/npcm7xx_boards.c
+++ b/hw/arm/npcm7xx_boards.c
@@ -126,13 +126,17 @@ static I2CBus *npcm7xx_i2c_get_bus(NPCM7xxState *soc, uint32_t num)
return I2C_BUS(qdev_get_child_bus(DEVICE(&soc->smbus[num]), "i2c-bus"));
}
-static void at24c_eeprom_init(NPCM7xxState *soc, int bus, uint8_t addr,
- uint32_t rsize)
+static void at24c_eeprom_init(I2CBus *i2c_bus, int bus, uint8_t addr,
+ uint32_t rsize, int unit_number)
{
- I2CBus *i2c_bus = npcm7xx_i2c_get_bus(soc, bus);
I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
DeviceState *dev = DEVICE(i2c_dev);
+ DriveInfo *dinfo;
+ dinfo = drive_get(IF_OTHER, bus, unit_number);
+ if (dinfo) {
+ qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
+ }
qdev_prop_set_uint32(dev, "rom-size", rsize);
i2c_slave_realize_and_unref(i2c_dev, i2c_bus, &error_abort);
}
@@ -239,8 +243,8 @@ static void quanta_gsj_i2c_init(NPCM7xxState *soc)
i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 3), "tmp105", 0x5c);
i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 4), "tmp105", 0x5c);
- at24c_eeprom_init(soc, 9, 0x55, 8192);
- at24c_eeprom_init(soc, 10, 0x55, 8192);
+ at24c_eeprom_init(npcm7xx_i2c_get_bus(soc, 9), 9, 0x55, 8192, 0);
+ at24c_eeprom_init(npcm7xx_i2c_get_bus(soc, 10), 10, 0x55, 8192, 1);
/*
* i2c-11:
--
2.33.1.1089.g2158813163f-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards
2021-11-03 22:01 ` [PATCH v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards Hao Wu
@ 2021-11-06 6:53 ` Markus Armbruster
2021-11-15 13:19 ` Kevin Wolf
1 sibling, 0 replies; 9+ messages in thread
From: Markus Armbruster @ 2021-11-06 6:53 UTC (permalink / raw)
To: Hao Wu
Cc: peter.maydell, thuth, qemu-block, venture, bin.meng,
richard.henderson, qemu-devel, hskinnemoen, kfting, qemu-arm,
Avi.Fishman, armbru, f4bug
Hao Wu <wuhaotsh@google.com> writes:
> We made 3 changes to the at24c_eeprom_init function in
> npcm7xx_boards.c:
>
> 1. We allow the function to take a I2CBus* as parameter. This allows
> us to attach an EEPROM device behind an I2C mux which is not
> possible with the old method.
>
> 2. We make at24c EEPROMs are backed by drives so that we can
> specify the content of the EEPROMs.
>
> 3. Instead of using i2c address as unit number, This patch assigns
> unique unit numbers for each eeproms in each board. This avoids
> conflict in providing multiple eeprom contents with the same address.
> In the old method if we specify two drives with the same unit number,
> the following error will occur: `Device with id 'none85' exists`.
When a commit does three things, chances are splitting it into three
commits would be an improvement. This is *not* a demand.
>
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> ---
> hw/arm/npcm7xx_boards.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
> index dec7d16ae5..9121e081fa 100644
> --- a/hw/arm/npcm7xx_boards.c
> +++ b/hw/arm/npcm7xx_boards.c
> @@ -126,13 +126,17 @@ static I2CBus *npcm7xx_i2c_get_bus(NPCM7xxState *soc, uint32_t num)
> return I2C_BUS(qdev_get_child_bus(DEVICE(&soc->smbus[num]), "i2c-bus"));
> }
>
> -static void at24c_eeprom_init(NPCM7xxState *soc, int bus, uint8_t addr,
> - uint32_t rsize)
> +static void at24c_eeprom_init(I2CBus *i2c_bus, int bus, uint8_t addr,
> + uint32_t rsize, int unit_number)
I'd keep bus and unit number together.
I'd name the new parameter @unit, to match drive_get().
> {
> - I2CBus *i2c_bus = npcm7xx_i2c_get_bus(soc, bus);
> I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
> DeviceState *dev = DEVICE(i2c_dev);
> + DriveInfo *dinfo;
>
> + dinfo = drive_get(IF_OTHER, bus, unit_number);
> + if (dinfo) {
> + qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
> + }
> qdev_prop_set_uint32(dev, "rom-size", rsize);
> i2c_slave_realize_and_unref(i2c_dev, i2c_bus, &error_abort);
> }
> @@ -239,8 +243,8 @@ static void quanta_gsj_i2c_init(NPCM7xxState *soc)
> i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 3), "tmp105", 0x5c);
> i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 4), "tmp105", 0x5c);
>
> - at24c_eeprom_init(soc, 9, 0x55, 8192);
> - at24c_eeprom_init(soc, 10, 0x55, 8192);
> + at24c_eeprom_init(npcm7xx_i2c_get_bus(soc, 9), 9, 0x55, 8192, 0);
> + at24c_eeprom_init(npcm7xx_i2c_get_bus(soc, 10), 10, 0x55, 8192, 1);
>
> /*
> * i2c-11:
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards
2021-11-03 22:01 ` [PATCH v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards Hao Wu
2021-11-06 6:53 ` Markus Armbruster
@ 2021-11-15 13:19 ` Kevin Wolf
2021-11-15 15:28 ` On configuring onboard block devices with -blockdev (was: [PATCH v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards) Markus Armbruster
1 sibling, 1 reply; 9+ messages in thread
From: Kevin Wolf @ 2021-11-15 13:19 UTC (permalink / raw)
To: Hao Wu
Cc: peter.maydell, thuth, qemu-block, venture, bin.meng,
richard.henderson, qemu-devel, hskinnemoen, kfting, qemu-arm,
Avi.Fishman, armbru, f4bug
Am 03.11.2021 um 23:01 hat Hao Wu geschrieben:
> We made 3 changes to the at24c_eeprom_init function in
> npcm7xx_boards.c:
>
> 1. We allow the function to take a I2CBus* as parameter. This allows
> us to attach an EEPROM device behind an I2C mux which is not
> possible with the old method.
>
> 2. We make at24c EEPROMs are backed by drives so that we can
> specify the content of the EEPROMs.
>
> 3. Instead of using i2c address as unit number, This patch assigns
> unique unit numbers for each eeproms in each board. This avoids
> conflict in providing multiple eeprom contents with the same address.
> In the old method if we specify two drives with the same unit number,
> the following error will occur: `Device with id 'none85' exists`.
>
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> ---
> hw/arm/npcm7xx_boards.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
> index dec7d16ae5..9121e081fa 100644
> --- a/hw/arm/npcm7xx_boards.c
> +++ b/hw/arm/npcm7xx_boards.c
> @@ -126,13 +126,17 @@ static I2CBus *npcm7xx_i2c_get_bus(NPCM7xxState *soc, uint32_t num)
> return I2C_BUS(qdev_get_child_bus(DEVICE(&soc->smbus[num]), "i2c-bus"));
> }
>
> -static void at24c_eeprom_init(NPCM7xxState *soc, int bus, uint8_t addr,
> - uint32_t rsize)
> +static void at24c_eeprom_init(I2CBus *i2c_bus, int bus, uint8_t addr,
> + uint32_t rsize, int unit_number)
> {
> - I2CBus *i2c_bus = npcm7xx_i2c_get_bus(soc, bus);
> I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
> DeviceState *dev = DEVICE(i2c_dev);
> + DriveInfo *dinfo;
>
> + dinfo = drive_get(IF_OTHER, bus, unit_number);
> + if (dinfo) {
> + qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
> + }
I may be missing the history of this series, but why do we have to use a
legacy DriveInfo here instead of adding a drive property to the board to
make this configurable with -blockdev?
Adding even more devices that can only be configured with -drive feels
like a step backwards to me.
Kevin
^ permalink raw reply [flat|nested] 9+ messages in thread
* On configuring onboard block devices with -blockdev (was: [PATCH v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards)
2021-11-15 13:19 ` Kevin Wolf
@ 2021-11-15 15:28 ` Markus Armbruster
0 siblings, 0 replies; 9+ messages in thread
From: Markus Armbruster @ 2021-11-15 15:28 UTC (permalink / raw)
To: Kevin Wolf
Cc: peter.maydell, thuth, qemu-block, venture, bin.meng,
richard.henderson, qemu-devel, hskinnemoen, Hao Wu, kfting,
qemu-arm, Avi.Fishman, armbru, f4bug
Kevin Wolf <kwolf@redhat.com> writes:
> Am 03.11.2021 um 23:01 hat Hao Wu geschrieben:
>> We made 3 changes to the at24c_eeprom_init function in
>> npcm7xx_boards.c:
>>
>> 1. We allow the function to take a I2CBus* as parameter. This allows
>> us to attach an EEPROM device behind an I2C mux which is not
>> possible with the old method.
>>
>> 2. We make at24c EEPROMs are backed by drives so that we can
>> specify the content of the EEPROMs.
>>
>> 3. Instead of using i2c address as unit number, This patch assigns
>> unique unit numbers for each eeproms in each board. This avoids
>> conflict in providing multiple eeprom contents with the same address.
>> In the old method if we specify two drives with the same unit number,
>> the following error will occur: `Device with id 'none85' exists`.
>>
>> Signed-off-by: Hao Wu <wuhaotsh@google.com>
>> ---
>> hw/arm/npcm7xx_boards.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
>> index dec7d16ae5..9121e081fa 100644
>> --- a/hw/arm/npcm7xx_boards.c
>> +++ b/hw/arm/npcm7xx_boards.c
>> @@ -126,13 +126,17 @@ static I2CBus *npcm7xx_i2c_get_bus(NPCM7xxState *soc, uint32_t num)
>> return I2C_BUS(qdev_get_child_bus(DEVICE(&soc->smbus[num]), "i2c-bus"));
>> }
>>
>> -static void at24c_eeprom_init(NPCM7xxState *soc, int bus, uint8_t addr,
>> - uint32_t rsize)
>> +static void at24c_eeprom_init(I2CBus *i2c_bus, int bus, uint8_t addr,
>> + uint32_t rsize, int unit_number)
>> {
>> - I2CBus *i2c_bus = npcm7xx_i2c_get_bus(soc, bus);
>> I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
>> DeviceState *dev = DEVICE(i2c_dev);
>> + DriveInfo *dinfo;
>>
>> + dinfo = drive_get(IF_OTHER, bus, unit_number);
>> + if (dinfo) {
>> + qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
>> + }
>
> I may be missing the history of this series, but why do we have to use a
> legacy DriveInfo here instead of adding a drive property to the board to
> make this configurable with -blockdev?
>
> Adding even more devices that can only be configured with -drive feels
> like a step backwards to me.
More like sideways.
The big unfinished part of -blockdev is configuring onboard devices with
it.
I took a stab at one instance in commit ebc29e1bea "pc: Support firmware
configuration with -blockdev", 2019-03-11. Quoting from the commit
message:
Mapping -drive if=none,... to -blockdev is a solved problem. With
if=T other than if=none, -drive additionally configures a block device
frontend. For non-onboard devices, that part maps to -device. Also a
solved problem. For onboard devices such as PC flash memory, we have
an unsolved problem.
This is actually an instance of a wider problem: our general device
configuration interface doesn't cover onboard devices. Instead, we have
a zoo of ad hoc interfaces that are much more limited. One of them is
-drive, which we'd rather deprecate, but can't until we have suitable
replacements for all its uses.
Sadly, I can't attack the wider problem today. So back to the narrow
problem.
My first idea was to reduce it to its solved buddy by using pluggable
instead of onboard devices for the flash memory. Workable, but it
requires some extra smarts in firmware descriptors and libvirt. Paolo
had an idea that is simpler for libvirt: keep the devices onboard, and
add machine properties for their block backends.
The implementation is less than straightforward, I'm afraid.
First, block backend properties are *qdev* properties. Machines can't
have those, as they're not devices. I could duplicate these qdev
properties as QOM properties, but I hate that.
More seriously, the properties do not belong to the machine, they
belong to the onboard flash devices. Adding them to the machine would
then require bad magic to somehow transfer them to the flash devices.
Fortunately, QOM provides the means to handle exactly this case: add
alias properties to the machine that forward to the onboard devices'
properties.
Properties need to be created in .instance_init() methods. For PC
machines, that's pc_machine_initfn(). To make alias properties work,
we need to create the onboard flash devices there, too. Requires
several bug fixes, in the previous commits. We also have to realize
the devices. More on that below.
The need to create onboard devices differently results in a non-trivial
refactoring. The need for keeping -drive working complicates things
further.
The "several bug fixes" took me 26 preparatory patches, plus at least
three more to fix regressions caused by one of them.
I then did the same for ARM virt in commit e0561e60f1 "hw/arm/virt:
Support firmware configuration with -blockdev", 2019-05-07. Only a few
preparatory patches, but the patch again includes non-trivial
refactoring.
Two new machines (ARM sbsa-ref and RISC-V virt) have since done it this
way from the start, both in 2019. Definitely easier than the
refactoring I did for PC machines and ARM virt.
More than a hundred drive_get() remain in some 70 files hw/. These
should all be for onboard block devices. I'm not aware of progress
since 2019.
If "replace them all by machine properties" is the way forward, we need
to get going. At the current rate of one file a year (measured
charitably), we'll be done around 2090, provided we don't add more
(we've added quite a few since I did the first replacement).
If it isn't a practical way forward, then what is?
And this is what makes me go "more like sideways". Hao Wu is doing what
the vast majority of the code does. Yes, that's not moving us forward
on this thorny set of problems. But it's hardly setting us back,
either.
Before we can ask people to help us move forward, we need to find the
way forward. I'm not sure we have.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 7/7] hw/arm: quanta-gbs-bmc add i2c devices
2021-11-03 22:01 [PATCH v4 3/7] hw/adc: Fix CONV bit in NPCM7XX ADC CON register Hao Wu
` (2 preceding siblings ...)
2021-11-03 22:01 ` [PATCH v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards Hao Wu
@ 2021-11-03 22:01 ` Hao Wu
3 siblings, 0 replies; 9+ messages in thread
From: Hao Wu @ 2021-11-03 22:01 UTC (permalink / raw)
To: peter.maydell
Cc: richard.henderson, qemu-arm, qemu-devel, wuhaotsh, venture,
Avi.Fishman, kfting, hskinnemoen, f4bug, bin.meng, qemu-block,
armbru, thuth
From: Patrick Venture <venture@google.com>
Adds supported i2c devices to the quanta-gbc-bmc board.
Signed-off-by: Patrick Venture <venture@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
---
hw/arm/npcm7xx_boards.c | 82 ++++++++++++++++++++++++-----------------
1 file changed, 49 insertions(+), 33 deletions(-)
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
index 9121e081fa..7fb1b3dbc2 100644
--- a/hw/arm/npcm7xx_boards.c
+++ b/hw/arm/npcm7xx_boards.c
@@ -276,10 +276,12 @@ static void quanta_gsj_fan_init(NPCM7xxMachine *machine, NPCM7xxState *soc)
static void quanta_gbs_i2c_init(NPCM7xxState *soc)
{
+ I2CSlave *i2c_mux;
+
+ /* i2c-0: */
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 0), TYPE_PCA9546, 0x71);
+
/*
- * i2c-0:
- * pca9546@71
- *
* i2c-1:
* pca9535@24
* pca9535@20
@@ -288,46 +290,60 @@ static void quanta_gbs_i2c_init(NPCM7xxState *soc)
* pca9535@23
* pca9535@25
* pca9535@26
- *
- * i2c-2:
- * sbtsi@4c
- *
- * i2c-5:
- * atmel,24c64@50 mb_fru
- * pca9546@71
- * - channel 0: max31725@54
- * - channel 1: max31725@55
- * - channel 2: max31725@5d
- * atmel,24c64@51 fan_fru
- * - channel 3: atmel,24c64@52 hsbp_fru
- *
+ */
+
+ /* i2c-2: sbtsi@4c */
+
+ /* i2c-5: */
+ /* mb_fru */
+ at24c_eeprom_init(npcm7xx_i2c_get_bus(soc, 5), 5, 0x50, 8192, 0);
+ i2c_mux = i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 5),
+ TYPE_PCA9546, 0x71);
+ /* max31725 is tmp105 compatible. */
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 0), "tmp105", 0x54);
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 1), "tmp105", 0x55);
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 2), "tmp105", 0x5d);
+ /* fan_fru */
+ at24c_eeprom_init(pca954x_i2c_get_bus(i2c_mux, 2), 5, 0x51, 8192, 1);
+ /* hsbp_fru */
+ at24c_eeprom_init(pca954x_i2c_get_bus(i2c_mux, 3), 5, 0x52, 8192, 2);
+
+ /*
* i2c-6:
* pca9545@73
*
* i2c-7:
* pca9545@72
- *
- * i2c-8:
- * adi,adm1272@10
- *
- * i2c-9:
- * pca9546@71
- * - channel 0: isil,isl68137@60
- * - channel 1: isil,isl68137@61
- * - channel 2: isil,isl68137@63
- * - channel 3: isil,isl68137@45
- *
+ */
+
+ /* i2c-8: */
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 8), "adm1272", 0x10);
+
+ /* i2c-9: */
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 9), TYPE_PCA9546, 0x71);
+ /*
+ * - channel 0: isil,isl68137@60
+ * - channel 1: isil,isl68137@61
+ * - channel 2: isil,isl68137@63
+ * - channel 3: isil,isl68137@45
+ */
+
+ /*
* i2c-10:
* pca9545@71
*
* i2c-11:
* pca9545@76
- *
- * i2c-12:
- * maxim,max34451@4e
- * isil,isl68137@5d
- * isil,isl68137@5e
- *
+ */
+
+ /* i2c-12: */
+ i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 12), "max34451", 0x4e);
+ /*
+ * isil,isl68137@5d
+ * isil,isl68137@5e
+ */
+
+ /*
* i2c-14:
* pca9545@70
*/
--
2.33.1.1089.g2158813163f-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread