Skip to contentSkip to conversation

serial: add BCM283x mini UART driver

PATCH Superseded Stephen Warren 3 participants 5 messages Mar 17, 2016 – Apr 11, 2016

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 serial: add BCM283x mini UART driver

The RPi3 typically uses the regular UART for high-speed communication with the Bluetooth device, leaving us the mini UART to use for the serial console. Add support for this UART so we can use it.

---
(This will be a dependency for the RPi 3 patches, so it'd be good if it could make it into mainline pretty quickly if acceptable.)

 drivers/serial/Makefile                      |   1 +
 drivers/serial/serial_bcm283x_mu.c           | 138 +++++++++++++++++++++++++++
 include/dm/platform_data/serial_bcm283x_mu.h |  22 +++++
 3 files changed, 161 insertions(+)
 create mode 100644 drivers/serial/serial_bcm283x_mu.c
 create mode 100644 include/dm/platform_data/serial_bcm283x_mu.h
drivers/serial/Makefile +1 -0
index 05bdf56c6fe9..ee7147a77abc 100644
@@ -38,6 +38,7 @@ obj-$(CONFIG_UNIPHIER_SERIAL) += serial_uniphier.o
 obj-$(CONFIG_STM32_SERIAL) += serial_stm32.o
 obj-$(CONFIG_PIC32_SERIAL) += serial_pic32.o
 obj-$(CONFIG_STM32X7_SERIAL) += serial_stm32x7.o
+obj-$(CONFIG_BCM283X_MU_SERIAL) += serial_bcm283x_mu.o
 
 ifndef CONFIG_SPL_BUILD
 obj-$(CONFIG_USB_TTY) += usbtty.o
drivers/serial/serial_bcm283x_mu.c new file +138 -0

138 lines not on this page · raw message

include/dm/platform_data/serial_bcm283x_mu.h new file +22 -0
new file mode 100644
index 000000000000..441594579549
@@ -0,0 +1,22 @@
+/*
+ * (C) Copyright 2016 Stephen Warren <[email protected]>
+ *
+ * Derived from pl01x code:
+ * Copyright (c) 2014 Google, Inc
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __serial_bcm283x_mu_h
+#define __serial_bcm283x_mu_h
+
+/*
+ *Information about a serial port
+ *
+ * @base: Register base address
+ */
+struct bcm283x_mu_serial_platdata {
+	unsigned long base;
+};
+
+#endif
Signature · 1 line
2.7.3
#1 lore raw

PATCH 1/1 serial: add BCM283x mini UART driver

Hi Stephen,

Quoted 8 lines · Stephen Warren

The RPi3 typically uses the regular UART for high-speed communication with the Bluetooth device, leaving us the mini UART to use for the serial console. Add support for this UART so we can use it.

---
(This will be a dependency for the RPi 3 patches, so it'd be good if it could make it into mainline pretty quickly if acceptable.)

Not sure if this went in already. But see comment below.

Quoted 73 lines
 drivers/serial/Makefile                      |   1 +
 drivers/serial/serial_bcm283x_mu.c           | 138 +++++++++++++++++++++++++++
 include/dm/platform_data/serial_bcm283x_mu.h |  22 +++++
 3 files changed, 161 insertions(+)
 create mode 100644 drivers/serial/serial_bcm283x_mu.c
 create mode 100644 include/dm/platform_data/serial_bcm283x_mu.h
drivers/serial/Makefile +1 -0
index 05bdf56c6fe9..ee7147a77abc 100644
@@ -38,6 +38,7 @@ obj-$(CONFIG_UNIPHIER_SERIAL) += serial_uniphier.o
 obj-$(CONFIG_STM32_SERIAL) += serial_stm32.o
 obj-$(CONFIG_PIC32_SERIAL) += serial_pic32.o
 obj-$(CONFIG_STM32X7_SERIAL) += serial_stm32x7.o
+obj-$(CONFIG_BCM283X_MU_SERIAL) += serial_bcm283x_mu.o
 
 ifndef CONFIG_SPL_BUILD
 obj-$(CONFIG_USB_TTY) += usbtty.o
drivers/serial/serial_bcm283x_mu.c new file +47 -0
new file mode 100644
index 000000000000..97b57cc52163
@@ -0,0 +1,138 @@
+/*
+ * (C) Copyright 2016 Stephen Warren <[email protected]>
+ *
+ * Derived from pl01x code:
+ *
+ * (C) Copyright 2000
+ * Rob Taylor, Flying Pig Systems. robt at flyingpig.com.
+ *
+ * (C) Copyright 2004
+ * ARM Ltd.
+ * Philippe Robin, <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+/* Simple U-Boot driver for the BCM283x mini UART */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <watchdog.h>
+#include <asm/io.h>
+#include <serial.h>
+#include <dm/platform_data/serial_bcm283x_mu.h>
+#include <linux/compiler.h>
+#include <fdtdec.h>
+
+struct bcm283x_mu_regs {
+       u32 io;
+       u32 iir;
+       u32 ier;
+       u32 lcr;
+       u32 mcr;
+       u32 lsr;
+       u32 msr;
+       u32 scratch;
+       u32 cntl;
+       u32 stat;
+       u32 baud;
+};
+
+#define BCM283X_MU_LCR_DATA_SIZE_8     3
+
+#define BCM283X_MU_LSR_TX_IDLE         BIT(6)
+/* This actually means not full, but is named not empty in the docs */
+#define BCM283X_MU_LSR_TX_EMPTY                BIT(5)
+#define BCM283X_MU_LSR_RX_READY                BIT(0)

drivers/serial/serial_bcm283x_mu.c line 57

Quoted 10 lines
drivers/serial/serial_bcm283x_mu.c new file +10 -0
+
+struct bcm283x_mu_priv {
+       struct bcm283x_mu_regs *regs;
+};
+
+static int bcm283x_mu_serial_setbrg(struct udevice *dev, int baudrate)
+{
+       struct bcm283x_mu_priv *priv = dev_get_priv(dev);
+       struct bcm283x_mu_regs *regs = priv->regs;
+       /* FIXME: Get this from plat data later */

Or device tree?

Quoted 52 lines
drivers/serial/serial_bcm283x_mu.c new file +52 -0
+       u32 clock_rate = 250000000;
+       u32 divider;
+
+       divider = clock_rate / (baudrate * 8);
+
+       writel(BCM283X_MU_LCR_DATA_SIZE_8, &regs->lcr);
+       writel(divider - 1, &regs->baud);
+
+       return 0;
+}
+
+static int bcm283x_mu_serial_probe(struct udevice *dev)
+{
+       struct bcm283x_mu_serial_platdata *plat = dev_get_platdata(dev);
+       struct bcm283x_mu_priv *priv = dev_get_priv(dev);
+
+       priv->regs = (struct bcm283x_mu_regs *)plat->base;
+
+       return 0;
+}
+
+static int bcm283x_mu_serial_getc(struct udevice *dev)
+{
+       struct bcm283x_mu_priv *priv = dev_get_priv(dev);
+       struct bcm283x_mu_regs *regs = priv->regs;
+       u32 data;
+
+       /* Wait until there is data in the FIFO */
+       if (!(readl(&regs->lsr) & BCM283X_MU_LSR_RX_READY))
+               return -EAGAIN;
+
+       data = readl(&regs->io);
+
+       return (int)data;
+}
+
+static int bcm283x_mu_serial_putc(struct udevice *dev, const char data)
+{
+       struct bcm283x_mu_priv *priv = dev_get_priv(dev);
+       struct bcm283x_mu_regs *regs = priv->regs;
+
+       /* Wait until there is space in the FIFO */
+       if (!(readl(&regs->lsr) & BCM283X_MU_LSR_TX_EMPTY))
+               return -EAGAIN;
+
+       /* Send the character */
+       writel(data, &regs->io);
+
+       return 0;
+}
+
+static int bcm283x_mu_serial_pending(struct udevice *dev, bool input)

drivers/serial/serial_bcm283x_mu.c line 119

Quoted 10 lines
drivers/serial/serial_bcm283x_mu.c new file +10 -0
+{
+       struct bcm283x_mu_priv *priv = dev_get_priv(dev);
+       struct bcm283x_mu_regs *regs = priv->regs;
+       unsigned int lsr = readl(&regs->lsr);
+
+       if (input) {
+               WATCHDOG_RESET();
+               return lsr & BCM283X_MU_LSR_RX_READY;
+       } else {
+               return !(lsr & BCM283X_MU_LSR_TX_IDLE);

These look like flags - be care to return 1 if there is an unknown number of characters, rather than (e.g. 4). The latter might cause the uclass to expect 4 characters to be present.

Suggest putting ? 1 : 0 or ? 0 : 1 on the end.

Quoted 48 lines
drivers/serial/serial_bcm283x_mu.c new file +19 -0
+       }
+}
+
+static const struct dm_serial_ops bcm283x_mu_serial_ops = {
+       .putc = bcm283x_mu_serial_putc,
+       .pending = bcm283x_mu_serial_pending,
+       .getc = bcm283x_mu_serial_getc,
+       .setbrg = bcm283x_mu_serial_setbrg,
+};
+
+U_BOOT_DRIVER(serial_bcm283x_mu) = {
+       .name = "serial_bcm283x_mu",
+       .id = UCLASS_SERIAL,
+       .platdata_auto_alloc_size = sizeof(struct bcm283x_mu_serial_platdata),
+       .probe = bcm283x_mu_serial_probe,
+       .ops = &bcm283x_mu_serial_ops,
+       .flags = DM_FLAG_PRE_RELOC,
+       .priv_auto_alloc_size = sizeof(struct bcm283x_mu_priv),
+};
include/dm/platform_data/serial_bcm283x_mu.h new file +22 -1
new file mode 100644
index 000000000000..441594579549
@@ -0,0 +1,22 @@
+/*
+ * (C) Copyright 2016 Stephen Warren <[email protected]>
+ *
+ * Derived from pl01x code:
+ * Copyright (c) 2014 Google, Inc
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#ifndef __serial_bcm283x_mu_h
+#define __serial_bcm283x_mu_h
+
+/*
+ *Information about a serial port
+ *
+ * @base: Register base address
+ */
+struct bcm283x_mu_serial_platdata {
+       unsigned long base;
+};
+
+#endif
--
Quoted 2 lines

2.7.3

Regards,
Simon

#2 lore raw

PATCH 1/1 serial: add BCM283x mini UART driver

Quoted 7 lines · Simon Glass

Hi Stephen,

Quoted 4 lines · Stephen Warren

The RPi3 typically uses the regular UART for high-speed communication with the Bluetooth device, leaving us the mini UART to use for the serial console. Add support for this UART so we can use it.

Quoted 8 lines
Quoted 4 lines

---
(This will be a dependency for the RPi 3 patches, so it'd be good if it could make it into mainline pretty quickly if acceptable.)

Not sure if this went in already. But see comment below.

It has, but I can send a fix.

Quoted 10 lines

drivers/serial/serial_bcm283x_mu.c line 58

Quoted 8 lines
drivers/serial/serial_bcm283x_mu.c +6 -0
 
+static int bcm283x_mu_serial_setbrg(struct udevice *dev, int baudrate)
+{
+       struct bcm283x_mu_priv *priv = dev_get_priv(dev);
+       struct bcm283x_mu_regs *regs = priv->regs;
+       /* FIXME: Get this from plat data later */
+       u32 clock_rate = 250000000;

Or device tree?

Well even if DT were used on this platform, the code right here would get the clock rate from platform data. Now, whether the platform data came from a board file or was parsed from DT is another matter.

Quoted 16 lines

drivers/serial/serial_bcm283x_mu.c line 119

Quoted 11 lines
drivers/serial/serial_bcm283x_mu.c +11 -0
+static int bcm283x_mu_serial_pending(struct udevice *dev, bool input)
+{
+       struct bcm283x_mu_priv *priv = dev_get_priv(dev);
+       struct bcm283x_mu_regs *regs = priv->regs;
+       unsigned int lsr = readl(&regs->lsr);
+
+       if (input) {
+               WATCHDOG_RESET();
+               return lsr & BCM283X_MU_LSR_RX_READY;
+       } else {
+               return !(lsr & BCM283X_MU_LSR_TX_IDLE);

These look like flags - be care to return 1 if there is an unknown number of characters, rather than (e.g. 4). The latter might cause the uclass to expect 4 characters to be present.

Quoted 1 line

Suggest putting ? 1 : 0 or ? 0 : 1 on the end.

Luckily BCM283X_MU_LSR_RX_READY is BIT(0) so there's no practical issue. A !! would make this more obvious at this point in the code though. Do you think that warrants a patch? The ! on the second return also ensure the correct return value.

#3 lore raw

PATCH 1/1 serial: add BCM283x mini UART driver

Quoted 51 lines · Stephen Warren
Quoted 7 lines · Simon Glass

Hi Stephen,

Quoted 4 lines · Stephen Warren

The RPi3 typically uses the regular UART for high-speed communication with the Bluetooth device, leaving us the mini UART to use for the serial console. Add support for this UART so we can use it.

Quoted 8 lines
Quoted 4 lines

---
(This will be a dependency for the RPi 3 patches, so it'd be good if it could make it into mainline pretty quickly if acceptable.)

Not sure if this went in already. But see comment below.

It has, but I can send a fix.

Quoted 10 lines

drivers/serial/serial_bcm283x_mu.c line 58

Quoted 8 lines
drivers/serial/serial_bcm283x_mu.c +6 -0
 
+static int bcm283x_mu_serial_setbrg(struct udevice *dev, int baudrate)
+{
+       struct bcm283x_mu_priv *priv = dev_get_priv(dev);
+       struct bcm283x_mu_regs *regs = priv->regs;
+       /* FIXME: Get this from plat data later */
+       u32 clock_rate = 250000000;

Or device tree?

Well even if DT were used on this platform, the code right here would get the clock rate from platform data. Now, whether the platform data came from a board file or was parsed from DT is another matter.

Quoted 16 lines

drivers/serial/serial_bcm283x_mu.c line 119

Quoted 11 lines
drivers/serial/serial_bcm283x_mu.c +11 -0
+static int bcm283x_mu_serial_pending(struct udevice *dev, bool input)
+{
+       struct bcm283x_mu_priv *priv = dev_get_priv(dev);
+       struct bcm283x_mu_regs *regs = priv->regs;
+       unsigned int lsr = readl(&regs->lsr);
+
+       if (input) {
+               WATCHDOG_RESET();
+               return lsr & BCM283X_MU_LSR_RX_READY;
+       } else {
+               return !(lsr & BCM283X_MU_LSR_TX_IDLE);

These look like flags - be care to return 1 if there is an unknown number of characters, rather than (e.g. 4). The latter might cause the uclass to expect 4 characters to be present.

Quoted 6 lines
Quoted 1 line

Suggest putting ? 1 : 0 or ? 0 : 1 on the end.

Luckily BCM283X_MU_LSR_RX_READY is BIT(0) so there's no practical issue. A !! would make this more obvious at this point in the code though. Do you think that warrants a patch? The ! on the second return also ensure the correct return value.

I hate the '!!' operator, how bad would it be to spell things out in the code in a bit less compact way but such that the compiler will still optimize things well.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160410/14e68d74/attachment.sig>
#4 lore raw

PATCH 1/1 serial: add BCM283x mini UART driver

In reply to Stephen Warren

Hi Stephen,

Quoted 60 lines · Stephen Warren
Quoted 16 lines · Simon Glass

Hi Stephen,

Quoted 10 lines · Stephen Warren

The RPi3 typically uses the regular UART for high-speed communication with
the Bluetooth device, leaving us the mini UART to use for the serial console. Add support for this UART so we can use it.

---
(This will be a dependency for the RPi 3 patches, so it'd be good if it could make it into mainline pretty quickly if acceptable.)

Quoted 3 lines

Not sure if this went in already. But see comment below.

It has, but I can send a fix.

Quoted 4 lines
Quoted 2 lines
drivers/serial/serial_bcm283x_mu.c +0 -0
Quoted 9 lines

drivers/serial/serial_bcm283x_mu.c line 58

Quoted 6 lines
drivers/serial/serial_bcm283x_mu.c +6 -0
+static int bcm283x_mu_serial_setbrg(struct udevice *dev, int baudrate)
+{
+       struct bcm283x_mu_priv *priv = dev_get_priv(dev);
+       struct bcm283x_mu_regs *regs = priv->regs;
+       /* FIXME: Get this from plat data later */
+       u32 clock_rate = 250000000;

Or device tree?

Well even if DT were used on this platform, the code right here would get the clock rate from platform data. Now, whether the platform data came from a board file or was parsed from DT is another matter.

Quoted 17 lines

drivers/serial/serial_bcm283x_mu.c line 119

Quoted 11 lines
drivers/serial/serial_bcm283x_mu.c +11 -0
+static int bcm283x_mu_serial_pending(struct udevice *dev, bool input)
+{
+       struct bcm283x_mu_priv *priv = dev_get_priv(dev);
+       struct bcm283x_mu_regs *regs = priv->regs;
+       unsigned int lsr = readl(&regs->lsr);
+
+       if (input) {
+               WATCHDOG_RESET();
+               return lsr & BCM283X_MU_LSR_RX_READY;
+       } else {
+               return !(lsr & BCM283X_MU_LSR_TX_IDLE);

These look like flags - be care to return 1 if there is an unknown number of characters, rather than (e.g. 4). The latter might cause the uclass to expect 4 characters to be present.

Quoted 7 lines
Quoted 1 line

Suggest putting ? 1 : 0 or ? 0 : 1 on the end.

Luckily BCM283X_MU_LSR_RX_READY is BIT(0) so there's no practical issue. A !! would make this more obvious at this point in the code though. Do you think that warrants a patch? The ! on the second return also ensure the correct return value.

I think it is worth a patch, in case perhaps someone else copies that code later. Agreed not a high priority though.

Regards,
Simon