[U-Boot] [PATCH] mmc: atmel: Add support fo atmel sdhci

[Yang, Wenyou]

Hi Shen Bo,

Thanks a lot for your advice.
No problem, They will be added in next version.

Thanks

Best Regards,
Wenyou Yang

> -----Original Message-----
> From: Bo Shen [mailto:voice.shen at gmail.com]
> Sent: 2015年9月17日 7:02
> To: Yang, Wenyou; U-Boot Mailing List
> Cc: Pantelis Antoniou
> Subject: Re: [U-Boot] [PATCH] mmc: atmel: Add support fo atmel sdhci
> 
> Hi Wenyou,
> 
> On 09/16/2015 04:23 PM, Wenyou Yang wrote:
> 
> I think you should add commit message here.
> 
> > Signed-off-by: Wenyou Yang <wenyou.yang at atmel.com>
> > ---
> >
> >   arch/arm/mach-at91/include/mach/atmel_sdhci.h |   13 +++++++++
> >   drivers/mmc/Makefile                          |    1 +
> >   drivers/mmc/atmel_sdhci.c                     |   35
> +++++++++++++++++++++++++
> >   3 files changed, 49 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..cf3bf89
> > --- /dev/null
> > +++ b/arch/arm/mach-at91/include/mach/atmel_sdhci.h
> > @@ -0,0 +1,13 @@
> > +/*
> > + * Copyright (c) 2015 Atmel Corporation
> > + *		      Wenyou.Yang <wenyou.yang at atmel.com>
> > + *
> > + * 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..64776a1
> > --- /dev/null
> > +++ b/drivers/mmc/atmel_sdhci.c
> > @@ -0,0 +1,35 @@
> > +/*
> > + * Copyright (C) 2015 Atmel Corporation
> > + *		      Wenyou.Yang <wenyou.yang at atmel.com>
> > + *
> > + * SPDX-License-Identifier:	GPL-2.0+
> > + */
> > +
> > +#include <common.h>
> > +#include <malloc.h>
> > +#include <sdhci.h>
> > +#include <asm/arch/clk.h>
> > +
> > +#define ATMEL_SDHC_MIN_FRQ	400000
> 
> Nit: maybe FREQ is better than FRQ?
> 
> > +
> > +int atmel_sdhci_init(void *regbase, u32 id) {
> > +	struct sdhci_host *host = NULL;
> > +	u32 max_clk, min_clk = ATMEL_SDHC_MIN_FRQ;
> > +
> > +	host = (struct sdhci_host *)malloc(sizeof(struct sdhci_host));
> 
> How about "host = malloc(sizeof(*host));"?
> 
> > +	if (!host) {
> > +		printf("atmel_sdhci_init: sdhci_host malloc fail\n");
> > +		return -1;
> 
> Maybe use -ENOMEM replace -1?
> 
> > +	}
> > +
> > +	host->name = "atmel_sdhci";
> > +	host->ioaddr = (void *)regbase;
> > +	host->quirks = 0;
> > +	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
> > +	max_clk = at91_get_periph_generated_clk(id);
> 
> As we discussed with your patch for "at91_get_periph_generated_clk", this
> function may failed. So, I think you need add error check here.
> 
> > +	add_sdhci(host, max_clk, min_clk);
> > +
> > +	return 0;
> > +}
> >
> 
> Best Regards,
> Bo Shen