Skip to content

Commit

Permalink
ssi: ssi_auto_connect_slaves() never does anything, drop
Browse files Browse the repository at this point in the history
ssi_auto_connect_slaves(parent, cs_line, bus) iterates over @parent's
QOM children @dev of type TYPE_SSI_SLAVE.  It puts these on @bus, and
sets cs_line[] to qdev_get_gpio_in_named(dev, SSI_GPIO_CS, 0).

Suspicious: there is no protection against overrunning cs_line[].

Turns out it's safe because ssi_auto_connect_slaves() never finds any
such children.  Its called by realize methods of some (but not all)
devices providing an SSI bus, and gets passed the device.

SSI slave devices are always created with ssi_create_slave_no_init(),
optionally via ssi_create_slave().  This adds them to their SSI bus.
It doesn't set their QOM parent.

ssi_create_slave_no_init() is always immediately followed by
qdev_init_nofail(), with no QOM parent assigned, so
device_set_realized() puts the device into the /machine/unattached/
orphanage.  None become QOM children of a device providing an SSI bus.

ssi_auto_connect_slaves() was added in commit b4ae3cf "ssi: Add
slave autoconnect helper".  I can't see which slaves it was supposed
to connect back then.

Cc: Alistair Francis <alistair@alistair23.me>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20200610053247.1583243-23-armbru@redhat.com>
  • Loading branch information
Markus Armbruster committed Jun 15, 2020
1 parent bd2f053 commit 7f16c76
Show file tree
Hide file tree
Showing 7 changed files with 0 additions and 46 deletions.
1 change: 0 additions & 1 deletion hw/ssi/aspeed_smc.c
Expand Up @@ -1356,7 +1356,6 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)

/* Setup cs_lines for slaves */
s->cs_lines = g_new0(qemu_irq, s->num_cs);
ssi_auto_connect_slaves(dev, s->cs_lines, s->spi);

for (i = 0; i < s->num_cs; ++i) {
sysbus_init_irq(sbd, &s->cs_lines[i]);
Expand Down
2 changes: 0 additions & 2 deletions hw/ssi/imx_spi.c
Expand Up @@ -424,8 +424,6 @@ static void imx_spi_realize(DeviceState *dev, Error **errp)
sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);

ssi_auto_connect_slaves(dev, s->cs_lines, s->bus);

for (i = 0; i < 4; ++i) {
sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->cs_lines[i]);
}
Expand Down
1 change: 0 additions & 1 deletion hw/ssi/mss-spi.c
Expand Up @@ -376,7 +376,6 @@ static void mss_spi_realize(DeviceState *dev, Error **errp)
s->spi = ssi_create_bus(dev, "spi");

sysbus_init_irq(sbd, &s->irq);
ssi_auto_connect_slaves(dev, &s->cs_line, s->spi);
sysbus_init_irq(sbd, &s->cs_line);

memory_region_init_io(&s->mmio, OBJECT(s), &spi_ops, s,
Expand Down
33 changes: 0 additions & 33 deletions hw/ssi/ssi.c
Expand Up @@ -142,36 +142,3 @@ static void ssi_slave_register_types(void)
}

type_init(ssi_slave_register_types)

typedef struct SSIAutoConnectArg {
qemu_irq **cs_linep;
SSIBus *bus;
} SSIAutoConnectArg;

static int ssi_auto_connect_slave(Object *child, void *opaque)
{
SSIAutoConnectArg *arg = opaque;
SSISlave *dev = (SSISlave *)object_dynamic_cast(child, TYPE_SSI_SLAVE);
qemu_irq cs_line;

if (!dev) {
return 0;
}

cs_line = qdev_get_gpio_in_named(DEVICE(dev), SSI_GPIO_CS, 0);
qdev_set_parent_bus(DEVICE(dev), BUS(arg->bus));
**arg->cs_linep = cs_line;
(*arg->cs_linep)++;
return 0;
}

void ssi_auto_connect_slaves(DeviceState *parent, qemu_irq *cs_line,
SSIBus *bus)
{
SSIAutoConnectArg arg = {
.cs_linep = &cs_line,
.bus = bus
};

object_child_foreach(OBJECT(parent), ssi_auto_connect_slave, &arg);
}
1 change: 0 additions & 1 deletion hw/ssi/xilinx_spi.c
Expand Up @@ -334,7 +334,6 @@ static void xilinx_spi_realize(DeviceState *dev, Error **errp)

sysbus_init_irq(sbd, &s->irq);
s->cs_lines = g_new0(qemu_irq, s->num_cs);
ssi_auto_connect_slaves(dev, s->cs_lines, s->spi);
for (i = 0; i < s->num_cs; ++i) {
sysbus_init_irq(sbd, &s->cs_lines[i]);
}
Expand Down
4 changes: 0 additions & 4 deletions hw/ssi/xilinx_spips.c
Expand Up @@ -1270,7 +1270,6 @@ static void xilinx_spips_realize(DeviceState *dev, Error **errp)
XilinxSPIPS *s = XILINX_SPIPS(dev);
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
XilinxSPIPSClass *xsc = XILINX_SPIPS_GET_CLASS(s);
qemu_irq *cs;
int i;

DB_PRINT_L(0, "realized spips\n");
Expand All @@ -1297,9 +1296,6 @@ static void xilinx_spips_realize(DeviceState *dev, Error **errp)

s->cs_lines = g_new0(qemu_irq, s->num_cs * s->num_busses);
s->cs_lines_state = g_new0(bool, s->num_cs * s->num_busses);
for (i = 0, cs = s->cs_lines; i < s->num_busses; ++i, cs += s->num_cs) {
ssi_auto_connect_slaves(DEVICE(s), cs, s->spi[i]);
}

sysbus_init_irq(sbd, &s->irq);
for (i = 0; i < s->num_cs * s->num_busses; ++i) {
Expand Down
4 changes: 0 additions & 4 deletions include/hw/ssi/ssi.h
Expand Up @@ -86,10 +86,6 @@ SSIBus *ssi_create_bus(DeviceState *parent, const char *name);

uint32_t ssi_transfer(SSIBus *bus, uint32_t val);

/* Automatically connect all children nodes a spi controller as slaves */
void ssi_auto_connect_slaves(DeviceState *parent, qemu_irq *cs_lines,
SSIBus *bus);

/* max111x.c */
void max111x_set_input(DeviceState *dev, int line, uint8_t value);

Expand Down

0 comments on commit 7f16c76

Please sign in to comment.