On Wed, 2014-04-16 at 14:40 +0200, Antoine Ténart wrote: > Add a Driver to support the SDHCI controller of the Marvell Berlin SoCs. > This controller supports 3 sockets. trivial notes: > diff --git a/drivers/mmc/host/sdhci-berlin.c b/drivers/mmc/host/sdhci-berlin.c [] > +static struct sdhci_ops sdhci_berlin_ops = { > + .get_max_clock = sdhci_pltfm_clk_get_max_clock, > +}; > + > +static struct sdhci_pltfm_data sdhci_berlin2_pdata = { > + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN | > + SDHCI_QUIRK_BROKEN_ADMA | > + SDHCI_QUIRK_BROKEN_TIMEOUT_VAL, > + .ops = &sdhci_berlin_ops, > +}; > + > +static struct sdhci_pltfm_data sdhci_berlin2q_pdata = { > + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN | > + SDHCI_QUIRK_INVERTED_WRITE_PROTECT | > + SDHCI_QUIRK_BROKEN_TIMEOUT_VAL, > + .ops = &sdhci_berlin_ops, > +}; Perhaps const? > +static int sdhci_berlin_probe(struct platform_device *pdev) > +{ [] > + host = sdhci_pltfm_init(pdev, > + (struct sdhci_pltfm_data *)device->data, 0); Unnecessary cast? Maybe: host = sdhci_pltfm_init(pdev, device->data, 0); > + if (IS_ERR(host)) > + return PTR_ERR(host); > + > + pltfm_host = sdhci_priv(host); > + > + clk = devm_clk_get(dev, NULL); > + if (IS_ERR(clk)) { > + dev_err(dev, "could not get clock: %ld\n", PTR_ERR(clk)); > + ret = PTR_ERR(clk); PTR_ERR is an int. Does this produce a compile warning? Maybe reverse these lines and use ret? ret = PTR_ERR(clk); dev_err(dev, count not get clock: %d\n", ret); -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html