From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Sat, 9 Apr 2016 21:45:36 -0600 Subject: [U-Boot] [PATCH] serial: add BCM283x mini UART driver In-Reply-To: References: <1458186401-31287-1-git-send-email-swarren@wwwdotorg.org> Message-ID: <5709CC60.1010801@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 04/09/2016 12:34 PM, Simon Glass wrote: > Hi Stephen, > > On 16 March 2016 at 21:46, Stephen Warren 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 >> --- >> (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 > > 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.