On 13.08.2021 20:31, Marek Marczykowski-Górecki wrote:
> Besides standard UART setup, this device needs enabling
> (vendor-specific) "Enhanced Control Bits" - otherwise disabling hardware
> control flow (MCR[2]) is ignored. Add appropriate quirk to the
> ns16550_setup_preirq(), similar to the handle_dw_usr_busy_quirk(). The
> new function act on Exar cards only (based on vendor ID).
While on IRC you did say you have a datasheet or alike for the specific
card you have in use, may I ask that you clarify why the logic is
applicable to all (past, present, and future) Exar cards?
> @@ -169,6 +170,21 @@ static void handle_dw_usr_busy_quirk(struct ns16550
> *uart)
> }
> }
>
> +static void enable_exar_enhanced_bits(struct ns16550 *uart)
> +{
> +#ifdef NS16550_PCI
> + if ( uart->bar &&
> + pci_conf_read16(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[2],
> + uart->ps_bdf[2]), PCI_VENDOR_ID) ==
> PCI_VENDOR_ID_EXAR )
> + {
> + /* Exar cards ignores setting MCR[2] (hardware flow control) unless
> + * "Enhanced control bits" is enabled.
> + */
Style nit: /* belongs on its own line as per ./CODING_STYLE.
> + ns_write_reg(uart, UART_XR_EFR, UART_EFR_ECB);
Wouldn't this better be a read-modify-write operation?
> --- a/xen/include/xen/8250-uart.h
> +++ b/xen/include/xen/8250-uart.h
> @@ -121,6 +121,10 @@
> /* Frequency of external clock source. This definition assumes PC platform.
> */
> #define UART_CLOCK_HZ 1843200
>
> +/* Exar specific */
> +#define UART_XR_EFR 0x09
> +#define UART_EFR_ECB 0x10
Please move the former into the group after the "/* Register offsets */"
comment and extend the comment on the latter to establish a link to
UART_XR_EFR.
Jan
