The latest patches to the DynamicTablesPkg help an SSDT generated to meet the SBBR requirement when an SPCR generation is desired.
But the auto-generated SSDT might be unable to describe the compatible but custom 16550 device on the non-SBBR compliant platform.
I wonder if an SSDT generation would be manageable when a user doesn't want to.

Thank you,
Irene

Hi Sami, Pierre, Alexei,
I wonder your thought about this topic.
Thank you,
Irene

To add a little more detail on what we were seeing our 16550 based serial has 4 byte spacing which the SPCR table is generated with correctly but then the dynamic table code creates a SSDT with the standard pnp hid/cid in the ssdt table which at least from my reading of the Linux driver looks like that only uses 1 byte spacing between registers. It is possible I missed something though.

Thanks,
Jeff

Get Outlook for Android<https://aka.ms/ghei36>

________________________________
From: Irene Park <ipark@...>
Sent: Wednesday, November 18, 2020 1:16:10 AM
To: discuss@edk2.groups.io <discuss@edk2.groups.io>; Irene Park <ipark@...>; Sami.Mujawar@... <Sami.Mujawar@...>; pierre.gondois@... <pierre.gondois@...>; Alexei Fedorov <Alexei.Fedorov@...>; Jeff Brasen <jbrasen@...>
Subject: RE: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

Hi Sami, Pierre, Alexei,
I wonder your thought about this topic.
Thank you,
Irene

-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of Irene Park
Sent: Tuesday, November 17, 2020 3:28 AM
To: discuss@edk2.groups.io
Subject: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

External email: Use caution opening links or attachments


The latest patches to the DynamicTablesPkg help an SSDT generated to meet the SBBR requirement when an SPCR generation is desired.
But the auto-generated SSDT might be unable to describe the compatible but custom 16550 device on the non-SBBR compliant platform.
I wonder if an SSDT generation would be manageable when a user doesn't want to.

Thank you,
Irene

Yes, that is the issue we are seeing.

Our COM acpi node has a different HID NVDA0100 which uses drivers\tty\serial\8250\8250_tegra.c in linux. It also has a _DSD node to expose the configured clock rate as that driver needs that as with device tree based bindings it gets that from the clock subsystem.

I did just test a forked copy of SsdtSerialPortFixupLib for our platfrom and it did work, but it seems like there would be an issue if someone wanted to use what is in DynamicPkg as the SPCR is marked as 4-byte access but the linux driver would use 1-byte accesses. Of course if we change the SPCR generation to be 1-byte that would break our systems table.


Thanks,

Jeff

________________________________
From: Sami Mujawar <Sami.Mujawar@...>
Sent: Thursday, November 19, 2020 10:31 AM
To: Jeff Brasen <jbrasen@...>; discuss@edk2.groups.io <discuss@edk2.groups.io>; Irene Park <ipark@...>; Pierre Gondois <Pierre.Gondois@...>; Alexei Fedorov <Alexei.Fedorov@...>; nd <nd@...>
Cc: Thanu Rangarajan <Thanu.Rangarajan@...>
Subject: RE: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

External email: Use caution opening links or attachments


Hi Irene, Jeff,



If I understand correctly, when the HID is PNP0501 the Linux driver ‘drivers\tty\serial\8250\8250_pnp.c’ is setting ‘uart.port.iotype = UPIO_MEM;’

This results in the read/write access size being 1 byte. Is this the problem you are seeing?



If so, are you intending to change the HID in your serial port SSDT? Or you are defining a property to specify the access size?



Please let me know. I am thinking if this problem can be solved more generically.



With regards to SSDT generation I can see a few options here:

1. Implement a SsdtSerialPortFixupLib for your platform.

This would mean implementing the interfaces in https://github.com/tianocore/edk2/blob/master/DynamicTablesPkg/Include/Library/SsdtSerialPortFixupLib.h



2. A Feature PCD DisableUartSsdtGeneration could be introduced with the default value being FALSE.



I would prefer Option 1 as it would keep the Dynamic Tables Core code SBBR compliant.



Regards,



Sami Mujawar



From: Jeff Brasen <jbrasen@...>
Sent: 18 November 2020 09:36 AM
To: discuss@edk2.groups.io; Irene Park <ipark@...>; Sami Mujawar <Sami.Mujawar@...>; Pierre Gondois <Pierre.Gondois@...>; Alexei Fedorov <Alexei.Fedorov@...>
Subject: Re: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg



To add a little more detail on what we were seeing our 16550 based serial has 4 byte spacing which the SPCR table is generated with correctly but then the dynamic table code creates a SSDT with the standard pnp hid/cid in the ssdt table which at least from my reading of the Linux driver looks like that only uses 1 byte spacing between registers. It is possible I missed something though.

Thanks,

Jeff

Get Outlook for Android<https://aka.ms/ghei36>



________________________________

From: Irene Park <ipark@...<mailto:ipark@...>>
Sent: Wednesday, November 18, 2020 1:16:10 AM
To: discuss@edk2.groups.io<mailto:discuss@edk2.groups.io> <discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>>; Irene Park <ipark@...<mailto:ipark@...>>; Sami.Mujawar@...<mailto:Sami.Mujawar@...> <Sami.Mujawar@...<mailto:Sami.Mujawar@...>>; pierre.gondois@...<mailto:pierre.gondois@...> <pierre.gondois@...<mailto:pierre.gondois@...>>; Alexei Fedorov <Alexei.Fedorov@...<mailto:Alexei.Fedorov@...>>; Jeff Brasen <jbrasen@...<mailto:jbrasen@...>>
Subject: RE: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg



Hi Sami, Pierre, Alexei,
I wonder your thought about this topic.
Thank you,
Irene

-----Original Message-----
From: discuss@edk2.groups.io<mailto:discuss@edk2.groups.io> <discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>> On Behalf Of Irene Park
Sent: Tuesday, November 17, 2020 3:28 AM
To: discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>
Subject: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

External email: Use caution opening links or attachments


The latest patches to the DynamicTablesPkg help an SSDT generated to meet the SBBR requirement when an SPCR generation is desired.
But the auto-generated SSDT might be unable to describe the compatible but custom 16550 device on the non-SBBR compliant platform.
I wonder if an SSDT generation would be manageable when a user doesn't want to.

Thank you,
Irene





IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

+Samer

From: Jeff Brasen <jbrasen@...>
Sent: 19 November 2020 10:17 PM
To: Sami Mujawar <Sami.Mujawar@...>; discuss@edk2.groups.io; Irene Park <ipark@...>; Pierre Gondois <Pierre.Gondois@...>; Alexei Fedorov <Alexei.Fedorov@...>; nd <nd@...>
Cc: Thanu Rangarajan <Thanu.Rangarajan@...>
Subject: Re: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

Yes, that is the issue we are seeing.

Our COM acpi node has a different HID NVDA0100 which uses drivers\tty\serial\8250\8250_tegra.c in linux. It also has a _DSD node to expose the configured clock rate as that driver needs that as with device tree based bindings it gets that from the clock subsystem.

I did just test a forked copy of SsdtSerialPortFixupLib for our platfrom and it did work, but it seems like there would be an issue if someone wanted to use what is in DynamicPkg as the SPCR is marked as 4-byte access but the linux driver would use 1-byte accesses. Of course if we change the SPCR generation to be 1-byte that would break our systems table.


Thanks,

Jeff

________________________________
From: Sami Mujawar <Sami.Mujawar@...<mailto:Sami.Mujawar@...>>
Sent: Thursday, November 19, 2020 10:31 AM
To: Jeff Brasen <jbrasen@...<mailto:jbrasen@...>>; discuss@edk2.groups.io<mailto:discuss@edk2.groups.io> <discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>>; Irene Park <ipark@...<mailto:ipark@...>>; Pierre Gondois <Pierre.Gondois@...<mailto:Pierre.Gondois@...>>; Alexei Fedorov <Alexei.Fedorov@...<mailto:Alexei.Fedorov@...>>; nd <nd@...<mailto:nd@...>>
Cc: Thanu Rangarajan <Thanu.Rangarajan@...<mailto:Thanu.Rangarajan@...>>
Subject: RE: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

External email: Use caution opening links or attachments


Hi Irene, Jeff,



If I understand correctly, when the HID is PNP0501 the Linux driver 'drivers\tty\serial\8250\8250_pnp.c' is setting 'uart.port.iotype = UPIO_MEM;'

This results in the read/write access size being 1 byte. Is this the problem you are seeing?



If so, are you intending to change the HID in your serial port SSDT? Or you are defining a property to specify the access size?



Please let me know. I am thinking if this problem can be solved more generically.



With regards to SSDT generation I can see a few options here:

1. Implement a SsdtSerialPortFixupLib for your platform.

This would mean implementing the interfaces in https://github.com/tianocore/edk2/blob/master/DynamicTablesPkg/Include/Library/SsdtSerialPortFixupLib.h



2. A Feature PCD DisableUartSsdtGeneration could be introduced with the default value being FALSE.



I would prefer Option 1 as it would keep the Dynamic Tables Core code SBBR compliant.



Regards,



Sami Mujawar



From: Jeff Brasen <jbrasen@...<mailto:jbrasen@...>>
Sent: 18 November 2020 09:36 AM
To: discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>; Irene Park <ipark@...<mailto:ipark@...>>; Sami Mujawar <Sami.Mujawar@...<mailto:Sami.Mujawar@...>>; Pierre Gondois <Pierre.Gondois@...<mailto:Pierre.Gondois@...>>; Alexei Fedorov <Alexei.Fedorov@...<mailto:Alexei.Fedorov@...>>
Subject: Re: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg



To add a little more detail on what we were seeing our 16550 based serial has 4 byte spacing which the SPCR table is generated with correctly but then the dynamic table code creates a SSDT with the standard pnp hid/cid in the ssdt table which at least from my reading of the Linux driver looks like that only uses 1 byte spacing between registers. It is possible I missed something though.

Thanks,

Jeff

Get Outlook for Android<https://aka.ms/ghei36>



________________________________

From: Irene Park <ipark@...<mailto:ipark@...>>
Sent: Wednesday, November 18, 2020 1:16:10 AM
To: discuss@edk2.groups.io<mailto:discuss@edk2.groups.io> <discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>>; Irene Park <ipark@...<mailto:ipark@...>>; Sami.Mujawar@...<mailto:Sami.Mujawar@...> <Sami.Mujawar@...<mailto:Sami.Mujawar@...>>; pierre.gondois@...<mailto:pierre.gondois@...> <pierre.gondois@...<mailto:pierre.gondois@...>>; Alexei Fedorov <Alexei.Fedorov@...<mailto:Alexei.Fedorov@...>>; Jeff Brasen <jbrasen@...<mailto:jbrasen@...>>
Subject: RE: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg



Hi Sami, Pierre, Alexei,
I wonder your thought about this topic.
Thank you,
Irene

-----Original Message-----
From: discuss@edk2.groups.io<mailto:discuss@edk2.groups.io> <discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>> On Behalf Of Irene Park
Sent: Tuesday, November 17, 2020 3:28 AM
To: discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>
Subject: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

External email: Use caution opening links or attachments


The latest patches to the DynamicTablesPkg help an SSDT generated to meet the SBBR requirement when an SPCR generation is desired.
But the auto-generated SSDT might be unable to describe the compatible but custom 16550 device on the non-SBBR compliant platform.
I wonder if an SSDT generation would be manageable when a user doesn't want to.

Thank you,
Irene



IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

I did a quick look in edk2-platforms, and found similar cases at least with a couple of other systems: Marvell Armada and OctenTx. Both of these use a _HID of "MRVL0001" (with its own clock-frequency _DSD) and a _CID of "HISI0031" to pickup tty/serial/8250/8250_dw.c, while publishing an SPCR indicating 16550 InterfaceType. See for example https://github.com/tianocore/edk2-platforms/tree/master/Silicon/Marvell/Armada7k8k/AcpiTables and https://github.com/tianocore/edk2-platforms/tree/master/Silicon/Marvell/OcteonTx/AcpiTables/T91

Other systems like the Socionext SynQuacer and the Hisilicon systems picks 8250_dw.c as well for one of the UART controllers, using CID of "HISI0031". However, the UART used in SPCR is another controller, which is a standard ARM PL011 UART ("ARMH0011") : https://github.com/tianocore/edk2-platforms/tree/master/Silicon/Socionext/SynQuacer/AcpiTables


Given the variations in the hardware implementations (and Linux 8250 drivers), and the fact they are not all strictly 16550 compatible, maybe SsdtSerialPortFixupLib can be flexible to allow platforms to select their own "16550" HID/CID (via PCDs for example)? This can be done while still defaulting to PNP0501/ PNP0500 for 16550 compatible UARTs.



From: Sami Mujawar <Sami.Mujawar@...>
Sent: Thursday, December 17, 2020 1:04 PM
To: Jeff Brasen (jbrasen@...) <jbrasen@...>; discuss@edk2.groups.io; Irene Park <ipark@...>; Pierre Gondois <Pierre.Gondois@...>; Alexei Fedorov <Alexei.Fedorov@...>; nd <nd@...>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Cc: Thanu Rangarajan <Thanu.Rangarajan@...>
Subject: RE: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

+Samer

From: Jeff Brasen <jbrasen@...<mailto:jbrasen@...>>
Sent: 19 November 2020 10:17 PM
To: Sami Mujawar <Sami.Mujawar@...<mailto:Sami.Mujawar@...>>; discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>; Irene Park <ipark@...<mailto:ipark@...>>; Pierre Gondois <Pierre.Gondois@...<mailto:Pierre.Gondois@...>>; Alexei Fedorov <Alexei.Fedorov@...<mailto:Alexei.Fedorov@...>>; nd <nd@...<mailto:nd@...>>
Cc: Thanu Rangarajan <Thanu.Rangarajan@...<mailto:Thanu.Rangarajan@...>>
Subject: Re: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

Yes, that is the issue we are seeing.

Our COM acpi node has a different HID NVDA0100 which uses drivers\tty\serial\8250\8250_tegra.c in linux. It also has a _DSD node to expose the configured clock rate as that driver needs that as with device tree based bindings it gets that from the clock subsystem.

I did just test a forked copy of SsdtSerialPortFixupLib for our platfrom and it did work, but it seems like there would be an issue if someone wanted to use what is in DynamicPkg as the SPCR is marked as 4-byte access but the linux driver would use 1-byte accesses. Of course if we change the SPCR generation to be 1-byte that would break our systems table.


Thanks,

Jeff

________________________________
From: Sami Mujawar <Sami.Mujawar@...<mailto:Sami.Mujawar@...>>
Sent: Thursday, November 19, 2020 10:31 AM
To: Jeff Brasen <jbrasen@...<mailto:jbrasen@...>>; discuss@edk2.groups.io<mailto:discuss@edk2.groups.io> <discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>>; Irene Park <ipark@...<mailto:ipark@...>>; Pierre Gondois <Pierre.Gondois@...<mailto:Pierre.Gondois@...>>; Alexei Fedorov <Alexei.Fedorov@...<mailto:Alexei.Fedorov@...>>; nd <nd@...<mailto:nd@...>>
Cc: Thanu Rangarajan <Thanu.Rangarajan@...<mailto:Thanu.Rangarajan@...>>
Subject: RE: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

External email: Use caution opening links or attachments


Hi Irene, Jeff,



If I understand correctly, when the HID is PNP0501 the Linux driver 'drivers\tty\serial\8250\8250_pnp.c' is setting 'uart.port.iotype = UPIO_MEM;'

This results in the read/write access size being 1 byte. Is this the problem you are seeing?



If so, are you intending to change the HID in your serial port SSDT? Or you are defining a property to specify the access size?



Please let me know. I am thinking if this problem can be solved more generically.



With regards to SSDT generation I can see a few options here:

1. Implement a SsdtSerialPortFixupLib for your platform.

This would mean implementing the interfaces in https://github.com/tianocore/edk2/blob/master/DynamicTablesPkg/Include/Library/SsdtSerialPortFixupLib.h



2. A Feature PCD DisableUartSsdtGeneration could be introduced with the default value being FALSE.



I would prefer Option 1 as it would keep the Dynamic Tables Core code SBBR compliant.



Regards,



Sami Mujawar



From: Jeff Brasen <jbrasen@...<mailto:jbrasen@...>>
Sent: 18 November 2020 09:36 AM
To: discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>; Irene Park <ipark@...<mailto:ipark@...>>; Sami Mujawar <Sami.Mujawar@...<mailto:Sami.Mujawar@...>>; Pierre Gondois <Pierre.Gondois@...<mailto:Pierre.Gondois@...>>; Alexei Fedorov <Alexei.Fedorov@...<mailto:Alexei.Fedorov@...>>
Subject: Re: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg



To add a little more detail on what we were seeing our 16550 based serial has 4 byte spacing which the SPCR table is generated with correctly but then the dynamic table code creates a SSDT with the standard pnp hid/cid in the ssdt table which at least from my reading of the Linux driver looks like that only uses 1 byte spacing between registers. It is possible I missed something though.

Thanks,

Jeff

Get Outlook for Android<https://aka.ms/ghei36>



________________________________

From: Irene Park <ipark@...<mailto:ipark@...>>
Sent: Wednesday, November 18, 2020 1:16:10 AM
To: discuss@edk2.groups.io<mailto:discuss@edk2.groups.io> <discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>>; Irene Park <ipark@...<mailto:ipark@...>>; Sami.Mujawar@...<mailto:Sami.Mujawar@...> <Sami.Mujawar@...<mailto:Sami.Mujawar@...>>; pierre.gondois@...<mailto:pierre.gondois@...> <pierre.gondois@...<mailto:pierre.gondois@...>>; Alexei Fedorov <Alexei.Fedorov@...<mailto:Alexei.Fedorov@...>>; Jeff Brasen <jbrasen@...<mailto:jbrasen@...>>
Subject: RE: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg



Hi Sami, Pierre, Alexei,
I wonder your thought about this topic.
Thank you,
Irene

-----Original Message-----
From: discuss@edk2.groups.io<mailto:discuss@edk2.groups.io> <discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>> On Behalf Of Irene Park
Sent: Tuesday, November 17, 2020 3:28 AM
To: discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>
Subject: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

External email: Use caution opening links or attachments


The latest patches to the DynamicTablesPkg help an SSDT generated to meet the SBBR requirement when an SPCR generation is desired.
But the auto-generated SSDT might be unable to describe the compatible but custom 16550 device on the non-SBBR compliant platform.
I wonder if an SSDT generation would be manageable when a user doesn't want to.

Thank you,
Irene



IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.