All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] hw: allow write_enable latch get/set
@ 2022-05-11 18:10 Iris Chen via
  2022-05-11 18:10 ` [PATCH 1/1] " Iris Chen via
  0 siblings, 1 reply; 5+ messages in thread
From: Iris Chen via @ 2022-05-11 18:10 UTC (permalink / raw)
  Cc: irischenlj, pdel, qemu-devel, qemu-arm, clg, patrick

Hey everyone, 

My patch adds the get/set property for the write_enable latch. Currently, it is not an exposed property but this change adds QOM visibility and makes this property modifiable. 

Acceptance tests have also been added to verify the value in the status register (after the value is set). 

Thanks, 
Iris

Iris Chen (1):
  hw: allow write_enable latch get/set

 hw/block/m25p80.c             | 30 ++++++++++++++++++++++++++++++
 tests/qtest/aspeed_smc-test.c | 20 ++++++++++++++++++++
 2 files changed, 50 insertions(+)

-- 
2.30.2



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

* [PATCH 1/1] hw: allow write_enable latch get/set
  2022-05-11 18:10 [PATCH 0/1] hw: allow write_enable latch get/set Iris Chen via
@ 2022-05-11 18:10 ` Iris Chen via
  0 siblings, 0 replies; 5+ messages in thread
From: Iris Chen via @ 2022-05-11 18:10 UTC (permalink / raw)
  Cc: irischenlj, pdel, qemu-devel, qemu-arm, clg, patrick

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



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

* Re: [PATCH 1/1] hw: allow write_enable latch get/set
  2022-05-11 18:45 ` [PATCH 1/1] " Iris Chen via
  2022-05-11 20:30   ` Peter Delevoryas
@ 2022-05-11 20:54   ` Cédric Le Goater
  1 sibling, 0 replies; 5+ messages in thread
From: Cédric Le Goater @ 2022-05-11 20:54 UTC (permalink / raw)
  To: Iris Chen; +Cc: pdel, qemu-devel, qemu-arm, patrick

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



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

* Re: [PATCH 1/1] hw: allow write_enable latch get/set
  2022-05-11 18:45 ` [PATCH 1/1] " Iris Chen via
@ 2022-05-11 20:30   ` Peter Delevoryas
  2022-05-11 20:54   ` Cédric Le Goater
  1 sibling, 0 replies; 5+ messages in thread
From: Peter Delevoryas @ 2022-05-11 20:30 UTC (permalink / raw)
  Cc: Cameron Esfahani via, qemu-arm, Cédric Le Goater, patrick,
	Iris Chen, Peter Delevoryas



> On May 11, 2022, at 11:45 AM, Iris Chen via <qemu-devel@nongnu.org> 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);
> +
> +    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);

I think the spacing here might be slightly off, try making the 2nd and 3rd lines
align with “klass” in the first line.

> }
> 
> 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}}");

Spacing is off here, make sure the indentation is set to 4 spaces.

> +
> +    spi_conf(CONF_ENABLE_W0);
> +	spi_ctrl_start_user();
> +	writeb(ASPEED_FLASH_BASE, RDSR);
> +	r = readb(ASPEED_FLASH_BASE);
> +	spi_ctrl_stop_user();

Spacing is off here too.

> +
> +	g_assert_cmphex(r, ==, 0x2);

Maybe consider using a constant instead of 0x2, like:

...
#include “qemu/bitops.h" 
...
#define SR_WEL BIT(1)
…
g_assert_cmphex(r, ==, SR_WEL);

Additionally, since we only care about testing the write enable
latch state (and not the other bits in the status register),
it could be:

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


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

* [PATCH 1/1] hw: allow write_enable latch get/set
  2022-05-11 18:45 [PATCH 0/1] " Iris Chen via
@ 2022-05-11 18:45 ` Iris Chen via
  2022-05-11 20:30   ` Peter Delevoryas
  2022-05-11 20:54   ` Cédric Le Goater
  0 siblings, 2 replies; 5+ messages in thread
From: Iris Chen via @ 2022-05-11 18:45 UTC (permalink / raw)
  Cc: irischenlj, pdel, qemu-devel, qemu-arm, clg, patrick

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



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

end of thread, other threads:[~2022-05-11 20:54 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-11 18:10 [PATCH 0/1] hw: allow write_enable latch get/set Iris Chen via
2022-05-11 18:10 ` [PATCH 1/1] " Iris Chen via
2022-05-11 18:45 [PATCH 0/1] " Iris Chen via
2022-05-11 18:45 ` [PATCH 1/1] " Iris Chen via
2022-05-11 20:30   ` Peter Delevoryas
2022-05-11 20:54   ` Cédric Le Goater

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.