All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] hw/misc/pca9552: Trace LEDs events, make LEDs 13-15 as GPIOs
@ 2020-06-16  9:45 Philippe Mathieu-Daudé
  2020-06-16  9:45 ` [PATCH 1/2] hw/misc/pca9552: Trace LED On/Off events Philippe Mathieu-Daudé
  2020-06-16  9:45 ` [PATCH 2/2] hw/misc/pca9552: Make LEDs 13-15 also GPIOs Philippe Mathieu-Daudé
  0 siblings, 2 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-16  9:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Andrew Jeffery, Joaquin de Andres,
	Philippe Mathieu-Daudé,
	Esteban Bosse, qemu-arm, Joel Stanley, Guenter Roeck,
	Cédric Le Goater

The PCA9552 LEDs 13-15 can also be used as GPIOs.

Philippe Mathieu-Daudé (2):
  hw/misc/pca9552: Trace LED On/Off events
  hw/misc/pca9552: Make LEDs 13-15 also GPIOs

 include/hw/misc/pca9552.h |  2 ++
 hw/misc/pca9552.c         | 25 +++++++++++++++++++++++++
 hw/misc/trace-events      |  3 +++
 3 files changed, 30 insertions(+)

-- 
2.21.3



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

* [PATCH 1/2] hw/misc/pca9552: Trace LED On/Off events
  2020-06-16  9:45 [PATCH 0/2] hw/misc/pca9552: Trace LEDs events, make LEDs 13-15 as GPIOs Philippe Mathieu-Daudé
@ 2020-06-16  9:45 ` Philippe Mathieu-Daudé
  2020-06-16 16:13   ` Cédric Le Goater
  2020-06-16  9:45 ` [PATCH 2/2] hw/misc/pca9552: Make LEDs 13-15 also GPIOs Philippe Mathieu-Daudé
  1 sibling, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-16  9:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Andrew Jeffery, Joaquin de Andres,
	Philippe Mathieu-Daudé,
	Esteban Bosse, qemu-arm, Joel Stanley, Guenter Roeck,
	Cédric Le Goater

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/pca9552.c    | 7 +++++++
 hw/misc/trace-events | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
index cac729e35a..a3d0decbff 100644
--- a/hw/misc/pca9552.c
+++ b/hw/misc/pca9552.c
@@ -17,6 +17,7 @@
 #include "migration/vmstate.h"
 #include "qapi/error.h"
 #include "qapi/visitor.h"
+#include "trace.h"
 
 #define PCA9552_LED_ON   0x0
 #define PCA9552_LED_OFF  0x1
@@ -45,9 +46,15 @@ static void pca9552_update_pin_input(PCA9552State *s)
         switch (config) {
         case PCA9552_LED_ON:
             s->regs[input_reg] |= 1 << input_shift;
+            if (input_shift < s->nr_leds) {
+                trace_pca9552_led_set(input_shift, true);
+            }
             break;
         case PCA9552_LED_OFF:
             s->regs[input_reg] &= ~(1 << input_shift);
+            if (input_shift < s->nr_leds) {
+                trace_pca9552_led_set(input_shift, false);
+            }
             break;
         case PCA9552_LED_PWM0:
         case PCA9552_LED_PWM1:
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 5561746866..ed80d0d1be 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -206,3 +206,6 @@ via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "secto
 # grlib_ahb_apb_pnp.c
 grlib_ahb_pnp_read(uint64_t addr, uint32_t value) "AHB PnP read addr:0x%03"PRIx64" data:0x%08x"
 grlib_apb_pnp_read(uint64_t addr, uint32_t value) "APB PnP read addr:0x%03"PRIx64" data:0x%08x"
+
+# pca9552.c
+pca9552_led_set(unsigned id, bool state) "LED#%d state:%u"
-- 
2.21.3



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

* [PATCH 2/2] hw/misc/pca9552: Make LEDs 13-15 also GPIOs
  2020-06-16  9:45 [PATCH 0/2] hw/misc/pca9552: Trace LEDs events, make LEDs 13-15 as GPIOs Philippe Mathieu-Daudé
  2020-06-16  9:45 ` [PATCH 1/2] hw/misc/pca9552: Trace LED On/Off events Philippe Mathieu-Daudé
@ 2020-06-16  9:45 ` Philippe Mathieu-Daudé
  2020-06-17  5:10   ` Cédric Le Goater
  1 sibling, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-16  9:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Andrew Jeffery, Joaquin de Andres,
	Philippe Mathieu-Daudé,
	Esteban Bosse, qemu-arm, Joel Stanley, Guenter Roeck,
	Cédric Le Goater

The PCA9552 has 3 GPIOs, add them.

See 'PCA9552 Product Datasheet Rev. 05 - 9 March 2006',
chapter 6.4 'Pins used as GPIOs':

  LED pins not used to control LEDs can be used as general
  purpose I/Os (GPIOs).
  For use as input, set LEDn to high-impedance (01) and then
  read the pin state via the input register.
  For use as output, connect external pull-up resistor to the
  pin and size it according to the DC recommended operating
  characteristics. LED output pin is HIGH when the output is
  programmed as high-impedance, and LOW when the output is
  programmed LOW through the ‘LED selector’ register. The
  output can be pulse-width controlled when PWM0 or PWM1 are
  used.

And chapter 8 'Application design-in information':

  LED0 to LED12 are used as LED drivers.
  LED13 to LED15 are used as regular GPIOs.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/misc/pca9552.h |  2 ++
 hw/misc/pca9552.c         | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/hw/misc/pca9552.h b/include/hw/misc/pca9552.h
index ebb43c63fe..7e47ea312d 100644
--- a/include/hw/misc/pca9552.h
+++ b/include/hw/misc/pca9552.h
@@ -15,6 +15,7 @@
 #define PCA9552(obj) OBJECT_CHECK(PCA9552State, (obj), TYPE_PCA9552)
 
 #define PCA9552_NR_REGS 10
+#define PCA9552_NR_GPIOS 3
 
 typedef struct PCA9552State {
     /*< private >*/
@@ -27,6 +28,7 @@ typedef struct PCA9552State {
     uint8_t regs[PCA9552_NR_REGS];
     uint8_t max_reg;
     uint8_t nr_leds;
+    qemu_irq gpio[PCA9552_NR_GPIOS];
 } PCA9552State;
 
 #endif
diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
index a3d0decbff..6ca6c0dbc2 100644
--- a/hw/misc/pca9552.c
+++ b/hw/misc/pca9552.c
@@ -12,8 +12,10 @@
 #include "qemu/osdep.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
+#include "qemu/bitops.h"
 #include "hw/misc/pca9552.h"
 #include "hw/misc/pca9552_regs.h"
+#include "hw/irq.h"
 #include "migration/vmstate.h"
 #include "qapi/error.h"
 #include "qapi/visitor.h"
@@ -48,12 +50,16 @@ static void pca9552_update_pin_input(PCA9552State *s)
             s->regs[input_reg] |= 1 << input_shift;
             if (input_shift < s->nr_leds) {
                 trace_pca9552_led_set(input_shift, true);
+            } else {
+                qemu_set_irq(s->gpio[input_shift - s->nr_leds], 1);
             }
             break;
         case PCA9552_LED_OFF:
             s->regs[input_reg] &= ~(1 << input_shift);
             if (input_shift < s->nr_leds) {
                 trace_pca9552_led_set(input_shift, false);
+            } else {
+                qemu_set_irq(s->gpio[input_shift - s->nr_leds], 0);
             }
             break;
         case PCA9552_LED_PWM0:
@@ -65,6 +71,16 @@ static void pca9552_update_pin_input(PCA9552State *s)
     }
 }
 
+static void pca9552_gpio_set(void *opaque, int n, int enable)
+{
+    PCA9552State *s = opaque;
+
+    /* LED13 to LED15 are used as regular GPIOs. */
+    s->regs[PCA9552_LS3] = deposit32(s->regs[PCA9552_LS3], n + 1, 1,
+                                     enable ? PCA9552_LED_ON : PCA9552_LED_OFF);
+    pca9552_update_pin_input(s);
+}
+
 static uint8_t pca9552_read(PCA9552State *s, uint8_t reg)
 {
     switch (reg) {
@@ -308,6 +324,8 @@ static void pca9552_initfn(Object *obj)
                             NULL, NULL);
         g_free(name);
     }
+    qdev_init_gpio_in(DEVICE(obj), pca9552_gpio_set, PCA9552_NR_GPIOS);
+    qdev_init_gpio_out(DEVICE(obj), s->gpio, PCA9552_NR_GPIOS);
 }
 
 static void pca9552_class_init(ObjectClass *klass, void *data)
-- 
2.21.3



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

* Re: [PATCH 1/2] hw/misc/pca9552: Trace LED On/Off events
  2020-06-16  9:45 ` [PATCH 1/2] hw/misc/pca9552: Trace LED On/Off events Philippe Mathieu-Daudé
@ 2020-06-16 16:13   ` Cédric Le Goater
  2020-06-16 17:15     ` Cédric Le Goater
  0 siblings, 1 reply; 7+ messages in thread
From: Cédric Le Goater @ 2020-06-16 16:13 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Peter Maydell, Andrew Jeffery, Joaquin de Andres, Esteban Bosse,
	qemu-arm, Joel Stanley, Guenter Roeck

On 6/16/20 11:45 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


Reviewed-by: Cédric Le Goater <clg@kaod.org>
Tested-by: Cédric Le Goater <clg@kaod.org>

Download an OpenBMC witherspoon flash image : 

wget https://openpower.xyz/job/openbmc-build/lastSuccessfulBuild/distro=ubuntu,label=builder,target=witherspoon/artifact/deploy/images/witherspoon/obmc-phosphor-image-witherspoon.ubi.mtd

and run :

qemu-system-arm -M witherspoon-bmc -nic user -drive file=obmc-phosphor-image-witherspoon.ubi.mtd,format=raw,if=mtd -nographic

The system has a set of daemons controlling the status of the leds :

 1148 root      7164 S    phosphor-ledcontroller -p /sys/class/leds/cffps1/68
 1149 root      7164 S    phosphor-ledcontroller -p /sys/class/leds/cffps1/69
 1150 root      7164 S    phosphor-ledcontroller -p /sys/class/leds/fan0
 1151 root      7164 S    phosphor-ledcontroller -p /sys/class/leds/fan1
 1152 root      7164 S    phosphor-ledcontroller -p /sys/class/leds/fan2
 1153 root      7164 S    phosphor-ledcontroller -p /sys/class/leds/fan3
 1154 root      7164 S    phosphor-ledcontroller -p /sys/class/leds/front/fault
 1155 root      7164 S    phosphor-ledcontroller -p /sys/class/leds/front/id
 1156 root      7164 S    phosphor-ledcontroller -p /sys/class/leds/front/power
 1157 root      7164 S    phosphor-ledcontroller -p /sys/class/leds/power/button
 1158 root      7164 S    phosphor-ledcontroller -p /sys/class/leds/rear/fault
 1159 root      7164 S    phosphor-ledcontroller -p /sys/class/leds/rear/id
 1160 root      7164 S    phosphor-ledcontroller -p /sys/class/leds/rear/power


If activated, the pca9552 traces will appear on the QEMU monitor.

Thanks,

C.

> ---
>  hw/misc/pca9552.c    | 7 +++++++
>  hw/misc/trace-events | 3 +++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
> index cac729e35a..a3d0decbff 100644
> --- a/hw/misc/pca9552.c
> +++ b/hw/misc/pca9552.c
> @@ -17,6 +17,7 @@
>  #include "migration/vmstate.h"
>  #include "qapi/error.h"
>  #include "qapi/visitor.h"
> +#include "trace.h"
>  
>  #define PCA9552_LED_ON   0x0
>  #define PCA9552_LED_OFF  0x1
> @@ -45,9 +46,15 @@ static void pca9552_update_pin_input(PCA9552State *s)
>          switch (config) {
>          case PCA9552_LED_ON:
>              s->regs[input_reg] |= 1 << input_shift;
> +            if (input_shift < s->nr_leds) {
> +                trace_pca9552_led_set(input_shift, true);
> +            }
>              break;
>          case PCA9552_LED_OFF:
>              s->regs[input_reg] &= ~(1 << input_shift);
> +            if (input_shift < s->nr_leds) {
> +                trace_pca9552_led_set(input_shift, false);
> +            }
>              break;
>          case PCA9552_LED_PWM0:
>          case PCA9552_LED_PWM1:
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index 5561746866..ed80d0d1be 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -206,3 +206,6 @@ via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "secto
>  # grlib_ahb_apb_pnp.c
>  grlib_ahb_pnp_read(uint64_t addr, uint32_t value) "AHB PnP read addr:0x%03"PRIx64" data:0x%08x"
>  grlib_apb_pnp_read(uint64_t addr, uint32_t value) "APB PnP read addr:0x%03"PRIx64" data:0x%08x"
> +
> +# pca9552.c
> +pca9552_led_set(unsigned id, bool state) "LED#%d state:%u"
> 



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

* Re: [PATCH 1/2] hw/misc/pca9552: Trace LED On/Off events
  2020-06-16 16:13   ` Cédric Le Goater
@ 2020-06-16 17:15     ` Cédric Le Goater
  2020-06-16 18:05       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 7+ messages in thread
From: Cédric Le Goater @ 2020-06-16 17:15 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Peter Maydell, Andrew Jeffery, Joaquin de Andres, Esteban Bosse,
	qemu-arm, Joel Stanley, Guenter Roeck

After a closer look,

>> @@ -45,9 +46,15 @@ static void pca9552_update_pin_input(PCA9552State *s)
>>          switch (config) {
>>          case PCA9552_LED_ON:
>>              s->regs[input_reg] |= 1 << input_shift;
>> +            if (input_shift < s->nr_leds) {
This seems like a superfluous test.

>> +                trace_pca9552_led_set(input_shift, true);

This should be using 'i' and the output is not very concise : 

564609@1592326881.815616:pca9552_led_set LED#0 state:1
564609@1592326881.815660:pca9552_led_set LED#1 state:1
564609@1592326881.815669:pca9552_led_set LED#2 state:1
564609@1592326881.815677:pca9552_led_set LED#3 state:1
564609@1592326881.815706:pca9552_led_set LED#4 state:0
564609@1592326881.815715:pca9552_led_set LED#5 state:0
564609@1592326881.815727:pca9552_led_set LED#6 state:0
564609@1592326881.815740:pca9552_led_set LED#7 state:0
564609@1592326881.815748:pca9552_led_set LED#8 state:0
564609@1592326881.815759:pca9552_led_set LED#9 state:0
564609@1592326881.815767:pca9552_led_set LED#10 state:0
564609@1592326881.815779:pca9552_led_set LED#11 state:0
564609@1592326881.815790:pca9552_led_set LED#12 state:0
564609@1592326881.815802:pca9552_led_set LED#13 state:1
564609@1592326881.815813:pca9552_led_set LED#14 state:1
564609@1592326881.815826:pca9552_led_set LED#15 state:1

I would instead simply dump the contents of the PCA9552_INPUT registers : 

LEDS = 1111000000000101
LEDS = 1111000000000111
....

C.


>> +            }
>>              break;
>>          case PCA9552_LED_OFF:
>>              s->regs[input_reg] &= ~(1 << input_shift);
>> +            if (input_shift < s->nr_leds) {
>> +                trace_pca9552_led_set(input_shift, false);
>> +            }
>>              break;
>>          case PCA9552_LED_PWM0:
>>          case PCA9552_LED_PWM1:
>> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
>> index 5561746866..ed80d0d1be 100644
>> --- a/hw/misc/trace-events
>> +++ b/hw/misc/trace-events
>> @@ -206,3 +206,6 @@ via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "secto
>>  # grlib_ahb_apb_pnp.c
>>  grlib_ahb_pnp_read(uint64_t addr, uint32_t value) "AHB PnP read addr:0x%03"PRIx64" data:0x%08x"
>>  grlib_apb_pnp_read(uint64_t addr, uint32_t value) "APB PnP read addr:0x%03"PRIx64" data:0x%08x"
>> +
>> +# pca9552.c
>> +pca9552_led_set(unsigned id, bool state) "LED#%d state:%u"
>>
> 



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

* Re: [PATCH 1/2] hw/misc/pca9552: Trace LED On/Off events
  2020-06-16 17:15     ` Cédric Le Goater
@ 2020-06-16 18:05       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-16 18:05 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel
  Cc: Peter Maydell, Andrew Jeffery, Joaquin de Andres, Esteban Bosse,
	qemu-arm, Joel Stanley, Guenter Roeck

On 6/16/20 7:15 PM, Cédric Le Goater wrote:
> After a closer look,
> 
>>> @@ -45,9 +46,15 @@ static void pca9552_update_pin_input(PCA9552State *s)
>>>          switch (config) {
>>>          case PCA9552_LED_ON:
>>>              s->regs[input_reg] |= 1 << input_shift;
>>> +            if (input_shift < s->nr_leds) {
> This seems like a superfluous test.

Indeed.

> 
>>> +                trace_pca9552_led_set(input_shift, true);
> 
> This should be using 'i' and the output is not very concise : 
> 
> 564609@1592326881.815616:pca9552_led_set LED#0 state:1
> 564609@1592326881.815660:pca9552_led_set LED#1 state:1
> 564609@1592326881.815669:pca9552_led_set LED#2 state:1
> 564609@1592326881.815677:pca9552_led_set LED#3 state:1
> 564609@1592326881.815706:pca9552_led_set LED#4 state:0
> 564609@1592326881.815715:pca9552_led_set LED#5 state:0
> 564609@1592326881.815727:pca9552_led_set LED#6 state:0
> 564609@1592326881.815740:pca9552_led_set LED#7 state:0
> 564609@1592326881.815748:pca9552_led_set LED#8 state:0
> 564609@1592326881.815759:pca9552_led_set LED#9 state:0
> 564609@1592326881.815767:pca9552_led_set LED#10 state:0
> 564609@1592326881.815779:pca9552_led_set LED#11 state:0
> 564609@1592326881.815790:pca9552_led_set LED#12 state:0
> 564609@1592326881.815802:pca9552_led_set LED#13 state:1
> 564609@1592326881.815813:pca9552_led_set LED#14 state:1
> 564609@1592326881.815826:pca9552_led_set LED#15 state:1
> 
> I would instead simply dump the contents of the PCA9552_INPUT registers : 
> 
> LEDS = 1111000000000101
> LEDS = 1111000000000111
> ....

Good idea, thanks!

> 
> C.
> 
> 
>>> +            }
>>>              break;
>>>          case PCA9552_LED_OFF:
>>>              s->regs[input_reg] &= ~(1 << input_shift);
>>> +            if (input_shift < s->nr_leds) {
>>> +                trace_pca9552_led_set(input_shift, false);
>>> +            }
>>>              break;
>>>          case PCA9552_LED_PWM0:
>>>          case PCA9552_LED_PWM1:
>>> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
>>> index 5561746866..ed80d0d1be 100644
>>> --- a/hw/misc/trace-events
>>> +++ b/hw/misc/trace-events
>>> @@ -206,3 +206,6 @@ via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "secto
>>>  # grlib_ahb_apb_pnp.c
>>>  grlib_ahb_pnp_read(uint64_t addr, uint32_t value) "AHB PnP read addr:0x%03"PRIx64" data:0x%08x"
>>>  grlib_apb_pnp_read(uint64_t addr, uint32_t value) "APB PnP read addr:0x%03"PRIx64" data:0x%08x"
>>> +
>>> +# pca9552.c
>>> +pca9552_led_set(unsigned id, bool state) "LED#%d state:%u"
>>>
>>
> 
> 


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

* Re: [PATCH 2/2] hw/misc/pca9552: Make LEDs 13-15 also GPIOs
  2020-06-16  9:45 ` [PATCH 2/2] hw/misc/pca9552: Make LEDs 13-15 also GPIOs Philippe Mathieu-Daudé
@ 2020-06-17  5:10   ` Cédric Le Goater
  0 siblings, 0 replies; 7+ messages in thread
From: Cédric Le Goater @ 2020-06-17  5:10 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Peter Maydell, Andrew Jeffery, Joaquin de Andres, Esteban Bosse,
	qemu-arm, Joel Stanley, Guenter Roeck

On 6/16/20 11:45 AM, Philippe Mathieu-Daudé wrote:
> The PCA9552 has 3 GPIOs, add them.

This is an example of an HW configuration. All pins can be used
as GPIOs. 
 
> See 'PCA9552 Product Datasheet Rev. 05 - 9 March 2006',
> chapter 6.4 'Pins used as GPIOs':
> 
>   LED pins not used to control LEDs can be used as general
>   purpose I/Os (GPIOs).
>   For use as input, set LEDn to high-impedance (01) and then
>   read the pin state via the input register.
>   For use as output, connect external pull-up resistor to the
>   pin and size it according to the DC recommended operating
>   characteristics. LED output pin is HIGH when the output is
>   programmed as high-impedance, and LOW when the output is
>   programmed LOW through the ‘LED selector’ register. The
>   output can be pulse-width controlled when PWM0 or PWM1 are
>   used.
>
>
> And chapter 8 'Application design-in information':
> 
>   LED0 to LED12 are used as LED drivers.
>   LED13 to LED15 are used as regular GPIOs.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/misc/pca9552.h |  2 ++
>  hw/misc/pca9552.c         | 18 ++++++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/include/hw/misc/pca9552.h b/include/hw/misc/pca9552.h
> index ebb43c63fe..7e47ea312d 100644
> --- a/include/hw/misc/pca9552.h
> +++ b/include/hw/misc/pca9552.h
> @@ -15,6 +15,7 @@
>  #define PCA9552(obj) OBJECT_CHECK(PCA9552State, (obj), TYPE_PCA9552)
>  
>  #define PCA9552_NR_REGS 10
> +#define PCA9552_NR_GPIOS 3
>  
>  typedef struct PCA9552State {
>      /*< private >*/
> @@ -27,6 +28,7 @@ typedef struct PCA9552State {
>      uint8_t regs[PCA9552_NR_REGS];
>      uint8_t max_reg;
>      uint8_t nr_leds;
> +    qemu_irq gpio[PCA9552_NR_GPIOS];
>  } PCA9552State;
>  
>  #endif
> diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
> index a3d0decbff..6ca6c0dbc2 100644
> --- a/hw/misc/pca9552.c
> +++ b/hw/misc/pca9552.c
> @@ -12,8 +12,10 @@
>  #include "qemu/osdep.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
> +#include "qemu/bitops.h"
>  #include "hw/misc/pca9552.h"
>  #include "hw/misc/pca9552_regs.h"
> +#include "hw/irq.h"
>  #include "migration/vmstate.h"
>  #include "qapi/error.h"
>  #include "qapi/visitor.h"
> @@ -48,12 +50,16 @@ static void pca9552_update_pin_input(PCA9552State *s)
>              s->regs[input_reg] |= 1 << input_shift;
>              if (input_shift < s->nr_leds) {
>                  trace_pca9552_led_set(input_shift, true);
> +            } else {
> +                qemu_set_irq(s->gpio[input_shift - s->nr_leds], 1);

It is simpler to set the irq level in pca9552_gpio_set().

>              }
>              break;
>          case PCA9552_LED_OFF:
>              s->regs[input_reg] &= ~(1 << input_shift);
>              if (input_shift < s->nr_leds) {
>                  trace_pca9552_led_set(input_shift, false);
> +            } else {
> +                qemu_set_irq(s->gpio[input_shift - s->nr_leds], 0);
>              }
>              break;
>          case PCA9552_LED_PWM0:
> @@ -65,6 +71,16 @@ static void pca9552_update_pin_input(PCA9552State *s)
>      }
>  }
>  
> +static void pca9552_gpio_set(void *opaque, int n, int enable)
> +{
> +    PCA9552State *s = opaque;
> +
> +    /* LED13 to LED15 are used as regular GPIOs. */
> +    s->regs[PCA9552_LS3] = deposit32(s->regs[PCA9552_LS3], n + 1, 1,
> +                                     enable ? PCA9552_LED_ON : PCA9552_LED_OFF);> +    pca9552_update_pin_input(s);

I would introduce a set_pin() helper instead. See pca9552_pin_get_config().
pca9552_gpio_set() would look like :

	pca9552_set_pin(s, n, enable ? PCA9552_LED_ON : PCA9552_LED_OFF);
	pca9552_update_pin_input(s);
	qemu_set_irq(s->gpio[n], enable);

> +}
> +
>  static uint8_t pca9552_read(PCA9552State *s, uint8_t reg)
>  {
>      switch (reg) {
> @@ -308,6 +324,8 @@ static void pca9552_initfn(Object *obj)
>                              NULL, NULL);
>          g_free(name);
>      }
> +    qdev_init_gpio_in(DEVICE(obj), pca9552_gpio_set, PCA9552_NR_GPIOS);
> +    qdev_init_gpio_out(DEVICE(obj), s->gpio, PCA9552_NR_GPIOS);
>  }
>  
>  static void pca9552_class_init(ObjectClass *klass, void *data)
> 



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

end of thread, other threads:[~2020-06-17  5:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-16  9:45 [PATCH 0/2] hw/misc/pca9552: Trace LEDs events, make LEDs 13-15 as GPIOs Philippe Mathieu-Daudé
2020-06-16  9:45 ` [PATCH 1/2] hw/misc/pca9552: Trace LED On/Off events Philippe Mathieu-Daudé
2020-06-16 16:13   ` Cédric Le Goater
2020-06-16 17:15     ` Cédric Le Goater
2020-06-16 18:05       ` Philippe Mathieu-Daudé
2020-06-16  9:45 ` [PATCH 2/2] hw/misc/pca9552: Make LEDs 13-15 also GPIOs Philippe Mathieu-Daudé
2020-06-17  5:10   ` 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.