Hi Kever, On 6 June 2017 at 21:23, Kever Yang <kever.y...@rock-chips.com> wrote: > Simon, > > > > On 06/07/2017 05:10 AM, Simon Glass wrote: >> >> Hi Pawel, >> >> On 6 June 2017 at 12:51, Paweł Jarosz <paweljarosz3...@gmail.com> wrote: >>> >>> rk3066 and rk3288 mmc designware ip's are very similiar. They differ in >>> internal dma support and max driver frequency. >>> >>> Signed-off-by: Paweł Jarosz <paweljarosz3...@gmail.com> >>> --- >>> drivers/mmc/rockchip_dw_mmc.c | 31 +++++++++++++++++++++++++++++-- >>> 1 file changed, 29 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/mmc/rockchip_dw_mmc.c >>> b/drivers/mmc/rockchip_dw_mmc.c >>> index 25a21e2..d94c395 100644 >>> --- a/drivers/mmc/rockchip_dw_mmc.c >>> +++ b/drivers/mmc/rockchip_dw_mmc.c >>> @@ -22,8 +22,14 @@ DECLARE_GLOBAL_DATA_PTR; >>> >>> struct rockchip_mmc_plat { >>> #if CONFIG_IS_ENABLED(OF_PLATDATA) >>> + >>> +#ifdef CONFIG_ROCKCHIP_RK3066 >>> + struct dtd_rockchip_rk2928_dw_mshc dtplat; >>> +#else >>> struct dtd_rockchip_rk3288_dw_mshc dtplat; >>> #endif >>> + >>> +#endif >>> struct mmc_config cfg; >>> struct mmc mmc; >>> }; >>> @@ -109,8 +115,11 @@ static int rockchip_dwmmc_probe(struct udevice *dev) >>> int ret; >>> >>> #if CONFIG_IS_ENABLED(OF_PLATDATA) >>> +#ifdef CONFIG_ROCKCHIP_RK3066 >>> + struct dtd_rockchip_rk2928_dw_mshc *dtplat = &plat->dtplat; >>> +#else >>> struct dtd_rockchip_rk3288_dw_mshc *dtplat = &plat->dtplat; >>> - >>> +#endif >> >> I am not keen on this - it will get ugly. Can you please do this: >> >> Create a new driver for rk3288 which just has the platdata stuff, >> rockchip_dwmmc_ofdata_to_platdata() and the U_BOOT_DRIVER(). >> Everything else should remain in this file. >> >> Then in a new patch, create a driver for rk3066 which uses the same >> common elements from rockchip_dw_mmc.c > > > I think I have discuss this with you online or offline for many times, > when OF_PLATADATA enabled, the dts is pre-compile by dtoc and then > there is structure like 'dtd_rockchip_rk2928_dw_mshc' which is from the dts > node compatible name, I propose to use the last compatible name in dtoc > instead of the first one, then we can get the same structure name and used > in drivers. > > I still not enable the dwmmc when OF_PLATDATA enabled on rk3399 because > of the same reason. > > I think we should fix this from the root cause, but not make different > driver > for different SoCs.
OK, well if you think that will work it is fine with me. I will take a look at the change. Regards, Simon > > Thanks, > - Kever >> >> >>> host->name = dev->name; >>> host->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]); >>> host->buswidth = dtplat->bus_width; >>> @@ -118,7 +127,12 @@ static int rockchip_dwmmc_probe(struct udevice *dev) >>> host->priv = dev; >>> host->dev_index = 0; >>> priv->fifo_depth = dtplat->fifo_depth; >>> + >>> +#ifdef CONFIG_ROCKCHIP_RK3066 >>> + priv->fifo_mode = 1; >>> +#else >>> priv->fifo_mode = 0; >>> +#endif >> >> What about the fifo_mode property in the DT? >> >> If you have to hard-code this you should use a .data parameter in >> rockchip_dwmmc_ids (e.g. RK2928, RK3288) and use that to determine the >> mode. But hopefully the DT is enough. >> >> For OF_PLATDATA however I suggest you have a different probe() which >> either sets up this value and then calls rockchip_dwmmc_probe(), or >> add it as a parameter to rockchip_dwmmc_probe(). >> >>> memcpy(priv->minmax, dtplat->clock_freq_min_max, >>> sizeof(priv->minmax)); >>> >>> ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, >>> &priv->clk); >>> @@ -162,14 +176,27 @@ static int rockchip_dwmmc_bind(struct udevice *dev) >>> } >>> >>> static const struct udevice_id rockchip_dwmmc_ids[] = { >>> + { .compatible = "rockchip,rk2928-dw-mshc" }, >>> { .compatible = "rockchip,rk3288-dw-mshc" }, >>> { } >>> }; >>> >>> +U_BOOT_DRIVER(rockchip_rk2928_dw_mshc) = { >>> + .name = "rockchip_rk2928_dw_mshc", >>> + .id = UCLASS_MMC, >>> + .of_match = rockchip_dwmmc_ids, >>> + .ofdata_to_platdata = rockchip_dwmmc_ofdata_to_platdata, >>> + .ops = &dm_dwmci_ops, >>> + .bind = rockchip_dwmmc_bind, >>> + .probe = rockchip_dwmmc_probe, >>> + .priv_auto_alloc_size = sizeof(struct rockchip_dwmmc_priv), >>> + .platdata_auto_alloc_size = sizeof(struct rockchip_mmc_plat), >>> +}; >>> + >>> U_BOOT_DRIVER(rockchip_dwmmc_drv) = { >>> .name = "rockchip_rk3288_dw_mshc", >>> .id = UCLASS_MMC, >>> - .of_match = rockchip_dwmmc_ids, >> >> I think you ne >>> >>> .ofdata_to_platdata = rockchip_dwmmc_ofdata_to_platdata, >>> .ops = &dm_dwmci_ops, >>> .bind = rockchip_dwmmc_bind, >>> -- >>> 2.7.4 >>> >> Regards, >> Simon >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> https://lists.denx.de/listinfo/u-boot > > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot