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

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, May 14, 2020 at 07:02:49PM +0530, Mukesh, Savaliya wrote:
> 
> On 5/6/2020 5:32 PM, 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?
> > 
> > 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.
> Removed mb() calls due to _relaxed() APIs taking care of the same.
> > 
> > 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.
> 
> I ran the script now also on same patch, didn't any warning/error. Do you
> see the error ? Below is the output:
> 
> ./scripts/checkpatch.pl
> 0001-serial-msm_geni_serial_console-Add-Earlycon-support.patch
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #58:
> new file mode 100644
> 
> total: 0 errors, 1 warnings, 511 lines checked

You need ' ' around the '+' characters.  Odd that it didn't catch that.

Also, you added a file with no MAINTAINERS entry, please add your name
to it so we know who to bother over time :)

thanks,

greg k-h