Patchwork [U-Boot,07/34] zynq: Add UART0, UART1 configs support

login
register
mail settings
Submitter Jagannadha Sutradharudu Teki
Date Nov. 5, 2013, 5:46 p.m.
Message ID <1249b50d-d1d1-42ad-b612-7aa9901598ad@VA3EHSMHS029.ehs.local>
Download mbox | patch
Permalink /patch/288609/
State Accepted
Delegated to: Michal Simek
Headers show

Comments

Jagannadha Sutradharudu Teki - Nov. 5, 2013, 5:46 p.m.
Zynq uart controller support two serial ports like
CONFIG_ZYNQ_SERIAL_UART0 and CONFIG_ZYNQ_SERIAL_UART1
enabled both so-that the respective board will define
these macros based on their usage.

Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
---
 include/configs/zynq.h | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)
Dinh Nguyen - Nov. 6, 2013, 4:17 a.m.
On 11/5/13 11:46 AM, Jagannadha Sutradharudu Teki wrote:
> Zynq uart controller support two serial ports like
> CONFIG_ZYNQ_SERIAL_UART0 and CONFIG_ZYNQ_SERIAL_UART1
> enabled both so-that the respective board will define
> these macros based on their usage.
>
> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
> ---
>  include/configs/zynq.h | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/include/configs/zynq.h b/include/configs/zynq.h
> index f32008b..1bcb28d 100644
> --- a/include/configs/zynq.h
> +++ b/include/configs/zynq.h
> @@ -33,10 +33,22 @@
>  	{300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200, 230400}
>  
>  /* Zynq Serial driver */
> -#define CONFIG_ZYNQ_SERIAL
> -#define CONFIG_ZYNQ_SERIAL_BASEADDR0	0xE0001000
> -#define CONFIG_ZYNQ_SERIAL_BAUDRATE0	CONFIG_BAUDRATE
> -#define CONFIG_ZYNQ_SERIAL_CLOCK0	50000000
> +#define CONFIG_ZYNQ_SERIAL_UART1
> +#ifdef CONFIG_ZYNQ_SERIAL_UART0
> +# define CONFIG_ZYNQ_SERIAL_BASEADDR0	0xE0000000
> +# define CONFIG_ZYNQ_SERIAL_BAUDRATE0	CONFIG_BAUDRATE
Why couldn't you just use CONFIG_BAUDRATE? Why do you need to add
another define?

Dinh
> +# define CONFIG_ZYNQ_SERIAL_CLOCK0	50000000
> +#endif
> +
> +#ifdef CONFIG_ZYNQ_SERIAL_UART1
> +# define CONFIG_ZYNQ_SERIAL_BASEADDR1	0xE0001000
> +# define CONFIG_ZYNQ_SERIAL_BAUDRATE1	CONFIG_BAUDRATE
> +# define CONFIG_ZYNQ_SERIAL_CLOCK1	50000000
> +#endif
> +
> +#if defined(CONFIG_ZYNQ_SERIAL_UART0) || defined(CONFIG_ZYNQ_SERIAL_UART1)
> +# define CONFIG_ZYNQ_SERIAL
> +#endif
>  
>  /* DCC driver */
>  #if defined(CONFIG_ZYNQ_DCC)
Dinh Nguyen - Nov. 6, 2013, 5 a.m.
On 11/5/13 11:46 AM, Jagannadha Sutradharudu Teki wrote:
> Zynq uart controller support two serial ports like
> CONFIG_ZYNQ_SERIAL_UART0 and CONFIG_ZYNQ_SERIAL_UART1
> enabled both so-that the respective board will define
> these macros based on their usage.
>
> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
> ---
>  include/configs/zynq.h | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/include/configs/zynq.h b/include/configs/zynq.h
> index f32008b..1bcb28d 100644
> --- a/include/configs/zynq.h
> +++ b/include/configs/zynq.h
> @@ -33,10 +33,22 @@
>  	{300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200, 230400}
>  
>  /* Zynq Serial driver */
> -#define CONFIG_ZYNQ_SERIAL
> -#define CONFIG_ZYNQ_SERIAL_BASEADDR0	0xE0001000
> -#define CONFIG_ZYNQ_SERIAL_BAUDRATE0	CONFIG_BAUDRATE
> -#define CONFIG_ZYNQ_SERIAL_CLOCK0	50000000
> +#define CONFIG_ZYNQ_SERIAL_UART1
> +#ifdef CONFIG_ZYNQ_SERIAL_UART0
> +# define CONFIG_ZYNQ_SERIAL_BASEADDR0	0xE0000000
> +# define CONFIG_ZYNQ_SERIAL_BAUDRATE0	CONFIG_BAUDRATE
Why can't the driver just make use of CONFIG_BAUDRATE? Why do you need
to add another
define?

Dinh
> +# define CONFIG_ZYNQ_SERIAL_CLOCK0	50000000
> +#endif
> +
> +#ifdef CONFIG_ZYNQ_SERIAL_UART1
> +# define CONFIG_ZYNQ_SERIAL_BASEADDR1	0xE0001000
> +# define CONFIG_ZYNQ_SERIAL_BAUDRATE1	CONFIG_BAUDRATE
> +# define CONFIG_ZYNQ_SERIAL_CLOCK1	50000000
> +#endif
> +
> +#if defined(CONFIG_ZYNQ_SERIAL_UART0) || defined(CONFIG_ZYNQ_SERIAL_UART1)
> +# define CONFIG_ZYNQ_SERIAL
> +#endif
>  
>  /* DCC driver */
>  #if defined(CONFIG_ZYNQ_DCC)
Michal Simek - Nov. 6, 2013, 6:45 a.m.
On 11/06/2013 05:17 AM, Dinh Nguyen wrote:
> 
> On 11/5/13 11:46 AM, Jagannadha Sutradharudu Teki wrote:
>> Zynq uart controller support two serial ports like
>> CONFIG_ZYNQ_SERIAL_UART0 and CONFIG_ZYNQ_SERIAL_UART1
>> enabled both so-that the respective board will define
>> these macros based on their usage.
>>
>> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>> ---
>>  include/configs/zynq.h | 20 ++++++++++++++++----
>>  1 file changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/configs/zynq.h b/include/configs/zynq.h
>> index f32008b..1bcb28d 100644
>> --- a/include/configs/zynq.h
>> +++ b/include/configs/zynq.h
>> @@ -33,10 +33,22 @@
>>  	{300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200, 230400}
>>  
>>  /* Zynq Serial driver */
>> -#define CONFIG_ZYNQ_SERIAL
>> -#define CONFIG_ZYNQ_SERIAL_BASEADDR0	0xE0001000
>> -#define CONFIG_ZYNQ_SERIAL_BAUDRATE0	CONFIG_BAUDRATE
>> -#define CONFIG_ZYNQ_SERIAL_CLOCK0	50000000
>> +#define CONFIG_ZYNQ_SERIAL_UART1
>> +#ifdef CONFIG_ZYNQ_SERIAL_UART0
>> +# define CONFIG_ZYNQ_SERIAL_BASEADDR0	0xE0000000
>> +# define CONFIG_ZYNQ_SERIAL_BAUDRATE0	CONFIG_BAUDRATE
> Why couldn't you just use CONFIG_BAUDRATE? Why do you need to add
> another define?

If we start to use the same CONFIG_BAUDRATE(or better gd->baudrate)
in the driver then we lose config options to setup different baudrate
for every single device.

I haven't seen any code for SERIAL_MULTI if you want to configure
2 serial drivers but on different baudrate.
If there is better way how to do it please let us know.

Thanks,
Michal
Dinh Nguyen - Nov. 6, 2013, 1:50 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512


On 11/6/13 12:45 AM, Michal Simek wrote:
> On 11/06/2013 05:17 AM, Dinh Nguyen wrote:
>>
>> On 11/5/13 11:46 AM, Jagannadha Sutradharudu Teki wrote:
>>> Zynq uart controller support two serial ports like
>>> CONFIG_ZYNQ_SERIAL_UART0 and CONFIG_ZYNQ_SERIAL_UART1
>>> enabled both so-that the respective board will define
>>> these macros based on their usage.
>>>
>>> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>>> ---
>>>  include/configs/zynq.h | 20 ++++++++++++++++----
>>>  1 file changed, 16 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/configs/zynq.h b/include/configs/zynq.h
>>> index f32008b..1bcb28d 100644
>>> --- a/include/configs/zynq.h
>>> +++ b/include/configs/zynq.h
>>> @@ -33,10 +33,22 @@
>>>      {300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200,
230400}
>>> 
>>>  /* Zynq Serial driver */
>>> -#define CONFIG_ZYNQ_SERIAL
>>> -#define CONFIG_ZYNQ_SERIAL_BASEADDR0    0xE0001000
>>> -#define CONFIG_ZYNQ_SERIAL_BAUDRATE0    CONFIG_BAUDRATE
>>> -#define CONFIG_ZYNQ_SERIAL_CLOCK0    50000000
>>> +#define CONFIG_ZYNQ_SERIAL_UART1
>>> +#ifdef CONFIG_ZYNQ_SERIAL_UART0
>>> +# define CONFIG_ZYNQ_SERIAL_BASEADDR0    0xE0000000
>>> +# define CONFIG_ZYNQ_SERIAL_BAUDRATE0    CONFIG_BAUDRATE
>> Why couldn't you just use CONFIG_BAUDRATE? Why do you need to add
>> another define?
>
> If we start to use the same CONFIG_BAUDRATE(or better gd->baudrate)
> in the driver then we lose config options to setup different baudrate
> for every single device.
>
> I haven't seen any code for SERIAL_MULTI if you want to configure
> 2 serial drivers but on different baudrate.
> If there is better way how to do it please let us know.
I guess the question then is what would be the use case for supporting
different baudrates on different device?

Dinh
>
>
> Thanks,
> Michal
>

-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.22 (Darwin)
Comment: GPGTools - https://gpgtools.org
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBCgAGBQJSekkTAAoJEBmUBAuBoyj03s8QAKZ/t9Ou2JAlcy8kg1Dqh1P5
CnBgoqekXEVJf8prPrK3sCx5l4bvmCY4yx84O7pqQFYW7uSWW3M7m6yxBoaQbDmx
nH6fDnd8aiAd1YgTpzbsoFI/ZJQ2hla/xnjFwzfjIfPtWrdqmqvltDmgKnRT1Zxi
HDQCgnQSQQKAvz7wZ+/U5i8xXI9nSEPALqvHrSpKrgBLWSAwbVCVf+FNYcPIoMh9
cZ9QNUW5FVmC9eLdCNUKr3mZoe2RIdrWS7/FPSQge9OjW84syoY6JPH7L0e4toDM
Wmf84+3/rbBw87aC6hbBYvX1jpeb/St7ylxJqTZQ/6HUKlDmWSR9H5isfz3ndaej
uT9pmK4dLDBaWRwdlSzogjsJEfbntiFYKiQsJmcL6ykB7yJCqiD4+ErF8SLCDAsD
1Dtvl+Kd6MeVXnoWuPc6XI0pJjabgGyJIHfZaEX3XfI3ybokedYxQpnavGrU2Dyz
YYQqRgnmaQQwzEI1eYjwazNNAqfdkM6s0olBYelON20yc685/3gQcLjNNs13U9i7
rT5ck+7oaW7+8jazDFjvYynuvdVAeeSwh0cSP3ZSHRYwS3ddAi8EG+HhOkrr3trZ
Hmbso2nF33ofjPe4ZJqQDfVyhBAeTcMx2p5L0d1piEkHKi77MTehpU1Fp3LAvSZ5
NUq9dEtCltJNFD0Cgd9Q
=O22Y
-----END PGP SIGNATURE-----
Michal Simek - Nov. 6, 2013, 5:43 p.m.
On 11/06/2013 02:50 PM, Dinh Nguyen wrote:
> 
> 
> On 11/6/13 12:45 AM, Michal Simek wrote:
>> On 11/06/2013 05:17 AM, Dinh Nguyen wrote:
>>>
>>> On 11/5/13 11:46 AM, Jagannadha Sutradharudu Teki wrote:
>>>> Zynq uart controller support two serial ports like
>>>> CONFIG_ZYNQ_SERIAL_UART0 and CONFIG_ZYNQ_SERIAL_UART1
>>>> enabled both so-that the respective board will define
>>>> these macros based on their usage.
>>>>
>>>> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>>>> ---
>>>>  include/configs/zynq.h | 20 ++++++++++++++++----
>>>>  1 file changed, 16 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/include/configs/zynq.h b/include/configs/zynq.h
>>>> index f32008b..1bcb28d 100644
>>>> --- a/include/configs/zynq.h
>>>> +++ b/include/configs/zynq.h
>>>> @@ -33,10 +33,22 @@
>>>>      {300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200,
> 230400}
>>>>
>>>>  /* Zynq Serial driver */
>>>> -#define CONFIG_ZYNQ_SERIAL
>>>> -#define CONFIG_ZYNQ_SERIAL_BASEADDR0    0xE0001000
>>>> -#define CONFIG_ZYNQ_SERIAL_BAUDRATE0    CONFIG_BAUDRATE
>>>> -#define CONFIG_ZYNQ_SERIAL_CLOCK0    50000000
>>>> +#define CONFIG_ZYNQ_SERIAL_UART1
>>>> +#ifdef CONFIG_ZYNQ_SERIAL_UART0
>>>> +# define CONFIG_ZYNQ_SERIAL_BASEADDR0    0xE0000000
>>>> +# define CONFIG_ZYNQ_SERIAL_BAUDRATE0    CONFIG_BAUDRATE
>>> Why couldn't you just use CONFIG_BAUDRATE? Why do you need to add
>>> another define?
> 
>> If we start to use the same CONFIG_BAUDRATE(or better gd->baudrate)
>> in the driver then we lose config options to setup different baudrate
>> for every single device.
> 
>> I haven't seen any code for SERIAL_MULTI if you want to configure
>> 2 serial drivers but on different baudrate.
>> If there is better way how to do it please let us know.
> I guess the question then is what would be the use case for supporting
> different baudrates on different device?

I don't have any particular case in my mind but based on my experience
with various customer they could have intention to replace old subsystem
by fpga and one serial console can go to standard output which is logged
by default and different serial output for debug purpose and both
can run on different baudrates.

I just think that with SERIAL_MULTI support various baudrate should be supported
and doing it through macros is one way to go.

Thanks,
Michal

Patch

diff --git a/include/configs/zynq.h b/include/configs/zynq.h
index f32008b..1bcb28d 100644
--- a/include/configs/zynq.h
+++ b/include/configs/zynq.h
@@ -33,10 +33,22 @@ 
 	{300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200, 230400}
 
 /* Zynq Serial driver */
-#define CONFIG_ZYNQ_SERIAL
-#define CONFIG_ZYNQ_SERIAL_BASEADDR0	0xE0001000
-#define CONFIG_ZYNQ_SERIAL_BAUDRATE0	CONFIG_BAUDRATE
-#define CONFIG_ZYNQ_SERIAL_CLOCK0	50000000
+#define CONFIG_ZYNQ_SERIAL_UART1
+#ifdef CONFIG_ZYNQ_SERIAL_UART0
+# define CONFIG_ZYNQ_SERIAL_BASEADDR0	0xE0000000
+# define CONFIG_ZYNQ_SERIAL_BAUDRATE0	CONFIG_BAUDRATE
+# define CONFIG_ZYNQ_SERIAL_CLOCK0	50000000
+#endif
+
+#ifdef CONFIG_ZYNQ_SERIAL_UART1
+# define CONFIG_ZYNQ_SERIAL_BASEADDR1	0xE0001000
+# define CONFIG_ZYNQ_SERIAL_BAUDRATE1	CONFIG_BAUDRATE
+# define CONFIG_ZYNQ_SERIAL_CLOCK1	50000000
+#endif
+
+#if defined(CONFIG_ZYNQ_SERIAL_UART0) || defined(CONFIG_ZYNQ_SERIAL_UART1)
+# define CONFIG_ZYNQ_SERIAL
+#endif
 
 /* DCC driver */
 #if defined(CONFIG_ZYNQ_DCC)