* Re: [RFC] Upstreaming Orange Pi RV2 vendor patches - Seeking guidance
2025-04-01 13:41 [RFC] Upstreaming Orange Pi RV2 vendor patches - Seeking guidance Sungjoon Moon
@ 2025-04-02 9:38 ` Quentin Schulz
0 siblings, 0 replies; 2+ messages in thread
From: Quentin Schulz @ 2025-04-02 9:38 UTC (permalink / raw)
To: Sungjoon Moon, u-boot
Hi Sungjoon Moon,
On 4/1/25 3:41 PM, Sungjoon Moon wrote:
> [You don't often get email from sumoon@seoulsaram.org. Learn why this is
> important at https://aka.ms/LearnAboutSenderIdentification ]
>
> I'm working on upstreaming vendor patches for the Orange Pi RV2 board,
> currently available at:
> https://eur02.safelinks.protection.outlook.com/?
> url=https%3A%2F%2Fgithub.com%2FOctopusET%2Fu-boot-
> orangepi%2Fpull%2F1%2Fcommits&data=05%7C02%7Cquentin.schulz%40cherry.de%7C32ac8a27848840e80f4f08dd71657036%7C5e0e1b5221b54e7b83bb514ec460677e%7C0%7C0%7C638791402679712024%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=VhrD%2BgKD0cCW5pZIKhy7BoiGtj5fyTKukkdVj116jIY%3D&reserved=0
>
> I'm rebasing these patches onto the latest U-Boot branch (v2025.04-rc5).
> To ensure the patches meet upstream standards, I have a few questions:
> 1. What specific steps should I take after rebasing to prepare these
> patches for upstream submission?
Not sure what exactly you're after here.
You need at the very least to run scripts/checkpatch.pl on all your
patches and have them pass successfully. Some warnings may be ignored in
some corner cases so ask if some warnings are odd or difficult to fix.
This can be run automatically if you're using b4[1] with `b4 prep --check`.
Your patches shouldn't break other boards, be it build time or runtime.
For build time, you could build everything and check that it still
compiles. I vaguely remember buildman tool is supposed to be able to do
that but I've used it maybe once.
> 2. The current patch set is quite large. Should I split it into
> smaller, more focused patches? If so, what would be the recommended
> approach for splitting?
https://docs.u-boot.org/en/latest/develop/sending_patches.html the whole
page is worth reading but to answer this question specifically:
"""
- Patches should always contain exactly one complete logical change, i.e.
- Changes that contain different, unrelated modifications shall be
submitted as separate patches, one patch per changeset.
- If one logical set of modifications affects or creates several
files, all these changes shall be submitted in a single patch.
- Non-functional changes, i.e. whitespace and reformatting changes,
should be done in separate patches marked as cosmetic. This separation
of functional and cosmetic changes greatly facilitates the review process.
"""
I'm of the opinion that it's easier to start with too many commits and
squash them on request than splitting commits.
If there are patches that the project could benefit from that would
impact more boards than the upcoming OrangePi RV2, you may consider
sending them in a separate series in advance. In short, series are
rarely partially merged, so if there are controversial patches in a
series, it could be beneficial to split into multiple, easier to review
and merge, series.
> 3. Are there any particular coding style or documentation requirements
> I should be aware of for the Orange Pi RV2 board support?
https://docs.u-boot.org/en/latest/develop/designprinciples.html
https://docs.u-boot.org/en/latest/develop/codingstyle.html
https://docs.u-boot.org/en/latest/develop/board_best_practices.html
Add tests when adding code.
Add documentation for your board to
https://docs.u-boot.org/en/latest/board/index.html
> 4. How should I handle any vendor-specific code that may not be
> suitable for upstream?
We all have different opinion on what may or may not be suitable for
upstream. Anything not sent upstream, we don't care, that's your problem
to handle. If you send patches that are unacceptable, we'll tell you and
we'll try to figure out ways to deal with them (easier by outright
rejecting them, or by suggesting ways to modify the changes so they are
acceptable).
What specifically do you have in mind as being not suitable for upstream?
> 5. Are there any maintainers or subsystem-specific mailing lists I
> should include when submitting these patches?
>
Use scripts/get_maintainer.pl on your patches, and Cc/To most/all people
and mailing list listed there.
This is done automatically by b4[1] with `b4 prep --auto-to-cc`.
[1] https://b4.docs.kernel.org/en/latest/contributor/overview.html
> Any guidance or best practices for upstreaming vendor patches would be
> greatly appreciated. Thank you for your time and assistance.
>
Good luck, have fun and looking forward to seeing patches on the mailing
list.
Cheers,
Quentin
^ permalink raw reply [flat|nested] 2+ messages in thread