| Message ID | 1466500528-8411-3-git-send-email-dirk.behme@de.bosch.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Dirk, On 21/06/16 10:15, Dirk Behme wrote: > Besides the 14MHz external clock, the SCIF might be clocked by an > internal 66MHz clock. Detect this clock based on the SCIF_DL register > being 0 (internal clock) or != 0 (external clock). Do you have a public link to the specification? > Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com> > --- > xen/drivers/char/scif-uart.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c > index bc157fe..678f46b 100644 > --- a/xen/drivers/char/scif-uart.c > +++ b/xen/drivers/char/scif-uart.c > @@ -107,8 +107,19 @@ static void __init scif_uart_init_preirq(struct serial_port *port) > scif_readw(uart, SCIF_SCLSR); > scif_writew(uart, SCIF_SCLSR, 0); > > - /* Select Baud rate generator output as a clock source */ > - scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); > + /* > + * Select Baud rate generator output as a clock source > + * The clock source can be an internal or external clock. > + * Depending on this the DL register is either 0 or contains > + * the divisor. I.e. we can use this to detect the clock > + * source and based on this can configure the CKE[1:0] bits > + * of the SCSCR register. > + */ > + if ( scif_readw(uart, SCIF_DL) ) > + scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); /* External clk */ > + else > + scif_writew(uart, SCIF_SCSCR, SCSCR_CKE00); /* Internal clk */ Why would we need to select the baud rate generator if the baud has been configured by the firmware? > + Please drop this newline. > > /* Setup protocol format and Baud rate, select Asynchronous mode */ > val = 0; > Regards,
Hi Julien, On 21.06.2016 14:20, Julien Grall wrote: > Hello Dirk, > > On 21/06/16 10:15, Dirk Behme wrote: >> Besides the 14MHz external clock, the SCIF might be clocked by an >> internal 66MHz clock. Detect this clock based on the SCIF_DL register >> being 0 (internal clock) or != 0 (external clock). > > Do you have a public link to the specification? I have to check this ;) >> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com> >> --- >> xen/drivers/char/scif-uart.c | 15 +++++++++++++-- >> 1 file changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c >> index bc157fe..678f46b 100644 >> --- a/xen/drivers/char/scif-uart.c >> +++ b/xen/drivers/char/scif-uart.c >> @@ -107,8 +107,19 @@ static void __init scif_uart_init_preirq(struct >> serial_port *port) >> scif_readw(uart, SCIF_SCLSR); >> scif_writew(uart, SCIF_SCLSR, 0); >> >> - /* Select Baud rate generator output as a clock source */ >> - scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); >> + /* >> + * Select Baud rate generator output as a clock source >> + * The clock source can be an internal or external clock. >> + * Depending on this the DL register is either 0 or contains >> + * the divisor. I.e. we can use this to detect the clock >> + * source and based on this can configure the CKE[1:0] bits >> + * of the SCSCR register. >> + */ >> + if ( scif_readw(uart, SCIF_DL) ) >> + scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); /* External clk */ >> + else >> + scif_writew(uart, SCIF_SCSCR, SCSCR_CKE00); /* Internal clk */ > > Why would we need to select the baud rate generator if the baud has been > configured by the firmware? Just to get the correct understanding: The proposal is to just remove the code which (wrongly) overwrites the correct settings done by the firmware? I.e. instead of doing the same thing the firmware is already doing, again (the if .. else ), the proposal is simply dropping the - /* Select Baud rate generator output as a clock source */ - scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); ? Best regards Dirk
On 21/06/16 13:30, Dirk Behme wrote: >>> diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c >>> index bc157fe..678f46b 100644 >>> --- a/xen/drivers/char/scif-uart.c >>> +++ b/xen/drivers/char/scif-uart.c >>> @@ -107,8 +107,19 @@ static void __init scif_uart_init_preirq(struct >>> serial_port *port) >>> scif_readw(uart, SCIF_SCLSR); >>> scif_writew(uart, SCIF_SCLSR, 0); >>> >>> - /* Select Baud rate generator output as a clock source */ >>> - scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); >>> + /* >>> + * Select Baud rate generator output as a clock source >>> + * The clock source can be an internal or external clock. >>> + * Depending on this the DL register is either 0 or contains >>> + * the divisor. I.e. we can use this to detect the clock >>> + * source and based on this can configure the CKE[1:0] bits >>> + * of the SCSCR register. >>> + */ >>> + if ( scif_readw(uart, SCIF_DL) ) >>> + scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); /* External clk */ >>> + else >>> + scif_writew(uart, SCIF_SCSCR, SCSCR_CKE00); /* Internal clk */ >> >> Why would we need to select the baud rate generator if the baud has been >> configured by the firmware? > > > Just to get the correct understanding: The proposal is to just remove > the code which (wrongly) overwrites the correct settings done by the > firmware? > > I.e. instead of doing the same thing the firmware is already doing, > again (the if .. else ), the proposal is simply dropping the > > > - /* Select Baud rate generator output as a clock source */ > - scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); > > ? Yes. However I don't have any spec in hand so I am not sure if it is correct. Regards,
diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c index bc157fe..678f46b 100644 --- a/xen/drivers/char/scif-uart.c +++ b/xen/drivers/char/scif-uart.c @@ -107,8 +107,19 @@ static void __init scif_uart_init_preirq(struct serial_port *port) scif_readw(uart, SCIF_SCLSR); scif_writew(uart, SCIF_SCLSR, 0); - /* Select Baud rate generator output as a clock source */ - scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); + /* + * Select Baud rate generator output as a clock source + * The clock source can be an internal or external clock. + * Depending on this the DL register is either 0 or contains + * the divisor. I.e. we can use this to detect the clock + * source and based on this can configure the CKE[1:0] bits + * of the SCSCR register. + */ + if ( scif_readw(uart, SCIF_DL) ) + scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); /* External clk */ + else + scif_writew(uart, SCIF_SCSCR, SCSCR_CKE00); /* Internal clk */ + /* Setup protocol format and Baud rate, select Asynchronous mode */ val = 0;
Besides the 14MHz external clock, the SCIF might be clocked by an internal 66MHz clock. Detect this clock based on the SCIF_DL register being 0 (internal clock) or != 0 (external clock). Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com> --- xen/drivers/char/scif-uart.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)