On Wed, May 06, 2020 at 05:03:31PM +0530, Mukesh, Savaliya wrote: > +static void msm_geni_serial_wr_char(struct uart_port *uport, int ch) > +{ > + writel_relaxed(ch, uport->membase+SE_GENI_TX_FIFOn); > + /* > + * Ensure FIFO write clear goes through before > + * next iteration. > + */ > + mb(); Can't you just write the above two lines as: writel(ch, uport->membase+SE_GENI_TX_FIFOn); ? Why put a mb() after a _relaxed() call? Will, I know I asked you about this on irc a while ago, is the above really correct? This happens other places in the driver. Also, Savaliya, please use checkpatch on your patch, you need some whitespace fixes in this code before I could accept it at the very least. thanks, greg k-h