Re: [U-Boot] [PATCH v5 4/8] MMC: sdhci: Add support for dove sdhci

Hi,

I have a few comments.

I myself not convenient with the capital letters on commit header, IMHO "MMC: dove_sdhci: " ==> "mmc: dove_sdhci: "

perhaps not much an issue we are taking about.

On Wed, Jun 26, 2013 at 2:57 AM, Sascha Silbe t-uboot@infra-silbe.de wrote:

This adds a driver for the sdhci controller found on Dove SoCs.

Signed-off-by: Sascha Silbe t-uboot@infra-silbe.de

v4->v5: no changes

arch/arm/include/asm/arch-dove/mmc.h | 27 ++++++++++ drivers/mmc/Makefile | 1 + drivers/mmc/dove_sdhci.c | 101 +++++++++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+)

diff --git a/arch/arm/include/asm/arch-dove/mmc.h b/arch/arm/include/asm/arch-dove/mmc.h new file mode 100644 index 0000000..579396c --- /dev/null +++ b/arch/arm/include/asm/arch-dove/mmc.h @@ -0,0 +1,27 @@ +/*

• • Marvell Dove SoC SDHCI
• • See file CREDITS for list of people who contributed to this
• • project.
• • This program is free software; you can redistribute it and/or
• • modify it under the terms of the GNU General Public License as
• • published by the Free Software Foundation; either version 2 of
• • the License, or (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • You should have received a copy of the GNU General Public License
• • along with this program; if not, write to the Free Software
• • Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
• • MA 02110-1301 USA
• */

+#ifndef _DOVEMMC_H +#define _DOVEMMC_H

+int dove_sdhci_init(int num); +#endif /* _DOVEMMC_H */

Do you really require this file just for a single func deceleration. I have a another thought like..just directly call dove_sdhci_init from board file.

diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile index 24648a2..074af75 100644 --- a/drivers/mmc/Makefile +++ b/drivers/mmc/Makefile @@ -28,6 +28,7 @@ LIB := $(obj)libmmc.o

COBJS-$(CONFIG_BFIN_SDH) += bfin_sdh.o COBJS-$(CONFIG_DAVINCI_MMC) += davinci_mmc.o +COBJS-$(CONFIG_DOVE_SDHCI) += dove_sdhci.o COBJS-$(CONFIG_FSL_ESDHC) += fsl_esdhc.o COBJS-$(CONFIG_FTSDC010) += ftsdc010_mci.o COBJS-$(CONFIG_GENERIC_MMC) += mmc.o diff --git a/drivers/mmc/dove_sdhci.c b/drivers/mmc/dove_sdhci.c new file mode 100644 index 0000000..ac15fd7 --- /dev/null +++ b/drivers/mmc/dove_sdhci.c @@ -0,0 +1,101 @@ +/*

• • Marvell Dove SDHCI driver
• • Sebastian Hesselbarth sebastian.hesselbarth@gmail.com
• • Based on linux drivers/mmc/host/sdhci-dove.c
• • by: Saeed Bishara saeed@marvell.com

• • Mike Rapoport <mike@compulab.co.il>
    

• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • You should have received a copy of the GNU General Public License
• • along with this program; if not, write to the Free Software
• • Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
• */

+#include <common.h> +#include <malloc.h> +#include <sdhci.h> +#include <asm/arch/dove.h>

+static u16 dove_sdhci_readw(struct sdhci_host *host, int reg) +{

•   u16 ret;
  

•   switch (reg) {
  

•   case SDHCI_HOST_VERSION:
  

•   case SDHCI_SLOT_INT_STATUS:
  

•           /* those registers don't exist */
  

•           return 0;
  

•   default:
  

•           ret = readw(host->ioaddr + reg);
  

•   }
  

•   return ret;
  

+}

+static u32 dove_sdhci_readl(struct sdhci_host *host, int reg) +{

•   u32 ret;
  

•   switch (reg) {
  

•   case SDHCI_CAPABILITIES:
  

•           ret = readl(host->ioaddr + reg);
  

•           /* Mask the support for 3.0V */
  

•           ret &= ~SDHCI_CAN_VDD_300;
  

•           break;
  

•   default:
  

•           ret = readl(host->ioaddr + reg);
  

•   }
  

•   return ret;
  

+}

+static struct sdhci_ops dove_sdhci_ops = {

•   .read_w = dove_sdhci_readw,
  

•   .read_l = dove_sdhci_readl,
  

+};

+static struct sdhci_host hosts[2] = {

•   {
  

•           .name = "Dove SDHCI0",
  

•           .ioaddr = (void *)DOVE_SDIO0_BASE,
  

•   },
  

•   {
  

•           .name = "Dove SDHCI1",
  

•           .ioaddr = (void *)DOVE_SDIO1_BASE,
  

•   },
  

+};

+int dove_sdhci_init(int num) +{

•   struct sdhci_host *host;
  

•   if (num < 0 || num > 1)
  

•           return 1;
  

What exactly this logic is for, are you able to detect the sdhci host at runtime out of 2. if not so may be you directly call dove_sdhci_init from board file with respective base.

•   host = &hosts[num];
  

•   if (host->version)
  

•           return 1;
  

•   host->quirks =
  

•           SDHCI_QUIRK_NO_HISPD_BIT |
  

•           SDHCI_QUIRK_BROKEN_R1B |
  

•           SDHCI_QUIRK_32BIT_DMA_ADDR;
  

•   host->version = SDHCI_SPEC_200;
  

Better to read the version using sdhci_readw() instead of direct assignment.

-- Thanks, Jagan.

•   host->ops = &dove_sdhci_ops;
  

•   add_sdhci(host, 50000000, 4000000);
  

•   return 0;
  

+}

1.8.2.1