| Message ID | 1342097668-25243-7-git-send-email-thomas.abraham@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, This version does not seems to consider previous reviews fully. Could you check the comments below? July 12, 2012, Thomas Abraham <thomas.abraham@linaro.org> wrote: > The instantiation of the Synopsis Designware controller on Exynos5250 > include extension for SDR and DDR specific tx/rx phase shift timing > and CIU internal divider. In addition to that, the option to skip the > command hold stage is also introduced. Add support for these Exynos5250 > specfic extenstions. > > Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com> > Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org> > --- > .../devicetree/bindings/mmc/synposis-dw-mshc.txt | 38 ++++++++++++++++++- > drivers/mmc/host/dw_mmc-pltfm.c | 15 +++++++ > drivers/mmc/host/dw_mmc.c | 40 +++++++++++++++++++- > drivers/mmc/host/dw_mmc.h | 14 +++++++ > include/linux/mmc/dw_mmc.h | 6 +++ > 5 files changed, 110 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt > b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt > index 3acd6c9..69d78c1 100644 > --- a/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt > +++ b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt > @@ -7,6 +7,8 @@ Required Properties: > > * compatible: should be one of the following > - snps,dw-mshc: for controllers compliant with synopsis dw-mshc. > + - samsung,exynos5250-dw-mshc: for controllers with Samsung > + Exynos5250 specific extentions. > > * reg: physical base address of the dw-mshc controller and size of its memory > region. > @@ -74,13 +76,45 @@ Aliases: > the following format 'mshc{n}' where n is a unique number for the alias. > > > +Samsung Exynos4/5 specific properties: > + > +Some of the variants of Exynos4 (such as Exynos4412) and Exynos5 SoC's > +includes few extensions to the Synopsis Designware Mobile Storage Host > +Controller. The following properties are used to describe those extensions. > + > +* samsung,dw-mshc-sdr-timing: Specifies the value of CUI clock divider, CIU > + clock phase shift value in transmit mode and CIU clock phase shift value in > + receive mode for single data rate mode operation. Refer notes of the valid > + values below. > + > +* samsung,dw-mshc-ddr-timing: Specifies the value of CUI clock divider, CIU > + clock phase shift value in transmit mode and CIU clock phase shift value in > + receive mode for double data rate mode operation. Refer notes of the valid > + values below. The order of the cells should be > + > + - First Cell: CIU clock divider value (applicable only for Exynos5 > + SoC's, should be zero for Exynos4 SoC's) > + - Second Cell: CIU clock phase shift value for tx mode. > + - Third Cell: CIU clock phase shift value for rx mode. > + > + Valid values for SDR and DDR CIU clock timing for Exynos5250: > + > + - valid values for CIU clock divider, tx phase shift and rx phase shift > + is 0 to 7. > + > + - When CIU clock divider value is set to 3, all possible 8 phase shift > + values can be used. > + > + - If CIU clock divider value is 0 (that is divide by 1), both tx and rx > + phase shift clocks should be 0. > + > Example: > > The MSHC controller node can be split into two portions, SoC specific and > board specific portions as listed below. > > dwmmc0@12200000 { > - compatible = "snps,dw-mshc"; > + compatible = "samsung,exynos5250-dw-mshc"; > reg = <0x12200000 0x1000>; > interrupts = <0 75 0>; > #address-cells = <1>; > @@ -94,6 +128,8 @@ Example: > no-write-protect; > fifo-depth = <0x80>; > card-detect-delay = <200>; > + samsung,dw-mshc-sdr-timing = <2 3 3>; > + samsung,dw-mshc-ddr-timing = <1 2 3>; > > slot@0 { > reg = <0>; > diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c > index 8d24f6d..900f412 100644 > --- a/drivers/mmc/host/dw_mmc-pltfm.c > +++ b/drivers/mmc/host/dw_mmc-pltfm.c > @@ -27,9 +27,24 @@ static struct dw_mci_drv_data synopsis_drv_data = { > .ctrl_type = DW_MCI_TYPE_SYNOPSIS, > }; > > +static unsigned long exynos5250_dwmmc_caps[4] = { > + MMC_CAP_UHS_DDR50 | MMC_CAP_1_8V_DDR | > + MMC_CAP_8_BIT_DATA | MMC_CAP_CMD23, > + MMC_CAP_CMD23, > + MMC_CAP_CMD23, > + MMC_CAP_CMD23, > +}; > + Kyungmin Park has already pointed . It's not still proper place for board specific caps. If I'm incorrect, please let me know. And why MMC_CAP_CMD23 is default caps for all channel of hosts? > +static struct dw_mci_drv_data exynos5250_drv_data = { > + .ctrl_type = DW_MCI_TYPE_EXYNOS5250, > + .caps = exynos5250_dwmmc_caps, > +}; > + > static const struct of_device_id dw_mci_pltfm_match[] = { > { .compatible = "snps,dw-mshc", > .data = (void *)&synopsis_drv_data, }, > + { .compatible = "samsung,exynos5250-dw-mshc", > + .data = (void *)&exynos5250_drv_data, }, > {}, > }; > MODULE_DEVICE_TABLE(of, dw_mci_pltfm_match); > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index 3bc276d..bbf1209 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -236,6 +236,7 @@ static void dw_mci_set_timeout(struct dw_mci *host) > static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd) > { > struct mmc_data *data; > + struct dw_mci_slot *slot = mmc_priv(mmc); > u32 cmdr; > cmd->error = -EINPROGRESS; > > @@ -265,6 +266,17 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd) > cmdr |= SDMMC_CMD_DAT_WR; > } > > + /* > + * Samsung Exynos5250 extends the use of CMD register with the use of > + * bit 29 (which is reserved on standard MSHC controllers) for > + * optionally bypassing the HOLD register for command and data. The > + * HOLD register should be bypassed in case there is no phase shift > + * applied on CMD/DATA that is sent to the card. > + */ > + if (slot->host->drv_data->ctrl_type == DW_MCI_TYPE_EXYNOS5250) > + if (SDMMC_CLKSEL_GET_SELCLK_DRV(mci_readl(slot->host, CLKSEL))) > + cmdr |= SDMMC_CMD_USE_HOLD_REG; > + > return cmdr; > } > > @@ -802,10 +814,19 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > regs = mci_readl(slot->host, UHS_REG); > > /* DDR mode set */ > - if (ios->timing == MMC_TIMING_UHS_DDR50) > + if (ios->timing == MMC_TIMING_UHS_DDR50) { > regs |= (0x1 << slot->id) << 16; > - else > + mci_writel(slot->host, CLKSEL, slot->host->ddr_timing); > + } else { > regs &= ~(0x1 << slot->id) << 16; > + mci_writel(slot->host, CLKSEL, slot->host->sdr_timing); > + } > + > + if (slot->host->drv_data->ctrl_type == DW_MCI_TYPE_EXYNOS5250) { > + slot->host->bus_hz = clk_get_rate(slot->host->ciu_clk); > + slot->host->bus_hz /= SDMMC_CLKSEL_GET_DIVRATIO( > + mci_readl(slot->host, CLKSEL)); > + } As you know, CLKSEL is specific for Samsung soc. 0x09C(CLKSEL) is reserved area in Synopsys memory map. In case of non-samsung-soc, we cannot ensure this usage. In previous version, I have suggested separating the variant into another file. > > mci_writel(slot->host, UHS_REG, regs); > > @@ -2086,6 +2107,7 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host) > struct dw_mci_board *pdata; > struct device *dev = host->dev; > struct device_node *np = dev->of_node; > + u32 timing[3]; > int idx, cnt; > > pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > @@ -2108,6 +2130,20 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host) > if (of_get_property(np, of_quriks[idx].quirk, NULL)) > pdata->quirks |= of_quriks[idx].id; > > + if (of_property_read_u32_array(dev->of_node, > + "samsung,dw-mshc-sdr-timing", timing, 3)) > + host->sdr_timing = DW_MCI_DEF_SDR_TIMING; Host of non-samsung will reach here. host->sdr_timing is needed for this host? host->ddr_timing is the same. > + else > + host->sdr_timing = SDMMC_CLKSEL_TIMING(timing[0], > + timing[1], timing[2]); > + > + if (of_property_read_u32_array(dev->of_node, > + "samsung,dw-mshc-ddr-timing", timing, 3)) > + host->ddr_timing = DW_MCI_DEF_DDR_TIMING; > + else > + host->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], > + timing[1], timing[2]); > + > if (of_property_read_u32(np, "fifo-depth", &pdata->fifo_depth)) > dev_info(dev, "fifo-depth property not found, using " > "value of FIFOTH register as default\n"); > diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h > index 1ecaa02..6c17282 100644 > --- a/drivers/mmc/host/dw_mmc.h > +++ b/drivers/mmc/host/dw_mmc.h > @@ -53,6 +53,7 @@ > #define SDMMC_IDINTEN 0x090 > #define SDMMC_DSCADDR 0x094 > #define SDMMC_BUFADDR 0x098 > +#define SDMMC_CLKSEL 0x09C /* specific to Samsung Exynos5250 */ > #define SDMMC_DATA(x) (x) > > /* > @@ -111,6 +112,7 @@ > #define SDMMC_INT_ERROR 0xbfc2 > /* Command register defines */ > #define SDMMC_CMD_START BIT(31) > +#define SDMMC_CMD_USE_HOLD_REG BIT(29) > #define SDMMC_CMD_CCS_EXP BIT(23) > #define SDMMC_CMD_CEATA_RD BIT(22) > #define SDMMC_CMD_UPD_CLK BIT(21) > @@ -142,6 +144,17 @@ > /* Version ID register define */ > #define SDMMC_GET_VERID(x) ((x) & 0xFFFF) > > +#define DW_MCI_DEF_SDR_TIMING 0x03030002 > +#define DW_MCI_DEF_DDR_TIMING 0x03020001 What is the basis for these timing? These values is board-specific. > +#define SDMMC_CLKSEL_CCLK_SAMPLE(x) (((x) & 3) << 0) > +#define SDMMC_CLKSEL_CCLK_DRIVE(x) (((x) & 3) << 16) > +#define SDMMC_CLKSEL_CCLK_DIVIDER(x) (((x) & 3) << 24) If it's for exynos5, it will be 7 not 3. > +#define SDMMC_CLKSEL_TIMING(x, y, z) (SDMMC_CLKSEL_CCLK_SAMPLE(x) | \ > + SDMMC_CLKSEL_CCLK_DRIVE(y) | \ > + SDMMC_CLKSEL_CCLK_DIVIDER(z)) > +#define SDMMC_CLKSEL_GET_DIVRATIO(x) ((((x) >> 24) & 0x7) + 1) > +#define SDMMC_CLKSEL_GET_SELCLK_DRV(x) (((x) >> 16) & 0x7) > + Is this patch considered only for exynos5250? In case of exynos4210, the number of bits is different. If upper macros is backward-compatible, it would be better. Best regards, Seungwon Jeon > /* Register access macros */ > #define mci_readl(dev, reg) \ > __raw_readl((dev)->regs + SDMMC_##reg) > @@ -184,6 +197,7 @@ extern int dw_mci_resume(struct dw_mci *host); > > /* Variations in the dw_mci controller */ > #define DW_MCI_TYPE_SYNOPSIS 0 > +#define DW_MCI_TYPE_EXYNOS5250 1 /* Samsung Exynos5250 Extensions */ > > /* dw_mci platform driver data */ > struct dw_mci_drv_data { > diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h > index ae45e4f..32c778f 100644 > --- a/include/linux/mmc/dw_mmc.h > +++ b/include/linux/mmc/dw_mmc.h > @@ -82,6 +82,8 @@ struct mmc_data; > * @biu_clk: Pointer to bus interface unit clock instance. > * @ciu_clk: Pointer to card interface unit clock instance. > * @slot: Slots sharing this MMC controller. > + * @sdr_timing: Clock phase shifting for driving and sampling in sdr mode > + * @ddr_timing: Clock phase shifting for driving and sampling in ddr mode > * @fifo_depth: depth of FIFO. > * @data_shift: log2 of FIFO item size. > * @part_buf_start: Start index in part_buf. > @@ -166,6 +168,10 @@ struct dw_mci { > struct clk *ciu_clk; > struct dw_mci_slot *slot[MAX_MCI_SLOTS]; > > + /* Phase Shift Value (for exynos5250 variant) */ > + u32 sdr_timing; > + u32 ddr_timing; > + > /* FIFO push and pull */ > int fifo_depth; > int data_shift; > -- > 1.6.6.rc2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 19 July 2012 09:21, Seungwon Jeon <tgih.jun@samsung.com> wrote: > Hi, > > This version does not seems to consider previous reviews fully. > Could you check the comments below? I did try to address all the comments. I will check again and resubmit if I have missed anything. > > July 12, 2012, Thomas Abraham <thomas.abraham@linaro.org> wrote: >> The instantiation of the Synopsis Designware controller on Exynos5250 >> include extension for SDR and DDR specific tx/rx phase shift timing >> and CIU internal divider. In addition to that, the option to skip the >> command hold stage is also introduced. Add support for these Exynos5250 >> specfic extenstions. >> >> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com> >> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org> >> --- >> .../devicetree/bindings/mmc/synposis-dw-mshc.txt | 38 ++++++++++++++++++- >> drivers/mmc/host/dw_mmc-pltfm.c | 15 +++++++ >> drivers/mmc/host/dw_mmc.c | 40 +++++++++++++++++++- >> drivers/mmc/host/dw_mmc.h | 14 +++++++ >> include/linux/mmc/dw_mmc.h | 6 +++ >> 5 files changed, 110 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt >> b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt >> index 3acd6c9..69d78c1 100644 >> --- a/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt >> +++ b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt >> @@ -7,6 +7,8 @@ Required Properties: >> >> * compatible: should be one of the following >> - snps,dw-mshc: for controllers compliant with synopsis dw-mshc. >> + - samsung,exynos5250-dw-mshc: for controllers with Samsung >> + Exynos5250 specific extentions. >> >> * reg: physical base address of the dw-mshc controller and size of its memory >> region. >> @@ -74,13 +76,45 @@ Aliases: >> the following format 'mshc{n}' where n is a unique number for the alias. >> >> >> +Samsung Exynos4/5 specific properties: >> + >> +Some of the variants of Exynos4 (such as Exynos4412) and Exynos5 SoC's >> +includes few extensions to the Synopsis Designware Mobile Storage Host >> +Controller. The following properties are used to describe those extensions. >> + >> +* samsung,dw-mshc-sdr-timing: Specifies the value of CUI clock divider, CIU >> + clock phase shift value in transmit mode and CIU clock phase shift value in >> + receive mode for single data rate mode operation. Refer notes of the valid >> + values below. >> + >> +* samsung,dw-mshc-ddr-timing: Specifies the value of CUI clock divider, CIU >> + clock phase shift value in transmit mode and CIU clock phase shift value in >> + receive mode for double data rate mode operation. Refer notes of the valid >> + values below. The order of the cells should be >> + >> + - First Cell: CIU clock divider value (applicable only for Exynos5 >> + SoC's, should be zero for Exynos4 SoC's) >> + - Second Cell: CIU clock phase shift value for tx mode. >> + - Third Cell: CIU clock phase shift value for rx mode. >> + >> + Valid values for SDR and DDR CIU clock timing for Exynos5250: >> + >> + - valid values for CIU clock divider, tx phase shift and rx phase shift >> + is 0 to 7. >> + >> + - When CIU clock divider value is set to 3, all possible 8 phase shift >> + values can be used. >> + >> + - If CIU clock divider value is 0 (that is divide by 1), both tx and rx >> + phase shift clocks should be 0. >> + >> Example: >> >> The MSHC controller node can be split into two portions, SoC specific and >> board specific portions as listed below. >> >> dwmmc0@12200000 { >> - compatible = "snps,dw-mshc"; >> + compatible = "samsung,exynos5250-dw-mshc"; >> reg = <0x12200000 0x1000>; >> interrupts = <0 75 0>; >> #address-cells = <1>; >> @@ -94,6 +128,8 @@ Example: >> no-write-protect; >> fifo-depth = <0x80>; >> card-detect-delay = <200>; >> + samsung,dw-mshc-sdr-timing = <2 3 3>; >> + samsung,dw-mshc-ddr-timing = <1 2 3>; >> >> slot@0 { >> reg = <0>; >> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c >> index 8d24f6d..900f412 100644 >> --- a/drivers/mmc/host/dw_mmc-pltfm.c >> +++ b/drivers/mmc/host/dw_mmc-pltfm.c >> @@ -27,9 +27,24 @@ static struct dw_mci_drv_data synopsis_drv_data = { >> .ctrl_type = DW_MCI_TYPE_SYNOPSIS, >> }; >> >> +static unsigned long exynos5250_dwmmc_caps[4] = { >> + MMC_CAP_UHS_DDR50 | MMC_CAP_1_8V_DDR | >> + MMC_CAP_8_BIT_DATA | MMC_CAP_CMD23, >> + MMC_CAP_CMD23, >> + MMC_CAP_CMD23, >> + MMC_CAP_CMD23, >> +}; >> + > Kyungmin Park has already pointed . > It's not still proper place for board specific caps. > If I'm incorrect, please let me know. > And why MMC_CAP_CMD23 is default caps for all channel of hosts? The cap listed above are specifying controller capabilities for dw-mmc controllers on Exynos5 SoC. They are not board specific caps. All the Exynos5 dw-mmc controllers can support MMC_CAP_CMD23 cap and hence, it has been listed for all the controllers. Please let me know if you feel there is any change required here. > >> +static struct dw_mci_drv_data exynos5250_drv_data = { >> + .ctrl_type = DW_MCI_TYPE_EXYNOS5250, >> + .caps = exynos5250_dwmmc_caps, >> +}; >> + >> static const struct of_device_id dw_mci_pltfm_match[] = { >> { .compatible = "snps,dw-mshc", >> .data = (void *)&synopsis_drv_data, }, >> + { .compatible = "samsung,exynos5250-dw-mshc", >> + .data = (void *)&exynos5250_drv_data, }, >> {}, >> }; >> MODULE_DEVICE_TABLE(of, dw_mci_pltfm_match); >> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >> index 3bc276d..bbf1209 100644 >> --- a/drivers/mmc/host/dw_mmc.c >> +++ b/drivers/mmc/host/dw_mmc.c >> @@ -236,6 +236,7 @@ static void dw_mci_set_timeout(struct dw_mci *host) >> static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd) >> { >> struct mmc_data *data; >> + struct dw_mci_slot *slot = mmc_priv(mmc); >> u32 cmdr; >> cmd->error = -EINPROGRESS; >> >> @@ -265,6 +266,17 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd) >> cmdr |= SDMMC_CMD_DAT_WR; >> } >> >> + /* >> + * Samsung Exynos5250 extends the use of CMD register with the use of >> + * bit 29 (which is reserved on standard MSHC controllers) for >> + * optionally bypassing the HOLD register for command and data. The >> + * HOLD register should be bypassed in case there is no phase shift >> + * applied on CMD/DATA that is sent to the card. >> + */ >> + if (slot->host->drv_data->ctrl_type == DW_MCI_TYPE_EXYNOS5250) >> + if (SDMMC_CLKSEL_GET_SELCLK_DRV(mci_readl(slot->host, CLKSEL))) >> + cmdr |= SDMMC_CMD_USE_HOLD_REG; >> + >> return cmdr; >> } >> >> @@ -802,10 +814,19 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) >> regs = mci_readl(slot->host, UHS_REG); >> >> /* DDR mode set */ >> - if (ios->timing == MMC_TIMING_UHS_DDR50) >> + if (ios->timing == MMC_TIMING_UHS_DDR50) { >> regs |= (0x1 << slot->id) << 16; >> - else >> + mci_writel(slot->host, CLKSEL, slot->host->ddr_timing); >> + } else { >> regs &= ~(0x1 << slot->id) << 16; >> + mci_writel(slot->host, CLKSEL, slot->host->sdr_timing); >> + } >> + >> + if (slot->host->drv_data->ctrl_type == DW_MCI_TYPE_EXYNOS5250) { >> + slot->host->bus_hz = clk_get_rate(slot->host->ciu_clk); >> + slot->host->bus_hz /= SDMMC_CLKSEL_GET_DIVRATIO( >> + mci_readl(slot->host, CLKSEL)); >> + } > As you know, CLKSEL is specific for Samsung soc. > 0x09C(CLKSEL) is reserved area in Synopsys memory map. > In case of non-samsung-soc, we cannot ensure this usage. > In previous version, I have suggested separating the variant into another file. There is a check for type of SoC before using 0x9C as CLKSEL register. Other implementations of dw-mmc might define custom register at 0x9C but this will code will not execute on other SoC's and will not break anything on other implementations. Regarding spliting this Exynos specific code into another file, I prefer not to do it for now. Spliting the code means adding new definitions of callback functions which I am not sure is really required. The present code is fairly simple one. > >> >> mci_writel(slot->host, UHS_REG, regs); >> >> @@ -2086,6 +2107,7 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host) >> struct dw_mci_board *pdata; >> struct device *dev = host->dev; >> struct device_node *np = dev->of_node; >> + u32 timing[3]; >> int idx, cnt; >> >> pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); >> @@ -2108,6 +2130,20 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host) >> if (of_get_property(np, of_quriks[idx].quirk, NULL)) >> pdata->quirks |= of_quriks[idx].id; >> >> + if (of_property_read_u32_array(dev->of_node, >> + "samsung,dw-mshc-sdr-timing", timing, 3)) >> + host->sdr_timing = DW_MCI_DEF_SDR_TIMING; > Host of non-samsung will reach here. > host->sdr_timing is needed for this host? host->ddr_timing is the same. Yes, but non-samsung hosts will not have have this property into their dts file. So the code within the condition will not execute on non-samsung hosts. SDR and DDR timing are required for Exynos5 SoC. > >> + else >> + host->sdr_timing = SDMMC_CLKSEL_TIMING(timing[0], >> + timing[1], timing[2]); >> + >> + if (of_property_read_u32_array(dev->of_node, >> + "samsung,dw-mshc-ddr-timing", timing, 3)) >> + host->ddr_timing = DW_MCI_DEF_DDR_TIMING; >> + else >> + host->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], >> + timing[1], timing[2]); >> + >> if (of_property_read_u32(np, "fifo-depth", &pdata->fifo_depth)) >> dev_info(dev, "fifo-depth property not found, using " >> "value of FIFOTH register as default\n"); >> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h >> index 1ecaa02..6c17282 100644 >> --- a/drivers/mmc/host/dw_mmc.h >> +++ b/drivers/mmc/host/dw_mmc.h >> @@ -53,6 +53,7 @@ >> #define SDMMC_IDINTEN 0x090 >> #define SDMMC_DSCADDR 0x094 >> #define SDMMC_BUFADDR 0x098 >> +#define SDMMC_CLKSEL 0x09C /* specific to Samsung Exynos5250 */ >> #define SDMMC_DATA(x) (x) >> >> /* >> @@ -111,6 +112,7 @@ >> #define SDMMC_INT_ERROR 0xbfc2 >> /* Command register defines */ >> #define SDMMC_CMD_START BIT(31) >> +#define SDMMC_CMD_USE_HOLD_REG BIT(29) >> #define SDMMC_CMD_CCS_EXP BIT(23) >> #define SDMMC_CMD_CEATA_RD BIT(22) >> #define SDMMC_CMD_UPD_CLK BIT(21) >> @@ -142,6 +144,17 @@ >> /* Version ID register define */ >> #define SDMMC_GET_VERID(x) ((x) & 0xFFFF) >> >> +#define DW_MCI_DEF_SDR_TIMING 0x03030002 >> +#define DW_MCI_DEF_DDR_TIMING 0x03020001 > What is the basis for these timing? > These values is board-specific. > >> +#define SDMMC_CLKSEL_CCLK_SAMPLE(x) (((x) & 3) << 0) >> +#define SDMMC_CLKSEL_CCLK_DRIVE(x) (((x) & 3) << 16) >> +#define SDMMC_CLKSEL_CCLK_DIVIDER(x) (((x) & 3) << 24) > If it's for exynos5, it will be 7 not 3. Yes, I missed that. Thanks. I will fix this. > >> +#define SDMMC_CLKSEL_TIMING(x, y, z) (SDMMC_CLKSEL_CCLK_SAMPLE(x) | \ >> + SDMMC_CLKSEL_CCLK_DRIVE(y) | \ >> + SDMMC_CLKSEL_CCLK_DIVIDER(z)) >> +#define SDMMC_CLKSEL_GET_DIVRATIO(x) ((((x) >> 24) & 0x7) + 1) >> +#define SDMMC_CLKSEL_GET_SELCLK_DRV(x) (((x) >> 16) & 0x7) >> + > Is this patch considered only for exynos5250? > In case of exynos4210, the number of bits is different. > If upper macros is backward-compatible, it would be better. These consider the Exynos4210 and Exynos4412 implementations as well. The device tree documentation clearly states that the possible values for each of the dividers. For Exynos4 SoC's, the divider value is between 1 to 4 (or 0 to 3). So a bit mask of 7 is backward compatilble for Exynos4. Thanks for your review and comments on this patch. Regards, Thomas. > > Best regards, > Seungwon Jeon > >> /* Register access macros */ >> #define mci_readl(dev, reg) \ >> __raw_readl((dev)->regs + SDMMC_##reg) >> @@ -184,6 +197,7 @@ extern int dw_mci_resume(struct dw_mci *host); >> >> /* Variations in the dw_mci controller */ >> #define DW_MCI_TYPE_SYNOPSIS 0 >> +#define DW_MCI_TYPE_EXYNOS5250 1 /* Samsung Exynos5250 Extensions */ >> >> /* dw_mci platform driver data */ >> struct dw_mci_drv_data { >> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h >> index ae45e4f..32c778f 100644 >> --- a/include/linux/mmc/dw_mmc.h >> +++ b/include/linux/mmc/dw_mmc.h >> @@ -82,6 +82,8 @@ struct mmc_data; >> * @biu_clk: Pointer to bus interface unit clock instance. >> * @ciu_clk: Pointer to card interface unit clock instance. >> * @slot: Slots sharing this MMC controller. >> + * @sdr_timing: Clock phase shifting for driving and sampling in sdr mode >> + * @ddr_timing: Clock phase shifting for driving and sampling in ddr mode >> * @fifo_depth: depth of FIFO. >> * @data_shift: log2 of FIFO item size. >> * @part_buf_start: Start index in part_buf. >> @@ -166,6 +168,10 @@ struct dw_mci { >> struct clk *ciu_clk; >> struct dw_mci_slot *slot[MAX_MCI_SLOTS]; >> >> + /* Phase Shift Value (for exynos5250 variant) */ >> + u32 sdr_timing; >> + u32 ddr_timing; >> + >> /* FIFO push and pull */ >> int fifo_depth; >> int data_shift; >> -- >> 1.6.6.rc2 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
July 20, 2012, Thomas Abraham <thomas.abraham@linaro.org> wrote: > On 19 July 2012 09:21, Seungwon Jeon <tgih.jun@samsung.com> wrote: > > Hi, > > > > This version does not seems to consider previous reviews fully. > > Could you check the comments below? > > I did try to address all the comments. I will check again and resubmit > if I have missed anything. > > > > > July 12, 2012, Thomas Abraham <thomas.abraham@linaro.org> wrote: > >> The instantiation of the Synopsis Designware controller on Exynos5250 > >> include extension for SDR and DDR specific tx/rx phase shift timing > >> and CIU internal divider. In addition to that, the option to skip the > >> command hold stage is also introduced. Add support for these Exynos5250 > >> specfic extenstions. > >> > >> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com> > >> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org> > >> --- > >> .../devicetree/bindings/mmc/synposis-dw-mshc.txt | 38 ++++++++++++++++++- > >> drivers/mmc/host/dw_mmc-pltfm.c | 15 +++++++ > >> drivers/mmc/host/dw_mmc.c | 40 +++++++++++++++++++- > >> drivers/mmc/host/dw_mmc.h | 14 +++++++ > >> include/linux/mmc/dw_mmc.h | 6 +++ > >> 5 files changed, 110 insertions(+), 3 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt > >> b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt > >> index 3acd6c9..69d78c1 100644 > >> --- a/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt > >> +++ b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt > >> @@ -7,6 +7,8 @@ Required Properties: > >> > >> * compatible: should be one of the following > >> - snps,dw-mshc: for controllers compliant with synopsis dw-mshc. > >> + - samsung,exynos5250-dw-mshc: for controllers with Samsung > >> + Exynos5250 specific extentions. > >> > >> * reg: physical base address of the dw-mshc controller and size of its memory > >> region. > >> @@ -74,13 +76,45 @@ Aliases: > >> the following format 'mshc{n}' where n is a unique number for the alias. > >> > >> > >> +Samsung Exynos4/5 specific properties: > >> + > >> +Some of the variants of Exynos4 (such as Exynos4412) and Exynos5 SoC's > >> +includes few extensions to the Synopsis Designware Mobile Storage Host > >> +Controller. The following properties are used to describe those extensions. > >> + > >> +* samsung,dw-mshc-sdr-timing: Specifies the value of CUI clock divider, CIU > >> + clock phase shift value in transmit mode and CIU clock phase shift value in > >> + receive mode for single data rate mode operation. Refer notes of the valid > >> + values below. > >> + > >> +* samsung,dw-mshc-ddr-timing: Specifies the value of CUI clock divider, CIU > >> + clock phase shift value in transmit mode and CIU clock phase shift value in > >> + receive mode for double data rate mode operation. Refer notes of the valid > >> + values below. The order of the cells should be > >> + > >> + - First Cell: CIU clock divider value (applicable only for Exynos5 > >> + SoC's, should be zero for Exynos4 SoC's) > >> + - Second Cell: CIU clock phase shift value for tx mode. > >> + - Third Cell: CIU clock phase shift value for rx mode. > >> + > >> + Valid values for SDR and DDR CIU clock timing for Exynos5250: > >> + > >> + - valid values for CIU clock divider, tx phase shift and rx phase shift > >> + is 0 to 7. > >> + > >> + - When CIU clock divider value is set to 3, all possible 8 phase shift > >> + values can be used. > >> + > >> + - If CIU clock divider value is 0 (that is divide by 1), both tx and rx > >> + phase shift clocks should be 0. > >> + > >> Example: > >> > >> The MSHC controller node can be split into two portions, SoC specific and > >> board specific portions as listed below. > >> > >> dwmmc0@12200000 { > >> - compatible = "snps,dw-mshc"; > >> + compatible = "samsung,exynos5250-dw-mshc"; > >> reg = <0x12200000 0x1000>; > >> interrupts = <0 75 0>; > >> #address-cells = <1>; > >> @@ -94,6 +128,8 @@ Example: > >> no-write-protect; > >> fifo-depth = <0x80>; > >> card-detect-delay = <200>; > >> + samsung,dw-mshc-sdr-timing = <2 3 3>; > >> + samsung,dw-mshc-ddr-timing = <1 2 3>; > >> > >> slot@0 { > >> reg = <0>; > >> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c > >> index 8d24f6d..900f412 100644 > >> --- a/drivers/mmc/host/dw_mmc-pltfm.c > >> +++ b/drivers/mmc/host/dw_mmc-pltfm.c > >> @@ -27,9 +27,24 @@ static struct dw_mci_drv_data synopsis_drv_data = { > >> .ctrl_type = DW_MCI_TYPE_SYNOPSIS, > >> }; > >> > >> +static unsigned long exynos5250_dwmmc_caps[4] = { > >> + MMC_CAP_UHS_DDR50 | MMC_CAP_1_8V_DDR | > >> + MMC_CAP_8_BIT_DATA | MMC_CAP_CMD23, > >> + MMC_CAP_CMD23, > >> + MMC_CAP_CMD23, > >> + MMC_CAP_CMD23, > >> +}; > >> + > > Kyungmin Park has already pointed . > > It's not still proper place for board specific caps. > > If I'm incorrect, please let me know. > > And why MMC_CAP_CMD23 is default caps for all channel of hosts? > > The cap listed above are specifying controller capabilities for dw-mmc > controllers on Exynos5 SoC. They are not board specific caps. All the > Exynos5 dw-mmc controllers can support MMC_CAP_CMD23 cap and hence, it > has been listed for all the controllers. Please let me know if you > feel there is any change required here. MMC_CAP_8_BIT_DATA could be dependent on board. I agree about MMC_CAP_CMD23. Additionally, MMC_CAP_CMD23 is applied for dw-mmc host driver without regard to Exynos5. > > > > >> +static struct dw_mci_drv_data exynos5250_drv_data = { > >> + .ctrl_type = DW_MCI_TYPE_EXYNOS5250, > >> + .caps = exynos5250_dwmmc_caps, > >> +}; > >> + > >> static const struct of_device_id dw_mci_pltfm_match[] = { > >> { .compatible = "snps,dw-mshc", > >> .data = (void *)&synopsis_drv_data, }, > >> + { .compatible = "samsung,exynos5250-dw-mshc", > >> + .data = (void *)&exynos5250_drv_data, }, > >> {}, > >> }; > >> MODULE_DEVICE_TABLE(of, dw_mci_pltfm_match); > >> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > >> index 3bc276d..bbf1209 100644 > >> --- a/drivers/mmc/host/dw_mmc.c > >> +++ b/drivers/mmc/host/dw_mmc.c > >> @@ -236,6 +236,7 @@ static void dw_mci_set_timeout(struct dw_mci *host) > >> static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd) > >> { > >> struct mmc_data *data; > >> + struct dw_mci_slot *slot = mmc_priv(mmc); > >> u32 cmdr; > >> cmd->error = -EINPROGRESS; > >> > >> @@ -265,6 +266,17 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command > *cmd) > >> cmdr |= SDMMC_CMD_DAT_WR; > >> } > >> > >> + /* > >> + * Samsung Exynos5250 extends the use of CMD register with the use of > >> + * bit 29 (which is reserved on standard MSHC controllers) for > >> + * optionally bypassing the HOLD register for command and data. The > >> + * HOLD register should be bypassed in case there is no phase shift > >> + * applied on CMD/DATA that is sent to the card. > >> + */ > >> + if (slot->host->drv_data->ctrl_type == DW_MCI_TYPE_EXYNOS5250) > >> + if (SDMMC_CLKSEL_GET_SELCLK_DRV(mci_readl(slot->host, CLKSEL))) > >> + cmdr |= SDMMC_CMD_USE_HOLD_REG; > >> + > >> return cmdr; > >> } > >> > >> @@ -802,10 +814,19 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > >> regs = mci_readl(slot->host, UHS_REG); > >> > >> /* DDR mode set */ > >> - if (ios->timing == MMC_TIMING_UHS_DDR50) > >> + if (ios->timing == MMC_TIMING_UHS_DDR50) { > >> regs |= (0x1 << slot->id) << 16; > >> - else > >> + mci_writel(slot->host, CLKSEL, slot->host->ddr_timing); > >> + } else { > >> regs &= ~(0x1 << slot->id) << 16; > >> + mci_writel(slot->host, CLKSEL, slot->host->sdr_timing); > >> + } > >> + > >> + if (slot->host->drv_data->ctrl_type == DW_MCI_TYPE_EXYNOS5250) { > >> + slot->host->bus_hz = clk_get_rate(slot->host->ciu_clk); > >> + slot->host->bus_hz /= SDMMC_CLKSEL_GET_DIVRATIO( > >> + mci_readl(slot->host, CLKSEL)); > >> + } > > As you know, CLKSEL is specific for Samsung soc. > > 0x09C(CLKSEL) is reserved area in Synopsys memory map. > > In case of non-samsung-soc, we cannot ensure this usage. > > In previous version, I have suggested separating the variant into another file. > > There is a check for type of SoC before using 0x9C as CLKSEL register. Do you mean checking DW_MCI_TYPE_EXYNOS5250? But Above two case(ddr_timing/sdr_timing), CLKSEL can be accessed on other soc's. > Other implementations of dw-mmc might define custom register at 0x9C Even so, register field can be different with Samsung soc. > but this will code will not execute on other SoC's and will not break > anything on other implementations. Regarding spliting this Exynos > specific code into another file, I prefer not to do it for now. > Spliting the code means adding new definitions of callback functions > which I am not sure is really required. The present code is fairly > simple one. Yes, callback functions might be needed to accommodate various implementation of host controller. It would be better to prepare this for other variant next. > > > > >> > >> mci_writel(slot->host, UHS_REG, regs); > >> > >> @@ -2086,6 +2107,7 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host) > >> struct dw_mci_board *pdata; > >> struct device *dev = host->dev; > >> struct device_node *np = dev->of_node; > >> + u32 timing[3]; > >> int idx, cnt; > >> > >> pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > >> @@ -2108,6 +2130,20 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host) > >> if (of_get_property(np, of_quriks[idx].quirk, NULL)) > >> pdata->quirks |= of_quriks[idx].id; > >> > >> + if (of_property_read_u32_array(dev->of_node, > >> + "samsung,dw-mshc-sdr-timing", timing, 3)) > >> + host->sdr_timing = DW_MCI_DEF_SDR_TIMING; > > Host of non-samsung will reach here. > > host->sdr_timing is needed for this host? host->ddr_timing is the same. > > Yes, but non-samsung hosts will not have have this property into their > dts file. So the code within the condition will not execute on > non-samsung hosts. SDR and DDR timing are required for Exynos5 SoC. Yes, these are required only for Exynos Soc. Non-samsung host will have default value here, but it seems to be meaningless. > > > > >> + else > >> + host->sdr_timing = SDMMC_CLKSEL_TIMING(timing[0], > >> + timing[1], timing[2]); > >> + > >> + if (of_property_read_u32_array(dev->of_node, > >> + "samsung,dw-mshc-ddr-timing", timing, 3)) > >> + host->ddr_timing = DW_MCI_DEF_DDR_TIMING; > >> + else > >> + host->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], > >> + timing[1], timing[2]); > >> + > >> if (of_property_read_u32(np, "fifo-depth", &pdata->fifo_depth)) > >> dev_info(dev, "fifo-depth property not found, using " > >> "value of FIFOTH register as default\n"); > >> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h > >> index 1ecaa02..6c17282 100644 > >> --- a/drivers/mmc/host/dw_mmc.h > >> +++ b/drivers/mmc/host/dw_mmc.h > >> @@ -53,6 +53,7 @@ > >> #define SDMMC_IDINTEN 0x090 > >> #define SDMMC_DSCADDR 0x094 > >> #define SDMMC_BUFADDR 0x098 > >> +#define SDMMC_CLKSEL 0x09C /* specific to Samsung Exynos5250 */ > >> #define SDMMC_DATA(x) (x) > >> > >> /* > >> @@ -111,6 +112,7 @@ > >> #define SDMMC_INT_ERROR 0xbfc2 > >> /* Command register defines */ > >> #define SDMMC_CMD_START BIT(31) > >> +#define SDMMC_CMD_USE_HOLD_REG BIT(29) > >> #define SDMMC_CMD_CCS_EXP BIT(23) > >> #define SDMMC_CMD_CEATA_RD BIT(22) > >> #define SDMMC_CMD_UPD_CLK BIT(21) > >> @@ -142,6 +144,17 @@ > >> /* Version ID register define */ > >> #define SDMMC_GET_VERID(x) ((x) & 0xFFFF) > >> > >> +#define DW_MCI_DEF_SDR_TIMING 0x03030002 > >> +#define DW_MCI_DEF_DDR_TIMING 0x03020001 > > What is the basis for these timing? > > These values is board-specific. One missed comment? > > > >> +#define SDMMC_CLKSEL_CCLK_SAMPLE(x) (((x) & 3) << 0) > >> +#define SDMMC_CLKSEL_CCLK_DRIVE(x) (((x) & 3) << 16) > >> +#define SDMMC_CLKSEL_CCLK_DIVIDER(x) (((x) & 3) << 24) > > If it's for exynos5, it will be 7 not 3. > > Yes, I missed that. Thanks. I will fix this. > > > > >> +#define SDMMC_CLKSEL_TIMING(x, y, z) (SDMMC_CLKSEL_CCLK_SAMPLE(x) | \ > >> + SDMMC_CLKSEL_CCLK_DRIVE(y) | \ > >> + SDMMC_CLKSEL_CCLK_DIVIDER(z)) > >> +#define SDMMC_CLKSEL_GET_DIVRATIO(x) ((((x) >> 24) & 0x7) + 1) > >> +#define SDMMC_CLKSEL_GET_SELCLK_DRV(x) (((x) >> 16) & 0x7) > >> + > > Is this patch considered only for exynos5250? > > In case of exynos4210, the number of bits is different. > > If upper macros is backward-compatible, it would be better. > > These consider the Exynos4210 and Exynos4412 implementations as well. > The device tree documentation clearly states that the possible values > for each of the dividers. For Exynos4 SoC's, the divider value is > between 1 to 4 (or 0 to 3). So a bit mask of 7 is backward compatilble > for Exynos4. Bit width is 2 for selclk_drv in exynos4210. So bit mask of 3 is proper. Let me clear it about divider value. In case of Exynos4 SoC's, divider value(DIVRATIO) is reserved and host doesn't modify. But value is fixed internally like following. Exynos4210 : 2 Exynos4412 : 4 Thanks, Seungwon Jeon > > Thanks for your review and comments on this patch. > > Regards, > Thomas. > > > > > Best regards, > > Seungwon Jeon > > > >> /* Register access macros */ > >> #define mci_readl(dev, reg) \ > >> __raw_readl((dev)->regs + SDMMC_##reg) > >> @@ -184,6 +197,7 @@ extern int dw_mci_resume(struct dw_mci *host); > >> > >> /* Variations in the dw_mci controller */ > >> #define DW_MCI_TYPE_SYNOPSIS 0 > >> +#define DW_MCI_TYPE_EXYNOS5250 1 /* Samsung Exynos5250 Extensions */ > >> > >> /* dw_mci platform driver data */ > >> struct dw_mci_drv_data { > >> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h > >> index ae45e4f..32c778f 100644 > >> --- a/include/linux/mmc/dw_mmc.h > >> +++ b/include/linux/mmc/dw_mmc.h > >> @@ -82,6 +82,8 @@ struct mmc_data; > >> * @biu_clk: Pointer to bus interface unit clock instance. > >> * @ciu_clk: Pointer to card interface unit clock instance. > >> * @slot: Slots sharing this MMC controller. > >> + * @sdr_timing: Clock phase shifting for driving and sampling in sdr mode > >> + * @ddr_timing: Clock phase shifting for driving and sampling in ddr mode > >> * @fifo_depth: depth of FIFO. > >> * @data_shift: log2 of FIFO item size. > >> * @part_buf_start: Start index in part_buf. > >> @@ -166,6 +168,10 @@ struct dw_mci { > >> struct clk *ciu_clk; > >> struct dw_mci_slot *slot[MAX_MCI_SLOTS]; > >> > >> + /* Phase Shift Value (for exynos5250 variant) */ > >> + u32 sdr_timing; > >> + u32 ddr_timing; > >> + > >> /* FIFO push and pull */ > >> int fifo_depth; > >> int data_shift; > >> -- > >> 1.6.6.rc2 > >> > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > >> the body of a message to majordomo@vger.kernel.org > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20 July 2012 16:08, Seungwon Jeon <tgih.jun@samsung.com> wrote: > July 20, 2012, Thomas Abraham <thomas.abraham@linaro.org> wrote: >> On 19 July 2012 09:21, Seungwon Jeon <tgih.jun@samsung.com> wrote: [...] >> >> +static unsigned long exynos5250_dwmmc_caps[4] = { >> >> + MMC_CAP_UHS_DDR50 | MMC_CAP_1_8V_DDR | >> >> + MMC_CAP_8_BIT_DATA | MMC_CAP_CMD23, >> >> + MMC_CAP_CMD23, >> >> + MMC_CAP_CMD23, >> >> + MMC_CAP_CMD23, >> >> +}; >> >> + >> > Kyungmin Park has already pointed . >> > It's not still proper place for board specific caps. >> > If I'm incorrect, please let me know. >> > And why MMC_CAP_CMD23 is default caps for all channel of hosts? >> >> The cap listed above are specifying controller capabilities for dw-mmc >> controllers on Exynos5 SoC. They are not board specific caps. All the >> Exynos5 dw-mmc controllers can support MMC_CAP_CMD23 cap and hence, it >> has been listed for all the controllers. Please let me know if you >> feel there is any change required here. > MMC_CAP_8_BIT_DATA could be dependent on board. A controller can have the MMC_CAP_8_BIT_DATA capability but the board will decide the bus-width. The bus-width is specified in the dts files of each board (or platform data). The bus-width for data transfer is then decided by the MMC core code based on the caps and the bus-width information. So MMC_CAP_8_BIT_DATA can be specified irrespective of whether the board supports 8-bit or not. > I agree about MMC_CAP_CMD23. > Additionally, MMC_CAP_CMD23 is applied for dw-mmc host driver without regard to Exynos5. The caps listed in exynos5250_dwmmc_caps is applicable only for Exynos5 SoC's. Could you please let me know if there is anything incorrect here. [...] >> >> + if (slot->host->drv_data->ctrl_type == DW_MCI_TYPE_EXYNOS5250) { >> >> + slot->host->bus_hz = clk_get_rate(slot->host->ciu_clk); >> >> + slot->host->bus_hz /= SDMMC_CLKSEL_GET_DIVRATIO( >> >> + mci_readl(slot->host, CLKSEL)); >> >> + } >> > As you know, CLKSEL is specific for Samsung soc. >> > 0x09C(CLKSEL) is reserved area in Synopsys memory map. >> > In case of non-samsung-soc, we cannot ensure this usage. >> > In previous version, I have suggested separating the variant into another file. >> >> There is a check for type of SoC before using 0x9C as CLKSEL register. > Do you mean checking DW_MCI_TYPE_EXYNOS5250? > But Above two case(ddr_timing/sdr_timing), CLKSEL can be accessed on other soc's. The tests have only been completed on Exynos5250. I do not have boards for other Samsung SoC's which have a dw_mmc port connected and used on the board. When we have other platforms tested with this patchset, we can extend the 'if' check in the above code for other SoC's. > >> Other implementations of dw-mmc might define custom register at 0x9C > Even so, register field can be different with Samsung soc. Yes, with the correct checks for the type of SoC, differences in the usage of 0x9C register can be handled. > >> but this will code will not execute on other SoC's and will not break >> anything on other implementations. Regarding spliting this Exynos >> specific code into another file, I prefer not to do it for now. >> Spliting the code means adding new definitions of callback functions >> which I am not sure is really required. The present code is fairly >> simple one. > Yes, callback functions might be needed to accommodate various implementation > of host controller. It would be better to prepare this for other variant next. > Ok. I will relook at these patches and check if we really need split these changes into a separate exynos specific file. If I again feel that such a split is not required, I will reply back to you with justification. [...] >> >> + if (of_property_read_u32_array(dev->of_node, >> >> + "samsung,dw-mshc-sdr-timing", timing, 3)) >> >> + host->sdr_timing = DW_MCI_DEF_SDR_TIMING; >> > Host of non-samsung will reach here. >> > host->sdr_timing is needed for this host? host->ddr_timing is the same. >> >> Yes, but non-samsung hosts will not have have this property into their >> dts file. So the code within the condition will not execute on >> non-samsung hosts. SDR and DDR timing are required for Exynos5 SoC. > Yes, these are required only for Exynos Soc. > Non-samsung host will have default value here, but it seems to be meaningless. Non-Samsung platforms will not have this property in their dts files. This property is required on only those platforms that want to define sdr and ddr timing values. It is not required on platforms that do no use it. > >> >> > >> >> + else >> >> + host->sdr_timing = SDMMC_CLKSEL_TIMING(timing[0], >> >> + timing[1], timing[2]); >> >> + >> >> + if (of_property_read_u32_array(dev->of_node, >> >> + "samsung,dw-mshc-ddr-timing", timing, 3)) >> >> + host->ddr_timing = DW_MCI_DEF_DDR_TIMING; >> >> + else >> >> + host->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], >> >> + timing[1], timing[2]); >> >> + >> >> if (of_property_read_u32(np, "fifo-depth", &pdata->fifo_depth)) >> >> dev_info(dev, "fifo-depth property not found, using " >> >> "value of FIFOTH register as default\n"); >> >> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h >> >> index 1ecaa02..6c17282 100644 >> >> --- a/drivers/mmc/host/dw_mmc.h >> >> +++ b/drivers/mmc/host/dw_mmc.h >> >> @@ -53,6 +53,7 @@ >> >> #define SDMMC_IDINTEN 0x090 >> >> #define SDMMC_DSCADDR 0x094 >> >> #define SDMMC_BUFADDR 0x098 >> >> +#define SDMMC_CLKSEL 0x09C /* specific to Samsung Exynos5250 */ >> >> #define SDMMC_DATA(x) (x) >> >> >> >> /* >> >> @@ -111,6 +112,7 @@ >> >> #define SDMMC_INT_ERROR 0xbfc2 >> >> /* Command register defines */ >> >> #define SDMMC_CMD_START BIT(31) >> >> +#define SDMMC_CMD_USE_HOLD_REG BIT(29) >> >> #define SDMMC_CMD_CCS_EXP BIT(23) >> >> #define SDMMC_CMD_CEATA_RD BIT(22) >> >> #define SDMMC_CMD_UPD_CLK BIT(21) >> >> @@ -142,6 +144,17 @@ >> >> /* Version ID register define */ >> >> #define SDMMC_GET_VERID(x) ((x) & 0xFFFF) >> >> >> >> +#define DW_MCI_DEF_SDR_TIMING 0x03030002 >> >> +#define DW_MCI_DEF_DDR_TIMING 0x03020001 >> > What is the basis for these timing? >> > These values is board-specific. > One missed comment? Yes, sorry, missed this last time. These are default values for SDR and DDR timing in case it is not specified in dts file. It may be better to make it mandatory to specify the SDR and DDR property in dts files (for Exynos5250) and remove this default values. Non-samsung platforms or platforms not using this property are unaffected by this. > >> > >> >> +#define SDMMC_CLKSEL_CCLK_SAMPLE(x) (((x) & 3) << 0) >> >> +#define SDMMC_CLKSEL_CCLK_DRIVE(x) (((x) & 3) << 16) >> >> +#define SDMMC_CLKSEL_CCLK_DIVIDER(x) (((x) & 3) << 24) >> > If it's for exynos5, it will be 7 not 3. >> >> Yes, I missed that. Thanks. I will fix this. >> >> > >> >> +#define SDMMC_CLKSEL_TIMING(x, y, z) (SDMMC_CLKSEL_CCLK_SAMPLE(x) | \ >> >> + SDMMC_CLKSEL_CCLK_DRIVE(y) | \ >> >> + SDMMC_CLKSEL_CCLK_DIVIDER(z)) >> >> +#define SDMMC_CLKSEL_GET_DIVRATIO(x) ((((x) >> 24) & 0x7) + 1) >> >> +#define SDMMC_CLKSEL_GET_SELCLK_DRV(x) (((x) >> 16) & 0x7) >> >> + >> > Is this patch considered only for exynos5250? >> > In case of exynos4210, the number of bits is different. >> > If upper macros is backward-compatible, it would be better. >> >> These consider the Exynos4210 and Exynos4412 implementations as well. >> The device tree documentation clearly states that the possible values >> for each of the dividers. For Exynos4 SoC's, the divider value is >> between 1 to 4 (or 0 to 3). So a bit mask of 7 is backward compatilble >> for Exynos4. > Bit width is 2 for selclk_drv in exynos4210. > So bit mask of 3 is proper. > > Let me clear it about divider value. > In case of Exynos4 SoC's, divider value(DIVRATIO) is reserved and host doesn't modify. > But value is fixed internally like following. > Exynos4210 : 2 > Exynos4412 : 4 Ok. I will relook into this. Thanks. Regards, Thomas. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
July 23, 2012, Thomas Abraham <thomas.abraham@linaro.org> wrote: > On 20 July 2012 16:08, Seungwon Jeon <tgih.jun@samsung.com> wrote: > > July 20, 2012, Thomas Abraham <thomas.abraham@linaro.org> wrote: > >> On 19 July 2012 09:21, Seungwon Jeon <tgih.jun@samsung.com> wrote: > > [...] > > >> >> +static unsigned long exynos5250_dwmmc_caps[4] = { > >> >> + MMC_CAP_UHS_DDR50 | MMC_CAP_1_8V_DDR | > >> >> + MMC_CAP_8_BIT_DATA | MMC_CAP_CMD23, > >> >> + MMC_CAP_CMD23, > >> >> + MMC_CAP_CMD23, > >> >> + MMC_CAP_CMD23, > >> >> +}; > >> >> + > >> > Kyungmin Park has already pointed . > >> > It's not still proper place for board specific caps. > >> > If I'm incorrect, please let me know. > >> > And why MMC_CAP_CMD23 is default caps for all channel of hosts? > >> > >> The cap listed above are specifying controller capabilities for dw-mmc > >> controllers on Exynos5 SoC. They are not board specific caps. All the > >> Exynos5 dw-mmc controllers can support MMC_CAP_CMD23 cap and hence, it > >> has been listed for all the controllers. Please let me know if you > >> feel there is any change required here. > > MMC_CAP_8_BIT_DATA could be dependent on board. > > A controller can have the MMC_CAP_8_BIT_DATA capability but the board > will decide the bus-width. The bus-width is specified in the dts files > of each board (or platform data). The bus-width for data transfer is > then decided by the MMC core code based on the caps and the bus-width > information. So MMC_CAP_8_BIT_DATA can be specified irrespective of > whether the board supports 8-bit or not. > > > I agree about MMC_CAP_CMD23. > > Additionally, MMC_CAP_CMD23 is applied for dw-mmc host driver without regard to Exynos5. > > The caps listed in exynos5250_dwmmc_caps is applicable only for > Exynos5 SoC's. Could you please let me know if there is anything > incorrect here. I mean that MMC_CAP_CMD23 is a capability which is implemented in driver without dependency of SOC. So, other soc also includes MMC_CAP_CMD23. It'd rather make a default caps than list in specific soc, considering the other soc. > > [...] > > >> >> + if (slot->host->drv_data->ctrl_type == DW_MCI_TYPE_EXYNOS5250) { > >> >> + slot->host->bus_hz = clk_get_rate(slot->host->ciu_clk); > >> >> + slot->host->bus_hz /= SDMMC_CLKSEL_GET_DIVRATIO( > >> >> + mci_readl(slot->host, CLKSEL)); > >> >> + } > >> > As you know, CLKSEL is specific for Samsung soc. > >> > 0x09C(CLKSEL) is reserved area in Synopsys memory map. > >> > In case of non-samsung-soc, we cannot ensure this usage. > >> > In previous version, I have suggested separating the variant into another file. > >> > >> There is a check for type of SoC before using 0x9C as CLKSEL register. > > Do you mean checking DW_MCI_TYPE_EXYNOS5250? > > But Above two case(ddr_timing/sdr_timing), CLKSEL can be accessed on other soc's. > > The tests have only been completed on Exynos5250. I do not have boards > for other Samsung SoC's which have a dw_mmc port connected and used on > the board. When we have other platforms tested with this patchset, we > can extend the 'if' check in the above code for other SoC's. My meaning seem to be passed incorrectly. + if (ios->timing == MMC_TIMING_UHS_DDR50) { regs |= (0x1 << slot->id) << 16; - else + mci_writel(slot->host, CLKSEL, slot->host->ddr_timing); What is the execution for non-samsung soc? CLKSEL register is valid only for Exynos. dw_mci_set_ios shoud be aware of this. + } else { regs &= ~(0x1 << slot->id) << 16; + mci_writel(slot->host, CLKSEL, slot->host->sdr_timing); This line is same. + } + > > > > >> Other implementations of dw-mmc might define custom register at 0x9C > > Even so, register field can be different with Samsung soc. > > Yes, with the correct checks for the type of SoC, differences in the > usage of 0x9C register can be handled. > > > > >> but this will code will not execute on other SoC's and will not break > >> anything on other implementations. Regarding spliting this Exynos > >> specific code into another file, I prefer not to do it for now. > >> Spliting the code means adding new definitions of callback functions > >> which I am not sure is really required. The present code is fairly > >> simple one. > > Yes, callback functions might be needed to accommodate various implementation > > of host controller. It would be better to prepare this for other variant next. > > > > Ok. I will relook at these patches and check if we really need split > these changes into a separate exynos specific file. If I again feel > that such a split is not required, I will reply back to you with > justification. > > [...] > > >> >> + if (of_property_read_u32_array(dev->of_node, > >> >> + "samsung,dw-mshc-sdr-timing", timing, 3)) > >> >> + host->sdr_timing = DW_MCI_DEF_SDR_TIMING; > >> > Host of non-samsung will reach here. > >> > host->sdr_timing is needed for this host? host->ddr_timing is the same. > >> > >> Yes, but non-samsung hosts will not have have this property into their > >> dts file. So the code within the condition will not execute on > >> non-samsung hosts. SDR and DDR timing are required for Exynos5 SoC. > > Yes, these are required only for Exynos Soc. > > Non-samsung host will have default value here, but it seems to be meaningless. > > Non-Samsung platforms will not have this property in their dts files. > This property is required on only those platforms that want to define > sdr and ddr timing values. It is not required on platforms that do no > use it. > > > > >> > >> > > >> >> + else > >> >> + host->sdr_timing = SDMMC_CLKSEL_TIMING(timing[0], > >> >> + timing[1], timing[2]); > >> >> + > >> >> + if (of_property_read_u32_array(dev->of_node, > >> >> + "samsung,dw-mshc-ddr-timing", timing, 3)) > >> >> + host->ddr_timing = DW_MCI_DEF_DDR_TIMING; > >> >> + else > >> >> + host->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], > >> >> + timing[1], timing[2]); > >> >> + > >> >> if (of_property_read_u32(np, "fifo-depth", &pdata->fifo_depth)) > >> >> dev_info(dev, "fifo-depth property not found, using " > >> >> "value of FIFOTH register as default\n"); > >> >> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h > >> >> index 1ecaa02..6c17282 100644 > >> >> --- a/drivers/mmc/host/dw_mmc.h > >> >> +++ b/drivers/mmc/host/dw_mmc.h > >> >> @@ -53,6 +53,7 @@ > >> >> #define SDMMC_IDINTEN 0x090 > >> >> #define SDMMC_DSCADDR 0x094 > >> >> #define SDMMC_BUFADDR 0x098 > >> >> +#define SDMMC_CLKSEL 0x09C /* specific to Samsung Exynos5250 */ > >> >> #define SDMMC_DATA(x) (x) > >> >> > >> >> /* > >> >> @@ -111,6 +112,7 @@ > >> >> #define SDMMC_INT_ERROR 0xbfc2 > >> >> /* Command register defines */ > >> >> #define SDMMC_CMD_START BIT(31) > >> >> +#define SDMMC_CMD_USE_HOLD_REG BIT(29) > >> >> #define SDMMC_CMD_CCS_EXP BIT(23) > >> >> #define SDMMC_CMD_CEATA_RD BIT(22) > >> >> #define SDMMC_CMD_UPD_CLK BIT(21) > >> >> @@ -142,6 +144,17 @@ > >> >> /* Version ID register define */ > >> >> #define SDMMC_GET_VERID(x) ((x) & 0xFFFF) > >> >> > >> >> +#define DW_MCI_DEF_SDR_TIMING 0x03030002 > >> >> +#define DW_MCI_DEF_DDR_TIMING 0x03020001 > >> > What is the basis for these timing? > >> > These values is board-specific. > > One missed comment? > > Yes, sorry, missed this last time. These are default values for SDR > and DDR timing in case it is not specified in dts file. It may be > better to make it mandatory to specify the SDR and DDR property in dts > files (for Exynos5250) and remove this default values. Non-samsung > platforms or platforms not using this property are unaffected by this. These value(0x03030002, 0x03020001) cannot be acceptable in specific board type of Exynos5250. Assuming that timing values are not specified in dts file and board is different, default values may be not proper. As you mentioned, other approach will be needed. Thanks, Seungwon Jeon > > > > >> > > >> >> +#define SDMMC_CLKSEL_CCLK_SAMPLE(x) (((x) & 3) << 0) > >> >> +#define SDMMC_CLKSEL_CCLK_DRIVE(x) (((x) & 3) << 16) > >> >> +#define SDMMC_CLKSEL_CCLK_DIVIDER(x) (((x) & 3) << 24) > >> > If it's for exynos5, it will be 7 not 3. > >> > >> Yes, I missed that. Thanks. I will fix this. > >> > >> > > >> >> +#define SDMMC_CLKSEL_TIMING(x, y, z) (SDMMC_CLKSEL_CCLK_SAMPLE(x) | \ > >> >> + SDMMC_CLKSEL_CCLK_DRIVE(y) | \ > >> >> + SDMMC_CLKSEL_CCLK_DIVIDER(z)) > >> >> +#define SDMMC_CLKSEL_GET_DIVRATIO(x) ((((x) >> 24) & 0x7) + 1) > >> >> +#define SDMMC_CLKSEL_GET_SELCLK_DRV(x) (((x) >> 16) & 0x7) > >> >> + > >> > Is this patch considered only for exynos5250? > >> > In case of exynos4210, the number of bits is different. > >> > If upper macros is backward-compatible, it would be better. > >> > >> These consider the Exynos4210 and Exynos4412 implementations as well. > >> The device tree documentation clearly states that the possible values > >> for each of the dividers. For Exynos4 SoC's, the divider value is > >> between 1 to 4 (or 0 to 3). So a bit mask of 7 is backward compatilble > >> for Exynos4. > > Bit width is 2 for selclk_drv in exynos4210. > > So bit mask of 3 is proper. > > > > Let me clear it about divider value. > > In case of Exynos4 SoC's, divider value(DIVRATIO) is reserved and host doesn't modify. > > But value is fixed internally like following. > > Exynos4210 : 2 > > Exynos4412 : 4 > > Ok. I will relook into this. Thanks. > > Regards, > Thomas. > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt index 3acd6c9..69d78c1 100644 --- a/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt +++ b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt @@ -7,6 +7,8 @@ Required Properties: * compatible: should be one of the following - snps,dw-mshc: for controllers compliant with synopsis dw-mshc. + - samsung,exynos5250-dw-mshc: for controllers with Samsung + Exynos5250 specific extentions. * reg: physical base address of the dw-mshc controller and size of its memory region. @@ -74,13 +76,45 @@ Aliases: the following format 'mshc{n}' where n is a unique number for the alias. +Samsung Exynos4/5 specific properties: + +Some of the variants of Exynos4 (such as Exynos4412) and Exynos5 SoC's +includes few extensions to the Synopsis Designware Mobile Storage Host +Controller. The following properties are used to describe those extensions. + +* samsung,dw-mshc-sdr-timing: Specifies the value of CUI clock divider, CIU + clock phase shift value in transmit mode and CIU clock phase shift value in + receive mode for single data rate mode operation. Refer notes of the valid + values below. + +* samsung,dw-mshc-ddr-timing: Specifies the value of CUI clock divider, CIU + clock phase shift value in transmit mode and CIU clock phase shift value in + receive mode for double data rate mode operation. Refer notes of the valid + values below. The order of the cells should be + + - First Cell: CIU clock divider value (applicable only for Exynos5 + SoC's, should be zero for Exynos4 SoC's) + - Second Cell: CIU clock phase shift value for tx mode. + - Third Cell: CIU clock phase shift value for rx mode. + + Valid values for SDR and DDR CIU clock timing for Exynos5250: + + - valid values for CIU clock divider, tx phase shift and rx phase shift + is 0 to 7. + + - When CIU clock divider value is set to 3, all possible 8 phase shift + values can be used. + + - If CIU clock divider value is 0 (that is divide by 1), both tx and rx + phase shift clocks should be 0. + Example: The MSHC controller node can be split into two portions, SoC specific and board specific portions as listed below. dwmmc0@12200000 { - compatible = "snps,dw-mshc"; + compatible = "samsung,exynos5250-dw-mshc"; reg = <0x12200000 0x1000>; interrupts = <0 75 0>; #address-cells = <1>; @@ -94,6 +128,8 @@ Example: no-write-protect; fifo-depth = <0x80>; card-detect-delay = <200>; + samsung,dw-mshc-sdr-timing = <2 3 3>; + samsung,dw-mshc-ddr-timing = <1 2 3>; slot@0 { reg = <0>; diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c index 8d24f6d..900f412 100644 --- a/drivers/mmc/host/dw_mmc-pltfm.c +++ b/drivers/mmc/host/dw_mmc-pltfm.c @@ -27,9 +27,24 @@ static struct dw_mci_drv_data synopsis_drv_data = { .ctrl_type = DW_MCI_TYPE_SYNOPSIS, }; +static unsigned long exynos5250_dwmmc_caps[4] = { + MMC_CAP_UHS_DDR50 | MMC_CAP_1_8V_DDR | + MMC_CAP_8_BIT_DATA | MMC_CAP_CMD23, + MMC_CAP_CMD23, + MMC_CAP_CMD23, + MMC_CAP_CMD23, +}; + +static struct dw_mci_drv_data exynos5250_drv_data = { + .ctrl_type = DW_MCI_TYPE_EXYNOS5250, + .caps = exynos5250_dwmmc_caps, +}; + static const struct of_device_id dw_mci_pltfm_match[] = { { .compatible = "snps,dw-mshc", .data = (void *)&synopsis_drv_data, }, + { .compatible = "samsung,exynos5250-dw-mshc", + .data = (void *)&exynos5250_drv_data, }, {}, }; MODULE_DEVICE_TABLE(of, dw_mci_pltfm_match); diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 3bc276d..bbf1209 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -236,6 +236,7 @@ static void dw_mci_set_timeout(struct dw_mci *host) static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd) { struct mmc_data *data; + struct dw_mci_slot *slot = mmc_priv(mmc); u32 cmdr; cmd->error = -EINPROGRESS; @@ -265,6 +266,17 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd) cmdr |= SDMMC_CMD_DAT_WR; } + /* + * Samsung Exynos5250 extends the use of CMD register with the use of + * bit 29 (which is reserved on standard MSHC controllers) for + * optionally bypassing the HOLD register for command and data. The + * HOLD register should be bypassed in case there is no phase shift + * applied on CMD/DATA that is sent to the card. + */ + if (slot->host->drv_data->ctrl_type == DW_MCI_TYPE_EXYNOS5250) + if (SDMMC_CLKSEL_GET_SELCLK_DRV(mci_readl(slot->host, CLKSEL))) + cmdr |= SDMMC_CMD_USE_HOLD_REG; + return cmdr; } @@ -802,10 +814,19 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) regs = mci_readl(slot->host, UHS_REG); /* DDR mode set */ - if (ios->timing == MMC_TIMING_UHS_DDR50) + if (ios->timing == MMC_TIMING_UHS_DDR50) { regs |= (0x1 << slot->id) << 16; - else + mci_writel(slot->host, CLKSEL, slot->host->ddr_timing); + } else { regs &= ~(0x1 << slot->id) << 16; + mci_writel(slot->host, CLKSEL, slot->host->sdr_timing); + } + + if (slot->host->drv_data->ctrl_type == DW_MCI_TYPE_EXYNOS5250) { + slot->host->bus_hz = clk_get_rate(slot->host->ciu_clk); + slot->host->bus_hz /= SDMMC_CLKSEL_GET_DIVRATIO( + mci_readl(slot->host, CLKSEL)); + } mci_writel(slot->host, UHS_REG, regs); @@ -2086,6 +2107,7 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host) struct dw_mci_board *pdata; struct device *dev = host->dev; struct device_node *np = dev->of_node; + u32 timing[3]; int idx, cnt; pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); @@ -2108,6 +2130,20 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host) if (of_get_property(np, of_quriks[idx].quirk, NULL)) pdata->quirks |= of_quriks[idx].id; + if (of_property_read_u32_array(dev->of_node, + "samsung,dw-mshc-sdr-timing", timing, 3)) + host->sdr_timing = DW_MCI_DEF_SDR_TIMING; + else + host->sdr_timing = SDMMC_CLKSEL_TIMING(timing[0], + timing[1], timing[2]); + + if (of_property_read_u32_array(dev->of_node, + "samsung,dw-mshc-ddr-timing", timing, 3)) + host->ddr_timing = DW_MCI_DEF_DDR_TIMING; + else + host->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], + timing[1], timing[2]); + if (of_property_read_u32(np, "fifo-depth", &pdata->fifo_depth)) dev_info(dev, "fifo-depth property not found, using " "value of FIFOTH register as default\n"); diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h index 1ecaa02..6c17282 100644 --- a/drivers/mmc/host/dw_mmc.h +++ b/drivers/mmc/host/dw_mmc.h @@ -53,6 +53,7 @@ #define SDMMC_IDINTEN 0x090 #define SDMMC_DSCADDR 0x094 #define SDMMC_BUFADDR 0x098 +#define SDMMC_CLKSEL 0x09C /* specific to Samsung Exynos5250 */ #define SDMMC_DATA(x) (x) /* @@ -111,6 +112,7 @@ #define SDMMC_INT_ERROR 0xbfc2 /* Command register defines */ #define SDMMC_CMD_START BIT(31) +#define SDMMC_CMD_USE_HOLD_REG BIT(29) #define SDMMC_CMD_CCS_EXP BIT(23) #define SDMMC_CMD_CEATA_RD BIT(22) #define SDMMC_CMD_UPD_CLK BIT(21) @@ -142,6 +144,17 @@ /* Version ID register define */ #define SDMMC_GET_VERID(x) ((x) & 0xFFFF) +#define DW_MCI_DEF_SDR_TIMING 0x03030002 +#define DW_MCI_DEF_DDR_TIMING 0x03020001 +#define SDMMC_CLKSEL_CCLK_SAMPLE(x) (((x) & 3) << 0) +#define SDMMC_CLKSEL_CCLK_DRIVE(x) (((x) & 3) << 16) +#define SDMMC_CLKSEL_CCLK_DIVIDER(x) (((x) & 3) << 24) +#define SDMMC_CLKSEL_TIMING(x, y, z) (SDMMC_CLKSEL_CCLK_SAMPLE(x) | \ + SDMMC_CLKSEL_CCLK_DRIVE(y) | \ + SDMMC_CLKSEL_CCLK_DIVIDER(z)) +#define SDMMC_CLKSEL_GET_DIVRATIO(x) ((((x) >> 24) & 0x7) + 1) +#define SDMMC_CLKSEL_GET_SELCLK_DRV(x) (((x) >> 16) & 0x7) + /* Register access macros */ #define mci_readl(dev, reg) \ __raw_readl((dev)->regs + SDMMC_##reg) @@ -184,6 +197,7 @@ extern int dw_mci_resume(struct dw_mci *host); /* Variations in the dw_mci controller */ #define DW_MCI_TYPE_SYNOPSIS 0 +#define DW_MCI_TYPE_EXYNOS5250 1 /* Samsung Exynos5250 Extensions */ /* dw_mci platform driver data */ struct dw_mci_drv_data { diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h index ae45e4f..32c778f 100644 --- a/include/linux/mmc/dw_mmc.h +++ b/include/linux/mmc/dw_mmc.h @@ -82,6 +82,8 @@ struct mmc_data; * @biu_clk: Pointer to bus interface unit clock instance. * @ciu_clk: Pointer to card interface unit clock instance. * @slot: Slots sharing this MMC controller. + * @sdr_timing: Clock phase shifting for driving and sampling in sdr mode + * @ddr_timing: Clock phase shifting for driving and sampling in ddr mode * @fifo_depth: depth of FIFO. * @data_shift: log2 of FIFO item size. * @part_buf_start: Start index in part_buf. @@ -166,6 +168,10 @@ struct dw_mci { struct clk *ciu_clk; struct dw_mci_slot *slot[MAX_MCI_SLOTS]; + /* Phase Shift Value (for exynos5250 variant) */ + u32 sdr_timing; + u32 ddr_timing; + /* FIFO push and pull */ int fifo_depth; int data_shift;