Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit

Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1520 bytes --]

Hi,

On Fri, Sep 09, 2011 at 08:05:46PM +0800, Roy Zang wrote:
> From: Xu lei <B33228@freescale.com>
> 
> Freescale eSDHC registers only support 32-bit accesses,
> this patch ensures that all Freescale eSDHC register accesses
> are 32-bit.

So, what about the byte-swapping that Anton needed? You are simply
removing that if I am not mistaken.

> Signed-off-by: Xu lei <B33228@freescale.com>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---
> This is a patch resend 
> http://patchwork.ozlabs.org/patch/106245/
> based on latest Linus tree.
> 
> Andrew,
> Could you help to  pick up this patch first?

mmc-list is not dead, it must go via Chris, I'd think.

>  drivers/mmc/host/sdhci-of-esdhc.c |   18 ++++++++++++++----
>  1 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
> index fe604df..40036f6 100644
> --- a/drivers/mmc/host/sdhci-of-esdhc.c
> +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> @@ -1,7 +1,7 @@
>  /*
>   * Freescale eSDHC controller driver.
>   *
> - * Copyright (c) 2007 Freescale Semiconductor, Inc.
> + * Copyright (c) 2007, 2010 Freescale Semiconductor, Inc.

I wonder if such a small change has impact on the copyright.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit

[Roy Zang]

From: Xu lei <B33228@freescale.com>

Freescale eSDHC registers only support 32-bit accesses,
this patch ensures that all Freescale eSDHC register accesses
are 32-bit.

Signed-off-by: Xu lei <B33228@freescale.com>
Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
This is a patch resend 
http://patchwork.ozlabs.org/patch/106245/
based on latest Linus tree.

Andrew,
Could you help to  pick up this patch first?
Thanks.
Roy

 drivers/mmc/host/sdhci-of-esdhc.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index fe604df..40036f6 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -1,7 +1,7 @@
 /*
  * Freescale eSDHC controller driver.
  *
- * Copyright (c) 2007 Freescale Semiconductor, Inc.
+ * Copyright (c) 2007, 2010 Freescale Semiconductor, Inc.
  * Copyright (c) 2009 MontaVista Software, Inc.
  *
  * Authors: Xiaobo Xie <X.Xie@freescale.com>
@@ -22,11 +22,21 @@
 static u16 esdhc_readw(struct sdhci_host *host, int reg)
 {
 	u16 ret;
+	int base = reg & ~0x3;
+	int shift = (reg & 0x2) * 8;
 
 	if (unlikely(reg == SDHCI_HOST_VERSION))
-		ret = in_be16(host->ioaddr + reg);
+		ret = in_be32(host->ioaddr + base) & 0xffff;
 	else
-		ret = sdhci_be32bs_readw(host, reg);
+		ret = (in_be32(host->ioaddr + base) >> shift) & 0xffff;
+	return ret;
+}
+
+static u8 esdhc_readb(struct sdhci_host *host, int reg)
+{
+	int base = reg & ~0x3;
+	int shift = (reg & 0x3) * 8;
+	u8 ret = (in_be32(host->ioaddr + base) >> shift) & 0xff;
 	return ret;
 }
 
@@ -74,7 +84,7 @@ static unsigned int esdhc_of_get_min_clock(struct sdhci_host *host)
 static struct sdhci_ops sdhci_esdhc_ops = {
 	.read_l = sdhci_be32bs_readl,
 	.read_w = esdhc_readw,
-	.read_b = sdhci_be32bs_readb,
+	.read_b = esdhc_readb,
 	.write_l = sdhci_be32bs_writel,
 	.write_w = esdhc_writew,
 	.write_b = esdhc_writeb,
-- 
1.6.0.6

RE: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit

[Zang Roy-R61911]

> -----Original Message-----
> From: Wolfram Sang [mailto:w.sang@pengutronix.de]
> Sent: Friday, September 09, 2011 19:40 PM
> To: Zang Roy-R61911
> Cc: linux-mmc@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; akpm@linux-
> foundation.org; cbouatmailru@gmail.com; Xu Lei-B33228; Kumar Gala
> Subject: Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit
>=20
> Hi,
>=20
> On Fri, Sep 09, 2011 at 08:05:46PM +0800, Roy Zang wrote:
> > From: Xu lei <B33228@freescale.com>
> >
> > Freescale eSDHC registers only support 32-bit accesses,
> > this patch ensures that all Freescale eSDHC register accesses
> > are 32-bit.
>=20
> So, what about the byte-swapping that Anton needed? You are simply
> removing that if I am not mistaken.
>=20
> > Signed-off-by: Xu lei <B33228@freescale.com>
> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> > Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> > ---
> > This is a patch resend
> > http://patchwork.ozlabs.org/patch/106245/
> > based on latest Linus tree.
> >
> > Andrew,
> > Could you help to  pick up this patch first?
>=20
> mmc-list is not dead, it must go via Chris, I'd think.
This is the third time that I re-send this patch to the list.  There is no =
comment.
That is why I ask Andrew's help.
>=20
> >  drivers/mmc/host/sdhci-of-esdhc.c |   18 ++++++++++++++----
> >  1 files changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci=
-of-
> esdhc.c
> > index fe604df..40036f6 100644
> > --- a/drivers/mmc/host/sdhci-of-esdhc.c
> > +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> > @@ -1,7 +1,7 @@
> >  /*
> >   * Freescale eSDHC controller driver.
> >   *
> > - * Copyright (c) 2007 Freescale Semiconductor, Inc.
> > + * Copyright (c) 2007, 2010 Freescale Semiconductor, Inc.
>=20
> I wonder if such a small change has impact on the copyright.
I just update the year for tracking. Anything wrong?
Roy

RE: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit

[Zang Roy-R61911]

> -----Original Message-----
> From: Wolfram Sang [mailto:w.sang@pengutronix.de]
> Sent: Friday, September 09, 2011 19:40 PM
> To: Zang Roy-R61911
> Cc: linux-mmc@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; akpm@linux-
> foundation.org; cbouatmailru@gmail.com; Xu Lei-B33228; Kumar Gala
> Subject: Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit
>=20
> Hi,
>=20
> On Fri, Sep 09, 2011 at 08:05:46PM +0800, Roy Zang wrote:
> > From: Xu lei <B33228@freescale.com>
> >
> > Freescale eSDHC registers only support 32-bit accesses,
> > this patch ensures that all Freescale eSDHC register accesses
> > are 32-bit.
>=20
> So, what about the byte-swapping that Anton needed? You are simply
> removing that if I am not mistaken.
I do not see any comment for this patch from Anton. You may miss the patch.=
 Please confirm.
I add Chris in the loop.
Thanks.
Roy

Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 802 bytes --]

On Fri, Sep 09, 2011 at 12:23:05PM +0000, Zang Roy-R61911 wrote:

> > > Freescale eSDHC registers only support 32-bit accesses,
> > > this patch ensures that all Freescale eSDHC register accesses
> > > are 32-bit.
> > 
> > So, what about the byte-swapping that Anton needed? You are simply
> > removing that if I am not mistaken.
> I do not see any comment for this patch from Anton.

Maybe he is busy?

> You may miss the patch. Please confirm.

I don't understand, what needs to be confirmed? I am simply wondering
what happens to the byteswapping that Anton (back then) intentionally
implemented.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

RE: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit

[Zang Roy-R61911]

> -----Original Message-----
> From: Wolfram Sang [mailto:w.sang@pengutronix.de]
> Sent: Friday, September 09, 2011 20:33 PM
> To: Zang Roy-R61911
> Cc: cjb@laptop.org; linux-mmc@vger.kernel.org; linuxppc-dev@lists.ozlabs.=
org;
> akpm@linux-foundation.org; cbouatmailru@gmail.com; Xu Lei-B33228; Kumar G=
ala
> Subject: Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit
>=20
> On Fri, Sep 09, 2011 at 12:23:05PM +0000, Zang Roy-R61911 wrote:
>=20
> > > > Freescale eSDHC registers only support 32-bit accesses,
> > > > this patch ensures that all Freescale eSDHC register accesses
> > > > are 32-bit.
> > >
> > > So, what about the byte-swapping that Anton needed? You are simply
> > > removing that if I am not mistaken.
> > I do not see any comment for this patch from Anton.
>=20
> Maybe he is busy?
Sounds fair.
>=20
> > You may miss the patch. Please confirm.
>=20
> I don't understand, what needs to be confirmed?
Confirm: there is no comment about this patch.=20
Previously, I sent this patch together  with another one. But technically t=
hey are separated patches.  I got some comments from Anton about the patch.=
 I will try to address the comment. but for this one, it is a stand along p=
atch. I'd like to push it first.

> I am simply wondering
> what happens to the byteswapping that Anton (back then) intentionally
> implemented.
I do not see any comment about byte swapping about this patch.
you may have mis-understanding here.
Roy

Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 870 bytes --]


> Previously, I sent this patch together  with another one. But
> technically they are separated patches.  I got some comments from
> Anton about the patch. I will try to address the comment. but for this
> one, it is a stand along patch. I'd like to push it first.

Okay.

> I do not see any comment about byte swapping about this patch.
> you may have mis-understanding here.

I was misguided, yes, sorry. BE/LE issues with additional byteswapping
are a mind twister :/ Having a second look, it looks okay to me. One
could think about putting this into sdhci_platform as well, but this can
be done later if needed. Sadly, I currently don't have hardware to test
it.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit

[Anton Vorontsov]

On Fri, Sep 09, 2011 at 08:05:46PM +0800, Roy Zang wrote:
> From: Xu lei <B33228@freescale.com>
> 
> Freescale eSDHC registers only support 32-bit accesses,
> this patch ensures that all Freescale eSDHC register accesses
> are 32-bit.
> 
> Signed-off-by: Xu lei <B33228@freescale.com>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---

The patch looks OK.

Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>

[...]
> +static u8 esdhc_readb(struct sdhci_host *host, int reg)
> +{
> +	int base = reg & ~0x3;
> +	int shift = (reg & 0x3) * 8;
> +	u8 ret = (in_be32(host->ioaddr + base) >> shift) & 0xff;
>  	return ret;
>  }

Though, I wonder if we could change sdhci_be32bs_read{b,w}, instead
of making this local to eSDHC.

The thing is: sdhci_be32bs_writeb() is using clrsetbits_be32,
so the write variant already uses 32-bit accessors, so nothing should
break if we switch sdhci_be32bs_readb() to in_be32().

But maybe it's safer if we do this in a separate patch, so that it
could be easily reverted without impacting eSDHC if something actually
breaks.

You decide. :-)

Thanks!

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

RE: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit

[Zang Roy-R61911]

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQW50b24gVm9yb250c292
IFttYWlsdG86Y2JvdWF0bWFpbHJ1QGdtYWlsLmNvbV0NCj4gU2VudDogRnJpZGF5LCBTZXB0ZW1i
ZXIgMDksIDIwMTEgMjI6MDcgUE0NCj4gVG86IFphbmcgUm95LVI2MTkxMQ0KPiBDYzogbGludXgt
bW1jQHZnZXIua2VybmVsLm9yZzsgbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmc7IGFrcG1A
bGludXgtDQo+IGZvdW5kYXRpb24ub3JnOyBYdSBMZWktQjMzMjI4OyBLdW1hciBHYWxhDQo+IFN1
YmplY3Q6IFJlOiBbUEFUQ0hdIGVTREhDOiBBY2Nlc3MgRnJlZXNjYWxlIGVTREhDIHJlZ2lzdGVy
cyBieSAzMi1iaXQNCj4gDQo+IE9uIEZyaSwgU2VwIDA5LCAyMDExIGF0IDA4OjA1OjQ2UE0gKzA4
MDAsIFJveSBaYW5nIHdyb3RlOg0KPiA+IEZyb206IFh1IGxlaSA8QjMzMjI4QGZyZWVzY2FsZS5j
b20+DQo+ID4NCj4gPiBGcmVlc2NhbGUgZVNESEMgcmVnaXN0ZXJzIG9ubHkgc3VwcG9ydCAzMi1i
aXQgYWNjZXNzZXMsDQo+ID4gdGhpcyBwYXRjaCBlbnN1cmVzIHRoYXQgYWxsIEZyZWVzY2FsZSBl
U0RIQyByZWdpc3RlciBhY2Nlc3Nlcw0KPiA+IGFyZSAzMi1iaXQuDQo+ID4NCj4gPiBTaWduZWQt
b2ZmLWJ5OiBYdSBsZWkgPEIzMzIyOEBmcmVlc2NhbGUuY29tPg0KPiA+IFNpZ25lZC1vZmYtYnk6
IFJveSBaYW5nIDx0aWUtZmVpLnphbmdAZnJlZXNjYWxlLmNvbT4NCj4gPiBTaWduZWQtb2ZmLWJ5
OiBLdW1hciBHYWxhIDxnYWxha0BrZXJuZWwuY3Jhc2hpbmcub3JnPg0KPiA+IC0tLQ0KPiANCj4g
VGhlIHBhdGNoIGxvb2tzIE9LLg0KPiANCj4gQWNrZWQtYnk6IEFudG9uIFZvcm9udHNvdiA8Y2Jv
dWF0bWFpbHJ1QGdtYWlsLmNvbT4NClRoYW5rcy4NCkFkZCBDaHJpcy4NCg0KPiANCj4gWy4uLl0N
Cj4gPiArc3RhdGljIHU4IGVzZGhjX3JlYWRiKHN0cnVjdCBzZGhjaV9ob3N0ICpob3N0LCBpbnQg
cmVnKQ0KPiA+ICt7DQo+ID4gKwlpbnQgYmFzZSA9IHJlZyAmIH4weDM7DQo+ID4gKwlpbnQgc2hp
ZnQgPSAocmVnICYgMHgzKSAqIDg7DQo+ID4gKwl1OCByZXQgPSAoaW5fYmUzMihob3N0LT5pb2Fk
ZHIgKyBiYXNlKSA+PiBzaGlmdCkgJiAweGZmOw0KPiA+ICAJcmV0dXJuIHJldDsNCj4gPiAgfQ0K
PiANCj4gVGhvdWdoLCBJIHdvbmRlciBpZiB3ZSBjb3VsZCBjaGFuZ2Ugc2RoY2lfYmUzMmJzX3Jl
YWR7Yix3fSwgaW5zdGVhZA0KPiBvZiBtYWtpbmcgdGhpcyBsb2NhbCB0byBlU0RIQy4NCj4gDQo+
IFRoZSB0aGluZyBpczogc2RoY2lfYmUzMmJzX3dyaXRlYigpIGlzIHVzaW5nIGNscnNldGJpdHNf
YmUzMiwNCj4gc28gdGhlIHdyaXRlIHZhcmlhbnQgYWxyZWFkeSB1c2VzIDMyLWJpdCBhY2Nlc3Nv
cnMsIHNvIG5vdGhpbmcgc2hvdWxkDQo+IGJyZWFrIGlmIHdlIHN3aXRjaCBzZGhjaV9iZTMyYnNf
cmVhZGIoKSB0byBpbl9iZTMyKCkuDQo+IA0KPiBCdXQgbWF5YmUgaXQncyBzYWZlciBpZiB3ZSBk
byB0aGlzIGluIGEgc2VwYXJhdGUgcGF0Y2gsIHNvIHRoYXQgaXQNCj4gY291bGQgYmUgZWFzaWx5
IHJldmVydGVkIHdpdGhvdXQgaW1wYWN0aW5nIGVTREhDIGlmIHNvbWV0aGluZyBhY3R1YWxseQ0K
PiBicmVha3MuDQo+IA0KPiBZb3UgZGVjaWRlLiA6LSkNCj4gDQo+IFRoYW5rcyENClRoZSBzdWdn
ZXN0aW9uIG1ha2VzIHNlbnNlLiBJJ2QgbGlrZSB0byBkbyB0aGlzIGluIGEgc2VwYXJhdGUgcGF0
Y2ggYWZ0ZXIgdGhpcyBwYXRjaCBhcHBsaWVkLg0KVGhhbmtzLg0KUm95DQo=

RE: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit

[Zang Roy-R61911]

> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-owner@vger.kernel=
.org]
> On Behalf Of Wolfram Sang
> Sent: Friday, September 09, 2011 21:09 PM
> To: Zang Roy-R61911
> Cc: cjb@laptop.org; linux-mmc@vger.kernel.org; linuxppc-dev@lists.ozlabs.=
org;
> akpm@linux-foundation.org; cbouatmailru@gmail.com; Xu Lei-B33228; Kumar G=
ala
> Subject: Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit
>=20
>=20
> > Previously, I sent this patch together  with another one. But
> > technically they are separated patches.  I got some comments from
> > Anton about the patch. I will try to address the comment. but for this
> > one, it is a stand along patch. I'd like to push it first.
>=20
> Okay.
Good.
>=20
> > I do not see any comment about byte swapping about this patch.
> > you may have mis-understanding here.
>=20
> I was misguided, yes, sorry. BE/LE issues with additional byteswapping
> are a mind twister :/ Having a second look, it looks okay to me. One
> could think about putting this into sdhci_platform as well, but this can
> be done later if needed. Sadly, I currently don't have hardware to test
> it.
I tested on 8536 and P4080/P5020/P3041 platform.
Anyone can help to pull it in?
Thanks.
Roy

Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit

[Chris Ball]

Hi Roy,

On Fri, Sep 09 2011, Roy Zang wrote:
> From: Xu lei <B33228@freescale.com>
>
> Freescale eSDHC registers only support 32-bit accesses,
> this patch ensures that all Freescale eSDHC register accesses
> are 32-bit.
>
> Signed-off-by: Xu lei <B33228@freescale.com>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>

Pushed to mmc-next for 3.2 with Anton's ACK now, thanks.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child