[PATCH v2] hw/misc/macio/gpio.c: Add constants for register bits

BALATON Zoltan posted 1 patch 9 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250224141026.3B36C4E6010@zero.eik.bme.hu
Maintainers: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
hw/misc/macio/gpio.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
[PATCH v2] hw/misc/macio/gpio.c: Add constants for register bits
Posted by BALATON Zoltan 9 months, 3 weeks ago
Add named constants for register bit values that should make it easier
to understand what these mean.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daud\ufffd\ufffd <philmd@linaro.org>
---
 hw/misc/macio/gpio.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/hw/misc/macio/gpio.c b/hw/misc/macio/gpio.c
index 4364afc84a..e87bfca1f5 100644
--- a/hw/misc/macio/gpio.c
+++ b/hw/misc/macio/gpio.c
@@ -34,6 +34,11 @@
 #include "qemu/module.h"
 #include "trace.h"
 
+enum MacioGPIORegisterBits {
+    OUT_DATA   = 1,
+    IN_DATA    = 2,
+    OUT_ENABLE = 4,
+};
 
 void macio_set_gpio(MacIOGPIOState *s, uint32_t gpio, bool state)
 {
@@ -41,14 +46,14 @@ void macio_set_gpio(MacIOGPIOState *s, uint32_t gpio, bool state)
 
     trace_macio_set_gpio(gpio, state);
 
-    if (s->gpio_regs[gpio] & 4) {
+    if (s->gpio_regs[gpio] & OUT_ENABLE) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "GPIO: Setting GPIO %d while it's an output\n", gpio);
     }
 
-    new_reg = s->gpio_regs[gpio] & ~2;
+    new_reg = s->gpio_regs[gpio] & ~IN_DATA;
     if (state) {
-        new_reg |= 2;
+        new_reg |= IN_DATA;
     }
 
     if (new_reg == s->gpio_regs[gpio]) {
@@ -107,12 +112,12 @@ static void macio_gpio_write(void *opaque, hwaddr addr, uint64_t value,
 
     addr -= 8;
     if (addr < 36) {
-        value &= ~2;
+        value &= ~IN_DATA;
 
-        if (value & 4) {
-            ibit = (value & 1) << 1;
+        if (value & OUT_ENABLE) {
+            ibit = (value & OUT_DATA) << 1;
         } else {
-            ibit = s->gpio_regs[addr] & 2;
+            ibit = s->gpio_regs[addr] & IN_DATA;
         }
 
         s->gpio_regs[addr] = value | ibit;
-- 
2.30.9
Re: [PATCH v2] hw/misc/macio/gpio.c: Add constants for register bits
Posted by Philippe Mathieu-Daudé 9 months, 2 weeks ago
On 24/2/25 15:10, BALATON Zoltan wrote:
> Add named constants for register bit values that should make it easier
> to understand what these mean.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/misc/macio/gpio.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)

Patch queued, thanks!

Re: [PATCH v2] hw/misc/macio/gpio.c: Add constants for register bits
Posted by Mark Cave-Ayland 9 months, 3 weeks ago
On 24/02/2025 14:10, BALATON Zoltan wrote:

> Add named constants for register bit values that should make it easier
> to understand what these mean.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/misc/macio/gpio.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/misc/macio/gpio.c b/hw/misc/macio/gpio.c
> index 4364afc84a..e87bfca1f5 100644
> --- a/hw/misc/macio/gpio.c
> +++ b/hw/misc/macio/gpio.c
> @@ -34,6 +34,11 @@
>   #include "qemu/module.h"
>   #include "trace.h"
>   
> +enum MacioGPIORegisterBits {
> +    OUT_DATA   = 1,
> +    IN_DATA    = 2,
> +    OUT_ENABLE = 4,
> +};
>   
>   void macio_set_gpio(MacIOGPIOState *s, uint32_t gpio, bool state)
>   {
> @@ -41,14 +46,14 @@ void macio_set_gpio(MacIOGPIOState *s, uint32_t gpio, bool state)
>   
>       trace_macio_set_gpio(gpio, state);
>   
> -    if (s->gpio_regs[gpio] & 4) {
> +    if (s->gpio_regs[gpio] & OUT_ENABLE) {
>           qemu_log_mask(LOG_GUEST_ERROR,
>                         "GPIO: Setting GPIO %d while it's an output\n", gpio);
>       }
>   
> -    new_reg = s->gpio_regs[gpio] & ~2;
> +    new_reg = s->gpio_regs[gpio] & ~IN_DATA;
>       if (state) {
> -        new_reg |= 2;
> +        new_reg |= IN_DATA;
>       }
>   
>       if (new_reg == s->gpio_regs[gpio]) {
> @@ -107,12 +112,12 @@ static void macio_gpio_write(void *opaque, hwaddr addr, uint64_t value,
>   
>       addr -= 8;
>       if (addr < 36) {
> -        value &= ~2;
> +        value &= ~IN_DATA;
>   
> -        if (value & 4) {
> -            ibit = (value & 1) << 1;
> +        if (value & OUT_ENABLE) {
> +            ibit = (value & OUT_DATA) << 1;
>           } else {
> -            ibit = s->gpio_regs[addr] & 2;
> +            ibit = s->gpio_regs[addr] & IN_DATA;
>           }
>   
>           s->gpio_regs[addr] = value | ibit;

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.