Re: [Xen-devel] [RFC 26/29] xen/arm: Add Exynos 4210 UART support for early printk

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-05-01 at 18:17 +0100, Anthony PERARD wrote:
> On 30/04/13 10:53, Ian Campbell wrote:
> > On Mon, 2013-04-29 at 00:02 +0100, Julien Grall wrote:
> >> ---
> >>  config/arm32.mk                    |    1 +
> >>  xen/arch/arm/Rules.mk              |    4 ++
> >>  xen/arch/arm/arm32/Makefile        |    1 +
> >>  xen/arch/arm/arm32/debug-exynos5.S |   81 
> >> ++++++++++++++++++++++++++++++++++++
> >>  4 files changed, 87 insertions(+)
> >>  create mode 100644 xen/arch/arm/arm32/debug-exynos5.S
> >>
> >> diff --git a/config/arm32.mk b/config/arm32.mk
> >> index 593a1d1..01c1490 100644
> >> --- a/config/arm32.mk
> >> +++ b/config/arm32.mk
> >> @@ -17,6 +17,7 @@ CFLAGS += -marm
> >>  # Possible value:
> >>  #   - none: no early printk
> >>  #   - pl011: printk with PL011 UART
> >> +#   - exynos5: printk with the second exynos 5 UART
> > 
> > From the subject I infer that this UART is an Exynos 4210? If so I think
> > that is the correct name to use here, consistent with the pl011 name
> > above. (the alternative is that the pl011 is wrong...).
> > 
> > I suppose in principal a single driver might apply to multiple platforms
> > but with different base addresses, but that at build time you need to
> > choose a specific platform, which implies a particular driver. The
> > current infrastructure doesn't really express that notion.
> > 
> > What about if we support a mutually exclusive (enforced only by the
> > clash of symbols at link time) set of:
> >     CONFIG_EARLY_UART_PL011
> >     CONFIG_EARLY_UART_EXYNOS4210
> > which if defined must be set to the physical address of the port?
> > 
> > This would mean users need to know the address of the UART on their
> > platform rather than just being able to name the platform though.
> > Perhaps that's a feature not a bug (i.e. they can choose which UART of
> > multiple UARTs if they want). I suppose we could also support
> > CONFIG_EARY_UART_PLATFORM=foo and translate into the above.
> > 
> > Or are there other hardcoded parameters (e.g. crystal rate) which stop
> > this?
> > 
> 
> This drivers have been written with the Exynos 5 platform in mind, so
> I'm not sure it can be called exynos4210.

Maybe I'm confused, I had assumed that exynos4210 was a UART logic block
which happened to have been dropped down onto the exynos5 SoC in much
the same way that the Cortex-A15 contains several pl011 UART logic
blocks. Is this not the case?

In exynos5250.dtsi in Linux the UART is described as compatible =
"samsung,exynos4210-uart", is 4210 actually a previous revision of the
Exynos processor which happens to have a compatible UART on it?

What I want to avoid is the case where we have dozens of UART drivers
which are all identical apart from the fact that they happen to be
integrated into a different SoC (all the other Exynos5xxx variants for
example), so I'd like to determine the most generic name for this driver
possible.

>  There would be few differences
> that I can extract from Linux source code, the physical address is
> different and the few bit to use FIFO are also different but it's not
> use here in the early printk. And I'm almost sure the clock stuff would
> be different on an exynos4.
> 
> So, in my humble opinion, we should use exynos5 or exynos5250.
> 
> 
> >> +/* Exynos 5 UART initialization
> >> + * r11: UART base address
> >> + * Clobber r0-r1 */
> >> +.globl early_uart_init
> >> +early_uart_init:
> >> +        /* init clock */
> >> +        ldr   r1, =0x10020000
> >> +        /* select MPLL (800MHz) source clock */
> >> +        ldr   r0, [r1, #0x250]
> >> +        and   r0, r0, #(~(0xf<<8))
> >> +        orr   r0, r0, #(0x6<<8)
> >> +        str   r0, [r1, #0x250]
> >> +        /* ration 800/(7+1) */
> > 
> > What does "ration" mean here? It doesn't seem to correspond to the
> > French for denominator, numerator, remainder or ratio (at least not
> > according to Google Translate ;-))
> 
> It mean there is a typo ;), someone or something add a N. I suspect it's
> something (the keyboard) :).

I should have thought of that! ;-)

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel