RE: [PATCH V6 3/4] mmc: sdhci-esdhc: make the writel/readl as the general APIs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



HI Wolfram:
Thanks for your comments.

Best Regards
Richard Zhu


> -----Original Message-----
> From: Wolfram Sang [mailto:w.sang@xxxxxxxxxxxxxx]
> Sent: Friday, March 18, 2011 4:55 AM
> To: Zhu Richard-R65037
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; linux-
> mmc@xxxxxxxxxxxxxxx; cjb@xxxxxxxxxx; avorontsov@xxxxxxxxxxxxx;
> eric@xxxxxxxxxx; linuxzsc@xxxxxxxxx; eric.miao@xxxxxxxxxx
> Subject: Re: [PATCH V6 3/4] mmc: sdhci-esdhc: make the writel/readl as
> the general APIs
>
> Hi Richard,
>
> On Thu, Mar 17, 2011 at 02:34:06PM +0800, Richard Zhu wrote:
> > Add one flag to indicate the GPIO CD/WP is enabled or not on imx
> > platforms, and reuse the writel/readl as the general APIs for imx
> > SOCs.
> >
> > Signed-off-by: Richard Zhu <Hong-Xing.Zhu@xxxxxxxxxxxxx>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c |   43
> ++++++++++++++++++++++++++++++------
> >  drivers/mmc/host/sdhci-pltfm.h     |    2 +-
> >  2 files changed, 37 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> > b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index 3b52485..5768e06 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/err.h>
> >  #include <linux/clk.h>
> >  #include <linux/gpio.h>
> > +#include <linux/slab.h>
> >  #include <linux/mmc/host.h>
> >  #include <linux/mmc/sdhci-pltfm.h>
> >  #include <mach/hardware.h>
> > @@ -24,6 +25,13 @@
> >  #include "sdhci-pltfm.h"
> >  #include "sdhci-esdhc.h"
> >
> > +#define IMX_GPIO_CD_WP                     (1 << 0)
>
> Hmm, that sounds like a macro for a gpio-number. What about
> ESDHC_FLAG_GPIO_FOR_CD_WP or something like that?
Accepted. The other quirk flag would be changed just refer to this format.
>
> > +
> > +struct pltfm_imx_data {
> > +   int flags;
> > +   u32 mod_val;
>
> Maybe 'scratchpad' was not the best name, but 'mod_val' seems worse to me.
Would use scratchpad to replace the mod_val later.
>
> Those are minor things, but there is one thing which needs respinning
> IMHO:
> While it looks functionally OK, I'll tell you (once again) that you cast
> too much. Please be careful about that in the future!
Got that.
>
> Regards,
>
>    Wolfram
>
> --
> Pengutronix e.K.                           | Wolfram Sang
> |
> Industrial Linux Solutions                 | http://www.pengutronix.de/
> |

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux