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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
