| Message ID | 1249b50d-d1d1-42ad-b612-7aa9901598ad@VA3EHSMHS029.ehs.local |
|---|---|
| State | Accepted |
| Delegated to: | Michal Simek |
| Headers | show |
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)
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)
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
-----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-----
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
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)
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(-)