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

drivers/serial/uart_ns16550: CMD_SET_DLF should be removed #17064

Closed
ghost opened this issue Jun 25, 2019 · 4 comments
Closed

drivers/serial/uart_ns16550: CMD_SET_DLF should be removed #17064

ghost opened this issue Jun 25, 2019 · 4 comments
Labels
bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@ghost
Copy link

ghost commented Jun 25, 2019

This part of the uart drv_cmd API should be either public-facing or removed.

The value of the DLF is a function of the baud rate, and IMO shouldn't be manipulated independently. It should be accounted for in set_baud_rate() (if at all), so the API and orphaned header should be removed.

@ghost ghost added bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug labels Jun 25, 2019
@ghost ghost self-assigned this Jun 25, 2019
@dcpleung
Copy link
Member

IIRC, DLF is a Synopsys extension to 16550 on their DesignWare IP block. It is there to manipulate the clock divisor to get certain baud rate.

@ghost
Copy link
Author

ghost commented Jun 27, 2019

IIRC, DLF is a Synopsys extension to 16550 on their DesignWare IP block. It is there to manipulate the clock divisor to get certain baud rate.

Yeah. It's a trim value of sorts, which accounts for the fact that common baud rates can't be precisely derived from "nice" clocks (e.g., 25MHz). Currently there's a method to set this at boot (via devicetree) just as there's a boot-time baud rate. Arguably if the value is going to be dynamic (that is, the baud rate is going to change at run time), then set_baud_rate() should be modified to compute the right value or use a table or something, rather than a separate ioctl-style call.

@dcpleung
Copy link
Member

Maybe something like the quirks in Linux kernel would work here. But then the driver will need to identify the chips (via vendor/device IDs) to apply those quirks. It could get complicated as the ID table often needs to be expanded for new chips.

@ghost
Copy link
Author

ghost commented Jun 27, 2019

Well, I think we can rely on the current scheme or some minor modification thereof: just use devicetree to say "yes, use DLF on this port". We need not include any DLF-related code if the DT for the board/SoC doesn't mention it.

@ghost ghost closed this as completed Oct 18, 2019
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

No branches or pull requests

1 participant