From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Thu, 4 Aug 2016 13:11:13 -0600 Subject: [U-Boot] [PATCH v2] serial: bcm283x_mu: Detect disabled serial device In-Reply-To: <1470294682-159882-1-git-send-email-agraf@suse.de> References: <1470294682-159882-1-git-send-email-agraf@suse.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 08/04/2016 01:11 AM, Alexander Graf wrote: > On the raspberry pi, you can disable the serial port to gain dynamic frequency > scaling which can get handy at times. > > However, in such a configuration the serial controller gets its rx queue filled > up with zero bytes which then happily get transmitted on to whoever calls > getc() today. > > This patch adds detection logic for that case by checking whether the RX pin is > mapped to GPIO15 and disables the mini uart if it is not mapped properly. > > That way we can leave the driver enabled in the tree and can determine during > runtime whether serial is usable or not, having a single binary that allows for > uart and non-uart operation. > diff --git a/drivers/serial/serial_bcm283x_mu.c b/drivers/serial/serial_bcm283x_mu.c > @@ -72,9 +87,18 @@ 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); > + struct bcm283x_gpio_regs *gpio = (struct bcm283x_gpio_regs *)plat->gpio; > > priv->regs = (struct bcm283x_mu_regs *)plat->base; > > + /* > + * The RPi3 disables the mini uart by default. The easiest way to find > + * out whether it is available is to check if the pin is muxed. > + */ > + if (((readl(&gpio->gpfsel1) >> BCM283X_GPIO_GPFSEL1_F15_SHIFT) & > + BCM283X_GPIO_ALTFUNC_MASK) != BCM283X_GPIO_ALTFUNC_5) > + priv->disabled = true; > + > return 0; Comment on the current implementation: Can't probe() return an error if the device should be disabled? That would avoid the need to check priv->disabled in all the other functions. Overall comment: I'd rather not put this logic into the UART driver itself; it is system-specific rather than device-specific. I'd also rather not have the UART driver touching GPIO registers; that's not very modular, and could cause problems if the Pi is converted to use DT to instantiate devices. Instead, can we put the logic into board/raspberrypi/rpi/rpi.c? I.e. have some early function come along and enable/disable the bcm2837_serials device object as appropriate? That way it isolates the code to the Pi specifically, and not any other bcm283x board. We'd want to wrap that code in #ifdef CONFIG_PL01X_SERIAL.