| Message ID | 51dde5250305623c420e78fa555800ef8fef0a09.1417056337.git.micky_ching@realsil.com.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 27, 2014 at 10:53:58AM +0800, micky_ching@realsil.com.cn wrote: > +static inline void rtsx_pci_write_be32(struct rtsx_pcr *pcr, u16 reg, u32 val) > +{ > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, val >> 24); > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, val >> 16); > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, val >> 8); > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, val); This assumes the cpu is little endian. First convert to big endian using cpu_to_be32() and then write it out. __be32 be_val = cpu_to_be32() rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, be_val); rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, be_val >> 8); rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, be_val >> 16); rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, be_val >> 24); (Written hurredly in my mail client. May be wrong). > +} > + > +static inline void rtsx_pci_write_le32(struct rtsx_pcr *pcr, u16 reg, u32 val) > +{ > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, val); > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, val >> 8); > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, val >> 16); > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, val >> 24); > +} We don't have a user for rtsx_pci_write_le32() so don't add it. regards, dan carpenter -- 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
DQpPbiAxMS8yNy8yMDE0IDExOjIzIFBNLCBEYW4gQ2FycGVudGVyIHdyb3RlOg0KPj4gK3N0YXRp YyBpbmxpbmUgdm9pZCBydHN4X3BjaV93cml0ZV9iZTMyKHN0cnVjdCBydHN4X3BjciAqcGNyLCB1 MTYgcmVnLCB1MzIgdmFsKQ0KPj4gPit7DQo+PiA+KwlydHN4X3BjaV9hZGRfY21kKHBjciwgV1JJ VEVfUkVHX0NNRCwgcmVnLCAgICAgMHhGRiwgdmFsID4+IDI0KTsNCj4+ID4rCXJ0c3hfcGNpX2Fk ZF9jbWQocGNyLCBXUklURV9SRUdfQ01ELCByZWcgKyAxLCAweEZGLCB2YWwgPj4gMTYpOw0KPj4g PisJcnRzeF9wY2lfYWRkX2NtZChwY3IsIFdSSVRFX1JFR19DTUQsIHJlZyArIDIsIDB4RkYsIHZh bCA+PiA4KTsNCj4+ID4rCXJ0c3hfcGNpX2FkZF9jbWQocGNyLCBXUklURV9SRUdfQ01ELCByZWcg KyAzLCAweEZGLCB2YWwpOw0KPiBUaGlzIGFzc3VtZXMgdGhlIGNwdSBpcyBsaXR0bGUgZW5kaWFu LiAgRmlyc3QgY29udmVydCB0byBiaWcgZW5kaWFuDQo+IHVzaW5nIGNwdV90b19iZTMyKCkgYW5k IHRoZW4gd3JpdGUgaXQgb3V0Lg0KPg0KPiAJX19iZTMyIGJlX3ZhbCA9IGNwdV90b19iZTMyKCkN Cj4NCj4gCXJ0c3hfcGNpX2FkZF9jbWQocGNyLCBXUklURV9SRUdfQ01ELCByZWcsICAgICAweEZG LCBiZV92YWwpOw0KPiAJcnRzeF9wY2lfYWRkX2NtZChwY3IsIFdSSVRFX1JFR19DTUQsIHJlZyAr IDEsIDB4RkYsIGJlX3ZhbCA+PiA4KTsNCj4gCXJ0c3hfcGNpX2FkZF9jbWQocGNyLCBXUklURV9S RUdfQ01ELCByZWcgKyAyLCAweEZGLCBiZV92YWwgPj4gMTYpOw0KPiAJcnRzeF9wY2lfYWRkX2Nt ZChwY3IsIFdSSVRFX1JFR19DTUQsIHJlZyArIDMsIDB4RkYsIGJlX3ZhbCA+PiAyNCk7DQo+DQo+ IChXcml0dGVuIGh1cnJlZGx5IGluIG15IG1haWwgY2xpZW50LiAgTWF5IGJlIHdyb25nKS4NCj4N CkkgdGhpbmsgd2UgYmV0dGVyIG5vdCB1c2UgY3B1X3RvX2JlMzIoKSBoZXJlLCBsZWF2ZSB0aGUg d29yayB0byBjYWxsZXINCm1heSBiZSBiZXR0ZXIuDQoNCmVnLCBpbiBzZF9vcHMuYyB0aGUgY21k LmFyZyBpcyBjb25zdHJ1Y3RlZCBiaXQgYnkgYml0LCB3ZSBjYW4gcHV0IHRoZSByaWdodA0KYnl0 ZSB0byB0aGUgcmlnaHQgcmVnaXN0ZXIgYnkgc2hpZnQsIHNvIHRoZSBlbmRpYW4gY2hlY2sgaXMg bm90IG5lZWQu -- 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 Fri, Nov 28, 2014 at 02:10:36AM +0000, ?? wrote: > eg, in sd_ops.c the cmd.arg is constructed bit by bit, we can put the right > byte to the right register by shift, so the endian check is not need. I looked at drivers/mmc/core/sd_ops.c and cmd.arg seems to be cpu endian. The new function assumes that the cpu is little endian and manually converts it to little endian. This is an endian bug. I think my fix is correct or something similar. regards, dan carpenter -- 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
Let's take an example, cmd.arg = ((ocr & 0xFF8000) != 0) << 8 | test_pattern; using "TP" name test_pattern for simplication , when we call: rtsx_pci_write_be32(pcr, SD_CMD1, cmd->arg); we should make sure TP write to SD_CMD4. If on "be" platform, then cpu_to_be32() do nothing, and TP is write to SD_CMD1, in this case, it is wrong. BR, micky. On 11/27/2014 11:23 PM, Dan Carpenter wrote: > On Thu, Nov 27, 2014 at 10:53:58AM +0800, micky_ching@realsil.com.cn wrote: >> +static inline void rtsx_pci_write_be32(struct rtsx_pcr *pcr, u16 reg, u32 val) >> +{ >> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, val >> 24); >> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, val >> 16); >> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, val >> 8); >> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, val); > This assumes the cpu is little endian. First convert to big endian > using cpu_to_be32() and then write it out. > > __be32 be_val = cpu_to_be32() > > rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, be_val); > rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, be_val >> 8); > rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, be_val >> 16); > rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, be_val >> 24); > > (Written hurredly in my mail client. May be wrong). > >> +} >> + >> +static inline void rtsx_pci_write_le32(struct rtsx_pcr *pcr, u16 reg, u32 val) >> +{ >> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, val); >> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, val >> 8); >> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, val >> 16); >> + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, val >> 24); >> +} > We don't have a user for rtsx_pci_write_le32() so don't add it. > > regards, > dan carpenter
Oh. You're right. I'm sorry for the noise. regards, dan carpenter -- 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 Thu, 27 Nov 2014, micky_ching@realsil.com.cn wrote: > From: Micky Ching <micky_ching@realsil.com.cn> > > Add helper function to write u32 to registers, if we want to put u32 > value to 4 continuous register, this can help us reduce tedious work. > > Signed-off-by: Micky Ching <micky_ching@realsil.com.cn> > --- > include/linux/mfd/rtsx_pci.h | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) Okay, I'm happy that Dan is now satisfied. Can I take this patch, or does patch 2 depend on it? > diff --git a/include/linux/mfd/rtsx_pci.h b/include/linux/mfd/rtsx_pci.h > index 74346d5..bf45ea2 100644 > --- a/include/linux/mfd/rtsx_pci.h > +++ b/include/linux/mfd/rtsx_pci.h > @@ -967,4 +967,19 @@ static inline u8 *rtsx_pci_get_cmd_data(struct rtsx_pcr *pcr) > return (u8 *)(pcr->host_cmds_ptr); > } > > +static inline void rtsx_pci_write_be32(struct rtsx_pcr *pcr, u16 reg, u32 val) > +{ > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, val >> 24); > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, val >> 16); > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, val >> 8); > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, val); > +} > + > +static inline void rtsx_pci_write_le32(struct rtsx_pcr *pcr, u16 reg, u32 val) > +{ > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, val); > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, val >> 8); > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, val >> 16); > + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, val >> 24); > +} > #endif
diff --git a/include/linux/mfd/rtsx_pci.h b/include/linux/mfd/rtsx_pci.h index 74346d5..bf45ea2 100644 --- a/include/linux/mfd/rtsx_pci.h +++ b/include/linux/mfd/rtsx_pci.h @@ -967,4 +967,19 @@ static inline u8 *rtsx_pci_get_cmd_data(struct rtsx_pcr *pcr) return (u8 *)(pcr->host_cmds_ptr); } +static inline void rtsx_pci_write_be32(struct rtsx_pcr *pcr, u16 reg, u32 val) +{ + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, val >> 24); + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, val >> 16); + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, val >> 8); + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, val); +} + +static inline void rtsx_pci_write_le32(struct rtsx_pcr *pcr, u16 reg, u32 val) +{ + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg, 0xFF, val); + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 1, 0xFF, val >> 8); + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 2, 0xFF, val >> 16); + rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, reg + 3, 0xFF, val >> 24); +} #endif