[edk2-devel] [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config

[Kun Qin]

Hi Sami,

Thank you for the help! I agree that we can drop this patch and merge 
the rest when the window is open.

Pierre,

Thanks for your input on the usage as well!

Regards,
Kun

On 8/17/2022 5:06 AM, Sami Mujawar wrote:
> Hi Kun,
>
> I plan to get this series merged when the merge window opens.
>
> If you agree, I will drop this patch before merging. Please let me 
> know if that is ok.
>
> Regards,
>
> Sami Mujawar
>
> On 17/08/2022 09:53 am, Pierre Gondois wrote:
>>
>>
>> On 8/17/22 02:17, Kun Qin wrote:
>>> Hi Pierre,
>>>
>>> You are correct that if CM_ARM_PCI_ADDRESS_MAP_INFO.PCI_SS_CONFIG
>>> is no longer being used, this patch is not needed. Thanks for 
>>> catching this.
>>>
>>> On the other hand, just for my learning purpose, could you please 
>>> let me know
>>> what the use case for "PCI_SS_CONFIG" is? It does not seem to be 
>>> used at all.
>>
>> I haven't seen any usecase neither so far, but it is to be used to 
>> reference a
>> PCI bus/device/function/register location to identify/configure a device
>> from what I understood.
>>
>>>
>>> Thanks again for testing these patches!
>>>
>>> Regards,
>>> Kun
>>>
>>> On 8/16/2022 8:33 AM, Pierre Gondois wrote:
>>>> Hello Kun,
>>>>
>>>> Is this patch still required ?
>>>> Cf: https://edk2.groups.io/g/devel/message/92204
>>>>
>>>> The CM_ARM_PCI_CONFIG_SPACE_INFO struct should be enough to describe
>>>> the PCI ECAM, so CM_ARM_PCI_ADDRESS_MAP_INFO.SpaceCode being set to
>>>> PCI_SS_CONFIG should be an invalid case.
>>>> If not I don't think a v5 should be necessary.
>>>>
>>>> Also I ran the patchset on KvmTool and everything was working.
>>>>
>>>> Regards,
>>>> Pierre
>>>>
>>>> On 8/11/22 00:28, Kun Qin wrote:
>>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3998
>>>>>
>>>>> This change added a switch case handling for PCI_SS_CONFIG during 
>>>>> SSDT
>>>>> generation. This will allow PCI config case return EFI_SUCCESS 
>>>>> instead of
>>>>> EFI_INVALID_PARAMETER.
>>>>>
>>>>> Cc: Sami Mujawar <Sami.Mujawar at arm.com>
>>>>> Cc: Alexei Fedorov <Alexei.Fedorov at arm.com>
>>>>>
>>>>> Co-authored-by: Joe Lopez <joelopez at microsoft.com>
>>>>> Signed-off-by: Kun Qin <kuqin12 at gmail.com>
>>>>> Reviewed-by: Pierre Gondois <pierre.gondois at arm.com>
>>>>> Reviewed-by: Sami Mujawar <sami.mujawar at arm.com>
>>>>> ---
>>>>>
>>>>> Notes:
>>>>>      v2:
>>>>>      - Added Reviewed-by tag [Pierre]
>>>>>           v3:
>>>>>      - No change
>>>>>           v4:
>>>>>      - Added Reviewed-by tag [Sami]
>>>>>
>>>>> DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c 
>>>>> | 5 +++++
>>>>>   1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git 
>>>>> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c 
>>>>> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c 
>>>>>
>>>>> index dd75fc27e60e..c6fbd09c43f8 100644
>>>>> --- 
>>>>> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>>>>> +++ 
>>>>> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>>>>> @@ -606,6 +606,11 @@ GeneratePciCrs (
>>>>>                      );
>>>>>           break;
>>>>>   +      case PCI_SS_CONFIG:
>>>>> +        // Do nothing
>>>>> +        Status = EFI_SUCCESS;
>>>>> +        break;
>>>>> +
>>>>>         default:
>>>>>           Status = EFI_INVALID_PARAMETER;
>>>>>       } // switch
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92529): https://edk2.groups.io/g/devel/message/92529
Mute This Topic: https://groups.io/mt/92947269/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-