diff mbox

[v3,2/2] wl18xx: add basic device-tree support

Message ID 1424362401-8228-2-git-send-email-eliad@wizery.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show

Commit Message

Eliad Peller Feb. 19, 2015, 4:13 p.m. UTC 
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@wizery.com>
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/ti/wlcore/sdio.c | 70 ++++++++++++++++++++++++++++++++---
 1 file changed, 64 insertions(+), 6 deletions(-)

Comments

Arnd Bergmann Feb. 27, 2015, 8:26 a.m. UTC | #1 
On Thursday 19 February 2015 18:13:21 Eliad Peller wrote:
> +
> +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)) {
> +               dev_err(dev, "No platform data set\n");
> +               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;
> +
> +       pdata = wl12xx_get_platform_data();
> +       if (!IS_ERR(pdata))
> +               return kmemdup(pdata, sizeof(*pdata), GFP_KERNEL);
> +
> +       return wlcore_probe_of(dev);
> +}

I think the probe_of needs to come first here, otherwise you cannot
override the pdata quirk with actual DT data.

I've looked up the what boards actually use this data and found that
all of them already support booting from DT: some omap2 boards using
arch/arm/mach-omap2/pdata-quirks.c to provide the data, and the
davinci 850evm. Can you make sure you add the correct data to all
of these dts files as part of your series and remove the
wl12xx_platform_data references?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eliad Peller March 8, 2015, 11:13 a.m. UTC | #2 
On Fri, Feb 27, 2015 at 10:26 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 19 February 2015 18:13:21 Eliad Peller wrote:
>> +
>> +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)) {
>> +               dev_err(dev, "No platform data set\n");
>> +               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;
>> +
>> +       pdata = wl12xx_get_platform_data();
>> +       if (!IS_ERR(pdata))
>> +               return kmemdup(pdata, sizeof(*pdata), GFP_KERNEL);
>> +
>> +       return wlcore_probe_of(dev);
>> +}
>
> I think the probe_of needs to come first here, otherwise you cannot
> override the pdata quirk with actual DT data.
>
makes sense. i'll change it.

> I've looked up the what boards actually use this data and found that
> all of them already support booting from DT: some omap2 boards using
> arch/arm/mach-omap2/pdata-quirks.c to provide the data, and the
> davinci 850evm. Can you make sure you add the correct data to all
> of these dts files as part of your series and remove the
> wl12xx_platform_data references?

AFAICT, these board files add wl12xx platform data, while the new DT
support is only for wl18xx.

Eliad.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann March 8, 2015, 9:53 p.m. UTC | #3 
On Sunday 08 March 2015 13:13:13 Eliad Peller wrote:
> 
> > I've looked up the what boards actually use this data and found that
> > all of them already support booting from DT: some omap2 boards using
> > arch/arm/mach-omap2/pdata-quirks.c to provide the data, and the
> > davinci 850evm. Can you make sure you add the correct data to all
> > of these dts files as part of your series and remove the
> > wl12xx_platform_data references?
> 
> AFAICT, these board files add wl12xx platform data, while the new DT
> support is only for wl18xx.
> 

How can you tell the difference? What I see is that omap3pandora
(and nothing else) calls wl1251_set_platform_data(), while 
da850-evm and all omap3/omap4 boards use wl12xx_set_platform_data().

The latter seems to refer to all wl12xx and wl18xx variants except
for wl1251, based on my (very limited) understanding of that code.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eliad Peller March 9, 2015, 7:18 a.m. UTC | #4 
On Sun, Mar 8, 2015 at 11:53 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Sunday 08 March 2015 13:13:13 Eliad Peller wrote:
>>
>> > I've looked up the what boards actually use this data and found that
>> > all of them already support booting from DT: some omap2 boards using
>> > arch/arm/mach-omap2/pdata-quirks.c to provide the data, and the
>> > davinci 850evm. Can you make sure you add the correct data to all
>> > of these dts files as part of your series and remove the
>> > wl12xx_platform_data references?
>>
>> AFAICT, these board files add wl12xx platform data, while the new DT
>> support is only for wl18xx.
>>
>
> How can you tell the difference? What I see is that omap3pandora
> (and nothing else) calls wl1251_set_platform_data(), while
> da850-evm and all omap3/omap4 boards use wl12xx_set_platform_data().
>
> The latter seems to refer to all wl12xx and wl18xx variants except
> for wl1251, based on my (very limited) understanding of that code.

right.
i got mislead because legacy_init_wl12xx() is defined there only for
CONFIG_WL12XX (and not for CONFIG_WL18XX).
it looks like only "isee,omap3-igep0020-rev-f" and
"isee,omap3-igep0020-rev-g" have pdata quirks for wl18xx (and thus
initialize the pdata clocks to 0).

sorry for the trivial question, but what's the standard way to submit
such patch? should i simply add a third patch to the patchset which
removes the pdata quirk and adds the missing dts definition? i don't
have such board, so i can only compile-test it.

thanks,
Eliad.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann March 9, 2015, 8:50 a.m. UTC | #5 
On Monday 09 March 2015 09:18:46 Eliad Peller wrote:
> On Sun, Mar 8, 2015 at 11:53 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Sunday 08 March 2015 13:13:13 Eliad Peller wrote:
> >>
> >> > I've looked up the what boards actually use this data and found that
> >> > all of them already support booting from DT: some omap2 boards using
> >> > arch/arm/mach-omap2/pdata-quirks.c to provide the data, and the
> >> > davinci 850evm. Can you make sure you add the correct data to all
> >> > of these dts files as part of your series and remove the
> >> > wl12xx_platform_data references?
> >>
> >> AFAICT, these board files add wl12xx platform data, while the new DT
> >> support is only for wl18xx.
> >>
> >
> > How can you tell the difference? What I see is that omap3pandora
> > (and nothing else) calls wl1251_set_platform_data(), while
> > da850-evm and all omap3/omap4 boards use wl12xx_set_platform_data().
> >
> > The latter seems to refer to all wl12xx and wl18xx variants except
> > for wl1251, based on my (very limited) understanding of that code.
> 
> right.
> i got mislead because legacy_init_wl12xx() is defined there only for
> CONFIG_WL12XX (and not for CONFIG_WL18XX).
> it looks like only "isee,omap3-igep0020-rev-f" and
> "isee,omap3-igep0020-rev-g" have pdata quirks for wl18xx (and thus
> initialize the pdata clocks to 0).

Ok.

> sorry for the trivial question, but what's the standard way to submit
> such patch? should i simply add a third patch to the patchset which
> removes the pdata quirk and adds the missing dts definition? i don't
> have such board, so i can only compile-test it.

Yes, I think that would be good. Depending on the complexity of the
patch, you can do it in multiple ways:

a) one patch to all the dts files, which also removes the
   legacy_init_wl12xx() infrastructure

b) one patch to add the .dts changes, a second patch to remove the
   code from pdata-quirks.c and a third to remove the functionality
   in the driver

c) one patch per board.

I'm fine with any of these, but the omap maintainers might have a
preference.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eliad Peller March 9, 2015, 3:13 p.m. UTC | #6 
On Mon, Mar 9, 2015 at 10:50 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 09 March 2015 09:18:46 Eliad Peller wrote:
>> On Sun, Mar 8, 2015 at 11:53 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Sunday 08 March 2015 13:13:13 Eliad Peller wrote:
>> >>
>> >> > I've looked up the what boards actually use this data and found that
>> >> > all of them already support booting from DT: some omap2 boards using
>> >> > arch/arm/mach-omap2/pdata-quirks.c to provide the data, and the
>> >> > davinci 850evm. Can you make sure you add the correct data to all
>> >> > of these dts files as part of your series and remove the
>> >> > wl12xx_platform_data references?
>> >>
>> >> AFAICT, these board files add wl12xx platform data, while the new DT
>> >> support is only for wl18xx.
>> >>
>> >
>> > How can you tell the difference? What I see is that omap3pandora
>> > (and nothing else) calls wl1251_set_platform_data(), while
>> > da850-evm and all omap3/omap4 boards use wl12xx_set_platform_data().
>> >
>> > The latter seems to refer to all wl12xx and wl18xx variants except
>> > for wl1251, based on my (very limited) understanding of that code.
>>
>> right.
>> i got mislead because legacy_init_wl12xx() is defined there only for
>> CONFIG_WL12XX (and not for CONFIG_WL18XX).
>> it looks like only "isee,omap3-igep0020-rev-f" and
>> "isee,omap3-igep0020-rev-g" have pdata quirks for wl18xx (and thus
>> initialize the pdata clocks to 0).
>
> Ok.
>
>> sorry for the trivial question, but what's the standard way to submit
>> such patch? should i simply add a third patch to the patchset which
>> removes the pdata quirk and adds the missing dts definition? i don't
>> have such board, so i can only compile-test it.
>
> Yes, I think that would be good. Depending on the complexity of the
> patch, you can do it in multiple ways:
>
> a) one patch to all the dts files, which also removes the
>    legacy_init_wl12xx() infrastructure
>
> b) one patch to add the .dts changes, a second patch to remove the
>    code from pdata-quirks.c and a third to remove the functionality
>    in the driver
>
> c) one patch per board.
>
> I'm fine with any of these, but the omap maintainers might have a
> preference.
>
thanks. i'll go with a single patch then (unless the maintainers
prefer otherwise).

Eliad.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
index d3dd7bf..c3bc178 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,59 @@  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,wl18xx" },
+	{ }
+};
+
+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)) {
+		dev_err(dev, "No platform data set\n");
+		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;
+
+	pdata = wl12xx_get_platform_data();
+	if (!IS_ERR(pdata))
+		return kmemdup(pdata, sizeof(*pdata), GFP_KERNEL);
+
+	return wlcore_probe_of(dev);
+}
+
+static void wlcore_del_platform_data(struct wl12xx_platform_data *pdata)
+{
+	kfree(pdata);
+}
+
 static int wl1271_probe(struct sdio_func *func,
 				  const struct sdio_device_id *id)
 {
@@ -245,12 +300,9 @@  static int wl1271_probe(struct sdio_func *func,
 	/* Use block mode for transferring over one block size of data */
 	func->card->quirks |= MMC_QUIRK_BLKSZ_FOR_BYTE_MODE;
 
-	pdev_data.pdata = wl12xx_get_platform_data();
-	if (IS_ERR(pdev_data.pdata)) {
-		ret = PTR_ERR(pdev_data.pdata);
-		dev_err(glue->dev, "missing wlan platform data: %d\n", ret);
+	pdev_data.pdata = wlcore_get_platform_data(&func->dev);
+	if (!pdev_data.pdata)
 		goto out_free_glue;
-	}
 
 	/* if sdio can keep power while host is suspended, enable wow */
 	mmcflags = sdio_get_host_pm_caps(func);
@@ -279,7 +331,7 @@  static int wl1271_probe(struct sdio_func *func,
 	if (!glue->core) {
 		dev_err(glue->dev, "can't allocate platform_device");
 		ret = -ENOMEM;
-		goto out_free_glue;
+		goto out_free_pdata;
 	}
 
 	glue->core->dev.parent = &func->dev;
@@ -313,6 +365,9 @@  static int wl1271_probe(struct sdio_func *func,
 out_dev_put:
 	platform_device_put(glue->core);
 
+out_free_pdata:
+	wlcore_del_platform_data(pdev_data.pdata);
+
 out_free_glue:
 	kfree(glue);
 
@@ -323,11 +378,14 @@  out:
 static void wl1271_remove(struct sdio_func *func)
 {
 	struct wl12xx_sdio_glue *glue = sdio_get_drvdata(func);
+	struct wlcore_platdev_data *pdev_data = glue->core->dev.platform_data;
+	struct wl12xx_platform_data *pdata = pdev_data->pdata;
 
 	/* Undo decrement done above in wl1271_probe */
 	pm_runtime_get_noresume(&func->dev);
 
 	platform_device_unregister(glue->core);
+	wlcore_del_platform_data(pdata);
 	kfree(glue);
 }