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

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

[Michal Simek]

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@xilinx.com>
---

 drivers/serial/ns16550.c | 6 ++++--
 include/ns16550.h        | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index 93dad338b375..28da9ddfd859 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -105,7 +105,7 @@ static void ns16550_writeb(NS16550_t port, int offset, int value)
 	 * As far as we know it doesn't make sense to support selection of
 	 * these options at run-time, so use the existing CONFIG options.
 	 */
-	serial_out_shift(addr, plat->reg_shift, value);
+	serial_out_shift(addr + plat->reg_offset, plat->reg_shift, value);
 }
 
 static int ns16550_readb(NS16550_t port, int offset)
@@ -116,7 +116,7 @@ static int ns16550_readb(NS16550_t port, int offset)
 	offset *= 1 << plat->reg_shift;
 	addr = map_physmem(plat->base, 0, MAP_NOCACHE) + offset;
 
-	return serial_in_shift(addr, plat->reg_shift);
+	return serial_in_shift(addr + plat->reg_offset, plat->reg_shift);
 }
 
 /* We can clean these up once everything is moved to driver model */
@@ -401,6 +401,8 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 		return -EINVAL;
 
 	plat->base = addr;
+	plat->reg_offset = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
+				     "reg-offset", 0);
 	plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
 					 "reg-shift", 0);
 	plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
diff --git a/include/ns16550.h b/include/ns16550.h
index 4e620676c453..5eeacd6ff945 100644
--- a/include/ns16550.h
+++ b/include/ns16550.h
@@ -54,6 +54,7 @@
  */
 struct ns16550_platdata {
 	unsigned long base;
+	int reg_offset;
 	int reg_shift;
 	int clock;
 };
-- 
1.9.1

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

[Simon Glass]

Hi Michal,

On 16 February 2016 at 08:17, Michal Simek <michal.simek@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@xilinx.com>
> ---
>
>  drivers/serial/ns16550.c | 6 ++++--
>  include/ns16550.h        | 1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Do you support the debug UART feature on your boards?

Regards,
Simon

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

[Michal Simek]

On 19.2.2016 21:55, Simon Glass wrote:
> Hi Michal,
> 
> On 16 February 2016 at 08:17, Michal Simek <michal.simek@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@xilinx.com>
>> ---
>>
>>  drivers/serial/ns16550.c | 6 ++++--
>>  include/ns16550.h        | 1 +
>>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@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.

Thanks,
Michal

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

[Simon Glass]

Hi Michal,

On 22 February 2016 at 00:40, Michal Simek <michal.simek@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@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@xilinx.com>
>>> ---
>>>
>>>  drivers/serial/ns16550.c | 6 ++++--
>>>  include/ns16550.h        | 1 +
>>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> Reviewed-by: Simon Glass <sjg@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.

It looks right to me.

Regards,
Simon

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

[Adam Ford]

On Tue, Feb 23, 2016 at 12:38 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Michal,
>
> On 22 February 2016 at 00:40, Michal Simek <michal.simek@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@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@xilinx.com>
>>>> ---
>>>>
>>>>  drivers/serial/ns16550.c | 6 ++++--
>>>>  include/ns16550.h        | 1 +
>>>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> Reviewed-by: Simon Glass <sjg@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) ?

> It looks right to me.
>
> Regards,
> Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

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

[Michal Simek]

On 24.2.2016 11:56, Adam Ford wrote:
> On Tue, Feb 23, 2016 at 12:38 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Michal,
>>
>> On 22 February 2016 at 00:40, Michal Simek <michal.simek@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@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@xilinx.com>
>>>>> ---
>>>>>
>>>>>  drivers/serial/ns16550.c | 6 ++++--
>>>>>  include/ns16550.h        | 1 +
>>>>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> Reviewed-by: Simon Glass <sjg@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.

Thanks,
Michal

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

[Derald D. Woods]

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@chromium.org> wrote:
> >> Hi Michal,
> >>
> >> On 22 February 2016 at 00:40, Michal Simek <michal.simek@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@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@xilinx.com>
> >>>>> ---
> >>>>>
> >>>>>  drivers/serial/ns16550.c | 6 ++++--
> >>>>>  include/ns16550.h        | 1 +
> >>>>>  2 files changed, 5 insertions(+), 2 deletions(-)
> >>>>
> >>>> Reviewed-by: Simon Glass <sjg@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.
> 

Would it be appropriate to initialize 'plat->reg_offset' to zero for the
non-OF_CONTROL cases?

Derald D. Woods

> Thanks,
> Michal
> 
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

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

[Simon Glass]

Hi,

On 24 February 2016 at 15:47, Derald D. Woods <woods.technical@gmail.com> 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@chromium.org> wrote:
>> >> Hi Michal,
>> >>
>> >> On 22 February 2016 at 00:40, Michal Simek <michal.simek@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@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@xilinx.com>
>> >>>>> ---
>> >>>>>
>> >>>>>  drivers/serial/ns16550.c | 6 ++++--
>> >>>>>  include/ns16550.h        | 1 +
>> >>>>>  2 files changed, 5 insertions(+), 2 deletions(-)
>> >>>>
>> >>>> Reviewed-by: Simon Glass <sjg@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.
>>
>
> Would it be appropriate to initialize 'plat->reg_offset' to zero for the
> non-OF_CONTROL cases?

It should not be needed as driver model promises to zero auto-allocated data.

- Simon

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

[Derald D. Woods]

On Wed, Feb 24, 2016 at 05:33:08PM -0700, Simon Glass wrote:
> Hi,
> 
> On 24 February 2016 at 15:47, Derald D. Woods <woods.technical@gmail.com> 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@chromium.org> wrote:
> >> >> Hi Michal,
> >> >>
> >> >> On 22 February 2016 at 00:40, Michal Simek <michal.simek@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@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@xilinx.com>
> >> >>>>> ---
> >> >>>>>
> >> >>>>>  drivers/serial/ns16550.c | 6 ++++--
> >> >>>>>  include/ns16550.h        | 1 +
> >> >>>>>  2 files changed, 5 insertions(+), 2 deletions(-)
> >> >>>>
> >> >>>> Reviewed-by: Simon Glass <sjg@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.
> >>
> >
> > Would it be appropriate to initialize 'plat->reg_offset' to zero for the
> > non-OF_CONTROL cases?
> 
> It should not be needed as driver model promises to zero auto-allocated data.
> 
> - Simon

Thanks Simon. 

The omap3_logic board (LogicPD Torpedo DM3730 devkit) stops booting at
this Git patch/commit. Dropping it allows the board to boot again.
There is probably some serial config that is missing? The one thing
that is variant, from other OMAP3 boards, is the use of UART1 for the
main console. I will continue searching for a solution.

Derald

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

[Derald D. Woods]

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@chromium.org> wrote:
> >> Hi Michal,
> >>
> >> On 22 February 2016 at 00:40, Michal Simek <michal.simek@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@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@xilinx.com>
> >>>>> ---
> >>>>>
> >>>>>  drivers/serial/ns16550.c | 6 ++++--
> >>>>>  include/ns16550.h        | 1 +
> >>>>>  2 files changed, 5 insertions(+), 2 deletions(-)
> >>>>
> >>>> Reviewed-by: Simon Glass <sjg@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.

Derald

> Thanks,
> Michal

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

[Michal Simek]

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@chromium.org> wrote:
>>>> Hi Michal,
>>>>
>>>> On 22 February 2016 at 00:40, Michal Simek <michal.simek@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@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@xilinx.com>
>>>>>>> ---
>>>>>>>
>>>>>>>  drivers/serial/ns16550.c | 6 ++++--
>>>>>>>  include/ns16550.h        | 1 +
>>>>>>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> Reviewed-by: Simon Glass <sjg@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?

The patch is quite simple and if your DT has no this property there are
not so many options what can be problematic.
I expect when you enable debug console you will get more information.

Thanks,
Michal

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

[Derald D. Woods]

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@chromium.org> wrote:
> >>>> Hi Michal,
> >>>>
> >>>> On 22 February 2016 at 00:40, Michal Simek <michal.simek@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@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@xilinx.com>
> >>>>>>> ---
> >>>>>>>
> >>>>>>>  drivers/serial/ns16550.c | 6 ++++--
> >>>>>>>  include/ns16550.h        | 1 +
> >>>>>>>  2 files changed, 5 insertions(+), 2 deletions(-)
> >>>>>>
> >>>>>> Reviewed-by: Simon Glass <sjg@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.

Derald

> The patch is quite simple and if your DT has no this property there are
> not so many options what can be problematic.
> I expect when you enable debug console you will get more information.
> 
> Thanks,
> Michal
> 

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

[Alexander Graf]

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@chromium.org> wrote:
>>>>>> Hi Michal,
>>>>>>
>>>>>> On 22 February 2016 at 00:40, Michal Simek <michal.simek@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@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@xilinx.com>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>>   drivers/serial/ns16550.c | 6 ++++--
>>>>>>>>>   include/ns16550.h        | 1 +
>>>>>>>>>   2 files changed, 5 insertions(+), 2 deletions(-)
>>>>>>>> Reviewed-by: Simon Glass <sjg@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;

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

[Derald D. Woods]

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@chromium.org> 
>>>>>> wrote:
>>>>>>> Hi Michal,
>>>>>>>
>>>>>>> On 22 February 2016 at 00:40, Michal Simek 
>>>>>>> <michal.simek@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@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@xilinx.com>
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>>   drivers/serial/ns16550.c | 6 ++++--
>>>>>>>>>>   include/ns16550.h        | 1 +
>>>>>>>>>>   2 files changed, 5 insertions(+), 2 deletions(-)
>>>>>>>>> Reviewed-by: Simon Glass <sjg@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

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

[Derald D. Woods]

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@chromium.org> 
>>>>>>> wrote:
>>>>>>>> Hi Michal,
>>>>>>>>
>>>>>>>> On 22 February 2016 at 00:40, Michal Simek 
>>>>>>>> <michal.simek@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@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@xilinx.com>
>>>>>>>>>>> ---
>>>>>>>>>>>
>>>>>>>>>>>   drivers/serial/ns16550.c | 6 ++++--
>>>>>>>>>>>   include/ns16550.h        | 1 +
>>>>>>>>>>>   2 files changed, 5 insertions(+), 2 deletions(-)
>>>>>>>>>> Reviewed-by: Simon Glass <sjg@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] [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@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@chromium.org>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Hi Michal,
>>>>>>>>>
>>>>>>>>> On 22 February 2016 at 00:40, Michal Simek
>>>>>>>>> <michal.simek@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@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@xilinx.com>
>>>>>>>>>>>> ---
>>>>>>>>>>>>
>>>>>>>>>>>>   drivers/serial/ns16550.c | 6 ++++--
>>>>>>>>>>>>   include/ns16550.h        | 1 +
>>>>>>>>>>>>   2 files changed, 5 insertions(+), 2 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>> Reviewed-by: Simon Glass <sjg@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

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

[Derald D. Woods]

On Sun, Feb 28, 2016 at 5:51 PM, Derald D. Woods 
<woods.technical@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@chromium.org>
>>>>>>>>> wrote:
>>>>>>>>>> Hi Michal,
>>>>>>>>>>
>>>>>>>>>> On 22 February 2016 at 00:40, Michal Simek
>>>>>>>>>> <michal.simek@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@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@xilinx.com>
>>>>>>>>>>>>> ---
>>>>>>>>>>>>>
>>>>>>>>>>>>>    drivers/serial/ns16550.c | 6 ++++--
>>>>>>>>>>>>>    include/ns16550.h        | 1 +
>>>>>>>>>>>>>    2 files changed, 5 insertions(+), 2 deletions(-)
>>>>>>>>>>>> Reviewed-by: Simon Glass <sjg@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
On 02/28/2016 09:56 PM, Adam Ford wrote:
> 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
>
>

According to "doc/driver-model/README.txt" the use of 'U_BOOT_DEVICE' 
will the problematic going forward.

For now, I would expect something like the following would work, without 
modifying the structure again:

[board/logicpd/omap3som/omap3logic.c]
...
static struct ns16550_platdata omap3logic_serial = {
     OMAP34XX_UART1,
     0,
     2,
     V_NS16550_CLK
};
...

[board/ti/beagle/beagle.c]
...
static const struct ns16550_platdata beagle_serial = {
     OMAP34XX_UART3,
     0,
     2,
     V_NS16550_CLK
};
...

All static instances of the structure initialization would be incorrect 
without adding the new 'reg_offset' member after 'base'.

If 'U_BOOT_DEVICE' is not expected to work anymore, then efforts may 
need to be directed towards the new DM/FDT way.


- Derald

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

[Simon Glass]

Hi,

On 28 February 2016 at 21:58, Derald D. Woods <woods.technical@gmail.com> wrote:
> On Sun, Feb 28, 2016 at 5:51 PM, Derald D. Woods <woods.technical@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@chromium.org>
>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Michal,
>>>>>>>>>>>
>>>>>>>>>>> On 22 February 2016 at 00:40, Michal Simek
>>>>>>>>>>> <michal.simek@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@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@xilinx.com>
>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>    drivers/serial/ns16550.c | 6 ++++--
>>>>>>>>>>>>>>    include/ns16550.h        | 1 +
>>>>>>>>>>>>>>    2 files changed, 5 insertions(+), 2 deletions(-)
>>>>>>>>>>>>>
>>>>>>>>>>>>> Reviewed-by: Simon Glass <sjg@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
>
> On 02/28/2016 09:56 PM, Adam Ford wrote:
>>
>> 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
>>
>>
>
> According to "doc/driver-model/README.txt" the use of 'U_BOOT_DEVICE' will
> the problematic going forward.
>
> For now, I would expect something like the following would work, without
> modifying the structure again:
>
> [board/logicpd/omap3som/omap3logic.c]
> ...
> static struct ns16550_platdata omap3logic_serial = {
>     OMAP34XX_UART1,
>     0,
>     2,
>     V_NS16550_CLK
> };
> ...
>
> [board/ti/beagle/beagle.c]
> ...
> static const struct ns16550_platdata beagle_serial = {
>     OMAP34XX_UART3,
>     0,
>     2,
>     V_NS16550_CLK
> };
> ...
>
> All static instances of the structure initialization would be incorrect
> without adding the new 'reg_offset' member after 'base'.
>
> If 'U_BOOT_DEVICE' is not expected to work anymore, then efforts may need to
> be directed towards the new DM/FDT way.

Yes indeed.

For platform data I'd recommend adding member names.

Regards,
Simon

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

[Michal Simek]

On 29.2.2016 06:15, Simon Glass wrote:
> Hi,
> 
> On 28 February 2016 at 21:58, Derald D. Woods <woods.technical@gmail.com> wrote:
>> On Sun, Feb 28, 2016 at 5:51 PM, Derald D. Woods <woods.technical@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@chromium.org>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hi Michal,
>>>>>>>>>>>>
>>>>>>>>>>>> On 22 February 2016 at 00:40, Michal Simek
>>>>>>>>>>>> <michal.simek@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@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@xilinx.com>
>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>    drivers/serial/ns16550.c | 6 ++++--
>>>>>>>>>>>>>>>    include/ns16550.h        | 1 +
>>>>>>>>>>>>>>>    2 files changed, 5 insertions(+), 2 deletions(-)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Reviewed-by: Simon Glass <sjg@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
>>
>> On 02/28/2016 09:56 PM, Adam Ford wrote:
>>>
>>> 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
>>>
>>>
>>
>> According to "doc/driver-model/README.txt" the use of 'U_BOOT_DEVICE' will
>> the problematic going forward.
>>
>> For now, I would expect something like the following would work, without
>> modifying the structure again:
>>
>> [board/logicpd/omap3som/omap3logic.c]
>> ...
>> static struct ns16550_platdata omap3logic_serial = {
>>     OMAP34XX_UART1,
>>     0,
>>     2,
>>     V_NS16550_CLK
>> };
>> ...
>>
>> [board/ti/beagle/beagle.c]
>> ...
>> static const struct ns16550_platdata beagle_serial = {
>>     OMAP34XX_UART3,
>>     0,
>>     2,
>>     V_NS16550_CLK
>> };
>> ...
>>
>> All static instances of the structure initialization would be incorrect
>> without adding the new 'reg_offset' member after 'base'.
>>
>> If 'U_BOOT_DEVICE' is not expected to work anymore, then efforts may need to
>> be directed towards the new DM/FDT way.
> 
> Yes indeed.
> 
> For platform data I'd recommend adding member names.

yes please.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160229/bd7b4292/attachment.sig>

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

[Simon Glass]

Hi,

On 28 February 2016 at 15:39, Alexander Graf <agraf@suse.de> 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@chromium.org>
>>>>>> wrote:
>>>>>>>
>>>>>>> Hi Michal,
>>>>>>>
>>>>>>> On 22 February 2016 at 00:40, Michal Simek <michal.simek@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@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@xilinx.com>
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>>   drivers/serial/ns16550.c | 6 ++++--
>>>>>>>>>>   include/ns16550.h        | 1 +
>>>>>>>>>>   2 files changed, 5 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> Reviewed-by: Simon Glass <sjg@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 picked up Alexander's suggestion: Still, we should fix everything up
to use member names.

Applied to u-boot-dm/next, thanks!

- Simon

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

[Adam Ford]

On Sat, Mar 12, 2016 at 7:54 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> On 28 February 2016 at 15:39, Alexander Graf <agraf@suse.de> 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@chromium.org>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Hi Michal,
>>>>>>>>
>>>>>>>> On 22 February 2016 at 00:40, Michal Simek <michal.simek@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@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@xilinx.com>
>>>>>>>>>>> ---
>>>>>>>>>>>
>>>>>>>>>>>   drivers/serial/ns16550.c | 6 ++++--
>>>>>>>>>>>   include/ns16550.h        | 1 +
>>>>>>>>>>>   2 files changed, 5 insertions(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>> Reviewed-by: Simon Glass <sjg@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 picked up Alexander's suggestion: Still, we should fix everything up
> to use member names.
>

As far as I know a bunch of impacted boards should be fixed now.
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=2f6ed3b89a1760ffa73123fb266de3d56eb5d88e

I think Alexander's patch shouldn't impact anything else.

adam

> Applied to u-boot-dm/next, thanks!
>
> - Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

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

[Michal Simek]

Hi Simon,


>> 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 picked up Alexander's suggestion: Still, we should fix everything up
> to use member names.
> 
> Applied to u-boot-dm/next, thanks!

I can't see this patch applied to your next branch.
Can you please check it?

Thanks,
Michal



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160331/e166401d/attachment.sig>

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

[Simon Glass]

Hi Michal,

On 31 March 2016 at 03:16, Michal Simek <monstr@monstr.eu> wrote:
> Hi Simon,
>
>
>>> 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 picked up Alexander's suggestion: Still, we should fix everything up
>> to use member names.
>>
>> Applied to u-boot-dm/next, thanks!
>
> I can't see this patch applied to your next branch.
> Can you please check it?

It's in mainline now - I have have updated the u-boot-dm/next branch already.

Regards,
Simon

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

[Michal Simek]

Hi Simon,

On 5.4.2016 02:03, Simon Glass wrote:
> Hi Michal,
> 
> On 31 March 2016 at 03:16, Michal Simek <monstr@monstr.eu> wrote:
>> Hi Simon,
>>
>>
>>>> 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 picked up Alexander's suggestion: Still, we should fix everything up
>>> to use member names.
>>>
>>> Applied to u-boot-dm/next, thanks!
>>
>> I can't see this patch applied to your next branch.
>> Can you please check it?
> 
> It's in mainline now - I have have updated the u-boot-dm/next branch already.

ok.

Thanks,
Michal

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

[Tom Rini]

On Tue, Feb 16, 2016 at 04:17:49PM +0100, Michal Simek 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@xilinx.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160219/7764f499/attachment.sig>