Skip to content
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

boards: sparkfun: Fix sparkfun_pro_micro_rp2040 gpiomap #69994

Merged
merged 1 commit into from
Apr 7, 2024

Conversation

ulmanyar
Copy link
Contributor

@ulmanyar ulmanyar commented Mar 8, 2024

The SparkFun Pro Micro pins are numbered 1, 0, GND, ... from top left whereas the SparkFun Pro Micro RP2040 and the RP2040 Community Edition boards are numbered 0, 1, GND, ... , so the pro_micro: connector gpio-map should reflect that.

Graphical Datasheet for SparkFun Pro Micro RP2040: https://cdn.sparkfun.com/assets/e/2/7/6/b/ProMicroRP2040_Graphical_Datasheet.pdf

Graphical Datasheet for SparkFun Pro Micro:
https://cdn.sparkfun.com/assets/f/d/8/0/d/ProMicro16MHzv2.pdf

@zephyrbot zephyrbot added the platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico) label Mar 8, 2024
@zephyrbot zephyrbot requested review from soburi and yonsch March 8, 2024 22:38
Copy link

github-actions bot commented Mar 8, 2024

Hello @ulmanyar, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@ulmanyar ulmanyar force-pushed the sparkfun-gpio-map branch 2 times, most recently from 87b26c2 to d2bf895 Compare March 8, 2024 22:47
@ulmanyar ulmanyar changed the title boards: arm: Fix sparkfun_pro_micro_rp2040 gpiomap boards: sparkfun: Fix sparkfun_pro_micro_rp2040 gpiomap Mar 8, 2024
@soburi
Copy link
Member

soburi commented Mar 9, 2024

Hi, @ulmanyar

The pro-micro connector is defined here,

This binding provides a nexus mapping for 18 pins, as depicted below:
0 TX0 RAW -
1 RX1 GND -
- GND RST -
- GND VCC -
2 D2 D21/A3 21
3 D3 D20/A2 20
4 A4 D19/A1 19
5 D5 D18/A0 18
6 D6 D15 15
7 D7 D14 14
8 D8 D16 16
9 D9 D10 10

but from this definition the settings before the change are correct.

@ulmanyar
Copy link
Contributor Author

The more I look at this, the more it seems like a game of telephone where inconsistencies propagate and multiply.

I would argue that the pro-micro connector definition you linked is incorrect, as I think it should reflect the pins of the original Pro Micro. I will add that to this PR.

I also looked at what boards are using this, and it seems it's only the SparkFun Pro Mirco RP2040 that I originally addressed, but also the Adafruit KB2040 so I can add that as well.

As a side note: in the naming of the KB2040 pins, they have labeled D0 and D1 as "CircuitPython Name", whereas Sparkfun Pro Micro label them as "Arduino", and I don't know if they are supposed to be 1:1 or not. I have never worked with CircuitPython, but in practice they do not seem to be 1:1 based on:
https://github.com/adafruit/circuitpython/blob/54e78c89fc9e0d2b31544dfd03c30a159e8540a4/ports/raspberrypi/boards/sparkfun_pro_micro_rp2040/pins.c#L25
https://github.com/adafruit/circuitpython/blob/ad6bc74f28fcfc8cfa656122f62ba27495721dc5/ports/raspberrypi/boards/adafruit_kb2040/pins.c#L17

@ulmanyar
Copy link
Contributor Author

Any additional feedback on this? This was merged downstream in ZMK but it would be great to be able to use the board from Zephyr directly.

Comment on lines 19 to 20
0 TX0 RAW -
1 RX1 GND -
1 TX0 RAW -
0 RX1 GND -
Copy link
Member

Choose a reason for hiding this comment

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

if there's two versions of the number it may be a good idea to make the description here reflect that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See comment below. I think this is the only variant in Zephyr.

Copy link
Member

Choose a reason for hiding this comment

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

Cool, then maybe add a comment after the table here describing that the first two pins are swapped on some boards and that this binding is used for both variants?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realised I could be clearer: This documentation describes the (original) SparkFun Pro Micro header and its corresponding pin numbers. In that original header, the order is 1, 0, GND, ...

Each Pro Micro-compatible board--such as SparkFun Pro Micro RP2040 which n.B. is a different board, or any board that wants to be Pro Micro-compatible--then defines a gpio_map that maps that board's pinout to the header described here so that shields can be defined using the &pro_micro pins and seamlessly be used with any Pro Micro-compatible board.

So in my opinion, it does not make sense to add every compatible board in this header description, as they are really not variants, but unique boards with compatible pinouts. I hope this clarifies the confusion I made!

Copy link
Member

Choose a reason for hiding this comment

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

Ok I see what you are saying now, sorry https://cdn.sparkfun.com/assets/e/2/7/6/b/ProMicroRP2040_Graphical_Datasheet.pdf got me really confused about the whole story.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, could you add a link to https://cdn.sparkfun.com/assets/f/d/8/0/d/ProMicro16MHzv2.pdf in this file maybe? Just for convenience, doubt that url will go anywhere anyway.

Copy link
Contributor Author

@ulmanyar ulmanyar Apr 6, 2024

Choose a reason for hiding this comment

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

There, fixed a few minor things (inconsistent use of both Dx/Ay and only Dx, as well as a typo where TX0 became TX0, and RXI became RX1), and added the link. This should be ready for merge now?

@fabiobaltieri
Copy link
Member

cc @petejohanson

@soburi
Copy link
Member

soburi commented Mar 25, 2024

The more I look at this, the more it seems like a game of telephone where inconsistencies propagate and multiply.

I would argue that the pro-micro connector definition you linked is incorrect, as I think it should reflect the pins of the original Pro Micro. I will add that to this PR.

I also looked at what boards are using this, and it seems it's only the SparkFun Pro Mirco RP2040 that I originally addressed, but also the Adafruit KB2040 so I can add that as well.

As a side note: in the naming of the KB2040 pins, they have labeled D0 and D1 as "CircuitPython Name", whereas Sparkfun Pro Micro label them as "Arduino", and I don't know if they are supposed to be 1:1 or not. I have never worked with CircuitPython, but in practice they do not seem to be 1:1 based on: https://github.com/adafruit/circuitpython/blob/54e78c89fc9e0d2b31544dfd03c30a159e8540a4/ports/raspberrypi/boards/sparkfun_pro_micro_rp2040/pins.c#L25 https://github.com/adafruit/circuitpython/blob/ad6bc74f28fcfc8cfa656122f62ba27495721dc5/ports/raspberrypi/boards/adafruit_kb2040/pins.c#L17

I checked some external connector definitions, and this modification seems to work fine. As @fabiobaltieri pointed out, please update the documentation.

@zephyrbot zephyrbot added the Release Notes To be mentioned in the release notes label Mar 30, 2024
The SparkFun Pro Micro header pins are numbered D1, D0, GND, ... from top
left whereas the SparkFun Pro Micro RP2040 and the Adafruit KB2040 boards
are using gpio 0, 1, GND, ... , so the pro_micro: connector gpio-map of
these boards should reflect that.

Graphical Datasheet for SparkFun Pro Micro RP2040:
https://cdn.sparkfun.com/assets/e/2/7/6/b/ProMicroRP2040_Graphical_Datasheet.pdf

Graphical Datasheet for SparkFun Pro Micro:
https://cdn.sparkfun.com/assets/f/d/8/0/d/ProMicro16MHzv2.pdf

Pinout of the Adafruit KB2040:
https://learn.adafruit.com/assets/106984

Please note that the KB2040 uses CircuitPython pin labels D0, D1 which does
not seemt correspond to the Arduino labels D0 and D1 used by the Pro Micro.

Signed-off-by: Daniel Irekvist <[email protected]>
@fabiobaltieri fabiobaltieri assigned kartben and unassigned yonsch Apr 6, 2024
@jhedberg jhedberg merged commit c38ea28 into zephyrproject-rtos:main Apr 7, 2024
21 checks passed
Copy link

github-actions bot commented Apr 7, 2024

Hi @ulmanyar!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

@ulmanyar ulmanyar deleted the sparkfun-gpio-map branch April 13, 2024 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico) Release Notes To be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants