[U-Boot] [PATCH] dm: ns16550: Add support for reg-offset property
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; };
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
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
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
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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
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
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
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
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
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;
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
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
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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@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
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@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
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@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
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
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=2f6ed3b89a1760ffa73123fb266d...
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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
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
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
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
participants (7)
-
Adam Ford -
Alexander Graf -
Derald D. Woods -
Michal Simek -
Michal Simek -
Simon Glass -
Tom Rini