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
