Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

andreiw
Copy link
Collaborator

@andreiw andreiw commented Apr 15, 2024

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

#24

@andreiw andreiw changed the title Spcr uart aml dev Define AML device ID/props for SPCR type 0x12 devices Apr 15, 2024
@leiflindholm
Copy link

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
Copy link
Collaborator

vlsunil commented Apr 16, 2024

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
Copy link
Collaborator Author

andreiw commented Apr 16, 2024

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
Copy link
Collaborator

vlsunil commented Apr 17, 2024

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
Copy link
Collaborator Author

andreiw commented Apr 17, 2024

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.

@andreiw andreiw merged commit 7bfa87e into riscv-non-isa:main Apr 17, 2024
2 checks passed
@andreiw andreiw deleted the spcr_uart_aml_dev branch April 17, 2024 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants