| Message ID | 1408356012-18867-1-git-send-email-Barry.Song@csr.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 18 August 2014 12:00, Barry Song <Barry.Song@csr.com> wrote: > From: Minda Chen <Minda.Chen@csr.com> > > After suspending, unplug the sdcard, and set sd WP lock, > insert it again, then resume the system. resume codes do > not check the the sdcard write-proctect lock. now check > it. > > Signed-off-by: Minda Chen <Minda.Chen@csr.com> > Signed-off-by: Barry Song <Baohua.Song@csr.com> > --- > drivers/mmc/core/sd.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > index 0c44510..890557a 100644 > --- a/drivers/mmc/core/sd.c > +++ b/drivers/mmc/core/sd.c > @@ -910,6 +910,7 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr, > int err; > u32 cid[4]; > u32 rocr = 0; > + bool oldro, ro; > > BUG_ON(!host); > WARN_ON(!host->claimed); > @@ -922,6 +923,12 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr, > if (memcmp(cid, oldcard->raw_cid, sizeof(cid)) != 0) > return -ENOENT; > > + if (host->ops->get_ro) { > + ro = host->ops->get_ro(host) ? true : false; > + oldro = mmc_card_readonly(oldcard) ? true : false; > + if (oldro ^ ro) > + return -ENOENT; > + } Seems like this code belongs better in mmc_sd_setup_card(). Could you try moving it there. Kind regards Uffe > card = oldcard; > } else { > /* > -- > 2.0.4 > > > > Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom > More information can be found at www.csr.com. Keep up to date with CSR on our technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, www.youtube.com/user/CSRplc, Facebook, www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at www.twitter.com/CSR_plc. > New for 2014, you can now access the wide range of products powered by aptX at www.aptx.com. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBVbGYgSGFu c3NvbiBbbWFpbHRvOnVsZi5oYW5zc29uQGxpbmFyby5vcmddDQo+IFNlbnQ6 IE1vbmRheSwgQXVndXN0IDE4LCAyMDE0IDc6NTggUE0NCj4gVG86IEJhcnJ5 IFNvbmcNCj4gQ2M6IENocmlzIEJhbGw7IGxpbnV4LW1tYzsgbGludXgtYXJt LWtlcm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnOyBETC1TSEEtDQo+IFdvcmtH cm91cExpbnV4OyBNaW5kYSBDaGVuOyBCYXJyeSBTb25nDQo+IFN1YmplY3Q6 IFJlOiBbUEFUQ0hdIG1tYzogY29yZTogc2Q6IGNoZWNrIGNhcmQgd3JpdGUt cHJvdGVjdCBsb2NrIHdoaWxlDQo+IHJlc3VtaW5nDQo+IA0KPiBPbiAxOCBB dWd1c3QgMjAxNCAxMjowMCwgQmFycnkgU29uZyA8QmFycnkuU29uZ0Bjc3Iu Y29tPiB3cm90ZToNCj4gPiBGcm9tOiBNaW5kYSBDaGVuIDxNaW5kYS5DaGVu QGNzci5jb20+DQo+ID4NCj4gPiBBZnRlciBzdXNwZW5kaW5nLCB1bnBsdWcg dGhlIHNkY2FyZCwgYW5kIHNldCBzZCBXUCBsb2NrLCBpbnNlcnQgaXQNCj4g PiBhZ2FpbiwgdGhlbiByZXN1bWUgdGhlIHN5c3RlbS4gcmVzdW1lIGNvZGVz IGRvIG5vdCBjaGVjayB0aGUgdGhlDQo+ID4gc2RjYXJkIHdyaXRlLXByb2N0 ZWN0IGxvY2suIG5vdyBjaGVjayBpdC4NCj4gPg0KPiA+IFNpZ25lZC1vZmYt Ynk6IE1pbmRhIENoZW4gPE1pbmRhLkNoZW5AY3NyLmNvbT4NCj4gPiBTaWdu ZWQtb2ZmLWJ5OiBCYXJyeSBTb25nIDxCYW9odWEuU29uZ0Bjc3IuY29tPg0K PiA+IC0tLQ0KPiA+ICBkcml2ZXJzL21tYy9jb3JlL3NkLmMgfCA3ICsrKysr KysNCj4gPiAgMSBmaWxlIGNoYW5nZWQsIDcgaW5zZXJ0aW9ucygrKQ0KPiA+ DQo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvbW1jL2NvcmUvc2QuYyBiL2Ry aXZlcnMvbW1jL2NvcmUvc2QuYyBpbmRleA0KPiA+IDBjNDQ1MTAuLjg5MDU1 N2EgMTAwNjQ0DQo+ID4gLS0tIGEvZHJpdmVycy9tbWMvY29yZS9zZC5jDQo+ ID4gKysrIGIvZHJpdmVycy9tbWMvY29yZS9zZC5jDQo+ID4gQEAgLTkxMCw2 ICs5MTAsNyBAQCBzdGF0aWMgaW50IG1tY19zZF9pbml0X2NhcmQoc3RydWN0 IG1tY19ob3N0DQo+ICpob3N0LCB1MzIgb2NyLA0KPiA+ICAgICAgICAgaW50 IGVycjsNCj4gPiAgICAgICAgIHUzMiBjaWRbNF07DQo+ID4gICAgICAgICB1 MzIgcm9jciA9IDA7DQo+ID4gKyAgICAgICBib29sIG9sZHJvLCBybzsNCj4g Pg0KPiA+ICAgICAgICAgQlVHX09OKCFob3N0KTsNCj4gPiAgICAgICAgIFdB Uk5fT04oIWhvc3QtPmNsYWltZWQpOw0KPiA+IEBAIC05MjIsNiArOTIzLDEy IEBAIHN0YXRpYyBpbnQgbW1jX3NkX2luaXRfY2FyZChzdHJ1Y3QgbW1jX2hv c3QNCj4gKmhvc3QsIHUzMiBvY3IsDQo+ID4gICAgICAgICAgICAgICAgIGlm IChtZW1jbXAoY2lkLCBvbGRjYXJkLT5yYXdfY2lkLCBzaXplb2YoY2lkKSkg IT0gMCkNCj4gPiAgICAgICAgICAgICAgICAgICAgICAgICByZXR1cm4gLUVO T0VOVDsNCj4gPg0KPiA+ICsgICAgICAgICAgICAgICBpZiAoaG9zdC0+b3Bz LT5nZXRfcm8pIHsNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICBybyA9 IGhvc3QtPm9wcy0+Z2V0X3JvKGhvc3QpID8gdHJ1ZSA6IGZhbHNlOw0KPiA+ ICsgICAgICAgICAgICAgICAgICAgICAgIG9sZHJvID0gbW1jX2NhcmRfcmVh ZG9ubHkob2xkY2FyZCkgPyB0cnVlIDogZmFsc2U7DQo+ID4gKyAgICAgICAg ICAgICAgICAgICAgICAgaWYgKG9sZHJvIF4gcm8pDQo+ID4gKyAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICByZXR1cm4gLUVOT0VOVDsNCj4gPiAr ICAgICAgICAgICAgICAgfQ0KPiANCj4gU2VlbXMgbGlrZSB0aGlzIGNvZGUg YmVsb25ncyBiZXR0ZXIgaW4gbW1jX3NkX3NldHVwX2NhcmQoKS4gQ291bGQg eW91IHRyeQ0KPiBtb3ZpbmcgaXQgdGhlcmUuDQpbQmFycnkgU29uZ10gDQpX ZWxsLiBIb3cgYWJvdXQ6DQoNCmRpZmYgLS1naXQgYS9kcml2ZXJzL21tYy9j b3JlL3NkLmMgYi9kcml2ZXJzL21tYy9jb3JlL3NkLmMNCmluZGV4IDBjNDQ1 MTAuLjY0M2JjODJkOCAxMDA2NDQNCi0tLSBhL2RyaXZlcnMvbW1jL2NvcmUv c2QuYw0KKysrIGIvZHJpdmVycy9tbWMvY29yZS9zZC5jDQpAQCAtODE1LDYg KzgxNSw3IEBAIGludCBtbWNfc2Rfc2V0dXBfY2FyZChzdHJ1Y3QgbW1jX2hv c3QgKmhvc3QsIHN0cnVjdCBtbWNfY2FyZCAqY2FyZCwNCiAJYm9vbCByZWlu aXQpDQogew0KIAlpbnQgZXJyOw0KKwlpbnQgb2xkcm8sIHJvID0gLTE7DQog DQogCWlmICghcmVpbml0KSB7DQogCQkvKg0KQEAgLTg2MSwyMyArODYyLDI4 IEBAIGludCBtbWNfc2Rfc2V0dXBfY2FyZChzdHJ1Y3QgbW1jX2hvc3QgKmhv c3QsIHN0cnVjdCBtbWNfY2FyZCAqY2FyZCwNCiAJLyoNCiAJICogQ2hlY2sg aWYgcmVhZC1vbmx5IHN3aXRjaCBpcyBhY3RpdmUuDQogCSAqLw0KLQlpZiAo IXJlaW5pdCkgew0KLQkJaW50IHJvID0gLTE7DQogDQotCQlpZiAoaG9zdC0+ b3BzLT5nZXRfcm8pIHsNCi0JCQltbWNfaG9zdF9jbGtfaG9sZChjYXJkLT5o b3N0KTsNCi0JCQlybyA9IGhvc3QtPm9wcy0+Z2V0X3JvKGhvc3QpOw0KLQkJ CW1tY19ob3N0X2Nsa19yZWxlYXNlKGNhcmQtPmhvc3QpOw0KLQkJfQ0KKwlp ZiAoaG9zdC0+b3BzLT5nZXRfcm8pIHsNCisJCW1tY19ob3N0X2Nsa19ob2xk KGNhcmQtPmhvc3QpOw0KKwkJcm8gPSBob3N0LT5vcHMtPmdldF9ybyhob3N0 KTsNCisJCW1tY19ob3N0X2Nsa19yZWxlYXNlKGNhcmQtPmhvc3QpOw0KKwl9 DQogDQotCQlpZiAocm8gPCAwKSB7DQotCQkJcHJfd2FybmluZygiJXM6IGhv c3QgZG9lcyBub3QgIg0KLQkJCQkic3VwcG9ydCByZWFkaW5nIHJlYWQtb25s eSAiDQotCQkJCSJzd2l0Y2guIGFzc3VtaW5nIHdyaXRlLWVuYWJsZS5cbiIs DQotCQkJCW1tY19ob3N0bmFtZShob3N0KSk7DQotCQl9IGVsc2UgaWYgKHJv ID4gMCkgew0KLQkJCW1tY19jYXJkX3NldF9yZWFkb25seShjYXJkKTsNCi0J CX0NCisJaWYgKHJvIDwgMCkNCisJCXByX3dhcm5pbmcoIiVzOiBob3N0IGRv ZXMgbm90ICINCisJCQkic3VwcG9ydCByZWFkaW5nIHJlYWQtb25seSAiDQor CQkJInN3aXRjaC4gYXNzdW1pbmcgd3JpdGUtZW5hYmxlLlxuIiwNCisJCQlt bWNfaG9zdG5hbWUoaG9zdCkpOw0KKw0KKwlpZiAoIXJlaW5pdCAmJiAocm8g PiAwKSkNCisJCW1tY19jYXJkX3NldF9yZWFkb25seShjYXJkKTsNCisJZWxz ZSBpZiAocmVpbml0ICYmIChybyA+PSAwKSkgew0KKwkJLyogY2hlY2sgaWYg dGhlIHdyaXRlLXByb3RlY3Rpb24gbG9jayBpcyBjaGFuZ2VkICovDQorCQlv bGRybyA9IG1tY19jYXJkX3JlYWRvbmx5KGNhcmQpID8gMSA6IDA7DQorCQly byA9IChybyA+IDApID8gMSA6IDA7DQorDQorCQlpZiAob2xkcm8gXiBybykN CisJCQlyZXR1cm4gLUVOT0VOVDsNCiAJfQ0KIA0KIAlyZXR1cm4gMDsNCj4g DQo+IEtpbmQgcmVnYXJkcw0KPiBVZmZlDQo+IA0KPiA+ICAgICAgICAgICAg ICAgICBjYXJkID0gb2xkY2FyZDsNCj4gPiAgICAgICAgIH0gZWxzZSB7DQo+ ID4gICAgICAgICAgICAgICAgIC8qDQotYmFycnkNCg0KCgpNZW1iZXIgb2Yg dGhlIENTUiBwbGMgZ3JvdXAgb2YgY29tcGFuaWVzLiBDU1IgcGxjIHJlZ2lz dGVyZWQgaW4gRW5nbGFuZCBhbmQgV2FsZXMsIHJlZ2lzdGVyZWQgbnVtYmVy IDQxODczNDYsIHJlZ2lzdGVyZWQgb2ZmaWNlIENodXJjaGlsbCBIb3VzZSwg Q2FtYnJpZGdlIEJ1c2luZXNzIFBhcmssIENvd2xleSBSb2FkLCBDYW1icmlk Z2UsIENCNCAwV1osIFVuaXRlZCBLaW5nZG9tCk1vcmUgaW5mb3JtYXRpb24g Y2FuIGJlIGZvdW5kIGF0IHd3dy5jc3IuY29tLiBLZWVwIHVwIHRvIGRhdGUg d2l0aCBDU1Igb24gb3VyIHRlY2huaWNhbCBibG9nLCB3d3cuY3NyLmNvbS9i bG9nLCBDU1IgcGVvcGxlIGJsb2csIHd3dy5jc3IuY29tL3Blb3BsZSwgWW91 VHViZSwgd3d3LnlvdXR1YmUuY29tL3VzZXIvQ1NScGxjLCBGYWNlYm9vaywg d3d3LmZhY2Vib29rLmNvbS9wYWdlcy9DU1IvMTkxMDM4NDM0MjUzNTM0LCBv ciBmb2xsb3cgdXMgb24gVHdpdHRlciBhdCB3d3cudHdpdHRlci5jb20vQ1NS X3BsYy4KTmV3IGZvciAyMDE0LCB5b3UgY2FuIG5vdyBhY2Nlc3MgdGhlIHdp ZGUgcmFuZ2Ugb2YgcHJvZHVjdHMgcG93ZXJlZCBieSBhcHRYIGF0IHd3dy5h cHR4LmNvbS4K -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 22 August 2014 08:55, Barry Song <Barry.Song@csr.com> wrote: >> -----Original Message----- >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org] >> Sent: Monday, August 18, 2014 7:58 PM >> To: Barry Song >> Cc: Chris Ball; linux-mmc; linux-arm-kernel@lists.infradead.org; DL-SHA- >> WorkGroupLinux; Minda Chen; Barry Song >> Subject: Re: [PATCH] mmc: core: sd: check card write-protect lock while >> resuming >> >> On 18 August 2014 12:00, Barry Song <Barry.Song@csr.com> wrote: >> > From: Minda Chen <Minda.Chen@csr.com> >> > >> > After suspending, unplug the sdcard, and set sd WP lock, insert it >> > again, then resume the system. resume codes do not check the the >> > sdcard write-proctect lock. now check it. >> > >> > Signed-off-by: Minda Chen <Minda.Chen@csr.com> >> > Signed-off-by: Barry Song <Baohua.Song@csr.com> >> > --- >> > drivers/mmc/core/sd.c | 7 +++++++ >> > 1 file changed, 7 insertions(+) >> > >> > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index >> > 0c44510..890557a 100644 >> > --- a/drivers/mmc/core/sd.c >> > +++ b/drivers/mmc/core/sd.c >> > @@ -910,6 +910,7 @@ static int mmc_sd_init_card(struct mmc_host >> *host, u32 ocr, >> > int err; >> > u32 cid[4]; >> > u32 rocr = 0; >> > + bool oldro, ro; >> > >> > BUG_ON(!host); >> > WARN_ON(!host->claimed); >> > @@ -922,6 +923,12 @@ static int mmc_sd_init_card(struct mmc_host >> *host, u32 ocr, >> > if (memcmp(cid, oldcard->raw_cid, sizeof(cid)) != 0) >> > return -ENOENT; >> > >> > + if (host->ops->get_ro) { >> > + ro = host->ops->get_ro(host) ? true : false; >> > + oldro = mmc_card_readonly(oldcard) ? true : false; >> > + if (oldro ^ ro) >> > + return -ENOENT; >> > + } >> >> Seems like this code belongs better in mmc_sd_setup_card(). Could you try >> moving it there. > [Barry Song] > Well. How about: > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > index 0c44510..643bc82d8 100644 > --- a/drivers/mmc/core/sd.c > +++ b/drivers/mmc/core/sd.c > @@ -815,6 +815,7 @@ int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card, > bool reinit) > { > int err; > + int oldro, ro = -1; > > if (!reinit) { > /* > @@ -861,23 +862,28 @@ int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card, > /* > * Check if read-only switch is active. > */ > - if (!reinit) { > - int ro = -1; > > - if (host->ops->get_ro) { > - mmc_host_clk_hold(card->host); > - ro = host->ops->get_ro(host); > - mmc_host_clk_release(card->host); > - } > + if (host->ops->get_ro) { > + mmc_host_clk_hold(card->host); > + ro = host->ops->get_ro(host); > + mmc_host_clk_release(card->host); > + } > > - if (ro < 0) { > - pr_warning("%s: host does not " > - "support reading read-only " > - "switch. assuming write-enable.\n", > - mmc_hostname(host)); > - } else if (ro > 0) { > - mmc_card_set_readonly(card); > - } > + if (ro < 0) > + pr_warning("%s: host does not " > + "support reading read-only " > + "switch. assuming write-enable.\n", > + mmc_hostname(host)); I don't wont this pr_warning to spam the log each resume. I suppose we could print it only for !reinit, right? > + > + if (!reinit && (ro > 0)) > + mmc_card_set_readonly(card); > + else if (reinit && (ro >= 0)) { > + /* check if the write-protection lock is changed */ > + oldro = mmc_card_readonly(card) ? 1 : 0; > + ro = (ro > 0) ? 1 : 0; > + > + if (oldro ^ ro) > + return -ENOENT; This will have the effect of quickly returning an error to the mmc bus' ->suspend callback. That path are not able remove the card device, which I guess is what you would like to happen, right? I am not completely sure, but I believe the card will still be operational after resuming. Moreover it will be using the initialization frequency and 1-bit bus width and thus performing poorly. This is definitely not what we want. How did you test this patch? Can you confirm my theory above? The card may only be removed from the mmc_rescan work, scheduled at PM_POST_SUSPEND phase. What I think you should do here, is to also set the card state into MMC_CARD_REMOVED, by invoking mmc_card_set_removed(). Thus the SD's bus_ops->detect callback will remove the card during this phase. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 0c44510..890557a 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -910,6 +910,7 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr, int err; u32 cid[4]; u32 rocr = 0; + bool oldro, ro; BUG_ON(!host); WARN_ON(!host->claimed); @@ -922,6 +923,12 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr, if (memcmp(cid, oldcard->raw_cid, sizeof(cid)) != 0) return -ENOENT; + if (host->ops->get_ro) { + ro = host->ops->get_ro(host) ? true : false; + oldro = mmc_card_readonly(oldcard) ? true : false; + if (oldro ^ ro) + return -ENOENT; + } card = oldcard; } else { /*