| Message ID | 20210518194118.755410-5-venture@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | With the pca954x i2c mux available, enable it on aspeed and nuvoton BMC boards. | expand |
On Tue, 18 May 2021 at 19:41, Patrick Venture <venture@google.com> wrote: > > Enables the pca954x muxes in the bmc board configuration. > > Signed-off-by: Patrick Venture <venture@google.com> > Reviewed-by: Hao Wu <wuhaotsh@google.com> Not sure about this one, there's no device tree for it in Linux. > --- > hw/arm/aspeed.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index 35a28b0e8b..27fd51980c 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -541,14 +541,16 @@ static void swift_bmc_i2c_init(AspeedMachineState *bmc) > > static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc) > { > + I2CSlave *i2c_mux; > AspeedSoCState *soc = &bmc->soc; > > /* bus 2 : */ > i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 2), "tmp105", 0x48); > i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 2), "tmp105", 0x49); > - /* bus 2 : pca9546 @ 0x73 */ > + i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 2), "pca9546", 0x73); > > - /* bus 3 : pca9548 @ 0x70 */ > + /* bus 3 : */ > + i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3), "pca9548", 0x70); > > /* bus 4 : */ > uint8_t *eeprom4_54 = g_malloc0(8 * 1024); > @@ -562,7 +564,7 @@ static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc) > /* bus 6 : */ > i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), "tmp105", 0x48); > i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), "tmp105", 0x49); > - /* bus 6 : pca9546 @ 0x73 */ > + i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), "pca9546", 0x73); > > /* bus 8 : */ > uint8_t *eeprom8_56 = g_malloc0(8 * 1024); > @@ -573,14 +575,12 @@ static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc) > /* bus 8 : adc128d818 @ 0x1d */ > /* bus 8 : adc128d818 @ 0x1f */ > > - /* > - * bus 13 : pca9548 @ 0x71 > - * - channel 3: > - * - tmm421 @ 0x4c > - * - tmp421 @ 0x4e > - * - tmp421 @ 0x4f > - */ > - > + /* bus 13 : */ > + i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 13), > + "pca9548", 0x71); > + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp421", 0x4c); > + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp421", 0x4e); > + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp421", 0x4f); > } > > static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc) > -- > 2.31.1.751.gd2f1c929bd-goog >
On Tue, May 18, 2021 at 4:27 PM Joel Stanley <joel@jms.id.au> wrote: > > On Tue, 18 May 2021 at 19:41, Patrick Venture <venture@google.com> wrote: > > > > Enables the pca954x muxes in the bmc board configuration. > > > > Signed-off-by: Patrick Venture <venture@google.com> > > Reviewed-by: Hao Wu <wuhaotsh@google.com> > > Not sure about this one, there's no device tree for it in Linux. Yeah, this was just a pick-up from grepping other BMC boards. I added these going off the comment alone. I'd be okay with dropping this in the series. > > > --- > > hw/arm/aspeed.c | 22 +++++++++++----------- > > 1 file changed, 11 insertions(+), 11 deletions(-) > > > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > > index 35a28b0e8b..27fd51980c 100644 > > --- a/hw/arm/aspeed.c > > +++ b/hw/arm/aspeed.c > > @@ -541,14 +541,16 @@ static void swift_bmc_i2c_init(AspeedMachineState *bmc) > > > > static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc) > > { > > + I2CSlave *i2c_mux; > > AspeedSoCState *soc = &bmc->soc; > > > > /* bus 2 : */ > > i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 2), "tmp105", 0x48); > > i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 2), "tmp105", 0x49); > > - /* bus 2 : pca9546 @ 0x73 */ > > + i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 2), "pca9546", 0x73); > > > > - /* bus 3 : pca9548 @ 0x70 */ > > + /* bus 3 : */ > > + i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3), "pca9548", 0x70); > > > > /* bus 4 : */ > > uint8_t *eeprom4_54 = g_malloc0(8 * 1024); > > @@ -562,7 +564,7 @@ static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc) > > /* bus 6 : */ > > i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), "tmp105", 0x48); > > i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), "tmp105", 0x49); > > - /* bus 6 : pca9546 @ 0x73 */ > > + i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), "pca9546", 0x73); > > > > /* bus 8 : */ > > uint8_t *eeprom8_56 = g_malloc0(8 * 1024); > > @@ -573,14 +575,12 @@ static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc) > > /* bus 8 : adc128d818 @ 0x1d */ > > /* bus 8 : adc128d818 @ 0x1f */ > > > > - /* > > - * bus 13 : pca9548 @ 0x71 > > - * - channel 3: > > - * - tmm421 @ 0x4c > > - * - tmp421 @ 0x4e > > - * - tmp421 @ 0x4f > > - */ > > - > > + /* bus 13 : */ > > + i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 13), > > + "pca9548", 0x71); > > + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp421", 0x4c); > > + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp421", 0x4e); > > + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp421", 0x4f); > > } > > > > static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc) > > -- > > 2.31.1.751.gd2f1c929bd-goog > >
On Wed, May 19, 2021 at 10:18 AM Patrick Venture <venture@google.com> wrote: > > On Tue, May 18, 2021 at 4:27 PM Joel Stanley <joel@jms.id.au> wrote: > > > > On Tue, 18 May 2021 at 19:41, Patrick Venture <venture@google.com> wrote: > > > > > > Enables the pca954x muxes in the bmc board configuration. > > > > > > Signed-off-by: Patrick Venture <venture@google.com> > > > Reviewed-by: Hao Wu <wuhaotsh@google.com> > > > > Not sure about this one, there's no device tree for it in Linux. > > Yeah, this was just a pick-up from grepping other BMC boards. I added > these going off the comment alone. I'd be okay with dropping this in > the series. In this case, the number of patches changed within a version change -- should I start a fresh series or just bump the version and drop the last patch? > > > > > > --- > > > hw/arm/aspeed.c | 22 +++++++++++----------- > > > 1 file changed, 11 insertions(+), 11 deletions(-) > > > > > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > > > index 35a28b0e8b..27fd51980c 100644 > > > --- a/hw/arm/aspeed.c > > > +++ b/hw/arm/aspeed.c > > > @@ -541,14 +541,16 @@ static void swift_bmc_i2c_init(AspeedMachineState *bmc) > > > > > > static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc) > > > { > > > + I2CSlave *i2c_mux; > > > AspeedSoCState *soc = &bmc->soc; > > > > > > /* bus 2 : */ > > > i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 2), "tmp105", 0x48); > > > i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 2), "tmp105", 0x49); > > > - /* bus 2 : pca9546 @ 0x73 */ > > > + i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 2), "pca9546", 0x73); > > > > > > - /* bus 3 : pca9548 @ 0x70 */ > > > + /* bus 3 : */ > > > + i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3), "pca9548", 0x70); > > > > > > /* bus 4 : */ > > > uint8_t *eeprom4_54 = g_malloc0(8 * 1024); > > > @@ -562,7 +564,7 @@ static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc) > > > /* bus 6 : */ > > > i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), "tmp105", 0x48); > > > i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), "tmp105", 0x49); > > > - /* bus 6 : pca9546 @ 0x73 */ > > > + i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), "pca9546", 0x73); > > > > > > /* bus 8 : */ > > > uint8_t *eeprom8_56 = g_malloc0(8 * 1024); > > > @@ -573,14 +575,12 @@ static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc) > > > /* bus 8 : adc128d818 @ 0x1d */ > > > /* bus 8 : adc128d818 @ 0x1f */ > > > > > > - /* > > > - * bus 13 : pca9548 @ 0x71 > > > - * - channel 3: > > > - * - tmm421 @ 0x4c > > > - * - tmp421 @ 0x4e > > > - * - tmp421 @ 0x4f > > > - */ > > > - > > > + /* bus 13 : */ > > > + i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 13), > > > + "pca9548", 0x71); > > > + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp421", 0x4c); > > > + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp421", 0x4e); > > > + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp421", 0x4f); > > > } > > > > > > static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc) > > > -- > > > 2.31.1.751.gd2f1c929bd-goog > > >
On Tue, 8 Jun 2021 at 19:56, Patrick Venture <venture@google.com> wrote: > > On Wed, May 19, 2021 at 10:18 AM Patrick Venture <venture@google.com> wrote: > > > > On Tue, May 18, 2021 at 4:27 PM Joel Stanley <joel@jms.id.au> wrote: > > > > > > On Tue, 18 May 2021 at 19:41, Patrick Venture <venture@google.com> wrote: > > > > > > > > Enables the pca954x muxes in the bmc board configuration. > > > > > > > > Signed-off-by: Patrick Venture <venture@google.com> > > > > Reviewed-by: Hao Wu <wuhaotsh@google.com> > > > > > > Not sure about this one, there's no device tree for it in Linux. > > > > Yeah, this was just a pick-up from grepping other BMC boards. I added > > these going off the comment alone. I'd be okay with dropping this in > > the series. > > In this case, the number of patches changed within a version change -- > should I start a fresh series or just bump the version and drop the > last patch? I wasn't saying we shouldn't include this change - it's good. I just didn't have any information to say whether it was correct or not. I see you chose to resend without this one, lets get Cedric to merge those patches. Cheers, Joel
On 6/9/21 3:58 AM, Joel Stanley wrote: > On Tue, 8 Jun 2021 at 19:56, Patrick Venture <venture@google.com> wrote: >> >> On Wed, May 19, 2021 at 10:18 AM Patrick Venture <venture@google.com> wrote: >>> >>> On Tue, May 18, 2021 at 4:27 PM Joel Stanley <joel@jms.id.au> wrote: >>>> >>>> On Tue, 18 May 2021 at 19:41, Patrick Venture <venture@google.com> wrote: >>>>> >>>>> Enables the pca954x muxes in the bmc board configuration. >>>>> >>>>> Signed-off-by: Patrick Venture <venture@google.com> >>>>> Reviewed-by: Hao Wu <wuhaotsh@google.com> >>>> >>>> Not sure about this one, there's no device tree for it in Linux. >>> >>> Yeah, this was just a pick-up from grepping other BMC boards. I added >>> these going off the comment alone. I'd be okay with dropping this in >>> the series. >> >> In this case, the number of patches changed within a version change -- >> should I start a fresh series or just bump the version and drop the >> last patch? > > I wasn't saying we shouldn't include this change - it's good. I just > didn't have any information to say whether it was correct or not. > > I see you chose to resend without this one, lets get Cedric to merge > those patches. I took these patches in the aspeed-6.1 branch : hw/arm: add quanta-gbs-bmc machine hw/arm: quanta-gbs-bmc add i2c comments hw/arm: gsj add i2c comments hw/arm: gsj add pca9548 hw/arm: quanta-q71l add pca954x muxes aspeed: sonorapass: enable pca954x muxes Peter, I can include them in an aspeed PR. Thanks, C.
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 35a28b0e8b..27fd51980c 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -541,14 +541,16 @@ static void swift_bmc_i2c_init(AspeedMachineState *bmc) static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc) { + I2CSlave *i2c_mux; AspeedSoCState *soc = &bmc->soc; /* bus 2 : */ i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 2), "tmp105", 0x48); i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 2), "tmp105", 0x49); - /* bus 2 : pca9546 @ 0x73 */ + i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 2), "pca9546", 0x73); - /* bus 3 : pca9548 @ 0x70 */ + /* bus 3 : */ + i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3), "pca9548", 0x70); /* bus 4 : */ uint8_t *eeprom4_54 = g_malloc0(8 * 1024); @@ -562,7 +564,7 @@ static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc) /* bus 6 : */ i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), "tmp105", 0x48); i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), "tmp105", 0x49); - /* bus 6 : pca9546 @ 0x73 */ + i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), "pca9546", 0x73); /* bus 8 : */ uint8_t *eeprom8_56 = g_malloc0(8 * 1024); @@ -573,14 +575,12 @@ static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc) /* bus 8 : adc128d818 @ 0x1d */ /* bus 8 : adc128d818 @ 0x1f */ - /* - * bus 13 : pca9548 @ 0x71 - * - channel 3: - * - tmm421 @ 0x4c - * - tmp421 @ 0x4e - * - tmp421 @ 0x4f - */ - + /* bus 13 : */ + i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 13), + "pca9548", 0x71); + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp421", 0x4c); + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp421", 0x4e); + i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp421", 0x4f); } static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)