riscv-non-isa / riscv-brs

The Boot and Runtime Services (BRS) specification provides the software requirements for system vendors and Operating System Vendors (OSVs) to interoperate with one another by providing expectations for the Operating System (OS) to utilize in acts of device discovery, system management, and other rich operations provided in this specification.
https://jira.riscv.org/browse/RVG-48
Creative Commons Attribution 4.0 International
38 stars 13 forks source link

Define AML device ID/props for SPCR type 0x12 devices #150

Closed andreiw closed 5 months ago

andreiw commented 5 months ago

Define AML device ID/props for SPCR type 0x12 devices

https://github.com/riscv-non-isa/riscv-brs/issues/24

leiflindholm commented 5 months ago

I have probably missed some conversation, but how do these devices differ from a fully-compliant 16550, and how do they differ from similar components used by other architectures?

vlsunil commented 5 months ago

Hi Andrei, I have the same question as Leif. We use PNP0501 for standard 16550 compatible UART in ACPI (Yeah, it is giving me lot of trouble, but that's a different issue :-) ). I am not sure why we need new ACPI ID which would warrant driver changes in every OS.

andreiw commented 5 months ago

Because PNP0501 assumes a certain reg stride (how far apart the 1 byte UART regs are... on Arm systems these were common to be on 4-byte boundaries) and I/O access capabilities (again, some Arm systems in my past only supported 4 byte accesses). Then you can have a varying RX FIFO (not detectable) and of course the bane of any UART integration - arbitrary clocking (esp for UARTs that are meant to drive high-speed blocks like BT HC)

You could say - just define the properties you want, but that will still break every PNP0501 driver with such devices, because these new properties are not part of the PNP0501 contract. It's kinda like why 0x12 was added to SPCR - you can't just use 0x0000 and 0x0001, as these assume a port I/O UART and the PC compatible crystal...

What happened on Arm is everyone defined their own HIDs and there was no common CID. So every new platform got a new driver change with it. So this ID should make things simpler. We may add new properties in the future, but that will be okay. The contract is that anyone implementing RSCV0003 will have to keep up.

vlsunil commented 5 months ago

Because PNP0501 assumes a certain reg stride (how far apart the 1 byte UART regs are... on Arm systems these were common to be on 4-byte boundaries) and I/O access capabilities (again, some Arm systems in my past only supported 4 byte accesses). Then you can have a varying RX FIFO (not detectable) and of course the bane of any UART integration - arbitrary clocking (esp for UARTs that are meant to drive high-speed blocks like BT HC)

You could say - just define the properties you want, but that will still break every PNP0501 driver with such devices, because these new properties are not part of the PNP0501 contract. It's kinda like why 0x12 was added to SPCR - you can't just use 0x0000 and 0x0001, as these assume a port I/O UART and the PC compatible crystal...

As we speak, I can see Linux PNP UART driver is getting updated to handle this case. https://www.spinics.net/lists/kernel/msg5179311.html. Atleast, linux appears to handle this with this patch series which is already ACKd. This will require ACPI to use _DSD to communicate this though.

What happened on Arm is everyone defined their own HIDs and there was no common CID. So every new platform got a new driver change with it. So this ID should make things simpler. We may add new properties in the future, but that will be okay. The contract is that anyone implementing RSCV0003 will have to keep up.

I would love to have a different ACPI ID defined for standard 16550. PNP0501 is enumerated via PNP bus which has weird enumeration logic in linux causing lot of issues when we need to order its probe (i.e after interrupt controller). So, if we have ACPI ID which can be enumerated via platform bus, it will help RISC-V not to change any thing except write new platform driver (provided maintainers agree). What if same IP is used in other architectures? Can a generic CID be defined in ACPI spec itself later so that same driver written for RSCV0003 can be used?

andreiw commented 5 months ago

Because PNP0501 assumes a certain reg stride (how far apart the 1 byte UART regs are... on Arm systems these were common to be on 4-byte boundaries) and I/O access capabilities (again, some Arm systems in my past only supported 4 byte accesses). Then you can have a varying RX FIFO (not detectable) and of course the bane of any UART integration - arbitrary clocking (esp for UARTs that are meant to drive high-speed blocks like BT HC) You could say - just define the properties you want, but that will still break every PNP0501 driver with such devices, because these new properties are not part of the PNP0501 contract. It's kinda like why 0x12 was added to SPCR - you can't just use 0x0000 and 0x0001, as these assume a port I/O UART and the PC compatible crystal...

As we speak, I can see Linux PNP UART driver is getting updated to handle this case. https://www.spinics.net/lists/kernel/msg5179311.html. Atleast, linux appears to handle this with this patch series which is already ACKd. This will require ACPI to use _DSD to communicate this though.

RSCV0003 also relies on _DSD for properties.

What happened on Arm is everyone defined their own HIDs and there was no common CID. So every new platform got a new driver change with it. So this ID should make things simpler. We may add new properties in the future, but that will be okay. The contract is that anyone implementing RSCV0003 will have to keep up.

I would love to have a different ACPI ID defined for standard 16550. PNP0501 is enumerated via PNP bus which has weird enumeration logic in linux causing lot of issues when we need to order its probe (i.e after interrupt controller). So, if we have ACPI ID which can be enumerated via platform bus, it will help RISC-V not to change any thing except write new platform driver (provided maintainers agree). What if same IP is used in other architectures? Can a generic CID be defined in ACPI spec itself later so that same driver written for RSCV0003 can be used?

Yes, we are aligned.