JH7110 GPIO driver
Details
Details
- Reviewers
mhorne - Group Reviewers
riscv - Commits
- rG5bae2ace2e55: jh7110_gpio: Add StarFive JH7110 GPIO controller driver
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
Updating clk.h's path which has been changed during this year and adding this file to files.starfive
Comment Actions
Revised and expanded version of the driver. Now contains methods getcaps, getname, toggle and pad configuration RW in setflags.
Comment Actions
Thanks for the update. Despite my many absences in the past, we should be able to move quickly on this and get it merged.
I had some WIP changes of my own on top of your previous version of the patch. Please see here: P663.
Some of the small needed fixes I've noted inline, but there is a larger structural change that is also required. In short, we should follow the common pattern for GPIO drivers by maintaining a cached copy of the pin state in memory as a struct gpio_pin array in the softc (see sc->gpio_pins in my version, or dwgpio.c).
I can quickly combine the two versions myself, if you like. But it is your work, so I would not take the honor. Let me know how you would like to proceed :)
FYI I also have a nearly-finished change implementing the fdt_pinctrl interface. This allows the driver to fetch and apply pre-configured pin state from the device tree. But this is not a blocker and can come later.
| sys/riscv/starfive/jh7110_gpio.c | ||
|---|---|---|
| 14–19 | Please sort alphabetically. | |
| 268 | ||
| 281 | ||
| 298 | ||
| 311–312 | And you can delete err. | |
| 319–330 | We should ensure that all controller initialization is complete before possibly attaching child devices. @vexeduxr did an audit of this recently; does this look right? | |
| sys/riscv/starfive/jh7110_gpio.c | ||
|---|---|---|
| 319–330 | Looks good! | |
Comment Actions
This small update includes the proposed smaller changes.
Your larger proposition looks like a straightforward application of a generic structure from other GPIO drivers, so if you like to add it by yourself, just go ahead. I considered writing just the same thing by myself (mostly after the example of SiFive's gpio driver) but I didn't understand why those data structures would be needed for this particular driver.