| Message ID | 20180704170310.56772d77@xhacker.debian (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2018-07-04 at 17:03 +0800, Jisheng Zhang wrote: Thanks for an update, my comments below. > For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a > valid divisor latch fraction register. The fractional divisor width is > 4bits ~ 6bits. I have read 4.00a spec a bit and didn't find this limitation. The fractional divider can fit up to 32 bits. > Now the preparation is done, it's easy to add the feature support. > This patch firstly checks the component version during probe, if > version >= 4.00a, then calculates the fractional divisor width, then > setups dw specific get_divisor() and set_divisor() hook. > +#define DW_FRAC_MIN_VERS 0x3430302A Do we really need this? My intuition (I, unfortunately, didn't find any evidence in Synopsys specs for UART) tells me that when it's unimplemented we would read back 0's, which is fine. I would test this a bit later. > > + unsigned int dlf_size:3; These bit fields (besides the fact you need 5) more or less for software quirk flags. In your case I would rather keep a u32 value of DFL mask as done for msr slightly above in this structure. > }; > > +/* > + * For version >= 4.00a, the dw uarts have a valid divisor latch > fraction > + * register. Calculate divisor with extra 4bits ~ 6bits fractional > portion. > + */ This comment kinda noise. Better to put actual formula from datasheet how this fractional divider is involved into calculus. > +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); If we have clock rate like 100MHz and 10 bits of fractional divider it would give an integer overflow. 4 here is a magic. Needs to be commented / described better. > + *frac = quot & (~0U >> (32 - d->dlf_size)); > + Operating on dfl_mask is simple as u64 quot = p->uartclk * (p->dfl_mask + 1); *frac = div64_u64(quot, baud * 16 * (p->dfl_mask + 1); return quot; (Perhaps some magic with types is needed, but you get the idea) > + return quot >> d->dlf_size; > +} > + > +static void dw8250_set_divisor(struct uart_port *p, unsigned int > baud, > + unsigned int quot, unsigned int > quot_frac) > +{ > + struct uart_8250_port *up = up_to_u8250p(p); > + > + serial_port_out(p, DW_UART_DLF >> p->regshift, quot_frac); It should use the helper, not playing tricks with serial_port_out(). > + serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB); > + serial_dl_write(up, quot); At some point it would be a helper, I think. We can call serial8250_do_set_divisor() here. So, perhaps we might export it. > +} > + > static void dw8250_quirks(struct uart_port *p, struct dw8250_data > *data) > { > if (p->dev->of_node) { > @@ -414,6 +445,28 @@ 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); > > + /* > + * For version >= 4.00a, the dw uarts have a valid divisor > latch > + * fraction register. Calculate the fractional divisor width. > + */ > + if (reg >= DW_FRAC_MIN_VERS) { > + struct dw8250_data *d = p->private_data; > + > + if (p->iotype == UPIO_MEM32BE) { > + iowrite32be(~0U, p->membase + DW_UART_DLF); > + reg = ioread32be(p->membase + DW_UART_DLF); > + } else { > + writel(~0U, p->membase + DW_UART_DLF); > + reg = readl(p->membase + DW_UART_DLF); > + } This should use some helpers. I'll prepare a patch soon and send it here, you may include it in your series. I think you need to clean up back them. So the flow like 1. Write all 1:s 2. Read back the value 3. Write all 0:s > + d->dlf_size = fls(reg); Just save value itself as dfl_mask. > + > + if (d->dlf_size) { > + p->get_divisor = dw8250_get_divisor; > + p->set_divisor = dw8250_set_divisor; > + } > + } > + > if (p->iotype == UPIO_MEM32BE) > reg = ioread32be(p->membase + DW_UART_CPR); > else
Hi Andy, On Wed, 04 Jul 2018 19:08:22 +0300 Andy Shevchenko wrote: > On Wed, 2018-07-04 at 17:03 +0800, Jisheng Zhang wrote: > > Thanks for an update, my comments below. > > > For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a > > valid divisor latch fraction register. The fractional divisor width is > > 4bits ~ 6bits. > > I have read 4.00a spec a bit and didn't find this limitation. > The fractional divider can fit up to 32 bits. the limitation isn't put in the register description, but in the description about fractional divisor width configure parameters. Searching DLF_SIZE will find it. From another side, 32bits fractional div is a waste, for example, let's assume the fract divisor = 0.9, if fractional width is 4 bits, DLF reg will be set to UP(0.9*2^4) = 15, the real frac divisor = 15/2^4 = 0.93; if fractional width is 32 bits, DLF reg will be set to UP(0.9*2^32) = 3865470567 the real frac divisor = 3865470567/2^32 = 0.90; > > > Now the preparation is done, it's easy to add the feature support. > > This patch firstly checks the component version during probe, if > > version >= 4.00a, then calculates the fractional divisor width, then > > setups dw specific get_divisor() and set_divisor() hook. > > > +#define DW_FRAC_MIN_VERS 0x3430302A > > Do we really need this? > > My intuition (I, unfortunately, didn't find any evidence in Synopsys > specs for UART) tells me that when it's unimplemented we would read back > 0's, which is fine. yeah, I agree with you. I will remove this version check in the new version > > I would test this a bit later. > > > > > > + unsigned int dlf_size:3; > > These bit fields (besides the fact you need 5) more or less for software > quirk flags. In your case I would rather keep a u32 value of DFL mask as > done for msr slightly above in this structure. OK. When setting the frac div, we use DLF size rather than mask, so could I only calculate the DLF size once then use an u8 value to hold the calculated DLF size rather than calculating every time? > > > }; > > > > +/* > > + * For version >= 4.00a, the dw uarts have a valid divisor latch > > fraction > > + * register. Calculate divisor with extra 4bits ~ 6bits fractional > > portion. > > + */ > > This comment kinda noise. Better to put actual formula from datasheet > how this fractional divider is involved into calculus. yeah. Will do. > > > +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); > > If we have clock rate like 100MHz and 10 bits of fractional divider it > would give an integer overflow. This depends on the fraction divider width. If it's only 4~6 bits, then we are fine. > > 4 here is a magic. Needs to be commented / described better. Yes. divisor = clk/(16*baud) = BRD(I) + BRD(F) "I" means integer, "F" means fractional 2^dlf_size*clk/(16*baud) = 2^dlf_size*(BRD(I) + BRD(F)) clk*2^(dlf_size - 4)/baud = 2^dlf_size*((BRD(I) + BRD(F)) the left part is "DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud)", let's assume it equals quot. then, BRD(I) = quot / 2^dlf_size = quot >> dlf_size, this is where "quot >> d->dlf_size" below from. BRD(F) = quot & dlf_mask = quot & (~0U >> (32 - dlf_size)) > > > + *frac = quot & (~0U >> (32 - d->dlf_size)); > > + > > Operating on dfl_mask is simple as > > u64 quot = p->uartclk * (p->dfl_mask + 1); Since the dlf_mask is always 2^n - 1, clk * (p->dlf_mask + 1) = clk << p->dlf_size, but the later is simpler > > *frac = div64_u64(quot, baud * 16 * (p->dfl_mask + 1); > return quot; quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud) *frac = quot & (~0U >> (32 - d->dlf_size)) return quot >> d->dlf_size; vs. quot = p->uartclk * (p->dfl_mask + 1); *frac = div64_u64(quot, baud * 16 * (p->dfl_mask + 1); return quot; shift vs mul. If the dlf width is only 4~6 bits, the first calculation can avoid 64bit div. I prefer the first calculation. > > (Perhaps some magic with types is needed, but you get the idea) > > > + return quot >> d->dlf_size; > > +} > > + > > +static void dw8250_set_divisor(struct uart_port *p, unsigned int > > baud, > > + unsigned int quot, unsigned int > > quot_frac) > > +{ > > + struct uart_8250_port *up = up_to_u8250p(p); > > + > > + serial_port_out(p, DW_UART_DLF >> p->regshift, quot_frac); > > It should use the helper, not playing tricks with serial_port_out(). I assume the helper here means the one you mentioned below, i.e in if (p->iotype == UPIO_MEM32BE) { ... } else { ... } > > > + serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB); > > + serial_dl_write(up, quot); > > At some point it would be a helper, I think. We can call > serial8250_do_set_divisor() here. So, perhaps we might export it. serial8250_do_set_divisor will drop the frac, that's not we want ;) > > > +} > > + > > static void dw8250_quirks(struct uart_port *p, struct dw8250_data > > *data) > > { > > if (p->dev->of_node) { > > @@ -414,6 +445,28 @@ 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); > > > > + /* > > + * For version >= 4.00a, the dw uarts have a valid divisor > > latch > > + * fraction register. Calculate the fractional divisor width. > > + */ > > + if (reg >= DW_FRAC_MIN_VERS) { > > + struct dw8250_data *d = p->private_data; > > + > > > + if (p->iotype == UPIO_MEM32BE) { > > + iowrite32be(~0U, p->membase + DW_UART_DLF); > > + reg = ioread32be(p->membase + DW_UART_DLF); > > + } else { > > + writel(~0U, p->membase + DW_UART_DLF); > > + reg = readl(p->membase + DW_UART_DLF); > > + } > > This should use some helpers. I'll prepare a patch soon and send it > here, you may include it in your series. Nice. Thanks. > > I think you need to clean up back them. So the flow like > > 1. Write all 1:s > 2. Read back the value > 3. Write all 0:s oh, yeah! will do > > > + d->dlf_size = fls(reg); > > Just save value itself as dfl_mask. we use the dlf size during calculation, so it's better to hold the dlf_size instead. Thanks for the kind review, Jisheng
On Thu, 5 Jul 2018 14:39:21 +0800 Jisheng Zhang wrote: <snip> > > > > > + serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB); > > > + serial_dl_write(up, quot); > > > > At some point it would be a helper, I think. We can call > > serial8250_do_set_divisor() here. So, perhaps we might export it. > > serial8250_do_set_divisor will drop the frac, that's not we want ;) > And most importantly, serial8250_do_set_divisor() will set a wrong BRD(I) for fractional capable DW uarts. For example, clk = 25MHZ, baud = 115200. In fractional capable DW uarts, we should set BRD(I) as 25000000/(16*115200) = 13 but serial8250_do_set_divisor() will set BRD(I) as DIV_ROUND_CLOSEST(25*1000000, 16*115200)) = 14 Thanks
On Thu, 2018-07-05 at 14:39 +0800, Jisheng Zhang wrote: > > On Wed, 2018-07-04 at 17:03 +0800, Jisheng Zhang wrote: My comments below. > > > For Synopsys DesignWare 8250 uart which version >= 4.00a, there's > > > a > > > valid divisor latch fraction register. The fractional divisor > > > width is > > > 4bits ~ 6bits. > > > > I have read 4.00a spec a bit and didn't find this limitation. > > The fractional divider can fit up to 32 bits. > > the limitation isn't put in the register description, but in the > description > about fractional divisor width configure parameters. Searching > DLF_SIZE will > find it. Found, thanks. > From another side, 32bits fractional div is a waste, for example, > let's > assume the fract divisor = 0.9, > if fractional width is 4 bits, DLF reg will be set to UP(0.9*2^4) = > 15, the > real frac divisor = 15/2^4 = 0.93; > if fractional width is 32 bits, DLF reg will be set to UP(0.9*2^32) = > 3865470567 > the real frac divisor = 3865470567/2^32 = 0.90; So, your example shows that 32-bit gives better value. Where is contradiction? > > I would test this a bit later. It reads back 0 on our hardware with 3.xx version of IP. > > > + unsigned int dlf_size:3; > > > > These bit fields (besides the fact you need 5) more or less for > > software > > quirk flags. In your case I would rather keep a u32 value of DFL > > mask as > > done for msr slightly above in this structure. > > OK. When setting the frac div, we use DLF size rather than mask, so > could > I only calculate the DLF size once then use an u8 value to hold the > calculated DLF size rather than calculating every time? Let's see below. > > > + quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), > > > baud); > > > > If we have clock rate like 100MHz and 10 bits of fractional divider > > it > > would give an integer overflow. > > This depends on the fraction divider width. If it's only 4~6 bits, > then we are fine. True. > > > > 4 here is a magic. Needs to be commented / described better. > > Yes. divisor = clk/(16*baud) = BRD(I) + BRD(F) > "I" means integer, "F" means fractional > > 2^dlf_size*clk/(16*baud) = 2^dlf_size*(BRD(I) + BRD(F)) > > clk*2^(dlf_size - 4)/baud = 2^dlf_size*((BRD(I) + BRD(F)) > > the left part is "DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), > baud)", > let's assume it equals quot. > > then, BRD(I) = quot / 2^dlf_size = quot >> dlf_size, this is where > "quot >> d->dlf_size" below from. > > BRD(F) = quot & dlf_mask = quot & (~0U >> (32 - dlf_size)) Yes, I understand from where it comes. It's a hard coded value of PS all over the serial code. And better use the same idiom as in other code, i.e. * 16 or / 16 depends on context. > > > + *frac = quot & (~0U >> (32 - d->dlf_size)); > > > + > > > > Operating on dfl_mask is simple as > > > > u64 quot = p->uartclk * (p->dfl_mask + 1); > > Since the dlf_mask is always 2^n - 1, > clk * (p->dlf_mask + 1) = clk << p->dlf_size, but the later > is simpler > > > > > *frac = div64_u64(quot, baud * 16 * (p->dfl_mask + 1); > > return quot; > > quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud) > *frac = quot & (~0U >> (32 - d->dlf_size)) > return quot >> d->dlf_size; > > vs. > > quot = p->uartclk * (p->dfl_mask + 1); > *frac = div64_u64(quot, baud * 16 * (p->dfl_mask + 1); > return quot; > > shift vs mul. > > If the dlf width is only 4~6 bits, the first calculation can avoid > 64bit div. I prefer the first calculation. OK, taking that into consideration and looking at the code snippets again I would to - keep two values // mask we get for free because it's needed in intermediate calculus // and it doesn't overflow u8 (4-6 bits by spec) u8 dlf_mask; u8 dlf_size; - implement function like below unsigned int quot; /* Consider maximum possible DLF_SIZE, i.e. 6 bits */ quot = DIV_ROUND_CLOSEST(p->uartclk * 4, baud); *frac = (quot >> (6 - dlf_size)) & dlf_mask; return quot >> dlf_size; Would you agree it looks slightly less complicated? > > > + serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB); > > > + serial_dl_write(up, quot); (1) > > > > At some point it would be a helper, I think. We can call > > serial8250_do_set_divisor() here. So, perhaps we might export it. > > serial8250_do_set_divisor will drop the frac, that's not we want ;) Can you check again? What I see is that for DW 8250 the _do_set_divisor() would be an equivalent to the two lines, i.e. (1). And basically at the end it should become just those two lines when the rest will implement their custom set_divisor(). > > This should use some helpers. I'll prepare a patch soon and send it > > here, you may include it in your series. > > Nice. Thanks. Sent. > > > + d->dlf_size = fls(reg); > > > > Just save value itself as dfl_mask. > > we use the dlf size during calculation, so it's better to hold the > dlf_size > instead. See above.
On Thu, 2018-07-05 at 14:54 +0800, Jisheng Zhang wrote: > On Thu, 5 Jul 2018 14:39:21 +0800 Jisheng Zhang wrote: > > <snip> > > > > > > > > + serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB); > > > > + serial_dl_write(up, quot); > > > > > > At some point it would be a helper, I think. We can call > > > serial8250_do_set_divisor() here. So, perhaps we might export > > > it. > > > > serial8250_do_set_divisor will drop the frac, that's not we want ;) > > > > And most importantly, serial8250_do_set_divisor() will set a wrong > BRD(I) > for fractional capable DW uarts. For example, clk = 25MHZ, baud = > 115200. > > In fractional capable DW uarts, we should set BRD(I) as > 25000000/(16*115200) = 13 > > but serial8250_do_set_divisor() will set BRD(I) as > DIV_ROUND_CLOSEST(25*1000000, 16*115200)) = 14 How come? It doesn't do any calculus (for DW 8250), it just writes few registers based on input.
On Fri, 6 Jul 2018 20:37:09 +0300 Andy Shevchenko wrote: > On Thu, 2018-07-05 at 14:39 +0800, Jisheng Zhang wrote: > > > On Wed, 2018-07-04 at 17:03 +0800, Jisheng Zhang wrote: > > My comments below. > > > > > For Synopsys DesignWare 8250 uart which version >= 4.00a, there's > > > > a > > > > valid divisor latch fraction register. The fractional divisor > > > > width is > > > > 4bits ~ 6bits. > > > > > > I have read 4.00a spec a bit and didn't find this limitation. > > > The fractional divider can fit up to 32 bits. > > > > the limitation isn't put in the register description, but in the > > description > > about fractional divisor width configure parameters. Searching > > DLF_SIZE will > > find it. > > Found, thanks. > > > From another side, 32bits fractional div is a waste, for example, > > let's > > assume the fract divisor = 0.9, > > if fractional width is 4 bits, DLF reg will be set to UP(0.9*2^4) = > > 15, the > > real frac divisor = 15/2^4 = 0.93; > > if fractional width is 32 bits, DLF reg will be set to UP(0.9*2^32) = > > 3865470567 > > the real frac divisor = 3865470567/2^32 = 0.90; > > So, your example shows that 32-bit gives better value. Where is > contradiction? The gain -- 0.03 is small, the pay is expensive ;) > > > > I would test this a bit later. > > It reads back 0 on our hardware with 3.xx version of IP. Thanks. So the ver check could be removed. > > > > > + unsigned int dlf_size:3; > > > > > > These bit fields (besides the fact you need 5) more or less for > > > software > > > quirk flags. In your case I would rather keep a u32 value of DFL > > > mask as > > > done for msr slightly above in this structure. > > > > OK. When setting the frac div, we use DLF size rather than mask, so > > could > > I only calculate the DLF size once then use an u8 value to hold the > > calculated DLF size rather than calculating every time? > > Let's see below. > > > > > + quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), > > > > baud); > > > > > > If we have clock rate like 100MHz and 10 bits of fractional divider > > > it > > > would give an integer overflow. > > > > This depends on the fraction divider width. If it's only 4~6 bits, > > then we are fine. > > True. > > > > > > > 4 here is a magic. Needs to be commented / described better. > > > > Yes. divisor = clk/(16*baud) = BRD(I) + BRD(F) > > "I" means integer, "F" means fractional > > > > 2^dlf_size*clk/(16*baud) = 2^dlf_size*(BRD(I) + BRD(F)) > > > > clk*2^(dlf_size - 4)/baud = 2^dlf_size*((BRD(I) + BRD(F)) > > > > the left part is "DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), > > baud)", > > let's assume it equals quot. > > > > then, BRD(I) = quot / 2^dlf_size = quot >> dlf_size, this is where > > "quot >> d->dlf_size" below from. > > > > BRD(F) = quot & dlf_mask = quot & (~0U >> (32 - dlf_size)) > > Yes, I understand from where it comes. It's a hard coded value of PS all > over the serial code. > > And better use the same idiom as in other code, i.e. * 16 or / 16 > depends on context. > > > > > + *frac = quot & (~0U >> (32 - d->dlf_size)); > > > > + > > > > > > Operating on dfl_mask is simple as > > > > > > u64 quot = p->uartclk * (p->dfl_mask + 1); > > > > Since the dlf_mask is always 2^n - 1, > > clk * (p->dlf_mask + 1) = clk << p->dlf_size, but the later > > is simpler > > > > > > > > *frac = div64_u64(quot, baud * 16 * (p->dfl_mask + 1); > > > return quot; > > > > quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud) > > *frac = quot & (~0U >> (32 - d->dlf_size)) > > return quot >> d->dlf_size; > > > > vs. > > > > quot = p->uartclk * (p->dfl_mask + 1); > > *frac = div64_u64(quot, baud * 16 * (p->dfl_mask + 1); > > return quot; > > > > shift vs mul. > > > > If the dlf width is only 4~6 bits, the first calculation can avoid > > 64bit div. I prefer the first calculation. > > OK, taking that into consideration and looking at the code snippets > again I would to > - keep two values > > // mask we get for free because it's needed in intermediate calculus > // > and it doesn't overflow u8 (4-6 bits by spec) > u8 dlf_mask; > u8 dlf_size; > > - implement function like below > > unsigned int quot; > > /* Consider maximum possible DLF_SIZE, i.e. 6 bits */ > quot = DIV_ROUND_CLOSEST(p->uartclk * 4, baud); > > *frac = (quot >> (6 - dlf_size)) & dlf_mask; > return quot >> dlf_size; > > Would you agree it looks slightly less complicated? Nice. I will follow this solution. > > > > > + serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB); > > > > + serial_dl_write(up, quot); > > (1) > > > > > > > At some point it would be a helper, I think. We can call > > > serial8250_do_set_divisor() here. So, perhaps we might export it. > > > > serial8250_do_set_divisor will drop the frac, that's not we want ;) > > Can you check again? What I see is that for DW 8250 the > _do_set_divisor() would be an equivalent to the two lines, i.e. (1). My bad, I mixed it with get_divisor. > > And basically at the end it should become just those two lines when the > rest will implement their custom set_divisor(). yes, makes sense. Will send a new version soon.
On Mon, Jul 9, 2018 at 9:04 AM, Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote: > On Fri, 6 Jul 2018 20:37:09 +0300 Andy Shevchenko wrote: > yes, makes sense. Will send a new version soon. Please, be sure you base it on top of tty-next and there is no warnings added (makes sense to run make W=1 as well),
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index aff04f1de3a5..b9630f633388 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -31,9 +31,12 @@ /* 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 */ +#define DW_FRAC_MIN_VERS 0x3430302A + /* Component Parameter Register bits */ #define DW_UART_CPR_ABP_DATA_WIDTH (3 << 0) #define DW_UART_CPR_AFCE_MODE (1 << 4) @@ -65,6 +68,7 @@ struct dw8250_data { unsigned int skip_autocfg:1; unsigned int uart_16550_compatible:1; + unsigned int dlf_size:3; }; static inline int dw8250_modify_msr(struct uart_port *p, int offset, int value) @@ -351,6 +355,33 @@ static bool dw8250_idma_filter(struct dma_chan *chan, void *param) return param == chan->device->dev->parent; } +/* + * For version >= 4.00a, the dw uarts have a valid divisor latch fraction + * register. Calculate divisor with extra 4bits ~ 6bits fractional portion. + */ +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 & (~0U >> (32 - d->dlf_size)); + + return quot >> d->dlf_size; +} + +static void dw8250_set_divisor(struct uart_port *p, unsigned int baud, + unsigned int quot, unsigned int quot_frac) +{ + struct uart_8250_port *up = up_to_u8250p(p); + + serial_port_out(p, DW_UART_DLF >> p->regshift, quot_frac); + serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB); + serial_dl_write(up, quot); +} + static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data) { if (p->dev->of_node) { @@ -414,6 +445,28 @@ 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); + /* + * For version >= 4.00a, the dw uarts have a valid divisor latch + * fraction register. Calculate the fractional divisor width. + */ + if (reg >= DW_FRAC_MIN_VERS) { + struct dw8250_data *d = p->private_data; + + if (p->iotype == UPIO_MEM32BE) { + iowrite32be(~0U, p->membase + DW_UART_DLF); + reg = ioread32be(p->membase + DW_UART_DLF); + } else { + writel(~0U, p->membase + DW_UART_DLF); + reg = readl(p->membase + DW_UART_DLF); + } + d->dlf_size = fls(reg); + + if (d->dlf_size) { + p->get_divisor = dw8250_get_divisor; + p->set_divisor = dw8250_set_divisor; + } + } + if (p->iotype == UPIO_MEM32BE) reg = ioread32be(p->membase + DW_UART_CPR); else
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 checks the component version during probe, if version >= 4.00a, then calculates the fractional divisor width, 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 | 53 +++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)