Skip to content

drivers: memc: Fix various APS6404 device driver issues #79513

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.

Sign up for GitHub

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
mmahadevan108 merged 1 commit into from
Nov 8, 2024
Merged

drivers: memc: Fix various APS6404 device driver issues #79513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 drivers/memc/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ if MEMC

config MEMC_INIT_PRIORITY
int "Initialization priority"
default 80 if MSPI
default 0
help
Memory controllers initialization priority.
Expand Down
13 changes: 11 additions & 2 deletions drivers/memc/memc_mspi_aps6404l.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@
return ret;
}

#if CONFIG_PM_DEVICE
static void acquire(const struct device *psram)
{
const struct memc_mspi_aps6404l_config *cfg = psram->config;
Expand All @@ -154,6 +155,7 @@
}
}
}
#endif /* CONFIG_PM_DEVICE */

static void release(const struct device *psram)
{
Expand Down Expand Up @@ -341,28 +343,34 @@
}
data->dev_cfg = cfg->tar_dev_cfg;

#if CONFIG_MSPI_TIMING
if (mspi_timing_config(cfg->bus, &cfg->dev_id, cfg->timing_cfg_mask,
(void *)&cfg->tar_timing_cfg)) {
LOG_ERR("Failed to config mspi timing/%u", __LINE__);
return -EIO;
}
data->timing_cfg = cfg->tar_timing_cfg;
#endif /* CONFIG_MSPI_TIMING */

#if CONFIG_MSPI_XIP
Copy link
Contributor

@RichardSWheatley RichardSWheatley Nov 3, 2024

Choose a reason for hiding this comment

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

Why are these ifdef's included of the enable is already there?

To save code space?

Copy link
Contributor

@swift-tk swift-tk Nov 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

When XIP is not supported by a specific hardware, CONFIG_MSPI_XIP=n is used to indicate that. This also means that xip_config may not be implemented, so calling it in device driver may result in error. Leaving it out of compile will not only save code space but also ensure the device driver is still functional without optional features.
Additionally, one can also choose to disable XIP even when it is supported.

if (cfg->tar_xip_cfg.enable) {
if (mspi_xip_config(cfg->bus, &cfg->dev_id, &cfg->tar_xip_cfg)) {
LOG_ERR("Failed to enable XIP/%u", __LINE__);
return -EIO;
}
data->xip_cfg = cfg->tar_xip_cfg;
}
#endif /* CONFIG_MSPI_XIP */

#if CONFIG_MSPI_SCRAMBLE
if (cfg->tar_scramble_cfg.enable) {
if (mspi_scramble_config(cfg->bus, &cfg->dev_id, &cfg->tar_scramble_cfg)) {
LOG_ERR("Failed to enable scrambling/%u", __LINE__);
return -EIO;
}
data->scramble_cfg = cfg->tar_scramble_cfg;
}
#endif /* MSPI_SCRAMBLE */

release(psram);

Expand Down Expand Up @@ -423,9 +431,10 @@
}
#define MSPI_TIMING_CONFIG_MASK(n) DT_INST_PROP(n, ambiq_timing_config_mask)
#else
#define MSPI_TIMING_CONFIG(n)
#define MSPI_TIMING_CONFIG_MASK(n)
#define MSPI_TIMING_CONFIG(n) {}
#define MSPI_TIMING_CONFIG_MASK(n) MSPI_TIMING_PARAM_DUMMY
#define MSPI_PORT(n) 0
#endif

Check notice on line 437 in drivers/memc/memc_mspi_aps6404l.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change 

drivers/memc/memc_mspi_aps6404l.c:437 
-#define MSPI_TIMING_CONFIG(n) {}
+#define MSPI_TIMING_CONFIG(n)                                                                      \
+	{                                                                                          \
+	}
 #define MSPI_TIMING_CONFIG_MASK(n) MSPI_TIMING_PARAM_DUMMY
-#define MSPI_PORT(n) 0
+#define MSPI_PORT(n)               0

Check notice on line 437 in drivers/memc/memc_mspi_aps6404l.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change 

drivers/memc/memc_mspi_aps6404l.c:437 
-#define MSPI_TIMING_CONFIG(n) {}
+#define MSPI_TIMING_CONFIG(n)                                                                      \
+	{                                                                                          \
+	}
 #define MSPI_TIMING_CONFIG_MASK(n) MSPI_TIMING_PARAM_DUMMY
-#define MSPI_PORT(n) 0
+#define MSPI_PORT(n)               0

#define MEMC_MSPI_APS6404L(n) \
static const struct memc_mspi_aps6404l_config \
Expand Down
Loading