DTS: Convert SAM UART/USART to DT_INST macros (plus pinctrl support) #24319

galak · 2020-04-13T18:49:31Z

Convert Atmel SAM UART & USART to the new DT_INST macros and as part of this conversion we introduce basic pinctrl support for the ATMEL SAM.

zephyrbot · 2020-04-13T18:52:15Z

Some checks failed. Please fix and resubmit.

checkpatch issues

-:816: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#816: FILE: dts/arm/atmel/pinctrl_atmel_sam.h:38:
+#define DT_ATMEL_PIN(inst, signal, port, pin, periph) \
+	p##port##pin##periph##_##inst##_##signal: \
+	p##port##pin##periph##_##inst##_##signal { \
+	atmel,pins = < &pio##port pin PERIPH_##periph >; }

-:817: ERROR:SPACING: spaces required around that ':' (ctx:VxE)
#817: FILE: dts/arm/atmel/pinctrl_atmel_sam.h:39:
+	p##port##pin##periph##_##inst##_##signal: \
 	                                        ^

-:819: ERROR:SPACING: space required after that ',' (ctx:VxV)
#819: FILE: dts/arm/atmel/pinctrl_atmel_sam.h:41:
+	atmel,pins = < &pio##port pin PERIPH_##periph >; }
 	     ^

-:819: ERROR:SPACING: spaces required around that '>' (ctx:WxO)
#819: FILE: dts/arm/atmel/pinctrl_atmel_sam.h:41:
+	atmel,pins = < &pio##port pin PERIPH_##periph >; }
 	                                              ^

- total: 3 errors, 1 warnings, 1524 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

nandojve · 2020-04-18T13:12:58Z

All Atmel drivers need use nodelabel. I'll test this PR on top of #24110.

galak · 2020-04-18T13:31:14Z

All Atmel drivers need use nodelabel. I'll test this PR on top of #24110.

Why?

nandojve · 2020-04-18T14:32:19Z

All Atmel drivers need use nodelabel. I'll test this PR on top of #24110.

Why?

Re worked to use DT_NODELABEL

static const struct uart_sam_dev_cfg uart##n##_sam_config = {		\
	.regs = (Uart *)DT_REG_ADDR(DT_NODELABEL(uart##n)),		\
	.periph_id = UART_PROP(n, peripheral_id),			\
	.pin_rx = DT_ATMEL_SAM_PIN(DT_NODELABEL(uart##n), 0),		\
	.pin_tx = DT_ATMEL_SAM_PIN(DT_NODELABEL(uart##n), 1),		\
	UART_SAM_IRQ_CALLBACK(n)					\
};									\

The above was tested and it works.

This is an old discussion, people questioned about it on the devicetree slack channel at 2020-04-17, inclusive. Atmel drivers don't work using DT_INST without a property to handle the HW instance. It can't instantiate the proper HW instance using the node instance itself, it simple don't match with HW. If you can't follow, try create a non regular case like disabling USART0 and then try enable only USART1 on SAM_V71_XULT, for instance, then you will see the problem. When you disable USART0 the USART1 will be DT_INST___0 and HW will be wrongly configured.

I already sent a patch that can address this but was declined in favor to use DT_NODELABEL without a technical answer at #23335. The #23338, #23332, #23359 are examples how to use HW instance + DT_INST based on #23335 but were closed in favor to use DT_NODELABEL.

I'm adding people that questioned on slack to see this case. I think is important everyone have this pretty clear.

@pabigot @mnkp @mbolivar

That said, I've been using since than what seems to be the right solution and sent the #24110 as first PR to collect feedback to continue Atmel drivers conversion.

stephanosio · 2020-04-18T14:51:05Z

@nandojve This PR adds pinctrl support and uses that to retrieve pin numbers from the device tree of each U(S)ART instance, so no need to use node label anymore.

#24110 (review)

stephanosio

@galak Thanks for doing this. Now we finally have a proper I/O handling framework in the device tree for SAM series SoC.

nandojve · 2020-04-18T15:13:11Z

@nandojve This PR adds pinctrl support and uses that to retrieve pin numbers from the device tree of each U(S)ART instance, so no need to use node label anymore.

#24110 (review)

Ohh boy! I was stuck on that and didn't realize!
I tested again and it worked with sam_v71_xult.

pabigot

After staring at this long enough I think it makes sense and looks clean. One anomaly in the documentation for the yaml (seems to cross compatibles).

dts/bindings/serial/atmel,sam-uart.yaml

dts/arm/atmel/pinctrl_atmel_sam.h

dts/arm/atmel/sam3x-pinctrl.dtsi

soc/arm/atmel_sam/common/soc_gpio.h

The atmel,pins property will be utilized to describe pin mux configuration. The property will be a phandle-array in which the phandle points to the given GPIO port the pin is on, the pin number, and the mux configuration. This change updates the atmel,sam-gpio binding to support that phandle-array and updates the associated SoC dtsi files as well. Signed-off-by: Kumar Gala <[email protected]>

dts/bindings/serial/atmel,sam-usart.yaml

nandojve

I don't see anymore and removed the DNM.

Add pinctl support for the SAM UART and SAM USART devices. We update the UART and USART bindings to have pinctrl-0 bindings that are expected to have 2 phandles to the RX & TX pinctrl nodes. The pinctrl nodes will have an 'atmel,pins' property that describes the GPIO port, pin and periphal configuration for that pin. We add sam*-pinctrl.dtsi files with all the various pin ctrl configuration operations supported by the given SoC family. These files are based on data extracted from the Atmel ASF HAL (in include/sam<FAMILY>/pio/*.h). Signed-off-by: Kumar Gala <[email protected]>

Add a set of macros that will create a struct soc_gpio_pin initialization based on data extracted from device tree. This should allow replacing the static data in soc_pinmap.h with data coming from devicetree instead. Signed-off-by: Kumar Gala <[email protected]>

Reworked uart_sam driver to utilize new DT_INST macros as part of this rework we also now get pin ctrl/mux configuration information from the device tree instead of via Kconfig and defines in soc_pinmap.h We remove defines from dts_fixup.h and soc_pinmap.h and associated Kconfig symbols that are no longer needed due to getting all that information from devicetree. Signed-off-by: Kumar Gala <[email protected]>

Reworked usart_sam driver to utilize new DT_INST macros as part of this rework we also now get pin ctrl/mux configuration information from the device tree instead of via Kconfig and defines in soc_pinmap.h We remove defines from dts_fixup.h and soc_pinmap.h and associated Kconfig symbols that are no longer needed due to getting all that information from devicetree. Signed-off-by: Kumar Gala <[email protected]>

galak requested review from fallrisk, ioannisg, MaureenHelm and nandojve as code owners April 13, 2020 18:49

galak requested review from mnkp and erwango April 13, 2020 18:49

zephyrbot added area: Devicetree area: Boards labels Apr 13, 2020

galak mentioned this pull request Apr 14, 2020

Convert SAM SoC SERIAL drivers to DT macros #24110

Closed

nandojve added the DNM This PR should not be merged (Do Not Merge) label Apr 18, 2020

stephanosio approved these changes Apr 18, 2020

View reviewed changes

pabigot self-requested a review April 18, 2020 15:27

pabigot reviewed Apr 18, 2020

View reviewed changes

dts/bindings/serial/atmel,sam-uart.yaml Outdated Show resolved Hide resolved

nandojve requested changes Apr 18, 2020

View reviewed changes

dts/arm/atmel/pinctrl_atmel_sam.h Show resolved Hide resolved

dts/arm/atmel/sam3x-pinctrl.dtsi Show resolved Hide resolved

soc/arm/atmel_sam/common/soc_gpio.h Outdated Show resolved Hide resolved

soc/arm/atmel_sam/common/soc_gpio.h Outdated Show resolved Hide resolved

galak force-pushed the dt-sam-inst-uart branch from 65fb348 to 6d4ab5c Compare April 18, 2020 17:33

galak force-pushed the dt-sam-inst-uart branch from 6d4ab5c to 6e88f3f Compare April 18, 2020 19:20

galak requested review from nandojve and pabigot April 18, 2020 19:20

nandojve requested changes Apr 18, 2020

View reviewed changes

dts/bindings/serial/atmel,sam-usart.yaml Outdated Show resolved Hide resolved

nandojve added platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) and removed DNM This PR should not be merged (Do Not Merge) labels Apr 18, 2020

nandojve approved these changes Apr 18, 2020

View reviewed changes

galak added 3 commits April 18, 2020 15:08

galak force-pushed the dt-sam-inst-uart branch from 6e88f3f to c59fa81 Compare April 18, 2020 20:08

nandojve mentioned this pull request Apr 18, 2020

Convert SAM SoC drivers to DT_INST #23107

Closed

61 tasks

galak merged commit e7c7f91 into zephyrproject-rtos:master Apr 18, 2020

galak deleted the dt-sam-inst-uart branch April 18, 2020 22:04

nandojve mentioned this pull request Apr 21, 2020

DTS: Convert SAM TWI/TWIHS I2C to DT_INST macros #24506

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DTS: Convert SAM UART/USART to DT_INST macros (plus pinctrl support) #24319

DTS: Convert SAM UART/USART to DT_INST macros (plus pinctrl support) #24319

galak commented Apr 13, 2020

zephyrbot commented Apr 13, 2020 •

edited

Loading

nandojve commented Apr 18, 2020

galak commented Apr 18, 2020

nandojve commented Apr 18, 2020 •

edited

Loading

stephanosio commented Apr 18, 2020 •

edited

Loading

stephanosio left a comment

nandojve commented Apr 18, 2020

pabigot left a comment

nandojve left a comment

DTS: Convert SAM UART/USART to DT_INST macros (plus pinctrl support) #24319

DTS: Convert SAM UART/USART to DT_INST macros (plus pinctrl support) #24319

Conversation

galak commented Apr 13, 2020

zephyrbot commented Apr 13, 2020 • edited Loading

checkpatch issues

nandojve commented Apr 18, 2020

galak commented Apr 18, 2020

nandojve commented Apr 18, 2020 • edited Loading

stephanosio commented Apr 18, 2020 • edited Loading

stephanosio left a comment

Choose a reason for hiding this comment

nandojve commented Apr 18, 2020

pabigot left a comment

Choose a reason for hiding this comment

nandojve left a comment

Choose a reason for hiding this comment

zephyrbot commented Apr 13, 2020 •

edited

Loading

nandojve commented Apr 18, 2020 •

edited

Loading

stephanosio commented Apr 18, 2020 •

edited

Loading