Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161596AbbKSVDz (ORCPT ); Thu, 19 Nov 2015 16:03:55 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:33137 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934121AbbKSVDx (ORCPT ); Thu, 19 Nov 2015 16:03:53 -0500 From: Laurent Pinchart To: Geert Uytterhoeven Cc: Greg Kroah-Hartman , Simon Horman , Magnus Damm , Yoshinori Sato , linux-serial@vger.kernel.org, linux-sh@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 18/25] serial: sh-sci: Prepare for multiple clocks and baud rate generators Date: Thu, 19 Nov 2015 23:04:02 +0200 Message-ID: <3383903.1R72kZGx62@avalon> User-Agent: KMail/4.14.8 (Linux/4.0.9-gentoo; KDE/4.14.8; x86_64; ; ) In-Reply-To: <1447958344-836-19-git-send-email-geert+renesas@glider.be> References: <1447958344-836-1-git-send-email-geert+renesas@glider.be> <1447958344-836-19-git-send-email-geert+renesas@glider.be> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10474 Lines: 330 Hi Geert, Thank you for the patch. On Thursday 19 November 2015 19:38:57 Geert Uytterhoeven wrote: > Refactor the clock and baud rate parameter code to ease adding support > for multiple clocks and baud rate generators later. > sci_scbrr_calc() now returns the bit rate error, so it can be compared > to the bit rate error for other baud rate generators. > > Signed-off-by: Geert Uytterhoeven > --- > drivers/tty/serial/sh-sci.c | 176 +++++++++++++++++++++++++++-------------- > 1 file changed, 120 insertions(+), 56 deletions(-) > > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c > index 726c96d5a511c222..12800e52f41953dc 100644 > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -2,6 +2,7 @@ > * SuperH on-chip serial module support. (SCI with no FIFO / with FIFO) > * > * Copyright (C) 2002 - 2011 Paul Mundt > + * Copyright (C) 2015 Glider bvba > * Modified to support SH7720 SCIF. Markus Brunner, Mark Jonas (Jul 2007). > * > * based off of the old drivers/char/sh-sci.c by: > @@ -76,6 +77,11 @@ enum { > ((port)->irqs[SCIx_ERI_IRQ] && \ > ((port)->irqs[SCIx_RXI_IRQ] < 0)) > > +enum SCI_CLKS { > + SCI_FCK, /* Functional Clock */ > + SCI_NUM_CLKS > +}; > + > struct sci_port { > struct uart_port port; > > @@ -92,8 +98,9 @@ struct sci_port { > struct timer_list break_timer; > int break_flag; > > - /* Function clock */ > - struct clk *fclk; > + /* Clocks */ > + struct clk *clks[SCI_NUM_CLKS]; > + unsigned long clk_rates[SCI_NUM_CLKS]; > > int irqs[SCIx_NR_IRQS]; > char *irqstr[SCIx_NR_IRQS]; > @@ -496,17 +503,24 @@ static int sci_probe_regmap(struct plat_sci_port *cfg) > > static void sci_port_enable(struct sci_port *sci_port) > { > + unsigned int i; > + > if (!sci_port->port.dev) > return; > > pm_runtime_get_sync(sci_port->port.dev); > > - clk_prepare_enable(sci_port->fclk); > - sci_port->port.uartclk = clk_get_rate(sci_port->fclk); > + for (i = 0; i < SCI_NUM_CLKS; i++) { > + clk_prepare_enable(sci_port->clks[i]); > + sci_port->clk_rates[i] = clk_get_rate(sci_port->clks[i]); > + } > + sci_port->port.uartclk = sci_port->clk_rates[SCI_FCK]; > } > > static void sci_port_disable(struct sci_port *sci_port) > { > + unsigned int i; > + > if (!sci_port->port.dev) > return; > > @@ -518,7 +532,8 @@ static void sci_port_disable(struct sci_port *sci_port) > del_timer_sync(&sci_port->break_timer); > sci_port->break_flag = 0; > > - clk_disable_unprepare(sci_port->fclk); > + for (i = SCI_NUM_CLKS; i-- > 0; ) > + clk_disable_unprepare(sci_port->clks[i]); > > pm_runtime_put_sync(sci_port->port.dev); > } > @@ -1657,6 +1672,7 @@ static int sci_notifier(struct notifier_block *self, > { > struct sci_port *sci_port; > unsigned long flags; > + unsigned int i; > > sci_port = container_of(self, struct sci_port, freq_transition); > > @@ -1664,7 +1680,9 @@ static int sci_notifier(struct notifier_block *self, > struct uart_port *port = &sci_port->port; > > spin_lock_irqsave(&port->lock, flags); > - port->uartclk = clk_get_rate(sci_port->fclk); > + for (i = 0; i < SCI_NUM_CLKS; i++) > + sci_port->clk_rates[i] = > + clk_get_rate(sci_port->clks[i]); > spin_unlock_irqrestore(&port->lock, flags); > } > > @@ -1907,11 +1925,12 @@ static void sci_shutdown(struct uart_port *port) > } > > /* calculate sample rate, BRR, and clock select */ > -static void sci_scbrr_calc(struct sci_port *s, unsigned int bps, > - unsigned long freq, int *brr, unsigned int *srr, > - unsigned int *cks) > +static int sci_scbrr_calc(struct sci_port *s, unsigned int bps, > + unsigned int *brr, unsigned int *srr, > + unsigned int *cks) > { > unsigned int min_sr, max_sr, shift, sr, br, a, b, c; > + unsigned long freq = s->clk_rates[SCI_FCK]; > int err, min_err = INT_MAX; > > if (s->sampling_rate) { > @@ -1963,6 +1982,7 @@ static void sci_scbrr_calc(struct sci_port *s, > unsigned int bps, > > dev_dbg(s->port.dev, "BRR: %u%+d bps using N %u SR %u cks %u\n", bps, > min_err, *brr, *srr + 1, *cks); > + return min_err; > } > > static void sci_reset(struct uart_port *port) > @@ -1984,11 +2004,14 @@ static void sci_reset(struct uart_port *port) > static void sci_set_termios(struct uart_port *port, struct ktermios > *termios, struct ktermios *old) > { > + unsigned int baud, smr_val = 0, scr_val = 0, i; > + unsigned int brr = 255, cks = 0, srr = 15; > + unsigned int brr1 = 255, cks1 = 0, srr1 = 15; > struct sci_port *s = to_sci_port(port); > const struct plat_sci_reg *reg; > - unsigned int baud, smr_val = 0, max_baud, cks = 0; > - int t = -1; > - unsigned int srr = 15; > + int min_err = INT_MAX, err; > + unsigned long max_freq = 0; > + int best_clk = -1; > > if ((termios->c_cflag & CSIZE) == CS7) > smr_val |= SCSMR_CHR; > @@ -2007,35 +2030,59 @@ static void sci_set_termios(struct uart_port *port, > struct ktermios *termios, * that the previous boot loader has enabled > required clocks and * setup the baud rate generator hardware for us > already. > */ > - if (port->uartclk) > - max_baud = port->uartclk / max(s->sampling_rate, 8U); > - else > - max_baud = 115200; > + if (!port->uartclk) { > + baud = uart_get_baud_rate(port, termios, old, 0, 115200); > + goto done; > + } > > - baud = uart_get_baud_rate(port, termios, old, 0, max_baud); > - if (likely(baud && port->uartclk)) > - sci_scbrr_calc(s, baud, port->uartclk, &t, &srr, &cks); > + for (i = 0; i < SCI_NUM_CLKS; i++) > + max_freq = max(max_freq, s->clk_rates[i]); > + > + baud = uart_get_baud_rate(port, termios, old, 0, > + max_freq / max(s->sampling_rate, 8U)); > + if (!baud) > + goto done; > + > + /* Functional Clock and standard Bit Rate Register */ > + err = sci_scbrr_calc(s, baud, &brr1, &srr1, &cks1); > + if (abs(err) < abs(min_err)) { > + best_clk = SCI_FCK; > + min_err = err; > + brr = brr1; > + srr = srr1; > + cks = cks1; > + } > + > +done: > + if (best_clk >= 0) > + dev_dbg(port->dev, "Using clk %pC for %u%+d bps\n", > + s->clks[best_clk], baud, min_err); > > sci_port_enable(s); > > sci_reset(port); > > - smr_val |= serial_port_in(port, SCSMR) & SCSMR_CKS; > - > uart_update_timeout(port, termios->c_cflag, baud); > > - dev_dbg(port->dev, "%s: SMR %x, cks %x, t %x, SCSCR %x\n", > - __func__, smr_val, cks, t, s->cfg->scscr); > - > - if (t >= 0) { > - serial_port_out(port, SCSMR, (smr_val & ~SCSMR_CKS) | cks); > - serial_port_out(port, SCBRR, t); > - reg = sci_getreg(port, HSSRR); > - if (reg->size) > + if (best_clk >= 0) { > + smr_val |= cks; > + dev_dbg(port->dev, "SMR 0x%x BRR %u SRR %u\n", smr_val, brr, > + srr); > + serial_port_out(port, SCSMR, smr_val); > + serial_port_out(port, SCBRR, brr); > + if (sci_getreg(port, HSSRR)->size) > serial_port_out(port, HSSRR, srr | HSCIF_SRE); > - udelay((1000000+(baud-1)) / baud); /* Wait one bit interval */ > - } else > + > + /* Wait one bit interval */ > + udelay((1000000 + (baud - 1)) / baud); > + } else { > + /* Don't touch the bit rate configuration */ > + scr_val = s->cfg->scscr & (SCSCR_CKE1 | SCSCR_CKE0); > + smr_val |= serial_port_in(port, SCSMR) & SCSMR_CKS; > + dev_dbg(port->dev, "SCR 0x%x SMR 0x%x\n", scr_val, smr_val); > + serial_port_out(port, SCSCR, scr_val); > serial_port_out(port, SCSMR, smr_val); > + } > > sci_init_pins(port, termios->c_cflag); > > @@ -2060,7 +2107,9 @@ static void sci_set_termios(struct uart_port *port, > struct ktermios *termios, serial_port_out(port, SCFCR, ctrl); > } > > - serial_port_out(port, SCSCR, s->cfg->scscr); > + scr_val |= s->cfg->scscr & ~(SCSCR_CKE1 | SCSCR_CKE0); > + dev_dbg(port->dev, "SCSCR 0x%x\n", scr_val); > + serial_port_out(port, SCSCR, scr_val); > > #ifdef CONFIG_SERIAL_SH_SCI_DMA > /* > @@ -2252,33 +2301,48 @@ static struct uart_ops sci_uart_ops = { > > static int sci_init_clocks(struct sci_port *sci_port, struct device *dev) > { > - /* Get the SCI functional clock. It's called "fck" on ARM. */ > - sci_port->fclk = devm_clk_get(dev, "fck"); > - if (PTR_ERR(sci_port->fclk) == -EPROBE_DEFER) > - return -EPROBE_DEFER; > - if (!IS_ERR(sci_port->fclk)) > - return 0; > + const char *clk_names[] = { > + [SCI_FCK] = "fck", > + }; > + struct clk *clk; > + unsigned int i; > > - /* > - * But it used to be called "sci_ick", and we need to maintain DT > - * backward compatibility. > - */ > - sci_port->fclk = devm_clk_get(dev, "sci_ick"); > - if (PTR_ERR(sci_port->fclk) == -EPROBE_DEFER) > - return -EPROBE_DEFER; > - if (!IS_ERR(sci_port->fclk)) > - return 0; > + for (i = 0; i < SCI_NUM_CLKS; i++) { > + clk = devm_clk_get(dev, clk_names[i]); > + if (PTR_ERR(clk) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > > - /* > - * Not all SH platforms declare a clock lookup entry for SCI devices, > - * in which case we need to get the global "peripheral_clk" clock. > - */ > - sci_port->fclk = devm_clk_get(dev, "peripheral_clk"); > - if (!IS_ERR(sci_port->fclk)) > - return 0; > + if (IS_ERR(clk) && i == SCI_FCK) { > + /* > + * "fck" used to be called "sci_ick", and we need to > + * maintain DT backward compatibility. > + */ > + clk = devm_clk_get(dev, "sci_ick"); > + if (PTR_ERR(clk) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + > + if (!IS_ERR(clk)) > + goto found; > + > + /* > + * Not all SH platforms declare a clock lookup entry > + * for SCI devices, in which case we need to get the > + * global "peripheral_clk" clock. > + */ > + clk = devm_clk_get(dev, "peripheral_clk"); > + if (!IS_ERR(clk)) > + goto found; > + > + dev_err(dev, "failed to get functional clock\n"); > + return PTR_ERR(clk); > + } > > - dev_err(dev, "failed to get functional clock\n"); > - return PTR_ERR(sci_port->fclk); > +found: > + if (!IS_ERR(clk)) > + dev_dbg(dev, "clk %u is %pC rate %pCr\n", i, clk, clk); > + sci_port->clks[i] = IS_ERR(clk) ? NULL : clk; Isn't it an issue that we can't tell apart the case where there is no clock specified in DT and the case where we can't get the clock due to another error ? > + } > + return 0; > } > > static int sci_init_single(struct platform_device *dev, -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/