diff mbox series

[1/2] acpi: ged: Add macro for acpi ged sleep register

Message ID 20240906021943.150494-2-maobibo@loongson.cn (mailing list archive)
State New
Headers show
Series Add FDT table support with acpi ged pm register | expand

Commit Message

maobibo Sept. 6, 2024, 2:19 a.m. UTC
Macro definition is added for acpi ged sleep register, so that ged
emulation driver can use this, also it can be used in FDT table if
ged is exposed with FDT table.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 hw/acpi/generic_event_device.c         | 6 +++---
 include/hw/acpi/generic_event_device.h | 3 +++
 2 files changed, 6 insertions(+), 3 deletions(-)

Comments

Michael S. Tsirkin Sept. 10, 2024, 5:32 p.m. UTC | #1
On Fri, Sep 06, 2024 at 10:19:42AM +0800, Bibo Mao wrote:
> Macro definition is added for acpi ged sleep register, so that ged
> emulation driver can use this, also it can be used in FDT table if
> ged is exposed with FDT table.
> 
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  hw/acpi/generic_event_device.c         | 6 +++---
>  include/hw/acpi/generic_event_device.h | 3 +++
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index 15b4c3ebbf..10a338877c 100644
> --- a/hw/acpi/generic_event_device.c
> +++ b/hw/acpi/generic_event_device.c
> @@ -201,9 +201,9 @@ static void ged_regs_write(void *opaque, hwaddr addr, uint64_t data,
>  
>      switch (addr) {
>      case ACPI_GED_REG_SLEEP_CTL:
> -        slp_typ = (data >> 2) & 0x07;
> -        slp_en  = (data >> 5) & 0x01;
> -        if (slp_en && slp_typ == 5) {
> +        slp_typ = (data & ACPI_GED_SLP_TYP_MASK) >> ACPI_GED_SLP_TYP_SHIFT;
> +        slp_en  = !!(data  & ACPI_GED_SLP_ENABLE);
> +        if (slp_en && slp_typ == ACPI_GED_SLP_TYP_S5) {
>              qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>          }
>          return;
> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
> index 40af3550b5..526fea6efe 100644
> --- a/include/hw/acpi/generic_event_device.h
> +++ b/include/hw/acpi/generic_event_device.h
> @@ -82,7 +82,10 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
>  #define ACPI_GED_RESET_VALUE       0x42
>  
>  /* ACPI_GED_REG_SLEEP_CTL.SLP_TYP value for S5 (aka poweroff) */
> +#define ACPI_GED_SLP_TYP_SHIFT     0x02
>  #define ACPI_GED_SLP_TYP_S5        0x05
> +#define ACPI_GED_SLP_TYP_MASK      0x1C
> +#define ACPI_GED_SLP_ENABLE        0x20

The comment is wrong now, isn't it?
Pls document each value, copying name from spec verbatim.

>  
>  #define GED_DEVICE      "GED"
>  #define AML_GED_EVT_REG "EREG"
> -- 
> 2.39.3
maobibo Sept. 11, 2024, 1 a.m. UTC | #2
On 2024/9/11 上午1:32, Michael S. Tsirkin wrote:
> On Fri, Sep 06, 2024 at 10:19:42AM +0800, Bibo Mao wrote:
>> Macro definition is added for acpi ged sleep register, so that ged
>> emulation driver can use this, also it can be used in FDT table if
>> ged is exposed with FDT table.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>   hw/acpi/generic_event_device.c         | 6 +++---
>>   include/hw/acpi/generic_event_device.h | 3 +++
>>   2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
>> index 15b4c3ebbf..10a338877c 100644
>> --- a/hw/acpi/generic_event_device.c
>> +++ b/hw/acpi/generic_event_device.c
>> @@ -201,9 +201,9 @@ static void ged_regs_write(void *opaque, hwaddr addr, uint64_t data,
>>   
>>       switch (addr) {
>>       case ACPI_GED_REG_SLEEP_CTL:
>> -        slp_typ = (data >> 2) & 0x07;
>> -        slp_en  = (data >> 5) & 0x01;
>> -        if (slp_en && slp_typ == 5) {
>> +        slp_typ = (data & ACPI_GED_SLP_TYP_MASK) >> ACPI_GED_SLP_TYP_SHIFT;
>> +        slp_en  = !!(data  & ACPI_GED_SLP_ENABLE);
>> +        if (slp_en && slp_typ == ACPI_GED_SLP_TYP_S5) {
>>               qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>>           }
>>           return;
>> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
>> index 40af3550b5..526fea6efe 100644
>> --- a/include/hw/acpi/generic_event_device.h
>> +++ b/include/hw/acpi/generic_event_device.h
>> @@ -82,7 +82,10 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
>>   #define ACPI_GED_RESET_VALUE       0x42
>>   
>>   /* ACPI_GED_REG_SLEEP_CTL.SLP_TYP value for S5 (aka poweroff) */
>> +#define ACPI_GED_SLP_TYP_SHIFT     0x02
>>   #define ACPI_GED_SLP_TYP_S5        0x05
>> +#define ACPI_GED_SLP_TYP_MASK      0x1C
>> +#define ACPI_GED_SLP_ENABLE        0x20
> 
> The comment is wrong now, isn't it?
> Pls document each value, copying name from spec verbatim.
The name comes from linux header files with little modification.
#define ACPI_X_WAKE_STATUS          0x80
#define ACPI_X_SLEEP_TYPE_MASK      0x1C
#define ACPI_X_SLEEP_TYPE_POSITION  0x02
#define ACPI_X_SLEEP_ENABLE         0x20

yeap, it will better if comes from spec verbatim. I will investigate and 
refresh the patch in next version.

Regards
Bibo Mao
> 
>>   
>>   #define GED_DEVICE      "GED"
>>   #define AML_GED_EVT_REG "EREG"
>> -- 
>> 2.39.3
diff mbox series

Patch

diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 15b4c3ebbf..10a338877c 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -201,9 +201,9 @@  static void ged_regs_write(void *opaque, hwaddr addr, uint64_t data,
 
     switch (addr) {
     case ACPI_GED_REG_SLEEP_CTL:
-        slp_typ = (data >> 2) & 0x07;
-        slp_en  = (data >> 5) & 0x01;
-        if (slp_en && slp_typ == 5) {
+        slp_typ = (data & ACPI_GED_SLP_TYP_MASK) >> ACPI_GED_SLP_TYP_SHIFT;
+        slp_en  = !!(data  & ACPI_GED_SLP_ENABLE);
+        if (slp_en && slp_typ == ACPI_GED_SLP_TYP_S5) {
             qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
         }
         return;
diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
index 40af3550b5..526fea6efe 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -82,7 +82,10 @@  OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
 #define ACPI_GED_RESET_VALUE       0x42
 
 /* ACPI_GED_REG_SLEEP_CTL.SLP_TYP value for S5 (aka poweroff) */
+#define ACPI_GED_SLP_TYP_SHIFT     0x02
 #define ACPI_GED_SLP_TYP_S5        0x05
+#define ACPI_GED_SLP_TYP_MASK      0x1C
+#define ACPI_GED_SLP_ENABLE        0x20
 
 #define GED_DEVICE      "GED"
 #define AML_GED_EVT_REG "EREG"