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

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

[Jisheng Zhang]

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 v3:
 - simplify the dw8250_get_divisor() implementation again.

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

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

[Jisheng Zhang]

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

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 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

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

[Jisheng Zhang]

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 <Jisheng.Zhang@synaptics.com>
---
 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

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

[Jisheng Zhang]

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 <Jisheng.Zhang@synaptics.com>
---
 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..ad08d7a3b93b 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 = div(I) + div(F)
+ * "I" means integer, "F" means fractional
+ * quot = div(I) = clk / (16 * baud)
+ * frac = div(F) * 2^dlf_size
+ *
+ * let rem = clk % (16 * baud)
+ * we have: div(F) * (16 * baud) = rem
+ * so frac = 2^dlf_size * rem / (16 * baud) = (rem << dlf_size) / (16 * baud)
+ */
+static unsigned int dw8250_get_divisor(struct uart_port *p,
+				       unsigned int baud,
+				       unsigned int *frac)
+{
+	unsigned int quot, rem;
+	struct dw8250_data *d = p->private_data;
+
+	quot = p->uartclk / (16 * baud);
+	rem = p->uartclk % (16 * baud);
+	*frac = DIV_ROUND_CLOSEST(rem << d->dlf_size, 16 * baud);
+
+	return quot;
+}
+
+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

Re: [PATCH v4 2/3] serial: 8250: export serial8250_do_set_divisor()

[Andy Shevchenko]

On Tue, 2018-07-10 at 11:13 +0800, Jisheng Zhang wrote:
> Some drivers could call serial8250_do_set_divisor() to complete its
> own set_divisor routine. Export this symbol for code reusing.
> 

I dunno Greg's preferences here, but it could be merged with patch 1.

In any case,

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

for this part.

> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
>  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);

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

[Andy Shevchenko]

On Tue, 2018-07-10 at 11: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.

Thanks for an update, my comments below.

> +/*
> + * divisor = div(I) + div(F)
> + * "I" means integer, "F" means fractional
> + * quot = div(I) = clk / (16 * baud)
> + * frac = div(F) * 2^dlf_size
> + *
> + * let rem = clk % (16 * baud)
> + * we have: div(F) * (16 * baud) = rem
> + * so frac = 2^dlf_size * rem / (16 * baud) = (rem << dlf_size) / (16
> * baud)
> + */
> +static unsigned int dw8250_get_divisor(struct uart_port *p,
> +				       unsigned int baud,
> +				       unsigned int *frac)
> +{

unsigned int base_baud = baud * 16;

> +	unsigned int quot, rem;
> +	struct dw8250_data *d = p->private_data;
> +
> +	quot = p->uartclk / (16 * baud);
> +	rem = p->uartclk % (16 * baud);
> +	*frac = DIV_ROUND_CLOSEST(rem << d->dlf_size, 16 * baud);
> +

While it looks indeed better, I would rather like to have a confirmation
it's working as designed.

For example, when I did some calculus, I cooked a preliminary check in
Python (easy and fast to prototype), for example:
https://gist.github.com/andy-shev/06b084488b3629898121 in Python, or
commit 9df461eca18f ("spi: pxa2xx: replace ugly table by approximation")
in the kernel.

Or another one here https://gist.github.com/andy-shev/8b2a73aeca2874f4cc
89 and commits c1a67b48f6a5 ("serial: 8250_pci: replace switch-case by
formula for Intel MID"), 21947ba654a6 ("serial: 8250_pci: replace
switch-case by formula")

P.S. The code itself looks good to me, thanks!

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

[Jisheng Zhang]

Hi Andy,

On Tue, 10 Jul 2018 19:19:21 +0300 Andy Shevchenko wrote:

> On Tue, 2018-07-10 at 11: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.  
> 
> Thanks for an update, my comments below.
> 
> > +/*
> > + * divisor = div(I) + div(F)
> > + * "I" means integer, "F" means fractional
> > + * quot = div(I) = clk / (16 * baud)
> > + * frac = div(F) * 2^dlf_size
> > + *
> > + * let rem = clk % (16 * baud)
> > + * we have: div(F) * (16 * baud) = rem
> > + * so frac = 2^dlf_size * rem / (16 * baud) = (rem << dlf_size) / (16
> > * baud)
> > + */
> > +static unsigned int dw8250_get_divisor(struct uart_port *p,
> > +				       unsigned int baud,
> > +				       unsigned int *frac)
> > +{  
> 
> unsigned int base_baud = baud * 16;

Good point. will send a new version.

> 
> > +	unsigned int quot, rem;
> > +	struct dw8250_data *d = p->private_data;
> > +
> > +	quot = p->uartclk / (16 * baud);
> > +	rem = p->uartclk % (16 * baud);
> > +	*frac = DIV_ROUND_CLOSEST(rem << d->dlf_size, 16 * baud);
> > +  
> 
> While it looks indeed better, I would rather like to have a confirmation
> it's working as designed.
> 
> For example, when I did some calculus, I cooked a preliminary check in
> Python (easy and fast to prototype), for example:
> https://gist.github.com/andy-shev/06b084488b3629898121 in Python, or
> commit 9df461eca18f ("spi: pxa2xx: replace ugly table by approximation")
> in the kernel.
> 
> Or another one here https://gist.github.com/andy-shev/8b2a73aeca2874f4cc
> 89 and commits c1a67b48f6a5 ("serial: 8250_pci: replace switch-case by
> formula for Intel MID"), 21947ba654a6 ("serial: 8250_pci: replace
> switch-case by formula")

My python coding skill is limited. So I wrote a simple c program to
do the check for common clks and baudrate combination. All passed. I
paste the code here:


#include <stdio.h>
#include <assert.h>
#include <math.h>

#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))

#define DIV_ROUND_CLOSEST(x, divisor)(			\
{							\
	typeof(x) __x = x;				\
	typeof(divisor) __d = divisor;			\
	(((typeof(x))-1) > 0 ||				\
	 ((typeof(divisor))-1) > 0 ||			\
	 (((__x) > 0) == ((__d) > 0))) ?		\
		(((__x) + ((__d) / 2)) / (__d)) :	\
		(((__x) - ((__d) / 2)) / (__d));	\
}							\
)

static unsigned int baud[] = {9600, 19200, 38400, 57600, 115200, 230400,
				460800, 921600, 1843200, 3250000, 4454400,
				576000, 1152000, 500000, 1000000, 1500000,
				2000000, 2500000, 3000000, 3500000, 4000000};

static unsigned int clk[] = {25000000, 50000000, 100000000, 133000000, 200000000};

static void check(int baud, int clk, int dlf_size)
{
	unsigned int rem, frac, quot;
	unsigned int base_baud = baud * 16;
	float div, divf;

	quot = clk / base_baud;
	rem = clk % base_baud;
	frac = DIV_ROUND_CLOSEST(rem << dlf_size, base_baud);

	div = (float)clk / base_baud;
	divf = div - (int)div;
	divf *= (1 << dlf_size);

	assert(quot == (int)div);
	assert(frac == (int)round(divf));
	printf("checked %d %d %d %d %d\n", baud, clk, dlf_size, quot, frac);
}

int main()
{
	int i, j, k;

	for (i = 0; i < ARRAY_SIZE(baud); i++) {
		for (j = 0; j < ARRAY_SIZE(clk); j++) {
			for (k = 4; k <= 6; k++) {
				check(baud[i], clk[j], k);
			}
		}
	}

	return 0;
}

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

[Jisheng Zhang]

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 <Jisheng.Zhang@synaptics.com>
---
 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..5a60c4814d62 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 = div(I) + div(F)
+ * "I" means integer, "F" means fractional
+ * quot = div(I) = clk / (16 * baud)
+ * frac = div(F) * 2^dlf_size
+ *
+ * let rem = clk % (16 * baud)
+ * we have: div(F) * (16 * baud) = rem
+ * so frac = 2^dlf_size * rem / (16 * baud) = (rem << dlf_size) / (16 * baud)
+ */
+static unsigned int dw8250_get_divisor(struct uart_port *p,
+				       unsigned int baud,
+				       unsigned int *frac)
+{
+	unsigned int quot, rem, base_baud = baud * 16;
+	struct dw8250_data *d = p->private_data;
+
+	quot = p->uartclk / base_baud;
+	rem = p->uartclk % base_baud;
+	*frac = DIV_ROUND_CLOSEST(rem << d->dlf_size, base_baud);
+
+	return quot;
+}
+
+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

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

[Andy Shevchenko]

On Wed, 2018-07-11 at 15:11 +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.
> 

You would need to resend entire series as v6.
Don't forget to add given tags.

But, wait a bit, I would like to check the algo (thanks for C program!).

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

[Andy Shevchenko]

On Wed, 2018-07-11 at 14:41 +0800, Jisheng Zhang wrote:
> On Tue, 10 Jul 2018 19:19:21 +0300 Andy Shevchenko wrote:

> > > +/*
> > > + * divisor = div(I) + div(F)
> > > + * "I" means integer, "F" means fractional
> > > + * quot = div(I) = clk / (16 * baud)
> > > + * frac = div(F) * 2^dlf_size
> > > + *
> > > + * let rem = clk % (16 * baud)
> > > + * we have: div(F) * (16 * baud) = rem
> > > + * so frac = 2^dlf_size * rem / (16 * baud) = (rem << dlf_size) /
> > > (16
> > > * baud)
> > > + */

> > > +	quot = p->uartclk / (16 * baud);
> > > +	rem = p->uartclk % (16 * baud);
> > > +	*frac = DIV_ROUND_CLOSEST(rem << d->dlf_size, 16 * baud

> My python coding skill is limited. So I wrote a simple c program to
> do the check for common clks and baudrate combination. All passed. I
> paste the code here:
> 

OK, I wrote test case in Python:
https://gist.github.com/andy-shev/5e980f1d752617ba814725248556ac19

Looks good to me.

Please, send v6 and assume my Reviewed-by for entire series.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy