[U-Boot] [PATCH] dm: ns16550: Add support for reg-offset property

[Adam Ford]

I first tried removing the 'const' in the board file as suggested by
Derald, but that wasn't successful.  I can boot with Alexander's
patch, but modifying the order inside the header seems weird to me.  I
haven't had any time to  look this weekend, but I wonder if something
in one of the files is modifying the structure and expects the order
of the structure to always be a certain order.

adam



On Sun, Feb 28, 2016 at 5:51 PM, Derald D. Woods
<woods.technical at gmail.com> wrote:
> On 02/28/2016 05:45 PM, Derald D. Woods wrote:
>>
>> On 02/28/2016 04:39 PM, Alexander Graf wrote:
>>>
>>>
>>>
>>> On 02/25/2016 02:38 PM, Derald D. Woods wrote:
>>>>
>>>> On Thu, Feb 25, 2016 at 09:11:24AM +0100, Michal Simek wrote:
>>>>>
>>>>> On 25.2.2016 05:47, Derald D. Woods wrote:
>>>>>>
>>>>>> On Wed, Feb 24, 2016 at 12:26:09PM +0100, Michal Simek wrote:
>>>>>>>
>>>>>>> On 24.2.2016 11:56, Adam Ford wrote:
>>>>>>>>
>>>>>>>> On Tue, Feb 23, 2016 at 12:38 AM, Simon Glass <sjg at chromium.org>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Hi Michal,
>>>>>>>>>
>>>>>>>>> On 22 February 2016 at 00:40, Michal Simek
>>>>>>>>> <michal.simek at xilinx.com> wrote:
>>>>>>>>>>
>>>>>>>>>> On 19.2.2016 21:55, Simon Glass wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Michal,
>>>>>>>>>>>
>>>>>>>>>>> On 16 February 2016 at 08:17, Michal Simek
>>>>>>>>>>> <michal.simek at xilinx.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> reg-offset is the part of standard 8250 binding in the kernel.
>>>>>>>>>>>> It is shifting start of address space by reg-offset.
>>>>>>>>>>>> On Xilinx platform this offset is typically 0x1000.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>>>>>>>>>>>> ---
>>>>>>>>>>>>
>>>>>>>>>>>>   drivers/serial/ns16550.c | 6 ++++--
>>>>>>>>>>>>   include/ns16550.h        | 1 +
>>>>>>>>>>>>   2 files changed, 5 insertions(+), 2 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>>>>>>>>>>
>>>>>>>>>>> Do you support the debug UART feature on your boards?
>>>>>>>>>>
>>>>>>>>>> yes. I do support it but there you can put just address plus
>>>>>>>>>> offset and
>>>>>>>>>> there is no reason to add one more option to Kconfig.
>>>>>>>>>> But let me know if you think that this is incorrect flow.
>>>>>>>>
>>>>>>>> This patch seems to break my OMAP3 board.  Does anyone know if I
>>>>>>>> need
>>>>>>>> to set a certain offset for OMAP3 to make this work (and where is
>>>>>>>> the
>>>>>>>> right place for it) ?
>>>>>>>
>>>>>>> Are you using DT init? Check your DT description if there is
>>>>>>> reg-offset
>>>>>>> property. I expect if your board worked before and you remove this
>>>>>>> property it will start to work again.
>>>>>>>
>>>>>> I am seeing the same problem with my BeagleBoard Rev. C4. There is
>>>>>> something common, to more than one board, happening with this commit.
>>>>>
>>>>> You should enable debug console and send the log.
>>>>> Do you have enough space for malloc?
>>>>>
>>>> I will have little time this weekend to go further. Some things will
>>>> need to be un-configured to have enough space. I am around 7 KiB over
>>>> with DEBUG enabled.
>>>
>>>
>>> I'm not quite sure what exactly is going wrong here - maybe some asm code
>>> is accessing the fields without proper offset generation?
>>>
>>> Either way, the patch below seems to fix the issue for me (on
>>> beaglebone):
>>>
>>> diff --git a/include/ns16550.h b/include/ns16550.h
>>> index 5eeacd6..1311f4c 100644
>>> --- a/include/ns16550.h
>>> +++ b/include/ns16550.h
>>> @@ -54,9 +54,9 @@
>>>   */
>>>  struct ns16550_platdata {
>>>         unsigned long base;
>>> -       int reg_offset;
>>>         int reg_shift;
>>>         int clock;
>>> +       int reg_offset;
>>>  };
>>>
>>>  struct udevice;
>>>
>>
>> I see the following grep results:
>>
>> $ grep -RI -e "const struct ns16550_platdata" .
>> ./arch/arm/cpu/armv7/am33xx/board.c:static const struct ns16550_platdata
>> am33xx_serial[] = {
>> ./arch/arm/cpu/arm926ejs/lpc32xx/devices.c:static const struct
>> ns16550_platdata lpc32xx_uart[] = {
>> ./board/timll/devkit8000/devkit8000.c:static const struct ns16550_platdata
>> devkit8000_serial = {
>> ./board/ti/beagle/beagle.c:static const struct ns16550_platdata
>> beagle_serial = {
>> ./board/logicpd/zoom1/zoom1.c:static const struct ns16550_platdata
>> zoom1_serial = {
>> ./board/logicpd/omap3som/omap3logic.c:static const struct ns16550_platdata
>> omap3logic_serial = {
>> ./board/quipos/cairo/cairo.c:static const struct ns16550_platdata
>> cairo_serial = {
>> ./board/lge/sniper/sniper.c:static const struct ns16550_platdata
>> serial_omap_platdata = {
>> ./board/isee/igep00x0/igep00x0.c:static const struct ns16550_platdata
>> igep_serial = {
>> ./board/overo/overo.c:static const struct ns16550_platdata overo_serial =
>> {
>>
>> Could the use of 'const' be a part of the problem?
>>
>> - Derald
>>
>>
>
> The structure initializers need rework for the additional member.
>
>
> - Derald
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot