Re: [PATCH 1/1] usbserial: cp210x - icount support for parity error checking

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Sun, Jun 21, 2020 at 11:45:11AM +0200, Jerry wrote:
> Greg Kroah-Hartman wrote on 6/21/20 10:58 AM:
> > On Sat, Jun 20, 2020 at 09:58:40PM +0200, Jerry wrote:
> > > usbserial: add cp210x support for icount to detect parity error in received data
> > Why is this here?
> > 
> Because it seems be mandatory?
> https://www.kernel.org/doc/html/latest/process/5.Posting.html#patch-formatting-and-changelogs
> 
> "A one-line description of what the patch does. This message should be
> enough for a reader who sees it with no other context to figure out the
> scope of the patch; it is the line that will show up in the “short form”
> changelogs. This message is usually formatted with the relevant subsystem
> name first, followed by the purpose of the patch. For example:
> gpio: fix build on CONFIG_GPIO_SYSFS=n"

Yes, that should have been the first line of the git commit, which ends
up being the subject line for your email.

> Did I misunderstand your rule or used wrong name of subsystem? Should I
> type?
> USB serial: add cp210x support for icount to detect parity error in received
> data

That would have been fine too, you can't do it twice, once as a subject
and once as the first line in the email, otherwise that would look
really odd, right?

> > > Motivation - current version of cp210x driver doesn't provide any way to detect
> > > a parity error in received data from userspace. Some serial protocols like STM32
> > > bootloader protect data only by even parity so application needs to detect
> > > whether parity error happened to read again peripheral data.
> > > 
> > > I created a simple patch which adds support for icount (ioctl TIOCGICOUNT) which
> > > sends GET_COMM_STATUS command to CP210X and according received flags increments
> > > fields for parity error, frame error, break and overrun.
> > > So application can detect an error condition after reading data from ttyUSB
> > > and repeat operation. There is no impact for applications which don't
> > > call ioctl TIOCGICOUNT.
> > > This patch is also placed at http://yyy.jrr.cz/cp210x.patch (my first patch)
> > Please read the section entitled "The canonical patch format" in the
> > kernel file, Documentation/SubmittingPatches for what is needed in order
> > to properly describe the change.
> I read it, but still not sure what exactly was wrong? Yes, I wrapped lines
> of description to 80 colums and now I noticed that only 75 columns is
> allowed but I doubt that it is all?

That is one thing, but also the "This patch..." should not be in a
changelog, right?  Look at the other changes sent to the list for
examples of how to do this.

> > > Signed-off-by: Jaromir Skorpil <Jerry@jrr.cz>
> > This does not match your From: line :(
> I supposed that only mail address in From line matter?
> I understand that real name is mandatory only for Signed-off-by field?

It has to match the From: line of your email to ensure that this really
is the same person.

thanks,

greg k-h