Skip to content

Alternative pin group based pinctrl implementation for RT1062 #42238

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 10 commits into from
Mar 15, 2022
Merged

Alternative pin group based pinctrl implementation for RT1062 #42238

merged 10 commits into from
Mar 15, 2022

Conversation

@danieldegrasse
Copy link
Contributor

@danieldegrasse danieldegrasse commented Jan 27, 2022

This PR is an alternative approach from #41810 which uses pin groups instead of pin nodes defined at the SOC level. An SOC level header file is still included for readability and so that the user is aware of all pinmux options present on the SOC.

The pin groups approach offers the following benefits:

  • multiple pinctrl nodes can be created with the same pinmux value, enabling pins to have a different states in different power modes without changing their mux settings
  • selecting a pinctrl group in the board level DTS is simpler and less error prone than individually selecting pins for that peripheral

@github-actions github-actions bot added manifest west manifest-hal_nxp DNM This PR should not be merged (Do Not Merge) labels Jan 27, 2022
@github-actions
Copy link

github-actions bot commented Jan 27, 2022 •
edited
Loading

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
hal_nxp zephyrproject-rtos/hal_nxp@072bf81 zephyrproject-rtos/hal_nxp@da6c141 (master) zephyrproject-rtos/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@danieldegrasse danieldegrasse changed the title Add pinctrl implementation for RT1062 Alternative pin group based pinctrl implementation for RT1062 Jan 27, 2022
@danieldegrasse danieldegrasse mentioned this pull request Jan 27, 2022
Closed
hakehuang
hakehuang reviewed Jan 28, 2022
View reviewed changes
hakehuang
hakehuang previously approved these changes Jan 28, 2022
View reviewed changes
gmarull
gmarull reviewed Jan 31, 2022
View reviewed changes
gmarull
gmarull reviewed Jan 31, 2022
View reviewed changes
@zephyrbot zephyrbot added area: Build System area: UART Universal Asynchronous Receiver-Transmitter labels Feb 1, 2022
@zephyrbot zephyrbot requested review from anangl and dcpleung February 1, 2022 13:12
hakehuang
hakehuang previously approved these changes Mar 7, 2022
View reviewed changes
tejlmand
tejlmand previously approved these changes Mar 7, 2022
View reviewed changes
Copy link
Contributor

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

lgtm - re-approved

gmarull
gmarull previously approved these changes Mar 7, 2022
View reviewed changes
@carlescufi
Copy link
Member

carlescufi commented Mar 7, 2022

@dleach02 ping

henrikbrixandersen
henrikbrixandersen requested changes Mar 7, 2022
View reviewed changes
Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Seeing that you add pinctrl support to the lpuart driver, I tried testing this on the twr_ke18f board (west build -b twr_ke18f samples/hello_world/ -- -DCONFIG_PINCTRL=y), which uses the Kinetis pinctrl driver along with the lpuart driver - but compilation of the lpuart driver is now failing (see #43486).

@danieldegrasse
Copy link
Contributor Author

danieldegrasse commented Mar 8, 2022 •
edited
Loading

Seeing that you add pinctrl support to the lpuart driver, I tried testing this on the twr_ke18f board

@henrikbrixandersen, just to confirm: #43486 fixed pinctrl support on the twr_ke18f, and the lpuart driver now works with pinctrl, correct? Just want to verify that there is not an issue with this PR, and that the bug was in the Kinetis pinctrl implementation

@danieldegrasse danieldegrasse dismissed stale reviews from gmarull, tejlmand, and hakehuang via e00e356 March 8, 2022 00:27
@hakehuang hakehuang self-requested a review March 8, 2022 03:17
hakehuang
hakehuang previously approved these changes Mar 8, 2022
View reviewed changes
@danieldegrasse danieldegrasse mentioned this pull request Mar 8, 2022
Merged
danieldegrasse and others added 10 commits March 10, 2022 16:12
@danieldegrasse
west.yml: Update to pull in pinctrl header from NXP HAL
ebea288 
NXP hal will define constants for pinmux options in RT pinctrl
implementation. Update hal revision to pull in dtsi file.

Signed-off-by: Daniel DeGrasse <[email protected]>
@hakehuang @danieldegrasse
dts: bindings: add pinctrl binding for nxp mcux rt1xxx
4f40480 
Add dts binding for rt1xxx pinctrl driver settings. A binding file is
present for the pinctrl node itself, and for the pinctrl child node that
defines all pinmux options

Signed-off-by: Hake Huang <[email protected]>
Signed-off-by: Daniel DeGrasse <[email protected]>
@hakehuang @danieldegrasse
dts: bindings: add pinctrl node to sai and lpuart
e8cee71 
enable pinctrl in i2s and lpuart driver bindings

Signed-off-by: Hake Huang <[email protected]>
Signed-off-by: Daniel DeGrasse <[email protected]>
@hakehuang @danieldegrasse
soc: add pinctrl header file definition for RT series
4697620 
Pinctrl requires header file with Z_PINCTRL_STATE_PINS_INIT macro
defined. Add header file for mcux RT pinctrl implementation.
Signed-off-by: Hake Huang <[email protected]>
Signed-off-by: Daniel DeGrasse <[email protected]>
@hakehuang @danieldegrasse
dts: imx_rt: add pinctrl and gpr for rt1xxx
aaadefc 
add dtsi settings for rt series
dtsi use gpr to replace pinmux
nxp iomuxc has gpr which has more settings than mux and io settings
current solution is to export gpr separately and access then directly

Signed-off-by: Hake Huang <[email protected]>
Signed-off-by: Daniel DeGrasse <[email protected]>
@hakehuang @danieldegrasse
drivers: pinctrl: add mcux_rt pinctrl driver
dbd4f2d 
Add pinctrl driver for rt1xxx

Signed-off-by: Hake Huang <[email protected]>
Signed-off-by: Daniel DeGrasse <[email protected]>
@hakehuang @danieldegrasse
drivers: uart: add pinctrl support in mcux lpuart
3cc8f3a 
enable pinctrl in lpuart driver, and clean up driver instance definition
macros

Signed-off-by: Hake Huang <[email protected]>
@hakehuang @danieldegrasse
drivers: sai: add pinctrl support in mcux sai
f7e7c86 
enable i2s pinctrl

Signed-off-by: Hake Huang <[email protected]>
@hakehuang @danieldegrasse
board: pinctrl: board config for RT1060
3a6b74d 
enable pin control for RT1060 EVK.

Signed-off-by: Hake Huang <[email protected]>
Signed-off-by: Daniel DeGrasse <[email protected]>
@hakehuang @danieldegrasse
dts: binding: rename pinmux to gpr
1a1babf 
rename pinmux to gpr
different from pinmux and io settings gpr will do more IO
settings.

Signed-off-by: Hake Huang <[email protected]>
@henrikbrixandersen
Copy link
Member

henrikbrixandersen commented Mar 15, 2022

@henrikbrixandersen, just to confirm: #43486 fixed pinctrl support on the twr_ke18f, and the lpuart driver now works with pinctrl, correct? Just want to verify that there is not an issue with this PR, and that the bug was in the Kinetis pinctrl implementation

@danieldegrasse Sorry, I somehow missed your question. Yes, the problem was in the Kinetis pinctrl implementation and was fixed by #43486.

@dleach02 dleach02 merged commit 084e6df into zephyrproject-rtos:main Mar 15, 2022
@dleach02 dleach02 deleted the pinctrl-pin-groups branch March 15, 2022 15:12
@danieldegrasse danieldegrasse mentioned this pull request Mar 28, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@henrikbrixandersen henrikbrixandersen henrikbrixandersen approved these changes

@gmarull gmarull gmarull approved these changes

@hakehuang hakehuang hakehuang left review comments

@tejlmand tejlmand tejlmand left review comments

@dleach02 dleach02 dleach02 left review comments

@carlescufi carlescufi Awaiting requested review from carlescufi

@galak galak Awaiting requested review from galak

@MaureenHelm MaureenHelm Awaiting requested review from MaureenHelm

@mmahadevan108 mmahadevan108 Awaiting requested review from mmahadevan108

@nashif nashif Awaiting requested review from nashif

@anangl anangl Awaiting requested review from anangl

@dcpleung dcpleung Awaiting requested review from dcpleung

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

Assignees

@dleach02 dleach02

Labels

area: API Changes to public APIs area: Boards area: Build System area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree area: I2S area: Modules area: UART Universal Asynchronous Receiver-Transmitter manifest manifest-hal_nxp platform: NXP NXP

Projects

None yet

Milestone

v3.1.0

Development

Successfully merging this pull request may close these issues.

8 participants

@danieldegrasse @gmarull @dleach02 @carlescufi @henrikbrixandersen @hakehuang @tejlmand @zephyrbot