earlycon issues in -next with amba-pl011 updates

[Peter Hurley]

On 09/03/2015 02:16 PM, Marc Zyngier wrote:
> On 03/09/15 19:03, Peter Hurley wrote:
>> On 09/03/2015 01:53 PM, Greg Kroah-Hartman wrote:
>>> On Thu, Sep 03, 2015 at 01:49:02PM -0400, Peter Hurley wrote:
>>>> On 09/03/2015 12:08 PM, Marc Zyngier wrote:
>>>>> On 03/09/15 16:52, Greg Kroah-Hartman wrote:
>>>>>> On Thu, Sep 03, 2015 at 11:23:15AM +0100, Marc Zyngier wrote:
>>>>>>> On 11/08/15 02:48, Jun Nie wrote:
>>>>>>>> 2015-08-11 7:23 GMT+08:00 Leif Lindholm <leif.lindholm at linaro.org>:
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> The kernelci.org bot picked up a complete boot failure (no output past
>>>>>>>>> UEFI stub) with next-20150806 and Tyler bisected it down to somewhere
>>>>>>>>> in
>>>>>>>>> 8cd90e5 uart: pl011: Add support to ZTE ZX296702 uart
>>>>>>>>> 09dcc7d uart: pl011: Improve LCRH register access decision
>>>>>>>>> 2c096a9 uart: pl011: Introduce register look up table
>>>>>>>>> 7b753f3 uart: pl011: Introduce register accessor
>>>>>>>>>
>>>>>>>>> The issue only appears with earlycon on command line, for pl011
>>>>>>>>> consoles.
>>>>>>>>>
>>>>>>>>> Some investigation shows that the cause lies with
>>>>>>>>> commit 7b753f318d14 ("uart: pl011: Introduce register accessor")
>>>>>>>>> and
>>>>>>>>> commit 2c096a9eedc6 ("uart: pl011: Introduce register look up table")
>>>>>>>>>
>>>>>>>>> Specifically, the changes to pl011_putc() are incorrect:
>>>>>>>>> The new pl011_ accessors take a (struct uart_amba_port *) input, but
>>>>>>>>> pl011_putc() directly uses the incoming (struct uart_port *) for this.
>>>>>>>>>
>>>>>>>>> Apart from ending up with an unintended/incorrect UART base address,
>>>>>>>>> the introduction of the lookup table for register offsets also means
>>>>>>>>> the accessors try to dereference (struct uart_amba_port *)->reg_lut.
>>>>>>>>>
>>>>>>>>> The below is a hack that shows/resolves the issue, but some
>>>>>>>>> refactoring of the original patches might be in order.
>>>>>>>>>
>>>>>>>>> /
>>>>>>>>>     Leif
>>>>>>>>
>>>>>>>> Leif,
>>>>>>>>
>>>>>>>> Sorry for the inconvenience. I do not have idea of early console till
>>>>>>>> now and I always have debug console for early panic debug. Learned
>>>>>>>> more from this issue.
>>>>>>>>
>>>>>>>> Suppose Peter's patch will resolve your issue.
>>>>>>>
>>>>>>> [+ Greg KH]
>>>>>>>
>>>>>>> So -next has now been broken for a while on a number of ARM platforms
>>>>>>> because of this (they simply cannot boot), and no progress has been made
>>>>>>> towards resolving this problem.
>>>>>>>
>>>>>>> Can we please drop this series (at least commits 7b753f3 and following)
>>>>>>> from -next until is has been reworked and reviewed?
>>>>>>
>>>>>> I don't have any patches in my -next tree, everything is in Linus's tree
>>>>>> now.  So if I've missed something, or need to revert something, please
>>>>>> let me know specifcally what to do.
>>>>>
>>>>> Gahhh... Given that there is no obvious fix
>>>>
>>>> Fix has been on list since Aug 10.
>>>>
>>>> http://www.spinics.net/lists/linux-serial/msg18576.html
>>>
>>> Now if only someone would have at least compile tested it :)
>>
>> On 08/11/2015 06:07 AM, Leif Lindholm wrote:
>>> It works, but builds with:
>>> drivers/tty/serial/amba-pl011.c:329:13: warning: ‘pl011_writeb’
>>> defined but not used [-Wunused-function]
>>>  static void pl011_writeb(struct uart_amba_port *uap, u8 val, int
>>>  index)
>>>              ^
>>> I was dithering about whether having _relaxed accessors in post boot
>>> code and plain ones in earlycon was an issue, but I guess it doesn't
>>> matter much.
>>
>> Russell wanted me to do a bunch of improvements, but I wrote that
>> should wait until 4.3-rc cycle.
> 
> It is well known that we merge "improvements" outside of the merge
> window. And what about starting with a series that is actually bisectable?
> 
> It is quite apparent that this code hasn't been tested enough, hasn't
> been reviewed enough, and breaks a wide range of platforms (almost
> anything that sits on and under my desk, TBH).
> 
> That's really for Greg to decide, but I'm not overly confident with the
> state of this series as it stands.

If you feel the code is not reliable enough/tested/not bisectable/whatever,
I have no objection to reverting and starting again.

But if the issue is only wrt earlycon, then my patch ought to address that.

Regards,
Peter Hurley