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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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