| Message ID | 1507604480-23246-2-git-send-email-chaotian.jing@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
[...] > + > +static const struct of_device_id msdc_of_ids[] = { > + { .compatible = "mediatek,mt8135-mmc", .data = &mt8135_compat}, > + { .compatible = "mediatek,mt8173-mmc", .data = &mt8173_compat}, > + { .compatible = "mediatek,mt2701-mmc", .data = &mt2701_compat}, > + { .compatible = "mediatek,mt2712-mmc", .data = &mt2712_compat}, > + {} > +}; > +MODULE_DEVICE_TABLE(of, msdc_of_ids); As already stated in the other reply. These new compatible changes needs to be discussed and acked before the driver starts using them. In other words, make patch3 to precede this one. [...] Kind regards Uffe
On Tue, 2017-10-10 at 09:26 +0200, Ulf Hansson wrote: > [...] > > > + > > +static const struct of_device_id msdc_of_ids[] = { > > + { .compatible = "mediatek,mt8135-mmc", .data = &mt8135_compat}, > > + { .compatible = "mediatek,mt8173-mmc", .data = &mt8173_compat}, > > + { .compatible = "mediatek,mt2701-mmc", .data = &mt2701_compat}, > > + { .compatible = "mediatek,mt2712-mmc", .data = &mt2712_compat}, > > + {} > > +}; > > +MODULE_DEVICE_TABLE(of, msdc_of_ids); > > As already stated in the other reply. These new compatible changes > needs to be discussed and acked before the driver starts using them. > > In other words, make patch3 to precede this one. but, then there will have a probe issue as mentioned in previous mail list. that's why I separate the binding file changes to 2 patches. > > [...] > > Kind regards > Uffe
On 10 October 2017 at 09:35, Chaotian Jing <chaotian.jing@mediatek.com> wrote: > On Tue, 2017-10-10 at 09:26 +0200, Ulf Hansson wrote: >> [...] >> >> > + >> > +static const struct of_device_id msdc_of_ids[] = { >> > + { .compatible = "mediatek,mt8135-mmc", .data = &mt8135_compat}, >> > + { .compatible = "mediatek,mt8173-mmc", .data = &mt8173_compat}, >> > + { .compatible = "mediatek,mt2701-mmc", .data = &mt2701_compat}, >> > + { .compatible = "mediatek,mt2712-mmc", .data = &mt2712_compat}, >> > + {} >> > +}; >> > +MODULE_DEVICE_TABLE(of, msdc_of_ids); >> >> As already stated in the other reply. These new compatible changes >> needs to be discussed and acked before the driver starts using them. >> >> In other words, make patch3 to precede this one. > but, then there will have a probe issue as mentioned in previous mail > list. that's why I separate the binding file changes to 2 patches. That sounds seriously wrong. Aren't the bindings backwards compatible? Kind regards Uffe
On Tue, 2017-10-10 at 10:09 +0200, Ulf Hansson wrote: > On 10 October 2017 at 09:35, Chaotian Jing <chaotian.jing@mediatek.com> wrote: > > On Tue, 2017-10-10 at 09:26 +0200, Ulf Hansson wrote: > >> [...] > >> > >> > + > >> > +static const struct of_device_id msdc_of_ids[] = { > >> > + { .compatible = "mediatek,mt8135-mmc", .data = &mt8135_compat}, > >> > + { .compatible = "mediatek,mt8173-mmc", .data = &mt8173_compat}, > >> > + { .compatible = "mediatek,mt2701-mmc", .data = &mt2701_compat}, > >> > + { .compatible = "mediatek,mt2712-mmc", .data = &mt2712_compat}, > >> > + {} > >> > +}; > >> > +MODULE_DEVICE_TABLE(of, msdc_of_ids); > >> > >> As already stated in the other reply. These new compatible changes > >> needs to be discussed and acked before the driver starts using them. > >> > >> In other words, make patch3 to precede this one. > > but, then there will have a probe issue as mentioned in previous mail > > list. that's why I separate the binding file changes to 2 patches. > > That sounds seriously wrong. Aren't the bindings backwards compatible? > > Kind regards > Uffe As Matthias mentioned before: " >> NAK, this has to be: >> You have to add the fallback compatible ("mediatek,mt8135-mmc") to the binding >> description as otherwise the driver does not get probed. >> >> Regards, >> Matthias " the original compatible in driver is "mediatek,mt8135-mmc", so that if drop "mediatek,mt8135-mmc" in bindings file(User may refer it and drop "mediatek,mt8135-mmc" in their DTS), the driver will not get probed.
On 10/10/2017 10:22 AM, Chaotian Jing wrote: > On Tue, 2017-10-10 at 10:09 +0200, Ulf Hansson wrote: >> On 10 October 2017 at 09:35, Chaotian Jing <chaotian.jing@mediatek.com> wrote: >>> On Tue, 2017-10-10 at 09:26 +0200, Ulf Hansson wrote: >>>> [...] >>>> >>>>> + >>>>> +static const struct of_device_id msdc_of_ids[] = { >>>>> + { .compatible = "mediatek,mt8135-mmc", .data = &mt8135_compat}, >>>>> + { .compatible = "mediatek,mt8173-mmc", .data = &mt8173_compat}, >>>>> + { .compatible = "mediatek,mt2701-mmc", .data = &mt2701_compat}, >>>>> + { .compatible = "mediatek,mt2712-mmc", .data = &mt2712_compat}, >>>>> + {} >>>>> +}; >>>>> +MODULE_DEVICE_TABLE(of, msdc_of_ids); >>>> >>>> As already stated in the other reply. These new compatible changes >>>> needs to be discussed and acked before the driver starts using them. >>>> >>>> In other words, make patch3 to precede this one. >>> but, then there will have a probe issue as mentioned in previous mail >>> list. that's why I separate the binding file changes to 2 patches. >> >> That sounds seriously wrong. Aren't the bindings backwards compatible? >> >> Kind regards >> Uffe > > As Matthias mentioned before: > " >>> NAK, this has to be: >>> You have to add the fallback compatible ("mediatek,mt8135-mmc") to > the binding >>> description as otherwise the driver does not get probed. >>> >>> Regards, >>> Matthias > " > the original compatible in driver is "mediatek,mt8135-mmc", so that if > drop "mediatek,mt8135-mmc" in bindings file(User may refer it and drop > "mediatek,mt8135-mmc" in their DTS), the driver will not get probed. > If you change the driver as you did in this patch, then there is no probing problem. Regards, Matthias
On Tue, 2017-10-10 at 10:32 +0200, Matthias Brugger wrote: > > On 10/10/2017 10:22 AM, Chaotian Jing wrote: > > On Tue, 2017-10-10 at 10:09 +0200, Ulf Hansson wrote: > >> On 10 October 2017 at 09:35, Chaotian Jing <chaotian.jing@mediatek.com> wrote: > >>> On Tue, 2017-10-10 at 09:26 +0200, Ulf Hansson wrote: > >>>> [...] > >>>> > >>>>> + > >>>>> +static const struct of_device_id msdc_of_ids[] = { > >>>>> + { .compatible = "mediatek,mt8135-mmc", .data = &mt8135_compat}, > >>>>> + { .compatible = "mediatek,mt8173-mmc", .data = &mt8173_compat}, > >>>>> + { .compatible = "mediatek,mt2701-mmc", .data = &mt2701_compat}, > >>>>> + { .compatible = "mediatek,mt2712-mmc", .data = &mt2712_compat}, > >>>>> + {} > >>>>> +}; > >>>>> +MODULE_DEVICE_TABLE(of, msdc_of_ids); > >>>> > >>>> As already stated in the other reply. These new compatible changes > >>>> needs to be discussed and acked before the driver starts using them. > >>>> > >>>> In other words, make patch3 to precede this one. > >>> but, then there will have a probe issue as mentioned in previous mail > >>> list. that's why I separate the binding file changes to 2 patches. > >> > >> That sounds seriously wrong. Aren't the bindings backwards compatible? > >> > >> Kind regards > >> Uffe > > > > As Matthias mentioned before: > > " > >>> NAK, this has to be: > >>> You have to add the fallback compatible ("mediatek,mt8135-mmc") to > > the binding > >>> description as otherwise the driver does not get probed. > >>> > >>> Regards, > >>> Matthias > > " > > the original compatible in driver is "mediatek,mt8135-mmc", so that if > > drop "mediatek,mt8135-mmc" in bindings file(User may refer it and drop > > "mediatek,mt8135-mmc" in their DTS), the driver will not get probed. > > > > If you change the driver as you did in this patch, then there is no probing problem. > > Regards, > Matthias Okay, will make patch3 to precede this one at next version.
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c index 267f7ab..643c795 100644 --- a/drivers/mmc/host/mtk-sd.c +++ b/drivers/mmc/host/mtk-sd.c @@ -95,6 +95,9 @@ #define MSDC_CFG_CKDIV (0xff << 8) /* RW */ #define MSDC_CFG_CKMOD (0x3 << 16) /* RW */ #define MSDC_CFG_HS400_CK_MODE (0x1 << 18) /* RW */ +#define MSDC_CFG_HS400_CK_MODE_EXTRA (0x1 << 22) /* RW */ +#define MSDC_CFG_CKDIV_EXTRA (0xfff << 8) /* RW */ +#define MSDC_CFG_CKMOD_EXTRA (0x3 << 20) /* RW */ /* MSDC_IOCON mask */ #define MSDC_IOCON_SDR104CKS (0x1 << 0) /* RW */ @@ -295,6 +298,10 @@ struct msdc_save_para { u32 emmc50_cfg0; }; +struct mtk_mmc_compatible { + u8 clk_div_bits; +}; + struct msdc_tune_para { u32 iocon; u32 pad_tune; @@ -309,6 +316,7 @@ struct msdc_delay_phase { struct msdc_host { struct device *dev; + const struct mtk_mmc_compatible *dev_comp; struct mmc_host *mmc; /* mmc structure */ int cmd_rsp; @@ -350,6 +358,31 @@ struct msdc_host { struct msdc_tune_para saved_tune_para; /* tune result of CMD21/CMD19 */ }; +static const struct mtk_mmc_compatible mt8135_compat = { + .clk_div_bits = 8, +}; + +static const struct mtk_mmc_compatible mt8173_compat = { + .clk_div_bits = 8, +}; + +static const struct mtk_mmc_compatible mt2701_compat = { + .clk_div_bits = 12, +}; + +static const struct mtk_mmc_compatible mt2712_compat = { + .clk_div_bits = 12, +}; + +static const struct of_device_id msdc_of_ids[] = { + { .compatible = "mediatek,mt8135-mmc", .data = &mt8135_compat}, + { .compatible = "mediatek,mt8173-mmc", .data = &mt8173_compat}, + { .compatible = "mediatek,mt2701-mmc", .data = &mt2701_compat}, + { .compatible = "mediatek,mt2712-mmc", .data = &mt2712_compat}, + {} +}; +MODULE_DEVICE_TABLE(of, msdc_of_ids); + static void sdr_set_bits(void __iomem *reg, u32 bs) { u32 val = readl(reg); @@ -509,7 +542,12 @@ static void msdc_set_timeout(struct msdc_host *host, u32 ns, u32 clks) timeout = (ns + clk_ns - 1) / clk_ns + clks; /* in 1048576 sclk cycle unit */ timeout = (timeout + (0x1 << 20) - 1) >> 20; - sdr_get_field(host->base + MSDC_CFG, MSDC_CFG_CKMOD, &mode); + if (host->dev_comp->clk_div_bits == 8) + sdr_get_field(host->base + MSDC_CFG, + MSDC_CFG_CKMOD, &mode); + else + sdr_get_field(host->base + MSDC_CFG, + MSDC_CFG_CKMOD_EXTRA, &mode); /*DDR mode will double the clk cycles for data timeout */ timeout = mode >= 2 ? timeout * 2 : timeout; timeout = timeout > 1 ? timeout - 1 : 0; @@ -548,7 +586,11 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz) flags = readl(host->base + MSDC_INTEN); sdr_clr_bits(host->base + MSDC_INTEN, flags); - sdr_clr_bits(host->base + MSDC_CFG, MSDC_CFG_HS400_CK_MODE); + if (host->dev_comp->clk_div_bits == 8) + sdr_clr_bits(host->base + MSDC_CFG, MSDC_CFG_HS400_CK_MODE); + else + sdr_clr_bits(host->base + MSDC_CFG, + MSDC_CFG_HS400_CK_MODE_EXTRA); if (timing == MMC_TIMING_UHS_DDR50 || timing == MMC_TIMING_MMC_DDR52 || timing == MMC_TIMING_MMC_HS400) { @@ -568,8 +610,12 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz) if (timing == MMC_TIMING_MMC_HS400 && hz >= (host->src_clk_freq >> 1)) { - sdr_set_bits(host->base + MSDC_CFG, - MSDC_CFG_HS400_CK_MODE); + if (host->dev_comp->clk_div_bits == 8) + sdr_set_bits(host->base + MSDC_CFG, + MSDC_CFG_HS400_CK_MODE); + else + sdr_set_bits(host->base + MSDC_CFG, + MSDC_CFG_HS400_CK_MODE_EXTRA); sclk = host->src_clk_freq >> 1; div = 0; /* div is ignore when bit18 is set */ } @@ -587,8 +633,15 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz) sclk = (host->src_clk_freq >> 2) / div; } } - sdr_set_field(host->base + MSDC_CFG, MSDC_CFG_CKMOD | MSDC_CFG_CKDIV, - (mode << 8) | div); + if (host->dev_comp->clk_div_bits == 8) + sdr_set_field(host->base + MSDC_CFG, + MSDC_CFG_CKMOD | MSDC_CFG_CKDIV, + (mode << 8) | div); + else + sdr_set_field(host->base + MSDC_CFG, + MSDC_CFG_CKMOD_EXTRA | MSDC_CFG_CKDIV_EXTRA, + (mode << 12) | div); + sdr_set_bits(host->base + MSDC_CFG, MSDC_CFG_CKPDN); while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB)) cpu_relax(); @@ -1617,12 +1670,17 @@ static int msdc_drv_probe(struct platform_device *pdev) struct mmc_host *mmc; struct msdc_host *host; struct resource *res; + const struct of_device_id *of_id; int ret; if (!pdev->dev.of_node) { dev_err(&pdev->dev, "No DT found\n"); return -EINVAL; } + + of_id = of_match_node(msdc_of_ids, pdev->dev.of_node); + if (!of_id) + return -EINVAL; /* Allocate MMC host for this device */ mmc = mmc_alloc_host(sizeof(struct msdc_host), &pdev->dev); if (!mmc) @@ -1686,11 +1744,15 @@ static int msdc_drv_probe(struct platform_device *pdev) msdc_of_property_parse(pdev, host); host->dev = &pdev->dev; + host->dev_comp = of_id->data; host->mmc = mmc; host->src_clk_freq = clk_get_rate(host->src_clk); /* Set host parameters to mmc */ mmc->ops = &mt_msdc_ops; - mmc->f_min = DIV_ROUND_UP(host->src_clk_freq, 4 * 255); + if (host->dev_comp->clk_div_bits == 8) + mmc->f_min = DIV_ROUND_UP(host->src_clk_freq, 4 * 255); + else + mmc->f_min = DIV_ROUND_UP(host->src_clk_freq, 4 * 4095); mmc->caps |= MMC_CAP_ERASE | MMC_CAP_CMD23; /* MMC core transfer sizes tunable parameters */ @@ -1839,12 +1901,6 @@ static int msdc_runtime_resume(struct device *dev) SET_RUNTIME_PM_OPS(msdc_runtime_suspend, msdc_runtime_resume, NULL) }; -static const struct of_device_id msdc_of_ids[] = { - { .compatible = "mediatek,mt8135-mmc", }, - {} -}; -MODULE_DEVICE_TABLE(of, msdc_of_ids); - static struct platform_driver mt_msdc_driver = { .probe = msdc_drv_probe, .remove = msdc_drv_remove,
mt2701/mt2712 has 12bit clock div, which is not compatible with mt8135/mt8173. and, some additional features will be added in mt2701/mt2712, so that need distinguish it by comatibale name. Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com> --- drivers/mmc/host/mtk-sd.c | 82 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 69 insertions(+), 13 deletions(-)