Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions boards/raspberrypi/rpi_pico2/rpi_pico2_rp2350a_m33.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ supported:
- adc
- clock
- counter
- crypto
- dma
- gpio
- hwinfo
Expand Down
1 change: 1 addition & 0 deletions drivers/crypto/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ zephyr_library_sources_ifdef(CONFIG_CRYPTO_MCUX_DCP crypto_mcux_dcp.c)
zephyr_library_sources_ifdef(CONFIG_CRYPTO_NPCX_SHA crypto_npcx_sha.c)
zephyr_library_sources_ifdef(CONFIG_CRYPTO_NRF_ECB crypto_nrf_ecb.c)
zephyr_library_sources_ifdef(CONFIG_CRYPTO_NXP_S32_HSE crypto_nxp_s32_hse.c)
zephyr_library_sources_ifdef(CONFIG_CRYPTO_RPI_PICO_SHA256 crypto_rpi_pico_sha256.c)
zephyr_library_sources_ifdef(CONFIG_CRYPTO_RTS5912_SHA crypto_rts5912_sha.c)
zephyr_library_sources_ifdef(CONFIG_CRYPTO_SI32 crypto_si32.c)
zephyr_library_sources_ifdef(CONFIG_CRYPTO_SMARTBOND crypto_smartbond.c)
Expand Down
1 change: 1 addition & 0 deletions drivers/crypto/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ source "drivers/crypto/Kconfig.mcux_dcp"
source "drivers/crypto/Kconfig.npcx"
source "drivers/crypto/Kconfig.nrf_ecb"
source "drivers/crypto/Kconfig.nxp_s32_hse"
source "drivers/crypto/Kconfig.rpi_pico"
source "drivers/crypto/Kconfig.rts5912"
source "drivers/crypto/Kconfig.si32"
source "drivers/crypto/Kconfig.smartbond"
Expand Down
10 changes: 10 additions & 0 deletions drivers/crypto/Kconfig.rpi_pico
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Copyright (c) 2025 TOKITA Hiroshi
# SPDX-License-Identifier: Apache-2.0

config CRYPTO_RPI_PICO_SHA256
bool "Raspberry Pi RP2 series SHA-256 Accelerator"
default y
depends on DT_HAS_RASPBERRYPI_PICO_SHA256_ENABLED
select PICOSDK_USE_SHA256
help
Enable driver for RP2 series SHA-256 accelerator
158 changes: 158 additions & 0 deletions drivers/crypto/crypto_rpi_pico_sha256.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/*
* Copyright (c) 2025 TOKITA Hiroshi
*
* SPDX-License-Identifier: Apache-2.0
*/

#define DT_DRV_COMPAT raspberrypi_pico_sha256

#include <zephyr/crypto/crypto.h>
#include <zephyr/kernel.h>
#include <zephyr/sys/util_macro.h>
#include <zephyr/sys/byteorder.h>

#include <pico/bootrom/lock.h>
#include <pico/sha256.h>

#include <zephyr/logging/log.h>
LOG_MODULE_REGISTER(crypto_rpi_pico_sha256, CONFIG_CRYPTO_LOG_LEVEL);

struct crypto_rpi_pico_sha256_data {
pico_sha256_state_t state;
struct k_mutex mutex;
};

static int crypto_rpi_pico_sha256_hash_handler(struct hash_ctx *ctx, struct hash_pkt *pkt,
bool finish)
{
struct crypto_rpi_pico_sha256_data *data = ctx->device->data;
int ret;

ret = k_mutex_lock(&data->mutex, K_FOREVER);
if (ret != 0) {
LOG_ERR("Failed to lock mutex: %d", ret);
return ret;
}

if (!data->state.locked) {
LOG_ERR("Invalid lock status: unlocked");
ret = -EINVAL;
goto end;
}

data->state.cache_used = 0;
data->state.cache.word = 0;
data->state.total_data_size = 0;
Comment on lines +43 to +45
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

State initialization should not be performed in the hash handler. These fields (cache_used, cache.word, total_data_size) should be initialized in hash_begin_session when the session starts, not at every hash operation. The current placement means multipart hashing can never work correctly even if implemented, as these fields would be reset on each call.

Suggested change
data->state.cache_used = 0;
data->state.cache.word = 0;
data->state.total_data_size = 0;
/* State initialization moved to session start (hash_begin_session) */

Copilot uses AI. Check for mistakes.

Comment on lines +43 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it OK to clear the state even if finish == false (multi step hash computation)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch.
It seems like we should also consider modifying the tests.
I'll look into it a bit.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like we should also consider modifying the tests.

While doing another review yesterday I realized that perhaps not all drivers support multipart hash computation. So in the end what you did might be already OK for the time being (you can extend that later if you prefer); please only add a check like the one done in that PR and I think it should be fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since there is an example of an implementation that is not supported, I will submit it using that implementation method for now.
I will support it soon.

sha256_err_not_ready_clear();
sha256_set_bswap(true);
sha256_start();
Comment on lines +47 to +49
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

Hardware initialization (sha256_err_not_ready_clear(), sha256_set_bswap(), sha256_start()) is performed on every hash operation call. This should be moved to hash_begin_session to initialize hardware once per session, not on every hash computation. This also prevents proper multipart hashing support in the future.

Copilot uses AI. Check for mistakes.

pico_sha256_update(&data->state, pkt->in_buf, pkt->in_len);
Copy link
Member

Choose a reason for hiding this comment

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

Are there limits on the input size by the HAL? If so, please test for them to return EINVAL

Copy link
Member Author

Choose a reason for hiding this comment

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

There are no particular restrictions, so it's not a problem.


if (!finish) {
LOG_ERR("Multipart hashing not supported yet");
ret = -ENOTSUP;
goto end;
}

pico_sha256_write_padding(&data->state);
sha256_wait_valid_blocking();

for (uint i = 0; i < 8; i++) {
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

The loop variable i should be declared as uint32_t instead of uint for consistency with Zephyr coding standards and to avoid potential portability issues. The uint type is not a standard C type.

Suggested change
for (uint i = 0; i < 8; i++) {
for (uint32_t i = 0; i < 8; i++) {

Copilot uses AI. Check for mistakes.
((uint32_t *)pkt->out_buf)[i] = BSWAP_32((uint32_t)sha256_hw->sum[i]);
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

[nitpick] The explicit cast (uint32_t) before passing to BSWAP_32 is redundant since BSWAP_32 already operates on and returns uint32_t. The cast can be removed for cleaner code: BSWAP_32(sha256_hw->sum[i])

Suggested change
((uint32_t *)pkt->out_buf)[i] = BSWAP_32((uint32_t)sha256_hw->sum[i]);
((uint32_t *)pkt->out_buf)[i] = BSWAP_32(sha256_hw->sum[i]);

Copilot uses AI. Check for mistakes.
}

ret = 0;

end:
k_mutex_unlock(&data->mutex);

return ret;
}

static int crypto_rpi_pico_sha256_query_hw_caps(const struct device *dev)
{
return CAP_SEPARATE_IO_BUFS | CAP_SYNC_OPS;
}

static int crypto_rpi_pico_sha256_hash_begin_session(const struct device *dev, struct hash_ctx *ctx,
enum hash_algo algo)
{
struct crypto_rpi_pico_sha256_data *data = dev->data;
int ret;

if (algo != CRYPTO_HASH_ALGO_SHA256) {
LOG_ERR("Unsupported algo: %d", algo);
return -EINVAL;
}

if (ctx->flags & ~(crypto_rpi_pico_sha256_query_hw_caps(dev))) {
LOG_ERR("Unsupported flag %x", ctx->flags);
return -EINVAL;
}

ret = k_mutex_lock(&data->mutex, K_FOREVER);
if (ret != 0) {
LOG_ERR("Failed to lock mutex: %d", ret);
return ret;
}

if (!bootrom_try_acquire_lock(BOOTROM_LOCK_SHA_256)) {
LOG_ERR("bootrom_try_acquire_lock failed");
ret = -EBUSY;
goto end;
}

data->state.locked = true;
ctx->hash_hndlr = crypto_rpi_pico_sha256_hash_handler;
ret = 0;

end:
k_mutex_unlock(&data->mutex);

return ret;
}

static int crypto_rpi_pico_sha256_hash_session_free(const struct device *dev, struct hash_ctx *ctx)
{
struct crypto_rpi_pico_sha256_data *data = dev->data;
int ret;

ret = k_mutex_lock(&data->mutex, K_FOREVER);
if (ret != 0) {
LOG_ERR("Failed to lock mutex: %d", ret);
return ret;
}

bootrom_release_lock(BOOTROM_LOCK_SHA_256);
data->state.locked = false;
ret = 0;

k_mutex_unlock(&data->mutex);

return ret;
}

static int crypto_rpi_pico_sha256_init(const struct device *dev)
{
struct crypto_rpi_pico_sha256_data *data = dev->data;

k_mutex_init(&data->mutex);

return 0;
}

static DEVICE_API(crypto, crypto_rpi_pico_sha256_crypto_api) = {
.query_hw_caps = crypto_rpi_pico_sha256_query_hw_caps,
.hash_begin_session = crypto_rpi_pico_sha256_hash_begin_session,
.hash_free_session = crypto_rpi_pico_sha256_hash_session_free,
};

#define CRYPTO_RPI_PICO_SHA256_INIT(idx) \
static struct crypto_rpi_pico_sha256_data crypto_rpi_pico_sha256_##idx##_data; \
DEVICE_DT_INST_DEFINE(idx, crypto_rpi_pico_sha256_init, NULL, \
&crypto_rpi_pico_sha256_##idx##_data, NULL, POST_KERNEL, \
CONFIG_CRYPTO_INIT_PRIORITY, &crypto_rpi_pico_sha256_crypto_api);

DT_INST_FOREACH_STATUS_OKAY(CRYPTO_RPI_PICO_SHA256_INIT)
8 changes: 8 additions & 0 deletions dts/bindings/crypto/raspberrypi,pico-sha256.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# Copyright (c) 2025 TOKITA Hiroshi
# SPDX-License-Identifier: Apache-2.0

description: RaspberryPi Pico SHA-256 accelerator

compatible: "raspberrypi,pico-sha256"

include: base.yaml
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

The binding should specify that the reg property is required, as the device tree node uses it (at address 0x400f8000). Similar crypto bindings (e.g., espressif,esp32-sha, intel,adsp-sha, ite,it8xxx2-sha) mark this property as required. Add:

properties:
  reg:
    required: true
Suggested change
include: base.yaml
include: base.yaml
properties:
reg:
required: true

Copilot uses AI. Check for mistakes.
6 changes: 6 additions & 0 deletions dts/vendor/raspberrypi/rpi_pico/rp2350.dtsi
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,12 @@
status = "disabled";
};

sha256: sha256@400f8000 {
compatible = "raspberrypi,pico-sha256";
reg = <0x400f8000 0x200>;
status = "disabled";
};

dma: dma@50000000 {
compatible = "raspberrypi,pico-dma";
reg = <0x50000000 DT_SIZE_K(64)>;
Expand Down
6 changes: 6 additions & 0 deletions modules/hal_rpi_pico/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ if(CONFIG_HAS_RPI_PICO)
zephyr_include_directories_ifdef(CONFIG_PICOSDK_USE_CLAIM
${common_dir}/hardware_claim/include)

zephyr_library_sources_ifdef(CONFIG_PICOSDK_USE_SHA256
${rp2_common_dir}/pico_sha256/sha256.c)
zephyr_include_directories_ifdef(CONFIG_PICOSDK_USE_SHA256
${rp2_common_dir}/hardware_sha256/include
${rp2_common_dir}/pico_sha256/include)

zephyr_include_directories_ifdef(CONFIG_RISCV
${common_dir}/pico_sync/include
${common_dir}/pico_time/include
Expand Down
5 changes: 5 additions & 0 deletions modules/hal_rpi_pico/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,8 @@ config PICOSDK_USE_RTC
bool
help
Use the RTC driver from pico-sdk

config PICOSDK_USE_SHA256
bool
help
Use the SHA-256 driver and utilities from pico-sdk
9 changes: 9 additions & 0 deletions tests/crypto/crypto_hash/socs/rp2350a_m33.overlay
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright (c) 2025 TOKITA Hiroshi
*
* SPDX-License-Identifier: Apache-2.0
*/

&sha256 {
status = "okay";
};
2 changes: 2 additions & 0 deletions tests/crypto/crypto_hash/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#define CRYPTO_DEV_COMPAT espressif_esp32_sha
#elif DT_HAS_COMPAT_STATUS_OKAY(nxp_s32_crypto_hse_mu)
#define CRYPTO_DEV_COMPAT nxp_s32_crypto_hse_mu
#elif DT_HAS_COMPAT_STATUS_OKAY(raspberrypi_pico_sha256)
#define CRYPTO_DEV_COMPAT raspberrypi_pico_sha256
#else
#error "You need to enable one crypto device"
#endif
Expand Down
1 change: 1 addition & 0 deletions tests/crypto/crypto_hash/testcase.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ tests:
platform_allow:
- native_sim
- nucleo_u575zi_q
- rpi_pico2/rp2350a/m33
integration_platforms:
- native_sim
- nucleo_u575zi_q
Expand Down
Loading