Peter Maydell <peter.mayd...@linaro.org> writes: > On Wed, 15 Sept 2021 at 05:28, Markus Armbruster <arm...@redhat.com> wrote: >> >> Peter Maydell <peter.mayd...@linaro.org> writes: >> > I'm not sure how best to sort this tangle out. We could: >> > * make controller devices pass in NULL as bus name; this >> > means that some bus names will change, which is an annoying >> > breakage but for these minor bus types we can probably >> > get away with it. This brings these buses into line with >> > how we've been handling uniqueness for ide and scsi. >> >> To gauge the breakage, we need a list of the affected bus names. > > Looking through, there are a few single-use or special > purpose buses I'm going to ignore for now (eg vmbus, or > the s390 ones). The four big bus types where controllers > often specify a bus name and override the 'autogenerate > unique name' handling are pci, ssi, sd, and i2c. (pci mostly > gets away with it I expect by machines only having one pci > bus.) Of those, I've gone through i2c. These are all the > places where we create a specifically-named i2c bus (via > i2c_init_bus()), together with the affected boards: > > hw/arm/pxa2xx.c > - the PXA SoC code creates both the intended-for-use > i2c buses (which get auto-names) and also several i2c > buses intended for internal board-code use only which > are all given the same name "dummy". > Boards: connex, verdex, tosa, mainstone, akita, spitz, > borzoi, terrier, z2 > hw/arm/stellaris.c > - The i2c controller names its bus "i2c". There is only one i2c > controller on these boards, so no name conflicts. > Boards: lm3s811evb, lm3s6965evb > hw/display/ati.c > - The ATI VGA device has an on-board i2c controller which it > connects the DDC that holds the EDID information. The bus is > always named "ati-vga.ddc", so if you have multiple of this > PCI device in the system the buses have the same names. > hw/display/sm501.c > - Same as ATI, but the bus name is "sm501.i2c" > hw/i2c/aspeed_i2c.c > - This I2C controller has either 14 or 16 (!) different i2c > buses, and it assigns them names "aspeed.i2c.N" for N = 0,1,2,... > The board code mostly seems to use these to wire up various > on-board i2c devices. > Boards: palmetto-bmc, supermicrox11-bmc, ast2500-evb, romulus-bmc, > swift-bmc, sonorapass-bmc, witherspoon-bmc, ast2600-evb, > tacoma-bmc, g220a-bmc, quanta-q71l-bmc, rainier-bmc > hw/i2c/bitbang_i2c.c > - the "GPIO to I2C bridge" device always names its bus "i2c". > Used only on musicpal, which only creates one of these buses. > Boards: musicpal > hw/i2c/exynos4210_i2c.c > - This i2c controller always names its bus "i2c". There are 9 > of these controllers on the board, so they all have clashing > names. > Boards: nuri, smdkc210 > hw/i2c/i2c_mux_pca954x.c > - This is an i2c multiplexer. All the child buses are named > "i2c-bus". The multiplexer is used by the aspeed and npcm7xx > boards. (There's a programmable way to get at individual > downstream i2c buses despite the name clash; none of the boards > using this multiplexer actually connect any devices downstream of > it yet.) > Boards: palmetto-bmc, supermicrox11-bmc, ast2500-evb, romulus-bmc, > swift-bmc, sonorapass-bmc, witherspoon-bmc, ast2600-evb, > tacoma-bmc, g220a-bmc, quanta-q71l-bmc, rainier-bmc, > npcm750-evb, quanta-gsj, quanta-gbs-bmc, kudo-bmc > hw/i2c/mpc_i2c.c > - This controller always names its bus "i2c". There is only one > of these controllers in the machine. > Boards: ppce500, mpc8544ds > hw/i2c/npcm7xx_smbus.c > - This controller always names its bus "i2c-bus". There are multiple > controllers on the boards. The name also clashes with the one used > by the pca954x muxes on these boards (see above). > Boards: npcm750-evb, quanta-gsj, quanta-gbs-bmc, kudo-bmc > hw/i2c/pm_smbus.c > - This is the PC SMBUS implementation (it is not a QOM device...) > The bus is always called "i2c". > Boards: haven't worked through; at least all the x86 PC-like > boards, I guess > hw/i2c/ppc4xx_i2c.c > - This controller always names its bus "i2c". The taihu and > ref405ep have only one controller, but sam460ex has two which > will have non-unique names. > Boards: taihu, ref405ep, sam460ex > hw/i2c/versatile_i2c.c > - This controller always names its bus "i2c". The MPS boards all > have multiples of this controller with clashing names; the others > have only one controller. > Boards: mps2-an385, mps2-an386, mps2-an500, mps2-an511, > mps2-an505, mps2-an521, mps3-an524, mps3-an547, > realview-eb, realview-eb-mpcore, realview-pb-a8, realview-pbx-a9, > versatileab, versatilepb, vexpress-a9, vexpress-a15 > > In a lot of these cases I suspect the i2c controllers are > provided either to allow connection of various internal-to-the-board > devices, or simply so that guest OS bootup code that initializes > the i2c controller doesn't fall over. However since there's > nothing stopping users from creating i2c devices themselves > on the commandline, some people might be doing that.
What are the devices they could add? Anything they could *reasonably* want to add? Breaking "unreasonable" uses could be defendable. The only spot where we set ->bus_type = TYPE_I2C_BUS is i2c_slave_class_init(). Therefore, only the concrete subtypes of TYPE_I2C_SLAVE can go on an i2c bus, which makes sense. These are: TYPE_ADM1272 "adm1272" TYPE_AER915 "aer915" TYPE_AT24C_EE "at24c-eeprom" TYPE_DS1338 "ds1338" "emc1413" "emc1414" TYPE_I2CDDC "i2c-ddc" TYPE_LM8323 "lm8323" TYPE_M41T80 "m41t80" TYPE_MAX34451 "max34451" TYPE_MAX7310 "max7310" TYPE_PCA9546 "pca9546" TYPE_PCA9548 "pca9548" TYPE_PCA9552 "pca9552" TYPE_PXA2XX_I2C_SLAVE "pxa2xx-i2c-slave" TYPE_SII9022 "sii9022" TYPE_SMBUS_DEVICE "smbus-device" TYPE_SMBUS_EEPROM "smbus-eeprom" TYPE_SMBUS_IPMI "smbus-ipmi" TYPE_SSD0303 "ssd0303" TYPE_TMP105 "tmp105" "tmp421" "tmp422" "tmp423" TYPE_TOSA_DAC "tosa_dac" TYPE_TWL92230 "twl92230" TYPE_WM8750 "wm8750" Grep hits in: hw/arm/aspeed.c hw/arm/musicpal.c hw/arm/npcm7xx_boards.c hw/arm/nseries.c hw/arm/pxa2xx.c hw/arm/realview.c hw/arm/spitz.c hw/arm/stellaris.c hw/arm/tosa.c hw/arm/versatilepb.c hw/arm/vexpress.c hw/arm/z2.c hw/audio/marvell_88w8618.c hw/audio/wm8750.c hw/display/ati.c hw/display/i2c-ddc.c hw/display/sii9022.c hw/display/sm501.c hw/display/ssd0303.c hw/display/xlnx_dp.c hw/gpio/max7310.c hw/i2c/i2c_mux_pca954x.c hw/i2c/pmbus_device.c hw/i2c/smbus_eeprom.c hw/i2c/smbus_slave.c hw/input/lm832x.c hw/ipmi/smbus_ipmi.c hw/misc/pca9552.c hw/nvram/eeprom_at24c.c hw/ppc/e500.c hw/ppc/sam460ex.c hw/rtc/ds1338.c hw/rtc/m41t80.c hw/rtc/twl92230.c hw/sensor/adm1272.c hw/sensor/emc141x.c hw/sensor/max34451.c hw/sensor/tmp105.c hw/sensor/tmp421.c include/hw/audio/wm8750.h include/hw/display/i2c-ddc.h include/hw/i2c/i2c_mux_pca954x.h include/hw/i2c/smbus_slave.h include/hw/input/lm832x.h include/hw/misc/pca9552.h include/hw/sensor/tmp105.h tests/qtest/adm1272-test.c tests/qtest/ds1338-test.c tests/qtest/emc141x-test.c tests/qtest/max34451-test.c tests/qtest/pca9552-test.c tests/qtest/tmp105-test.c > > In some of these cases (eg the i2c bus on the ATI VGA driver) > I suspect the desired behaviour is "unique bus name based on > a standard template, eg 'ati-vga.ddc.0/1/...'. It sounds like > we can't do that, though. We could add yet another case to qbus_init(): if name is non-null, and bus->parent has a flag set, we use "%s.%d", name, bus->parent->num_child_bus. "Could" doesn not imply "should". > (Also they probably don't want to > permit users to command-line plug i2c devices into it...) Then they should massage the bus so it doesn't accept additional devices. Didn't you post a patch related to this recently?