| Message ID | 1460589100-23968-1-git-send-email-angelo@sysam.it |
|---|---|
| State | Superseded |
| Delegated to: | Pantelis Antoniou |
| Headers | show |
On 04/14/2016 01:11 AM, Angelo Dureghello wrote: > This patch allows to read back the EXT_CSD[179] partition_config > register, just specifying the dev param: > > U-Boot> mmc partconf 0 > EXT_CSD[179], PARTITION_CONFIG register: > BOOT_ACK: 0 > BOOT_PARTITION_ENABLE: 0 > PARTITION_ACCESS: 0 > > Signed-off-by: Angelo Dureghello <angelo@sysam.it> > --- > Changes for v2: > - fixed commit message > - added white lines in cmd/mmc.c > - fixed help in cmd/mmc.c > --- > cmd/mmc.c | 35 +++++++++++++++++++++++++++++------ > drivers/mmc/mmc.c | 33 +++++++++++++++++++++++++++++++++ > include/mmc.h | 6 ++++++ > 3 files changed, 68 insertions(+), 6 deletions(-) > > diff --git a/cmd/mmc.c b/cmd/mmc.c > index 39ef072..de28749 100644 > --- a/cmd/mmc.c > +++ b/cmd/mmc.c > @@ -697,6 +697,24 @@ static int do_mmc_boot_resize(cmd_tbl_t *cmdtp, int flag, > printf("EMMC RPMB partition Size %d MB\n", rpmbsize); > return CMD_RET_SUCCESS; > } > + > +static int do_mmc_partconf_read(struct mmc *mmc) > +{ > + int err; > + u8 ack, part_num, access; > + > + err = mmc_get_part_conf(mmc, &ack, &part_num, &access); > + if (err) > + return CMD_RET_FAILURE; > + > + puts("EXT_CSD[179], PARTITION_CONFIG register:\n"); > + printf("BOOT_ACK: %d\n", ack); > + printf("BOOT_PARTITION_ENABLE: %d\n", part_num); > + printf("PARTITION_ACCESS: %d\n", access); Make it into one printf() call here. > + return 0; > +} > + > static int do_mmc_partconf(cmd_tbl_t *cmdtp, int flag, > int argc, char * const argv[]) > { > @@ -704,13 +722,10 @@ static int do_mmc_partconf(cmd_tbl_t *cmdtp, int flag, > struct mmc *mmc; > u8 ack, part_num, access; > > - if (argc != 5) > + if (argc != 2 && argc != 5) > return CMD_RET_USAGE; > > dev = simple_strtoul(argv[1], NULL, 10); > - ack = simple_strtoul(argv[2], NULL, 10); > - part_num = simple_strtoul(argv[3], NULL, 10); > - access = simple_strtoul(argv[4], NULL, 10); > > mmc = init_mmc_device(dev, false); > if (!mmc) > @@ -721,9 +736,17 @@ static int do_mmc_partconf(cmd_tbl_t *cmdtp, int flag, > return CMD_RET_FAILURE; > } > > + if (argc == 2) > + return do_mmc_partconf_read(mmc); > + > + ack = simple_strtoul(argv[2], NULL, 10); > + part_num = simple_strtoul(argv[3], NULL, 10); > + access = simple_strtoul(argv[4], NULL, 10); > + > /* acknowledge to be sent during boot operation */ > return mmc_set_part_conf(mmc, ack, part_num, access); > } > + > static int do_mmc_rst_func(cmd_tbl_t *cmdtp, int flag, > int argc, char * const argv[]) > { > @@ -858,8 +881,8 @@ U_BOOT_CMD( > " - Set the BOOT_BUS_WIDTH field of the specified device\n" > "mmc bootpart-resize <dev> <boot part size MB> <RPMB part size MB>\n" > " - Change sizes of boot and RPMB partitions of specified device\n" > - "mmc partconf dev boot_ack boot_partition partition_access\n" > - " - Change the bits of the PARTITION_CONFIG field of the specified device\n" > + "mmc partconf dev [boot_ack boot_partition partition_access]\n" > + " - Show or change the bits of the PARTITION_CONFIG field of the specified device\n" > "mmc rst-function dev value\n" > " - Change the RST_n_FUNCTION field of the specified device\n" > " WARNING: This is a write-once field and 0 / 1 / 2 are the only valid values.\n" > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > index d3c22ab..4cb3706 100644 > --- a/drivers/mmc/mmc.c > +++ b/drivers/mmc/mmc.c > @@ -1954,6 +1954,39 @@ int mmc_set_part_conf(struct mmc *mmc, u8 ack, u8 part_num, u8 access) > } > > /* > + * Read EXT_CSD[179] which is PARTITION_CONFIG (formerly BOOT_CONFIG) > + * and returning the extracted fields BOOT_ACK, BOOT_PARTITION_ENABLE and > + * PARTITION_ACCESS. > + * > + * Returns 0 on success. > + */ > +int mmc_get_part_conf(struct mmc *mmc, u8 *ack, u8 *part_num, u8 *access) > +{ > + int err; > + u8 part_conf; > + > + ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN); > + > + mmc->erase_grp_size = 1; > + mmc->part_config = MMCPART_NOAVAILABLE; > + > + if (IS_SD(mmc) || (mmc->version < MMC_VERSION_4)) > + return -1; return code from errno.h > + > + err = mmc_send_ext_csd(mmc, ext_csd); > + if (err) > + return err; > + > + part_conf = ext_csd[EXT_CSD_PART_CONF]; > + > + *ack = EXT_CSD_EXTRACT_BOOT_ACK(part_conf); > + *part_num = EXT_CSD_EXTRACT_BOOT_PART(part_conf); > + *access = EXT_CSD_EXTRACT_PARTITION_ACCESS(part_conf); > + > + return 0; > +} > + > +/* > * Modify EXT_CSD[162] which is RST_n_FUNCTION based on the given value > * for enable. Note that this is a write-once field for non-zero values. > * > diff --git a/include/mmc.h b/include/mmc.h > index cdb56e7..4b34b31 100644 > --- a/include/mmc.h > +++ b/include/mmc.h > @@ -222,6 +222,10 @@ > #define EXT_CSD_BOOT_PART_NUM(x) (x << 3) > #define EXT_CSD_PARTITION_ACCESS(x) (x << 0) > > +#define EXT_CSD_EXTRACT_BOOT_ACK(x) ((x >> 6) & 1) Should be (((x) >> 6) & 1) , notice the parenthesis around x. > +#define EXT_CSD_EXTRACT_BOOT_PART(x) ((x >> 3) & 0x7) > +#define EXT_CSD_EXTRACT_PARTITION_ACCESS(x) (x & 0x7) DTTO in the two above > #define EXT_CSD_BOOT_BUS_WIDTH_MODE(x) (x << 3) These should be fixed in a separate patch, can you please do it ? > #define EXT_CSD_BOOT_BUS_WIDTH_RESET(x) (x << 2) > #define EXT_CSD_BOOT_BUS_WIDTH_WIDTH(x) (x) > @@ -428,6 +432,8 @@ int mmc_boot_partition_size_change(struct mmc *mmc, unsigned long bootsize, > unsigned long rpmbsize); > /* Function to modify the PARTITION_CONFIG field of EXT_CSD */ > int mmc_set_part_conf(struct mmc *mmc, u8 ack, u8 part_num, u8 access); > +/* Function to read back the PARTITION_CONFIG field of EXT_CSD */ > +int mmc_get_part_conf(struct mmc *mmc, u8 *ack, u8 *part_num, u8 *access); > /* Function to modify the BOOT_BUS_WIDTH field of EXT_CSD */ > int mmc_set_boot_bus_width(struct mmc *mmc, u8 width, u8 reset, u8 mode); > /* Function to modify the RST_n_FUNCTION field of EXT_CSD */ >
Hi Marek, thanks for the review On 14/04/2016 01:16, Marek Vasut wrote: > On 04/14/2016 01:11 AM, Angelo Dureghello wrote: >> This patch allows to read back the EXT_CSD[179] partition_config >> register, just specifying the dev param: >> >> U-Boot> mmc partconf 0 >> EXT_CSD[179], PARTITION_CONFIG register: >> BOOT_ACK: 0 >> BOOT_PARTITION_ENABLE: 0 >> PARTITION_ACCESS: 0 >> >> Signed-off-by: Angelo Dureghello <angelo@sysam.it> >> --- >> Changes for v2: >> - fixed commit message >> - added white lines in cmd/mmc.c >> - fixed help in cmd/mmc.c >> --- >> cmd/mmc.c | 35 +++++++++++++++++++++++++++++------ >> drivers/mmc/mmc.c | 33 +++++++++++++++++++++++++++++++++ >> include/mmc.h | 6 ++++++ >> 3 files changed, 68 insertions(+), 6 deletions(-) >> >> diff --git a/cmd/mmc.c b/cmd/mmc.c >> index 39ef072..de28749 100644 >> --- a/cmd/mmc.c >> +++ b/cmd/mmc.c >> @@ -697,6 +697,24 @@ static int do_mmc_boot_resize(cmd_tbl_t *cmdtp, int flag, >> printf("EMMC RPMB partition Size %d MB\n", rpmbsize); >> return CMD_RET_SUCCESS; >> } >> + >> +static int do_mmc_partconf_read(struct mmc *mmc) >> +{ >> + int err; >> + u8 ack, part_num, access; >> + >> + err = mmc_get_part_conf(mmc, &ack, &part_num, &access); >> + if (err) >> + return CMD_RET_FAILURE; >> + >> + puts("EXT_CSD[179], PARTITION_CONFIG register:\n"); >> + printf("BOOT_ACK: %d\n", ack); >> + printf("BOOT_PARTITION_ENABLE: %d\n", part_num); >> + printf("PARTITION_ACCESS: %d\n", access); > > Make it into one printf() call here. > Is it really a mandatory rule ? :) Anyway ack, doing it in a single print. >> + return 0; >> +} >> + >> static int do_mmc_partconf(cmd_tbl_t *cmdtp, int flag, >> int argc, char * const argv[]) >> { >> @@ -704,13 +722,10 @@ static int do_mmc_partconf(cmd_tbl_t *cmdtp, int flag, >> struct mmc *mmc; >> u8 ack, part_num, access; >> >> - if (argc != 5) >> + if (argc != 2 && argc != 5) >> return CMD_RET_USAGE; >> >> dev = simple_strtoul(argv[1], NULL, 10); >> - ack = simple_strtoul(argv[2], NULL, 10); >> - part_num = simple_strtoul(argv[3], NULL, 10); >> - access = simple_strtoul(argv[4], NULL, 10); >> >> mmc = init_mmc_device(dev, false); >> if (!mmc) >> @@ -721,9 +736,17 @@ static int do_mmc_partconf(cmd_tbl_t *cmdtp, int flag, >> return CMD_RET_FAILURE; >> } >> >> + if (argc == 2) >> + return do_mmc_partconf_read(mmc); >> + >> + ack = simple_strtoul(argv[2], NULL, 10); >> + part_num = simple_strtoul(argv[3], NULL, 10); >> + access = simple_strtoul(argv[4], NULL, 10); >> + >> /* acknowledge to be sent during boot operation */ >> return mmc_set_part_conf(mmc, ack, part_num, access); >> } >> + >> static int do_mmc_rst_func(cmd_tbl_t *cmdtp, int flag, >> int argc, char * const argv[]) >> { >> @@ -858,8 +881,8 @@ U_BOOT_CMD( >> " - Set the BOOT_BUS_WIDTH field of the specified device\n" >> "mmc bootpart-resize <dev> <boot part size MB> <RPMB part size MB>\n" >> " - Change sizes of boot and RPMB partitions of specified device\n" >> - "mmc partconf dev boot_ack boot_partition partition_access\n" >> - " - Change the bits of the PARTITION_CONFIG field of the specified device\n" >> + "mmc partconf dev [boot_ack boot_partition partition_access]\n" >> + " - Show or change the bits of the PARTITION_CONFIG field of the specified device\n" >> "mmc rst-function dev value\n" >> " - Change the RST_n_FUNCTION field of the specified device\n" >> " WARNING: This is a write-once field and 0 / 1 / 2 are the only valid values.\n" >> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c >> index d3c22ab..4cb3706 100644 >> --- a/drivers/mmc/mmc.c >> +++ b/drivers/mmc/mmc.c >> @@ -1954,6 +1954,39 @@ int mmc_set_part_conf(struct mmc *mmc, u8 ack, u8 part_num, u8 access) >> } >> >> /* >> + * Read EXT_CSD[179] which is PARTITION_CONFIG (formerly BOOT_CONFIG) >> + * and returning the extracted fields BOOT_ACK, BOOT_PARTITION_ENABLE and >> + * PARTITION_ACCESS. >> + * >> + * Returns 0 on success. >> + */ >> +int mmc_get_part_conf(struct mmc *mmc, u8 *ack, u8 *part_num, u8 *access) >> +{ >> + int err; >> + u8 part_conf; >> + >> + ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN); >> + >> + mmc->erase_grp_size = 1; >> + mmc->part_config = MMCPART_NOAVAILABLE; >> + >> + if (IS_SD(mmc) || (mmc->version < MMC_VERSION_4)) >> + return -1; > > return code from errno.h > Ok >> + >> + err = mmc_send_ext_csd(mmc, ext_csd); >> + if (err) >> + return err; >> + >> + part_conf = ext_csd[EXT_CSD_PART_CONF]; >> + >> + *ack = EXT_CSD_EXTRACT_BOOT_ACK(part_conf); >> + *part_num = EXT_CSD_EXTRACT_BOOT_PART(part_conf); >> + *access = EXT_CSD_EXTRACT_PARTITION_ACCESS(part_conf); >> + >> + return 0; >> +} >> + >> +/* >> * Modify EXT_CSD[162] which is RST_n_FUNCTION based on the given value >> * for enable. Note that this is a write-once field for non-zero values. >> * >> diff --git a/include/mmc.h b/include/mmc.h >> index cdb56e7..4b34b31 100644 >> --- a/include/mmc.h >> +++ b/include/mmc.h >> @@ -222,6 +222,10 @@ >> #define EXT_CSD_BOOT_PART_NUM(x) (x << 3) >> #define EXT_CSD_PARTITION_ACCESS(x) (x << 0) >> >> +#define EXT_CSD_EXTRACT_BOOT_ACK(x) ((x >> 6) & 1) > > Should be (((x) >> 6) & 1) , notice the parenthesis around x. > Ok >> +#define EXT_CSD_EXTRACT_BOOT_PART(x) ((x >> 3) & 0x7) >> +#define EXT_CSD_EXTRACT_PARTITION_ACCESS(x) (x & 0x7) > > DTTO in the two above Ok > >> #define EXT_CSD_BOOT_BUS_WIDTH_MODE(x) (x << 3) > > These should be fixed in a separate patch, can you please do it ? > >> #define EXT_CSD_BOOT_BUS_WIDTH_RESET(x) (x << 2) >> #define EXT_CSD_BOOT_BUS_WIDTH_WIDTH(x) (x) >> @@ -428,6 +432,8 @@ int mmc_boot_partition_size_change(struct mmc *mmc, unsigned long bootsize, >> unsigned long rpmbsize); >> /* Function to modify the PARTITION_CONFIG field of EXT_CSD */ >> int mmc_set_part_conf(struct mmc *mmc, u8 ack, u8 part_num, u8 access); >> +/* Function to read back the PARTITION_CONFIG field of EXT_CSD */ >> +int mmc_get_part_conf(struct mmc *mmc, u8 *ack, u8 *part_num, u8 *access); >> /* Function to modify the BOOT_BUS_WIDTH field of EXT_CSD */ >> int mmc_set_boot_bus_width(struct mmc *mmc, u8 width, u8 reset, u8 mode); >> /* Function to modify the RST_n_FUNCTION field of EXT_CSD */ >> > > Going with version 3 Regards, Angelo Dureghello
Am 14.04.2016 um 18:56 schrieb Angelo Dureghello: > On 14/04/2016 01:16, Marek Vasut wrote: >> On 04/14/2016 01:11 AM, Angelo Dureghello wrote: >>> diff --git a/include/mmc.h b/include/mmc.h >>> index cdb56e7..4b34b31 100644 >>> --- a/include/mmc.h >>> +++ b/include/mmc.h >>> @@ -222,6 +222,10 @@ >>> #define EXT_CSD_BOOT_PART_NUM(x) (x << 3) >>> #define EXT_CSD_PARTITION_ACCESS(x) (x << 0) >>> >>> +#define EXT_CSD_EXTRACT_BOOT_ACK(x) ((x >> 6) & 1) >> >> Should be (((x) >> 6) & 1) , notice the parenthesis around x. >> > > Ok FTR I had reviewed your use cases and they looked okay, but Marek is right that parenthesis are safer in such expressions. Cheers, Andreas
diff --git a/cmd/mmc.c b/cmd/mmc.c index 39ef072..de28749 100644 --- a/cmd/mmc.c +++ b/cmd/mmc.c @@ -697,6 +697,24 @@ static int do_mmc_boot_resize(cmd_tbl_t *cmdtp, int flag, printf("EMMC RPMB partition Size %d MB\n", rpmbsize); return CMD_RET_SUCCESS; } + +static int do_mmc_partconf_read(struct mmc *mmc) +{ + int err; + u8 ack, part_num, access; + + err = mmc_get_part_conf(mmc, &ack, &part_num, &access); + if (err) + return CMD_RET_FAILURE; + + puts("EXT_CSD[179], PARTITION_CONFIG register:\n"); + printf("BOOT_ACK: %d\n", ack); + printf("BOOT_PARTITION_ENABLE: %d\n", part_num); + printf("PARTITION_ACCESS: %d\n", access); + + return 0; +} + static int do_mmc_partconf(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { @@ -704,13 +722,10 @@ static int do_mmc_partconf(cmd_tbl_t *cmdtp, int flag, struct mmc *mmc; u8 ack, part_num, access; - if (argc != 5) + if (argc != 2 && argc != 5) return CMD_RET_USAGE; dev = simple_strtoul(argv[1], NULL, 10); - ack = simple_strtoul(argv[2], NULL, 10); - part_num = simple_strtoul(argv[3], NULL, 10); - access = simple_strtoul(argv[4], NULL, 10); mmc = init_mmc_device(dev, false); if (!mmc) @@ -721,9 +736,17 @@ static int do_mmc_partconf(cmd_tbl_t *cmdtp, int flag, return CMD_RET_FAILURE; } + if (argc == 2) + return do_mmc_partconf_read(mmc); + + ack = simple_strtoul(argv[2], NULL, 10); + part_num = simple_strtoul(argv[3], NULL, 10); + access = simple_strtoul(argv[4], NULL, 10); + /* acknowledge to be sent during boot operation */ return mmc_set_part_conf(mmc, ack, part_num, access); } + static int do_mmc_rst_func(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { @@ -858,8 +881,8 @@ U_BOOT_CMD( " - Set the BOOT_BUS_WIDTH field of the specified device\n" "mmc bootpart-resize <dev> <boot part size MB> <RPMB part size MB>\n" " - Change sizes of boot and RPMB partitions of specified device\n" - "mmc partconf dev boot_ack boot_partition partition_access\n" - " - Change the bits of the PARTITION_CONFIG field of the specified device\n" + "mmc partconf dev [boot_ack boot_partition partition_access]\n" + " - Show or change the bits of the PARTITION_CONFIG field of the specified device\n" "mmc rst-function dev value\n" " - Change the RST_n_FUNCTION field of the specified device\n" " WARNING: This is a write-once field and 0 / 1 / 2 are the only valid values.\n" diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index d3c22ab..4cb3706 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -1954,6 +1954,39 @@ int mmc_set_part_conf(struct mmc *mmc, u8 ack, u8 part_num, u8 access) } /* + * Read EXT_CSD[179] which is PARTITION_CONFIG (formerly BOOT_CONFIG) + * and returning the extracted fields BOOT_ACK, BOOT_PARTITION_ENABLE and + * PARTITION_ACCESS. + * + * Returns 0 on success. + */ +int mmc_get_part_conf(struct mmc *mmc, u8 *ack, u8 *part_num, u8 *access) +{ + int err; + u8 part_conf; + + ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN); + + mmc->erase_grp_size = 1; + mmc->part_config = MMCPART_NOAVAILABLE; + + if (IS_SD(mmc) || (mmc->version < MMC_VERSION_4)) + return -1; + + err = mmc_send_ext_csd(mmc, ext_csd); + if (err) + return err; + + part_conf = ext_csd[EXT_CSD_PART_CONF]; + + *ack = EXT_CSD_EXTRACT_BOOT_ACK(part_conf); + *part_num = EXT_CSD_EXTRACT_BOOT_PART(part_conf); + *access = EXT_CSD_EXTRACT_PARTITION_ACCESS(part_conf); + + return 0; +} + +/* * Modify EXT_CSD[162] which is RST_n_FUNCTION based on the given value * for enable. Note that this is a write-once field for non-zero values. * diff --git a/include/mmc.h b/include/mmc.h index cdb56e7..4b34b31 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -222,6 +222,10 @@ #define EXT_CSD_BOOT_PART_NUM(x) (x << 3) #define EXT_CSD_PARTITION_ACCESS(x) (x << 0) +#define EXT_CSD_EXTRACT_BOOT_ACK(x) ((x >> 6) & 1) +#define EXT_CSD_EXTRACT_BOOT_PART(x) ((x >> 3) & 0x7) +#define EXT_CSD_EXTRACT_PARTITION_ACCESS(x) (x & 0x7) + #define EXT_CSD_BOOT_BUS_WIDTH_MODE(x) (x << 3) #define EXT_CSD_BOOT_BUS_WIDTH_RESET(x) (x << 2) #define EXT_CSD_BOOT_BUS_WIDTH_WIDTH(x) (x) @@ -428,6 +432,8 @@ int mmc_boot_partition_size_change(struct mmc *mmc, unsigned long bootsize, unsigned long rpmbsize); /* Function to modify the PARTITION_CONFIG field of EXT_CSD */ int mmc_set_part_conf(struct mmc *mmc, u8 ack, u8 part_num, u8 access); +/* Function to read back the PARTITION_CONFIG field of EXT_CSD */ +int mmc_get_part_conf(struct mmc *mmc, u8 *ack, u8 *part_num, u8 *access); /* Function to modify the BOOT_BUS_WIDTH field of EXT_CSD */ int mmc_set_boot_bus_width(struct mmc *mmc, u8 width, u8 reset, u8 mode); /* Function to modify the RST_n_FUNCTION field of EXT_CSD */
This patch allows to read back the EXT_CSD[179] partition_config register, just specifying the dev param: U-Boot> mmc partconf 0 EXT_CSD[179], PARTITION_CONFIG register: BOOT_ACK: 0 BOOT_PARTITION_ENABLE: 0 PARTITION_ACCESS: 0 Signed-off-by: Angelo Dureghello <angelo@sysam.it> --- Changes for v2: - fixed commit message - added white lines in cmd/mmc.c - fixed help in cmd/mmc.c --- cmd/mmc.c | 35 +++++++++++++++++++++++++++++------ drivers/mmc/mmc.c | 33 +++++++++++++++++++++++++++++++++ include/mmc.h | 6 ++++++ 3 files changed, 68 insertions(+), 6 deletions(-)