Hi Piotr, 2017-03-06 22:39 GMT+09:00 Piotr Sroka <piotrs@xxxxxxxxxxx>: > PHY settings can be different for different platforms and SoCs. > Fixed PHY input delays was replaced with SoC specific compatible data. > DTS properties are used for configuration new PHY DLL delays. Probably you are familiar with this IP. Please teach me this. With this patch, we will have two groups for PHY parameters. (A) specified via a data array associated with a compatible string SDHCI_CDNS_PHY_DLY_SD_HS SDHCI_CDNS_PHY_DLY_SD_DEFAULT SDHCI_CDNS_PHY_DLY_UHS_SDR12 SDHCI_CDNS_PHY_DLY_UHS_SDR25 SDHCI_CDNS_PHY_DLY_UHS_SDR50 SDHCI_CDNS_PHY_DLY_UHS_DDR50 SDHCI_CDNS_PHY_DLY_EMMC_LEGACY SDHCI_CDNS_PHY_DLY_EMMC_SDR SDHCI_CDNS_PHY_DLY_EMMC_DDR (B) specified with DT property SDHCI_CDNS_PHY_DLY_SDCLK SDHCI_CDNS_PHY_DLY_HSMMC SDHCI_CDNS_PHY_DLY_STROBE I am confused. What is the difference between (A) and (B)? A little more minor issues below. > > /* > * The tuned val register is 6 bit-wide, but not the whole of the range is > @@ -62,10 +66,24 @@ > */ > #define SDHCI_CDNS_MAX_TUNING_LOOP 40 > > +static const struct of_device_id sdhci_cdns_match[]; > + You need not add this declaration if you use of_device_get_match_data(). (please see below) > @@ -90,13 +108,77 @@ static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv, > return 0; > } > > -static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv) > +static int sdhci_cdns_phy_in_delay_init(struct sdhci_cdns_priv *priv, > + const struct sdhci_cdns_config *config) > +{ > + int ret = 0; The initializer "= 0" is unneeded here because the variable is re-assigned with a return value of functions. > @@ -254,7 +338,16 @@ static int sdhci_cdns_probe(struct platform_device *pdev) > if (ret) > goto free; > > - sdhci_cdns_phy_init(priv); > + match = of_match_node(sdhci_cdns_match, dev->of_node); > + if (match && match->data) { > + ret = sdhci_cdns_phy_in_delay_init(priv, match->data); > + if (ret) > + goto free; > + } You can simplify the code with of_device_get_match_data(): config = of_device_get_match_data(dev); if (config) { ret = sdhci_cdns_phy_in_delay_init(priv, config); if (ret) goto free; } > static const struct of_device_id sdhci_cdns_match[] = { > - { .compatible = "socionext,uniphier-sd4hc" }, > + { > + .compatible = "socionext,uniphier-sd4hc", > + .data = &sdhci_cdns_uniphier_config > + }, > { .compatible = "cdns,sd4hc" }, I prefer cdns,sd4hc indented in the same way: { .compatible = "cdns,sd4hc", }, -- Best Regards Masahiro Yamada -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html