| Message ID | 1343417387-13953-5-git-send-email-i.mitsyanko@samsung.com |
|---|---|
| State | New |
| Headers | show |
Igor Mitsyanko <i.mitsyanko@samsung.com> writes: > Standard capacity cards SDSC use byte unit address while SDHC and SDXC Cards use > block unit address (512 bytes) when setting ERASE_START and ERASE_END with CMD32 > and CMD33, we have to account for this. > > Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com> > --- > hw/sd.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/hw/sd.c b/hw/sd.c > index d0674d5..f7aa580 100644 > --- a/hw/sd.c > +++ b/hw/sd.c > @@ -482,8 +482,10 @@ static void sd_erase(SDState *sd) > return; > } > > - start = sd_addr_to_wpnum(sd->erase_start); > - end = sd_addr_to_wpnum(sd->erase_end); > + start = sd_addr_to_wpnum(sd->ocr & (1 << 30) ? > + (uint64_t)sd->erase_start * 512 : sd->erase_start); > + end = sd_addr_to_wpnum(sd->ocr & (1 << 30) ? > + (uint64_t)sd->erase_end * 512 : sd->erase_end); > sd->erase_start = 0; > sd->erase_end = 0; > sd->csd[14] |= 0x40; Is this a bug fix? If yes, I recommend to state that clearly in the subject, say "hw/sd.c: Fix erase for high capacity cards".
On 07/31/2012 01:29 PM, Markus Armbruster wrote: > Igor Mitsyanko <i.mitsyanko@samsung.com> writes: > >> Standard capacity cards SDSC use byte unit address while SDHC and SDXC Cards use >> block unit address (512 bytes) when setting ERASE_START and ERASE_END with CMD32 >> and CMD33, we have to account for this. >> >> Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com> >> --- >> hw/sd.c | 6 ++++-- >> 1 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/hw/sd.c b/hw/sd.c >> index d0674d5..f7aa580 100644 >> --- a/hw/sd.c >> +++ b/hw/sd.c >> @@ -482,8 +482,10 @@ static void sd_erase(SDState *sd) >> return; >> } >> >> - start = sd_addr_to_wpnum(sd->erase_start); >> - end = sd_addr_to_wpnum(sd->erase_end); >> + start = sd_addr_to_wpnum(sd->ocr & (1 << 30) ? >> + (uint64_t)sd->erase_start * 512 : sd->erase_start); >> + end = sd_addr_to_wpnum(sd->ocr & (1 << 30) ? >> + (uint64_t)sd->erase_end * 512 : sd->erase_end); >> sd->erase_start = 0; >> sd->erase_end = 0; >> sd->csd[14] |= 0x40; > Is this a bug fix? > > If yes, I recommend to state that clearly in the subject, say "hw/sd.c: > Fix erase for high capacity cards". > Yes it is, I'll change subject in next version then.
On 27 July 2012 20:29, Igor Mitsyanko <i.mitsyanko@samsung.com> wrote: > Standard capacity cards SDSC use byte unit address while SDHC and SDXC Cards use > block unit address (512 bytes) when setting ERASE_START and ERASE_END with CMD32 > and CMD33, we have to account for this. > > Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com> > --- > hw/sd.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/hw/sd.c b/hw/sd.c > index d0674d5..f7aa580 100644 > --- a/hw/sd.c > +++ b/hw/sd.c > @@ -482,8 +482,10 @@ static void sd_erase(SDState *sd) > return; > } > > - start = sd_addr_to_wpnum(sd->erase_start); > - end = sd_addr_to_wpnum(sd->erase_end); > + start = sd_addr_to_wpnum(sd->ocr & (1 << 30) ? > + (uint64_t)sd->erase_start * 512 : sd->erase_start); > + end = sd_addr_to_wpnum(sd->ocr & (1 << 30) ? > + (uint64_t)sd->erase_end * 512 : sd->erase_end); I think this would be a little clearer phrased as: uint64_t erase_start = sd->erase_start; uint64_t erase_end = sd->erase_end; [...] if (extract32(sd->ocr, 30, 1)) { /* High capacity memory card: erase units are 512 byte blocks */ erase_start *= 512; erase_end *= 512; } start = sd_addr_to_wpnum(erase_start); end = sd_addr_to_wpnum(erase_end); (I don't insist on the extract32() if you don't like it, but I would like to avoid the repeated inline ?: ops.) -- PMM
On 07/31/2012 06:34 PM, Peter Maydell wrote: > On 27 July 2012 20:29, Igor Mitsyanko <i.mitsyanko@samsung.com> wrote: >> Standard capacity cards SDSC use byte unit address while SDHC and SDXC Cards use >> block unit address (512 bytes) when setting ERASE_START and ERASE_END with CMD32 >> and CMD33, we have to account for this. >> >> Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com> >> --- >> hw/sd.c | 6 ++++-- >> 1 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/hw/sd.c b/hw/sd.c >> index d0674d5..f7aa580 100644 >> --- a/hw/sd.c >> +++ b/hw/sd.c >> @@ -482,8 +482,10 @@ static void sd_erase(SDState *sd) >> return; >> } >> >> - start = sd_addr_to_wpnum(sd->erase_start); >> - end = sd_addr_to_wpnum(sd->erase_end); >> + start = sd_addr_to_wpnum(sd->ocr & (1 << 30) ? >> + (uint64_t)sd->erase_start * 512 : sd->erase_start); >> + end = sd_addr_to_wpnum(sd->ocr & (1 << 30) ? >> + (uint64_t)sd->erase_end * 512 : sd->erase_end); > > I think this would be a little clearer phrased as: > > uint64_t erase_start = sd->erase_start; > uint64_t erase_end = sd->erase_end; > > [...] > > if (extract32(sd->ocr, 30, 1)) { > /* High capacity memory card: erase units are 512 byte blocks */ > erase_start *= 512; > erase_end *= 512; > } > > start = sd_addr_to_wpnum(erase_start); > end = sd_addr_to_wpnum(erase_end); > > (I don't insist on the extract32() if you don't like it, but I > would like to avoid the repeated inline ?: ops.) > > -- PMM > > I like it, I extensively use your new fancy extract/deposit api in my local work.
diff --git a/hw/sd.c b/hw/sd.c index d0674d5..f7aa580 100644 --- a/hw/sd.c +++ b/hw/sd.c @@ -482,8 +482,10 @@ static void sd_erase(SDState *sd) return; } - start = sd_addr_to_wpnum(sd->erase_start); - end = sd_addr_to_wpnum(sd->erase_end); + start = sd_addr_to_wpnum(sd->ocr & (1 << 30) ? + (uint64_t)sd->erase_start * 512 : sd->erase_start); + end = sd_addr_to_wpnum(sd->ocr & (1 << 30) ? + (uint64_t)sd->erase_end * 512 : sd->erase_end); sd->erase_start = 0; sd->erase_end = 0; sd->csd[14] |= 0x40;
Standard capacity cards SDSC use byte unit address while SDHC and SDXC Cards use block unit address (512 bytes) when setting ERASE_START and ERASE_END with CMD32 and CMD33, we have to account for this. Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com> --- hw/sd.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)