Skip to content

CC13XX CC26XX : transition from pinmux to pinctrl #44492

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
Merged
merged 3 commits into from
Apr 18, 2022
Merged

CC13XX CC26XX : transition from pinmux to pinctrl #44492

merged 3 commits into from
Apr 18, 2022

Conversation

@vaishnavachath
Copy link
Member

@vaishnavachath vaishnavachath commented Apr 3, 2022 •
edited
Loading

This pull request contains the changes to migrate from pinmux to pinctrl API for CC13XX and CC26XX based platforms as outlined in #39740 .
The pull request contains the following changes in the respective commits:

  1. Introduces the pinctrl driver for CC13XX/CC26XX platform.
  2. migrate existing consumers to use the new driver, pinctrl API.
  3. remove obsolete pinmux driver.
  4. fix white-space/styling issues for defines moved from hal/driverlib/ioc.h - intentionally kept as separate commit for now, can be squashed with the first commit if that is preferred.

The change-set was tested on CC1352R sensor-tag to verify the I2C/SPI/UART IOC functionality is not broken.

@github-actions github-actions bot added area: API Changes to public APIs area: Boards area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: I2C area: SPI SPI bus labels Apr 3, 2022
@vaishnavachath
Copy link
Member Author

vaishnavachath commented Apr 3, 2022

@cfriedt @bwitherspoon Can you please review the PR?

@cfriedt cfriedt mentioned this pull request Apr 3, 2022
Closed
29 tasks
cfriedt
cfriedt previously approved these changes Apr 3, 2022
View reviewed changes
Copy link
Member

@cfriedt cfriedt 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 so much for getting these changes in!

@vaishnavachath - would be great if you could convince TI to allow you to be platform maintainer for Zephyr 😃

gmarull
gmarull requested changes Apr 4, 2022
View reviewed changes
@zephyrbot zephyrbot added area: UART Universal Asynchronous Receiver-Transmitter area: Pinmux area: Pinctrl labels Apr 4, 2022
@vaishnavachath vaishnavachath force-pushed the cc13xx_cc26xx_pinmux_pinctrl branch from 967e044 to 272d876 Compare April 5, 2022 02:29
mbolivar-nordic
mbolivar-nordic reviewed Apr 8, 2022
View reviewed changes
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic left a comment

Choose a reason for hiding this comment

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

Other than @gmarull's comment on missing examples and details, the bindings changes LGTM.

@cfriedt
Copy link
Member

cfriedt commented Apr 12, 2022

@vaishnavachath - can you make the changes requested by @gmarull ?

@vaishnavachath
Copy link
Member Author

vaishnavachath commented Apr 12, 2022

@vaishnavachath - can you make the changes requested by @gmarull ?

@cfriedt , Yes I will send the updates soon.
Thank you @gmarull , @mbolivar-nordic , @cfriedt for your review, sorry for the delay in sending the updates.

I have made the trivial updates but on the standard pin properties suggestion, had slight confusion whether to provide all the pin properties configuration option through DT or to provide a relevant subset(pull, mode, input-enable), of the configuration as pin properties.
https://github.com/zephyrproject-rtos/zephyr/blob/272d87649897a553b407d6621670ab64ea27b3ec/include/dt-bindings/pinctrl/cc13xx_cc26xx-pinctrl.h has multiple possible modes.

@vaishnavachath vaishnavachath force-pushed the cc13xx_cc26xx_pinmux_pinctrl branch from 272d876 to 5295a86 Compare April 15, 2022 11:21
@vaishnavachath
Copy link
Member Author

vaishnavachath commented Apr 15, 2022

@gmarull @cfriedt @mbolivar-nordic , I have made the suggested changes, can you please review the updates.
Changes made:

  • implement standard properties (bias-pull-up, bias-pull-down .etc)
  • update DT binding documentation with examples
  • earlier the styling fixes on driverlib header was separate commit which created confusion, that was fixed in the initial commit itself

mbolivar-nordic
mbolivar-nordic previously approved these changes Apr 15, 2022
View reviewed changes
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic left a comment

Choose a reason for hiding this comment

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

Bindings LGTM. The rest I only reviewed quickly, but it all seems reasonable as well. Thanks for the update!

@cfriedt
Copy link
Member

cfriedt commented Apr 15, 2022

@vaishnavachath - looks like some minor changes to get the last two tests to pass. Are you running with Twister?

gmarull
gmarull previously approved these changes Apr 16, 2022
View reviewed changes
Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

LGTM, one minor non-blocking nitpick

@vaishnavachath vaishnavachath dismissed stale reviews from gmarull and mbolivar-nordic via e9bc04a April 18, 2022 01:06
@vaishnavachath vaishnavachath force-pushed the cc13xx_cc26xx_pinmux_pinctrl branch from 5295a86 to e9bc04a Compare April 18, 2022 01:06
@vaishnavachath
drivers: pinctrl: add CC13XX/CC26XX pinctrl driver
6a2eb84 
Add pinctrl driver for CC13XX/CC26XX family of SoCs
to facilitate transition from pinmux to pinctrl.

`IOCPortConfigureSet()` from TI hal driverlib used to
implement the generic pinctrl driver.

Signed-off-by: Vaishnav Achath <[email protected]>
@vaishnavachath
drivers: CC1XX/CC26XX based boards: transition to pinctrl driver
9f87f1b 
This commit has the necessary changes to update the consumers
of pinmux driver(SPI, I2C, UART) and update the board specific
files to use the pinctrl interface.

Signed-off-by: Vaishnav Achath <[email protected]>
@vaishnavachath
drivers: pinmux: remove cc13xx_cc26xx pinmux driver
c251fb9 
all the consumers of the obsolete pinmux driver is
updated to use pinctrl API, this commit removes
the pinmux driver and assosciated sections.

Signed-off-by: Vaishnav Achath <[email protected]>
@vaishnavachath vaishnavachath force-pushed the cc13xx_cc26xx_pinmux_pinctrl branch from e9bc04a to c251fb9 Compare April 18, 2022 01:07
@vaishnavachath
Copy link
Member Author

vaishnavachath commented Apr 18, 2022

@vaishnavachath - looks like some minor changes to get the last two tests to pass. Are you running with Twister?

@cfriedt made small mistake in cc26xx dtsi while updating which is fixed now and tests are passing.
@gmarull updated with suggested minor change in pinctrl_cc13xx_cc26xx.c

@cfriedt cfriedt merged commit 98f1a98 into zephyrproject-rtos:main Apr 18, 2022
@vaishnavachath vaishnavachath mentioned this pull request Sep 17, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cfriedt cfriedt cfriedt approved these changes

@gmarull gmarull gmarull approved these changes

@nashif nashif Awaiting requested review from nashif

@carlescufi carlescufi Awaiting requested review from carlescufi

@galak galak Awaiting requested review from galak

@MaureenHelm MaureenHelm Awaiting requested review from MaureenHelm

@tbursztyka tbursztyka Awaiting requested review from tbursztyka

@dcpleung dcpleung Awaiting requested review from dcpleung

@mnkp mnkp Awaiting requested review from mnkp

@teburd teburd Awaiting requested review from teburd

@mbolivar-nordic mbolivar-nordic Awaiting requested review from mbolivar-nordic

Assignees

No one assigned

Labels

area: API Changes to public APIs area: Boards area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree area: I2C area: Pinctrl area: Pinmux area: SPI SPI bus area: UART Universal Asynchronous Receiver-Transmitter

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vaishnavachath @cfriedt @gmarull @mbolivar-nordic @zephyrbot