| Message ID | 1315569946-21386-1-git-send-email-tie-fei.zang@freescale.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
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
> -----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 > > 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. 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. > > > 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. I just update the year for tracking. Anything wrong? Roy
> -----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 > > 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. 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
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
> -----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 Gala > Subject: Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit > > 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? Sounds fair. > > > You may miss the patch. Please confirm. > > I don't understand, what needs to be confirmed? Confirm: there is no comment about this patch. 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. > 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
> 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
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!
> -----Original Message----- > From: Anton Vorontsov [mailto:cbouatmailru@gmail.com] > Sent: Friday, September 09, 2011 22:07 PM > To: Zang Roy-R61911 > Cc: linux-mmc@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; akpm@linux- > foundation.org; Xu Lei-B33228; Kumar Gala > Subject: Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit > > 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> Thanks. Add Chris. > > [...] > > +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! The suggestion makes sense. I'd like to do this in a separate patch after this patch applied. Thanks. Roy
> -----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 Gala > Subject: Re: [PATCH] eSDHC: Access Freescale eSDHC registers by 32-bit > > > > 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. Good. > > > 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. I tested on 8536 and P4080/P5020/P3041 platform. Anyone can help to pull it in? Thanks. Roy
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.
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,