On Monday 28 September 2015 14:35:13 Yangbo Lu wrote:
> +struct esdhc_host {
> + u8 vendor_ver;
> + u8 spec_ver;
> + u16 xfer_mode_shadow;
> + struct sdhci_host *host;
> +};
> +
> +static struct esdhc_host *esdhc;
Please avoid global variables like this one and keep the state
within the per-device structure.
>
> -static const struct sdhci_pltfm_data sdhci_esdhc_pdata = {
> - /*
> - * card detection could be handled via GPIO
> - * eSDHC cannot support End Attribute in NOP ADMA descriptor
> - */
> - .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_CARD_DETECTION
> - | SDHCI_QUIRK_NO_CARD_NO_RESET
> - | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
> - .ops = &sdhci_esdhc_ops,
> +static struct sdhci_ops sdhci_esdhc_ops = {
> + .set_clock = esdhc_of_set_clock,
> + .enable_dma = esdhc_of_enable_dma,
> + .get_max_clock = esdhc_of_get_max_clock,
> + .get_min_clock = esdhc_of_get_min_clock,
> + .adma_workaround = esdhc_of_adma_workaround,
> + .set_bus_width = esdhc_pltfm_set_bus_width,
> + .reset = esdhc_reset,
> + .set_uhs_signaling = sdhci_set_uhs_signaling,
> };
> +static struct sdhci_pltfm_data sdhci_esdhc_pdata;
> +
> +static void esdhc_setup_pltfm_data(struct platform_device *pdev)
> +{
> + struct device_node *np;
> +
> + np = pdev->dev.of_node;
> + if (of_get_property(np, "little-endian", NULL)) {
> + sdhci_esdhc_ops.read_l = esdhc_le_readl;
> + sdhci_esdhc_ops.read_w = esdhc_le_readw;
> + sdhci_esdhc_ops.read_b = esdhc_le_readb;
> + sdhci_esdhc_ops.write_l = esdhc_le_writel;
> + sdhci_esdhc_ops.write_w = esdhc_le_writew;
> + sdhci_esdhc_ops.write_b = esdhc_le_writeb;
> + } else {
> + sdhci_esdhc_ops.read_l = esdhc_be_readl;
> + sdhci_esdhc_ops.read_w = esdhc_be_readw;
> + sdhci_esdhc_ops.read_b = esdhc_be_readb;
> + sdhci_esdhc_ops.write_l = esdhc_be_writel;
> + sdhci_esdhc_ops.write_w = esdhc_be_writew;
> + sdhci_esdhc_ops.write_b = esdhc_be_writeb;
> + }
Just define two instances of the sdhci_esdhc_ops structure and
pick one or the other pointer, rather changing the members individually.
That makes it easier to read and gives smaller code. Similarly, use
two instances of sdhci_pltfm_data.
Ideally also mark the structure as 'const', as we normally do for
structures with callback pointers. (this might require a larger
cleanup though).
> static int sdhci_esdhc_probe(struct platform_device *pdev)
> {
> @@ -360,10 +563,15 @@ static int sdhci_esdhc_probe(struct platform_device *pdev)
> struct device_node *np;
> int ret;
>
> + esdhc_setup_pltfm_data(pdev);
> +
> host = sdhci_pltfm_init(pdev, &sdhci_esdhc_pdata, 0);
> if (IS_ERR(host))
> return PTR_ERR(host);
>
> + esdhc = kmalloc(sizeof(struct esdhc_host), GFP_KERNEL);
> + esdhc_init(esdhc, host);
> +
You can combine these, by passing sizeof(struct esdhc_host) as the 'priv'
argument of the sdhci_pltfm_init function.
Arnd
--
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
