[U-Boot] [PATCH 08/13] rockchip: mmc: support rk3066 mmc

[Simon Glass]

Hi Pawel,

On 6 June 2017 at 12:51, Paweł Jarosz <paweljarosz3691 at 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 <paweljarosz3691 at 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

>         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