[U-Boot] [PATCH v2] serial: bcm283x_mu: Detect disabled serial device

[Alexander Graf]

On 09.08.16 06:28, Stephen Warren wrote:
> On 08/04/2016 05:15 PM, Alexander Graf wrote:
>>
>>> On 04 Aug 2016, at 20:11, Stephen Warren <swarren at 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?
> 
> Yes, assuming that U-Boot runs just fine with HDMI console only, I think
> it's fine to unset CONFIG_REQUIRE_SERIAL_CONSOLE.
> 
>>> 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 :).

So I couldn't find a sane way to fail probing based on something defined
in the board file, reusing the existing gpio device.

However, there's an easy alternative. We can make the console code just
ignore our serial device if we set its pointer to NULL. That way we
still have the device, but can contain all logic to disable usage of the
mini uart to the board file.


Alex