On Wed, 27 Jan 2021 15:13:05 +0100
Greg KH <[email protected]> wrote:
> On Wed, Jan 27, 2021 at 10:08:09PM +0800, carlis wrote:
> > On Wed, 27 Jan 2021 14:51:55 +0100
> > Greg KH <[email protected]> wrote:
> >
> > > On Wed, Jan 27, 2021 at 09:42:52PM +0800, Carlis wrote:
> > > > From: zhangxuezhi <[email protected]>
> > > >
> > > > For st7789v ic,when we need continuous full screen refresh, it
> > > > is best to wait for the TE signal arrive to avoid screen tearing
> > > >
> > > > Signed-off-by: zhangxuezhi <[email protected]>
> > >
> > > Please slow down and wait at least a day between patch
> > > submissions, there is no rush here.
> > >
> > > And also, ALWAYS run scripts/checkpatch.pl on your submissions, so
> > > that you don't have a maintainer asking you about basic problems,
> > > like are in this current patch :(
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > hi,
> > This is my first patch contribution to Linux, so some of the rules
> > are not very clear .In addition, I can confirm that before sending
> > patch, I check it with checkPatch.py every time.Thank you very much
> > for your help
>
> Please read Documentation/SubmittingPatches which has a link to the
> checklist and other documentation you should read.
>
> And I doubt you are running checkpatch on your submission, as there is
> obvious coding style issues in it. If so, please provide the output
> as it must be broken :(
>
> thanks,
>
> greg k-h
hi, the patch v11 checkpatch.pl output is below:
carlis@bf-rmsz-10:~/work/linux-kernel/linux$ ./scripts/checkpatch.pl
0001-staging-fbtft-add-tearing-signal-detect.patch total: 0 errors, 0
warnings, 0 checks, 176 lines checked
0001-staging-fbtft-add-tearing-signal-detect.patch has no obvious style
problems and is ready for submission.
regards
zhangxuezhi
On Wed, Jan 27, 2021 at 10:17:08PM +0800, carlis wrote:
> On Wed, 27 Jan 2021 15:13:05 +0100
> Greg KH <[email protected]> wrote:
>
> > On Wed, Jan 27, 2021 at 10:08:09PM +0800, carlis wrote:
> > > On Wed, 27 Jan 2021 14:51:55 +0100
> > > Greg KH <[email protected]> wrote:
> > >
> > > > On Wed, Jan 27, 2021 at 09:42:52PM +0800, Carlis wrote:
> > > > > From: zhangxuezhi <[email protected]>
> > > > >
> > > > > For st7789v ic,when we need continuous full screen refresh, it
> > > > > is best to wait for the TE signal arrive to avoid screen tearing
> > > > >
> > > > > Signed-off-by: zhangxuezhi <[email protected]>
> > > >
> > > > Please slow down and wait at least a day between patch
> > > > submissions, there is no rush here.
> > > >
> > > > And also, ALWAYS run scripts/checkpatch.pl on your submissions, so
> > > > that you don't have a maintainer asking you about basic problems,
> > > > like are in this current patch :(
> > > >
> > > > thanks,
> > > >
> > > > greg k-h
> > >
> > > hi,
> > > This is my first patch contribution to Linux, so some of the rules
> > > are not very clear .In addition, I can confirm that before sending
> > > patch, I check it with checkPatch.py every time.Thank you very much
> > > for your help
> >
> > Please read Documentation/SubmittingPatches which has a link to the
> > checklist and other documentation you should read.
> >
> > And I doubt you are running checkpatch on your submission, as there is
> > obvious coding style issues in it. If so, please provide the output
> > as it must be broken :(
> >
> > thanks,
> >
> > greg k-h
> hi, the patch v11 checkpatch.pl output is below:
>
> carlis@bf-rmsz-10:~/work/linux-kernel/linux$ ./scripts/checkpatch.pl
> 0001-staging-fbtft-add-tearing-signal-detect.patch total: 0 errors, 0
> warnings, 0 checks, 176 lines checked
>
> 0001-staging-fbtft-add-tearing-signal-detect.patch has no obvious style
> problems and is ready for submission.
Wow, my apologies!
Andy and Joe, there's something wrong here that is missing the fact that
a line is being indented with spaces and not tabs in the patch
at https://lore.kernel.org/r/[email protected]
Any ideas what broke?
thanks,
greg k-h
On Wed, Jan 27, 2021 at 03:25:20PM +0100, Greg KH wrote:
> On Wed, Jan 27, 2021 at 10:17:08PM +0800, carlis wrote:
> > On Wed, 27 Jan 2021 15:13:05 +0100
> > Greg KH <[email protected]> wrote:
> >
> > > On Wed, Jan 27, 2021 at 10:08:09PM +0800, carlis wrote:
> > > > On Wed, 27 Jan 2021 14:51:55 +0100
> > > > Greg KH <[email protected]> wrote:
> > > >
> > > > > On Wed, Jan 27, 2021 at 09:42:52PM +0800, Carlis wrote:
> > > > > > From: zhangxuezhi <[email protected]>
> > > > > >
> > > > > > For st7789v ic,when we need continuous full screen refresh, it
> > > > > > is best to wait for the TE signal arrive to avoid screen tearing
> > > > > >
> > > > > > Signed-off-by: zhangxuezhi <[email protected]>
> > > > >
> > > > > Please slow down and wait at least a day between patch
> > > > > submissions, there is no rush here.
> > > > >
> > > > > And also, ALWAYS run scripts/checkpatch.pl on your submissions, so
> > > > > that you don't have a maintainer asking you about basic problems,
> > > > > like are in this current patch :(
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h
> > > >
> > > > hi,
> > > > This is my first patch contribution to Linux, so some of the rules
> > > > are not very clear .In addition, I can confirm that before sending
> > > > patch, I check it with checkPatch.py every time.Thank you very much
> > > > for your help
> > >
> > > Please read Documentation/SubmittingPatches which has a link to the
> > > checklist and other documentation you should read.
> > >
> > > And I doubt you are running checkpatch on your submission, as there is
> > > obvious coding style issues in it. If so, please provide the output
> > > as it must be broken :(
> > >
> > > thanks,
> > >
> > > greg k-h
> > hi, the patch v11 checkpatch.pl output is below:
> >
> > carlis@bf-rmsz-10:~/work/linux-kernel/linux$ ./scripts/checkpatch.pl
> > 0001-staging-fbtft-add-tearing-signal-detect.patch total: 0 errors, 0
> > warnings, 0 checks, 176 lines checked
> >
> > 0001-staging-fbtft-add-tearing-signal-detect.patch has no obvious style
> > problems and is ready for submission.
>
> Wow, my apologies!
>
> Andy and Joe, there's something wrong here that is missing the fact that
> a line is being indented with spaces and not tabs in the patch
> at https://lore.kernel.org/r/[email protected]
>
> Any ideas what broke?
>
/*Tearing Effect Line On*/
Comments are the exception to the "no spaces at the start of a line"
rule. I was expecting that the kbuild-bot would send a Smatch warning
for inconsistent indenting, but comments are not counted there either.
I'm sort of surprised that we don't have checkpatch rule about the
missing space characters. It should be: "/* Tearing Effect Line On */".
regards,
dan carpenter
On Wed, Jan 27, 2021 at 05:49:46PM +0300, Dan Carpenter wrote:
> On Wed, Jan 27, 2021 at 03:25:20PM +0100, Greg KH wrote:
> > On Wed, Jan 27, 2021 at 10:17:08PM +0800, carlis wrote:
> > > On Wed, 27 Jan 2021 15:13:05 +0100
> > > Greg KH <[email protected]> wrote:
> > >
> > > > On Wed, Jan 27, 2021 at 10:08:09PM +0800, carlis wrote:
> > > > > On Wed, 27 Jan 2021 14:51:55 +0100
> > > > > Greg KH <[email protected]> wrote:
> > > > >
> > > > > > On Wed, Jan 27, 2021 at 09:42:52PM +0800, Carlis wrote:
> > > > > > > From: zhangxuezhi <[email protected]>
> > > > > > >
> > > > > > > For st7789v ic,when we need continuous full screen refresh, it
> > > > > > > is best to wait for the TE signal arrive to avoid screen tearing
> > > > > > >
> > > > > > > Signed-off-by: zhangxuezhi <[email protected]>
> > > > > >
> > > > > > Please slow down and wait at least a day between patch
> > > > > > submissions, there is no rush here.
> > > > > >
> > > > > > And also, ALWAYS run scripts/checkpatch.pl on your submissions, so
> > > > > > that you don't have a maintainer asking you about basic problems,
> > > > > > like are in this current patch :(
> > > > > >
> > > > > > thanks,
> > > > > >
> > > > > > greg k-h
> > > > >
> > > > > hi,
> > > > > This is my first patch contribution to Linux, so some of the rules
> > > > > are not very clear .In addition, I can confirm that before sending
> > > > > patch, I check it with checkPatch.py every time.Thank you very much
> > > > > for your help
> > > >
> > > > Please read Documentation/SubmittingPatches which has a link to the
> > > > checklist and other documentation you should read.
> > > >
> > > > And I doubt you are running checkpatch on your submission, as there is
> > > > obvious coding style issues in it. If so, please provide the output
> > > > as it must be broken :(
> > > >
> > > > thanks,
> > > >
> > > > greg k-h
> > > hi, the patch v11 checkpatch.pl output is below:
> > >
> > > carlis@bf-rmsz-10:~/work/linux-kernel/linux$ ./scripts/checkpatch.pl
> > > 0001-staging-fbtft-add-tearing-signal-detect.patch total: 0 errors, 0
> > > warnings, 0 checks, 176 lines checked
> > >
> > > 0001-staging-fbtft-add-tearing-signal-detect.patch has no obvious style
> > > problems and is ready for submission.
> >
> > Wow, my apologies!
> >
> > Andy and Joe, there's something wrong here that is missing the fact that
> > a line is being indented with spaces and not tabs in the patch
> > at https://lore.kernel.org/r/[email protected]
> >
> > Any ideas what broke?
> >
>
> /*Tearing Effect Line On*/
>
> Comments are the exception to the "no spaces at the start of a line"
> rule. I was expecting that the kbuild-bot would send a Smatch warning
> for inconsistent indenting, but comments are not counted there either.
>
> I'm sort of surprised that we don't have checkpatch rule about the
> missing space characters. It should be: "/* Tearing Effect Line On */".
That was going to be my next question, lots of comments added in this
patch don't have spaces...
thanks,
greg k-h
On Wed, 2021-01-27 at 17:49 +0300, Dan Carpenter wrote:
> On Wed, Jan 27, 2021 at 03:25:20PM +0100, Greg KH wrote:
> > Andy and Joe, there's something wrong here that is missing the fact that
> > a line is being indented with spaces and not tabs in the patch
> > at https://lore.kernel.org/r/[email protected]
> >
> > Any ideas what broke?
>
> ????/*Tearing Effect Line On*/
>
> Comments are the exception to the "no spaces at the start of a line"
> rule. I was expecting that the kbuild-bot would send a Smatch warning
> for inconsistent indenting, but comments are not counted there either.
>
> I'm sort of surprised that we don't have checkpatch rule about the
> missing space characters. It should be: "/* Tearing Effect Line On */".
You could always write your own rule...
checkpatch doesn't care if a comment looks like
/********************/
or
/*foobarfoobarfoobar*/
> Comments are the exception to the "no spaces at the start of a line"
> rule. I was expecting that the kbuild-bot would send a Smatch warning
> for inconsistent indenting, but comments are not counted there either.
>
> I'm sort of surprised that we don't have checkpatch rule about the
> missing space characters. It should be: "/* Tearing Effect Line On */".
Maybe this but the "preceded by a tab" test is pretty noisy.
---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4f8494527139..72347e82d384 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3720,6 +3720,22 @@ sub process {
s/(\(\s*$Type\s*\))[ \t]+/$1/;
}
}
+
+# Comment styles
+# Initial comment only lines that have a leading space
+ if ($rawline =~ m{^\+([ \t]+)(?:/\*|//)} && $1 =~ / /) {
+ WARN("COMMENT_STYLE",
+ "Initial comment lines should be indented only with tabs\n" . $herecurr);
+# comments not aligned on tabs
+ } elsif ($rawline !~ m{^\+(?:/\*|//)} &&
+ $rawline =~ m{^\+.*[^\t](?:/\*|//)}) {
+ CHK("COMMENT_STYLE",
+ "Comments should generally be preceded by a tab\n" . $herecurr);
+ }
+
+# comment initiators should generally be followed by a space if using words
+ if ($rawline =~ m{^\+.*(?:/\*|//)\w}) {
+ WARN("COMMENT_STYLE",
+ "Comment text should use a space after the comment initiator\n" . $herecurr);
+ }
# Block comment styles
# Networking with an initial /*
On Wed, 27 Jan 2021 16:02:35 +0100
Greg KH <[email protected]> wrote:
> On Wed, Jan 27, 2021 at 05:49:46PM +0300, Dan Carpenter wrote:
> > On Wed, Jan 27, 2021 at 03:25:20PM +0100, Greg KH wrote:
> > > On Wed, Jan 27, 2021 at 10:17:08PM +0800, carlis wrote:
> > > > On Wed, 27 Jan 2021 15:13:05 +0100
> > > > Greg KH <[email protected]> wrote:
> > > >
> > > > > On Wed, Jan 27, 2021 at 10:08:09PM +0800, carlis wrote:
> > > > > > On Wed, 27 Jan 2021 14:51:55 +0100
> > > > > > Greg KH <[email protected]> wrote:
> > > > > >
> > > > > > > On Wed, Jan 27, 2021 at 09:42:52PM +0800, Carlis wrote:
> > > > > > >
> > > > > > > > From: zhangxuezhi <[email protected]>
> > > > > > > >
> > > > > > > > For st7789v ic,when we need continuous full screen
> > > > > > > > refresh, it is best to wait for the TE signal arrive to
> > > > > > > > avoid screen tearing
> > > > > > > >
> > > > > > > > Signed-off-by: zhangxuezhi <[email protected]>
> > > > > > > >
> > > > > > >
> > > > > > > Please slow down and wait at least a day between patch
> > > > > > > submissions, there is no rush here.
> > > > > > >
> > > > > > > And also, ALWAYS run scripts/checkpatch.pl on your
> > > > > > > submissions, so that you don't have a maintainer asking
> > > > > > > you about basic problems, like are in this current patch
> > > > > > > :(
> > > > > > >
> > > > > > > thanks,
> > > > > > >
> > > > > > > greg k-h
> > > > > >
> > > > > > hi,
> > > > > > This is my first patch contribution to Linux, so some of
> > > > > > the rules are not very clear .In addition, I can confirm
> > > > > > that before sending patch, I check it with checkPatch.py
> > > > > > every time.Thank you very much for your help
> > > > >
> > > > > Please read Documentation/SubmittingPatches which has a link
> > > > > to the checklist and other documentation you should read.
> > > > >
> > > > > And I doubt you are running checkpatch on your submission, as
> > > > > there is obvious coding style issues in it. If so, please
> > > > > provide the output as it must be broken :(
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h
> > > > hi, the patch v11 checkpatch.pl output is below:
> > > >
> > > > carlis@bf-rmsz-10:~/work/linux-kernel/linux$
> > > > ./scripts/checkpatch.pl
> > > > 0001-staging-fbtft-add-tearing-signal-detect.patch total: 0
> > > > errors, 0 warnings, 0 checks, 176 lines checked
> > > >
> > > > 0001-staging-fbtft-add-tearing-signal-detect.patch has no
> > > > obvious style problems and is ready for submission.
> > >
> > > Wow, my apologies!
> > >
> > > Andy and Joe, there's something wrong here that is missing the
> > > fact that a line is being indented with spaces and not tabs in
> > > the patch at
> > > https://lore.kernel.org/r/[email protected]
> > >
> > > Any ideas what broke?
> > >
> >
> > /*Tearing Effect Line On*/
> >
> > Comments are the exception to the "no spaces at the start of a line"
> > rule. I was expecting that the kbuild-bot would send a Smatch
> > warning for inconsistent indenting, but comments are not counted
> > there either.
> >
> > I'm sort of surprised that we don't have checkpatch rule about the
> > missing space characters. It should be: "/* Tearing Effect Line On
> > */".
>
> That was going to be my next question, lots of comments added in this
> patch don't have spaces...
>
> thanks,
>
> greg k-h
Ok,i will fix it in patch v12 tomorrow
regards,
zhangxuezhi