On Sat Jan 4, 2025 at 2:18 AM AEST, Chalapathi V wrote:
> Create a spi buses with distict names on each socket so that responders
> are attached to correct SPI controllers.
>
> QOM tree on a 2 socket machine:
> (qemu) info qom-tree
> /machine (powernv10-machine)
> /chip[0] (power10_v2.0-pnv-chip)
> /pib_spic[0] (pnv-spi)
> /chip0.pnv.spi.bus.0 (SSI)
> /xscom-spi[0] (memory-region)
> /chip[1] (power10_v2.0-pnv-chip)
> /pib_spic[0] (pnv-spi)
> /chip1.pnv.spi.bus.0 (SSI)
> /xscom-spi[0] (memory-region)
Mechanics of the patch looks fine. I don't know about the name
though.
I think "pnv-spi-bus" is the right name for the bus. Using dots as
with chip0. makes it seem like each element is part of a topology.
Would chip0.pnv-spi-bus be better?
I don't suppose there is a good way to create an alias so existing
cmdline works and refers to the bus on chip0? Maybe the chip0 bus
could just not have the chip0. prefix?
Thanks,
Nick
>
> Signed-off-by: Chalapathi V <chalapath...@linux.ibm.com>
> ---
> include/hw/ssi/pnv_spi.h | 3 ++-
> hw/ppc/pnv.c | 2 ++
> hw/ssi/pnv_spi.c | 5 +++--
> tests/qtest/pnv-spi-seeprom-test.c | 2 +-
> 4 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/include/hw/ssi/pnv_spi.h b/include/hw/ssi/pnv_spi.h
> index 9878d9a25f..7fc5da1f84 100644
> --- a/include/hw/ssi/pnv_spi.h
> +++ b/include/hw/ssi/pnv_spi.h
> @@ -31,7 +31,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PnvSpi, PNV_SPI)
> #define PNV_SPI_REG_SIZE 8
> #define PNV_SPI_REGS 7
>
> -#define TYPE_PNV_SPI_BUS "pnv-spi-bus"
> +#define TYPE_PNV_SPI_BUS "pnv.spi.bus"
> typedef struct PnvSpi {
> SysBusDevice parent_obj;
>
> @@ -42,6 +42,7 @@ typedef struct PnvSpi {
> Fifo8 rx_fifo;
> /* SPI object number */
> uint32_t spic_num;
> + uint32_t chip_id;
> uint8_t transfer_len;
> uint8_t responder_select;
> /* To verify if shift_n1 happens prior to shift_n2 */
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 11fd477b71..ce23892fdf 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -2226,6 +2226,8 @@ static void pnv_chip_power10_realize(DeviceState *dev,
> Error **errp)
> /* pib_spic[2] connected to 25csm04 which implements 1 byte transfer
> */
> object_property_set_int(OBJECT(&chip10->pib_spic[i]), "transfer_len",
> (i == 2) ? 1 : 4, &error_fatal);
> + object_property_set_int(OBJECT(&chip10->pib_spic[i]), "chip-id",
> + chip->chip_id, &error_fatal);
> if (!sysbus_realize(SYS_BUS_DEVICE(OBJECT
> (&chip10->pib_spic[i])), errp)) {
> return;
> diff --git a/hw/ssi/pnv_spi.c b/hw/ssi/pnv_spi.c
> index 87eac666bb..41beb559c6 100644
> --- a/hw/ssi/pnv_spi.c
> +++ b/hw/ssi/pnv_spi.c
> @@ -1116,14 +1116,15 @@ static const MemoryRegionOps pnv_spi_xscom_ops = {
>
> static const Property pnv_spi_properties[] = {
> DEFINE_PROP_UINT32("spic_num", PnvSpi, spic_num, 0),
> + DEFINE_PROP_UINT32("chip-id", PnvSpi, chip_id, 0),
> DEFINE_PROP_UINT8("transfer_len", PnvSpi, transfer_len, 4),
> };
>
> static void pnv_spi_realize(DeviceState *dev, Error **errp)
> {
> PnvSpi *s = PNV_SPI(dev);
> - g_autofree char *name = g_strdup_printf(TYPE_PNV_SPI_BUS ".%d",
> - s->spic_num);
> + g_autofree char *name = g_strdup_printf("chip%d." TYPE_PNV_SPI_BUS ".%d",
> + s->chip_id, s->spic_num);
> s->ssi_bus = ssi_create_bus(dev, name);
> s->cs_line = g_new0(qemu_irq, 1);
> qdev_init_gpio_out_named(DEVICE(s), s->cs_line, "cs", 1);
> diff --git a/tests/qtest/pnv-spi-seeprom-test.c
> b/tests/qtest/pnv-spi-seeprom-test.c
> index 57f20af76e..ef1005a926 100644
> --- a/tests/qtest/pnv-spi-seeprom-test.c
> +++ b/tests/qtest/pnv-spi-seeprom-test.c
> @@ -92,7 +92,7 @@ static void test_spi_seeprom(const void *data)
> qts = qtest_initf("-machine powernv10 -smp 2,cores=2,"
> "threads=1 -accel tcg,thread=single -nographic "
> "-blockdev node-name=pib_spic2,driver=file,"
> - "filename=%s -device 25csm04,bus=pnv-spi-bus.2,cs=0,"
> + "filename=%s -device
> 25csm04,bus=chip0.pnv.spi.bus.2,cs=0,"
> "drive=pib_spic2", tmp_path);
> spi_seeprom_transaction(qts, chip);
> qtest_quit(qts);
