| Message ID | 20200330052419.3804-4-xypron.glpk@gmx.de |
|---|---|
| State | Accepted, archived |
| Commit | d5210e4589294b4c356e7c2ac598cda8d738aec8 |
| Headers | show |
| Series | mmc: manage boot area protection | expand |
Hi, On 3/30/20 2:24 PM, Heinrich Schuchardt wrote: > Boot partitions of eMMC devices can be power on or permanently write > protected. Let the 'mmc info' command display the protection state. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > cmd/mmc.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/cmd/mmc.c b/cmd/mmc.c > index 6f3cb85cc0..d62c85e439 100644 > --- a/cmd/mmc.c > +++ b/cmd/mmc.c > @@ -54,6 +54,8 @@ static void print_mmcinfo(struct mmc *mmc) > if (!IS_SD(mmc) && mmc->version >= MMC_VERSION_4_41) { > bool has_enh = (mmc->part_support & ENHNCD_SUPPORT) != 0; > bool usr_enh = has_enh && (mmc->part_attr & EXT_CSD_ENH_USR); > + u8 wp, ext_csd[MMC_MAX_BLOCK_LEN]; > + int ret; > > #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING) > puts("HC WP Group Size: "); > @@ -90,6 +92,28 @@ static void print_mmcinfo(struct mmc *mmc) > putc('\n'); > } > } > + ret = mmc_send_ext_csd(mmc, ext_csd); > + if (ret) > + return; Is it really needed to call mmc_send_ext_csd() at here. ext_csd register value was already read somewhere. Best Regards, Jaehoon Chung > + wp = ext_csd[EXT_CSD_BOOT_WP_STATUS]; > + for (i = 0; i < 2; ++i) { > + printf("Boot area %d is ", i); > + switch (wp & 3) { > + case 0: > + printf("not write protected\n"); > + break; > + case 1: > + printf("power on protected\n"); > + break; > + case 2: > + printf("permanently protected\n"); > + break; > + default: > + printf("in reserved protection state\n"); > + break; > + } > + wp >>= 2; > + } > } > } > static struct mmc *init_mmc_device(int dev, bool force_init) > -- > 2.25.1 > > >
On 4/1/20 5:04 AM, Jaehoon Chung wrote: > Hi, > > On 3/30/20 2:24 PM, Heinrich Schuchardt wrote: >> Boot partitions of eMMC devices can be power on or permanently write >> protected. Let the 'mmc info' command display the protection state. >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >> --- >> cmd/mmc.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/cmd/mmc.c b/cmd/mmc.c >> index 6f3cb85cc0..d62c85e439 100644 >> --- a/cmd/mmc.c >> +++ b/cmd/mmc.c >> @@ -54,6 +54,8 @@ static void print_mmcinfo(struct mmc *mmc) >> if (!IS_SD(mmc) && mmc->version >= MMC_VERSION_4_41) { >> bool has_enh = (mmc->part_support & ENHNCD_SUPPORT) != 0; >> bool usr_enh = has_enh && (mmc->part_attr & EXT_CSD_ENH_USR); >> + u8 wp, ext_csd[MMC_MAX_BLOCK_LEN]; >> + int ret; >> >> #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING) >> puts("HC WP Group Size: "); >> @@ -90,6 +92,28 @@ static void print_mmcinfo(struct mmc *mmc) >> putc('\n'); >> } >> } >> + ret = mmc_send_ext_csd(mmc, ext_csd); >> + if (ret) >> + return; > > Is it really needed to call mmc_send_ext_csd() at here. > ext_csd register value was already read somewhere. struct mmc does not contain the extended CSD. Performancewise it does not make sense to carry a copy which would have to be updated after each command that could potentially update it like SWITCH. The call to mmc_send_ext_csd() does not lead to any visible performance issue here. Best regards Heinrich > > Best Regards, > Jaehoon Chung > >> + wp = ext_csd[EXT_CSD_BOOT_WP_STATUS]; >> + for (i = 0; i < 2; ++i) { >> + printf("Boot area %d is ", i); >> + switch (wp & 3) { >> + case 0: >> + printf("not write protected\n"); >> + break; >> + case 1: >> + printf("power on protected\n"); >> + break; >> + case 2: >> + printf("permanently protected\n"); >> + break; >> + default: >> + printf("in reserved protection state\n"); >> + break; >> + } >> + wp >>= 2; >> + } >> } >> } >> static struct mmc *init_mmc_device(int dev, bool force_init) >> -- >> 2.25.1 >> >> >> >
On 4/1/20 3:05 PM, Heinrich Schuchardt wrote: > On 4/1/20 5:04 AM, Jaehoon Chung wrote: >> Hi, >> >> On 3/30/20 2:24 PM, Heinrich Schuchardt wrote: >>> Boot partitions of eMMC devices can be power on or permanently write >>> protected. Let the 'mmc info' command display the protection state. >>> >>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >>> --- >>> cmd/mmc.c | 24 ++++++++++++++++++++++++ >>> 1 file changed, 24 insertions(+) >>> >>> diff --git a/cmd/mmc.c b/cmd/mmc.c >>> index 6f3cb85cc0..d62c85e439 100644 >>> --- a/cmd/mmc.c >>> +++ b/cmd/mmc.c >>> @@ -54,6 +54,8 @@ static void print_mmcinfo(struct mmc *mmc) >>> if (!IS_SD(mmc) && mmc->version >= MMC_VERSION_4_41) { >>> bool has_enh = (mmc->part_support & ENHNCD_SUPPORT) != 0; >>> bool usr_enh = has_enh && (mmc->part_attr & EXT_CSD_ENH_USR); >>> + u8 wp, ext_csd[MMC_MAX_BLOCK_LEN]; >>> + int ret; >>> >>> #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING) >>> puts("HC WP Group Size: "); >>> @@ -90,6 +92,28 @@ static void print_mmcinfo(struct mmc *mmc) >>> putc('\n'); >>> } >>> } >>> + ret = mmc_send_ext_csd(mmc, ext_csd); >>> + if (ret) >>> + return; >> >> Is it really needed to call mmc_send_ext_csd() at here. >> ext_csd register value was already read somewhere. > > struct mmc does not contain the extended CSD. Performancewise it does > not make sense to carry a copy which would have to be updated after each > command that could potentially update it like SWITCH. Why doesn't make sense to carry a copy? Its value can be updated whenever it's changed. If someone want to set "write protect", then it has to follow sequence. read ext_csd -> check value -> send_switch command Then it's possible to keep its value.. And some values are one-time programmable. it's not efficient about sending command to read again. Best Regards, Jaehoon Chung > > The call to mmc_send_ext_csd() does not lead to any visible performance > issue here. > > Best regards > > Heinrich > >> >> Best Regards, >> Jaehoon Chung >> >>> + wp = ext_csd[EXT_CSD_BOOT_WP_STATUS]; >>> + for (i = 0; i < 2; ++i) { >>> + printf("Boot area %d is ", i); >>> + switch (wp & 3) { >>> + case 0: >>> + printf("not write protected\n"); >>> + break; >>> + case 1: >>> + printf("power on protected\n"); >>> + break; >>> + case 2: >>> + printf("permanently protected\n"); >>> + break; >>> + default: >>> + printf("in reserved protection state\n"); >>> + break; >>> + } >>> + wp >>= 2; >>> + } >>> } >>> } >>> static struct mmc *init_mmc_device(int dev, bool force_init) >>> -- >>> 2.25.1 >>> >>> >>> >> > > >
diff --git a/cmd/mmc.c b/cmd/mmc.c index 6f3cb85cc0..d62c85e439 100644 --- a/cmd/mmc.c +++ b/cmd/mmc.c @@ -54,6 +54,8 @@ static void print_mmcinfo(struct mmc *mmc) if (!IS_SD(mmc) && mmc->version >= MMC_VERSION_4_41) { bool has_enh = (mmc->part_support & ENHNCD_SUPPORT) != 0; bool usr_enh = has_enh && (mmc->part_attr & EXT_CSD_ENH_USR); + u8 wp, ext_csd[MMC_MAX_BLOCK_LEN]; + int ret; #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING) puts("HC WP Group Size: "); @@ -90,6 +92,28 @@ static void print_mmcinfo(struct mmc *mmc) putc('\n'); } } + ret = mmc_send_ext_csd(mmc, ext_csd); + if (ret) + return; + wp = ext_csd[EXT_CSD_BOOT_WP_STATUS]; + for (i = 0; i < 2; ++i) { + printf("Boot area %d is ", i); + switch (wp & 3) { + case 0: + printf("not write protected\n"); + break; + case 1: + printf("power on protected\n"); + break; + case 2: + printf("permanently protected\n"); + break; + default: + printf("in reserved protection state\n"); + break; + } + wp >>= 2; + } } } static struct mmc *init_mmc_device(int dev, bool force_init)
Boot partitions of eMMC devices can be power on or permanently write protected. Let the 'mmc info' command display the protection state. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- cmd/mmc.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) -- 2.25.1