U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] doc: develop/sending_patches: add tips for series
@ 2026-11-04 19:48 Ralph Siemsen
  2026-11-05 11:00 ` Quentin Schulz
  0 siblings, 1 reply; 6+ messages in thread
From: Ralph Siemsen @ 2026-11-04 19:48 UTC (permalink / raw)
  To: u-boot; +Cc: Tom Rini, Quentin Schulz, Neil Armstrong, Ralph Siemsen,
	E Shattow

Related patches can be grouped into a "patch series". Add some tips on
how to decide how much belongs into a "patch series".

Signed-off-by: Ralph Siemsen <ralph.siemsen@linaro.org>
Reviewed-by: E Shattow <e@freeshell.de>
--
Changes for v2:
- small wording changes to help non-native English speakers
- add R-b

Link to v1:
https://patch.msgid.link/20261028-doc-sending-patches-v1-1-66b0f1b276de@linaro.org

---
Signed-off-by: Ralph Siemsen <ralph.siemsen@linaro.org>
---
To: u-boot@lists.u-boot-project.org
Cc: Tom Rini <trini@konsulko.com>
Cc: Quentin Schulz <quentin.schulz@cherry.de>
Cc: Ralph Siemsen <ralph.siemsen@linaro.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
---
 doc/develop/sending_patches.rst | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/doc/develop/sending_patches.rst b/doc/develop/sending_patches.rst
index b4f82eb18e1..92bb2e203b8 100644
--- a/doc/develop/sending_patches.rst
+++ b/doc/develop/sending_patches.rst
@@ -184,6 +184,38 @@ General Patch Submission Rules
   cases, you did something wrong if your patch exceeds this limit. Think again
   if you should not split it into separate logical parts.
 
+Tips for creating patch series
+------------------------------
+
+A series of related patches can be sent as a "patch series", as described
+above. Individual patches should be small and self contained. Likewise,
+this also applies to each "patch series". Keep the following in mind:
+
+* large series are harder to review. Maintainers are busy and may postpone
+  large reviews, whereas smaller reviews are more easily accommodated.
+
+* tools like ``b4`` and ``patman`` will collect reviewers for each patch,
+  but then send the entire series to all reviewers found. This is not ideal
+  since it floods maintainers with patches they might not care about.
+
+A better approach is to break up a large series into smaller sub-series,
+for example by subsystem (clock, pinctrl, reset, etc.). This allows better
+targeting of only the relevant reviewers for each sub-series.
+
+Of course there are sometimes cases where a large patch series is okay:
+
+* when the changes are all in a specific area: for example when fixing a bug
+  in a driver, but also doing some refactoring or cleanup in the same code
+
+* when there are unavoidable dependencies between patches: keeping them
+  together avoids having to explain how multiple series are dependent
+
+In summary, the goal should be to break up changes into smaller pieces,
+so the change can be more easily understood by reviewers. Combining multiple
+patches into a series makes sense as a way of grouping related changes, but
+if this results in a very large number of reviewers (eg. 25 or more) then
+the series should probably be split into smaller sub pieces.
+
 Attributing Code, Copyrights, Signing
 -------------------------------------
 

---
base-commit: d1d45c926a571fb6c1e6d7105c280833646915ea
change-id: 20261028-doc-sending-patches-d3a464feff98

Best regards,
--  
Ralph Siemsen <ralph.siemsen@linaro.org>


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] doc: develop/sending_patches: add tips for series
  2026-11-04 19:48 [PATCH v2] doc: develop/sending_patches: add tips for series Ralph Siemsen
@ 2026-11-05 11:00 ` Quentin Schulz
  2026-11-05 14:33   ` Ralph Siemsen
  0 siblings, 1 reply; 6+ messages in thread
From: Quentin Schulz @ 2026-11-05 11:00 UTC (permalink / raw)
  To: Ralph Siemsen, u-boot; +Cc: Tom Rini, Neil Armstrong, E Shattow

Hi Ralph,

First, thanks for following up with the discussion we had on IRC, much 
appreciated!

On 11/4/26 8:48 PM, Ralph Siemsen wrote:
> Related patches can be grouped into a "patch series". Add some tips on
> how to decide how much belongs into a "patch series".
> 
> Signed-off-by: Ralph Siemsen <ralph.siemsen@linaro.org>
> Reviewed-by: E Shattow <e@freeshell.de>
> --
> Changes for v2:
> - small wording changes to help non-native English speakers
> - add R-b
> 
> Link to v1:
> https://patch.msgid.link/20261028-doc-sending-patches-v1-1-66b0f1b276de@linaro.org
> 
> ---
> Signed-off-by: Ralph Siemsen <ralph.siemsen@linaro.org>
> ---
> To: u-boot@lists.u-boot-project.org
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Quentin Schulz <quentin.schulz@cherry.de>
> Cc: Ralph Siemsen <ralph.siemsen@linaro.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>   doc/develop/sending_patches.rst | 32 ++++++++++++++++++++++++++++++++
>   1 file changed, 32 insertions(+)
> 
> diff --git a/doc/develop/sending_patches.rst b/doc/develop/sending_patches.rst
> index b4f82eb18e1..92bb2e203b8 100644
> --- a/doc/develop/sending_patches.rst
> +++ b/doc/develop/sending_patches.rst
> @@ -184,6 +184,38 @@ General Patch Submission Rules
>     cases, you did something wrong if your patch exceeds this limit. Think again
>     if you should not split it into separate logical parts.
>   
> +Tips for creating patch series
> +------------------------------
> +
> +A series of related patches can be sent as a "patch series", as described
> +above. Individual patches should be small and self contained. Likewise,
> +this also applies to each "patch series". Keep the following in mind:
> +
> +* large series are harder to review. Maintainers are busy and may postpone
> +  large reviews, whereas smaller reviews are more easily accommodated.
> +
> +* tools like ``b4`` and ``patman`` will collect reviewers for each patch,
> +  but then send the entire series to all reviewers found. This is not ideal
> +  since it floods maintainers with patches they might not care about.
> +
> +A better approach is to break up a large series into smaller sub-series,
> +for example by subsystem (clock, pinctrl, reset, etc.). This allows better
> +targeting of only the relevant reviewers for each sub-series.
> +
> +Of course there are sometimes cases where a large patch series is okay:
> +
> +* when the changes are all in a specific area: for example when fixing a bug
> +  in a driver, but also doing some refactoring or cleanup in the same code
> +

issue: This is typically the things we don't want to see I believe. 
Separate bugs from refactoring, cleanup, and new features. The argument 
is: we want the bug to be fixed as soon as possible, but we *reaaaaally* 
like to merge full series at once, so having this kind of changes mixed 
in a series means it's likely we'll either ask the contributor to split 
the series, or if the non-bug-fixing part of the series is controversial 
or not in a good state yet, unnecessarily postpone the merging of the 
bug fix.

> +* when there are unavoidable dependencies between patches: keeping them
> +  together avoids having to explain how multiple series are dependent
> +

thought: People usually struggle to detect the *actual* dependencies and 
are usually leaning too much on "I need those two drivers to be merged 
for the board to work" for example. We've seen this more recently with a 
full SoC bring-up being sent as one series. Yes, the SoC is unusable 
without all the minimally required drivers, but each individual driver 
can and should be sent a separate series. I'm not sure how we can word 
this better, maybe providing a real-life example would be nice. This can 
always be improved later on, so not a blocker.

With the issue in the previous paragraph fixed, I think this is a good 
improvement and we should merge as soon as possible.

Cheers,
Quentin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] doc: develop/sending_patches: add tips for series
  2026-11-05 11:00 ` Quentin Schulz
@ 2026-11-05 14:33   ` Ralph Siemsen
  2026-11-05 15:46     ` Quentin Schulz
  0 siblings, 1 reply; 6+ messages in thread
From: Ralph Siemsen @ 2026-11-05 14:33 UTC (permalink / raw)
  To: Quentin Schulz; +Cc: u-boot, Tom Rini, Neil Armstrong, E Shattow

On Thu, Nov 05, 2026 at 12:00:27PM +0100, Quentin Schulz wrote:
>
>On 11/4/26 8:48 PM, Ralph Siemsen wrote:
>>+Of course there are sometimes cases where a large patch series is 
>>okay:
>>+
>>+* when the changes are all in a specific area: for example when fixing a bug
>>+  in a driver, but also doing some refactoring or cleanup in the same code
>
>issue: This is typically the things we don't want to see I believe. 
>Separate bugs from refactoring, cleanup, and new features. The 
>argument is: we want the bug to be fixed as soon as possible, but we 
>*reaaaaally* like to merge full series at once, so having this kind of 
>changes mixed in a series means it's likely we'll either ask the 
>contributor to split the series, or if the non-bug-fixing part of the 
>series is controversial or not in a good state yet, unnecessarily 
>postpone the merging of the bug fix.

Maybe I should have said: "*directly related* refactoring or cleanup"?

Ideally a bug fix should be its own patch, and certainly it should not 
include any new features. But sometimes a bit of refactor/cleanup helps 
to make a bug fix easier to implement and/or understand.

>>+* when there are unavoidable dependencies between patches: keeping them
>>+  together avoids having to explain how multiple series are dependent
>
>thought: People usually struggle to detect the *actual* dependencies 
>and are usually leaning too much on "I need those two drivers to be 
>merged for the board to work" for example. We've seen this more 
>recently with a full SoC bring-up being sent as one series. Yes, the 
>SoC is unusable without all the minimally required drivers, but each 
>individual driver can and should be sent a separate series. I'm not 
>sure how we can word this better, maybe providing a real-life example 
>would be nice. This can always be improved later on, so not a blocker.

Having struggled with this myself, while I do agree with the idea of 
using separate series, it does complicate the development process. It 
means a branch for each series, and merging them together for each 
compile/test cycle. Only after doing this dozens of times did I realize 
that I could stack the branches on top of each other rather than merging 
each time. Then "git history fixup" and "git history reword" made it 
much easier to modify the stack. But all of this has to be learned and 
it is not immediately obvious.

I also agree that it is hard to detect the actual dependencies. And in 
fact they are not static: when a reviewer of one series requests 
changes, it can often impact other series. Which suggests that there is 
a dependency, even if it was not apparent initially.

>With the issue in the previous paragraph fixed, I think this is a good 
>improvement and we should merge as soon as possible.

Perhaps I should just drop the whole "Of course there are sometimes 
cases where a large patch series..." section?

Regards,
Ralph

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] doc: develop/sending_patches: add tips for series
  2026-11-05 14:33   ` Ralph Siemsen
@ 2026-11-05 15:46     ` Quentin Schulz
  2026-11-06 18:41       ` Ralph Siemsen
  0 siblings, 1 reply; 6+ messages in thread
From: Quentin Schulz @ 2026-11-05 15:46 UTC (permalink / raw)
  To: Ralph Siemsen; +Cc: u-boot, Tom Rini, Neil Armstrong, E Shattow

Hi Ralph,

On 11/5/26 3:33 PM, Ralph Siemsen wrote:
> On Thu, Nov 05, 2026 at 12:00:27PM +0100, Quentin Schulz wrote:
>>
>> On 11/4/26 8:48 PM, Ralph Siemsen wrote:
>>> +Of course there are sometimes cases where a large patch series is okay:
>>> +
>>> +* when the changes are all in a specific area: for example when 
>>> fixing a bug
>>> +  in a driver, but also doing some refactoring or cleanup in the 
>>> same code
>>
>> issue: This is typically the things we don't want to see I believe. 
>> Separate bugs from refactoring, cleanup, and new features. The 
>> argument is: we want the bug to be fixed as soon as possible, but we 
>> *reaaaaally* like to merge full series at once, so having this kind of 
>> changes mixed in a series means it's likely we'll either ask the 
>> contributor to split the series, or if the non-bug-fixing part of the 
>> series is controversial or not in a good state yet, unnecessarily 
>> postpone the merging of the bug fix.
> 
> Maybe I should have said: "*directly related* refactoring or cleanup"?
> 
> Ideally a bug fix should be its own patch, and certainly it should not 
> include any new features. But sometimes a bit of refactor/cleanup helps 
> to make a bug fix easier to implement and/or understand.
> 

Sometimes it's tempting to squeeze some refactoring that isn't actually 
necessary. I'm sure there are exceptions, but the more there is in 
context for a bug fix to be merged, the longer it'll take for the bug to 
be fixed, that's unfortunately the reality. We can strongly 
encourage/discourage people to do something, but there are of course 
always exceptions. Best judgment applies, but everybody has a different 
one and it gets trained over time :)

Something along the lines of "In the event a refactoring drastically 
simplifies bug fixing, it may be part of the same series, but it should 
be understood that this means the review will likely take much longer 
than if it was the bug fix alone in the series".

>>> +* when there are unavoidable dependencies between patches: keeping them
>>> +  together avoids having to explain how multiple series are dependent
>>
>> thought: People usually struggle to detect the *actual* dependencies 
>> and are usually leaning too much on "I need those two drivers to be 
>> merged for the board to work" for example. We've seen this more 
>> recently with a full SoC bring-up being sent as one series. Yes, the 
>> SoC is unusable without all the minimally required drivers, but each 
>> individual driver can and should be sent a separate series. I'm not 
>> sure how we can word this better, maybe providing a real-life example 
>> would be nice. This can always be improved later on, so not a blocker.
> 
> Having struggled with this myself, while I do agree with the idea of 
> using separate series, it does complicate the development process. It 
> means a branch for each series, and merging them together for each 
> compile/test cycle. Only after doing this dozens of times did I realize 
> that I could stack the branches on top of each other rather than merging 
> each time. Then "git history fixup" and "git history reword" made it 
> much easier to modify the stack. But all of this has to be learned and 
> it is not immediately obvious.
> 

There needs to be a compromise between friction for the contributor and 
friction for the maintainer/reviewer. Unfortunately, the balance is 
already so skewed in Open Source Software in general that more friction 
for the individual contributor than the reviewer is currently necessary 
for the maintainers/reviewers to barely avoid burning out. I 
unfortunately do not have an answer other than "we know this isn't super 
nice, but this is what we could come up with that makes merging your 
patches more likely and faster with the resources we have today".

> I also agree that it is hard to detect the actual dependencies. And in 
> fact they are not static: when a reviewer of one series requests 
> changes, it can often impact other series. Which suggests that there is 
> a dependency, even if it was not apparent initially.
> 

At some point, there's also "don't send too many series at once". If 
your series A requires big changes in series B, it either means it 
should be series A+B, or that series B was sent too early and we should 
focus on answering design questions in series A before having a look at 
series B. Sometimes patience pays off in the long run.

>> With the issue in the previous paragraph fixed, I think this is a good 
>> improvement and we should merge as soon as possible.
> 
> Perhaps I should just drop the whole "Of course there are sometimes 
> cases where a large patch series..." section?
> 

I like the second bullet point. It also gives a way out to people: we 
aren't super strict and sometimes we're fine with a series with 
"different subsystems but somewhat related". I prefer the patches to 
exist even if we don't review them quick than having the contributor 
agonize over what it is we exactly want. We can always tell them once 
their series is sent how we would have liked it to be formatted. This is 
something I've learned over the years by doing, the same with learning 
how to do bisectable changes, fine-grained commits and write commit logs 
that are (I hope) helpful to reviewers. I believe Tom's position in 
general is to leave enough wiggle room so people can figure out how 
things work best for them (e.g. we don't have super strict rules for 
custodians or merging, unlike the kernel for example).

Cheers,
Quentin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] doc: develop/sending_patches: add tips for series
  2026-11-05 15:46     ` Quentin Schulz
@ 2026-11-06 18:41       ` Ralph Siemsen
  2026-11-12 15:48         ` Quentin Schulz
  0 siblings, 1 reply; 6+ messages in thread
From: Ralph Siemsen @ 2026-11-06 18:41 UTC (permalink / raw)
  To: Quentin Schulz; +Cc: u-boot, Tom Rini, Neil Armstrong, E Shattow

On Thu, Nov 05, 2026 at 04:46:17PM +0100, Quentin Schulz wrote:
>Hi Ralph,
>
>On 11/5/26 3:33 PM, Ralph Siemsen wrote:
>>I also agree that it is hard to detect the actual dependencies. And in 
>>fact they are not static: when a reviewer of one series requests 
>>changes, it can often impact other series. Which suggests that there 
>>is a dependency, even if it was not apparent initially.
>>
>
>At some point, there's also "don't send too many series at once". If 
>your series A requires big changes in series B, it either means it 
>should be series A+B, or that series B was sent too early and we 
>should focus on answering design questions in series A before having a 
>look at series B. Sometimes patience pays off in the long run.

When adding a new SoC, we end up with series A+B and often many more. 
All of that code needs to be written in order to get the device to boot, 
so it is generally not possible to "upstream" any of it until it's all 
functional. (I don't like to send out untested code!)

The problem then becomes, which series to send first. Inevitably, in at 
least one of the series, a reviewer will suggest another approach, 
perhaps to extend some existing subsystem, or simply to do certain 
things a different way.

Unfortunately such suggestions frequently result in changes to the other 
series. These changes are hard to anticipate or plan for. Sometimes I 
try to send an early preview "RFC" to help identify such issues. But 
even then, during the actual merge, further changes are requested, once 
again with ripple effects on other series.

Sending out the series in a different order would not avoid this problem 
in general. Specifically because it is hard to know which parts will 
generate friction with reviewers. My guesses have been wrong more often 
than not in this regard.

Not sure there is anything we can really do about it, but I can totally 
understand why someone would send large "add new SoC xyz" patch series. 
It keeps everything together and the ripple effect of one change can be 
seen more clearly, than if all the pieces were done in separate series 
over a longer time period, with goalposts being moved along the way.

Anyhow, I've posted v3, with a focus on making it easier for the 
reviewers and the future code maintainer.

Regards,
Ralph

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] doc: develop/sending_patches: add tips for series
  2026-11-06 18:41       ` Ralph Siemsen
@ 2026-11-12 15:48         ` Quentin Schulz
  0 siblings, 0 replies; 6+ messages in thread
From: Quentin Schulz @ 2026-11-12 15:48 UTC (permalink / raw)
  To: Ralph Siemsen; +Cc: u-boot, Tom Rini, Neil Armstrong, E Shattow

Hi Ralph,

On 11/6/26 7:41 PM, Ralph Siemsen wrote:
> On Thu, Nov 05, 2026 at 04:46:17PM +0100, Quentin Schulz wrote:
>> Hi Ralph,
>>
>> On 11/5/26 3:33 PM, Ralph Siemsen wrote:
>>> I also agree that it is hard to detect the actual dependencies. And 
>>> in fact they are not static: when a reviewer of one series requests 
>>> changes, it can often impact other series. Which suggests that there 
>>> is a dependency, even if it was not apparent initially.
>>>
>>
>> At some point, there's also "don't send too many series at once". If 
>> your series A requires big changes in series B, it either means it 
>> should be series A+B, or that series B was sent too early and we 
>> should focus on answering design questions in series A before having a 
>> look at series B. Sometimes patience pays off in the long run.
> 
> When adding a new SoC, we end up with series A+B and often many more. 
> All of that code needs to be written in order to get the device to boot, 
> so it is generally not possible to "upstream" any of it until it's all 
> functional. (I don't like to send out untested code!)
> 
> The problem then becomes, which series to send first. Inevitably, in at 
> least one of the series, a reviewer will suggest another approach, 
> perhaps to extend some existing subsystem, or simply to do certain 
> things a different way.
> 
> Unfortunately such suggestions frequently result in changes to the other 
> series. These changes are hard to anticipate or plan for. Sometimes I 
> try to send an early preview "RFC" to help identify such issues. But 
> even then, during the actual merge, further changes are requested, once 
> again with ripple effects on other series.
> 
> Sending out the series in a different order would not avoid this problem 
> in general. Specifically because it is hard to know which parts will 
> generate friction with reviewers. My guesses have been wrong more often 
> than not in this regard.
> 

Send what's the minimum to reach U-Boot proper and interact with the 
console. I would go as far as saying it's fine if you cannot do anything 
with it like loading and booting Linux next until other series are sent 
and merged. Baby steps. On select platforms (like Rockchip recently), 
you don't even need support for a storage medium to load U-Boot proper 
as you could load the whole thing over USB via the BootROM. This also 
gives the opportunity of other people in the community to work on other 
parts of the SoC bring-up in parallel.

> Not sure there is anything we can really do about it, but I can totally 
> understand why someone would send large "add new SoC xyz" patch series.

I understand as well, but the bigger it is with the more susbsytems 
involved, the less likely people are going to spend time on it or look 
at it. For some reason I'm in Cc of many many patches for the last year 
or so, and I don't care about probably 90% of it, if the title of the 
cover letter doesn't ring a bell, it goes straight to my archive, I 
don't even uncollapse the thread to look at individual patches in the 
series.
I don't have an answer on what is the best compromise, and I think we 
both agree on no definitive answer is possible, so on my side I have 
nothing more to add to this topic here.

Cheers,
Quentin

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-11-12 15:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-04 19:48 [PATCH v2] doc: develop/sending_patches: add tips for series Ralph Siemsen
2026-11-05 11:00 ` Quentin Schulz
2026-11-05 14:33   ` Ralph Siemsen
2026-11-05 15:46     ` Quentin Schulz
2026-11-06 18:41       ` Ralph Siemsen
2026-11-12 15:48         ` Quentin Schulz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox