[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 1/3] xen/arm: drivers: scif: Remove dead code
Hello Oleksandr,
On 21.06.2016 14:54, Oleksandr Tyshchenko wrote:
On Tue, Jun 21, 2016 at 3:15 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
On 21/06/16 11:16, Oleksandr Tyshchenko wrote:
Hi, Dirk.
Hello Oleksandr,
Hello Julien.
On Tue, Jun 21, 2016 at 12:15 PM, Dirk Behme <dirk.behme@xxxxxxxxxxxx>
wrote:
In scif_uart_init() uart->baud is set to BAUD_AUTO. So its a basic error
if this is different later. Detect this by an ASSERT, but remove the
dead code normally never reached.
Signed-off-by: Dirk Behme <dirk.behme@xxxxxxxxxxxx>
---
xen/drivers/char/scif-uart.c | 23 ++++++-----------------
1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
index 51a2233..ca88c0f 100644
--- a/xen/drivers/char/scif-uart.c
+++ b/xen/drivers/char/scif-uart.c
@@ -143,23 +143,12 @@ static void __init scif_uart_init_preirq(struct
serial_port *port)
scif_writew(uart, SCIF_SCSMR, val);
ASSERT( uart->clock_hz > 0 );
- if ( uart->baud != BAUD_AUTO )
- {
- /* Setup desired Baud rate */
- divisor = uart->clock_hz / (uart->baud << 4);
- ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
- scif_writew(uart, SCIF_DL, (uint16_t)divisor);
- /* Selects the frequency divided clock (SC_CLK external input)
*/
- scif_writew(uart, SCIF_CKS, 0);
- udelay(1000000 / uart->baud + 1);
This part of code might be used for people who are not satisfied with
default baudrate which has been set in U-Boot.
Can you elaborate? If the baud rate is different, it will not be possible to
get output from U-boot.
If we remove this we will take away the opportunity to just change
uart->baud from BAUD_AUTO to desired one.
I have some doubt that the current code is valid. The clock frequency is
hardcoded (see SCIF_CLK_FREQ), so are you saying that the frequency is
always the same across all the platforms?
No.
This driver has been initially written for Renesas Lager board based
on R-Car H2 SoC which
has SCIF compatible UART. And the current code is valid for it. I have
tested both auto and
variable (38400, 115200) baudrates.
Could you help me a little to understand this?
The driver has
scif_uart_init()
{
...
struct scif_uart *uart;
...
uart->baud = BAUD_AUTO;
...
}
I checked the code for struct scif_uart but it isn't used anywhere
outside this driver. So uart->baud is a driver local variable, which
looks to me isn't used anywhere useful.
What have I missed?
Best regards
Dirk
But, I am afraid that current code won't be suitable for
all of the boards which have the same UART IP. It depends at least
from clock source
(external/internal) and clock frequency.
I would rather avoid to keep dead code (or not accessible without hacking
Xen). For what is worth, we recently removed a similar code from the PL011
driver as this should be correctly configured by the firmware.
- }
- else
- {
- /* Read current Baud rate */
- divisor = scif_readw(uart, SCIF_DL);
- ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
- uart->baud = uart->clock_hz / (divisor << 4);
- }
+ ASSERT( uart->baud == BAUD_AUTO );
+
+ /* Read current Baud rate */
+ divisor = scif_readw(uart, SCIF_DL);
+ ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
+ uart->baud = uart->clock_hz / (divisor << 4);
/* Setup trigger level for TX/RX FIFOs */
scif_writew(uart, SCIF_SCFCR, SCFCR_RTRG11 | SCFCR_TTRG11);
--
2.8.0
Regards,
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
