[PATCH v5 2/3] wl18xx: add basic device-tree support

[Javier Martinez Canillas]

Hello Eliad,

On Mon, Mar 9, 2015 at 4:36 PM, Eliad Peller <eliad at wizery.com> wrote:
> When running with device-tree, we no longer have a board file
> that can set up the platform data for wlcore.
> Allow this data to be passed from DT.
>
> For now, parse only the irq used. Other (optional) properties
> can be added later on.
>
> Signed-off-by: Ido Yariv <ido at wizery.com>
> Signed-off-by: Eliad Peller <eliad at wizery.com>
> ---

I see this is a v5 but I don't know what was changed from prior
revisions. It would be good if the patches had a versions history.

>  drivers/net/wireless/ti/wlcore/sdio.c | 80 ++++++++++++++++++++++++++++++++---
>  1 file changed, 74 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
> index d3dd7bf..ee556ac 100644
> --- a/drivers/net/wireless/ti/wlcore/sdio.c
> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
> @@ -34,6 +34,8 @@
>  #include <linux/wl12xx.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/printk.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
>
>  #include "wlcore.h"
>  #include "wl12xx_80211.h"
> @@ -214,6 +216,69 @@ static struct wl1271_if_operations sdio_ops = {
>         .set_block_size = wl1271_sdio_set_block_size,
>  };
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id wlcore_sdio_of_match_table[] = {
> +       { .compatible = "ti,wl1801" },
> +       { .compatible = "ti,wl1805" },
> +       { .compatible = "ti,wl1807" },
> +       { .compatible = "ti,wl1831" },
> +       { .compatible = "ti,wl1835" },
> +       { .compatible = "ti,wl1837" },
> +       { }
> +};
> +
> +static struct wl12xx_platform_data *wlcore_probe_of(struct device *dev)
> +{
> +       struct device_node *np = dev->of_node;
> +       struct wl12xx_platform_data *pdata;
> +
> +       if (!np || !of_match_node(wlcore_sdio_of_match_table, np))
> +               return NULL;
> +
> +       pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> +       if (!pdata)
> +               return NULL;
> +
> +       pdata->irq = irq_of_parse_and_map(np, 0);
> +       if (!pdata->irq) {
> +               dev_err(dev, "No irq in platform data\n");
> +               kfree(pdata);
> +               return NULL;
> +       }
> +
> +       return pdata;
> +}
> +#else
> +static struct wl12xx_platform_data *wlcore_probe_of(struct device *dev)
> +{
> +       return NULL;
> +}
> +#endif
> +
> +static struct wl12xx_platform_data *
> +wlcore_get_platform_data(struct device *dev)
> +{
> +       struct wl12xx_platform_data *pdata;
> +
> +       /* first, look for DT data */

I thought it was the opposite, that platform data should over-rule DT.
That way you can still use the data filled in
arch/arm/mach-omap2/pdata-quirks.c even after the driver supports your
new DT binding.

> +       pdata = wlcore_probe_of(dev);
> +       if (pdata)
> +               return pdata;
> +
> +       /* if not found - fallback to static platform data */
> +       pdata = wl12xx_get_platform_data();
> +       if (!IS_ERR(pdata))
> +               return kmemdup(pdata, sizeof(*pdata), GFP_KERNEL);
> +
> +       dev_err(dev, "No platform data set\n");
> +       return NULL;
> +}
> +
> +static void wlcore_del_platform_data(struct wl12xx_platform_data *pdata)
> +{
> +       kfree(pdata);
> +}
> +

This function seems to be an unnecessary, why not just call kfree() directly?

Or better, maybe the resource-managed devm_*() functions can be used
so the data doesn't have to be explicitly freed?

Best regards,
Javier