Message ID | 1458186401-31287-1-git-send-email-swarren@wwwdotorg.org |
---|---|
State | Superseded |
Headers | show |
Hi Stephen, On 16 March 2016 at 21:46, Stephen Warren <swarren@wwwdotorg.org> wrote: > 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. > > Signed-off-by: Stephen Warren <swarren@wwwdotorg.org> > --- > (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.) Reviewed-by: Simon Glass <sjg@chromium.org> Not sure if this went in already. But see comment below. > > 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 > > diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile > index 05bdf56c6fe9..ee7147a77abc 100644 > --- a/drivers/serial/Makefile > +++ b/drivers/serial/Makefile > @@ -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 > diff --git a/drivers/serial/serial_bcm283x_mu.c b/drivers/serial/serial_bcm283x_mu.c > new file mode 100644 > index 000000000000..97b57cc52163 > --- /dev/null > +++ b/drivers/serial/serial_bcm283x_mu.c > @@ -0,0 +1,138 @@ > +/* > + * (C) Copyright 2016 Stephen Warren <swarren@wwwdotorg.org> > + * > + * Derived from pl01x code: > + * > + * (C) Copyright 2000 > + * Rob Taylor, Flying Pig Systems. robt@flyingpig.com. > + * > + * (C) Copyright 2004 > + * ARM Ltd. > + * Philippe Robin, <philippe.robin@arm.com> > + * > + * 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) > + > +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? > + u32 clock_rate = 250000000; > + u32 divider; > + > + divider = clock_rate / (baudrate * 8); > + > + writel(BCM283X_MU_LCR_DATA_SIZE_8, ®s->lcr); > + writel(divider - 1, ®s->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(®s->lsr) & BCM283X_MU_LSR_RX_READY)) > + return -EAGAIN; > + > + data = readl(®s->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(®s->lsr) & BCM283X_MU_LSR_TX_EMPTY)) > + return -EAGAIN; > + > + /* Send the character */ > + writel(data, ®s->io); > + > + return 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(®s->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. > + } > +} > + > +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), > +}; > diff --git a/include/dm/platform_data/serial_bcm283x_mu.h b/include/dm/platform_data/serial_bcm283x_mu.h > new file mode 100644 > index 000000000000..441594579549 > --- /dev/null > +++ b/include/dm/platform_data/serial_bcm283x_mu.h > @@ -0,0 +1,22 @@ > +/* > + * (C) Copyright 2016 Stephen Warren <swarren@wwwdotorg.org> > + * > + * 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 > -- > 2.7.3 > Regards, Simon
On 04/09/2016 12:34 PM, Simon Glass wrote: > Hi Stephen, > > On 16 March 2016 at 21:46, Stephen Warren <swarren@wwwdotorg.org> wrote: >> 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. >> >> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org> >> --- >> (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.) > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Not sure if this went in already. But see comment below. It has, but I can send a fix. >> diff --git a/drivers/serial/serial_bcm283x_mu.c b/drivers/serial/serial_bcm283x_mu.c >> +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. >> +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(®s->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. 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.
On Sat, Apr 09, 2016 at 09:45:36PM -0600, Stephen Warren wrote: > On 04/09/2016 12:34 PM, Simon Glass wrote: > >Hi Stephen, > > > >On 16 March 2016 at 21:46, Stephen Warren <swarren@wwwdotorg.org> wrote: > >>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. > >> > >>Signed-off-by: Stephen Warren <swarren@wwwdotorg.org> > >>--- > >>(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.) > > > >Reviewed-by: Simon Glass <sjg@chromium.org> > > > >Not sure if this went in already. But see comment below. > > It has, but I can send a fix. > > >>diff --git a/drivers/serial/serial_bcm283x_mu.c b/drivers/serial/serial_bcm283x_mu.c > > >>+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. > > >>+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(®s->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. > > 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.
Hi Stephen, On 9 April 2016 at 21:45, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 04/09/2016 12:34 PM, Simon Glass wrote: >> >> Hi Stephen, >> >> On 16 March 2016 at 21:46, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> >>> 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. >>> >>> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org> >>> --- >>> (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.) >> >> >> Reviewed-by: Simon Glass <sjg@chromium.org> >> >> Not sure if this went in already. But see comment below. > > > It has, but I can send a fix. > >>> diff --git a/drivers/serial/serial_bcm283x_mu.c >>> b/drivers/serial/serial_bcm283x_mu.c > > >>> +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. > >>> +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(®s->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. > > > 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
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile index 05bdf56c6fe9..ee7147a77abc 100644 --- a/drivers/serial/Makefile +++ b/drivers/serial/Makefile @@ -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 diff --git a/drivers/serial/serial_bcm283x_mu.c b/drivers/serial/serial_bcm283x_mu.c new file mode 100644 index 000000000000..97b57cc52163 --- /dev/null +++ b/drivers/serial/serial_bcm283x_mu.c @@ -0,0 +1,138 @@ +/* + * (C) Copyright 2016 Stephen Warren <swarren@wwwdotorg.org> + * + * Derived from pl01x code: + * + * (C) Copyright 2000 + * Rob Taylor, Flying Pig Systems. robt@flyingpig.com. + * + * (C) Copyright 2004 + * ARM Ltd. + * Philippe Robin, <philippe.robin@arm.com> + * + * 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) + +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 */ + u32 clock_rate = 250000000; + u32 divider; + + divider = clock_rate / (baudrate * 8); + + writel(BCM283X_MU_LCR_DATA_SIZE_8, ®s->lcr); + writel(divider - 1, ®s->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(®s->lsr) & BCM283X_MU_LSR_RX_READY)) + return -EAGAIN; + + data = readl(®s->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(®s->lsr) & BCM283X_MU_LSR_TX_EMPTY)) + return -EAGAIN; + + /* Send the character */ + writel(data, ®s->io); + + return 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(®s->lsr); + + if (input) { + WATCHDOG_RESET(); + return lsr & BCM283X_MU_LSR_RX_READY; + } else { + return !(lsr & BCM283X_MU_LSR_TX_IDLE); + } +} + +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), +}; diff --git a/include/dm/platform_data/serial_bcm283x_mu.h b/include/dm/platform_data/serial_bcm283x_mu.h new file mode 100644 index 000000000000..441594579549 --- /dev/null +++ b/include/dm/platform_data/serial_bcm283x_mu.h @@ -0,0 +1,22 @@ +/* + * (C) Copyright 2016 Stephen Warren <swarren@wwwdotorg.org> + * + * 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
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. Signed-off-by: Stephen Warren <swarren@wwwdotorg.org> --- (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