Re: [PATCH v3 2/2] serial: 8250: Add new 8250-core based Broadcom STB driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12. 02. 21, 20:57, Al Cooper wrote:
Add a UART driver for the new Broadcom 8250 based STB UART. The new
UART is backward compatible with the standard 8250, but has some
additional features. The new features include a high accuracy baud
rate clock system and DMA support.

The driver will use the new optional BAUD MUX clock to select the best
one of the four master clocks (81MHz, 108MHz, 64MHz and 48MHz) to feed
the baud rate selection logic for any requested baud rate.  This allows
for more accurate BAUD rates when high speed baud rates are selected.
...
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_bcm7271.c
...
+static void brcmuart_rx_isr(struct uart_port *up, u32 rx_isr)
+{
+	struct brcmuart_priv *priv = up->private_data;
+	struct device *dev = up->dev;
+	u32 rx_done_isr;
+	u32 check_isr;
+	char seq_err[] = "RX buffer ready out of sequence, restarting RX DMA\n";

What's the purpose of this on-stack variable?

+static void init_real_clk_rates(struct device *dev, struct brcmuart_priv *priv)
+{
+	int x;
+	int rc;
+
+	priv->default_mux_rate = clk_get_rate(priv->baud_mux_clk);
+	dev_dbg(dev, "Default BAUD MUX Clock rate is %lu\n",
+		priv->default_mux_rate);
+
+	for (x = 0; x < ARRAY_SIZE(priv->real_rates); x++) {
+		if (priv->rate_table[x] == 0) {
+			priv->real_rates[x] = 0;
+			continue;
+		}
+		rc = clk_set_rate(priv->baud_mux_clk, priv->rate_table[x]);
+		if (rc) {
+			dev_err(dev, "Error selecting BAUD MUX clock for %u\n",
+				priv->rate_table[x]);
+			priv->real_rates[x] = priv->rate_table[x];
+		} else {
+			priv->real_rates[x] = clk_get_rate(priv->baud_mux_clk);
+		}
+	}
+	 clk_set_rate(priv->baud_mux_clk, priv->default_mux_rate);

This is only weirdly indented.

+}
+
+static void set_clock_mux(struct uart_port *up, struct brcmuart_priv *priv,
+			u32 baud)
+{
+	u32 percent;
+	u32 best_percent = UINT_MAX;
+	u32 quot;
+	u32 best_quot = 1;
+	u32 rate;
+	int best_index = -1;
+	u64 hires_rate;
+	u64 hires_baud;
+	u64 hires_err;
+	int rc;
+	int i;
+	int real_baud;
+
+	/* If the Baud Mux Clock was not specified, just return */
+	if (priv->baud_mux_clk == NULL)
+		return;
+
+	/* Find the closest match for specified baud */
+	for (i = 0; i < ARRAY_SIZE(priv->real_rates); i++) {
+		if (priv->real_rates[i] == 0)
+			continue;
+		rate = priv->real_rates[i] / 16;
+		quot = DIV_ROUND_CLOSEST(rate, baud);
+		if (!quot)
+			continue;
+
+		/* increase resolution to get xx.xx percent */
+		hires_rate = (u64)rate * 10000;
+		hires_baud = (u64)baud * 10000;
+
+		hires_err = div_u64(hires_rate, (u64)quot);
+
+		/* get the delta */
+		if (hires_err > hires_baud)
+			hires_err = (hires_err - hires_baud);
+		else
+			hires_err = (hires_baud - hires_err);
+
+		percent = (unsigned long)DIV_ROUND_CLOSEST_ULL(hires_err, baud);
+		dev_dbg(up->dev,
+			"Baud rate: %u, MUX Clk: %u, Error: %u.%u%%\n",
+			baud, priv->real_rates[i], percent / 100,
+			percent % 100);
+		if (percent < best_percent) {
+			best_percent = percent;
+			best_index = i;
+			best_quot = quot;
+		}
+	}
+	if (best_index == -1) {
+		dev_err(up->dev, "Error, %d BAUD rate is too fast.\n", baud);
+		return;
+	}
+	rate = priv->real_rates[best_index];
+	rc = clk_set_rate(priv->baud_mux_clk, rate);
+	if (rc)
+		dev_err(up->dev, "Error selecting BAUD MUX clock\n");
+
+	/* Error over 3 percent will cause data errors */
+	if (best_percent > 300)
+		dev_err(up->dev, "Error, baud: %d has %u.%u%% error\n",
+			baud, percent / 100, percent % 100);
+
+	real_baud = rate / 16 / best_quot;
+	dev_dbg(up->dev, "Selecting BAUD MUX rate: %u\n", rate);
+	dev_dbg(up->dev, "Requested baud: %u, Actual baud: %u\n",
+		baud, real_baud);
+
+	/* calc nanoseconds for 1.5 characters time at the given baud rate */
+	i = 1000000000 / real_baud / 10;

NSEC_PER_SEC here?

+	i += (i / 2);
+	priv->char_wait = ns_to_ktime(i);
+
+	up->uartclk = rate;
+}

...

+static int __maybe_unused brcmuart_resume(struct device *dev)
+{
+	struct brcmuart_priv *priv = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(priv->baud_mux_clk);
+	if (ret)
+		dev_err(dev, "Error enabling BAUD MUX clock\n");
+
+	/*
+	 * The hardware goes back to it's default after suspend
+	 * so get the "clk" back in sync.
+	 */
+	ret = clk_set_rate(priv->baud_mux_clk, priv->default_mux_rate);
+	if (ret)
+		dev_err(dev, "Error restoring default BAUD MUX clock\n");
+	if (priv->dma_enabled) {
+		brcmuart_arbitration(priv, 1);
+		brcmuart_init_dma_hardware(priv);
+		start_rx_dma(serial8250_get_port(priv->line));
+	}
+	serial8250_resume_port(priv->line);

All these cannot fail? Or the above can, so does proceeding further without an error make sense?

+	return 0;
+}
....
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -501,6 +501,7 @@ config SERIAL_8250_PXA
  	  applicable to both devicetree and legacy boards, and early console is
  	  part of its support.
+

Why adding a newline here?

  config SERIAL_8250_TEGRA
  	tristate "8250 support for Tegra serial ports"
  	default SERIAL_8250

regards,
--
js
suse labs



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux