Dear Jaehoon,
On Thu, 2013-12-26 at 14:05 +0900, Jaehoon Chung wrote:
> Hi, Chin.
>
> On 12/19/2013 02:16 AM, Chin Liang See wrote:
> >
> > +
> > +#define CLKMGR_PERPLLGRP_EN_REG (SOCFPGA_CLKMGR_ADDRESS + 0xA0)
> > +#define CLKMGR_SDMMC_CLK_ENABLE (1 << 8)
> > +#define SYSMGR_SDMMCGRP_CTRL_REG (SOCFPGA_SYSMGR_ADDRESS + 0x108)
> Where is SOCFPGA_CLKMGR_ADDRESS defined?
This is located at platform specific declaration file.
>
> > +#define SYSMGR_SDMMC_CTRL_GET_DRVSEL(x) (((x) >> 0) & 0x7)
> ((x) & 0x7) is more readable?
> > +#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
> > + ((((drvsel) << 0) & 0x7) | (((smplsel) << 3) & 0x38))
> > +
> > +static char *SOCFPGA_NAME = "SOCFPGA DWMMC";
> > +
> > +static void socfpga_dwmci_clksel(struct dwmci_host *host)
> > +{
> > + unsigned int en;
> > + unsigned int drvsel;
> > + unsigned int smplsel;
> > +
> > + /* Disable SDMMC clock. */
> > + en = readl(CLKMGR_PERPLLGRP_EN_REG);
> > + en &= ~CLKMGR_SDMMC_CLK_ENABLE;
> > + writel(en, CLKMGR_PERPLLGRP_EN_REG);
> > +
> > + /* Configures drv_sel and smpl_sel */
> > + drvsel = 3;
> > + smplsel = 0;
> Is this value static? then why is value assigned drvsel and smpsel at here?
> I didn't know that SOCFPGA is only used with drvsel = 3, smplsel = 0.
> But if you need to change this value for other SoC version in future, I think
> that hard coding is not good.
Good suggestion as I put them as macro now for v3
>
> > +
> > + debug("%s: drvsel %d smplsel %d\n", __FUNCTION__, drvsel, smplsel);
> > + writel(SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel),
> > + SYSMGR_SDMMCGRP_CTRL_REG);
> > +
> > + debug("%s: SYSMGR_SDMMCGRP_CTRL_REG = 0x%x\n", __FUNCTION__,
> > + readl(SYSMGR_SDMMCGRP_CTRL_REG));
> > + /* Enable SDMMC clock */
> > + en = readl(CLKMGR_PERPLLGRP_EN_REG);
> > + en |= CLKMGR_SDMMC_CLK_ENABLE;
> > + writel(en, CLKMGR_PERPLLGRP_EN_REG);
> > +}
> > +
> > +int socfpga_dwmmc_init(u32 regbase, int bus_width, int index)
> > +{
> > + struct dwmci_host *host = NULL;
> > + host = calloc(sizeof(struct dwmci_host), 1);
> > + if (!host) {
> > + printf("dwmci_host calloc fail!\n");
> > + return 1;
> > + }
> > +
> > + host->name = SOCFPGA_NAME;
> > + host->ioaddr = (void *)regbase;
> > + host->buswidth = bus_width;
> > + host->clksel = socfpga_dwmci_clksel;
> > + host->dev_index = index;
> > + /* fixed clock divide by 4 which due to the SDMMC wrapper */
> > + host->bus_hz = CONFIG_DWMMC_BUS_HZ;
> I didn't want to use the CONFIG_DWMMC_BUS_HZ.
Yup, this is SOCFPGA specific and I am using CONFIG_SOCFPGA_DWMMC_BUS_HZ
for v3
Thanks
Chin Liang
>
> > + host->fifoth_val = MSIZE(0x2) |
> > + RX_WMARK(CONFIG_DWMMC_FIFO_DEPTH / 2 - 1) |
> > + TX_WMARK(CONFIG_DWMMC_FIFO_DEPTH / 2);
> > +
> > + add_dwmci(host, host->bus_hz, 400000);
> add_dwmci() has the return value.
>
> Best Regards,
> Jaehoon Chung
> > +
> > + return 0;
> > +}
> > +
> >
>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
