* [PATCHv2 1/2] mmc-utils: Refactor switch to allow custom timeout
@ 2022-10-12 19:03 Christian Löhle
2022-10-13 8:19 ` Avri Altman
0 siblings, 1 reply; 3+ messages in thread
From: Christian Löhle @ 2022-10-12 19:03 UTC (permalink / raw)
To: Avri Altman, Ulf Hansson, Linux MMC List
Certain commands require a longer switch timeout.
Refactor accordingly to allow e.g. for future sanitize change.
Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
---
mmc_cmds.c | 83 +++++++++++++++++++++++++++++++++++-------------------
1 file changed, 54 insertions(+), 29 deletions(-)
diff --git a/mmc_cmds.c b/mmc_cmds.c
index 2957aa9..8bc7a56 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -54,6 +54,8 @@
#define WPTYPE_PWRON 2
#define WPTYPE_PERM 3
+#define SWITCH_TIMEOUT_MS (10 * 1000)
+
int read_extcsd(int fd, __u8 *ext_csd)
{
@@ -76,7 +78,7 @@ int read_extcsd(int fd, __u8 *ext_csd)
return ret;
}
-int write_extcsd_value(int fd, __u8 index, __u8 value)
+int write_extcsd_value(int fd, __u8 index, __u8 value, unsigned int timeout_ms)
{
int ret = 0;
struct mmc_ioc_cmd idata;
@@ -89,6 +91,7 @@ int write_extcsd_value(int fd, __u8 index, __u8 value)
(value << 8) |
EXT_CSD_CMD_SET_NORMAL;
idata.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
+ idata.cmd_timeout_ms = timeout_ms;
ret = ioctl(fd, MMC_IOC_CMD, &idata);
if (ret)
@@ -341,7 +344,8 @@ int do_writeprotect_boot_set(int nargs, char **argv)
value |= permanent ? EXT_CSD_BOOT_WP_B_PERM_WP_EN
: EXT_CSD_BOOT_WP_B_PWR_WP_EN;
- ret = write_extcsd_value(fd, EXT_CSD_BOOT_WP, value);
+ ret = write_extcsd_value(fd, EXT_CSD_BOOT_WP, value,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to "
"EXT_CSD[%d] in %s\n",
@@ -508,7 +512,8 @@ int do_writeprotect_user_set(int nargs, char **argv)
break;
}
if (user_wp != ext_csd[EXT_CSD_USER_WP]) {
- ret = write_extcsd_value(fd, EXT_CSD_USER_WP, user_wp);
+ ret = write_extcsd_value(fd, EXT_CSD_USER_WP, user_wp,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Error setting EXT_CSD\n");
exit(1);
@@ -526,7 +531,7 @@ int do_writeprotect_user_set(int nargs, char **argv)
}
if (wptype != WPTYPE_NONE) {
ret = write_extcsd_value(fd, EXT_CSD_USER_WP,
- ext_csd[EXT_CSD_USER_WP]);
+ ext_csd[EXT_CSD_USER_WP], SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Error restoring EXT_CSD\n");
exit(1);
@@ -571,7 +576,8 @@ int do_disable_512B_emulation(int nargs, char **argv)
if (native_sector_size && !data_sector_size &&
(wr_rel_param & EN_REL_WR)) {
- ret = write_extcsd_value(fd, EXT_CSD_USE_NATIVE_SECTOR, 1);
+ ret = write_extcsd_value(fd, EXT_CSD_USE_NATIVE_SECTOR, 1,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
@@ -650,7 +656,8 @@ int do_write_boot_en(int nargs, char **argv)
else
value &= ~EXT_CSD_PART_CONFIG_ACC_ACK;
- ret = write_extcsd_value(fd, EXT_CSD_PART_CONFIG, value);
+ ret = write_extcsd_value(fd, EXT_CSD_PART_CONFIG, value,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to "
"EXT_CSD[%d] in %s\n",
@@ -720,7 +727,8 @@ int do_boot_bus_conditions_set(int nargs, char **argv)
printf("Changing ext_csd[BOOT_BUS_CONDITIONS] from 0x%02x to 0x%02x\n",
ext_csd[EXT_CSD_BOOT_BUS_CONDITIONS], value);
- ret = write_extcsd_value(fd, EXT_CSD_BOOT_BUS_CONDITIONS, value);
+ ret = write_extcsd_value(fd, EXT_CSD_BOOT_BUS_CONDITIONS, value,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to "
"EXT_CSD[%d] in %s\n",
@@ -771,7 +779,8 @@ int do_hwreset(int value, int nargs, char **argv)
exit(1);
}
- ret = write_extcsd_value(fd, EXT_CSD_RST_N_FUNCTION, value);
+ ret = write_extcsd_value(fd, EXT_CSD_RST_N_FUNCTION, value,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr,
"Could not write 0x%02x to EXT_CSD[%d] in %s\n",
@@ -825,9 +834,11 @@ int do_write_bkops_en(int nargs, char **argv)
fprintf(stderr, "%s doesn't support AUTO_EN in the BKOPS_EN register\n", device);
exit(1);
}
- ret = write_extcsd_value(fd, EXT_CSD_BKOPS_EN, BKOPS_AUTO_ENABLE);
+ ret = write_extcsd_value(fd, EXT_CSD_BKOPS_EN, BKOPS_AUTO_ENABLE,
+ SWITCH_TIMEOUT_MS);
} else if (strcmp(en_type, "manual") == 0) {
- ret = write_extcsd_value(fd, EXT_CSD_BKOPS_EN, BKOPS_MAN_ENABLE);
+ ret = write_extcsd_value(fd, EXT_CSD_BKOPS_EN, BKOPS_MAN_ENABLE,
+ SWITCH_TIMEOUT_MS);
} else {
fprintf(stderr, "%s invalid mode for BKOPS_EN requested: %s. Valid options: auto or manual\n", en_type, device);
exit(1);
@@ -1002,7 +1013,8 @@ int set_partitioning_setting_completed(int dry_run, const char * const device,
}
fprintf(stderr, "setting OTP PARTITION_SETTING_COMPLETED!\n");
- ret = write_extcsd_value(fd, EXT_CSD_PARTITION_SETTING_COMPLETED, 0x1);
+ ret = write_extcsd_value(fd, EXT_CSD_PARTITION_SETTING_COMPLETED, 0x1,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x1 to "
"EXT_CSD[%d] in %s\n",
@@ -1188,7 +1200,8 @@ int do_create_gp_partition(int nargs, char **argv)
gp_size_mult = (length_kib + align/2l) / align;
/* set EXT_CSD_ERASE_GROUP_DEF bit 0 */
- ret = write_extcsd_value(fd, EXT_CSD_ERASE_GROUP_DEF, 0x1);
+ ret = write_extcsd_value(fd, EXT_CSD_ERASE_GROUP_DEF, 0x1,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x1 to EXT_CSD[%d] in %s\n",
EXT_CSD_ERASE_GROUP_DEF, device);
@@ -1197,7 +1210,7 @@ int do_create_gp_partition(int nargs, char **argv)
value = (gp_size_mult >> 16) & 0xff;
address = EXT_CSD_GP_SIZE_MULT_1_2 + (partition - 1) * 3;
- ret = write_extcsd_value(fd, address, value);
+ ret = write_extcsd_value(fd, address, value, SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
value, address, device);
@@ -1205,7 +1218,7 @@ int do_create_gp_partition(int nargs, char **argv)
}
value = (gp_size_mult >> 8) & 0xff;
address = EXT_CSD_GP_SIZE_MULT_1_1 + (partition - 1) * 3;
- ret = write_extcsd_value(fd, address, value);
+ ret = write_extcsd_value(fd, address, value, SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
value, address, device);
@@ -1213,7 +1226,7 @@ int do_create_gp_partition(int nargs, char **argv)
}
value = gp_size_mult & 0xff;
address = EXT_CSD_GP_SIZE_MULT_1_0 + (partition - 1) * 3;
- ret = write_extcsd_value(fd, address, value);
+ ret = write_extcsd_value(fd, address, value, SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
value, address, device);
@@ -1226,7 +1239,8 @@ int do_create_gp_partition(int nargs, char **argv)
else
value &= ~(1 << partition);
- ret = write_extcsd_value(fd, EXT_CSD_PARTITIONS_ATTRIBUTE, value);
+ ret = write_extcsd_value(fd, EXT_CSD_PARTITIONS_ATTRIBUTE, value,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write EXT_CSD_ENH_%x to EXT_CSD[%d] in %s\n",
partition, EXT_CSD_PARTITIONS_ATTRIBUTE, device);
@@ -1240,7 +1254,7 @@ int do_create_gp_partition(int nargs, char **argv)
else
value &= (0xF << (4 * ((partition % 2))));
- ret = write_extcsd_value(fd, address, value);
+ ret = write_extcsd_value(fd, address, value, SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%x to EXT_CSD[%d] in %s\n",
value, address, device);
@@ -1317,7 +1331,8 @@ int do_enh_area_set(int nargs, char **argv)
enh_start_addr *= align;
/* set EXT_CSD_ERASE_GROUP_DEF bit 0 */
- ret = write_extcsd_value(fd, EXT_CSD_ERASE_GROUP_DEF, 0x1);
+ ret = write_extcsd_value(fd, EXT_CSD_ERASE_GROUP_DEF, 0x1,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x1 to "
"EXT_CSD[%d] in %s\n",
@@ -1327,7 +1342,8 @@ int do_enh_area_set(int nargs, char **argv)
/* write to ENH_START_ADDR and ENH_SIZE_MULT and PARTITIONS_ATTRIBUTE's ENH_USR bit */
value = (enh_start_addr >> 24) & 0xff;
- ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_3, value);
+ ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_3, value,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to "
"EXT_CSD[%d] in %s\n", value,
@@ -1335,7 +1351,8 @@ int do_enh_area_set(int nargs, char **argv)
exit(1);
}
value = (enh_start_addr >> 16) & 0xff;
- ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_2, value);
+ ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_2, value,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to "
"EXT_CSD[%d] in %s\n", value,
@@ -1343,7 +1360,8 @@ int do_enh_area_set(int nargs, char **argv)
exit(1);
}
value = (enh_start_addr >> 8) & 0xff;
- ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_1, value);
+ ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_1, value,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to "
"EXT_CSD[%d] in %s\n", value,
@@ -1351,7 +1369,8 @@ int do_enh_area_set(int nargs, char **argv)
exit(1);
}
value = enh_start_addr & 0xff;
- ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_0, value);
+ ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_0, value,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to "
"EXT_CSD[%d] in %s\n", value,
@@ -1360,7 +1379,8 @@ int do_enh_area_set(int nargs, char **argv)
}
value = (enh_size_mult >> 16) & 0xff;
- ret = write_extcsd_value(fd, EXT_CSD_ENH_SIZE_MULT_2, value);
+ ret = write_extcsd_value(fd, EXT_CSD_ENH_SIZE_MULT_2, value,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to "
"EXT_CSD[%d] in %s\n", value,
@@ -1368,7 +1388,7 @@ int do_enh_area_set(int nargs, char **argv)
exit(1);
}
value = (enh_size_mult >> 8) & 0xff;
- ret = write_extcsd_value(fd, EXT_CSD_ENH_SIZE_MULT_1, value);
+ ret = write_extcsd_value(fd, EXT_CSD_ENH_SIZE_MULT_1, value, SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to "
"EXT_CSD[%d] in %s\n", value,
@@ -1376,7 +1396,8 @@ int do_enh_area_set(int nargs, char **argv)
exit(1);
}
value = enh_size_mult & 0xff;
- ret = write_extcsd_value(fd, EXT_CSD_ENH_SIZE_MULT_0, value);
+ ret = write_extcsd_value(fd, EXT_CSD_ENH_SIZE_MULT_0, value,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to "
"EXT_CSD[%d] in %s\n", value,
@@ -1384,7 +1405,8 @@ int do_enh_area_set(int nargs, char **argv)
exit(1);
}
value = ext_csd[EXT_CSD_PARTITIONS_ATTRIBUTE] | EXT_CSD_ENH_USR;
- ret = write_extcsd_value(fd, EXT_CSD_PARTITIONS_ATTRIBUTE, value);
+ ret = write_extcsd_value(fd, EXT_CSD_PARTITIONS_ATTRIBUTE, value,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write EXT_CSD_ENH_USR to "
"EXT_CSD[%d] in %s\n",
@@ -1455,7 +1477,8 @@ int do_write_reliability_set(int nargs, char **argv)
}
value = ext_csd[EXT_CSD_WR_REL_SET] | (1<<partition);
- ret = write_extcsd_value(fd, EXT_CSD_WR_REL_SET, value);
+ ret = write_extcsd_value(fd, EXT_CSD_WR_REL_SET, value,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
value, EXT_CSD_WR_REL_SET, device);
@@ -1998,7 +2021,8 @@ int do_sanitize(int nargs, char **argv)
exit(1);
}
- ret = write_extcsd_value(fd, EXT_CSD_SANITIZE_START, 1);
+ ret = write_extcsd_value(fd, EXT_CSD_SANITIZE_START, 1,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
1, EXT_CSD_SANITIZE_START, device);
@@ -2587,7 +2611,8 @@ int do_cache_ctrl(int value, int nargs, char **argv)
device);
exit(1);
}
- ret = write_extcsd_value(fd, EXT_CSD_CACHE_CTRL, value);
+ ret = write_extcsd_value(fd, EXT_CSD_CACHE_CTRL, value,
+ SWITCH_TIMEOUT_MS);
if (ret) {
fprintf(stderr,
"Could not write 0x%02x to EXT_CSD[%d] in %s\n",
--
2.37.3
Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782
^ permalink raw reply related [flat|nested] 3+ messages in thread
* RE: [PATCHv2 1/2] mmc-utils: Refactor switch to allow custom timeout
2022-10-12 19:03 [PATCHv2 1/2] mmc-utils: Refactor switch to allow custom timeout Christian Löhle
@ 2022-10-13 8:19 ` Avri Altman
2022-10-13 8:35 ` Christian Löhle
0 siblings, 1 reply; 3+ messages in thread
From: Avri Altman @ 2022-10-13 8:19 UTC (permalink / raw)
To: Christian Löhle, Ulf Hansson, Linux MMC List
> Certain commands require a longer switch timeout.
> Refactor accordingly to allow e.g. for future sanitize change.
>
> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
> ---
> mmc_cmds.c | 83 +++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 54 insertions(+), 29 deletions(-)
>
> diff --git a/mmc_cmds.c b/mmc_cmds.c
> index 2957aa9..8bc7a56 100644
> --- a/mmc_cmds.c
> +++ b/mmc_cmds.c
> @@ -54,6 +54,8 @@
> #define WPTYPE_PWRON 2
> #define WPTYPE_PERM 3
>
> +#define SWITCH_TIMEOUT_MS (10 * 1000)
Why 10 seconds?
Maybe add comment or at least explain in your commit log.
> +
>
> int read_extcsd(int fd, __u8 *ext_csd)
> {
> @@ -76,7 +78,7 @@ int read_extcsd(int fd, __u8 *ext_csd)
> return ret;
> }
>
> -int write_extcsd_value(int fd, __u8 index, __u8 value)
> +int write_extcsd_value(int fd, __u8 index, __u8 value, unsigned int
> +timeout_ms)
> {
> int ret = 0;
> struct mmc_ioc_cmd idata;
> @@ -89,6 +91,7 @@ int write_extcsd_value(int fd, __u8 index, __u8 value)
> (value << 8) |
> EXT_CSD_CMD_SET_NORMAL;
> idata.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
> + idata.cmd_timeout_ms = timeout_ms;
If cmd_timeout_ms not set, mmc_sanitize will use 240s - MMC_SANITIZE_TIMEOUT_MS.
I thought that you need more time, or did I get it wrong?
Also, I am uncomfortable that you are setting this value for all other commands.
Thanks,
Avri
>
> ret = ioctl(fd, MMC_IOC_CMD, &idata);
> if (ret)
> @@ -341,7 +344,8 @@ int do_writeprotect_boot_set(int nargs, char **argv)
> value |= permanent ? EXT_CSD_BOOT_WP_B_PERM_WP_EN
> : EXT_CSD_BOOT_WP_B_PWR_WP_EN;
>
> - ret = write_extcsd_value(fd, EXT_CSD_BOOT_WP, value);
> + ret = write_extcsd_value(fd, EXT_CSD_BOOT_WP, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to "
> "EXT_CSD[%d] in %s\n", @@ -508,7 +512,8 @@ int
> do_writeprotect_user_set(int nargs, char **argv)
> break;
> }
> if (user_wp != ext_csd[EXT_CSD_USER_WP]) {
> - ret = write_extcsd_value(fd, EXT_CSD_USER_WP, user_wp);
> + ret = write_extcsd_value(fd, EXT_CSD_USER_WP, user_wp,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Error setting EXT_CSD\n");
> exit(1); @@ -526,7 +531,7 @@ int
> do_writeprotect_user_set(int nargs, char **argv)
> }
> if (wptype != WPTYPE_NONE) {
> ret = write_extcsd_value(fd, EXT_CSD_USER_WP,
> - ext_csd[EXT_CSD_USER_WP]);
> + ext_csd[EXT_CSD_USER_WP],
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Error restoring EXT_CSD\n");
> exit(1);
> @@ -571,7 +576,8 @@ int do_disable_512B_emulation(int nargs, char **argv)
>
> if (native_sector_size && !data_sector_size &&
> (wr_rel_param & EN_REL_WR)) {
> - ret = write_extcsd_value(fd, EXT_CSD_USE_NATIVE_SECTOR, 1);
> + ret = write_extcsd_value(fd, EXT_CSD_USE_NATIVE_SECTOR, 1,
> + SWITCH_TIMEOUT_MS);
>
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
> @@ -650,7 +656,8 @@ int do_write_boot_en(int nargs, char **argv)
> else
> value &= ~EXT_CSD_PART_CONFIG_ACC_ACK;
>
> - ret = write_extcsd_value(fd, EXT_CSD_PART_CONFIG, value);
> + ret = write_extcsd_value(fd, EXT_CSD_PART_CONFIG, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to "
> "EXT_CSD[%d] in %s\n", @@ -720,7 +727,8 @@ int
> do_boot_bus_conditions_set(int nargs, char **argv)
> printf("Changing ext_csd[BOOT_BUS_CONDITIONS] from 0x%02x to
> 0x%02x\n",
> ext_csd[EXT_CSD_BOOT_BUS_CONDITIONS], value);
>
> - ret = write_extcsd_value(fd, EXT_CSD_BOOT_BUS_CONDITIONS, value);
> + ret = write_extcsd_value(fd, EXT_CSD_BOOT_BUS_CONDITIONS, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to "
> "EXT_CSD[%d] in %s\n", @@ -771,7 +779,8 @@ int do_hwreset(int
> value, int nargs, char **argv)
> exit(1);
> }
>
> - ret = write_extcsd_value(fd, EXT_CSD_RST_N_FUNCTION, value);
> + ret = write_extcsd_value(fd, EXT_CSD_RST_N_FUNCTION, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr,
> "Could not write 0x%02x to EXT_CSD[%d] in %s\n", @@ -825,9
> +834,11 @@ int do_write_bkops_en(int nargs, char **argv)
> fprintf(stderr, "%s doesn't support AUTO_EN in the BKOPS_EN
> register\n", device);
> exit(1);
> }
> - ret = write_extcsd_value(fd, EXT_CSD_BKOPS_EN,
> BKOPS_AUTO_ENABLE);
> + ret = write_extcsd_value(fd, EXT_CSD_BKOPS_EN,
> BKOPS_AUTO_ENABLE,
> + SWITCH_TIMEOUT_MS);
> } else if (strcmp(en_type, "manual") == 0) {
> - ret = write_extcsd_value(fd, EXT_CSD_BKOPS_EN,
> BKOPS_MAN_ENABLE);
> + ret = write_extcsd_value(fd, EXT_CSD_BKOPS_EN,
> BKOPS_MAN_ENABLE,
> + SWITCH_TIMEOUT_MS);
> } else {
> fprintf(stderr, "%s invalid mode for BKOPS_EN requested: %s. Valid
> options: auto or manual\n", en_type, device);
> exit(1);
> @@ -1002,7 +1013,8 @@ int set_partitioning_setting_completed(int dry_run,
> const char * const device,
> }
>
> fprintf(stderr, "setting OTP PARTITION_SETTING_COMPLETED!\n");
> - ret = write_extcsd_value(fd, EXT_CSD_PARTITION_SETTING_COMPLETED,
> 0x1);
> + ret = write_extcsd_value(fd, EXT_CSD_PARTITION_SETTING_COMPLETED,
> 0x1,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x1 to "
> "EXT_CSD[%d] in %s\n", @@ -1188,7 +1200,8 @@ int
> do_create_gp_partition(int nargs, char **argv)
> gp_size_mult = (length_kib + align/2l) / align;
>
> /* set EXT_CSD_ERASE_GROUP_DEF bit 0 */
> - ret = write_extcsd_value(fd, EXT_CSD_ERASE_GROUP_DEF, 0x1);
> + ret = write_extcsd_value(fd, EXT_CSD_ERASE_GROUP_DEF, 0x1,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x1 to EXT_CSD[%d] in %s\n",
> EXT_CSD_ERASE_GROUP_DEF, device); @@ -1197,7 +1210,7 @@
> int do_create_gp_partition(int nargs, char **argv)
>
> value = (gp_size_mult >> 16) & 0xff;
> address = EXT_CSD_GP_SIZE_MULT_1_2 + (partition - 1) * 3;
> - ret = write_extcsd_value(fd, address, value);
> + ret = write_extcsd_value(fd, address, value, SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
> value, address, device); @@ -1205,7 +1218,7 @@ int
> do_create_gp_partition(int nargs, char **argv)
> }
> value = (gp_size_mult >> 8) & 0xff;
> address = EXT_CSD_GP_SIZE_MULT_1_1 + (partition - 1) * 3;
> - ret = write_extcsd_value(fd, address, value);
> + ret = write_extcsd_value(fd, address, value, SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
> value, address, device); @@ -1213,7 +1226,7 @@ int
> do_create_gp_partition(int nargs, char **argv)
> }
> value = gp_size_mult & 0xff;
> address = EXT_CSD_GP_SIZE_MULT_1_0 + (partition - 1) * 3;
> - ret = write_extcsd_value(fd, address, value);
> + ret = write_extcsd_value(fd, address, value, SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
> value, address, device); @@ -1226,7 +1239,8 @@ int
> do_create_gp_partition(int nargs, char **argv)
> else
> value &= ~(1 << partition);
>
> - ret = write_extcsd_value(fd, EXT_CSD_PARTITIONS_ATTRIBUTE, value);
> + ret = write_extcsd_value(fd, EXT_CSD_PARTITIONS_ATTRIBUTE, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write EXT_CSD_ENH_%x to EXT_CSD[%d] in
> %s\n",
> partition, EXT_CSD_PARTITIONS_ATTRIBUTE, device); @@ -1240,7
> +1254,7 @@ int do_create_gp_partition(int nargs, char **argv)
> else
> value &= (0xF << (4 * ((partition % 2))));
>
> - ret = write_extcsd_value(fd, address, value);
> + ret = write_extcsd_value(fd, address, value, SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%x to EXT_CSD[%d] in %s\n",
> value, address, device); @@ -1317,7 +1331,8 @@ int
> do_enh_area_set(int nargs, char **argv)
> enh_start_addr *= align;
>
> /* set EXT_CSD_ERASE_GROUP_DEF bit 0 */
> - ret = write_extcsd_value(fd, EXT_CSD_ERASE_GROUP_DEF, 0x1);
> + ret = write_extcsd_value(fd, EXT_CSD_ERASE_GROUP_DEF, 0x1,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x1 to "
> "EXT_CSD[%d] in %s\n", @@ -1327,7 +1342,8 @@ int
> do_enh_area_set(int nargs, char **argv)
>
> /* write to ENH_START_ADDR and ENH_SIZE_MULT and
> PARTITIONS_ATTRIBUTE's ENH_USR bit */
> value = (enh_start_addr >> 24) & 0xff;
> - ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_3, value);
> + ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_3, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to "
> "EXT_CSD[%d] in %s\n", value, @@ -1335,7 +1351,8 @@ int
> do_enh_area_set(int nargs, char **argv)
> exit(1);
> }
> value = (enh_start_addr >> 16) & 0xff;
> - ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_2, value);
> + ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_2, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to "
> "EXT_CSD[%d] in %s\n", value, @@ -1343,7 +1360,8 @@ int
> do_enh_area_set(int nargs, char **argv)
> exit(1);
> }
> value = (enh_start_addr >> 8) & 0xff;
> - ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_1, value);
> + ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_1, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to "
> "EXT_CSD[%d] in %s\n", value, @@ -1351,7 +1369,8 @@ int
> do_enh_area_set(int nargs, char **argv)
> exit(1);
> }
> value = enh_start_addr & 0xff;
> - ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_0, value);
> + ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_0, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to "
> "EXT_CSD[%d] in %s\n", value, @@ -1360,7 +1379,8 @@ int
> do_enh_area_set(int nargs, char **argv)
> }
>
> value = (enh_size_mult >> 16) & 0xff;
> - ret = write_extcsd_value(fd, EXT_CSD_ENH_SIZE_MULT_2, value);
> + ret = write_extcsd_value(fd, EXT_CSD_ENH_SIZE_MULT_2, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to "
> "EXT_CSD[%d] in %s\n", value, @@ -1368,7 +1388,7 @@ int
> do_enh_area_set(int nargs, char **argv)
> exit(1);
> }
> value = (enh_size_mult >> 8) & 0xff;
> - ret = write_extcsd_value(fd, EXT_CSD_ENH_SIZE_MULT_1, value);
> + ret = write_extcsd_value(fd, EXT_CSD_ENH_SIZE_MULT_1, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to "
> "EXT_CSD[%d] in %s\n", value, @@ -1376,7 +1396,8 @@ int
> do_enh_area_set(int nargs, char **argv)
> exit(1);
> }
> value = enh_size_mult & 0xff;
> - ret = write_extcsd_value(fd, EXT_CSD_ENH_SIZE_MULT_0, value);
> + ret = write_extcsd_value(fd, EXT_CSD_ENH_SIZE_MULT_0, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to "
> "EXT_CSD[%d] in %s\n", value, @@ -1384,7 +1405,8 @@ int
> do_enh_area_set(int nargs, char **argv)
> exit(1);
> }
> value = ext_csd[EXT_CSD_PARTITIONS_ATTRIBUTE] | EXT_CSD_ENH_USR;
> - ret = write_extcsd_value(fd, EXT_CSD_PARTITIONS_ATTRIBUTE, value);
> + ret = write_extcsd_value(fd, EXT_CSD_PARTITIONS_ATTRIBUTE, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write EXT_CSD_ENH_USR to "
> "EXT_CSD[%d] in %s\n", @@ -1455,7 +1477,8 @@ int
> do_write_reliability_set(int nargs, char **argv)
> }
>
> value = ext_csd[EXT_CSD_WR_REL_SET] | (1<<partition);
> - ret = write_extcsd_value(fd, EXT_CSD_WR_REL_SET, value);
> + ret = write_extcsd_value(fd, EXT_CSD_WR_REL_SET, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
> value, EXT_CSD_WR_REL_SET, device); @@ -1998,7 +2021,8
> @@ int do_sanitize(int nargs, char **argv)
> exit(1);
> }
>
> - ret = write_extcsd_value(fd, EXT_CSD_SANITIZE_START, 1);
> + ret = write_extcsd_value(fd, EXT_CSD_SANITIZE_START, 1,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
> 1, EXT_CSD_SANITIZE_START, device); @@ -2587,7 +2611,8 @@
> int do_cache_ctrl(int value, int nargs, char **argv)
> device);
> exit(1);
> }
> - ret = write_extcsd_value(fd, EXT_CSD_CACHE_CTRL, value);
> + ret = write_extcsd_value(fd, EXT_CSD_CACHE_CTRL, value,
> + SWITCH_TIMEOUT_MS);
> if (ret) {
> fprintf(stderr,
> "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
> --
> 2.37.3
>
> Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz Managing Director: Dr.
> Jan Peter Berns.
> Commercial register of local courts: Freiburg HRB381782
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCHv2 1/2] mmc-utils: Refactor switch to allow custom timeout
2022-10-13 8:19 ` Avri Altman
@ 2022-10-13 8:35 ` Christian Löhle
0 siblings, 0 replies; 3+ messages in thread
From: Christian Löhle @ 2022-10-13 8:35 UTC (permalink / raw)
To: Avri Altman, Ulf Hansson, Linux MMC List
>> Certain commands require a longer switch timeout.
>> Refactor accordingly to allow e.g. for future sanitize change.
>>
>> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
>> ---
>> mmc_cmds.c | 83
>> +++++++++++++++++++++++++++++++++++-------------------
>> 1 file changed, 54 insertions(+), 29 deletions(-)
>>
>> diff --git a/mmc_cmds.c b/mmc_cmds.c
>> index 2957aa9..8bc7a56 100644
>> --- a/mmc_cmds.c
>> +++ b/mmc_cmds.c
>> @@ -54,6 +54,8 @@
>> #define WPTYPE_PWRON 2
>> #define WPTYPE_PERM 3
>>
>> +#define SWITCH_TIMEOUT_MS (10 * 1000)
>Why 10 seconds?
>Maybe add comment or at least explain in your commit log.
>
>> +
>>
>> int read_extcsd(int fd, __u8 *ext_csd) { @@ -76,7 +78,7 @@ int
>> read_extcsd(int fd, __u8 *ext_csd)
>> return ret;
>> }
>>
>> -int write_extcsd_value(int fd, __u8 index, __u8 value)
>> +int write_extcsd_value(int fd, __u8 index, __u8 value, unsigned int
>> +timeout_ms)
>> {
>> int ret = 0;
>> struct mmc_ioc_cmd idata;
>> @@ -89,6 +91,7 @@ int write_extcsd_value(int fd, __u8 index, __u8 value)
>> (value << 8) |
>> EXT_CSD_CMD_SET_NORMAL;
>> idata.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
>> + idata.cmd_timeout_ms = timeout_ms;
>If cmd_timeout_ms not set, mmc_sanitize will use 240s - MMC_SANITIZE_TIMEOUT_MS.
>I thought that you need more time, or did I get it wrong?
>
> Also, I am uncomfortable that you are setting this value for all other commands.
Thanks for your comments, I agree.
I will resend a v3 that leaves the cmd_timeout_ms setting to the kernel unless the sanitize user-specified timeout is used.
Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-10-13 8:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-12 19:03 [PATCHv2 1/2] mmc-utils: Refactor switch to allow custom timeout Christian Löhle
2022-10-13 8:19 ` Avri Altman
2022-10-13 8:35 ` Christian Löhle
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.