* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.