From: Johan Hovold <johan@kernel.org>
To: Tony Chung <tony467913@gmail.com>
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org
Subject: Re: [PATCH 2/3] drivers: usb: serial: mos7840: added optimized register setups for common baudrates.
Date: Tue, 11 Feb 2025 11:16:37 +0100 [thread overview]
Message-ID: <Z6sjhS_1KY1jjdur@hovoldconsulting.com> (raw)
In-Reply-To: <20241024100901.69883-3-tony467913@gmail.com>
On Thu, Oct 24, 2024 at 06:09:03PM +0800, Tony Chung wrote:
> added optimized register setups for common baudrates.
First, how did you determine the base clocks here? Do you have access to
some documentation or did you just measure them?
These are all derived from the internal PLL IIUC?
> Signed-off-by: Tony Chung <tony467913@gmail.com>
> ---
> drivers/usb/serial/mos7840.c | 114 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 111 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
> index 362875a53..acc16737b 100644
> --- a/drivers/usb/serial/mos7840.c
> +++ b/drivers/usb/serial/mos7840.c
> @@ -1063,11 +1063,116 @@ static int mos7840_calc_baud_rate_divisor(struct usb_serial_port *port,
> {
> dev_dbg(&port->dev, "%s - %d\n", __func__, baudRate);
>
> - if (baudRate <= 115200) {
> + // divisor = (256*DLM)+DLL
> + // baudrate = InputCLK/(16*Divisor)
Style nit: Please avoid using c99 comments (here and below), just use a
block comment here and add spaces around the operators.
> + if (baudRate == 50) {
> + *divisor = (256*0x09)+0x04; // DLM=0x09, DLL=0x04
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 75) {
> + *divisor = (256*0x06)+0x02; // DLM=0x06, DLL=0x02
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 110) {
> + *divisor = (256*0x04)+0x19; // DLM=0x04, DLL=0x19
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 134) {
> + *divisor = (256*0x03)+0x5d; // DLM=0x03, DLL=0x5d
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 150) {
> + *divisor = (256*0x03)+0x01; // DLM=0x03, DLL=0x01
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 300) {
> + *divisor = (256*0x01)+0x81; // DLM=0x01, DLL=0x81
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 600) {
> + *divisor = 0xc0; // DLM=0, DLL=0xc0
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 1200) {
> + *divisor = 0x60; // DLM=0, DLL=0x60
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 1800) {
> + *divisor = 0x40; // DLM=0, DLL=0x40
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 2400) {
> + *divisor = 0x30; // DLM=0, DLL=0x30
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 4800) {
> + *divisor = 0x18; // DLM=0, DLL=0x18
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 7200) {
> + *divisor = 0x10; // DLM=0, DLL=0x10
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 9600) {
> + *divisor = 0x0c; // DLM=0, DLL=0x0c
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 14400) {
> + *divisor = 0x08; // DLM=0, DLL=0x08
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 19200) {
> + *divisor = 0x06; // DLM=0, DLL=0x06
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 28800) {
> + *divisor = 0x04; // DLM=0, DLL=0x04
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 38400) {
> + *divisor = 0x03; // DLM=0, DLL=0x03
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 57600) {
> + *divisor = 0x02; // DLM=0, DLL=0x02
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 115200) {
> + *divisor = 0x01; // DLM=0, DLL=0x01
> + *clk_sel_val = 0x0; // clock source = 1.846153846M
> + } else if (baudRate == 230400) {
> + *divisor = 0x01; // DLM=0, DLL=0x01
> + *clk_sel_val = 0x10; // clock source = 3.692307692M
> + } else if (baudRate == 460800) {
> + *divisor = 0x01; // DLM=0, DLL=0x01
> + *clk_sel_val = 0x30; // clock source = 7.384615384M
> + } else if (baudRate == 806400) {
> + *divisor = 0x01; // DLM=0, DLL=0x01
> + *clk_sel_val = 0x40; // clock source = 12.923076922M
> + } else if (baudRate == 921600) {
> + *divisor = 0x01; // DLM=0, DLL=0x01
> + *clk_sel_val = 0x50; // clock source = 14.769230768M
> + } else if (baudRate == 25000) {
> + *divisor = 0x78; // DLM=0, DLL=0x78
> + *clk_sel_val = 0x70; // clock source=48M
> + } else if (baudRate == 50000) {
> + *divisor = 0x3c; // DLM=0, DLL=0x3c
> + *clk_sel_val = 0x70; // clock source=48M
> + } else if (baudRate == 75000) {
> + *divisor = 0x28; // DLM=0, DLL=0x28
> + *clk_sel_val = 0x70; // clock source=48M
> + } else if (baudRate == 100000) {
> + *divisor = 0x1e; // DLM=0, DLL=0x1e
> + *clk_sel_val = 0x70; // clock source=48M
> + } else if (baudRate == 250000) {
> + *divisor = 0x0c; // DLM=0, DLL=0x0c
> + *clk_sel_val = 0x70; // clock source=48M
> + } else if (baudRate == 300000) {
> + *divisor = 0x0a; // DLM=0, DLL=0x0a
> + *clk_sel_val = 0x70; // clock source=48M
> + } else if (baudRate == 500000) {
> + *divisor = 0x06; // DLM=0, DLL=0x06
> + *clk_sel_val = 0x70; // clock source=48M
> + } else if (baudRate == 600000) {
> + *divisor = 0x05; // DLM=0, DLL=0x05
> + *clk_sel_val = 0x70; // clock source=48M
> + } else if (baudRate == 1000000) {
> + *divisor = 0x03; // DLM=0, DLL=0x03
> + *clk_sel_val = 0x70; // clock source=48M
> + } else if (baudRate == 3000000) {
> + *divisor = 0x01; // DLM=0, DLL=0x01
> + *clk_sel_val = 0x70; // clock source=48M
> +
> + } else if (baudRate == 1500000) {
> + *divisor = 0x01; // DLM=0, DLL=0x01
> + *clk_sel_val = 0x60; // clock source=24M
> +
You should be able to determine at least most of the above by just
calculating the divisors from the base clocks. Perhaps you need a lookup
table as well to determine which base to use in some cases too.
Note sure we need to add non-standard rates like 25000 etc either.
> + } else if (baudRate <= 115200) {
> *divisor = 115200 / baudRate;
> *clk_sel_val = 0x0;
> - }
> - if ((baudRate > 115200) && (baudRate <= 230400)) {
> + } else if ((baudRate > 115200) && (baudRate <= 230400)) {
> *divisor = 230400 / baudRate;
> *clk_sel_val = 0x10;
> } else if ((baudRate > 230400) && (baudRate <= 403200)) {
> @@ -1088,6 +1193,9 @@ static int mos7840_calc_baud_rate_divisor(struct usb_serial_port *port,
> } else if ((baudRate > 1572864) && (baudRate <= 3145728)) {
> *divisor = 3145728 / baudRate;
> *clk_sel_val = 0x70;
> + } else {
> + dev_dbg(&port->dev, "func: %s -baudrate %d not supported.\n", __func__, baudRate);
Here you could drop "func: %s -" and the period, if you need this at
all.
> + return -1;
The calling code does not check for errors, so either fix that or,
better still, make sure the function always returns a valid divisor
(e.g. by making sure the input rate is always valid).
> }
> return 0;
> }
Johan
next prev parent reply other threads:[~2025-02-11 10:16 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-24 10:08 [PATCH 0/3] drivers: usb: serial: mos7840: Improved baud rate support Tony Chung
2024-10-24 10:09 ` [PATCH 1/3] drivers: usb: serial: mos7840: Add defines for clock select register offset Tony Chung
2025-02-11 9:44 ` Johan Hovold
2024-10-24 10:09 ` [PATCH 2/3] drivers: usb: serial: mos7840: added optimized register setups for common baudrates Tony Chung
2025-02-11 10:16 ` Johan Hovold [this message]
2024-10-24 10:09 ` [PATCH 3/3] driver: usb: serial: mos7840: add more baudrate options Tony Chung
2025-02-11 13:08 ` Johan Hovold
2025-02-11 9:41 ` [PATCH 0/3] drivers: usb: serial: mos7840: Improved baud rate support Johan Hovold
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z6sjhS_1KY1jjdur@hovoldconsulting.com \
--to=johan@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=tony467913@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®