[PATCH v3] hw: m25p80: allow write_enable latch get/set

Iris Chen via posted 1 patch 1 week ago
hw/block/m25p80.c              |  1 +
tests/qtest/aspeed_gpio-test.c | 40 +++++++------------------------
tests/qtest/aspeed_smc-test.c  | 43 ++++++++++++++++++++++++++++++++++
tests/qtest/libqtest.c         | 24 +++++++++++++++++++
tests/qtest/libqtest.h         | 22 +++++++++++++++++
5 files changed, 98 insertions(+), 32 deletions(-)
[PATCH v3] hw: m25p80: allow write_enable latch get/set
Posted by Iris Chen via 1 week ago
The write_enable latch property is not currently exposed.
This commit makes it a modifiable property.

Signed-off-by: Iris Chen <irischenlj@fb.com>
---
v3: Addressed comments by Peter and Cedric.
v2: Ran ./scripts/checkpatch.pl on the patch and added a description. Fixed comments regarding DEFINE_PROP_BOOL.

 hw/block/m25p80.c              |  1 +
 tests/qtest/aspeed_gpio-test.c | 40 +++++++------------------------
 tests/qtest/aspeed_smc-test.c  | 43 ++++++++++++++++++++++++++++++++++
 tests/qtest/libqtest.c         | 24 +++++++++++++++++++
 tests/qtest/libqtest.h         | 22 +++++++++++++++++
 5 files changed, 98 insertions(+), 32 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 430d1298a8..4283b990af 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -1558,6 +1558,7 @@ static int m25p80_pre_save(void *opaque)
 
 static Property m25p80_properties[] = {
     /* This is default value for Micron flash */
+    DEFINE_PROP_BOOL("write-enable", Flash, write_enable, false),
     DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
     DEFINE_PROP_UINT8("spansion-cr1nv", Flash, spansion_cr1nv, 0x0),
     DEFINE_PROP_UINT8("spansion-cr2nv", Flash, spansion_cr2nv, 0x8),
diff --git a/tests/qtest/aspeed_gpio-test.c b/tests/qtest/aspeed_gpio-test.c
index c1003f2d1b..bac63e8742 100644
--- a/tests/qtest/aspeed_gpio-test.c
+++ b/tests/qtest/aspeed_gpio-test.c
@@ -28,30 +28,6 @@
 #include "qapi/qmp/qdict.h"
 #include "libqtest-single.h"
 
-static bool qom_get_bool(QTestState *s, const char *path, const char *property)
-{
-    QDict *r;
-    bool b;
-
-    r = qtest_qmp(s, "{ 'execute': 'qom-get', 'arguments': "
-                     "{ 'path': %s, 'property': %s } }", path, property);
-    b = qdict_get_bool(r, "return");
-    qobject_unref(r);
-
-    return b;
-}
-
-static void qom_set_bool(QTestState *s, const char *path, const char *property,
-                         bool value)
-{
-    QDict *r;
-
-    r = qtest_qmp(s, "{ 'execute': 'qom-set', 'arguments': "
-                     "{ 'path': %s, 'property': %s, 'value': %i } }",
-                     path, property, value);
-    qobject_unref(r);
-}
-
 static void test_set_colocated_pins(const void *data)
 {
     QTestState *s = (QTestState *)data;
@@ -60,14 +36,14 @@ static void test_set_colocated_pins(const void *data)
      * gpioV4-7 occupy bits within a single 32-bit value, so we want to make
      * sure that modifying one doesn't affect the other.
      */
-    qom_set_bool(s, "/machine/soc/gpio", "gpioV4", true);
-    qom_set_bool(s, "/machine/soc/gpio", "gpioV5", false);
-    qom_set_bool(s, "/machine/soc/gpio", "gpioV6", true);
-    qom_set_bool(s, "/machine/soc/gpio", "gpioV7", false);
-    g_assert(qom_get_bool(s, "/machine/soc/gpio", "gpioV4"));
-    g_assert(!qom_get_bool(s, "/machine/soc/gpio", "gpioV5"));
-    g_assert(qom_get_bool(s, "/machine/soc/gpio", "gpioV6"));
-    g_assert(!qom_get_bool(s, "/machine/soc/gpio", "gpioV7"));
+    qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV4", true);
+    qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV5", false);
+    qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV6", true);
+    qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV7", false);
+    g_assert(qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV4"));
+    g_assert(!qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV5"));
+    g_assert(qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV6"));
+    g_assert(!qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV7"));
 }
 
 int main(int argc, char **argv)
diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index 87b40a0ef1..ec233315e6 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -26,6 +26,7 @@
 #include "qemu/osdep.h"
 #include "qemu/bswap.h"
 #include "libqtest-single.h"
+#include "qemu/bitops.h"
 
 /*
  * ASPEED SPI Controller registers
@@ -40,6 +41,7 @@
 #define   CTRL_FREADMODE       0x1
 #define   CTRL_WRITEMODE       0x2
 #define   CTRL_USERMODE        0x3
+#define SR_WEL BIT(1)
 
 #define ASPEED_FMC_BASE    0x1E620000
 #define ASPEED_FLASH_BASE  0x20000000
@@ -49,6 +51,8 @@
  */
 enum {
     JEDEC_READ = 0x9f,
+    RDSR = 0x5,
+    WRDI = 0x4,
     BULK_ERASE = 0xc7,
     READ = 0x03,
     PP = 0x02,
@@ -348,6 +352,44 @@ static void test_write_page_mem(void)
     flash_reset();
 }
 
+static void test_read_status_reg(void)
+{
+    uint8_t r;
+
+    spi_conf(CONF_ENABLE_W0);
+
+    spi_ctrl_start_user();
+    writeb(ASPEED_FLASH_BASE, RDSR);
+    r = readb(ASPEED_FLASH_BASE);
+    spi_ctrl_stop_user();
+
+    g_assert_cmphex(r & SR_WEL, ==, 0);
+    g_assert(!qtest_qom_get_bool
+            (global_qtest, "/machine/soc/fmc/ssi.0/child[0]", "write-enable"));
+
+    spi_ctrl_start_user();
+    writeb(ASPEED_FLASH_BASE, WREN);
+    writeb(ASPEED_FLASH_BASE, RDSR);
+    r = readb(ASPEED_FLASH_BASE);
+    spi_ctrl_stop_user();
+
+    g_assert_cmphex(r & SR_WEL, ==, SR_WEL);
+    g_assert(qtest_qom_get_bool
+            (global_qtest, "/machine/soc/fmc/ssi.0/child[0]", "write-enable"));
+
+    spi_ctrl_start_user();
+    writeb(ASPEED_FLASH_BASE, WRDI);
+    writeb(ASPEED_FLASH_BASE, RDSR);
+    r = readb(ASPEED_FLASH_BASE);
+    spi_ctrl_stop_user();
+
+    g_assert_cmphex(r & SR_WEL, ==, 0);
+    g_assert(!qtest_qom_get_bool
+            (global_qtest, "/machine/soc/fmc/ssi.0/child[0]", "write-enable"));
+
+    flash_reset();
+}
+
 static char tmp_path[] = "/tmp/qtest.m25p80.XXXXXX";
 
 int main(int argc, char **argv)
@@ -373,6 +415,7 @@ int main(int argc, char **argv)
     qtest_add_func("/ast2400/smc/write_page", test_write_page);
     qtest_add_func("/ast2400/smc/read_page_mem", test_read_page_mem);
     qtest_add_func("/ast2400/smc/write_page_mem", test_write_page_mem);
+    qtest_add_func("/ast2400/smc/read_status_reg", test_read_status_reg);
 
     ret = g_test_run();
 
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 228357f1ea..a9904eba82 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1423,3 +1423,27 @@ void qtest_client_inproc_recv(void *opaque, const char *str)
     g_string_append(qts->rx, str);
     return;
 }
+
+void qtest_qom_set_bool(QTestState *s, const char *path, const char *property,
+                         bool value)
+{
+    QDict *r;
+
+    r = qtest_qmp(s, "{ 'execute': 'qom-set', 'arguments': "
+                     "{ 'path': %s, 'property': %s, 'value': %i } }",
+                     path, property, value);
+    qobject_unref(r);
+}
+
+bool qtest_qom_get_bool(QTestState *s, const char *path, const char *property)
+{
+    QDict *r;
+    bool b;
+
+    r = qtest_qmp(s, "{ 'execute': 'qom-get', 'arguments': "
+                     "{ 'path': %s, 'property': %s } }", path, property);
+    b = qdict_get_bool(r, "return");
+    qobject_unref(r);
+
+    return b;
+}
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 4ab0cad326..94b187837d 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -783,4 +783,26 @@ QTestState *qtest_inproc_init(QTestState **s, bool log, const char* arch,
                     void (*send)(void*, const char*));
 
 void qtest_client_inproc_recv(void *opaque, const char *str);
+
+/**
+ * qtest_qom_set_bool:
+ * @s: QTestState instance to operate on.
+ * @path: Path to the property being set.
+ * @property: Property being set.
+ * @value: Value to set the property.
+ *
+ * Set the property with passed in value.
+ */
+void qtest_qom_set_bool(QTestState *s, const char *path, const char *property,
+                         bool value);
+
+/**
+ * qtest_qom_get_bool:
+ * @s: QTestState instance to operate on.
+ * @path: Path to the property being retrieved.
+ * @property: Property from where the value is being retrieved.
+ *
+ * Returns: Value retrieved from property.
+ */
+bool qtest_qom_get_bool(QTestState *s, const char *path, const char *property);
 #endif
-- 
2.30.2
Re: [PATCH v3] hw: m25p80: allow write_enable latch get/set
Posted by Francisco Iglesias 6 days, 18 hours ago
On [2022 May 12] Thu 22:50:22, Iris Chen via wrote:
> The write_enable latch property is not currently exposed.
> This commit makes it a modifiable property.
> 
> Signed-off-by: Iris Chen <irischenlj@fb.com>
> ---
> v3: Addressed comments by Peter and Cedric.
> v2: Ran ./scripts/checkpatch.pl on the patch and added a description. Fixed comments regarding DEFINE_PROP_BOOL.
> 
>  hw/block/m25p80.c              |  1 +
>  tests/qtest/aspeed_gpio-test.c | 40 +++++++------------------------
>  tests/qtest/aspeed_smc-test.c  | 43 ++++++++++++++++++++++++++++++++++
>  tests/qtest/libqtest.c         | 24 +++++++++++++++++++
>  tests/qtest/libqtest.h         | 22 +++++++++++++++++
>  5 files changed, 98 insertions(+), 32 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 430d1298a8..4283b990af 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -1558,6 +1558,7 @@ static int m25p80_pre_save(void *opaque)
>  
>  static Property m25p80_properties[] = {
>      /* This is default value for Micron flash */
> +    DEFINE_PROP_BOOL("write-enable", Flash, write_enable, false),

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

>      DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
>      DEFINE_PROP_UINT8("spansion-cr1nv", Flash, spansion_cr1nv, 0x0),
>      DEFINE_PROP_UINT8("spansion-cr2nv", Flash, spansion_cr2nv, 0x8),
> diff --git a/tests/qtest/aspeed_gpio-test.c b/tests/qtest/aspeed_gpio-test.c
> index c1003f2d1b..bac63e8742 100644
> --- a/tests/qtest/aspeed_gpio-test.c
> +++ b/tests/qtest/aspeed_gpio-test.c
> @@ -28,30 +28,6 @@
>  #include "qapi/qmp/qdict.h"
>  #include "libqtest-single.h"
>  
> -static bool qom_get_bool(QTestState *s, const char *path, const char *property)
> -{
> -    QDict *r;
> -    bool b;
> -
> -    r = qtest_qmp(s, "{ 'execute': 'qom-get', 'arguments': "
> -                     "{ 'path': %s, 'property': %s } }", path, property);
> -    b = qdict_get_bool(r, "return");
> -    qobject_unref(r);
> -
> -    return b;
> -}
> -
> -static void qom_set_bool(QTestState *s, const char *path, const char *property,
> -                         bool value)
> -{
> -    QDict *r;
> -
> -    r = qtest_qmp(s, "{ 'execute': 'qom-set', 'arguments': "
> -                     "{ 'path': %s, 'property': %s, 'value': %i } }",
> -                     path, property, value);
> -    qobject_unref(r);
> -}
> -
>  static void test_set_colocated_pins(const void *data)
>  {
>      QTestState *s = (QTestState *)data;
> @@ -60,14 +36,14 @@ static void test_set_colocated_pins(const void *data)
>       * gpioV4-7 occupy bits within a single 32-bit value, so we want to make
>       * sure that modifying one doesn't affect the other.
>       */
> -    qom_set_bool(s, "/machine/soc/gpio", "gpioV4", true);
> -    qom_set_bool(s, "/machine/soc/gpio", "gpioV5", false);
> -    qom_set_bool(s, "/machine/soc/gpio", "gpioV6", true);
> -    qom_set_bool(s, "/machine/soc/gpio", "gpioV7", false);
> -    g_assert(qom_get_bool(s, "/machine/soc/gpio", "gpioV4"));
> -    g_assert(!qom_get_bool(s, "/machine/soc/gpio", "gpioV5"));
> -    g_assert(qom_get_bool(s, "/machine/soc/gpio", "gpioV6"));
> -    g_assert(!qom_get_bool(s, "/machine/soc/gpio", "gpioV7"));
> +    qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV4", true);
> +    qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV5", false);
> +    qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV6", true);
> +    qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV7", false);
> +    g_assert(qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV4"));
> +    g_assert(!qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV5"));
> +    g_assert(qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV6"));
> +    g_assert(!qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV7"));
>  }
>  
>  int main(int argc, char **argv)
> diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
> index 87b40a0ef1..ec233315e6 100644
> --- a/tests/qtest/aspeed_smc-test.c
> +++ b/tests/qtest/aspeed_smc-test.c
> @@ -26,6 +26,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/bswap.h"
>  #include "libqtest-single.h"
> +#include "qemu/bitops.h"
>  
>  /*
>   * ASPEED SPI Controller registers
> @@ -40,6 +41,7 @@
>  #define   CTRL_FREADMODE       0x1
>  #define   CTRL_WRITEMODE       0x2
>  #define   CTRL_USERMODE        0x3
> +#define SR_WEL BIT(1)
>  
>  #define ASPEED_FMC_BASE    0x1E620000
>  #define ASPEED_FLASH_BASE  0x20000000
> @@ -49,6 +51,8 @@
>   */
>  enum {
>      JEDEC_READ = 0x9f,
> +    RDSR = 0x5,
> +    WRDI = 0x4,
>      BULK_ERASE = 0xc7,
>      READ = 0x03,
>      PP = 0x02,
> @@ -348,6 +352,44 @@ static void test_write_page_mem(void)
>      flash_reset();
>  }
>  
> +static void test_read_status_reg(void)
> +{
> +    uint8_t r;
> +
> +    spi_conf(CONF_ENABLE_W0);
> +
> +    spi_ctrl_start_user();
> +    writeb(ASPEED_FLASH_BASE, RDSR);
> +    r = readb(ASPEED_FLASH_BASE);
> +    spi_ctrl_stop_user();
> +
> +    g_assert_cmphex(r & SR_WEL, ==, 0);
> +    g_assert(!qtest_qom_get_bool
> +            (global_qtest, "/machine/soc/fmc/ssi.0/child[0]", "write-enable"));
> +
> +    spi_ctrl_start_user();
> +    writeb(ASPEED_FLASH_BASE, WREN);
> +    writeb(ASPEED_FLASH_BASE, RDSR);
> +    r = readb(ASPEED_FLASH_BASE);
> +    spi_ctrl_stop_user();
> +
> +    g_assert_cmphex(r & SR_WEL, ==, SR_WEL);
> +    g_assert(qtest_qom_get_bool
> +            (global_qtest, "/machine/soc/fmc/ssi.0/child[0]", "write-enable"));
> +
> +    spi_ctrl_start_user();
> +    writeb(ASPEED_FLASH_BASE, WRDI);
> +    writeb(ASPEED_FLASH_BASE, RDSR);
> +    r = readb(ASPEED_FLASH_BASE);
> +    spi_ctrl_stop_user();
> +
> +    g_assert_cmphex(r & SR_WEL, ==, 0);
> +    g_assert(!qtest_qom_get_bool
> +            (global_qtest, "/machine/soc/fmc/ssi.0/child[0]", "write-enable"));
> +
> +    flash_reset();
> +}
> +
>  static char tmp_path[] = "/tmp/qtest.m25p80.XXXXXX";
>  
>  int main(int argc, char **argv)
> @@ -373,6 +415,7 @@ int main(int argc, char **argv)
>      qtest_add_func("/ast2400/smc/write_page", test_write_page);
>      qtest_add_func("/ast2400/smc/read_page_mem", test_read_page_mem);
>      qtest_add_func("/ast2400/smc/write_page_mem", test_write_page_mem);
> +    qtest_add_func("/ast2400/smc/read_status_reg", test_read_status_reg);
>  
>      ret = g_test_run();
>  
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 228357f1ea..a9904eba82 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -1423,3 +1423,27 @@ void qtest_client_inproc_recv(void *opaque, const char *str)
>      g_string_append(qts->rx, str);
>      return;
>  }
> +
> +void qtest_qom_set_bool(QTestState *s, const char *path, const char *property,
> +                         bool value)
> +{
> +    QDict *r;
> +
> +    r = qtest_qmp(s, "{ 'execute': 'qom-set', 'arguments': "
> +                     "{ 'path': %s, 'property': %s, 'value': %i } }",
> +                     path, property, value);
> +    qobject_unref(r);
> +}
> +
> +bool qtest_qom_get_bool(QTestState *s, const char *path, const char *property)
> +{
> +    QDict *r;
> +    bool b;
> +
> +    r = qtest_qmp(s, "{ 'execute': 'qom-get', 'arguments': "
> +                     "{ 'path': %s, 'property': %s } }", path, property);
> +    b = qdict_get_bool(r, "return");
> +    qobject_unref(r);
> +
> +    return b;
> +}
> diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
> index 4ab0cad326..94b187837d 100644
> --- a/tests/qtest/libqtest.h
> +++ b/tests/qtest/libqtest.h
> @@ -783,4 +783,26 @@ QTestState *qtest_inproc_init(QTestState **s, bool log, const char* arch,
>                      void (*send)(void*, const char*));
>  
>  void qtest_client_inproc_recv(void *opaque, const char *str);
> +
> +/**
> + * qtest_qom_set_bool:
> + * @s: QTestState instance to operate on.
> + * @path: Path to the property being set.
> + * @property: Property being set.
> + * @value: Value to set the property.
> + *
> + * Set the property with passed in value.
> + */
> +void qtest_qom_set_bool(QTestState *s, const char *path, const char *property,
> +                         bool value);
> +
> +/**
> + * qtest_qom_get_bool:
> + * @s: QTestState instance to operate on.
> + * @path: Path to the property being retrieved.
> + * @property: Property from where the value is being retrieved.
> + *
> + * Returns: Value retrieved from property.
> + */
> +bool qtest_qom_get_bool(QTestState *s, const char *path, const char *property);
>  #endif
> -- 
> 2.30.2
> 
>
Re: [PATCH v3] hw: m25p80: allow write_enable latch get/set
Posted by Cédric Le Goater 1 week ago
Thomas, and others,

Are you ok with the qtest extension proposed by Iris ?

If so, I can take them through an aspeed PR.

Thanks,

C.

On 5/13/22 07:50, Iris Chen via wrote:
> The write_enable latch property is not currently exposed.
> This commit makes it a modifiable property.
> 
> Signed-off-by: Iris Chen <irischenlj@fb.com>
> ---
> v3: Addressed comments by Peter and Cedric.
> v2: Ran ./scripts/checkpatch.pl on the patch and added a description. Fixed comments regarding DEFINE_PROP_BOOL.
> 
>   hw/block/m25p80.c              |  1 +
>   tests/qtest/aspeed_gpio-test.c | 40 +++++++------------------------
>   tests/qtest/aspeed_smc-test.c  | 43 ++++++++++++++++++++++++++++++++++
>   tests/qtest/libqtest.c         | 24 +++++++++++++++++++
>   tests/qtest/libqtest.h         | 22 +++++++++++++++++
>   5 files changed, 98 insertions(+), 32 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 430d1298a8..4283b990af 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -1558,6 +1558,7 @@ static int m25p80_pre_save(void *opaque)
>   
>   static Property m25p80_properties[] = {
>       /* This is default value for Micron flash */
> +    DEFINE_PROP_BOOL("write-enable", Flash, write_enable, false),
>       DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
>       DEFINE_PROP_UINT8("spansion-cr1nv", Flash, spansion_cr1nv, 0x0),
>       DEFINE_PROP_UINT8("spansion-cr2nv", Flash, spansion_cr2nv, 0x8),
> diff --git a/tests/qtest/aspeed_gpio-test.c b/tests/qtest/aspeed_gpio-test.c
> index c1003f2d1b..bac63e8742 100644
> --- a/tests/qtest/aspeed_gpio-test.c
> +++ b/tests/qtest/aspeed_gpio-test.c
> @@ -28,30 +28,6 @@
>   #include "qapi/qmp/qdict.h"
>   #include "libqtest-single.h"
>   
> -static bool qom_get_bool(QTestState *s, const char *path, const char *property)
> -{
> -    QDict *r;
> -    bool b;
> -
> -    r = qtest_qmp(s, "{ 'execute': 'qom-get', 'arguments': "
> -                     "{ 'path': %s, 'property': %s } }", path, property);
> -    b = qdict_get_bool(r, "return");
> -    qobject_unref(r);
> -
> -    return b;
> -}
> -
> -static void qom_set_bool(QTestState *s, const char *path, const char *property,
> -                         bool value)
> -{
> -    QDict *r;
> -
> -    r = qtest_qmp(s, "{ 'execute': 'qom-set', 'arguments': "
> -                     "{ 'path': %s, 'property': %s, 'value': %i } }",
> -                     path, property, value);
> -    qobject_unref(r);
> -}
> -
>   static void test_set_colocated_pins(const void *data)
>   {
>       QTestState *s = (QTestState *)data;
> @@ -60,14 +36,14 @@ static void test_set_colocated_pins(const void *data)
>        * gpioV4-7 occupy bits within a single 32-bit value, so we want to make
>        * sure that modifying one doesn't affect the other.
>        */
> -    qom_set_bool(s, "/machine/soc/gpio", "gpioV4", true);
> -    qom_set_bool(s, "/machine/soc/gpio", "gpioV5", false);
> -    qom_set_bool(s, "/machine/soc/gpio", "gpioV6", true);
> -    qom_set_bool(s, "/machine/soc/gpio", "gpioV7", false);
> -    g_assert(qom_get_bool(s, "/machine/soc/gpio", "gpioV4"));
> -    g_assert(!qom_get_bool(s, "/machine/soc/gpio", "gpioV5"));
> -    g_assert(qom_get_bool(s, "/machine/soc/gpio", "gpioV6"));
> -    g_assert(!qom_get_bool(s, "/machine/soc/gpio", "gpioV7"));
> +    qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV4", true);
> +    qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV5", false);
> +    qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV6", true);
> +    qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV7", false);
> +    g_assert(qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV4"));
> +    g_assert(!qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV5"));
> +    g_assert(qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV6"));
> +    g_assert(!qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV7"));
>   }
>   
>   int main(int argc, char **argv)
> diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
> index 87b40a0ef1..ec233315e6 100644
> --- a/tests/qtest/aspeed_smc-test.c
> +++ b/tests/qtest/aspeed_smc-test.c
> @@ -26,6 +26,7 @@
>   #include "qemu/osdep.h"
>   #include "qemu/bswap.h"
>   #include "libqtest-single.h"
> +#include "qemu/bitops.h"
>   
>   /*
>    * ASPEED SPI Controller registers
> @@ -40,6 +41,7 @@
>   #define   CTRL_FREADMODE       0x1
>   #define   CTRL_WRITEMODE       0x2
>   #define   CTRL_USERMODE        0x3
> +#define SR_WEL BIT(1)
>   
>   #define ASPEED_FMC_BASE    0x1E620000
>   #define ASPEED_FLASH_BASE  0x20000000
> @@ -49,6 +51,8 @@
>    */
>   enum {
>       JEDEC_READ = 0x9f,
> +    RDSR = 0x5,
> +    WRDI = 0x4,
>       BULK_ERASE = 0xc7,
>       READ = 0x03,
>       PP = 0x02,
> @@ -348,6 +352,44 @@ static void test_write_page_mem(void)
>       flash_reset();
>   }
>   
> +static void test_read_status_reg(void)
> +{
> +    uint8_t r;
> +
> +    spi_conf(CONF_ENABLE_W0);
> +
> +    spi_ctrl_start_user();
> +    writeb(ASPEED_FLASH_BASE, RDSR);
> +    r = readb(ASPEED_FLASH_BASE);
> +    spi_ctrl_stop_user();
> +
> +    g_assert_cmphex(r & SR_WEL, ==, 0);
> +    g_assert(!qtest_qom_get_bool
> +            (global_qtest, "/machine/soc/fmc/ssi.0/child[0]", "write-enable"));
> +
> +    spi_ctrl_start_user();
> +    writeb(ASPEED_FLASH_BASE, WREN);
> +    writeb(ASPEED_FLASH_BASE, RDSR);
> +    r = readb(ASPEED_FLASH_BASE);
> +    spi_ctrl_stop_user();
> +
> +    g_assert_cmphex(r & SR_WEL, ==, SR_WEL);
> +    g_assert(qtest_qom_get_bool
> +            (global_qtest, "/machine/soc/fmc/ssi.0/child[0]", "write-enable"));
> +
> +    spi_ctrl_start_user();
> +    writeb(ASPEED_FLASH_BASE, WRDI);
> +    writeb(ASPEED_FLASH_BASE, RDSR);
> +    r = readb(ASPEED_FLASH_BASE);
> +    spi_ctrl_stop_user();
> +
> +    g_assert_cmphex(r & SR_WEL, ==, 0);
> +    g_assert(!qtest_qom_get_bool
> +            (global_qtest, "/machine/soc/fmc/ssi.0/child[0]", "write-enable"));
> +
> +    flash_reset();
> +}
> +
>   static char tmp_path[] = "/tmp/qtest.m25p80.XXXXXX";
>   
>   int main(int argc, char **argv)
> @@ -373,6 +415,7 @@ int main(int argc, char **argv)
>       qtest_add_func("/ast2400/smc/write_page", test_write_page);
>       qtest_add_func("/ast2400/smc/read_page_mem", test_read_page_mem);
>       qtest_add_func("/ast2400/smc/write_page_mem", test_write_page_mem);
> +    qtest_add_func("/ast2400/smc/read_status_reg", test_read_status_reg);
>   
>       ret = g_test_run();
>   
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 228357f1ea..a9904eba82 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -1423,3 +1423,27 @@ void qtest_client_inproc_recv(void *opaque, const char *str)
>       g_string_append(qts->rx, str);
>       return;
>   }
> +
> +void qtest_qom_set_bool(QTestState *s, const char *path, const char *property,
> +                         bool value)
> +{
> +    QDict *r;
> +
> +    r = qtest_qmp(s, "{ 'execute': 'qom-set', 'arguments': "
> +                     "{ 'path': %s, 'property': %s, 'value': %i } }",
> +                     path, property, value);
> +    qobject_unref(r);
> +}
> +
> +bool qtest_qom_get_bool(QTestState *s, const char *path, const char *property)
> +{
> +    QDict *r;
> +    bool b;
> +
> +    r = qtest_qmp(s, "{ 'execute': 'qom-get', 'arguments': "
> +                     "{ 'path': %s, 'property': %s } }", path, property);
> +    b = qdict_get_bool(r, "return");
> +    qobject_unref(r);
> +
> +    return b;
> +}
> diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
> index 4ab0cad326..94b187837d 100644
> --- a/tests/qtest/libqtest.h
> +++ b/tests/qtest/libqtest.h
> @@ -783,4 +783,26 @@ QTestState *qtest_inproc_init(QTestState **s, bool log, const char* arch,
>                       void (*send)(void*, const char*));
>   
>   void qtest_client_inproc_recv(void *opaque, const char *str);
> +
> +/**
> + * qtest_qom_set_bool:
> + * @s: QTestState instance to operate on.
> + * @path: Path to the property being set.
> + * @property: Property being set.
> + * @value: Value to set the property.
> + *
> + * Set the property with passed in value.
> + */
> +void qtest_qom_set_bool(QTestState *s, const char *path, const char *property,
> +                         bool value);
> +
> +/**
> + * qtest_qom_get_bool:
> + * @s: QTestState instance to operate on.
> + * @path: Path to the property being retrieved.
> + * @property: Property from where the value is being retrieved.
> + *
> + * Returns: Value retrieved from property.
> + */
> +bool qtest_qom_get_bool(QTestState *s, const char *path, const char *property);
>   #endif
Re: [PATCH v3] hw: m25p80: allow write_enable latch get/set
Posted by Thomas Huth 1 week ago
On 13/05/2022 15.54, Cédric Le Goater wrote:
> Thomas, and others,
> 
> Are you ok with the qtest extension proposed by Iris ?
> 
> If so, I can take them through an aspeed PR.

Fine for me!

> On 5/13/22 07:50, Iris Chen via wrote:
>> The write_enable latch property is not currently exposed.
>> This commit makes it a modifiable property.
>>
>> Signed-off-by: Iris Chen <irischenlj@fb.com>
>> ---
>> v3: Addressed comments by Peter and Cedric.
>> v2: Ran ./scripts/checkpatch.pl on the patch and added a description. 
>> Fixed comments regarding DEFINE_PROP_BOOL.

Acked-by: Thomas Huth <thuth@redhat.com>



Re: [PATCH v3] hw: m25p80: allow write_enable latch get/set
Posted by Cédric Le Goater 1 week ago
On 5/13/22 16:37, Thomas Huth wrote:
> On 13/05/2022 15.54, Cédric Le Goater wrote:
>> Thomas, and others,
>>
>> Are you ok with the qtest extension proposed by Iris ?
>>
>> If so, I can take them through an aspeed PR.
> 
> Fine for me!
> 
>> On 5/13/22 07:50, Iris Chen via wrote:
>>> The write_enable latch property is not currently exposed.
>>> This commit makes it a modifiable property.
>>>
>>> Signed-off-by: Iris Chen <irischenlj@fb.com>
>>> ---
>>> v3: Addressed comments by Peter and Cedric.
>>> v2: Ran ./scripts/checkpatch.pl on the patch and added a description. Fixed comments regarding DEFINE_PROP_BOOL.
> 
> Acked-by: Thomas Huth <thuth@redhat.com>
> 
> 

Thanks Thomas,


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

C.