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

[Will Deacon <will@kernel.org>]

On Wed, May 06, 2020 at 02:02:37PM +0200, Greg KH wrote:
> 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?

writel() usually puts the barrier /before/ the I/O write, since it's
normally used to signal the readiness of a DMA buffer, e.g.:

	ptr = dma_map(...);
	ptr->data = tx_data;
	writel(dev->ctrl_reg, SEND_DATA); // Device must see tx_data

but this driver looks like it only cares about PIO rather than DMA, in which
case there's no need for mb() or writel(); writel_relaxed() should do the
trick because we just need to ensure ordering of the writes hitting the
device. From memory-barriers.txt:

  ... they [relaxed accesses] are still guaranteed to be ordered with
  respect to other accesses from the same CPU thread to the same
  peripheral when operating on __iomem pointers mapped with the default
  I/O attributes.

Will