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

[Cédric Le Goater]

Hello Iris,

[ Fixing Thomas email ]

On 5/12/22 02:54, Iris Chen via wrote:
> The write_enable latch property is not currently exposed.
> This commit makes it a modifiable property using get/set methods.
>
> Signed-off-by: Iris Chen <irischenlj@fb.com>
> ---
> Ran ./scripts/checkpatch.pl on the patch and added a description. 
> Fixed comments regarding DEFINE_PROP_BOOL.

You should run 'make check' also.

With the removal of "qapi/visitor.h" and the property name change
in the test,

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

One comment below,

>   hw/block/m25p80.c             |  2 ++
>   tests/qtest/aspeed_smc-test.c | 23 +++++++++++++++++++++++
>   2 files changed, 25 insertions(+)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 430d1298a8..019beb5b78 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -35,6 +35,7 @@
>   #include "qapi/error.h"
>   #include "trace.h"
>   #include "qom/object.h"
> +#include "qapi/visitor.h"
>   
>   /* Fields for FlashPartInfo->flags */
>   
> @@ -1558,6 +1559,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_smc-test.c b/tests/qtest/aspeed_smc-test.c
> index 87b40a0ef1..fcc156bc00 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,7 @@
>    */
>   enum {
>       JEDEC_READ = 0x9f,
> +    RDSR = 0x5,
>       BULK_ERASE = 0xc7,
>       READ = 0x03,
>       PP = 0x02,
> @@ -348,6 +351,25 @@ static void test_write_page_mem(void)
>       flash_reset();
>   }
>   
> +static void test_read_status_reg(void)
> +{
> +    uint8_t r;
> +
> +    qmp("{ 'execute': 'qom-set', 'arguments': "
> +       "{'path': '/machine/soc/fmc/ssi.0/child[0]', 'property': 'WEL', 'value': 
> true}}");

Peter added qom_get_bool() and qom_set_bool() helpers in
aspeed_gpio-test.c, it might be interesting to reuse.

There are similar ones in the npcm7xx tests, btw.

Thanks,

C.



> +
> +    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, ==, SR_WEL);
> +
> +    flash_reset();
> +}
> +
>   static char tmp_path[] = "/tmp/qtest.m25p80.XXXXXX";
>   
>   int main(int argc, char **argv)
> @@ -373,6 +395,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();
>