| Message ID | a3f86a5075c565002cd4bed0befb7d167fa0f105.1406689494.git.lv.zheng@intel.com |
|---|---|
| State | Accepted |
| Commit | 54ea4247d389059674aaacad7af224459b135fb9 |
| Headers | show |
Hi Timur, oh, you already noticed this issue, sorry for the noise in my previous email. On 2015年05月02日 06:43, Timur Tabi wrote: > On Tue, Jul 29, 2014 at 11:21 PM, Lv Zheng <lv.zheng@intel.com> wrote: >> From: Tomasz Nowicki <tomasz.nowicki@linaro.org> >> >> New fields and new subtables. Tomasz Nowicki. >> tomasz.nowicki@linaro.org >> >> Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org> >> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> >> Signed-off-by: Bob Moore <robert.moore@intel.com> >> Signed-off-by: Lv Zheng <lv.zheng@intel.com> > > Hi, I know this patch is old, but something confuses me about it: > >> +/* Common GTDT subtable header */ >> + >> +struct acpi_gtdt_header { >> + u8 type; >> + u16 length; >> +}; > > I'm trying to write a function that parses the watchdog structure > (acpi_gtdt_watchdog). The first entry in that structure is > acpi_gtdt_header. Looking at the ACPI specification, I see that this > is correct: the type is one byte, and the length is two bytes. > > However, this means that I cannot use acpi_parse_entries() to parse > the watchdog subtable: > > int __init > acpi_parse_entries(char *id, unsigned long table_size, > acpi_tbl_entry_handler handler, > struct acpi_table_header *table_header, > int entry_id, unsigned int max_entries) > > acpi_tbl_entry_handler takes an acpi_subtable_header as its first > parameter. However, that structure looks like this: > > struct acpi_subtable_header { > u8 type; > u8 length; > }; > > This is not compatible, so I'm confused now. How do I properly parse > the watchdog subtable, if I cannot use acpi_parse_entries? This is really a pain. I noticed this when I was prototyping the code for GTDT in ACPI 5.1, and I even sent an email to Charles to ask him if there are mistakes here, but there are chances that more than 255 bytes in that structure so we must use u16 for the length. > > For context, here is my patch: > > http://www.spinics.net/lists/linux-watchdog/msg06240.html > > Scroll down to function arm_sbsa_wdt_parse_gtdt(). The typecast in > first line is invalid: > > + struct acpi_gtdt_watchdog *wdg = (struct acpi_gtdt_watchdog *)header; > > because of the mismatch. I don't know how to fix this. Yes, you can't use acpi_parse_entries() anymore, in my opinion, you can introduce a similar function as acpi_parse_entries(), you can refer to some code that parsing the structures with u16 length, the code I'm familiar it's the code for Intel DMAR (similar with ARM IORT), for DMAR table parsing, it has the same head as GTDT watchdog timer: /* DMAR subtable header */ struct acpi_dmar_header { u16 type; u16 length; }; please refer to drivers/iommu/dmar.c and drivers/iommu/intel-iommu.c, hope it helps. Thanks Hanjun > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h index c2295cc..41ad964 100644 --- a/include/acpi/actbl3.h +++ b/include/acpi/actbl3.h @@ -241,33 +241,96 @@ struct acpi_s3pt_suspend { /******************************************************************************* * - * GTDT - Generic Timer Description Table (ACPI 5.0) - * Version 1 + * GTDT - Generic Timer Description Table (ACPI 5.1) + * Version 2 * ******************************************************************************/ struct acpi_table_gtdt { struct acpi_table_header header; /* Common ACPI table header */ - u64 address; - u32 flags; - u32 secure_pl1_interrupt; - u32 secure_pl1_flags; - u32 non_secure_pl1_interrupt; - u32 non_secure_pl1_flags; + u64 counter_block_addresss; + u32 reserved; + u32 secure_el1_interrupt; + u32 secure_el1_flags; + u32 non_secure_el1_interrupt; + u32 non_secure_el1_flags; u32 virtual_timer_interrupt; u32 virtual_timer_flags; - u32 non_secure_pl2_interrupt; - u32 non_secure_pl2_flags; + u32 non_secure_el2_interrupt; + u32 non_secure_el2_flags; + u64 counter_read_block_address; + u32 platform_timer_count; + u32 platform_timer_offset; }; -/* Values for Flags field above */ +/* Flag Definitions: Timer Block Physical Timers and Virtual timers */ + +#define ACPI_GTDT_INTERRUPT_MODE (1) +#define ACPI_GTDT_INTERRUPT_POLARITY (1<<1) +#define ACPI_GTDT_ALWAYS_ON (1<<2) + +/* Common GTDT subtable header */ + +struct acpi_gtdt_header { + u8 type; + u16 length; +}; + +/* Values for GTDT subtable type above */ + +enum acpi_gtdt_type { + ACPI_GTDT_TYPE_TIMER_BLOCK = 0, + ACPI_GTDT_TYPE_WATCHDOG = 1, + ACPI_GTDT_TYPE_RESERVED = 2 /* 2 and greater are reserved */ +}; -#define ACPI_GTDT_MAPPED_BLOCK_PRESENT 1 +/* GTDT Subtables, correspond to Type in struct acpi_gtdt_header */ + +/* 0: Generic Timer Block */ + +struct acpi_gtdt_timer_block { + struct acpi_gtdt_header header; + u8 reserved; + u64 block_address; + u32 timer_count; + u32 timer_offset; +}; + +/* Timer Sub-Structure, one per timer */ + +struct acpi_gtdt_timer_entry { + u8 frame_number; + u8 reserved[3]; + u64 base_address; + u64 el0_base_address; + u32 timer_interrupt; + u32 timer_flags; + u32 virtual_timer_interrupt; + u32 virtual_timer_flags; + u32 common_flags; +}; + +/* Flag Definitions: common_flags above */ + +#define ACPI_GTDT_GT_IS_SECURE_TIMER (1) +#define ACPI_GTDT_GT_ALWAYS_ON (1<<1) + +/* 1: SBSA Generic Watchdog Structure */ + +struct acpi_gtdt_watchdog { + struct acpi_gtdt_header header; + u8 reserved; + u64 refresh_frame_address; + u64 control_frame_address; + u32 timer_interrupt; + u32 timer_flags; +}; -/* Values for all "TimerFlags" fields above */ +/* Flag Definitions: timer_flags above */ -#define ACPI_GTDT_INTERRUPT_MODE 1 -#define ACPI_GTDT_INTERRUPT_POLARITY 2 +#define ACPI_GTDT_WATCHDOG_IRQ_MODE (1) +#define ACPI_GTDT_WATCHDOG_IRQ_POLARITY (1<<1) +#define ACPI_GTDT_WATCHDOG_SECURE (1<<2) /******************************************************************************* *