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

[Anthony PERARD <anthony.perard@citrix.com>]

On 02/05/13 08:58, Ian Campbell wrote:
> 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?

Sorry, I've been confused, I did not take close look enough to how the
Linux drivers was written, as the manual does not say anything about the
history. So yes, I feel better in calling the driver exynos4210, with
maybe a different name for the paddr which is exynos5250 specific I suppose.

> 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?

Yes, it seams to be the case, sorry for the confusion.

> 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.


-- 
Anthony PERARD