-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
[TOPIC-GPIO] Update gpio_intel_apl driver to new GPIO API #20400
[TOPIC-GPIO] Update gpio_intel_apl driver to new GPIO API #20400
Conversation
All checks are passing now. Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages. |
7c4b558
to
6a5dea2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with minor comments.
6a5dea2
to
516485a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed the driver code. I didn't review the DTS since I'm not familiar with the architecture.
516485a
to
6b1bcac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have any objections merging this for the topic branch, though the statement that pulls can't be verified (though they're supported) is a bit worrisome.
It would be good if @galak had a quick look and verified DTS usage. It's a bit non standard. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the DTS looks a little odd, Hopefully @dcpleung can point me at a manual/specs for the SoC to understand the register layout and why we need 'offset' and multiple GPIO controller nodes.
There are 4 GPIO controllers on the SoC, where each one has more than 32 pins. Since the GPIO API is based on the assumption that one port has 32 pins, this driver has to divide one controller into a few driver instances (hence the pin offsets). The datasheet is @ https://www.intel.com/content/www/us/en/processors/pentium/pentium-celeron-n-series-j-series-datasheet-vol-3.html, and GPIO is chapter 22. The section 22.[1-4] describe the register sets for the 4 GPIO controllers. |
As suggested in the API telecon: https://github.com/pabigot/zephyr/commits/pr/20400 provides a nexus map that can be used to translate GPIO references from the controller to subdevices. I.e. devicetree source can use If you find this useful you should cherrypick the map and squash the overlay updates. The bash script used to generate the bulk of the map may be kept for reference. |
6b1bcac
to
b3b9fd9
Compare
@pabigot Thank you. I have updated the PR with your changes. :) |
b3b9fd9
to
aeb4eb3
Compare
There were complains about the unit-address and first reg not matching. So update the DTS file to match. Signed-off-by: Daniel Leung <daniel.leung@intel.com>
Update driver code to use new GPIO configuration flags such as GPIO_ACTIVE_LOW. Also add implementation of new port_* driver API as well as gpio_pin_interrupt_configure function. Signed-off-by: Daniel Leung <daniel.leung@intel.com>
This breaks down the GPIO controller definition in DTS into multiple entries. This allows these controllers to be referenced by other DTS, and test board overlay files. And also we can remove the entries in the dts fixup file. Signed-off-by: Daniel Leung <daniel.leung@intel.com>
This maps devicetree GPIO phandle arrays from the full controller pin range to the sub-controllers required by Zephyr's limit of 32 pins per controller device. Signed-off-by: Peter A. Bigot <pab@pabigot.com>
This adds a board overlay for up_squared to be used with the GPIO sample. Using the overlay is because the pins being used are not actually LED and switch but GPIO pins used as such. Signed-off-by: Daniel Leung <daniel.leung@intel.com>
This adds a board overlay for up_squared to be used with the GPIO basic API test. Signed-off-by: Daniel Leung <daniel.leung@intel.com>
This adds a board overlay for up_squared to be used with the GPIO basic API test. Signed-off-by: Daniel Leung <daniel.leung@intel.com>
aeb4eb3
to
824e2fb
Compare
This updates the
gpio_intel_apl
to the new GPIO API. This also changes how the GPIO controllers are defined in the DTS, to a) remove the entries indts_fixup.h
, and b) allow referencing these controllers from other DTS files. Also adds a few board overlay files to the new GPIO API tests and the sample GPIO app.This has been tested on the
up_squared
board. However, these is a FPGA acting as I/O buffer on the HAT where settings are controlled by BIOS. Pins can be set as input or output as GPIOs. Since the FPGA is actually driving the pins, pull-up/pull-down tests on the input pin do not work correctly.