Re: [PATCH V2] serial: msm_geni_serial_console : Add Earlycon support

["Mukesh, Savaliya" <msavaliy@codeaurora.org>]

On 5/6/2020 5:35 PM, Greg KH wrote:
> On Wed, May 06, 2020 at 05:03:31PM +0530, Mukesh, Savaliya wrote:
>> +		/*
>> +		 * If the WM bit never set, then the Tx state machine is not
>> +		 * in a valid state, so break, cancel/abort any existing
>> +		 * command. Unfortunately the current data being written is
>> +		 * lost.
>> +		 */
>> +		while (!msm_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
>> +						M_TX_FIFO_WATERMARK_EN, true))
>> +			break;
> Am I reading this correctly in that if the bit is set properly, you will
> just loop for forever?
>
> That feels wrong, are you sure this is correct?

It seems correct to me. The API returns true if function sees the BIT is 
set else returns false.

Mostly it's true and returns true, hence it shouldn't loop.

>
>> +		chars_to_write = min((unsigned int)(count - i),
>> +							avail_fifo_bytes);
>> +		if ((chars_to_write << 1) > avail_fifo_bytes)
>> +			chars_to_write = (avail_fifo_bytes >> 1);
>> +		uart_console_write(uport, (s + i), chars_to_write,
>> +					msm_geni_serial_wr_char);
>> +		writel_relaxed(M_TX_FIFO_WATERMARK_EN,
>> +			uport->membase+SE_GENI_M_IRQ_CLEAR);
>> +		/* Ensure this goes through before polling for WM IRQ again.*/
>> +		mb();
> Again, writel() on its own should be fine here, and elsewhere.
Removed mb() from whole file as _relaxed takes care of it.
>
> thanks,
>
> greg k-h