Skip to content

Add SunFounder Pironman 5 overlay #6087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 8, 2024

Conversation

cavonlee
Copy link

@cavonlee cavonlee commented Apr 3, 2024

No description provided.

Copy link
Contributor

@pelwell pelwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few minor issues I've discussed in inline comments, but the main problem with this overlay is that it doesn't have either a Makefile entry (so it won't get built) or a README entry (so people won't know what it does or how to use it).

Makefile entries should be in alphabetical/ASCII order, as should the README, and README entries need 2 blank lines between them, strict indentation (match the others) and no TAB characters. If you get it wrong, the dtoverlay checker will complain. For this first attempt it said:

Overlays without documentation:
  sunfounder-pironman5
Overlays missing from the Makefile:
  sunfounder-pironman5
Failed

fragment@2 {
target-path = "/";
__overlay__ {
gpio_ir: ir-receiver@12 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The convention is to use hexadecimal "addresses" after the "@". The "12" here is 0x12, i.e. 18, which matches the gpio-ir overlay you've based this on. Because you've changed the GPIO to (decimal/denary) 12, you should also change the address to "@c". In that way it can coexist with a regular gpio-ir overlay on GPIO 18.

fragment@3 {
target = <&gpio>;
__overlay__ {
gpio_ir_pins: gpio_ir_pins@12 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"@c" again.

/plugin/;

/ {
compatible = "brcm,bcm2835";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a lot of overlays that use spaces for indents, including gpio-ir. I'm not bothered about these warnings.

ir = <&gpio_ir>,"status";
ir_pins = <&gpio_ir>,"gpios:4", <&gpio_ir>,"reg:0", <&gpio_ir_pins>,"brcm,pins:0", <&gpio_ir_pins>,"reg:0";
};

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete this blank line, and put a newline at the end of the last line to keep git happy.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Great, Thanks for all the suggestions!

@pelwell pelwell merged commit 2aaec90 into raspberrypi:rpi-6.1.y Apr 8, 2024
@popcornmix
Copy link
Collaborator

Note: this was merged to 6.1 kernel which is no longer used by RPiOS.
It will need cherry-picking to 6.6 (and later) kernels before it's in standard builds.

@cavonlee
Copy link
Author

Note: this was merged to 6.1 kernel which is no longer used by RPiOS. It will need cherry-picking to 6.6 (and later) kernels before it's in standard builds.

Thanks for the note, So do i need to cherry pick it in 6.6, than make another pull request?

and, when it will be on the raspberry pi os? just wait the next raspberry pi os update? or maybe a kernal upgrade will take affect?

@pelwell
Copy link
Contributor

pelwell commented Apr 15, 2024

So do i need to cherry pick it in 6.6,

No, it's OK - it's there now (b429eb7).

when it will be on the raspberry pi os?

It shouldn't be too long until the next update to the stable branch.

@cavonlee
Copy link
Author

Great! Thank you pelwell!

popcornmix added a commit to raspberrypi/rpi-firmware that referenced this pull request Apr 18, 2024
See: raspberrypi/linux#6110

kernel: DTS: overlays: fix Pi 5 midi-over-UART
See: raspberrypi/linux#6109

kernel: DTS: rp1: fix setting xHCI TX burst fifo thresholds
See: raspberrypi/linux#6107

kernel: Fix UVC gadget support on 32-bit systems
See: raspberrypi/linux#6095

kernel: Add SunFounder Pironman 5 overlay
See: raspberrypi/linux#6087

kernel: configs: Add various Intel Ethernet drivers
See: raspberrypi/linux#5797
See: raspberrypi/linux#6102

kernel: ARM: dts: Move virtgpio under the firmware node
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Apr 18, 2024
See: raspberrypi/linux#6110

kernel: DTS: overlays: fix Pi 5 midi-over-UART
See: raspberrypi/linux#6109

kernel: DTS: rp1: fix setting xHCI TX burst fifo thresholds
See: raspberrypi/linux#6107

kernel: Fix UVC gadget support on 32-bit systems
See: raspberrypi/linux#6095

kernel: Add SunFounder Pironman 5 overlay
See: raspberrypi/linux#6087

kernel: configs: Add various Intel Ethernet drivers
See: raspberrypi/linux#5797
See: raspberrypi/linux#6102

kernel: ARM: dts: Move virtgpio under the firmware node
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants