All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Support marking individual qbus buses as 'full'
@ 2021-09-03 15:14 Peter Maydell
  2021-09-03 15:14 ` [PATCH 1/4] qdev: Support marking individual " Peter Maydell
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Peter Maydell @ 2021-09-03 15:14 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster

By default, QEMU will allow devices to be plugged into a bus up to
the bus class's device count limit.  If the user creates a device on
the command line or via the monitor and doesn't explicitly specify
the bus to plug it in, QEMU will plug it into the first non-full bus
that it finds.

This is fine in most cases, but some machines have multiple buses of
a given type, some of which are dedicated to on-board devices and
some of which have an externally exposed connector for user-pluggable
devices.

In particular, the various MPS2 and MPS3 boards have multiple
I2C buses. Some of these are used purely for on-board devices like
the touchscreen controller or the DDR4 EEPROM, and some are routed
to the "shield" expansion connector. Currently if the user creates
an I2C device (eg with "-device tmp105") it gets plugged into whatever
the search happens to find first, which for some of these boards
is one of the internal-devices-only buses.

The first patch in this series adds a new function qbus_mark_full()
which marks an individual qbus as full, and makes the "find an
available bus" search code honour that. The intention is that the
board code can handle internal-devices-only buses like
 * create the bus controller
 * create all the internal devices, and plug them into that bus
 * call qbus_mark_full() on the bus

Patches 2-4 use this to mark the internal-only i2c buses on
the various mps2/mps3 machines as 'full'. (As it happens, we
don't model any of the internal i2c devices on these boards
yet, so the 'full' buses won't have any devices on them.) 

This is a minor behaviour change for existing command-lines
for these boards, since an i2c device will now get plugged in
in a different place; but none of these boards are versioned
and very few people will be manually creating i2c devices anyway.

thanks
-- PMM

Peter Maydell (4):
  qdev: Support marking individual buses as 'full'
  hw/arm/mps2-tz.c: Add extra data parameter to MakeDevFn
  hw/arm/mps2-tz.c: Mark internal-only I2C buses as 'full'
  hw/arm/mps2.c: Mark internal-only I2C buses as 'full'

 include/hw/qdev-core.h | 24 +++++++++++
 hw/arm/mps2-tz.c       | 92 +++++++++++++++++++++++++++++-------------
 hw/arm/mps2.c          | 12 +++++-
 softmmu/qdev-monitor.c |  7 +++-
 4 files changed, 106 insertions(+), 29 deletions(-)

-- 
2.20.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] qdev: Support marking individual buses as 'full'
  2021-09-03 15:14 [PATCH 0/4] Support marking individual qbus buses as 'full' Peter Maydell
@ 2021-09-03 15:14 ` Peter Maydell
  2021-09-04  8:48   ` Richard Henderson
  2021-09-03 15:14 ` [PATCH 2/4] hw/arm/mps2-tz.c: Add extra data parameter to MakeDevFn Peter Maydell
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2021-09-03 15:14 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster

By default, QEMU will allow devices to be plugged into a bus up to
the bus class's device count limit.  If the user creates a device on
the command line or via the monitor and doesn't explicitly specify
the bus to plug it in, QEMU will plug it into the first non-full bus
that it finds.

This is fine in most cases, but some machines have multiple buses of
a given type, some of which are dedicated to on-board devices and
some of which have an externally exposed connector for user-pluggable
devices. One example is I2C buses.

Provide a new function qbus_mark_full() so that a machine model can
mark this kind of "internal only" bus as 'full' after it has created
all the devices that should be plugged into that bus. The "find a
non-full bus" algorithm will then skip the internal-only bus when
looking for a place to plug in user-created devices.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/qdev-core.h | 24 ++++++++++++++++++++++++
 softmmu/qdev-monitor.c |  7 ++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index bafc311bfa1..762f9584dde 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -264,6 +264,7 @@ struct BusState {
     HotplugHandler *hotplug_handler;
     int max_index;
     bool realized;
+    bool full;
     int num_children;
 
     /*
@@ -798,6 +799,29 @@ static inline bool qbus_is_hotpluggable(BusState *bus)
    return bus->hotplug_handler;
 }
 
+/**
+ * qbus_mark_full: Mark this bus as full, so no more devices can be attached
+ * @bus: Bus to mark as full
+ *
+ * By default, QEMU will allow devices to be plugged into a bus up
+ * to the bus class's device count limit. Calling this function
+ * marks a particular bus as full, so that no more devices can be
+ * plugged into it. In particular this means that the bus will not
+ * be considered as a candidate for plugging in devices created by
+ * the user on the commandline or via the monitor.
+ * If a machine has multiple buses of a given type, such as I2C,
+ * where some of those buses in the real hardware are used only for
+ * internal devices and some are exposed via expansion ports, you
+ * can use this function to mark the internal-only buses as full
+ * after you have created all their internal devices. Then user
+ * created devices will appear on the expansion-port bus where
+ * guest software expects them.
+ */
+static inline void qbus_mark_full(BusState *bus)
+{
+    bus->full = true;
+}
+
 void device_listener_register(DeviceListener *listener);
 void device_listener_unregister(DeviceListener *listener);
 
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index a304754ab91..0705f008466 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -435,7 +435,12 @@ static DeviceState *qbus_find_dev(BusState *bus, char *elem)
 
 static inline bool qbus_is_full(BusState *bus)
 {
-    BusClass *bus_class = BUS_GET_CLASS(bus);
+    BusClass *bus_class;
+
+    if (bus->full) {
+        return true;
+    }
+    bus_class = BUS_GET_CLASS(bus);
     return bus_class->max_dev && bus->num_children >= bus_class->max_dev;
 }
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] hw/arm/mps2-tz.c: Add extra data parameter to MakeDevFn
  2021-09-03 15:14 [PATCH 0/4] Support marking individual qbus buses as 'full' Peter Maydell
  2021-09-03 15:14 ` [PATCH 1/4] qdev: Support marking individual " Peter Maydell
@ 2021-09-03 15:14 ` Peter Maydell
  2021-09-04  8:52   ` Richard Henderson
  2021-09-03 15:14 ` [PATCH 3/4] hw/arm/mps2-tz.c: Mark internal-only I2C buses as 'full' Peter Maydell
  2021-09-03 15:14 ` [PATCH 4/4] hw/arm/mps2.c: " Peter Maydell
  3 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2021-09-03 15:14 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster

The mps2-tz boards use a data-driven structure to create the devices
that sit behind peripheral protection controllers.  Currently the
functions which create these devices are passed an 'opaque' pointer
which is always the address within the machine struct of the device
to create, and some "all devices need this" information like irqs and
addresses.

If a specific device needs more information than this, it is
currently not possible to pass that through from the PPCInfo
data structure. Add support for passing an extra data parameter,
so that we can more flexibly handle the needs of specific
device types. To provide some type-safety we make this extra
parameter a pointer to a union (which initially has no members).

In particular, we would like to be able to indicate which of the
i2c controllers are for on-board devices only and which are
connected to the external 'shield' expansion port; a subsequent
patch will use this mechanism for that purpose.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/mps2-tz.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index e23830f4b7d..746ba3cc59e 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -373,6 +373,10 @@ static qemu_irq get_sse_irq_in(MPS2TZMachineState *mms, int irqno)
     }
 }
 
+/* Union describing the device-specific extra data we pass to the devfn. */
+typedef union PPCExtraData {
+} PPCExtraData;
+
 /* Most of the devices in the AN505 FPGA image sit behind
  * Peripheral Protection Controllers. These data structures
  * define the layout of which devices sit behind which PPCs.
@@ -382,7 +386,8 @@ static qemu_irq get_sse_irq_in(MPS2TZMachineState *mms, int irqno)
  */
 typedef MemoryRegion *MakeDevFn(MPS2TZMachineState *mms, void *opaque,
                                 const char *name, hwaddr size,
-                                const int *irqs);
+                                const int *irqs,
+                                const PPCExtraData *extradata);
 
 typedef struct PPCPortInfo {
     const char *name;
@@ -391,6 +396,7 @@ typedef struct PPCPortInfo {
     hwaddr addr;
     hwaddr size;
     int irqs[3]; /* currently no device needs more IRQ lines than this */
+    PPCExtraData extradata; /* to pass device-specific info to the devfn */
 } PPCPortInfo;
 
 typedef struct PPCInfo {
@@ -401,7 +407,8 @@ typedef struct PPCInfo {
 static MemoryRegion *make_unimp_dev(MPS2TZMachineState *mms,
                                     void *opaque,
                                     const char *name, hwaddr size,
-                                    const int *irqs)
+                                    const int *irqs,
+                                    const PPCExtraData *extradata)
 {
     /* Initialize, configure and realize a TYPE_UNIMPLEMENTED_DEVICE,
      * and return a pointer to its MemoryRegion.
@@ -417,7 +424,7 @@ static MemoryRegion *make_unimp_dev(MPS2TZMachineState *mms,
 
 static MemoryRegion *make_uart(MPS2TZMachineState *mms, void *opaque,
                                const char *name, hwaddr size,
-                               const int *irqs)
+                               const int *irqs, const PPCExtraData *extradata)
 {
     /* The irq[] array is tx, rx, combined, in that order */
     MPS2TZMachineClass *mmc = MPS2TZ_MACHINE_GET_CLASS(mms);
@@ -441,7 +448,7 @@ static MemoryRegion *make_uart(MPS2TZMachineState *mms, void *opaque,
 
 static MemoryRegion *make_scc(MPS2TZMachineState *mms, void *opaque,
                               const char *name, hwaddr size,
-                              const int *irqs)
+                              const int *irqs, const PPCExtraData *extradata)
 {
     MPS2SCC *scc = opaque;
     DeviceState *sccdev;
@@ -465,7 +472,7 @@ static MemoryRegion *make_scc(MPS2TZMachineState *mms, void *opaque,
 
 static MemoryRegion *make_fpgaio(MPS2TZMachineState *mms, void *opaque,
                                  const char *name, hwaddr size,
-                                 const int *irqs)
+                                 const int *irqs, const PPCExtraData *extradata)
 {
     MPS2FPGAIO *fpgaio = opaque;
     MPS2TZMachineClass *mmc = MPS2TZ_MACHINE_GET_CLASS(mms);
@@ -480,7 +487,8 @@ static MemoryRegion *make_fpgaio(MPS2TZMachineState *mms, void *opaque,
 
 static MemoryRegion *make_eth_dev(MPS2TZMachineState *mms, void *opaque,
                                   const char *name, hwaddr size,
-                                  const int *irqs)
+                                  const int *irqs,
+                                  const PPCExtraData *extradata)
 {
     SysBusDevice *s;
     NICInfo *nd = &nd_table[0];
@@ -500,7 +508,8 @@ static MemoryRegion *make_eth_dev(MPS2TZMachineState *mms, void *opaque,
 
 static MemoryRegion *make_eth_usb(MPS2TZMachineState *mms, void *opaque,
                                   const char *name, hwaddr size,
-                                  const int *irqs)
+                                  const int *irqs,
+                                  const PPCExtraData *extradata)
 {
     /*
      * The AN524 makes the ethernet and USB share a PPC port.
@@ -543,7 +552,7 @@ static MemoryRegion *make_eth_usb(MPS2TZMachineState *mms, void *opaque,
 
 static MemoryRegion *make_mpc(MPS2TZMachineState *mms, void *opaque,
                               const char *name, hwaddr size,
-                              const int *irqs)
+                              const int *irqs, const PPCExtraData *extradata)
 {
     TZMPC *mpc = opaque;
     int i = mpc - &mms->mpc[0];
@@ -615,7 +624,7 @@ static void remap_irq_fn(void *opaque, int n, int level)
 
 static MemoryRegion *make_dma(MPS2TZMachineState *mms, void *opaque,
                               const char *name, hwaddr size,
-                              const int *irqs)
+                              const int *irqs, const PPCExtraData *extradata)
 {
     /* The irq[] array is DMACINTR, DMACINTERR, DMACINTTC, in that order */
     PL080State *dma = opaque;
@@ -672,7 +681,7 @@ static MemoryRegion *make_dma(MPS2TZMachineState *mms, void *opaque,
 
 static MemoryRegion *make_spi(MPS2TZMachineState *mms, void *opaque,
                               const char *name, hwaddr size,
-                              const int *irqs)
+                              const int *irqs, const PPCExtraData *extradata)
 {
     /*
      * The AN505 has five PL022 SPI controllers.
@@ -694,7 +703,7 @@ static MemoryRegion *make_spi(MPS2TZMachineState *mms, void *opaque,
 
 static MemoryRegion *make_i2c(MPS2TZMachineState *mms, void *opaque,
                               const char *name, hwaddr size,
-                              const int *irqs)
+                              const int *irqs, const PPCExtraData *extradata)
 {
     ArmSbconI2CState *i2c = opaque;
     SysBusDevice *s;
@@ -707,7 +716,7 @@ static MemoryRegion *make_i2c(MPS2TZMachineState *mms, void *opaque,
 
 static MemoryRegion *make_rtc(MPS2TZMachineState *mms, void *opaque,
                               const char *name, hwaddr size,
-                              const int *irqs)
+                              const int *irqs, const PPCExtraData *extradata)
 {
     PL031State *pl031 = opaque;
     SysBusDevice *s;
@@ -1084,7 +1093,7 @@ static void mps2tz_common_init(MachineState *machine)
             }
 
             mr = pinfo->devfn(mms, pinfo->opaque, pinfo->name, pinfo->size,
-                              pinfo->irqs);
+                              pinfo->irqs, &pinfo->extradata);
             portname = g_strdup_printf("port[%d]", port);
             object_property_set_link(OBJECT(ppc), portname, OBJECT(mr),
                                      &error_fatal);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] hw/arm/mps2-tz.c: Mark internal-only I2C buses as 'full'
  2021-09-03 15:14 [PATCH 0/4] Support marking individual qbus buses as 'full' Peter Maydell
  2021-09-03 15:14 ` [PATCH 1/4] qdev: Support marking individual " Peter Maydell
  2021-09-03 15:14 ` [PATCH 2/4] hw/arm/mps2-tz.c: Add extra data parameter to MakeDevFn Peter Maydell
@ 2021-09-03 15:14 ` Peter Maydell
  2021-09-04  8:53   ` Richard Henderson
  2021-09-03 15:14 ` [PATCH 4/4] hw/arm/mps2.c: " Peter Maydell
  3 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2021-09-03 15:14 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster

The various MPS2 boards have multiple I2C buses: typically a bus
dedicated to the audio configuration, one for the LCD touchscreen
controller, one for a DDR4 EEPROM, and two which are connected to the
external Shield expansion connector.  Mark the buses which are used
only for board-internal devices as 'full' so that if the user creates
i2c devices on the commandline without specifying a bus name then
they will be connected to the I2C controller used for the Shield
connector, where guest software will expect them.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/mps2-tz.c | 57 ++++++++++++++++++++++++++++++++++++------------
 1 file changed, 43 insertions(+), 14 deletions(-)

diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index 746ba3cc59e..f40e854dec7 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -375,6 +375,7 @@ static qemu_irq get_sse_irq_in(MPS2TZMachineState *mms, int irqno)
 
 /* Union describing the device-specific extra data we pass to the devfn. */
 typedef union PPCExtraData {
+    bool i2c_internal;
 } PPCExtraData;
 
 /* Most of the devices in the AN505 FPGA image sit behind
@@ -711,6 +712,20 @@ static MemoryRegion *make_i2c(MPS2TZMachineState *mms, void *opaque,
     object_initialize_child(OBJECT(mms), name, i2c, TYPE_ARM_SBCON_I2C);
     s = SYS_BUS_DEVICE(i2c);
     sysbus_realize(s, &error_fatal);
+
+    /*
+     * If this is an internal-use-only i2c bus, mark it full
+     * so that user-created i2c devices are not plugged into it.
+     * If we implement models of any on-board i2c devices that
+     * plug in to one of the internal-use-only buses, then we will
+     * need to create and plugging those in here before we mark the
+     * bus as full.
+     */
+    if (extradata->i2c_internal) {
+        BusState *qbus = qdev_get_child_bus(DEVICE(i2c), "i2c");
+        qbus_mark_full(qbus);
+    }
+
     return sysbus_mmio_get_region(s, 0);
 }
 
@@ -921,10 +936,14 @@ static void mps2tz_common_init(MachineState *machine)
                 { "uart2", make_uart, &mms->uart[2], 0x40202000, 0x1000, { 36, 37, 44 } },
                 { "uart3", make_uart, &mms->uart[3], 0x40203000, 0x1000, { 38, 39, 45 } },
                 { "uart4", make_uart, &mms->uart[4], 0x40204000, 0x1000, { 40, 41, 46 } },
-                { "i2c0", make_i2c, &mms->i2c[0], 0x40207000, 0x1000 },
-                { "i2c1", make_i2c, &mms->i2c[1], 0x40208000, 0x1000 },
-                { "i2c2", make_i2c, &mms->i2c[2], 0x4020c000, 0x1000 },
-                { "i2c3", make_i2c, &mms->i2c[3], 0x4020d000, 0x1000 },
+                { "i2c0", make_i2c, &mms->i2c[0], 0x40207000, 0x1000, {},
+                  { .i2c_internal = true /* touchscreen */ } },
+                { "i2c1", make_i2c, &mms->i2c[1], 0x40208000, 0x1000, {},
+                  { .i2c_internal = true /* audio conf */ } },
+                { "i2c2", make_i2c, &mms->i2c[2], 0x4020c000, 0x1000, {},
+                  { .i2c_internal = false /* shield 0 */ } },
+                { "i2c3", make_i2c, &mms->i2c[3], 0x4020d000, 0x1000, {},
+                  { .i2c_internal = false /* shield 1 */ } },
             },
         }, {
             .name = "apb_ppcexp2",
@@ -965,15 +984,20 @@ static void mps2tz_common_init(MachineState *machine)
         }, {
             .name = "apb_ppcexp1",
             .ports = {
-                { "i2c0", make_i2c, &mms->i2c[0], 0x41200000, 0x1000 },
-                { "i2c1", make_i2c, &mms->i2c[1], 0x41201000, 0x1000 },
+                { "i2c0", make_i2c, &mms->i2c[0], 0x41200000, 0x1000, {},
+                  { .i2c_internal = true /* touchscreen */ } },
+                { "i2c1", make_i2c, &mms->i2c[1], 0x41201000, 0x1000, {},
+                  { .i2c_internal = true /* audio conf */ } },
                 { "spi0", make_spi, &mms->spi[0], 0x41202000, 0x1000, { 52 } },
                 { "spi1", make_spi, &mms->spi[1], 0x41203000, 0x1000, { 53 } },
                 { "spi2", make_spi, &mms->spi[2], 0x41204000, 0x1000, { 54 } },
-                { "i2c2", make_i2c, &mms->i2c[2], 0x41205000, 0x1000 },
-                { "i2c3", make_i2c, &mms->i2c[3], 0x41206000, 0x1000 },
+                { "i2c2", make_i2c, &mms->i2c[2], 0x41205000, 0x1000, {},
+                  { .i2c_internal = false /* shield 0 */ } },
+                { "i2c3", make_i2c, &mms->i2c[3], 0x41206000, 0x1000, {},
+                  { .i2c_internal = false /* shield 1 */ } },
                 { /* port 7 reserved */ },
-                { "i2c4", make_i2c, &mms->i2c[4], 0x41208000, 0x1000 },
+                { "i2c4", make_i2c, &mms->i2c[4], 0x41208000, 0x1000, {},
+                  { .i2c_internal = true /* DDR4 EEPROM */ } },
             },
         }, {
             .name = "apb_ppcexp2",
@@ -1015,15 +1039,20 @@ static void mps2tz_common_init(MachineState *machine)
         }, {
             .name = "apb_ppcexp1",
             .ports = {
-                { "i2c0", make_i2c, &mms->i2c[0], 0x49200000, 0x1000 },
-                { "i2c1", make_i2c, &mms->i2c[1], 0x49201000, 0x1000 },
+                { "i2c0", make_i2c, &mms->i2c[0], 0x49200000, 0x1000, {},
+                  { .i2c_internal = true /* touchscreen */ } },
+                { "i2c1", make_i2c, &mms->i2c[1], 0x49201000, 0x1000, {},
+                  { .i2c_internal = true /* audio conf */ } },
                 { "spi0", make_spi, &mms->spi[0], 0x49202000, 0x1000, { 53 } },
                 { "spi1", make_spi, &mms->spi[1], 0x49203000, 0x1000, { 54 } },
                 { "spi2", make_spi, &mms->spi[2], 0x49204000, 0x1000, { 55 } },
-                { "i2c2", make_i2c, &mms->i2c[2], 0x49205000, 0x1000 },
-                { "i2c3", make_i2c, &mms->i2c[3], 0x49206000, 0x1000 },
+                { "i2c2", make_i2c, &mms->i2c[2], 0x49205000, 0x1000, {},
+                  { .i2c_internal = false /* shield 0 */ } },
+                { "i2c3", make_i2c, &mms->i2c[3], 0x49206000, 0x1000, {},
+                  { .i2c_internal = false /* shield 1 */ } },
                 { /* port 7 reserved */ },
-                { "i2c4", make_i2c, &mms->i2c[4], 0x49208000, 0x1000 },
+                { "i2c4", make_i2c, &mms->i2c[4], 0x49208000, 0x1000, {},
+                  { .i2c_internal = true /* DDR4 EEPROM */ } },
             },
         }, {
             .name = "apb_ppcexp2",
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] hw/arm/mps2.c: Mark internal-only I2C buses as 'full'
  2021-09-03 15:14 [PATCH 0/4] Support marking individual qbus buses as 'full' Peter Maydell
                   ` (2 preceding siblings ...)
  2021-09-03 15:14 ` [PATCH 3/4] hw/arm/mps2-tz.c: Mark internal-only I2C buses as 'full' Peter Maydell
@ 2021-09-03 15:14 ` Peter Maydell
  2021-09-04  8:54   ` Richard Henderson
  3 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2021-09-03 15:14 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster

The various MPS2 boards implemented in mps2.c have multiple I2C
buses: a bus dedicated to the audio configuration, one for the LCD
touchscreen controller, and two which are connected to the external
Shield expansion connector.  Mark the buses which are used only for
board-internal devices as 'full' so that if the user creates i2c
devices on the commandline without specifying a bus name then they
will be connected to the I2C controller used for the Shield
connector, where guest software will expect them.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/mps2.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index 4634aa1a1ca..bb76fa68890 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -428,7 +428,17 @@ static void mps2_common_init(MachineState *machine)
                                          0x40023000,    /* Audio */
                                          0x40029000,    /* Shield0 */
                                          0x4002a000};   /* Shield1 */
-        sysbus_create_simple(TYPE_ARM_SBCON_I2C, i2cbase[i], NULL);
+        DeviceState *dev;
+
+        dev = sysbus_create_simple(TYPE_ARM_SBCON_I2C, i2cbase[i], NULL);
+        if (i < 2) {
+            /*
+             * internal-only bus: mark it full to avoid user-created
+             * i2c devices being plugged into it.
+             */
+            BusState *qbus = qdev_get_child_bus(dev, "i2c");
+            qbus_mark_full(qbus);
+        }
     }
     create_unimplemented_device("i2s", 0x40024000, 0x400);
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] qdev: Support marking individual buses as 'full'
  2021-09-03 15:14 ` [PATCH 1/4] qdev: Support marking individual " Peter Maydell
@ 2021-09-04  8:48   ` Richard Henderson
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2021-09-04  8:48 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster

On 9/3/21 5:14 PM, Peter Maydell wrote:
> By default, QEMU will allow devices to be plugged into a bus up to
> the bus class's device count limit.  If the user creates a device on
> the command line or via the monitor and doesn't explicitly specify
> the bus to plug it in, QEMU will plug it into the first non-full bus
> that it finds.
> 
> This is fine in most cases, but some machines have multiple buses of
> a given type, some of which are dedicated to on-board devices and
> some of which have an externally exposed connector for user-pluggable
> devices. One example is I2C buses.
> 
> Provide a new function qbus_mark_full() so that a machine model can
> mark this kind of "internal only" bus as 'full' after it has created
> all the devices that should be plugged into that bus. The "find a
> non-full bus" algorithm will then skip the internal-only bus when
> looking for a place to plug in user-created devices.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/qdev-core.h | 24 ++++++++++++++++++++++++
>   softmmu/qdev-monitor.c |  7 ++++++-
>   2 files changed, 30 insertions(+), 1 deletion(-)

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

r~


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] hw/arm/mps2-tz.c: Add extra data parameter to MakeDevFn
  2021-09-03 15:14 ` [PATCH 2/4] hw/arm/mps2-tz.c: Add extra data parameter to MakeDevFn Peter Maydell
@ 2021-09-04  8:52   ` Richard Henderson
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2021-09-04  8:52 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster

On 9/3/21 5:14 PM, Peter Maydell wrote:
> The mps2-tz boards use a data-driven structure to create the devices
> that sit behind peripheral protection controllers.  Currently the
> functions which create these devices are passed an 'opaque' pointer
> which is always the address within the machine struct of the device
> to create, and some "all devices need this" information like irqs and
> addresses.
> 
> If a specific device needs more information than this, it is
> currently not possible to pass that through from the PPCInfo
> data structure. Add support for passing an extra data parameter,
> so that we can more flexibly handle the needs of specific
> device types. To provide some type-safety we make this extra
> parameter a pointer to a union (which initially has no members).
> 
> In particular, we would like to be able to indicate which of the
> i2c controllers are for on-board devices only and which are
> connected to the external 'shield' expansion port; a subsequent
> patch will use this mechanism for that purpose.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   hw/arm/mps2-tz.c | 35 ++++++++++++++++++++++-------------
>   1 file changed, 22 insertions(+), 13 deletions(-)

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

r~


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] hw/arm/mps2-tz.c: Mark internal-only I2C buses as 'full'
  2021-09-03 15:14 ` [PATCH 3/4] hw/arm/mps2-tz.c: Mark internal-only I2C buses as 'full' Peter Maydell
@ 2021-09-04  8:53   ` Richard Henderson
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2021-09-04  8:53 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster

On 9/3/21 5:14 PM, Peter Maydell wrote:
> The various MPS2 boards have multiple I2C buses: typically a bus
> dedicated to the audio configuration, one for the LCD touchscreen
> controller, one for a DDR4 EEPROM, and two which are connected to the
> external Shield expansion connector.  Mark the buses which are used
> only for board-internal devices as 'full' so that if the user creates
> i2c devices on the commandline without specifying a bus name then
> they will be connected to the I2C controller used for the Shield
> connector, where guest software will expect them.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   hw/arm/mps2-tz.c | 57 ++++++++++++++++++++++++++++++++++++------------
>   1 file changed, 43 insertions(+), 14 deletions(-)

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

r~


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] hw/arm/mps2.c: Mark internal-only I2C buses as 'full'
  2021-09-03 15:14 ` [PATCH 4/4] hw/arm/mps2.c: " Peter Maydell
@ 2021-09-04  8:54   ` Richard Henderson
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2021-09-04  8:54 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster

On 9/3/21 5:14 PM, Peter Maydell wrote:
> The various MPS2 boards implemented in mps2.c have multiple I2C
> buses: a bus dedicated to the audio configuration, one for the LCD
> touchscreen controller, and two which are connected to the external
> Shield expansion connector.  Mark the buses which are used only for
> board-internal devices as 'full' so that if the user creates i2c
> devices on the commandline without specifying a bus name then they
> will be connected to the I2C controller used for the Shield
> connector, where guest software will expect them.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


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

r~


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-09-04  8:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-03 15:14 [PATCH 0/4] Support marking individual qbus buses as 'full' Peter Maydell
2021-09-03 15:14 ` [PATCH 1/4] qdev: Support marking individual " Peter Maydell
2021-09-04  8:48   ` Richard Henderson
2021-09-03 15:14 ` [PATCH 2/4] hw/arm/mps2-tz.c: Add extra data parameter to MakeDevFn Peter Maydell
2021-09-04  8:52   ` Richard Henderson
2021-09-03 15:14 ` [PATCH 3/4] hw/arm/mps2-tz.c: Mark internal-only I2C buses as 'full' Peter Maydell
2021-09-04  8:53   ` Richard Henderson
2021-09-03 15:14 ` [PATCH 4/4] hw/arm/mps2.c: " Peter Maydell
2021-09-04  8:54   ` Richard Henderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.