diff mbox series

[4/4] aspeed: sonorapass: enable pca954x muxes

Message ID 20210518194118.755410-5-venture@google.com
State New
Headers show
Series With the pca954x i2c mux available, enable it on aspeed and nuvoton BMC boards. | expand

Commit Message

Patrick Venture May 18, 2021, 7:41 p.m. UTC
Enables the pca954x muxes in the bmc board configuration.

Signed-off-by: Patrick Venture <venture@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
---
 hw/arm/aspeed.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Comments

Joel Stanley May 18, 2021, 11:27 p.m. UTC | #1
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
>
Patrick Venture May 19, 2021, 5:18 p.m. UTC | #2
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
> >
Patrick Venture June 8, 2021, 7:55 p.m. UTC | #3
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
> > >
Joel Stanley June 9, 2021, 1:58 a.m. UTC | #4
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
C├ędric Le Goater June 9, 2021, 5:23 a.m. UTC | #5
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 mbox series

Patch

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)