[RFC] Upstreaming Orange Pi RV2 vendor patches - Seeking guidance

[Quentin Schulz]

Hi Sungjoon Moon,

On 4/1/25 3:41 PM, Sungjoon Moon wrote:
> [You don't often get email from sumoon at 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