[U-Boot] [PATCH] serial: add BCM283x mini UART driver

[Tom Rini]

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 at 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 at 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 at 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(&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.
> 
> 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.

-- 
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>