Skip to content

Conversation

@fimohame
Copy link
Contributor

This commit enables the pm device driver support for the spi_silabs_siwx91x_gspi driver.

@fimohame fimohame force-pushed the device_pm_silabs_siwx91x_gspi branch from acb2e26 to 065dc37 Compare August 12, 2025 09:19
return ret;
}

cfg->reg->GSPI_CONFIG1_b.GSPI_MANUAL_CSN = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

What about setting the high-performance enable bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked it wasn't affecting the functionality as such

Copy link
Contributor

Choose a reason for hiding this comment

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

It will affect functionality when operating at high speed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have addressed it.

@fimohame fimohame force-pushed the device_pm_silabs_siwx91x_gspi branch from 065dc37 to 7e30b65 Compare August 12, 2025 09:32
@fimohame fimohame marked this pull request as ready for review August 12, 2025 09:42
@fimohame fimohame requested a review from smalae August 12, 2025 09:42
@zephyrbot zephyrbot added area: SPI SPI bus platform: Silabs Silicon Labs labels Aug 12, 2025
smalae
smalae previously requested changes Aug 12, 2025
return ret;
}

cfg->reg->GSPI_CONFIG1_b.GSPI_MANUAL_CSN = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

It will affect functionality when operating at high speed

@fimohame fimohame force-pushed the device_pm_silabs_siwx91x_gspi branch from 7e30b65 to 9dd86ee Compare August 12, 2025 09:51
@fimohame fimohame requested a review from smalae August 12, 2025 09:51
@fimohame fimohame force-pushed the device_pm_silabs_siwx91x_gspi branch 2 times, most recently from e5de48e to 704902a Compare August 12, 2025 10:05
@fimohame fimohame force-pushed the device_pm_silabs_siwx91x_gspi branch from 704902a to 08f5860 Compare August 12, 2025 10:22
@fimohame fimohame force-pushed the device_pm_silabs_siwx91x_gspi branch 2 times, most recently from d40cc0f to d0e2bd4 Compare August 19, 2025 06:29
@sonarqubecloud
Copy link

@decsny decsny removed their request for review August 20, 2025 01:34
@fimohame fimohame marked this pull request as draft September 24, 2025 06:50
@fimohame fimohame force-pushed the device_pm_silabs_siwx91x_gspi branch 2 times, most recently from 03b0e31 to 24e143f Compare September 24, 2025 21:22
@fimohame fimohame marked this pull request as ready for review September 25, 2025 07:24

if (status >= 0 && status != DMA_STATUS_COMPLETE) {
return;
goto pm_put_exit;
Copy link
Member

Choose a reason for hiding this comment

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

you don't want to put the device runtime if the DMA transfer is not completed. in that case, you could turn off the power domain and then killed the working uDMA.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed


if (!spi_siwx91x_is_dma_enabled_instance(dev) && asynchronous) {
ret = -ENOTSUP;
goto pm_put_exit;
Copy link
Member

Choose a reason for hiding this comment

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

you will never call the "pm_device_runtime_put" since you are in the case of an asynchronous transfer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed

if (ret) {
spi_context_release(&data->ctx, ret);
return ret;
goto pm_put_exit;
Copy link
Member

Choose a reason for hiding this comment

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

same here in the case you are in an asynchronous transfer

@fimohame fimohame force-pushed the device_pm_silabs_siwx91x_gspi branch from 24e143f to dd1fc5e Compare October 17, 2025 05:53
@fimohame fimohame force-pushed the device_pm_silabs_siwx91x_gspi branch from dd1fc5e to cbfa627 Compare October 17, 2025 07:09
@Martinhoff-maker
Copy link
Member

Martinhoff-maker commented Oct 17, 2025

Since SPI will use GPDMA instead of uDMA with #97534, I think we need to implement PM in GPDMA driver in order to have PM properly working with SPI.
Other than that, LGTM now but would like to test with GPDMA and PM.

}

if (!asynchronous) {
pm_device_runtime_put(dev);
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I'm misunderstanding something, this doesn't seem correct to me. If DMA is used for a blocking transfer, won't the runtime_put already have happened in the DMA complete callback? It seems to me like this should be done inside the else above, and not be related to asynchronous at all.

Copy link
Contributor Author

@fimohame fimohame Oct 22, 2025

Choose a reason for hiding this comment

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

Your correct. It's been addressed now

This commit enables the pm device driver support
for the spi_silabs_siwx91x_gspi driver.

Signed-off-by: S Mohamed Fiaz <[email protected]>
@fimohame fimohame force-pushed the device_pm_silabs_siwx91x_gspi branch from cbfa627 to a0af92e Compare October 22, 2025 10:55
@fimohame fimohame requested a review from asmellby October 22, 2025 11:03
@sonarqubecloud
Copy link

@jhedberg jhedberg dismissed smalae’s stale review October 22, 2025 13:29

Seems like the raised issue has been addressed

@cfriedt cfriedt merged commit bae6364 into zephyrproject-rtos:main Oct 22, 2025
26 checks passed
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.

8 participants