drivers: flash: Addition of MCHP g3 driver by sr-mchp · Pull Request #109747 · zephyrproject-rtos/zephyr

sr-mchp · 2026-05-25T07:25:12Z

Addition of device tree nodes for supporting flash peripheral for PIC32CZ_CA series of devices.
Flash G3 driver implementation.
Update to the board metadata to list flash partitions and sections to support pic32cz_ca80 and pic32cz_ca90 series of boards.
Addition of board overlay files to enable flash tests.

nandojve

Overall seems fine.
I only recommend you do another pass and separate distinct logical blocks to make a easy reading.

nandojve · 2026-05-26T16:37:35Z

 	soc {
-		flash0: flash@8000000 {
-			reg = <0x08000000 DT_SIZE_M(1)>;
+		nvmctrl: nvmctrl@44002000 {


Hi,

Fix all the commit messages:

brief: 52 chars
description: <=72

dts: arm: microchip: NVMCTRL/Flash G3 driver Added device tree nodes and binding files for supporting NVMCTRL and Flash peripherals for PIC32CZ_CA series of devices.

nandojve · 2026-05-26T16:41:46Z

+static inline const struct flash_config *get_flash_config(const struct device *dev)
+{
+	if (dev->ops.init != NULL) {
+#if MCHP_CHOSEN_FLASH_COVERS_ALL


There is ways in zephyr to check if the app chose a bootloader.
Could that works here? Is there other use cases that are not only a special bootloader usecase?

nandojve · 2026-05-26T16:46:08Z

+	}
+	k_sem_take(&data->mutex_lock, K_FOREVER);
+	(void)memcpy(data_buff, (uint32_t *)(config->base_addr + offset), no_of_bytes);
+	k_sem_give(&data->mutex_lock);


Is easy to read when you separate these two distinct logical blocks.

} + k_sem_take(&data->mutex_lock, K_FOREVER); (void)memcpy(data_buff, (uint32_t *)(config->base_addr + offset), no_of_bytes); k_sem_give(&data->mutex_lock);

I mean, the below is not tight with the previous operation.
You could apply this in other places to make this distinction.

nandojve · 2026-05-26T16:48:25Z

+#define FLASH_MCHP_G3_CONFIG_PAGE_LAYOUT(n)
+#endif /* CONFIG_FLASH_PAGE_LAYOUT */
+
+#define MCHP_MAX(a, b) (((a) > (b)) ? (a) : (b))


In zephyr we define many util functions in include/sys/. I recommend a visit there to check the options.

zephyr/include/zephyr/sys/util.h

Line 391 in cdb5827

#define MAX(a, b) Z_INTERNAL_MAX(a, b)

nandojve · 2026-05-26T16:51:30Z

+      - pic32cz_ca80_cult
+      - pic32cz_ca90_cult


The main goal is make sure that NVMCTRL_G3 is tested. No need to add 2 devices that share the same driver.

sunil-abraham

Driver modification needed from an architectural perspective.

sunil-abraham · 2026-05-27T11:54:42Z

+		/* App build: zephyr,flash (flash1) covers all partitions.
+		 * Redirect to the child device (same as old sanitize_dev).
+		 */
+		const struct device *chosen = DEVICE_DT_GET(DT_CHOSEN(zephyr_flash));


Using DT_CHOSEN(zephyr_flash) inside a flash driver does not make architectural sense. Please refactor the code.

sunil-abraham · 2026-05-27T11:54:47Z

+
+		return (const struct flash_config *)chosen->config;
+#else
+		/* MCUboot build: zephyr,flash (flash0/BFM) is too small.


Driver should not depend on the application using it. Please modify the design.

Devicetree & binding files for NVMCTRL PIC32CZ_CA series of devices Signed-off-by: Shankar Ramasamy <shankar.ramasamy@microchip.com>

NVMCTRL G3 driver implementation to support PIC32CZ_CA device series Signed-off-by: Shankar Ramasamy <shankar.ramasamy@microchip.com>

Add Flash partitions and board metadata for pic32cz_ca80/90_cult boards Signed-off-by: Shankar Ramasamy <shankar.ramasamy@microchip.com>

Add board support files for pic32cz_ca80_cult to enable Flash test cases Signed-off-by: Shankar Ramasamy <shankar.ramasamy@microchip.com>

sonarqubecloud · 2026-06-03T16:27:40Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

JarmouniA · 2026-06-03T16:01:08Z


 config FLASH_MCHP_NVMCTRL_G1
-	bool "Microchip G1 Flash Driver for NVMCTRL"
+	bool


Why remove the prompt?

JarmouniA · 2026-06-03T16:01:24Z

 	  space, not just at aligned addresses.
+
+config FLASH_MCHP_NVMCTRL_G3
+	bool


shouldn't be hidden

JarmouniA · 2026-06-03T16:05:39Z

+
+LOG_MODULE_REGISTER(flash_mchp_nvmctrl_g3);
+
+#define SOC_NV_FLASH_NODE                 DT_NODELABEL(flash0)


shouldn't be using node labels inside driver, use instance macros.

zephyr/drivers/flash/flash_mcux_flexspi_nor.c

Line 30 in ad056d5

#define FLEXSPI_NOR_SOC_NV_FLASH_NODE(node_id) \

JarmouniA · 2026-06-03T16:09:47Z

+}
+static struct nvmctrl_data nvmctrl_g3_data;
+static const struct nvmctrl_config nvmctrl_g3_config = {
+	.clock_dev = DEVICE_DT_GET(DT_NODELABEL(clock)),


what clock is that? this is not properly structured.

In general you shouldn't rely on node labels in drivers, they can be changed easily.

JarmouniA · 2026-06-03T16:12:29Z

 	soc {
-		flash0: flash@8000000 {
-			reg = <0x08000000 DT_SIZE_M(1)>;
+		nvmctrl: nvmctrl@44002000 {


Suggested change

nvmctrl: nvmctrl@44002000 {

nvmctrl: flash-comtroller@44002000 {

apply for all occurrences

JarmouniA · 2026-06-03T16:27:42Z

Why does this header exist!?

JarmouniA · 2026-06-03T16:30:53Z

+ */
+
+/**
+ * @file mchp_nvmctrl_g3.h


missing API versioning
https://docs.zephyrproject.org/latest/contribute/style/doxygen.html#api-versioning

JarmouniA · 2026-06-03T16:37:22Z

    extra_args:
      - DTC_OVERLAY_FILE="./boards/${BOARD}_andestech_qspi_nor_xip.overlay"
      - CONF_FILE="./prj.conf ./boards/${BOARD}_andestech_qspi_nor_xip.conf"
+  drivers.flash.common.mchp_pic32:


why is a new test scenario needed since it's not applying any special config.
drivers.flash.common.default does it.

JarmouniA · 2026-06-03T16:39:18Z

      - frdm_imxrt1186/mimxrt1186/cm7
    integration_platforms:
      - nrf9160dk/nrf9160
+  drivers.flash.erase_blocks:


Suggested change

drivers.flash.erase_blocks:

JarmouniA · 2026-06-03T16:39:27Z

             dt_label_compat_enabled("slot1_partition", "zephyr,mapped-partition"))
    integration_platforms:
      - imx95_evk/mimx9596/m7
+  drivers.flash.interface_test:


Suggested change

drivers.flash.interface_test:

sr-mchp force-pushed the mchp_flash_g3_driver branch from 4d35b74 to 68a62f2 Compare May 25, 2026 08:18

sr-mchp marked this pull request as ready for review May 25, 2026 10:25

zephyrbot added area: Flash platform: Microchip PIC32 area: Tests Issues related to a particular existing or missing test area: Devicetree Binding PR modifies or adds a Device Tree binding area: Boards/SoCs labels May 25, 2026

zephyrbot requested review from Albort12138, ArunMCHP, AzharMCHP, Farsin-Nasar-Microchip, JarmouniA, NhMchp, de-nordic, fabin-mchp, kartben, mchp-asif, nandojve, nordicjm, rghaddab, sunil-abraham and sylvioalves May 25, 2026 10:27

zephyrbot assigned de-nordic May 25, 2026

sr-mchp force-pushed the mchp_flash_g3_driver branch from 68a62f2 to 3460766 Compare May 26, 2026 10:56

nandojve requested changes May 26, 2026

View reviewed changes

sunil-abraham requested changes May 27, 2026

View reviewed changes

sr-mchp force-pushed the mchp_flash_g3_driver branch from 3460766 to 13d3b6d Compare June 3, 2026 14:48

zephyrbot requested a review from nashif June 3, 2026 14:50

dts: arm: microchip: NVMCTRL/Flash G3 driver

6ba8eab

Devicetree & binding files for NVMCTRL PIC32CZ_CA series of devices Signed-off-by: Shankar Ramasamy <shankar.ramasamy@microchip.com>

sr-mchp force-pushed the mchp_flash_g3_driver branch from 13d3b6d to 53f4fb2 Compare June 3, 2026 15:02

sr-mchp added 3 commits June 3, 2026 15:52

drivers: nvmctrl: microchip: NVMCTRL/Flash G3 driver

9522eb3

NVMCTRL G3 driver implementation to support PIC32CZ_CA device series Signed-off-by: Shankar Ramasamy <shankar.ramasamy@microchip.com>

boards: microchip: pic32cz_ca: Add Flash support

f942198

Add Flash partitions and board metadata for pic32cz_ca80/90_cult boards Signed-off-by: Shankar Ramasamy <shankar.ramasamy@microchip.com>

tests: flash: microchip: test files for pic32cz_ca

db58da7

Add board support files for pic32cz_ca80_cult to enable Flash test cases Signed-off-by: Shankar Ramasamy <shankar.ramasamy@microchip.com>

sr-mchp force-pushed the mchp_flash_g3_driver branch from 53f4fb2 to db58da7 Compare June 3, 2026 15:54

JarmouniA requested changes Jun 3, 2026

View reviewed changes


		LOG_MODULE_REGISTER(flash_mchp_nvmctrl_g3);

		#define SOC_NV_FLASH_NODE DT_NODELABEL(flash0)

	nvmctrl: nvmctrl@44002000 {
	nvmctrl: flash-comtroller@44002000 {

Conversation

sr-mchp commented May 25, 2026

Uh oh!

nandojve left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sunil-abraham left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Jun 3, 2026

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants