Skip to content

Conversation

@gmarull
Copy link
Member

@gmarull gmarull commented Oct 23, 2024

This PR adds a new API to request/release GD domains, so that some identified issues can be handled.

WIP, subject to change. This also requires a new nrf-regtool release.

@gmarull gmarull marked this pull request as ready for review October 24, 2024 11:41
@gmarull gmarull changed the title soc: nrf54h20: add support for requesting/releasing GD domains soc: nrf54h20: fix some power consumption issues by managing GD domains on certain scenarios Oct 24, 2024
@gmarull gmarull assigned anangl and unassigned ceolin Oct 24, 2024
Copy link
Member

@hubertmis hubertmis left a comment

Choose a reason for hiding this comment

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

I think we also need something in pinctrl similar to what is implemented in gpio_nrfx.c. We need to safely control fast pins when fast peripherals get deactivated. It requires GPIO RETAIN feature

Copy link
Contributor

Choose a reason for hiding this comment

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

This retention bit still needs to be cleared again at some point when the pin is going back into active use either via the gpio peripheral or the peripheral owning the pin e.g. spim.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the retention should be cleared between ret = nrf_gpd_request(NRF_GPD_FAST_ACTIVE1); gets applied and the first attempt to reconfigure pins (if (flags & GPIO_OUTPUT_INIT_HIGH) { nrf_gpio_port_out_set(cfg->port, BIT(pin));).

If pins are reconfigured in any other function in this module, it should handle retention in a similar way.

@gmarull gmarull requested a review from anangl October 30, 2024 17:37
anangl
anangl previously approved these changes Oct 30, 2024
karstenkoenig
karstenkoenig previously approved these changes Oct 31, 2024
Add binding for Global Power Domain found in nRF54Hx SoCs.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Add the global power domain entry. This domain is not memory-mapped
but controlled using NRFS services.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
So that it can be used to manually control certain power domains.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Add a new soc-level API that allows to manually request/release global
power domains.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
This patch introduces a new flag to indicate if a peripheral belongs
to FAST_ACTIVE1 domain. This way, pinctrl knows when to request the
SLOW_ACTIVE domain (where CTRLSEL multiplexer resides).

Signed-off-by: Gerard Marull-Paretas <[email protected]>
It is required for some pinctrl changes.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
This patch allows to _safely_ configure GPIO ports that have their pad
on FAST_ACTIVE1 domain.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
So that we do not get false warnings about consistent spacing around
'*'.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
This API needs to be called by FAST peripherals before/after
disabling/enabling them.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
When GPD is managed by pinctrl, pins retention needs to be controlled by
the driver to avoid glitches.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
It is enabled by default if we enable device PM, but we do not want
this, otherwise we get linker errors (PM subsys, fun guaranteed!).

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Copy link
Member

@hubertmis hubertmis left a comment

Choose a reason for hiding this comment

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

The GPIO IN is most probably not handled yet. But I think we can add it in a next PR.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how RETAIN works with input pins. It's safer to apply it to output for now. We can figure out later how to correctly handle input

Copy link
Member

Choose a reason for hiding this comment

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

I'm trying to figure out the power domain management required for the input pins.

It seems that gpio_nrfx_pin_interrupt_configure() should be enhanced:

If the

if (!(BIT(pin) & cfg->edge_sense) &&
	    (mode == GPIO_INT_MODE_EDGE) &&
	    (nrf_gpio_pin_dir_get(abs_pin) == NRF_GPIO_PIN_DIR_INPUT))

is met and the driver uses event IN, the relevant power domain should be kept powered up (nrf_gpd_request()). Alternatively, if the driver uses event SENSE, the GPIO.RETAIN should be applied to the pin (cfg->port->RETAINSET = mask).

However, I'm not sure in which procedure this configuration should be cleared. @nordic-krch, could you help to identify the correct place?

Copy link
Contributor

@karstenkoenig karstenkoenig left a comment

Choose a reason for hiding this comment

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

This wouldn't work for pins defined as input on the fast ports, but that was broken before so probably easier to sort in follow up PR as suggest by Hubert to avoid complicating this any further

@carlescufi carlescufi added this to the v4.0.0 milestone Oct 31, 2024
@carlescufi carlescufi added the bug The issue is a bug, or the PR is fixing a bug label Oct 31, 2024
@mmahadevan108 mmahadevan108 merged commit 969326b into zephyrproject-rtos:main Nov 1, 2024
27 checks passed
@gmarull gmarull deleted the nrf54h20-gpd branch November 4, 2024 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Coding Guidelines Coding guidelines and style area: GPIO area: Pinctrl area: Power Management area: UART Universal Asynchronous Receiver-Transmitter bug The issue is a bug, or the PR is fixing a bug platform: nRF Nordic nRFx

Projects

None yet

Development

Successfully merging this pull request may close these issues.