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

[Cédric Le Goater]

Hello Iris,

You need to add a description to the patch, may be use what you wrote
in the cover letter to start with, and a Signed-off-by tag.

Before sending, please run :

  $ ./scripts/checkpatch.pl <patch>

and

  $ ./scripts/get_maintainer.pl <patch>

to know who to send to.


The long story is here :

  https://qemu.readthedocs.io/en/latest/devel/submitting-a-patch.html

On 5/11/22 20:45, Iris Chen via wrote:
> ---
>   hw/block/m25p80.c             | 30 ++++++++++++++++++++++++++++++
>   tests/qtest/aspeed_smc-test.c | 20 ++++++++++++++++++++
>   2 files changed, 50 insertions(+)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 430d1298a8..fb72704e5a 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 */
>   
> @@ -1646,6 +1647,31 @@ static const VMStateDescription vmstate_m25p80 = {
>       }
>   };
>   
> +static void m25p80_get_wel(Object *obj, Visitor *v, const char *name,
> +                            void *opaque, Error **errp)
> +{
> +    Flash *s = M25P80(obj);
> +
> +    assert(strcmp(name, "WEL") == 0);

That's not necessary.

> +
> +    visit_type_bool(v, name, &s->write_enable, errp);
> +}
> +
> +static void m25p80_set_wel(Object *obj, Visitor *v, const char *name,
> +                            void *opaque, Error **errp)
> +{
> +    Flash *s = M25P80(obj);
> +    bool value;
> +
> +    assert(strcmp(name, "WEL") == 0);
> +
> +    if (!visit_type_bool(v, name, &value, errp)) {
> +        return;
> +    }
> +
> +    s->write_enable = value;
> +}
> +
>   static void m25p80_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -1660,6 +1686,10 @@ static void m25p80_class_init(ObjectClass *klass, void 
> *data)
>       device_class_set_props(dc, m25p80_properties);
>       dc->reset = m25p80_reset;
>       mc->pi = data;
> +
> +    object_class_property_add(klass, "WEL", "bool",
> +                            m25p80_get_wel,
> +                            m25p80_set_wel, NULL, NULL);

Instead, you could add a :

    DEFINE_PROP_BOOL("write-enable", Flash, write_enable, false);

under m25p80_properties.

Thanks,

C.

>   }
>   
>   static const TypeInfo m25p80_info = {
> diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
> index 87b40a0ef1..af885a9c9d 100644
> --- a/tests/qtest/aspeed_smc-test.c
> +++ b/tests/qtest/aspeed_smc-test.c
> @@ -49,6 +49,7 @@
>    */
>   enum {
>       JEDEC_READ = 0x9f,
> +    RDSR = 0x5,
>       BULK_ERASE = 0xc7,
>       READ = 0x03,
>       PP = 0x02,
> @@ -348,6 +349,24 @@ 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}}");
> +
> +    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, ==, 0x2);
> +
> +       flash_reset();
> +}
> +
>   static char tmp_path[] = "/tmp/qtest.m25p80.XXXXXX";
>   
>   int main(int argc, char **argv)
> @@ -373,6 +392,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();
>