Hi Oleksandr,
On 4/15/19 12:30 PM, Oleksandr wrote:
On 14.04.19 19:55, Julien Grall wrote:
Hi Oleksandr,
Hi Julien
On 4/8/19 11:14 AM, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com>
Extend driver to be able to handle other SCIF(X) compatible
interfaces as well. These interfaces have lot in common,
but mostly differ in offsets and bits for some registers.
Can you briefly explain in the commit message what differs?
Sure
[...]
@@ -65,13 +100,16 @@ static void scif_uart_interrupt(int irq, void
*data, struct cpu_user_regs *regs)
serial_rx_interrupt(port, regs);
/* Error Interrupt */
- if ( status & SCIF_ERRORS )
- scif_writew(uart, SCIF_SCFSR, ~SCIF_ERRORS);
- if ( scif_readw(uart, SCIF_SCLSR) & SCLSR_ORER )
- scif_writew(uart, SCIF_SCLSR, 0);
+ if ( status & params->error_mask )
+ scif_writew(uart, params->status_reg, ~params->error_mask);
+ if ( params->overrun_reg != params->status_reg )
Below you will write the same register twice if overrun_reg ==
status_reg (see scif_uart_init_preirq). Would there be any issue if
you do the same here?
I didn't expect any issue to write the same register twice during
initialization to simplify the code, that why I agreed to remove the
check in V1.
But I am not sure about doing so here. We could simplify the code by
just removing "if ( params->overrun_reg != params->status_reg )",
but we would need to do extra operation for SCIFA which has overrun_reg
== status_reg.
What way do you prefer?
It is not about preference, it is about correctness. In my previous
reply, I pointed out that you define overrun_mask for SCIFA but it will
never be used. Without any explanation, the code looks pretty wrong.
Looking at the next patch, you get away from any problem with SCIFA
because the overrun_mask is a subset of error_mask. But I still don't
see why trying to prevent to write a second time is an issue, the more
the Linux driver does not seem to do this dance.
So at best, you need to explain in the commit message why you try to
skip the register when it is the same.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
