qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm/aspeed: rainier: Add i2c eeproms and muxes
@ 2021-06-25  5:06 Joel Stanley
  2021-06-25  8:36 ` Cédric Le Goater
  2021-06-28  4:32 ` Andrew Jeffery
  0 siblings, 2 replies; 5+ messages in thread
From: Joel Stanley @ 2021-06-25  5:06 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Andrew Jeffery
  Cc: Patrick Venture, qemu-arm, qemu-devel

These are the devices documented by the Rainier device tree. With this
we can see the guest discovering the multiplexers and probing the eeprom
devices:

 i2c i2c-2: Added multiplexed i2c bus 16
 i2c i2c-2: Added multiplexed i2c bus 17
 i2c i2c-2: Added multiplexed i2c bus 18
 i2c i2c-2: Added multiplexed i2c bus 19
 i2c-mux-gpio i2cmux: 4 port mux on 1e78a180.i2c-bus adapter
 at24 20-0050: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
 i2c i2c-4: Added multiplexed i2c bus 20
 at24 21-0051: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
 i2c i2c-4: Added multiplexed i2c bus 21
 at24 22-0052: 8192 byte 24c64 EEPROM, writable, 1 bytes/write

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 hw/arm/aspeed.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 1301e8fdffb2..7ed22294c6eb 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -677,6 +677,10 @@ static void g220a_bmc_i2c_init(AspeedMachineState *bmc)
 static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
 {
     AspeedSoCState *soc = &bmc->soc;
+    I2CSlave *i2c_mux;
+
+    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 0), 0x51,
+                          g_malloc0(32 * 1024));
 
     /* The rainier expects a TMP275 but a TMP105 is compatible */
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), TYPE_TMP105,
@@ -685,11 +689,25 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
                      0x49);
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), TYPE_TMP105,
                      0x4a);
+    i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4),
+                                      "pca9546", 0x70);
+    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 0), 0x50,
+                          g_malloc0(64 * 1024));
+    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 1), 0x51,
+                          g_malloc0(64 * 1024));
+    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 2), 0x52,
+                          g_malloc0(64 * 1024));
 
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 5), TYPE_TMP105,
                      0x48);
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 5), TYPE_TMP105,
                      0x49);
+    i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 5),
+                                      "pca9546", 0x70);
+    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 0), 0x50,
+                          g_malloc0(64 * 1024));
+    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 1), 0x51,
+                          g_malloc0(64 * 1024));
 
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), TYPE_TMP105,
                      0x48);
@@ -697,6 +715,16 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
                      0x4a);
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), TYPE_TMP105,
                      0x4b);
+    i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6),
+                                      "pca9546", 0x70);
+    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 0), 0x50,
+                          g_malloc0(64 * 1024));
+    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 1), 0x51,
+                          g_malloc0(64 * 1024));
+    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 2), 0x50,
+                          g_malloc0(64 * 1024));
+    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 3), 0x51,
+                          g_malloc0(64 * 1024));
 
     /* Bus 7: TODO dps310@76 */
     /* Bus 7: TODO max31785@52 */
@@ -704,11 +732,19 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
     /* Bus 7: TODO si7021-a20@20 */
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7), TYPE_TMP105,
                      0x48);
+    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 7), 0x50,
+                          g_malloc0(64 * 1024));
+    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 7), 0x51,
+                          g_malloc0(64 * 1024));
 
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8), TYPE_TMP105,
                      0x48);
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8), TYPE_TMP105,
                      0x4a);
+    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 8), 0x50,
+                          g_malloc0(64 * 1024));
+    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 8), 0x51,
+                          g_malloc0(64 * 1024));
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8), "pca9552", 0x61);
     /* Bus 8: ucd90320@11 */
     /* Bus 8: ucd90320@b */
@@ -716,14 +752,34 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
 
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), "tmp423", 0x4c);
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), "tmp423", 0x4d);
+    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 9), 0x50,
+                          g_malloc0(128 * 1024));
 
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 10), "tmp423", 0x4c);
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 10), "tmp423", 0x4d);
+    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 10), 0x50,
+                          g_malloc0(128 * 1024));
 
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 11), TYPE_TMP105,
                      0x48);
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 11), TYPE_TMP105,
                      0x49);
+    i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 11),
+                                      "pca9546", 0x70);
+    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 0), 0x50,
+                          g_malloc0(64 * 1024));
+    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 1), 0x51,
+                          g_malloc0(64 * 1024));
+
+
+    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 13), 0x50,
+                          g_malloc0(64 * 1024));
+
+    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 14), 0x50,
+                          g_malloc0(64 * 1024));
+
+    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 15), 0x50,
+                          g_malloc0(64 * 1024));
 }
 
 static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm/aspeed: rainier: Add i2c eeproms and muxes
  2021-06-25  5:06 [PATCH] arm/aspeed: rainier: Add i2c eeproms and muxes Joel Stanley
@ 2021-06-25  8:36 ` Cédric Le Goater
  2021-06-28  4:32 ` Andrew Jeffery
  1 sibling, 0 replies; 5+ messages in thread
From: Cédric Le Goater @ 2021-06-25  8:36 UTC (permalink / raw)
  To: Joel Stanley, Peter Maydell, Andrew Jeffery
  Cc: Patrick Venture, qemu-arm, qemu-devel

On 6/25/21 7:06 AM, Joel Stanley wrote:
> These are the devices documented by the Rainier device tree. With this
> we can see the guest discovering the multiplexers and probing the eeprom
> devices:
> 
>  i2c i2c-2: Added multiplexed i2c bus 16
>  i2c i2c-2: Added multiplexed i2c bus 17
>  i2c i2c-2: Added multiplexed i2c bus 18
>  i2c i2c-2: Added multiplexed i2c bus 19
>  i2c-mux-gpio i2cmux: 4 port mux on 1e78a180.i2c-bus adapter
>  at24 20-0050: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
>  i2c i2c-4: Added multiplexed i2c bus 20
>  at24 21-0051: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
>  i2c i2c-4: Added multiplexed i2c bus 21
>  at24 22-0052: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>


Reviewed-by: Cédric Le Goater <clg@kaod.org>


Thanks,

C. 


> ---
>  hw/arm/aspeed.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 1301e8fdffb2..7ed22294c6eb 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -677,6 +677,10 @@ static void g220a_bmc_i2c_init(AspeedMachineState *bmc)
>  static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
>  {
>      AspeedSoCState *soc = &bmc->soc;
> +    I2CSlave *i2c_mux;
> +
> +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 0), 0x51,
> +                          g_malloc0(32 * 1024));
>  
>      /* The rainier expects a TMP275 but a TMP105 is compatible */
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), TYPE_TMP105,
> @@ -685,11 +689,25 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
>                       0x49);
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), TYPE_TMP105,
>                       0x4a);
> +    i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4),
> +                                      "pca9546", 0x70);
> +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 0), 0x50,
> +                          g_malloc0(64 * 1024));
> +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 1), 0x51,
> +                          g_malloc0(64 * 1024));
> +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 2), 0x52,
> +                          g_malloc0(64 * 1024));
>  
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 5), TYPE_TMP105,
>                       0x48);
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 5), TYPE_TMP105,
>                       0x49);
> +    i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 5),
> +                                      "pca9546", 0x70);
> +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 0), 0x50,
> +                          g_malloc0(64 * 1024));
> +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 1), 0x51,
> +                          g_malloc0(64 * 1024));
>  
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), TYPE_TMP105,
>                       0x48);
> @@ -697,6 +715,16 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
>                       0x4a);
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), TYPE_TMP105,
>                       0x4b);
> +    i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6),
> +                                      "pca9546", 0x70);
> +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 0), 0x50,
> +                          g_malloc0(64 * 1024));
> +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 1), 0x51,
> +                          g_malloc0(64 * 1024));
> +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 2), 0x50,
> +                          g_malloc0(64 * 1024));
> +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 3), 0x51,
> +                          g_malloc0(64 * 1024));
>  
>      /* Bus 7: TODO dps310@76 */
>      /* Bus 7: TODO max31785@52 */
> @@ -704,11 +732,19 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
>      /* Bus 7: TODO si7021-a20@20 */
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7), TYPE_TMP105,
>                       0x48);
> +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 7), 0x50,
> +                          g_malloc0(64 * 1024));
> +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 7), 0x51,
> +                          g_malloc0(64 * 1024));
>  
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8), TYPE_TMP105,
>                       0x48);
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8), TYPE_TMP105,
>                       0x4a);
> +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 8), 0x50,
> +                          g_malloc0(64 * 1024));
> +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 8), 0x51,
> +                          g_malloc0(64 * 1024));
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8), "pca9552", 0x61);
>      /* Bus 8: ucd90320@11 */
>      /* Bus 8: ucd90320@b */
> @@ -716,14 +752,34 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
>  
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), "tmp423", 0x4c);
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), "tmp423", 0x4d);
> +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 9), 0x50,
> +                          g_malloc0(128 * 1024));
>  
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 10), "tmp423", 0x4c);
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 10), "tmp423", 0x4d);
> +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 10), 0x50,
> +                          g_malloc0(128 * 1024));
>  
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 11), TYPE_TMP105,
>                       0x48);
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 11), TYPE_TMP105,
>                       0x49);
> +    i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 11),
> +                                      "pca9546", 0x70);
> +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 0), 0x50,
> +                          g_malloc0(64 * 1024));
> +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 1), 0x51,
> +                          g_malloc0(64 * 1024));
> +
> +
> +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 13), 0x50,
> +                          g_malloc0(64 * 1024));
> +
> +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 14), 0x50,
> +                          g_malloc0(64 * 1024));
> +
> +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 15), 0x50,
> +                          g_malloc0(64 * 1024));
>  }
>  
>  static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
> 



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm/aspeed: rainier: Add i2c eeproms and muxes
  2021-06-25  5:06 [PATCH] arm/aspeed: rainier: Add i2c eeproms and muxes Joel Stanley
  2021-06-25  8:36 ` Cédric Le Goater
@ 2021-06-28  4:32 ` Andrew Jeffery
  2021-06-28  4:49   ` Joel Stanley
  1 sibling, 1 reply; 5+ messages in thread
From: Andrew Jeffery @ 2021-06-28  4:32 UTC (permalink / raw)
  To: Joel Stanley, Cédric Le Goater, Peter Maydell
  Cc: Patrick Venture, qemu-arm, Cameron Esfahani via



On Fri, 25 Jun 2021, at 14:36, Joel Stanley wrote:
> These are the devices documented by the Rainier device tree. With this
> we can see the guest discovering the multiplexers and probing the eeprom
> devices:
> 
>  i2c i2c-2: Added multiplexed i2c bus 16
>  i2c i2c-2: Added multiplexed i2c bus 17
>  i2c i2c-2: Added multiplexed i2c bus 18
>  i2c i2c-2: Added multiplexed i2c bus 19
>  i2c-mux-gpio i2cmux: 4 port mux on 1e78a180.i2c-bus adapter
>  at24 20-0050: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
>  i2c i2c-4: Added multiplexed i2c bus 20
>  at24 21-0051: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
>  i2c i2c-4: Added multiplexed i2c bus 21
>  at24 22-0052: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  hw/arm/aspeed.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 1301e8fdffb2..7ed22294c6eb 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -677,6 +677,10 @@ static void g220a_bmc_i2c_init(AspeedMachineState *bmc)
>  static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
>  {
>      AspeedSoCState *soc = &bmc->soc;
> +    I2CSlave *i2c_mux;
> +
> +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 0), 0x51,
> +                          g_malloc0(32 * 1024));
>  
>      /* The rainier expects a TMP275 but a TMP105 is compatible */
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), TYPE_TMP105,
> @@ -685,11 +689,25 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
>                       0x49);
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), TYPE_TMP105,
>                       0x4a);
> +    i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4),
> +                                      "pca9546", 0x70);
> +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 0), 0x50,
> +                          g_malloc0(64 * 1024));

The EEPROMs described in the Rainier devicetree are Atmel AT2x devices and
not SMBus EEPROMs. The SMBus EEPROM model uses SMBus block reads and 
writes which are not what the AT2x driver issues. If you try to read or 
write data under Linux from the EEPROMs in this patch you just get 
corrupt results. So as far as I can see the patch needs to be reworked.

Andrew


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm/aspeed: rainier: Add i2c eeproms and muxes
  2021-06-28  4:32 ` Andrew Jeffery
@ 2021-06-28  4:49   ` Joel Stanley
  2021-06-28  4:50     ` Joel Stanley
  0 siblings, 1 reply; 5+ messages in thread
From: Joel Stanley @ 2021-06-28  4:49 UTC (permalink / raw)
  To: Andrew Jeffery
  Cc: Cameron Esfahani via, Peter Maydell, qemu-arm,
	Cédric Le Goater, Patrick Venture

On Mon, 28 Jun 2021 at 04:33, Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
>
> On Fri, 25 Jun 2021, at 14:36, Joel Stanley wrote:
> > These are the devices documented by the Rainier device tree. With this
> > we can see the guest discovering the multiplexers and probing the eeprom
> > devices:
> >
> >  i2c i2c-2: Added multiplexed i2c bus 16
> >  i2c i2c-2: Added multiplexed i2c bus 17
> >  i2c i2c-2: Added multiplexed i2c bus 18
> >  i2c i2c-2: Added multiplexed i2c bus 19
> >  i2c-mux-gpio i2cmux: 4 port mux on 1e78a180.i2c-bus adapter
> >  at24 20-0050: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
> >  i2c i2c-4: Added multiplexed i2c bus 20
> >  at24 21-0051: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
> >  i2c i2c-4: Added multiplexed i2c bus 21
> >  at24 22-0052: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
> >
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> >  hw/arm/aspeed.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 56 insertions(+)
> >
> > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> > index 1301e8fdffb2..7ed22294c6eb 100644
> > --- a/hw/arm/aspeed.c
> > +++ b/hw/arm/aspeed.c
> > @@ -677,6 +677,10 @@ static void g220a_bmc_i2c_init(AspeedMachineState *bmc)
> >  static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
> >  {
> >      AspeedSoCState *soc = &bmc->soc;
> > +    I2CSlave *i2c_mux;
> > +
> > +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 0), 0x51,
> > +                          g_malloc0(32 * 1024));
> >
> >      /* The rainier expects a TMP275 but a TMP105 is compatible */
> >      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), TYPE_TMP105,
> > @@ -685,11 +689,25 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
> >                       0x49);
> >      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), TYPE_TMP105,
> >                       0x4a);
> > +    i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4),
> > +                                      "pca9546", 0x70);
> > +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 0), 0x50,
> > +                          g_malloc0(64 * 1024));
>
> The EEPROMs described in the Rainier devicetree are Atmel AT2x devices and
> not SMBus EEPROMs. The SMBus EEPROM model uses SMBus block reads and
> writes which are not what the AT2x driver issues. If you try to read or
> write data under Linux from the EEPROMs in this patch you just get
> corrupt results. So as far as I can see the patch needs to be reworked.

That's fine, these are just there so the drivers can probe. As you can
see the devices have no data in them either, so it's nowhere near an
incomplete model.

If you want to extend the modelling to have the correct VPD data, and
allow writes where applicable, that would be welcome.

Cheers,

Joel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm/aspeed: rainier: Add i2c eeproms and muxes
  2021-06-28  4:49   ` Joel Stanley
@ 2021-06-28  4:50     ` Joel Stanley
  0 siblings, 0 replies; 5+ messages in thread
From: Joel Stanley @ 2021-06-28  4:50 UTC (permalink / raw)
  To: Andrew Jeffery
  Cc: Cameron Esfahani via, Peter Maydell, qemu-arm,
	Cédric Le Goater, Patrick Venture

On Mon, 28 Jun 2021 at 04:49, Joel Stanley <joel@jms.id.au> wrote:
>
> On Mon, 28 Jun 2021 at 04:33, Andrew Jeffery <andrew@aj.id.au> wrote:
> >
> >
> >
> > On Fri, 25 Jun 2021, at 14:36, Joel Stanley wrote:
> > > These are the devices documented by the Rainier device tree. With this
> > > we can see the guest discovering the multiplexers and probing the eeprom
> > > devices:
> > >
> > >  i2c i2c-2: Added multiplexed i2c bus 16
> > >  i2c i2c-2: Added multiplexed i2c bus 17
> > >  i2c i2c-2: Added multiplexed i2c bus 18
> > >  i2c i2c-2: Added multiplexed i2c bus 19
> > >  i2c-mux-gpio i2cmux: 4 port mux on 1e78a180.i2c-bus adapter
> > >  at24 20-0050: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
> > >  i2c i2c-4: Added multiplexed i2c bus 20
> > >  at24 21-0051: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
> > >  i2c i2c-4: Added multiplexed i2c bus 21
> > >  at24 22-0052: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
> > >
> > > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > > ---
> > >  hw/arm/aspeed.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 56 insertions(+)
> > >
> > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> > > index 1301e8fdffb2..7ed22294c6eb 100644
> > > --- a/hw/arm/aspeed.c
> > > +++ b/hw/arm/aspeed.c
> > > @@ -677,6 +677,10 @@ static void g220a_bmc_i2c_init(AspeedMachineState *bmc)
> > >  static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
> > >  {
> > >      AspeedSoCState *soc = &bmc->soc;
> > > +    I2CSlave *i2c_mux;
> > > +
> > > +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 0), 0x51,
> > > +                          g_malloc0(32 * 1024));
> > >
> > >      /* The rainier expects a TMP275 but a TMP105 is compatible */
> > >      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), TYPE_TMP105,
> > > @@ -685,11 +689,25 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
> > >                       0x49);
> > >      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), TYPE_TMP105,
> > >                       0x4a);
> > > +    i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4),
> > > +                                      "pca9546", 0x70);
> > > +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 0), 0x50,
> > > +                          g_malloc0(64 * 1024));
> >
> > The EEPROMs described in the Rainier devicetree are Atmel AT2x devices and
> > not SMBus EEPROMs. The SMBus EEPROM model uses SMBus block reads and
> > writes which are not what the AT2x driver issues. If you try to read or
> > write data under Linux from the EEPROMs in this patch you just get
> > corrupt results. So as far as I can see the patch needs to be reworked.
>
> That's fine, these are just there so the drivers can probe. As you can
> see the devices have no data in them either, so it's nowhere near an
> incomplete model.

nowhere near a complete model.

>
> If you want to extend the modelling to have the correct VPD data, and
> allow writes where applicable, that would be welcome.
>
> Cheers,
>
> Joel


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-06-28  4:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-25  5:06 [PATCH] arm/aspeed: rainier: Add i2c eeproms and muxes Joel Stanley
2021-06-25  8:36 ` Cédric Le Goater
2021-06-28  4:32 ` Andrew Jeffery
2021-06-28  4:49   ` Joel Stanley
2021-06-28  4:50     ` Joel Stanley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).