[PATCH 1/2] hw/pflash: refactor pflash_data_write()

Gerd Hoffmann posted 2 patches 10 months, 3 weeks ago
Maintainers: "Philippe Mathieu-Daudé" <philmd@linaro.org>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
There is a newer version of this series
[PATCH 1/2] hw/pflash: refactor pflash_data_write()
Posted by Gerd Hoffmann 10 months, 3 weeks ago
Move the offset calculation, do it once at the start of the function and
let the 'p' variable point directly to the memory location which should
be updated.  This makes it simpler to update other buffers than
pfl->storage in an upcoming patch.  No functional change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/block/pflash_cfi01.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 3e2dc08bd78f..67f1c9773ab3 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -403,33 +403,35 @@ static void pflash_update(PFlashCFI01 *pfl, int offset,
 static inline void pflash_data_write(PFlashCFI01 *pfl, hwaddr offset,
                                      uint32_t value, int width, int be)
 {
-    uint8_t *p = pfl->storage;
+    uint8_t *p;
 
     trace_pflash_data_write(pfl->name, offset, width, value, pfl->counter);
+    p = pfl->storage + offset;
+
     switch (width) {
     case 1:
-        p[offset] = value;
+        p[0] = value;
         break;
     case 2:
         if (be) {
-            p[offset] = value >> 8;
-            p[offset + 1] = value;
+            p[0] = value >> 8;
+            p[1] = value;
         } else {
-            p[offset] = value;
-            p[offset + 1] = value >> 8;
+            p[0] = value;
+            p[1] = value >> 8;
         }
         break;
     case 4:
         if (be) {
-            p[offset] = value >> 24;
-            p[offset + 1] = value >> 16;
-            p[offset + 2] = value >> 8;
-            p[offset + 3] = value;
+            p[0] = value >> 24;
+            p[1] = value >> 16;
+            p[2] = value >> 8;
+            p[3] = value;
         } else {
-            p[offset] = value;
-            p[offset + 1] = value >> 8;
-            p[offset + 2] = value >> 16;
-            p[offset + 3] = value >> 24;
+            p[0] = value;
+            p[1] = value >> 8;
+            p[2] = value >> 16;
+            p[3] = value >> 24;
         }
         break;
     }
-- 
2.43.0
Re: [PATCH 1/2] hw/pflash: refactor pflash_data_write()
Posted by Philippe Mathieu-Daudé 10 months, 3 weeks ago
On 5/1/24 14:58, Gerd Hoffmann wrote:
> Move the offset calculation, do it once at the start of the function and
> let the 'p' variable point directly to the memory location which should
> be updated.  This makes it simpler to update other buffers than
> pfl->storage in an upcoming patch.  No functional change.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hw/block/pflash_cfi01.c | 30 ++++++++++++++++--------------
>   1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 3e2dc08bd78f..67f1c9773ab3 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -403,33 +403,35 @@ static void pflash_update(PFlashCFI01 *pfl, int offset,
>   static inline void pflash_data_write(PFlashCFI01 *pfl, hwaddr offset,
>                                        uint32_t value, int width, int be)
>   {
> -    uint8_t *p = pfl->storage;
> +    uint8_t *p;
>   
>       trace_pflash_data_write(pfl->name, offset, width, value, pfl->counter);
> +    p = pfl->storage + offset;
> +
>       switch (width) {
>       case 1:
> -        p[offset] = value;
> +        p[0] = value;
>           break;
>       case 2:
>           if (be) {
> -            p[offset] = value >> 8;
> -            p[offset + 1] = value;
> +            p[0] = value >> 8;
> +            p[1] = value;
>           } else {
> -            p[offset] = value;
> -            p[offset + 1] = value >> 8;
> +            p[0] = value;
> +            p[1] = value >> 8;
>           }
>           break;
>       case 4:
>           if (be) {
> -            p[offset] = value >> 24;
> -            p[offset + 1] = value >> 16;
> -            p[offset + 2] = value >> 8;
> -            p[offset + 3] = value;
> +            p[0] = value >> 24;
> +            p[1] = value >> 16;
> +            p[2] = value >> 8;
> +            p[3] = value;
>           } else {
> -            p[offset] = value;
> -            p[offset + 1] = value >> 8;
> -            p[offset + 2] = value >> 16;
> -            p[offset + 3] = value >> 24;
> +            p[0] = value;
> +            p[1] = value >> 8;
> +            p[2] = value >> 16;
> +            p[3] = value >> 24;
>           }
>           break;
>       }

Close to commit c3d25271b2 ("hw/block/pflash_cfi02: Use the ldst
API in pflash_write()"):

   if (be) {
       stn_be_p(p, width, value);
   } else {
       stn_le_p(p, width, value);
   }