On 02/07/2017 11:05 PM, Timur Tabi wrote: > Christopher Covington wrote: >> The Qualcomm Datacenter Technologies QDF2400 family of SoCs contains a >> custom (non-PrimeCell) implementation of the SBSA UART. Occasionally the >> BUSY bit in the Flag Register gets stuck as 1, erratum 44 for both 2432v1 >> and 2400v1 SoCs. Checking that the Transmit FIFO Empty (TXFE) bit is 0, >> instead of checking that the BUSY bit is 1, works around the issue. To >> facilitate this substitution when UART AMBA Port (UAP) data is available, >> introduce vendor-specific inversion of Feature Register bits. To keep the >> change small, this patch only works around the full console case, where UAP >> data is available, and does not handle the erratum for earlycon, as the UAP >> data is not available then.
>> +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44 >> +static struct vendor_data vendor_qdt_qdf2400_e44 = { >> + .reg_offset = pl011_std_offsets, >> + .fr_busy = UART011_FR_TXFE, >> + .fr_dsr = UART01x_FR_DSR, >> + .fr_cts = UART01x_FR_CTS, >> + .fr_ri = UART011_FR_RI, >> + .inv_fr = UART011_FR_TXFE, >> + .access_32b = true, >> + .oversampling = false, >> + .dma_threshold = false, >> + .cts_event_workaround = false, >> + .always_enabled = true, >> + .fixed_options = true, >> +}; >> +#else >> +#define vendor_qdt_qdf2400_e44 vendor_sbsa >> +#endif > > Instead of the #else, just put the #ifdef inside qdf2400_e44(). That > way, the function always returns False if > CONFIG_QCOM_QDF2400_ERRATUM_44 is not defined. I'll consolidate the #ifdefery around qdf2400_e44(). > Also, don't you need to add a definition of .inv_fr in vendor_sbsa > and the other vendor_xxx structures? The current struct definitions appear to (ab)use implicit zero-initialization, so I did too. >> + >> static u16 pl011_st_offsets[REG_ARRAY_SIZE] = { >> [REG_DR] = UART01x_DR, >> [REG_ST_DMAWM] = ST_UART011_DMAWM, >> @@ -1518,7 +1538,7 @@ static unsigned int pl011_tx_empty(struct uart_port >> *port) >> { >> struct uart_amba_port *uap = >> container_of(port, struct uart_amba_port, port); >> - unsigned int status = pl011_read(uap, REG_FR); >> + unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr; >> return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ? >> 0 : TIOCSER_TEMT; >> } >> @@ -2218,7 +2238,8 @@ pl011_console_write(struct console *co, const char *s, >> unsigned int count) >> * Finally, wait for transmitter to become empty >> * and restore the TCR >> */ >> - while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy) >> + while ((pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr) >> + & uap->vendor->fr_busy) > > I really think the XOR logic needs to be documented wherever it's > used. It's just too confusing. References such as the following for basic bit operations like setting, clearing, and toggling are easy enough to come by. http://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit-in-c-c Cov -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html