Skip to content

Conversation

@danieldegrasse
Copy link
Contributor

@danieldegrasse danieldegrasse commented Mar 8, 2022

This PR enables pinctrl for all drivers used by NXP Kinetis SOCs, and disables pinmux drivers for in tree Kinetis boards. The in tree pinmux driver is still present, but is no longer enabled at the SOC level. All in tree Kinetis boards should now be converted for pinctrl support.

Dependent on #42238 for pinctrl support in the MCUX LPUART driver.

@danieldegrasse
Copy link
Contributor Author

@hakehuang Could you run CI on this PR, specifically targeting Kinetis boards? I want to make sure all the pinmux settings are correct.

@hakehuang
Copy link
Contributor

@danieldegrasse CI is running will feedback tomorrow.

Copy link
Member

@henrikbrixandersen henrikbrixandersen 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, thank you. I know it's still in draft, but I took the liberty to review it anyway :)

I noticed that in some drivers you conditionally use pinctrl, others you always use it regardless if it is enabled or not. I take it the conditional ones are due to the same drivers being in use in other SoC families (e.g. i.MX RT)?

Anyways, for drivers that unconditionally use the pinctrl API, I think the Kconfig symbols needs to depend on CONFIG_PINCTRL?

@hakehuang
Copy link
Contributor

@danieldegrasse on frdm_k64f I meet two driver test error.

  1. tests/driver/adc. failure log is
    adc_failure_log.txt
  2. tests/driver/spi/spi_loopback. failure log is
    spi_failure_log.txt

for spi case, you need short the sin and sout which are: ptd2 - ptd3

the two above cases are OK in mainline weekly testing which is zephyr-v3.0.0-838-gc050d1f2ec.

@danieldegrasse
Copy link
Contributor Author

danieldegrasse commented Mar 10, 2022

@henrikbrixandersen Thanks for the feedback. That is why some drivers have conditional pinctrl enablement- once full iMX.RT support is merged, that should be dropped.

As far as the Kconfig symbols, I think that rather than depending on CONFIG_PINCTRL the drivers should select CONFIG_PINCTRL. This way, a new user to the project doesn't need to debug Kconfig symbols to understand why a driver isn't being included.

@danieldegrasse
Copy link
Contributor Author

@hakehuang I reproduced and fixed the ADC test failure, but I cannot reproduce the SPI failure. I tried testing with and without the mcux-dspi-dma.conf overlay applied, and in both cases the test passed when ptd2 and ptd3 were shorted. However, the pinctrl driver did have an error regarding the setting of the slew rate (SRE field), where it incorrectly set SRE=1 when a fast slew rate was selected, so I've fixed that.

@hakehuang
Copy link
Contributor

@hakehuang I reproduced and fixed the ADC test failure, but I cannot reproduce the SPI failure. I tried testing with and without the mcux-dspi-dma.conf overlay applied, and in both cases the test passed when ptd2 and ptd3 were shorted. However, the pinctrl driver did have an error regarding the setting of the slew rate (SRE field), where it incorrectly set SRE=1 when a fast slew rate was selected, so I've fixed that.

I will re-run CI based your updates, and feedback

@hakehuang
Copy link
Contributor

hakehuang commented Mar 14, 2022

I will re-run CI based your updates, and feedback

@danieldegrasse All CI testing PASS. except frdm_kl25z

@danieldegrasse
Copy link
Contributor Author

@hakehuang I have updated the LPSCI UART peripheral driver for the KL25Z to support pinctrl, which should fix the CI failure. Can you retry CI on that board?

@hakehuang
Copy link
Contributor

hakehuang commented Mar 15, 2022

have updated the LPSCI UART peripheral driver for the KL25Z to support pinctrl, which should fix the CI failure. Can you retry CI on that board?

running. @danieldegrasse all PASS.

@danieldegrasse danieldegrasse marked this pull request as ready for review March 15, 2022 15:21
Remove unnecessary usage of pinmux driver for kinetis boards with
ethernet

Signed-off-by: Daniel DeGrasse <[email protected]>
Enable pinctrl support for mcux flexcan driver

Signed-off-by: Daniel DeGrasse <[email protected]>
Remove flexcan pinmux configuration for kinetis boards, as flexcan
driver uses pinctrl

Signed-off-by: Daniel DeGrasse <[email protected]>
Enable pinctrl for kinetis adc16 mcux driver

Signed-off-by: Daniel DeGrasse <[email protected]>
adc16 driver supports pinctrl, so remove unused pinmux from kinetis
boards

Signed-off-by: Daniel DeGrasse <[email protected]>
Enable pinctrl for kinetis lpsci uart driver

Signed-off-by: Daniel DeGrasse <[email protected]>
Remove lpuart pinmux setting for all kinetis boards, as lpuart driver
supports pinctrl.

Signed-off-by: Daniel DeGrasse <[email protected]>
Enable pinctrl for mcux lpi2c driver

Signed-off-by: Daniel DeGrasse <[email protected]>
remove pinmux for lpi2c devices on kinetis boards, as these boards
support pinctrl

Signed-off-by: Daniel DeGrasse <[email protected]>
Enable pinctrl for lpspi driver

Signed-off-by: Daniel DeGrasse <[email protected]>
Add dts binding and pinctrl definition for flexio in twr_ke18f. This
allows ke18f pinmux code to apply pinctrl selections for flexio.

Signed-off-by: Daniel DeGrasse <[email protected]>
LPSPI peripheral driver supports pinctrl. Move twr_ke18f pinmux to use
pinmux for lpspi, and apply dynamic pinctrl states in order to select
correct chip select pin.

Also add pinmux settings to LPSPI for RT1060, so that LPSPI peripheral
driver will continue to work.

Signed-off-by: Daniel DeGrasse <[email protected]>
Enable pinctrl support for mcux pwm capture pwt driver

Signed-off-by: Daniel DeGrasse <[email protected]>
Remove pinmux configuration for kinetis boards using pwt, as this driver
supports pinctrl

Signed-off-by: Daniel DeGrasse <[email protected]>
enable pinctrl for adc12 driver

Signed-off-by: Daniel DeGrasse <[email protected]>
Remove pinmux configuration for kinetis boards using adc12, as pinctrl
is supported by driver

Signed-off-by: Daniel DeGrasse <[email protected]>
Add pinctrl support for mcux acmp sensor driver

Signed-off-by: Daniel DeGrasse <[email protected]>
Remove pinmux usage for acmp peripheral on kinetis boards, as mcux_acmp
driver supports pinctrl

Signed-off-by: Daniel DeGrasse <[email protected]>
add pinctrl support to dac_mcux_dac32 driver

Signed-off-by: Daniel DeGrasse <[email protected]>
dac32 driver supports pinctrl. Remove pinmux usage on kinetis boards
with this IP.

Signed-off-by: Daniel DeGrasse <[email protected]>
add pinctrl support for kinetis tpm driver

Signed-off-by: Daniel DeGrasse <[email protected]>
tpm driver supports pinctrl. Remove pinmux usage for kinetis boards.

Signed-off-by: Daniel DeGrasse <[email protected]>
Remove pinmux file where possible, and remove all pinmux driver usage
where file cannot be removed. All kinetis boards no longer use pinmux
driver.

Signed-off-by: Daniel DeGrasse <[email protected]>
disable pinmux driver for nxp kinetis SOCs, since all boards use pinctrl

Signed-off-by: Daniel DeGrasse <[email protected]>
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, great work

@hakehuang hakehuang self-requested a review March 22, 2022 04:48
@carlescufi carlescufi merged commit 8979f87 into zephyrproject-rtos:main Mar 22, 2022
@danieldegrasse danieldegrasse deleted the kinetis-full-pinctrl branch March 22, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants