On 9/7/21 4:05 PM, Peter Delevoryas wrote: > > >> On Sep 7, 2021, at 1:59 AM, Joel Stanley <j...@jms.id.au> wrote: >> >> On Mon, 6 Sept 2021 at 13:31, <p...@fb.com> wrote: >>> >>> From: Peter Delevoryas <p...@fb.com> >>> >>> This adds a new machine type "fuji-bmc" based on the following device tree: >>> >>> https://github.com/torvalds/linux/blob/40cb6373b46/arch/arm/boot/dts/aspeed-bmc-facebook-fuji.dts >>> >>> Most of the i2c devices are not there, they're added here: >>> >>> https://github.com/facebook/openbmc/blob/fb2ed12002fb/meta-facebook/meta-fuji/recipes-utils/openbmc-utils/files/setup_i2c.sh >>> >>> I tested this by building a Fuji image from Facebook's OpenBMC repo, >>> booting, and ssh'ing from host-to-guest. >>> >>> Signed-off-by: Peter Delevoryas <p...@fb.com> >> >> Reviewed-by: Joel Stanley <j...@jms.id.au> > > Thanks! > >> >>> +static void fuji_bmc_i2c_init(AspeedMachineState *bmc) >>> +{ >>> + AspeedSoCState *soc = &bmc->soc; >>> + I2CBus *i2c[144] = {}; >>> + >>> + for (int i = 0; i < 16; i++) { >>> + i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i); >>> + } >>> + I2CBus *i2c180 = i2c[2]; >>> + I2CBus *i2c480 = i2c[8]; >>> + I2CBus *i2c600 = i2c[11]; >>> + >>> + get_pca9548_channels(i2c180, 0x70, &i2c[16]); >> >> Wow, this is interesting. How did you go about testing it? Are you >> sure you didn't overwrite any of the pointers? >> >> It might be worth coming up with a better way of describing all of the >> i2c buses for future machines. >> >> Cheers, >> >> Joel > > Ah, yes, so, I took the compiled device tree output and printed it out, > and it has a complete list of the i2c aliases from all of the component > device tree’s, like this: > > dtc openbmc/build-fuji/tmp/deploy/images/fuji/aspeed-bmc-facebook-fuji.dtb > > aliases { > i2c0 = "/ahb/apb/bus@1e78a000/i2c-bus@80"; > i2c1 = "/ahb/apb/bus@1e78a000/i2c-bus@100"; > i2c2 = "/ahb/apb/bus@1e78a000/i2c-bus@180"; > i2c3 = "/ahb/apb/bus@1e78a000/i2c-bus@200"; > i2c4 = "/ahb/apb/bus@1e78a000/i2c-bus@280"; > i2c5 = "/ahb/apb/bus@1e78a000/i2c-bus@300"; > i2c6 = "/ahb/apb/bus@1e78a000/i2c-bus@380"; > i2c7 = "/ahb/apb/bus@1e78a000/i2c-bus@400"; > i2c8 = "/ahb/apb/bus@1e78a000/i2c-bus@480"; > i2c9 = "/ahb/apb/bus@1e78a000/i2c-bus@500"; > i2c10 = "/ahb/apb/bus@1e78a000/i2c-bus@580"; > i2c11 = "/ahb/apb/bus@1e78a000/i2c-bus@600"; > i2c12 = "/ahb/apb/bus@1e78a000/i2c-bus@680"; > i2c13 = "/ahb/apb/bus@1e78a000/i2c-bus@700"; > i2c14 = "/ahb/apb/bus@1e78a000/i2c-bus@780"; > i2c15 = "/ahb/apb/bus@1e78a000/i2c-bus@800"; > ... > i2c16 = "/ahb/apb/bus@1e78a000/i2c-bus@180/i2c-switch@70/i2c@0"; > i2c17 = "/ahb/apb/bus@1e78a000/i2c-bus@180/i2c-switch@70/i2c@1"; > i2c18 = "/ahb/apb/bus@1e78a000/i2c-bus@180/i2c-switch@70/i2c@2"; > i2c19 = "/ahb/apb/bus@1e78a000/i2c-bus@180/i2c-switch@70/i2c@3"; > i2c20 = "/ahb/apb/bus@1e78a000/i2c-bus@180/i2c-switch@70/i2c@4"; > i2c21 = "/ahb/apb/bus@1e78a000/i2c-bus@180/i2c-switch@70/i2c@5"; > i2c22 = "/ahb/apb/bus@1e78a000/i2c-bus@180/i2c-switch@70/i2c@6"; > i2c23 = "/ahb/apb/bus@1e78a000/i2c-bus@180/i2c-switch@70/i2c@7"; > i2c24 = "/ahb/apb/bus@1e78a000/i2c-bus@480/i2c-switch@70/i2c@0"; > i2c25 = "/ahb/apb/bus@1e78a000/i2c-bus@480/i2c-switch@70/i2c@1"; > i2c26 = "/ahb/apb/bus@1e78a000/i2c-bus@480/i2c-switch@70/i2c@2"; > i2c27 = "/ahb/apb/bus@1e78a000/i2c-bus@480/i2c-switch@70/i2c@3"; > i2c28 = "/ahb/apb/bus@1e78a000/i2c-bus@480/i2c-switch@70/i2c@4"; > i2c29 = "/ahb/apb/bus@1e78a000/i2c-bus@480/i2c-switch@70/i2c@5"; > i2c30 = "/ahb/apb/bus@1e78a000/i2c-bus@480/i2c-switch@70/i2c@6"; > i2c31 = "/ahb/apb/bus@1e78a000/i2c-bus@480/i2c-switch@70/i2c@7"; > i2c40 = "/ahb/apb/bus@1e78a000/i2c-bus@600/i2c-switch@77/i2c@0"; > i2c41 = "/ahb/apb/bus@1e78a000/i2c-bus@600/i2c-switch@77/i2c@1"; > i2c42 = "/ahb/apb/bus@1e78a000/i2c-bus@600/i2c-switch@77/i2c@2"; > i2c43 = "/ahb/apb/bus@1e78a000/i2c-bus@600/i2c-switch@77/i2c@3"; > i2c44 = "/ahb/apb/bus@1e78a000/i2c-bus@600/i2c-switch@77/i2c@4"; > i2c46 = "/ahb/apb/bus@1e78a000/i2c-bus@600/i2c-switch@77/i2c@6"; > ... > i2c143 = > "/ahb/apb/bus@1e78a000/i2c-bus@600/i2c-switch@77/i2c@7/i2c-switch@76/i2c@7"; > }; > > And setup_i2c.sh’s first parameter is referencing these aliases: > > # # i2c-mux 2, channel 2 > i2c_device_add 17 0x4c lm75 #SCM temp. sensor > i2c_device_add 17 0x4d lm75 #SCM temp. sensor > > # # i2c-mux 2, channel 3 > i2c_device_add 19 0x52 24c64 #EEPROM > > # # i2c-mux 2, channel 4 > i2c_device_add 20 0x50 24c02 #BMC54616S EEPROM > > # # i2c-mux 2, channel 6 > i2c_device_add 22 0x52 24c02 #BMC54616S EEPROM > > My first idea was to make some kind of tree definition > describing the i2c switches and then do a breadth first > search/etc to enumerate all of the buses. > > I2CBus *i2c[144] = {} > // Initialize base set of i2c buses. > i2c[0] = … > i2c[1] = … > … > i2c[15] = … > // Initialize switch definitions, includes > // some kind of reference to its parent bus. > struct { … } switches[] = {…} > // Populate i2c buses using switch definitions. > for (int i = 0; i < sizeof(switches)/sizeof(switches[0]); i++) { > I2CSlave *switch = find_switch(i2c, switches[i]); > ^^^^Recursive function or something > for (int j = 0; j < 8; j++) { > // Special case because fuji datasheet skips 32..40 > if (n == 32) { > n = 40; > } > i2c[n++] = aspeed_i2c_get_bus(switch, j); > } > } > > I’m realizing, I probably should have done this, but I also realized > there’s not that many switches in the system, so if you just automate > the get_channels() part (x8 buses for each switch) then it was > not unreasonable to just manually write out the locations for each switch. > > As far as testing: I didn’t do a lot, I mostly just eyeball’d it > more after writing it out and then I looked at the boot logs, so > I’m still not _absolutely_ sure that I got it right. Let me know > if you guys think I should refactor this!
It think it's fine. Fixes can come later on if needed. Phil, Are we OK with this ? I would like to include this patch in the v2 of the aspeed PR. Thanks, C.
