Skip to content

rpi_pico: Fix DTC warnings #53431

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.

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

Merged
merged 2 commits into from
Jan 3, 2023

Conversation

yonsch
Copy link
Collaborator

@yonsch yonsch commented Jan 3, 2023

This PR fixes two DTC warnings that have been present for some time:

Warning (unique_unit_address_if_enabled): /soc/pin-controller@40014000: duplicate unit-address (also used in node /soc/gpio@40014000)

and:

unit address and first address in 'reg' (0x10000000) don't match for /soc/flash-controller@10000000/flash@0

The first was caused by the pinctrl and gpio bank sharing the same unit address. The RP2040 doesn't have an actual pin control unit, so it does not need a unit address.
The second was caused by a conflict of the unit address of the flash as defined by the SoC and the board. There wasn't a clear distinction between the flash controller and the flash itself. This was fixed for the SoC, board and driver.

The pinctrl node of the RP2040 had the same unit address as the GPIO
bank, causing a DTC warning. To fix this, the pinctrl's address was
removed, as it does not require any.

Signed-off-by: Yonatan Schachter <[email protected]>
@yonsch yonsch added the platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico) label Jan 3, 2023
@de-nordic
Copy link
Collaborator

Not an RPI pico expert and got some questions:

  1. the DTS sets some chosen values that reference specific flash and flash controller, so should the code follow these chosen values?
  2. or the chosen are set for Zephyr but the code can only support the specific flash and flash-controller so it is better to address them directly, rather than via chosen?

The addresses of the flash and flash controller of the RP2040
SoC were mixed up. There was no clear distinction between the
flash and the flash controller, which was unclear but also
caused a DTC warning.
This commit makes the distinction clearer: The SSI peripheral at
0x18000000 is the flash controller, and the flash itself starts
at 0x10000000. The flash driver and rpi_pico.dts were fixed
accordingly.

Signed-off-by: Yonatan Schachter <[email protected]>
@yonsch yonsch force-pushed the pico_dts_warnings branch from 5fcab80 to d570422 Compare January 3, 2023 09:45
@yonsch
Copy link
Collaborator Author

yonsch commented Jan 3, 2023

@de-nordic The RP2040 uses an external flash, that is always mapped to 0x10000000. The HAL code addresses that directly. With Zephyr, I suppose using chosen to select that flash is more appropriate, so I fixed the driver as you suggested.

@de-nordic
Copy link
Collaborator

@de-nordic The RP2040 uses an external flash, that is always mapped to 0x10000000. The HAL code addresses that directly. With Zephyr, I suppose using chosen to select that flash is more appropriate, so I fixed the driver as you suggested.

I have accepted the solution as is. But still have some questions that may require some discussion and addressing.

So the HAL determines how this works, the DTS only describes the state for the Zephyr.
Does HAL use DTS to get these addresses or has them hardcoded somehow?
Will changing DTS will affect how HAL sees the flash, or it will only change how Zephyr (and Zephyr compilation) sees the underlying hardware?

If HAL exports the configuration via some header, shouldn't it be used in the driver rather than the DTS?
Assuming that HAL does not use DTS: this is RPI specific driver, for SoC integral controller, shouldn't the driver use the same identifiers as HAL does?
Just wondering...

@carlescufi carlescufi merged commit abe50cc into zephyrproject-rtos:main Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Flash platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants