> On 04 Aug 2016, at 20:11, Stephen Warren <swar...@wwwdotorg.org> wrote:
> 
> 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.

I guess I should’ve put that in a comment somewhere. Unfortunately we can’t. If 
I just return an error on probe, U-Boot will panic because we tell it in a 
CONFIG define that we require a serial port (grep for 
CONFIG_REQUIRE_SERIAL_CONSOLE).

We could maybe try to unset that define instead?

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

We can do that if we can fail at probe time. If we absolutely must have a 
serial driver to work in the first place, that doesn’t work. I can try to poke 
at it, but it’ll be a few days I think :).


Alex

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to