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

[Peter Delevoryas]

> On May 11, 2022, at 6:38 PM, Peter Delevoryas <pdel@fb.com> wrote:
> 
> 
> 
>> On May 11, 2022, at 5:54 PM, Iris Chen <irischenlj@fb.com> 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.
>> 
>> 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"
> 
> I think you might be able to remove “#include qapi/visitor.h" now that you’re 
> using DEFINE_PROP_BOOL.
> 
>> 
>> /* 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}}");

Oh, additionally: I don’t think this test passes, “WEL” needs to be converted
to “write-enable” in this qmp() call.

$ QTEST_LOG=1 ./build/tests/qtest/aspeed_smc-test -p 
/arm/ast2400/smc/read_status_reg
[I 0.000000] OPENED
[R +0.139474] endianness
[S +0.139509] OK little
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 0, "major": 7}, "package": 
"v7.0.0-943-g1e08a558ee-dirty"}, "capabilities": ["oob"]}}
{"execute": "qmp_capabilities"}

{"return": {}}
/arm/ast2400/smc/read_status_reg: {"execute": "qom-set", "arguments": {"path": 
"/machine/soc/fmc/ssi.0/child[0]", "property": "WEL", "value": true}}

{"error": {"class": "GenericError", "desc": "Property 'n25q256a.WEL' not 
found"}}
[R +0.141846] readl 0x1e620000
[S +0.141869] OK 0x0000000000000002
[R +0.141936] writel 0x1e620000 0x10002
[S +0.141946] OK
[R +0.141977] readl 0x1e620010
[S +0.141984] OK 0x0000000000000004
[R +0.142009] writel 0x1e620010 0x7
[S +0.142018] OK
[R +0.142043] writel 0x1e620010 0x3
[S +0.142047] OK
[R +0.142068] writeb 0x20000000 0x5
[S +0.142078] OK
[R +0.142117] readb 0x20000000
[S +0.142125] OK 0x0000000000000000
[R +0.142149] readl 0x1e620010
[S +0.142156] OK 0x0000000000000003
[R +0.142180] writel 0x1e620010 0x7
[S +0.142187] OK
**
ERROR:../tests/qtest/aspeed_smc-test.c:368:test_read_status_reg: assertion 
failed (r & SR_WEL == SR_WEL): (0x00000000 == 0x00000002)
[I +0.291823] CLOSED
Aborted (core dumped)

>> +
>> +
>> + 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();
>> 
>> -- 
>> 2.30.2