LinuxLists.cc - [PATCH v3 0/3] serial: 8250_dw: add fractional divisor support

2018-07-09 08:22:38

Subject: [PATCH v3 0/3] serial: 8250_dw: add fractional divisor support

For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
valid divisor latch fraction register. The fractional divisor width is
4bits ~ 6bits.

patch1 introduces necessary hooks to 8250 core.
patch2 exports serial8250_do_set_divisor()
patch3 implements the fractional divisor support for Synopsys DW 8250.

Since v2:
- rebase to tty-next branch, since I need one patch from Andy which
is in tty-next
- drop the patch "serial: 8250: let serial8250_get_divisor() get
uart_port * as param" since it's in tty-next now.
- add a new patch to export serial8250_do_set_divisor(), and reuse it
to complete dw8250_set_divisor().
- remove DW 8250 version check, since the DLF register always exists
and if fractional divisor isn't supported, the register read as 0
- add comments to explain how dw8250_get_divisor() get quot and frac.
- the frac calcuation is simplified with well implemented GENMASK
- Add Andy's Reviewed-by tag to patch1.

Since v1:
- add an extra patch to let serial8250_get_divisor() get uart_port *
as param
- take Andy's suggestions to "integrates hooks in the same way like
it's done for the rest of 8250 ones". Many thanks to Andy.

Jisheng Zhang (3):
serial: 8250: introduce get_divisor() and set_divisor() hook
serial: 8250: export serial8250_do_set_divisor()
serial: 8250_dw: add fractional divisor support

drivers/tty/serial/8250/8250_core.c | 4 +++
drivers/tty/serial/8250/8250_dw.c | 45 +++++++++++++++++++++++++++++
drivers/tty/serial/8250/8250_port.c | 30 +++++++++++++++----
include/linux/serial_8250.h | 3 ++
include/linux/serial_core.h | 7 +++++
5 files changed, 84 insertions(+), 5 deletions(-)

--
2.18.0

2018-07-09 08:23:41

by Jisheng Zhang

[permalink] [raw]

Subject: serial: 8250: introduce get_divisor() and set_divisor() hook

Add these two hooks so that they can be overridden with driver specific
implementations.

Signed-off-by: Jisheng Zhang <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_core.c | 4 ++++
drivers/tty/serial/8250/8250_port.c | 27 +++++++++++++++++++++++----
include/linux/serial_core.h | 7 +++++++
3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 9342fc2ee7df..a0bb77290747 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1023,6 +1023,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
uart->port.get_mctrl = up->port.get_mctrl;
if (up->port.set_mctrl)
uart->port.set_mctrl = up->port.set_mctrl;
+ if (up->port.get_divisor)
+ uart->port.get_divisor = up->port.get_divisor;
+ if (up->port.set_divisor)
+ uart->port.set_divisor = up->port.set_divisor;
if (up->port.startup)
uart->port.startup = up->port.startup;
if (up->port.shutdown)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 709fe6b4265c..ce0dc17f18ee 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2498,9 +2498,9 @@ static unsigned int npcm_get_divisor(struct uart_8250_port *up,
return DIV_ROUND_CLOSEST(port->uartclk, 16 * baud + 2) - 2;
}

-static unsigned int serial8250_get_divisor(struct uart_port *port,
- unsigned int baud,
- unsigned int *frac)
+static unsigned int serial8250_do_get_divisor(struct uart_port *port,
+ unsigned int baud,
+ unsigned int *frac)
{
struct uart_8250_port *up = up_to_u8250p(port);
unsigned int quot;
@@ -2532,6 +2532,16 @@ static unsigned int serial8250_get_divisor(struct uart_port *port,
return quot;
}

+static unsigned int serial8250_get_divisor(struct uart_port *port,
+ unsigned int baud,
+ unsigned int *frac)
+{
+ if (port->get_divisor)
+ return port->get_divisor(port, baud, frac);
+
+ return serial8250_do_get_divisor(port, baud, frac);
+}
+
static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
tcflag_t c_cflag)
{
@@ -2570,7 +2580,7 @@ static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
return cval;
}

-static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
+static void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
unsigned int quot, unsigned int quot_frac)
{
struct uart_8250_port *up = up_to_u8250p(port);
@@ -2603,6 +2613,15 @@ static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
}
}

+static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
+ unsigned int quot, unsigned int quot_frac)
+{
+ if (port->set_divisor)
+ port->set_divisor(port, baud, quot, quot_frac);
+ else
+ serial8250_do_set_divisor(port, baud, quot, quot_frac);
+}
+
static unsigned int serial8250_get_baud_rate(struct uart_port *port,
struct ktermios *termios,
struct ktermios *old)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 06ea4eeb09ab..406edae44ca3 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -127,6 +127,13 @@ struct uart_port {
struct ktermios *);
unsigned int (*get_mctrl)(struct uart_port *);
void (*set_mctrl)(struct uart_port *, unsigned int);
+ unsigned int (*get_divisor)(struct uart_port *,
+ unsigned int baud,
+ unsigned int *frac);
+ void (*set_divisor)(struct uart_port *,
+ unsigned int baud,
+ unsigned int quot,
+ unsigned int quot_frac);
int (*startup)(struct uart_port *port);
void (*shutdown)(struct uart_port *port);
void (*throttle)(struct uart_port *port);
--
2.18.0

2018-07-09 08:24:09

by Jisheng Zhang

[permalink] [raw]

Subject: [PATCH v3 1/3] serial: 8250: introduce get_divisor() and set_divisor() hook

2018-07-09 08:24:47

by Jisheng Zhang

[permalink] [raw]

Subject: Re: serial: 8250: introduce get_divisor() and set_divisor() hook

oops, sorry, please ignore this patch.

On Mon, 9 Jul 2018 16:19:22 +0800
Jisheng Zhang <[email protected]> wrote:

> Add these two hooks so that they can be overridden with driver specific
> implementations.
>
> Signed-off-by: Jisheng Zhang <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> ---
> drivers/tty/serial/8250/8250_core.c | 4 ++++
> drivers/tty/serial/8250/8250_port.c | 27 +++++++++++++++++++++++----
> include/linux/serial_core.h | 7 +++++++
> 3 files changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 9342fc2ee7df..a0bb77290747 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -1023,6 +1023,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
> uart->port.get_mctrl = up->port.get_mctrl;
> if (up->port.set_mctrl)
> uart->port.set_mctrl = up->port.set_mctrl;
> + if (up->port.get_divisor)
> + uart->port.get_divisor = up->port.get_divisor;
> + if (up->port.set_divisor)
> + uart->port.set_divisor = up->port.set_divisor;
> if (up->port.startup)
> uart->port.startup = up->port.startup;
> if (up->port.shutdown)
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 709fe6b4265c..ce0dc17f18ee 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -2498,9 +2498,9 @@ static unsigned int npcm_get_divisor(struct uart_8250_port *up,
> return DIV_ROUND_CLOSEST(port->uartclk, 16 * baud + 2) - 2;
> }
>
> -static unsigned int serial8250_get_divisor(struct uart_port *port,
> - unsigned int baud,
> - unsigned int *frac)
> +static unsigned int serial8250_do_get_divisor(struct uart_port *port,
> + unsigned int baud,
> + unsigned int *frac)
> {
> struct uart_8250_port *up = up_to_u8250p(port);
> unsigned int quot;
> @@ -2532,6 +2532,16 @@ static unsigned int serial8250_get_divisor(struct uart_port *port,
> return quot;
> }
>
> +static unsigned int serial8250_get_divisor(struct uart_port *port,
> + unsigned int baud,
> + unsigned int *frac)
> +{
> + if (port->get_divisor)
> + return port->get_divisor(port, baud, frac);
> +
> + return serial8250_do_get_divisor(port, baud, frac);
> +}
> +
> static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
> tcflag_t c_cflag)
> {
> @@ -2570,7 +2580,7 @@ static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
> return cval;
> }
>
> -static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
> +static void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
> unsigned int quot, unsigned int quot_frac)
> {
> struct uart_8250_port *up = up_to_u8250p(port);
> @@ -2603,6 +2613,15 @@ static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
> }
> }
>
> +static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
> + unsigned int quot, unsigned int quot_frac)
> +{
> + if (port->set_divisor)
> + port->set_divisor(port, baud, quot, quot_frac);
> + else
> + serial8250_do_set_divisor(port, baud, quot, quot_frac);
> +}
> +
> static unsigned int serial8250_get_baud_rate(struct uart_port *port,
> struct ktermios *termios,
> struct ktermios *old)
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 06ea4eeb09ab..406edae44ca3 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -127,6 +127,13 @@ struct uart_port {
> struct ktermios *);
> unsigned int (*get_mctrl)(struct uart_port *);
> void (*set_mctrl)(struct uart_port *, unsigned int);
> + unsigned int (*get_divisor)(struct uart_port *,
> + unsigned int baud,
> + unsigned int *frac);
> + void (*set_divisor)(struct uart_port *,
> + unsigned int baud,
> + unsigned int quot,
> + unsigned int quot_frac);
> int (*startup)(struct uart_port *port);
> void (*shutdown)(struct uart_port *port);
> void (*throttle)(struct uart_port *port);

2018-07-09 08:25:57

by Jisheng Zhang

[permalink] [raw]

Subject: [PATCH v3 2/3] serial: 8250: export serial8250_do_set_divisor()

Some drivers could call serial8250_do_set_divisor() to complete its
own set_divisor routine. Export this symbol for code reusing.

Signed-off-by: Jisheng Zhang <[email protected]>
---
drivers/tty/serial/8250/8250_port.c | 5 +++--
include/linux/serial_8250.h | 3 +++
2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index ce0dc17f18ee..945f8dc2d50f 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2580,8 +2580,8 @@ static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
return cval;
}

-static void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
- unsigned int quot, unsigned int quot_frac)
+void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
+ unsigned int quot, unsigned int quot_frac)
{
struct uart_8250_port *up = up_to_u8250p(port);

@@ -2612,6 +2612,7 @@ static void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
serial_port_out(port, 0x2, quot_frac);
}
}
+EXPORT_SYMBOL_GPL(serial8250_do_set_divisor);

static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
unsigned int quot, unsigned int quot_frac)
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 76b9db71e489..18e21427bce4 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -160,6 +160,9 @@ extern void serial8250_do_shutdown(struct uart_port *port);
extern void serial8250_do_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate);
extern void serial8250_do_set_mctrl(struct uart_port *port, unsigned int mctrl);
+extern void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
+ unsigned int quot,
+ unsigned int quot_frac);
extern int fsl8250_handle_irq(struct uart_port *port);
int serial8250_handle_irq(struct uart_port *port, unsigned int iir);
unsigned char serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr);
--
2.18.0

2018-07-09 08:27:10

by Jisheng Zhang

[permalink] [raw]

Subject: [PATCH v3 3/3] serial: 8250_dw: add fractional divisor support

For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
valid divisor latch fraction register. The fractional divisor width is
4bits ~ 6bits.

Now the preparation is done, it's easy to add the feature support.
This patch firstly tries to get the fractional divisor width during
probe, then setups dw specific get_divisor() and set_divisor() hook.

Signed-off-by: Jisheng Zhang <[email protected]>
---
drivers/tty/serial/8250/8250_dw.c | 45 +++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index fa8a00e8c9c6..e90c3d229f00 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -31,6 +31,7 @@

/* Offsets for the DesignWare specific registers */
#define DW_UART_USR 0x1f /* UART Status Register */
+#define DW_UART_DLF 0xc0 /* Divisor Latch Fraction Register */
#define DW_UART_CPR 0xf4 /* Component Parameter Register */
#define DW_UART_UCV 0xf8 /* UART Component Version */

@@ -55,6 +56,7 @@

struct dw8250_data {
u8 usr_reg;
+ u8 dlf_size;
int line;
int msr_mask_on;
int msr_mask_off;
@@ -366,6 +368,37 @@ static bool dw8250_idma_filter(struct dma_chan *chan, void *param)
return param == chan->device->dev->parent;
}

+/*
+ * divisor = clk / (16 * baud) = div(I) + div(F)
+ * "I" means integer, "F" means fractional
+ *
+ * 2^dlf_size * clk / (16 * baud) = 2^dlf_size * (div(I) + div(F))
+ * so, (clk << (dlf_siz - 4)) / baud = (div(I) + div(F)) << dlf_size
+ *
+ * let quot = DIV_ROUND_CLOSEST(clk << (dlf_size - 4), baud), we get
+ * div(I) = quot >> dlf_size
+ * div(F) = quot & dlf_mask, where dlf_mask = GENMASK(dlf_size - 1, 0)
+ */
+static unsigned int dw8250_get_divisor(struct uart_port *p,
+ unsigned int baud,
+ unsigned int *frac)
+{
+ unsigned int quot;
+ struct dw8250_data *d = p->private_data;
+
+ quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud);
+ *frac = quot & GENMASK(d->dlf_size - 1, 0);
+
+ return quot >> d->dlf_size;
+}
+
+static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
+ unsigned int quot, unsigned int quot_frac)
+{
+ dw8250_writel_ext(p, DW_UART_DLF, quot_frac);
+ serial8250_do_set_divisor(p, baud, quot, quot_frac);
+}
+
static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
{
if (p->dev->of_node) {
@@ -426,6 +459,18 @@ static void dw8250_setup_port(struct uart_port *p)
dev_dbg(p->dev, "Designware UART version %c.%c%c\n",
(reg >> 24) & 0xff, (reg >> 16) & 0xff, (reg >> 8) & 0xff);

+ dw8250_writel_ext(p, DW_UART_DLF, ~0U);
+ reg = dw8250_readl_ext(p, DW_UART_DLF);
+ dw8250_writel_ext(p, DW_UART_DLF, 0);
+
+ if (reg) {
+ struct dw8250_data *d = p->private_data;
+
+ d->dlf_size = fls(reg);
+ p->get_divisor = dw8250_get_divisor;
+ p->set_divisor = dw8250_set_divisor;
+ }
+
reg = dw8250_readl_ext(p, DW_UART_CPR);
if (!reg)
return;
--
2.18.0

2018-07-09 08:36:27

by Jisheng Zhang

[permalink] [raw]

Subject: Re: [PATCH v3 3/3] serial: 8250_dw: add fractional divisor support

Hi Andy,

On Mon, 9 Jul 2018 16:23:15 +0800 Jisheng Zhang wrote:

> For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
> valid divisor latch fraction register. The fractional divisor width is
> 4bits ~ 6bits.
>
> Now the preparation is done, it's easy to add the feature support.
> This patch firstly tries to get the fractional divisor width during
> probe, then setups dw specific get_divisor() and set_divisor() hook.
>
> Signed-off-by: Jisheng Zhang <[email protected]>
> ---
> drivers/tty/serial/8250/8250_dw.c | 45 +++++++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index fa8a00e8c9c6..e90c3d229f00 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -31,6 +31,7 @@
>
> /* Offsets for the DesignWare specific registers */
> #define DW_UART_USR 0x1f /* UART Status Register */
> +#define DW_UART_DLF 0xc0 /* Divisor Latch Fraction Register */
> #define DW_UART_CPR 0xf4 /* Component Parameter Register */
> #define DW_UART_UCV 0xf8 /* UART Component Version */
>
> @@ -55,6 +56,7 @@
>
> struct dw8250_data {
> u8 usr_reg;
> + u8 dlf_size;
> int line;
> int msr_mask_on;
> int msr_mask_off;
> @@ -366,6 +368,37 @@ static bool dw8250_idma_filter(struct dma_chan *chan, void *param)
> return param == chan->device->dev->parent;
> }
>
> +/*
> + * divisor = clk / (16 * baud) = div(I) + div(F)
> + * "I" means integer, "F" means fractional
> + *
> + * 2^dlf_size * clk / (16 * baud) = 2^dlf_size * (div(I) + div(F))
> + * so, (clk << (dlf_siz - 4)) / baud = (div(I) + div(F)) << dlf_size
> + *
> + * let quot = DIV_ROUND_CLOSEST(clk << (dlf_size - 4), baud), we get
> + * div(I) = quot >> dlf_size
> + * div(F) = quot & dlf_mask, where dlf_mask = GENMASK(dlf_size - 1, 0)
> + */
> +static unsigned int dw8250_get_divisor(struct uart_port *p,
> + unsigned int baud,
> + unsigned int *frac)
> +{
> + unsigned int quot;
> + struct dw8250_data *d = p->private_data;
> +
> + quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud);
> + *frac = quot & GENMASK(d->dlf_size - 1, 0);
> +
> + return quot >> d->dlf_size;

After more consideration, I sent out this version for the following two
points:

1. the max frac divisor width is 6bits now, but we dunno whether future IP
extends it or not. This patch version can still support > 6bits width except
the overflow concern. But fixing overflow(if there is) is as simple as using\
the DIV_ROUND_CLOSEST_ULL macro.

2. this version makes use of well implemented GENMASK to get the dlf_mask,
the micro is well understood, I think. so the code is simplified as well.

3. the magic "4" is explained in the comments, I hope it could help the
code.

what do you think?

Thanks in advance,
Jisheng

> +}
> +
> +static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
> + unsigned int quot, unsigned int quot_frac)
> +{
> + dw8250_writel_ext(p, DW_UART_DLF, quot_frac);
> + serial8250_do_set_divisor(p, baud, quot, quot_frac);
> +}
> +
> static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
> {
> if (p->dev->of_node) {
> @@ -426,6 +459,18 @@ static void dw8250_setup_port(struct uart_port *p)
> dev_dbg(p->dev, "Designware UART version %c.%c%c\n",
> (reg >> 24) & 0xff, (reg >> 16) & 0xff, (reg >> 8) & 0xff);
>
> + dw8250_writel_ext(p, DW_UART_DLF, ~0U);
> + reg = dw8250_readl_ext(p, DW_UART_DLF);
> + dw8250_writel_ext(p, DW_UART_DLF, 0);
> +
> + if (reg) {
> + struct dw8250_data *d = p->private_data;
> +
> + d->dlf_size = fls(reg);
> + p->get_divisor = dw8250_get_divisor;
> + p->set_divisor = dw8250_set_divisor;
> + }
> +
> reg = dw8250_readl_ext(p, DW_UART_CPR);
> if (!reg)
> return;

2018-07-10 02:10:36

by Jisheng Zhang

[permalink] [raw]

Subject: Re: [PATCH v3 3/3] serial: 8250_dw: add fractional divisor support

Hi Andy,

On Mon, 9 Jul 2018 16:32:41 +0800 Jisheng Zhang wrote:

> Hi Andy,
>
> On Mon, 9 Jul 2018 16:23:15 +0800 Jisheng Zhang wrote:
>
> > For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
> > valid divisor latch fraction register. The fractional divisor width is
> > 4bits ~ 6bits.
> >
> > Now the preparation is done, it's easy to add the feature support.
> > This patch firstly tries to get the fractional divisor width during
> > probe, then setups dw specific get_divisor() and set_divisor() hook.
> >
> > Signed-off-by: Jisheng Zhang <[email protected]>
> > ---
> > drivers/tty/serial/8250/8250_dw.c | 45 +++++++++++++++++++++++++++++++
> > 1 file changed, 45 insertions(+)
> >
> > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> > index fa8a00e8c9c6..e90c3d229f00 100644
> > --- a/drivers/tty/serial/8250/8250_dw.c
> > +++ b/drivers/tty/serial/8250/8250_dw.c
> > @@ -31,6 +31,7 @@
> >
> > /* Offsets for the DesignWare specific registers */
> > #define DW_UART_USR 0x1f /* UART Status Register */
> > +#define DW_UART_DLF 0xc0 /* Divisor Latch Fraction Register */
> > #define DW_UART_CPR 0xf4 /* Component Parameter Register */
> > #define DW_UART_UCV 0xf8 /* UART Component Version */
> >
> > @@ -55,6 +56,7 @@
> >
> > struct dw8250_data {
> > u8 usr_reg;
> > + u8 dlf_size;
> > int line;
> > int msr_mask_on;
> > int msr_mask_off;
> > @@ -366,6 +368,37 @@ static bool dw8250_idma_filter(struct dma_chan *chan, void *param)
> > return param == chan->device->dev->parent;
> > }
> >
> > +/*
> > + * divisor = clk / (16 * baud) = div(I) + div(F)
> > + * "I" means integer, "F" means fractional
> > + *
> > + * 2^dlf_size * clk / (16 * baud) = 2^dlf_size * (div(I) + div(F))
> > + * so, (clk << (dlf_siz - 4)) / baud = (div(I) + div(F)) << dlf_size
> > + *
> > + * let quot = DIV_ROUND_CLOSEST(clk << (dlf_size - 4), baud), we get
> > + * div(I) = quot >> dlf_size
> > + * div(F) = quot & dlf_mask, where dlf_mask = GENMASK(dlf_size - 1, 0)
> > + */
> > +static unsigned int dw8250_get_divisor(struct uart_port *p,
> > + unsigned int baud,
> > + unsigned int *frac)
> > +{
> > + unsigned int quot;
> > + struct dw8250_data *d = p->private_data;
> > +
> > + quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud);
> > + *frac = quot & GENMASK(d->dlf_size - 1, 0);
> > +
> > + return quot >> d->dlf_size;
>
> After more consideration, I sent out this version for the following two
> points:
>
> 1. the max frac divisor width is 6bits now, but we dunno whether future IP
> extends it or not. This patch version can still support > 6bits width except
> the overflow concern. But fixing overflow(if there is) is as simple as using\
> the DIV_ROUND_CLOSEST_ULL macro.
>
> 2. this version makes use of well implemented GENMASK to get the dlf_mask,
> the micro is well understood, I think. so the code is simplified as well.
>
> 3. the magic "4" is explained in the comments, I hope it could help the
> code.

I agree with your "making the code simple" concern. So I have a consideration
again. I think I made the code complex unnecessarily, the simplest get_divisor code
could be:

get_divisor()
{
unsigned int quot, rem;

quot = clk / (16 * baud);
rem = clk % (16 *baud);
*frac = DIV_ROUND_CLOSEST(rem << dlf_size, 16*baud);

return quot;
}

Compared with previous version, this one adds one more div instruction,
but remove several "shift, and" instructions, the performance isn't that bad.
From another side, even this version gets a trivial performance regression,
the get_divisor() doesn't sit on the hot code path. Making the code simpler is
more important.

I will send a new version.

Thanks