[PATCH] docs: fb: Remove framebuffer scrollback option for boot

[PATCH] docs: fb: Remove framebuffer scrollback option for boot

[Bhaskar Chowdhury]

This patch remove the scrollback option under boot options.
Plus readjust the numbers for the options in that section.

Commit 973c096f6a85(vgacon: remove software scrollback support)
Commit 50145474f6ef(fbcon: remove soft scrollback code)

Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
---
 Documentation/fb/fbcon.rst | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/Documentation/fb/fbcon.rst b/Documentation/fb/fbcon.rst
index e57a3d1d085a..328f6980698c 100644
--- a/Documentation/fb/fbcon.rst
+++ b/Documentation/fb/fbcon.rst
@@ -87,15 +87,8 @@ C. Boot options
 	Note, not all drivers can handle font with widths not divisible by 8,
 	such as vga16fb.
 
-2. fbcon=scrollback:<value>[k]
 
-	The scrollback buffer is memory that is used to preserve display
-	contents that has already scrolled past your view.  This is accessed
-	by using the Shift-PageUp key combination.  The value 'value' is any
-	integer. It defaults to 32KB.  The 'k' suffix is optional, and will
-	multiply the 'value' by 1024.
-
-3. fbcon=map:<0123>
+2. fbcon=map:<0123>
 
 	This is an interesting option. It tells which driver gets mapped to
 	which console. The value '0123' is a sequence that gets repeated until
@@ -116,7 +109,7 @@ C. Boot options
 	Later on, when you want to map the console the to the framebuffer
 	device, you can use the con2fbmap utility.
 
-4. fbcon=vc:<n1>-<n2>
+3. fbcon=vc:<n1>-<n2>
 
 	This option tells fbcon to take over only a range of consoles as
 	specified by the values 'n1' and 'n2'. The rest of the consoles
@@ -127,7 +120,7 @@ C. Boot options
 	is typically located on the same video card.  Thus, the consoles that
 	are controlled by the VGA console will be garbled.
 
-5. fbcon=rotate:<n>
+4. fbcon=rotate:<n>
 
 	This option changes the orientation angle of the console display. The
 	value 'n' accepts the following:
@@ -152,21 +145,21 @@ C. Boot options
 	Actually, the underlying fb driver is totally ignorant of console
 	rotation.
 
-6. fbcon=margin:<color>
+5. fbcon=margin:<color>
 
 	This option specifies the color of the margins. The margins are the
 	leftover area at the right and the bottom of the screen that are not
 	used by text. By default, this area will be black. The 'color' value
 	is an integer number that depends on the framebuffer driver being used.
 
-7. fbcon=nodefer
+6. fbcon=nodefer
 
 	If the kernel is compiled with deferred fbcon takeover support, normally
 	the framebuffer contents, left in place by the firmware/bootloader, will
 	be preserved until there actually is some text is output to the console.
 	This option causes fbcon to bind immediately to the fbdev device.
 
-8. fbcon=logo-pos:<location>
+7. fbcon=logo-pos:<location>
 
 	The only possible 'location' is 'center' (without quotes), and when
 	given, the bootup logo is moved from the default top-left corner
@@ -174,7 +167,7 @@ C. Boot options
 	displayed due to multiple CPUs, the collected line of logos is moved
 	as a whole.
 
-9. fbcon=logo-count:<n>
+8. fbcon=logo-count:<n>
 
 	The value 'n' overrides the number of bootup logos. 0 disables the
 	logo, and -1 gives the default which is the number of online CPUs.
-- 
2.26.2

Re: [PATCH] docs: fb: Remove framebuffer scrollback option for boot

[Willy Tarreau]

Bhaskar,

your patches still all use very similar subjects and commit messages
which are pretty confusing as they only differ by words unrelated to
their real differences. It is important that the commit messages help
the reader guess what is being touched, so if you're splitting your
work into multiple patches, you need to indicate the difference in
each message. What I can propose to make things clearer:

    docs: fb: Remove framebuffer scrollback boot option
    docs: fb: Remove matroxfb scrollback boot option
    docs: fb: Remove sstfb scrollback boot option
    docs: fb: Remove vesafb scrollback boot option

Alternately they can all be merged into the first one under the same
name, but then the detailed commit message should specifically list
them.

In addition below:

On Wed, Sep 16, 2020 at 03:55:11AM +0530, Bhaskar Chowdhury wrote:
> This patch remove the scrollback option under boot options.
> Plus readjust the numbers for the options in that section.
> 
> Commit 973c096f6a85(vgacon: remove software scrollback support)
> Commit 50145474f6ef(fbcon: remove soft scrollback code)

This is still not clear. The message should indicate the "why" more
than the "what" which can be figured from the patch. In addition,
only the fbcon commit is a cause for these changes. Last, Greg
mentioned that the format is 'commit xxx ("subject")'.

What about this:

  The "scrollback" boot option was removed by commit 50145474f6ef
  ("fbcon: remove soft scrollback code"), but the doc for fbcon was
  not updated.  This patch updates the fbcon doc and renumbers the
  sections.

If you merge all your patches together, you can have this:

  The "scrollback" boot option was removed by commit 50145474f6ef
  ("fbcon: remove soft scrollback code"), but the fb docs were not
  updated.  This patch removes reference to this option in the fbcon,
  matroxfb, sstfb and vesafb docs and renumbers the sections as needed.

And please increase your version so that it's more obvious that this
replaces previous series. Call it v3 or v4 or whatever higher than
the highest you've ever sent so that it's easier for your readers to
ignore the older ones. Ideally after your signed-off-by you should
add a "---" line with a quick changelog indicating what changed from
the previous ones (just for reviewers, this will not be merged), for
example:

   Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
   ---
   v3: clarify message description, update all fb drivers in the same patch
   v2: reword commit message

Hoping this helps,
Willy

Re: [PATCH] docs: fb: Remove framebuffer scrollback option for boot

[Bhaskar Chowdhury]

[-- Attachment #1: Type: text/plain, Size: 3814 bytes --]

On 04:25 Wed 16 Sep 2020, Willy Tarreau wrote:
Thank you so much Willy. I am trying to get the essence of your mail ...few
responses below..

>Bhaskar,
>
>your patches still all use very similar subjects and commit messages
>which are pretty confusing as they only differ by words unrelated to
>their real differences. It is important that the commit messages help
>the reader guess what is being touched, so if you're splitting your
>work into multiple patches, you need to indicate the difference in
>each message. What I can propose to make things clearer:
>
>    docs: fb: Remove framebuffer scrollback boot option
>    docs: fb: Remove matroxfb scrollback boot option
>    docs: fb: Remove sstfb scrollback boot option
>    docs: fb: Remove vesafb scrollback boot option
>
  I would prefer to stick with this model , what you suggested with different
  subject and more relevant text.

>Alternately they can all be merged into the first one under the same
>name, but then the detailed commit message should specifically list
>them.
>
>In addition below:
>
>On Wed, Sep 16, 2020 at 03:55:11AM +0530, Bhaskar Chowdhury wrote:
>> This patch remove the scrollback option under boot options.
>> Plus readjust the numbers for the options in that section.
>> 
>> Commit 973c096f6a85(vgacon: remove software scrollback support)
>> Commit 50145474f6ef(fbcon: remove soft scrollback code)
>
>This is still not clear. The message should indicate the "why" more
>than the "what" which can be figured from the patch. In addition,
>only the fbcon commit is a cause for these changes. Last, Greg
>mentioned that the format is 'commit xxx ("subject")'.
>
>What about this:
>
>  The "scrollback" boot option was removed by commit 50145474f6ef
>  ("fbcon: remove soft scrollback code"), but the doc for fbcon was
>  not updated.  This patch updates the fbcon doc and renumbers the
>  sections.
>

I lost it ...by head failed me ...do you want me copy more information text
from the actual commit??

>If you merge all your patches together, you can have this:
>
>  The "scrollback" boot option was removed by commit 50145474f6ef
>  ("fbcon: remove soft scrollback code"), but the fb docs were not
>  updated.  This patch removes reference to this option in the fbcon,
>  matroxfb, sstfb and vesafb docs and renumbers the sections as needed.
>
>And please increase your version so that it's more obvious that this
>replaces previous series. Call it v3 or v4 or whatever higher than
>the highest you've ever sent so that it's easier for your readers to
>ignore the older ones. Ideally after your signed-off-by you should
>add a "---" line with a quick changelog indicating what changed from
>the previous ones (just for reviewers, this will not be merged), for
>example:
>
Okay, before sending this new set ...yesterday I wrote a mail to everyone ,
that I wanted to get rid of this damn versioning thing and will sent patches
afresh. So, ask everyone involved please ignore what have sent previously and
that mail too. I am sure that missed your eyes ...and I understand why :)

But I am still reluctant to bump up the version number and would like to sent
all the 4 patches AFRESH(which has nothing to do with the previous patchtes I
have sent) .

Can I do that Willy??? 


>   Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
>   ---
>   v3: clarify message description, update all fb drivers in the same patch
>   v2: reword commit message
>
This chainlog of information in the patch getting spirial and not helpful
AFAIK. I mean, the version bumping and provide information appended to every
other following mail. If that is significant then I understand ...but for this thing....I
hope you get me...

>Hoping this helps,
>Willy

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] docs: fb: Remove framebuffer scrollback option for boot

[Willy Tarreau]

On Wed, Sep 16, 2020 at 08:34:16AM +0530, Bhaskar Chowdhury wrote:
> > In addition below:
> > 
> > On Wed, Sep 16, 2020 at 03:55:11AM +0530, Bhaskar Chowdhury wrote:
> > > This patch remove the scrollback option under boot options.
> > > Plus readjust the numbers for the options in that section.
> > > 
> > > Commit 973c096f6a85(vgacon: remove software scrollback support)
> > > Commit 50145474f6ef(fbcon: remove soft scrollback code)
> > 
> > This is still not clear. The message should indicate the "why" more
> > than the "what" which can be figured from the patch. In addition,
> > only the fbcon commit is a cause for these changes. Last, Greg
> > mentioned that the format is 'commit xxx ("subject")'.
> > 
> > What about this:
> > 
> >  The "scrollback" boot option was removed by commit 50145474f6ef
> >  ("fbcon: remove soft scrollback code"), but the doc for fbcon was
> >  not updated.  This patch updates the fbcon doc and renumbers the
> >  sections.
> > 
> 
> I lost it ...by head failed me ...do you want me copy more information text
> from the actual commit??

No, it's just that your commit messages are not clear at all. You
mention that you remove something and point another commit without
any explanation about the link between them. I'm just proposing an
example of how to better explain the justification for your patch.

As a rule of thumb, when you propose a patch for inclusion, always
think that you're trying to sell it to someone else who will have
to take care of its consequences forever. So you must put all the
good arguments on it, just as if it was printed on the package of a
product. Of course here it's just a tiny doc patch but the principle
remains, the why and how still needs to be clear.

> > If you merge all your patches together, you can have this:
> > 
> >  The "scrollback" boot option was removed by commit 50145474f6ef
> >  ("fbcon: remove soft scrollback code"), but the fb docs were not
> >  updated.  This patch removes reference to this option in the fbcon,
> >  matroxfb, sstfb and vesafb docs and renumbers the sections as needed.
> > 
> > And please increase your version so that it's more obvious that this
> > replaces previous series. Call it v3 or v4 or whatever higher than
> > the highest you've ever sent so that it's easier for your readers to
> > ignore the older ones. Ideally after your signed-off-by you should
> > add a "---" line with a quick changelog indicating what changed from
> > the previous ones (just for reviewers, this will not be merged), for
> > example:
> > 
> Okay, before sending this new set ...yesterday I wrote a mail to everyone ,
> that I wanted to get rid of this damn versioning thing and will sent patches
> afresh. So, ask everyone involved please ignore what have sent previously and
> that mail too. I am sure that missed your eyes ...and I understand why :)
> 
> But I am still reluctant to bump up the version number and would like to sent
> all the 4 patches AFRESH(which has nothing to do with the previous patchtes I
> have sent) .

Who cares ? Is a version expensive in any way ? Is there any shame in
sending multiple versions ? Put yourself in the place of your recipients.
They receive 500-2000 emails a day, get yours possibly in a random order
if their connection is a bit flaky, and are not always perfectly sure
which ones are still relevant after some comments were emitted by others.
The version is just a simple way to say "ignore everything I sent before
this one". If you send a v3, it will be obvious that anything without any
version or marked v2 can safely be ignored.

In addition you say that you send a fresh series, but even that series was
very confusing, with patches having almost the same names more or less one
word ("stanza" vs "line") so it was not clear whether each patch was a new
attempt at fixing a typo in the previous one or for something different. A
version number clarifies that. And numbering the patches as well by the way.

> Can I do that Willy???

You can do whatever you want. We all find the rules annoying when we have
to follow them but convenient from the recipient side. Thus it's just a
matter of whether or not you want to increase the chances that your patches
are caught by someone and merged. The more confusing they are, the less
likely someone will be willing to spend valuable time on them, and once
they're out of the visible window they're forgotten. I'd suggest that you
usually go the easy way first and once you get some reviews you need to
be careful to address them completely and accurately (or indicate your
disapproval and why).

> >   Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
> >   ---
> >   v3: clarify message description, update all fb drivers in the same patch
> >   v2: reword commit message
> > 
> This chainlog of information in the patch getting spirial and not helpful
> AFAIK. I mean, the version bumping and provide information appended to every
> other following mail. If that is significant then I understand ...but for this thing....I
> hope you get me...

I'd say that for that trivial a patch it's probably not very useful,
just like it can be dropped in a fast round trip with a person on a
short topic. But when you get some reviews, reviewers never know if
you wrote the new series before, during or after their review, and not
knowing if you took their comments into consideration either makes
them hesitant to review again, or irritated seeing that they were not
considered yet. Just for this it's a good practice to mention what
comments were addressed here. It's just an Ack for the reviewers to
say "I tried to follow your advice, if it's still wrong, let me know".
For example Greg made some very precise comments about the format he
expected for the commit IDs, that apparently you still got wrong several
times. How can he know if you tried to address that and failed, or just
read it after sending ?

Willy

Re: [PATCH] docs: fb: Remove framebuffer scrollback option for boot

[Bhaskar Chowdhury]

[-- Attachment #1: Type: text/plain, Size: 6876 bytes --]

On 06:44 Wed 16 Sep 2020, Willy Tarreau wrote:
>On Wed, Sep 16, 2020 at 08:34:16AM +0530, Bhaskar Chowdhury wrote:
>> > In addition below:
>> > 
>> > On Wed, Sep 16, 2020 at 03:55:11AM +0530, Bhaskar Chowdhury wrote:
>> > > This patch remove the scrollback option under boot options.
>> > > Plus readjust the numbers for the options in that section.
>> > > 
>> > > Commit 973c096f6a85(vgacon: remove software scrollback support)
>> > > Commit 50145474f6ef(fbcon: remove soft scrollback code)
>> > 
>> > This is still not clear. The message should indicate the "why" more
>> > than the "what" which can be figured from the patch. In addition,
>> > only the fbcon commit is a cause for these changes. Last, Greg
>> > mentioned that the format is 'commit xxx ("subject")'.
>> > 
>> > What about this:
>> > 
>> >  The "scrollback" boot option was removed by commit 50145474f6ef
>> >  ("fbcon: remove soft scrollback code"), but the doc for fbcon was
>> >  not updated.  This patch updates the fbcon doc and renumbers the
>> >  sections.
>> > 
>> 
>> I lost it ...by head failed me ...do you want me copy more information text
>> from the actual commit??
>
>No, it's just that your commit messages are not clear at all. You
>mention that you remove something and point another commit without
>any explanation about the link between them. I'm just proposing an
>example of how to better explain the justification for your patch.
>
>As a rule of thumb, when you propose a patch for inclusion, always
>think that you're trying to sell it to someone else who will have
>to take care of its consequences forever. So you must put all the
>good arguments on it, just as if it was printed on the package of a
>product. Of course here it's just a tiny doc patch but the principle
>remains, the why and how still needs to be clear.
>
>> > If you merge all your patches together, you can have this:
>> > 
>> >  The "scrollback" boot option was removed by commit 50145474f6ef
>> >  ("fbcon: remove soft scrollback code"), but the fb docs were not
>> >  updated.  This patch removes reference to this option in the fbcon,
>> >  matroxfb, sstfb and vesafb docs and renumbers the sections as needed.
>> > 
>> > And please increase your version so that it's more obvious that this
>> > replaces previous series. Call it v3 or v4 or whatever higher than
>> > the highest you've ever sent so that it's easier for your readers to
>> > ignore the older ones. Ideally after your signed-off-by you should
>> > add a "---" line with a quick changelog indicating what changed from
>> > the previous ones (just for reviewers, this will not be merged), for
>> > example:
>> > 
>> Okay, before sending this new set ...yesterday I wrote a mail to everyone ,
>> that I wanted to get rid of this damn versioning thing and will sent patches
>> afresh. So, ask everyone involved please ignore what have sent previously and
>> that mail too. I am sure that missed your eyes ...and I understand why :)
>> 
>> But I am still reluctant to bump up the version number and would like to sent
>> all the 4 patches AFRESH(which has nothing to do with the previous patchtes I
>> have sent) .
>
>Who cares ? Is a version expensive in any way ? Is there any shame in
>sending multiple versions ? Put yourself in the place of your recipients.
>They receive 500-2000 emails a day, get yours possibly in a random order
>if their connection is a bit flaky, and are not always perfectly sure
>which ones are still relevant after some comments were emitted by others.
>The version is just a simple way to say "ignore everything I sent before
>this one". If you send a v3, it will be obvious that anything without any
>version or marked v2 can safely be ignored.
>
>In addition you say that you send a fresh series, but even that series was
>very confusing, with patches having almost the same names more or less one
>word ("stanza" vs "line") so it was not clear whether each patch was a new
>attempt at fixing a typo in the previous one or for something different. A
>version number clarifies that. And numbering the patches as well by the way.
>
>> Can I do that Willy???
>
>You can do whatever you want. We all find the rules annoying when we have
>to follow them but convenient from the recipient side. Thus it's just a
>matter of whether or not you want to increase the chances that your patches
>are caught by someone and merged. The more confusing they are, the less
>likely someone will be willing to spend valuable time on them, and once
>they're out of the visible window they're forgotten. I'd suggest that you
>usually go the easy way first and once you get some reviews you need to
>be careful to address them completely and accurately (or indicate your
>disapproval and why).
>
>> >   Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
>> >   ---
>> >   v3: clarify message description, update all fb drivers in the same patch
>> >   v2: reword commit message
>> > 
>> This chainlog of information in the patch getting spirial and not helpful
>> AFAIK. I mean, the version bumping and provide information appended to every
>> other following mail. If that is significant then I understand ...but for this thing....I
>> hope you get me...
>
>I'd say that for that trivial a patch it's probably not very useful,
>just like it can be dropped in a fast round trip with a person on a
>short topic. But when you get some reviews, reviewers never know if
>you wrote the new series before, during or after their review, and not
>knowing if you took their comments into consideration either makes
>them hesitant to review again, or irritated seeing that they were not
>considered yet. Just for this it's a good practice to mention what
>comments were addressed here. It's just an Ack for the reviewers to
>say "I tried to follow your advice, if it's still wrong, let me know".
>For example Greg made some very precise comments about the format he
>expected for the commit IDs, that apparently you still got wrong several
>times. How can he know if you tried to address that and failed, or just
>read it after sending ?
>
>Willy


Hmmm ....okay...every words your wrote is absolutely right and that is how it
is been practicing here in this project. 

I am not sure why the hell I deviated from that rule or practice.

My sincere apologies to everyone ,whoever getting my mail for this specific
patch series.

Look like we need someone good at sending patches proper way. 

It seems, I am going to get into the "kill file" of lot of people ...meh ..heck

Keeping all the words in mind  Yours and Greg's(Please don't fret) ..well another
hoorah at the patch making and send of this series. And I sincerely don't know
this time who bothers about my sending ....I do hope ..

  
~Bhaskar 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]