Re: [PATCH v3 08/10] tegra: add generic debug UART support

["antonynpavlov@gmail.com" <antonynpavlov@gmail.com>]

On Wed, 10 Apr 2013 11:18:01 +0200
Lucas Stach <dev@lynxeye.de> wrote:

> Am Montag, den 08.04.2013, 17:03 +0400 schrieb antonynpavlov@gmail.com:
> > On Mon, 08 Apr 2013 14:24:06 +0200
> > Lucas Stach <dev@lynxeye.de> wrote:
> > 
> > > Am Montag, den 08.04.2013, 15:46 +0400 schrieb antonynpavlov@gmail.com:
> > > > On Mon, 08 Apr 2013 12:57:26 +0200
> > > > Lucas Stach <dev@lynxeye.de> wrote:
> > > > 
> > > > > Hi Antony,
> > > > > 
> > > > > Am Freitag, den 05.04.2013, 00:32 +0400 schrieb antonynpavlov@gmail.com:
> > > > > > On Tue,  2 Apr 2013 08:19:10 +0200
> > > > > > Lucas Stach <dev@lynxeye.de> wrote:
> > > > > > 
> > > > > > > ODMdata tells us which UART to use for debugging purposes. This is
> > > > > > > agreed upon in both the upstream Linux kernel and U-Boot, so do it the
> > > > > > > same way in barebox.
> > > > > > 
> > > > > > This patch disables serial console on my Toshiba AC100.
> > > > > > 
> > > > > Hm that's unfortunate, but I already suspected that your ODMdata might
> > > > > be wrong.
> > > > > What happens if you don't remove the serial init in your board? Do you
> > > > > get a duplicate serial, or just your board defined one? If it still
> > > > > works for you this way, we might take this as a compromise for now.
> > > > 
> > > > IMHO, the right way is to get of_serial.c from linux and put some records 
> > > > like this 
> > > >  uarta: serial@70006000 {
> > > >                 compatible = "nvidia,tegra20-uart";
> > > >                 reg = <0x70006000 0x40>;
> > > >                 reg-shift = <2>;
> > > >                 interrupts = <0 36 0x04>;
> > > >                 nvidia,dma-request-selector = <&apbdma 8>;
> > > >                 clocks = <&tegra_car 6>;
> > > >                 status = "disabled";
> > > >   };
> > > > 
> > > > to arch/arm/dts/tegra20.dtsi.
> > > > 
> > > 
> > > No, this doesn't help with the decision which UART barebox should use
> > > for it's output. This is a configuration option and therefore not
> > > allowed to reside in the DT, so NVIDIA decided to put this information
> > > into the ODMdata. Both U-Boot and the Linux kernel(for earlyconsole) use
> > > the information stored in this location.
> > > 
> > > You can always activate more consoles from board code or DT, but the
> > > first/debug console should IMHO be handled the same way it's done in
> > > Linux.
> > 
> > Yes, this doesn't help to select the console UART, but it provide  the
> > information about platform available UARTs.
> > The selection of console port is board-dependent (e.g. the chip can have
> > many UARTs, but the board can use only one of them), so let's move this
> > selection to board configuration section.
> > 
> > Moreother, Toshiba AC100 is a netbook (or smartbook :), in normal mode it
> > use keyboard and LCD screen for user interaction, not UART.
> > 
> The generic serial patch in the series v4 now introduces a Kconfig to
> override the ODMdata value. I folded in the right config into the ac100
> defconfig, so this patch should no longer break your console.
> 
> Also I fixed a bug that may lead to wrong ODMdata console detection, so
> could you please test if it maybe also works with the automatic
> detection for you now?

I have just checked your patches with
 CONFIG_TEGRA_UART_ODMDATA=y
and with
 CONFIG_TEGRA_UART_A=y

Both configurations work fine!

There is a very small issues (see my next messages), but IMHO, the patches are ready for commiting.

-- 
Best regards,
  Antony Pavlov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox