From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?ISO-8859-1?Q?Jos=E9_Miguel_Gon=E7alves?= <jose.goncalves@inov.pt>
Date: Thu, 13 Sep 2012 10:30:23 +0100
Subject: [U-Boot] [PATCH 3/7] S3C24XX: Add serial driver
In-Reply-To: <201209131117.51999.marex@denx.de>
References: <1347448523-19565-1-git-send-email-jose.goncalves@inov.pt>
	<201209122301.51132.marex@denx.de> <50512ED2.4020803@inov.pt>
	<201209131117.51999.marex@denx.de>
Message-ID: <5051A7AF.20009@inov.pt>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Marek,

On 09/13/2012 10:17 AM, Marek Vasut wrote:
> Dear Jos? Miguel Gon?alves,
>
>> Hi Marek,
>>
>> On 09/12/2012 10:01 PM, Marek Vasut wrote:
>>> Dear Jos? Miguel Gon?alves,
>>>
>>>> Serial driver for the S3C24XX SoCs.
>>>>
>>>> Signed-off-by: Jos? Miguel Gon?alves <jose.goncalves@inov.pt>
>>>> ---
>>>>
>>>>    drivers/serial/Makefile         |    1 +
>>>>    drivers/serial/s3c24xx_serial.c |  146
>>>>
>>>> +++++++++++++++++++++++++++++++++++++++ 2 files changed, 147
>>>> insertions(+)
>>>>
>>>>    create mode 100644 drivers/serial/s3c24xx_serial.c
>>>>
>>>> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
>>>> index 65d0f23..2cbdaac 100644
>>>> --- a/drivers/serial/Makefile
>>>> +++ b/drivers/serial/Makefile
>>>> @@ -52,6 +52,7 @@ COBJS-$(CONFIG_PL011_SERIAL) += serial_pl01x.o
>>>>
>>>>    COBJS-$(CONFIG_PXA_SERIAL) += serial_pxa.o
>>>>    COBJS-$(CONFIG_SA1100_SERIAL) += serial_sa1100.o
>>>>    COBJS-$(CONFIG_S3C24X0_SERIAL) += serial_s3c24x0.o
>>>>
>>>> +COBJS-$(CONFIG_S3C24XX_SERIAL) += s3c24xx_serial.o
>>> What's the difference between those two drivers ?!
>> No substantial differences exists. The UART controller block is the same
>> in all S3C24XX chips. One difference is the number of UARTs. The more
>> recent chips (S3C2416 & S3C2450) have 4 instead of the 3 found on the
>> old ones. Besides that, the driver that I submitted uses a more precise
>> method for baudrate generation.
> So we will have two drivers for the same hardware? No way ... Use the original
> one and apply incremental patches onto it to improve it.
>
>>>> +
>>>> +#ifdef CONFIG_SERIAL0
>>>> +#define UART_NR	S3C24XX_UART0
>>>> +
>>>> +#elif defined(CONFIG_SERIAL1)
>>>> +#define UART_NR	S3C24XX_UART1
>>>> +
>>>> +#elif defined(CONFIG_SERIAL2)
>>>> +#define UART_NR	S3C24XX_UART2
>>>> +
>>>> +#elif defined(CONFIG_SERIAL3)
>>>> +#define UART_NR	S3C24XX_UART3
>>>> +
>>>> +#else
>>>> +#error "Bad: you didn't configure serial ..."
>>> Error itself is "Bad:" so remove it
>> OK.
>>
>>>> +#endif
>>>> +
>>>> +#define barrier() asm volatile("" ::: "memory")
>>> Is that even used ?
>> Yes. Without it the GCC optimization removes the loop at the end of the
>> baurate generation routine.
> So it's yet another accessor issue.
> [...]
>
> But anyway, there's more. I'd like to teach you how to do things properly. So
> let's focus on the in-tree driver and fix that one. Incrementally and in small
> steps.

OK, I will figure out the best way to do this. If I have any doubts I'll 
be back to you...

Best regards,
Jos? Gon?alves