Hi Shannon, On 11/29/18 3:24 AM, Shannon Zhao wrote: > > > On 2018/11/29 1:26, Auger Eric wrote: >>>>> struct AcpiIortSmmu3 { >>>>> ACPI_IORT_NODE_HEADER_DEF >>>>> uint64_t base_address; >>>>> @@ -639,6 +645,8 @@ struct AcpiIortSmmu3 { >>>>> uint32_t pri_gsiv; >>>>> uint32_t gerr_gsiv; >>>>> uint32_t sync_gsiv; >>>>> + uint32_t pxm; >>> So if we use this field ,we need to set the flags with >>> ACPI_IORT_SMMU_V3_PXM_VALID >>>>> + uint32_t id_mapping_index; >>> And if we use this field, it needs to set the revision to at least 1. >> But is it harmful to add those fields in the struct as this patch does? >> >> - in our case ACPI_IORT_SMMU_V3_PXM_VALID flag is not set so the field >> value is ignored according to the spec and arm_smmu_v3_set_proximity() >> will not do anything. >> >> - SMMU control interrupts are all GSIV based so spec says that deviceID >> index is ignored. >> >> So eventually the fact the struct size is changing over the revisions >> does not look a problem because the node length is part of the struct >> and the offset to the ID array also is part of the structure. >> >> So I could have left the structure as before (because we don't use the >> functionalities associated to those fields). But on the other hand it's >> good to upgrade the struct according to Rev D now. >> >> So I think the patch is correct, isn't? > Yes, I think it's not harmful but it would be better to add some > comments to explain why we don't increase the revision number ATM. OK thank you for your input. I will repost for 3.2 and will upgrade the table structs and their associated revision fields.
Thanks Eric > > Thanks, > Shannon >