| Message ID | 20220305000656.1944589-2-patrick@stwcx.xyz |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] hw/arm/aspeed: allow missing spi_model | expand |
On 3/5/22 01:06, Patrick Williams wrote: > Add the 'bletchley-bmc' machine type based on the kernel DTS[1] and > hardware schematics available to me. The i2c model is as complete as > the current QEMU models support, but in some cases I substituted devices > that are close enough for present functionality. Strap registers are > kept the same as the AST2600-EVB until I'm able to confirm correct > values with physical hardware. > > This has been tested with an openbmc image built from [2] plus a kernel > patch[3] for the SPI flash module. > > 1. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm/boot/dts/aspeed-bmc-facebook-bletchley.dts?id=a8c729e966c4e9d033242d948b0e53c2a62d32e2 > 2. https://github.com/openbmc/openbmc/commit/b9432b980d7f63f7512ffbcc7124386ba896dfc6 > 3. https://github.com/openbmc/linux/commit/25b566b9a9d7f5d4f10c1b7304007bdb286eefd7 > > Signed-off-by: Patrick Williams <patrick@stwcx.xyz> > --- > hw/arm/aspeed.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 77 insertions(+) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index 617a1ecbdc..3dc2ede823 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -167,6 +167,11 @@ struct AspeedMachineState { > #define FUJI_BMC_HW_STRAP1 0x00000000 > #define FUJI_BMC_HW_STRAP2 0x00000000 > > +/* Bletchley hardware value */ > +/* TODO: Leave same as EVB for now. */ > +#define BLETCHLEY_BMC_HW_STRAP1 AST2600_EVB_HW_STRAP1 > +#define BLETCHLEY_BMC_HW_STRAP2 AST2600_EVB_HW_STRAP2 > + > /* > * The max ram region is for firmwares that scan the address space > * with load/store to guess how much RAM the SoC has. > @@ -901,6 +906,54 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc) > } > } > > +#define TYPE_TMP421 "tmp421" > + > +static void bletchley_bmc_i2c_init(AspeedMachineState *bmc) > +{ > + AspeedSoCState *soc = &bmc->soc; > + I2CBus *i2c[13] = {}; > + for (int i = 0; i < 13; i++) { > + if ((i == 8) || (i == 11)) { > + continue; > + } > + i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i); > + } > + > + /* Bus 0 - 5 all have the same config. */ > + for (int i = 0; i < 6; i++) { > + /* Missing model: ti,ina230 @ 0x45 */ > + /* Missing model: mps,mp5023 @ 0x40 */ > + i2c_slave_create_simple(i2c[i], TYPE_TMP421, 0x4f); > + /* Missing model: nxp,pca9539 @ 0x76, but PCA9552 works enough */ > + i2c_slave_create_simple(i2c[i], TYPE_PCA9552, 0x76); > + i2c_slave_create_simple(i2c[i], TYPE_PCA9552, 0x67); > + /* Missing model: fsc,fusb302 @ 0x22 */ > + } > + > + /* Bus 6 */ > + at24c_eeprom_init(i2c[6], 0x56, 65536); > + /* Missing model: nxp,pcf85263 @ 0x51 , but ds1338 works enough */ > + i2c_slave_create_simple(i2c[6], "ds1338", 0x51); > + > + > + /* Bus 7 */ > + at24c_eeprom_init(i2c[7], 0x54, 65536); > + > + /* Bus 9 */ > + i2c_slave_create_simple(i2c[9], TYPE_TMP421, 0x4f); > + > + /* Bus 10 */ > + i2c_slave_create_simple(i2c[10], TYPE_TMP421, 0x4f); > + /* Missing model: ti,hdc1080 @ 0x40 */ > + i2c_slave_create_simple(i2c[10], TYPE_PCA9552, 0x67); > + > + /* Bus 12 */ > + /* Missing model: adi,adm1278 @ 0x11 */ > + i2c_slave_create_simple(i2c[12], TYPE_TMP421, 0x4c); > + i2c_slave_create_simple(i2c[12], TYPE_TMP421, 0x4d); > + i2c_slave_create_simple(i2c[12], TYPE_PCA9552, 0x67); > +} > + > static bool aspeed_get_mmio_exec(Object *obj, Error **errp) > { > return ASPEED_MACHINE(obj)->mmio_exec; > @@ -1224,6 +1277,26 @@ static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data) > aspeed_soc_num_cpus(amc->soc_name); > }; > > + > +static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data) > +{ > + MachineClass *mc = MACHINE_CLASS(oc); > + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); > + > + mc->desc = "Facebook Bletchley BMC (Cortex-A7)"; > + amc->soc_name = "ast2600-a3"; > + amc->hw_strap1 = BLETCHLEY_BMC_HW_STRAP1; > + amc->hw_strap2 = BLETCHLEY_BMC_HW_STRAP2; > + amc->fmc_model = "w25q01jvq"; So we need this patch : http://patchwork.ozlabs.org/project/qemu-devel/patch/20220304180920.1780992-1-patrick@stwcx.xyz/ May be I can take it through my queue. > + amc->spi_model = NULL; > + amc->num_cs = 1; There are two flash devices on the FMC. I can fix it inline since it is the only change I would request. Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > + amc->macs_mask = ASPEED_MAC2_ON; > + amc->i2c_init = bletchley_bmc_i2c_init; > + mc->default_ram_size = 512 * MiB; > + mc->default_cpus = mc->min_cpus = mc->max_cpus = > + aspeed_soc_num_cpus(amc->soc_name); > +} > + > static const TypeInfo aspeed_machine_types[] = { > { > .name = MACHINE_TYPE_NAME("palmetto-bmc"), > @@ -1277,6 +1350,10 @@ static const TypeInfo aspeed_machine_types[] = { > .name = MACHINE_TYPE_NAME("fuji-bmc"), > .parent = TYPE_ASPEED_MACHINE, > .class_init = aspeed_machine_fuji_class_init, > + }, { > + .name = MACHINE_TYPE_NAME("bletchley-bmc"), > + .parent = TYPE_ASPEED_MACHINE, > + .class_init = aspeed_machine_bletchley_class_init, > }, { > .name = TYPE_ASPEED_MACHINE, > .parent = TYPE_MACHINE,
On Sat, Mar 05, 2022 at 08:57:47AM +0100, Cédric Le Goater wrote: > On 3/5/22 01:06, Patrick Williams wrote: > > + mc->desc = "Facebook Bletchley BMC (Cortex-A7)"; > > + amc->soc_name = "ast2600-a3"; > > + amc->hw_strap1 = BLETCHLEY_BMC_HW_STRAP1; > > + amc->hw_strap2 = BLETCHLEY_BMC_HW_STRAP2; > > + amc->fmc_model = "w25q01jvq"; > > So we need this patch : > > http://patchwork.ozlabs.org/project/qemu-devel/patch/20220304180920.1780992-1-patrick@stwcx.xyz/ > > May be I can take it through my queue. Yes, it does. I had sent that one earlier and probably should have been clear on the dependency. > > + amc->spi_model = NULL; > > + amc->num_cs = 1; > > There are two flash devices on the FMC. I can fix it inline since > it is the only change I would request. Yes, there are. I think all of the Facebook systems have dual FMC, for redundancy in hardware, but we can get by in QEMU with just a single one. I'll see however you fix it up and see I can update all the other systems as well. We have an internal patch to expand the CS on FMC to 2 but we haven't upstreamed it yet and I'm worried it will break some users w.r.t. the CLI changing for adding images. My recollection is that the Romulus CI on OpenBMC relies on the PNOR being the 2nd argument. > Reviewed-by: Cédric Le Goater <clg@kaod.org> Thank you Cedric!
>> There are two flash devices on the FMC. I can fix it inline since >> it is the only change I would request. > > Yes, there are. I think all of the Facebook systems have dual FMC, for > redundancy in hardware, but we can get by in QEMU with just a single one. yes, the kernel will complain though and I don't know how robust the spi-nor based driver is. I think you sent a patch for a related issue. The newer spi-mem driver should be fine. > I'll see however you fix it up and see I can update all the other systems as > well. ok. may be for 7.1 then. > We have an internal patch to expand the CS on FMC to 2 but we haven't > upstreamed it yet and I'm worried it will break some users w.r.t. the CLI > changing for adding images. Yes. That's the problem. I am afraid some CI systems will break with these change in a newer QEMU. The command line options will need to adapt. > My recollection is that the Romulus CI on OpenBMC relies on the PNOR > being the 2nd argument. That's the initial assumption made years ago. First mtd device is FMC, second is the PNOR. It is reaching its limits. I am looking at improving the command line argument to support: -drive file=<file>,format=raw,id=drive0 -device mx66l1g45g,bus=ssi.0,drive=drive0 which we would clearly define the topology. Adding a cs=[0-5] or and addr=[0-5] is the next step. Thanks, C.
On Tue, Mar 08, 2022 at 09:14:07AM +0100, Cédric Le Goater wrote: > > >> There are two flash devices on the FMC. I can fix it inline since > >> it is the only change I would request. > > > > Yes, there are. I think all of the Facebook systems have dual FMC, for > > redundancy in hardware, but we can get by in QEMU with just a single one. > > yes, the kernel will complain though and I don't know how robust > the spi-nor based driver is. I think you sent a patch for a related > issue. > > The newer spi-mem driver should be fine. Oh yes. I already forgot that I'm running with that patch since Joel added it to our backport 5.15 branch. One of the reasons I wrote that patch was to make QEMU not kpanic. :( > > > I'll see however you fix it up and see I can update all the other systems as > > well. > > ok. may be for 7.1 then. > > > We have an internal patch to expand the CS on FMC to 2 but we haven't > > upstreamed it yet and I'm worried it will break some users w.r.t. the CLI > > changing for adding images. > > Yes. That's the problem. I am afraid some CI systems will break with > these change in a newer QEMU. The command line options will need to > adapt. My recollection is that the Romulus CI uses a branch of QEMU that at this point is rather old anyhow. We should be able to fix up the CI scripts at the same time we upgrade. Are you or Andrew J maintaining that branch? > > My recollection is that the Romulus CI on OpenBMC relies on the PNOR > > being the 2nd argument. > > That's the initial assumption made years ago. First mtd device is FMC, > second is the PNOR. It is reaching its limits. > > I am looking at improving the command line argument to support: > > -drive file=<file>,format=raw,id=drive0 -device mx66l1g45g,bus=ssi.0,drive=drive0 > > which we would clearly define the topology. Adding a cs=[0-5] or and > addr=[0-5] is the next step. Seems fine to me.
On Tue, 8 Mar 2022 at 17:23, Patrick Williams <patrick@stwcx.xyz> wrote: > > On Tue, Mar 08, 2022 at 09:14:07AM +0100, Cédric Le Goater wrote: > > > > >> There are two flash devices on the FMC. I can fix it inline since > > >> it is the only change I would request. > > > > > > Yes, there are. I think all of the Facebook systems have dual FMC, for > > > redundancy in hardware, but we can get by in QEMU with just a single one. > > > > yes, the kernel will complain though and I don't know how robust > > the spi-nor based driver is. I think you sent a patch for a related > > issue. > > > > The newer spi-mem driver should be fine. > > Oh yes. I already forgot that I'm running with that patch since Joel added it > to our backport 5.15 branch. One of the reasons I wrote that patch was to make > QEMU not kpanic. :( > > > > > > I'll see however you fix it up and see I can update all the other systems as > > > well. > > > > ok. may be for 7.1 then. > > > > > We have an internal patch to expand the CS on FMC to 2 but we haven't > > > upstreamed it yet and I'm worried it will break some users w.r.t. the CLI > > > changing for adding images. > > > > Yes. That's the problem. I am afraid some CI systems will break with > > these change in a newer QEMU. The command line options will need to > > adapt. > > My recollection is that the Romulus CI uses a branch of QEMU that at this point > is rather old anyhow. We should be able to fix up the CI scripts at the same > time we upgrade. It looks like that branch missed the 7.2 boat. Given it's imminent release, we should update the branch to Cedric's aspeed-7.0 when 7.0 is tagged. With this branch could do CI on Rainier (or S6Q, or transformers) with the eMMC image. I think there's value in doing CI on both eMMC and SPI machines. I'd like to see a boot test added for all of the machines in CI. Most (all?) machines will get to bmc standby by booting on a generic Qemu machine, such as the evb. The machines that do have more of the system modelled could boot that instead, and in the future add further tests. > > Are you or Andrew J maintaining that branch? > > > > My recollection is that the Romulus CI on OpenBMC relies on the PNOR > > > being the 2nd argument. > > > > That's the initial assumption made years ago. First mtd device is FMC, > > second is the PNOR. It is reaching its limits. > > > > I am looking at improving the command line argument to support: > > > > -drive file=<file>,format=raw,id=drive0 -device mx66l1g45g,bus=ssi.0,drive=drive0 > > > > which we would clearly define the topology. Adding a cs=[0-5] or and > > addr=[0-5] is the next step. > > Seems fine to me. > > -- > Patrick Williams
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 617a1ecbdc..3dc2ede823 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -167,6 +167,11 @@ struct AspeedMachineState { #define FUJI_BMC_HW_STRAP1 0x00000000 #define FUJI_BMC_HW_STRAP2 0x00000000 +/* Bletchley hardware value */ +/* TODO: Leave same as EVB for now. */ +#define BLETCHLEY_BMC_HW_STRAP1 AST2600_EVB_HW_STRAP1 +#define BLETCHLEY_BMC_HW_STRAP2 AST2600_EVB_HW_STRAP2 + /* * The max ram region is for firmwares that scan the address space * with load/store to guess how much RAM the SoC has. @@ -901,6 +906,54 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc) } } +#define TYPE_TMP421 "tmp421" + +static void bletchley_bmc_i2c_init(AspeedMachineState *bmc) +{ + AspeedSoCState *soc = &bmc->soc; + I2CBus *i2c[13] = {}; + for (int i = 0; i < 13; i++) { + if ((i == 8) || (i == 11)) { + continue; + } + i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i); + } + + /* Bus 0 - 5 all have the same config. */ + for (int i = 0; i < 6; i++) { + /* Missing model: ti,ina230 @ 0x45 */ + /* Missing model: mps,mp5023 @ 0x40 */ + i2c_slave_create_simple(i2c[i], TYPE_TMP421, 0x4f); + /* Missing model: nxp,pca9539 @ 0x76, but PCA9552 works enough */ + i2c_slave_create_simple(i2c[i], TYPE_PCA9552, 0x76); + i2c_slave_create_simple(i2c[i], TYPE_PCA9552, 0x67); + /* Missing model: fsc,fusb302 @ 0x22 */ + } + + /* Bus 6 */ + at24c_eeprom_init(i2c[6], 0x56, 65536); + /* Missing model: nxp,pcf85263 @ 0x51 , but ds1338 works enough */ + i2c_slave_create_simple(i2c[6], "ds1338", 0x51); + + + /* Bus 7 */ + at24c_eeprom_init(i2c[7], 0x54, 65536); + + /* Bus 9 */ + i2c_slave_create_simple(i2c[9], TYPE_TMP421, 0x4f); + + /* Bus 10 */ + i2c_slave_create_simple(i2c[10], TYPE_TMP421, 0x4f); + /* Missing model: ti,hdc1080 @ 0x40 */ + i2c_slave_create_simple(i2c[10], TYPE_PCA9552, 0x67); + + /* Bus 12 */ + /* Missing model: adi,adm1278 @ 0x11 */ + i2c_slave_create_simple(i2c[12], TYPE_TMP421, 0x4c); + i2c_slave_create_simple(i2c[12], TYPE_TMP421, 0x4d); + i2c_slave_create_simple(i2c[12], TYPE_PCA9552, 0x67); +} + static bool aspeed_get_mmio_exec(Object *obj, Error **errp) { return ASPEED_MACHINE(obj)->mmio_exec; @@ -1224,6 +1277,26 @@ static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data) aspeed_soc_num_cpus(amc->soc_name); }; + +static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "Facebook Bletchley BMC (Cortex-A7)"; + amc->soc_name = "ast2600-a3"; + amc->hw_strap1 = BLETCHLEY_BMC_HW_STRAP1; + amc->hw_strap2 = BLETCHLEY_BMC_HW_STRAP2; + amc->fmc_model = "w25q01jvq"; + amc->spi_model = NULL; + amc->num_cs = 1; + amc->macs_mask = ASPEED_MAC2_ON; + amc->i2c_init = bletchley_bmc_i2c_init; + mc->default_ram_size = 512 * MiB; + mc->default_cpus = mc->min_cpus = mc->max_cpus = + aspeed_soc_num_cpus(amc->soc_name); +} + static const TypeInfo aspeed_machine_types[] = { { .name = MACHINE_TYPE_NAME("palmetto-bmc"), @@ -1277,6 +1350,10 @@ static const TypeInfo aspeed_machine_types[] = { .name = MACHINE_TYPE_NAME("fuji-bmc"), .parent = TYPE_ASPEED_MACHINE, .class_init = aspeed_machine_fuji_class_init, + }, { + .name = MACHINE_TYPE_NAME("bletchley-bmc"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_bletchley_class_init, }, { .name = TYPE_ASPEED_MACHINE, .parent = TYPE_MACHINE,
Add the 'bletchley-bmc' machine type based on the kernel DTS[1] and hardware schematics available to me. The i2c model is as complete as the current QEMU models support, but in some cases I substituted devices that are close enough for present functionality. Strap registers are kept the same as the AST2600-EVB until I'm able to confirm correct values with physical hardware. This has been tested with an openbmc image built from [2] plus a kernel patch[3] for the SPI flash module. 1. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm/boot/dts/aspeed-bmc-facebook-bletchley.dts?id=a8c729e966c4e9d033242d948b0e53c2a62d32e2 2. https://github.com/openbmc/openbmc/commit/b9432b980d7f63f7512ffbcc7124386ba896dfc6 3. https://github.com/openbmc/linux/commit/25b566b9a9d7f5d4f10c1b7304007bdb286eefd7 Signed-off-by: Patrick Williams <patrick@stwcx.xyz> --- hw/arm/aspeed.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+)