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.

Sign up for GitHub

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

Jump to bottom

drivers: gpio: Add Davinci gpio controller support #61316

Merged
merged 1 commit into from
Aug 31, 2023
Merged

drivers: gpio: Add Davinci gpio controller support #61316

merged 1 commit into from
Aug 31, 2023

Conversation

slpp95prashanth
Copy link
Contributor

@slpp95prashanth slpp95prashanth commented Aug 9, 2023 •
edited
Loading

Davinci gpio controller support to add various soc gpio support (J721E, AM654).

TRM for J721e https://www.ti.com/lit/zip/spruil1
File: spruil1c.pdf
GPIO: section 12.1.2

BeagleBone AI_64 https://beagleboard.org/ai-64

DhruvaG2000
DhruvaG2000 suggested changes Aug 9, 2023
View reviewed changes
Copy link
Contributor

@DhruvaG2000 DhruvaG2000 left a comment

Choose a reason for hiding this comment

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

Have left inline comments

drivers/gpio/gpio_davinci.c Outdated Show resolved Hide resolved
drivers/gpio/Kconfig.davinci Outdated Show resolved Hide resolved
drivers/gpio/gpio_davinci.c Outdated Show resolved Hide resolved
drivers/gpio/gpio_davinci.c Show resolved Hide resolved
drivers/gpio/gpio_davinci.c Outdated Show resolved Hide resolved
dts/bindings/gpio/ti,am654-gpio-nexus.yaml Outdated Show resolved Hide resolved
@vaishnavachath vaishnavachath self-requested a review August 9, 2023 07:22
dnltz
dnltz reviewed Aug 10, 2023
View reviewed changes
Copy link
Collaborator

@dnltz dnltz left a comment

Choose a reason for hiding this comment

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

Left some inline comments too.

drivers/gpio/CMakeLists.txt Outdated Show resolved Hide resolved
dts/bindings/gpio/ti,am654-gpio-nexus.yaml Outdated Show resolved Hide resolved
dts/bindings/gpio/ti,am654-gpio-nexus.yaml Outdated Show resolved Hide resolved
drivers/gpio/gpio_davinci.c Outdated Show resolved Hide resolved
drivers/gpio/gpio_davinci.c Show resolved Hide resolved
drivers/gpio/gpio_davinci.c Outdated Show resolved Hide resolved
drivers/gpio/gpio_davinci.c Show resolved Hide resolved
dnltz
dnltz requested changes Aug 11, 2023
View reviewed changes
Copy link
Collaborator

@dnltz dnltz left a comment

Choose a reason for hiding this comment

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

Good job! I only see one issue with how you broke the arguments.

drivers/gpio/gpio_davinci.c Outdated Show resolved Hide resolved
dnltz
dnltz approved these changes Aug 14, 2023
View reviewed changes
Copy link
Collaborator

@dnltz dnltz left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

@DhruvaG2000
Copy link
Contributor

@slpp95prashanth any reason to still keep this a draft PR? Please mark as ready if you finally think it's in a good state to review and merge. (unless there's some "depends on" PR that needs to go in before this). If so then please specify which PR

@slpp95prashanth slpp95prashanth marked this pull request as ready for review August 15, 2023 12:01
@zephyrbot zephyrbot added platform: TI SimpleLink Texas Instruments SimpleLink MCU area: Devicetree Binding PR modifies or adds a Device Tree binding area: GPIO labels Aug 15, 2023
DhruvaG2000
DhruvaG2000 suggested changes Aug 16, 2023
View reviewed changes
Copy link
Contributor

@DhruvaG2000 DhruvaG2000 left a comment

Choose a reason for hiding this comment

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

Left some comments, please take a look

drivers/gpio/gpio_davinci.c Show resolved Hide resolved
dts/bindings/gpio/ti,davinci-gpio.yaml Outdated Show resolved Hide resolved
drivers/gpio/gpio_davinci.c Show resolved Hide resolved
drivers/gpio/gpio_davinci.c Show resolved Hide resolved
DhruvaG2000
DhruvaG2000 approved these changes Aug 16, 2023
View reviewed changes
Copy link
Contributor

@DhruvaG2000 DhruvaG2000 left a comment

Choose a reason for hiding this comment

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

LGTM. thanks for addressing all my concerns!

vaishnavachath
vaishnavachath requested changes Aug 17, 2023
View reviewed changes
drivers/gpio/gpio_davinci.c Show resolved Hide resolved
drivers/gpio/gpio_davinci.c Show resolved Hide resolved
@slpp95prashanth slpp95prashanth force-pushed the zephyr_gpio branch 2 times, most recently from 9cfe425 to 6ee0b65 Compare August 17, 2023 11:56
vaishnavachath
vaishnavachath previously approved these changes Aug 21, 2023
View reviewed changes
Copy link
Collaborator

@vaishnavachath vaishnavachath left a comment

Choose a reason for hiding this comment

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

Looks Good, this should ideally be merged only after #59191 is merged, if someone tests and confirms functionality on AM62 A53/M4 target that would be great.

@dnltz
Copy link
Collaborator

dnltz commented Aug 21, 2023

Looks Good, this should ideally be merged only after #59191 is merged, if someone tests and confirms functionality on AM62 A53/M4 target that would be great.

I can do that today/tomorrow!

aasinclair
aasinclair previously approved these changes Aug 24, 2023
View reviewed changes
Copy link
Contributor

@aasinclair aasinclair left a comment

Choose a reason for hiding this comment

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

Looks good.
Some of the whitespace looks a little strange, for future contributions you may wish to consider using clang-format to autoformat the code, rather than aligning things manually.

drivers/gpio/gpio_davinci.c Outdated Show resolved Hide resolved
@slpp95prashanth
drivers: gpio: Add Davinci gpio controller support
6226697 
Davinci gpio controller support to add various soc gpio
support (J721E, AM654).

TRM for J721e https://www.ti.com/lit/zip/spruil1
File: spruil1c.pdf
GPIO: section 12.1.2

BeagleBone AI_64 https://beagleboard.org/ai-64

Signed-off-by: Prashanth S <slpp95prashanth@yahoo.com>
@galak galak assigned henrikbrixandersen and unassigned galak Aug 30, 2023
gmarull
gmarull previously requested changes Aug 31, 2023
View reviewed changes
drivers/gpio/gpio_davinci.c Show resolved Hide resolved
@gmarull gmarull dismissed their stale review August 31, 2023 08:23

not applicable

@carlescufi carlescufi merged commit 12996d5 into zephyrproject-rtos:main Aug 31, 2023
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmarull gmarull gmarull left review comments

@aasinclair aasinclair aasinclair approved these changes

@vaishnavachath vaishnavachath vaishnavachath approved these changes

@dnltz dnltz dnltz approved these changes

@DhruvaG2000 DhruvaG2000 DhruvaG2000 approved these changes

@galak galak Awaiting requested review from galak

@mnkp mnkp Awaiting requested review from mnkp

@cfriedt cfriedt Awaiting requested review from cfriedt

@henrikbrixandersen henrikbrixandersen Awaiting requested review from henrikbrixandersen

@vanti vanti Awaiting requested review from vanti

@kartben kartben Awaiting requested review from kartben

Assignees
No one assigned
Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: GPIO platform: TI SimpleLink Texas Instruments SimpleLink MCU
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.