Hi Bo Shen, Thank you for your review.
> -----Original Message----- > From: Bo Shen [mailto:[email protected]] > Sent: 2015年10月28日 11:03 > To: Yang, Wenyou > Cc: U-Boot Mailing List; [email protected]; Pantelis Antoniou > Subject: Re: [PATCH v3] mmc: atmel: Add atmel sdhci support > > Hi Wenyou, > > On 10/26/2015 11:38 AM, Wenyou Yang wrote: > > The SDHCI is introduced by sama5d2, named as Secure Digital Multimedia > > Card Controller(SDMMC). It supports the embedded MultiMedia Card > > (e.MMC) Specification V4.41, the SD Memory Card Specification V3.0, > > and the SDIO > > V3.0 specification. It is compliant with the SD Host Controller > > Standard > > V3.0 specification. > > > > Signed-off-by: Wenyou Yang <[email protected]> > > --- > > > > Changes in v3: > > 1./ return -ENODEV instead of -1, when failing to get proper clock. > > 2./ add free host instance when failing to get proper clock. > > > > Changes in v2: > > - According to Bo Shen's comments, > > 1./ change the macro ATMEL_SDHC_MIN_FRQ -> > ATMEL_SDHC_MIN_FREQ. > > 2./ change the return value to -ENOMEM and print log for malloc failure. > > 3./ add the failed return and log for getting improper clock. > > 4./ add more commit log. > > > > arch/arm/mach-at91/include/mach/atmel_sdhci.h | 13 ++++++++ > > drivers/mmc/Makefile | 1 + > > drivers/mmc/atmel_sdhci.c | 40 > +++++++++++++++++++++++++ > > 3 files changed, 54 insertions(+) > > create mode 100644 arch/arm/mach-at91/include/mach/atmel_sdhci.h > > create mode 100644 drivers/mmc/atmel_sdhci.c > > > > diff --git a/arch/arm/mach-at91/include/mach/atmel_sdhci.h > > b/arch/arm/mach-at91/include/mach/atmel_sdhci.h > > new file mode 100644 > > index 0000000..9652bc2 > > --- /dev/null > > +++ b/arch/arm/mach-at91/include/mach/atmel_sdhci.h > > @@ -0,0 +1,13 @@ > > +/* > > + * Copyright (c) 2015 Atmel Corporation > > + * Wenyou.Yang <[email protected]> > > + * > > + * SPDX-License-Identifier: GPL-2.0+ > > + */ > > + > > +#ifndef __ATMEL_SDHCI_H > > +#define __ATMEL_SDHCI_H > > + > > +int atmel_sdhci_init(void *regbase, u32 id); > > + > > +#endif > > diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile index > > 99d0295..5d35705 100644 > > --- a/drivers/mmc/Makefile > > +++ b/drivers/mmc/Makefile > > @@ -8,6 +8,7 @@ > > obj-$(CONFIG_DM_MMC) += mmc-uclass.o > > > > obj-$(CONFIG_ARM_PL180_MMCI) += arm_pl180_mmci.o > > +obj-$(CONFIG_ATMEL_SDHCI) += atmel_sdhci.o > > obj-$(CONFIG_BCM2835_SDHCI) += bcm2835_sdhci.o > > obj-$(CONFIG_BFIN_SDH) += bfin_sdh.o > > obj-$(CONFIG_DAVINCI_MMC) += davinci_mmc.o diff --git > > a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c new file mode > > 100644 index 0000000..185a36e > > --- /dev/null > > +++ b/drivers/mmc/atmel_sdhci.c > > @@ -0,0 +1,40 @@ > > +/* > > + * Copyright (C) 2015 Atmel Corporation > > + * Wenyou.Yang <[email protected]> > > + * > > + * SPDX-License-Identifier: GPL-2.0+ > > + */ > > + > > +#include <common.h> > > +#include <malloc.h> > > +#include <sdhci.h> > > +#include <asm/arch/clk.h> > > + > > +#define ATMEL_SDHC_MIN_FREQ 400000 > > + > > +int atmel_sdhci_init(void *regbase, u32 id) > > I think the regbase type should be u32, if you keep it as "void *", then the > following > don't need to be covert to "void *". You are right. The following convert should be removed. > > > +{ > > + struct sdhci_host *host = NULL; > > I think it is no need to pass NULL to host. Yes, NULL is not needed. > > > + u32 max_clk, min_clk = ATMEL_SDHC_MIN_FREQ; > > + > > + host = (struct sdhci_host *)malloc(sizeof(struct sdhci_host)); > > Maybe calloc will be better. calloc() is better, zero-initialized the buffer > > > + if (!host) { > > + printf("%s: sdhci_host malloc failed\n", __func__); > > + return -ENOMEM; > > + } > > + > > + host->name = "atmel_sdhci"; > > + host->ioaddr = (void *)regbase; > > Here no need to do covert if the original type is the same. Yes, removed. > > > + host->quirks = 0; > > + host->version = sdhci_readw(host, SDHCI_HOST_VERSION); > > + max_clk = at91_get_periph_generated_clk(id); > > + if (!max_clk) { > > + printf("%s: Failed to get the proper clock\n", __func__); > > + free(host); > > + return -ENODEV; > > + } > > + > > + add_sdhci(host, max_clk, min_clk); > > + > > + return 0; > > +} > > > > Best Regards, > Bo Shen Best Regards, Wenyou Yang _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
