dt: bindings: add wl18xx wireless device

drivers/net/wireless/ti/wlcore/sdio.c +1 -0

@@ -323,11 +388,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;

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 <[email protected]>

Signed-off-by Eliad Peller <[email protected]>

---

 drivers/net/wireless/ti/wlcore/sdio.c | 80 ++++++++++++++++++++++++++++++++---
 1 file changed, 74 insertions(+), 6 deletions(-)

drivers/net/wireless/ti/wlcore/sdio.c +37 -0

index d3dd7bf..ee556ac 100644
@@ -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)
+{

drivers/net/wireless/ti/wlcore/sdio.c +8 -0

+       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);

Quoted 4 lines

drivers/net/wireless/ti/wlcore/sdio.c +4 -0

+
+static struct wl12xx_platform_data *
+wlcore_get_platform_data(struct device *dev)
+{

drivers/net/wireless/ti/wlcore/sdio.c static struct wl1271_if_operations sdio_ops = { · line 263

Quoted 3 lines

drivers/net/wireless/ti/wlcore/sdio.c +3 -0

+       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.

Quoted 8 lines

drivers/net/wireless/ti/wlcore/sdio.c +8 -0

+       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);

drivers/net/wireless/ti/wlcore/sdio.c static struct wl1271_if_operations sdio_ops = { · line 281

Quoted 10 lines

drivers/net/wireless/ti/wlcore/sdio.c +10 -0

+
+       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?

Quoted 4 lines · On Wednesday 11 March 2015 01:34:19 Javier Martinez Canillas wrote:

Quoted 4 lines

drivers/net/wireless/ti/wlcore/sdio.c +4 -0

+
+static struct wl12xx_platform_data *
+wlcore_get_platform_data(struct device *dev)
+{

Quoted 8 lines

drivers/net/wireless/ti/wlcore/sdio.c static struct wl1271_if_operations sdio_ops = { · line 263

Quoted 3 lines

drivers/net/wireless/ti/wlcore/sdio.c +3 -0

+       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.

No, the pdata-quirks stuff for this driver must die, it was a hack that only exists because we previously could not attach data to an sdio function.

Quoted 14 lines · Arnd Bergmann

Quoted 12 lines · On Wednesday 11 March 2015 01:34:19 Javier Martinez Canillas wrote:

drivers/net/wireless/ti/wlcore/sdio.c static struct wl1271_if_operations sdio_ops = { · line 263

Quoted 7 lines

drivers/net/wireless/ti/wlcore/sdio.c +7 -0

+
+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.

Quoted 4 lines

No, the pdata-quirks stuff for this driver must die, it was a hack that only exists because we previously could not attach data to an sdio function.

Ok sorry, I misunderstood and thought that the output from the discussion in patch 3/3 was that the pdata could not still be removed due not having a way to configure the clocks for wl12xx.

Quoted 8 lines

drivers/net/wireless/ti/wlcore/sdio.c static struct wl1271_if_operations sdio_ops = { · line 281

Quoted 5 lines

drivers/net/wireless/ti/wlcore/sdio.c +5 -0

+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?

Quoted 2 lines

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

As I said earlier, I think it would be best not to dynamically allocate anything here at all. As Eliad explained, the data is used by two different drivers:
wl12xx and wl18xx, and only the latter is converted for now, but after the conversion, it should not need the platform data structure any more, only the irq number that gets passed in from DT.

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +0 -0

@@ -64,4 +64,13 @@
        vmmc-supply = <&lbep5clwmc_wlen>;
        bus-width = <4>;
        non-removable;

Quoted 10 lines · On Monday 09 March 2015 17:36:42 Eliad Peller wrote:

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +4 -0

@@ -64,4 +64,13 @@
        vmmc-supply = <&lbep5clwmc_wlen>;
        bus-width = <4>;
        non-removable;
+
+       #address-cells = <1>;
+       #size-cells = <0>;
+       wlcore: wlcore at 2 {

arch/arm/boot/dts/omap3-igep0030-rev-g.dts

Quoted 6 lines

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +5 -0

+               compatible = "ti,wl1835";
+               reg = <2>;
+               interrupt-parent = <&gpio5>;
+               interrupts = <8 IRQ_TYPE_NONE>;
+       };
 

Why IRQ_TYPE_NONE?

I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start.

Quoted 11 lines · Arnd Bergmann

Quoted 10 lines · On Monday 09 March 2015 17:36:42 Eliad Peller wrote:

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +4 -0

@@ -64,4 +64,13 @@
        vmmc-supply = <&lbep5clwmc_wlen>;
        bus-width = <4>;
        non-removable;
+
+       #address-cells = <1>;
+       #size-cells = <0>;
+       wlcore: wlcore at 2 {

Quoted 9 lines

arch/arm/boot/dts/omap3-igep0030-rev-g.dts

Quoted 6 lines

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +5 -0

+               compatible = "ti,wl1835";
+               reg = <2>;
+               interrupt-parent = <&gpio5>;
+               interrupts = <8 IRQ_TYPE_NONE>;
+       };
 

Why IRQ_TYPE_NONE?

i simply mirrored the current board file (which only sets the irq number).

Quoted 4 lines

I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start.

not sure i'm following - can you elaborate?

i'll summarize the way i see it. please correct me if i'm wrong.

both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings.

for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver)

do you have something else in mind?

Quoted 44 lines · Eliad Peller

Quoted 11 lines · Arnd Bergmann

Quoted 10 lines · On Monday 09 March 2015 17:36:42 Eliad Peller wrote:

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +4 -0

@@ -64,4 +64,13 @@
        vmmc-supply = <&lbep5clwmc_wlen>;
        bus-width = <4>;
        non-removable;
+
+       #address-cells = <1>;
+       #size-cells = <0>;
+       wlcore: wlcore at 2 {

Quoted 9 lines

arch/arm/boot/dts/omap3-igep0030-rev-g.dts

Quoted 6 lines

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +5 -0

+               compatible = "ti,wl1835";
+               reg = <2>;
+               interrupt-parent = <&gpio5>;
+               interrupts = <8 IRQ_TYPE_NONE>;
+       };
 

Why IRQ_TYPE_NONE?

i simply mirrored the current board file (which only sets the irq number).

Quoted 4 lines

I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start.

not sure i'm following - can you elaborate?

i'll summarize the way i see it. please correct me if i'm wrong.

both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings.

for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver)

Quoted 1 line

do you have something else in mind?

I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts.

Quoted 45 lines · Tony Lindgren

Quoted 44 lines · Eliad Peller

Quoted 11 lines · Arnd Bergmann

Quoted 10 lines · On Monday 09 March 2015 17:36:42 Eliad Peller wrote:

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +4 -0

@@ -64,4 +64,13 @@
        vmmc-supply = <&lbep5clwmc_wlen>;
        bus-width = <4>;
        non-removable;
+
+       #address-cells = <1>;
+       #size-cells = <0>;
+       wlcore: wlcore at 2 {

Quoted 9 lines

arch/arm/boot/dts/omap3-igep0030-rev-g.dts

Quoted 6 lines

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +5 -0

+               compatible = "ti,wl1835";
+               reg = <2>;
+               interrupt-parent = <&gpio5>;
+               interrupts = <8 IRQ_TYPE_NONE>;
+       };
 

Why IRQ_TYPE_NONE?

i simply mirrored the current board file (which only sets the irq number).

Quoted 4 lines

I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start.

not sure i'm following - can you elaborate?

i'll summarize the way i see it. please correct me if i'm wrong.

both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings.

for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver)

Quoted 4 lines

Quoted 1 line

do you have something else in mind?

I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts.

I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for "compulab,omap3-sbc-t3730" which initializes wl12xx device.

Quoted 46 lines · On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote:

Quoted 45 lines · Tony Lindgren

Quoted 44 lines · Eliad Peller

Quoted 11 lines · Arnd Bergmann

Quoted 10 lines · On Monday 09 March 2015 17:36:42 Eliad Peller wrote:

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +4 -0

@@ -64,4 +64,13 @@
        vmmc-supply = <&lbep5clwmc_wlen>;
        bus-width = <4>;
        non-removable;
+
+       #address-cells = <1>;
+       #size-cells = <0>;
+       wlcore: wlcore at 2 {

Quoted 9 lines

arch/arm/boot/dts/omap3-igep0030-rev-g.dts

Quoted 6 lines

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +5 -0

+               compatible = "ti,wl1835";
+               reg = <2>;
+               interrupt-parent = <&gpio5>;
+               interrupts = <8 IRQ_TYPE_NONE>;
+       };
 

Why IRQ_TYPE_NONE?

i simply mirrored the current board file (which only sets the irq number).

Quoted 4 lines

I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start.

not sure i'm following - can you elaborate?

i'll summarize the way i see it. please correct me if i'm wrong.

both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings.

for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver)

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

do you have something else in mind?

I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts.

Right, that was my idea.

Quoted 3 lines

I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for "compulab,omap3-sbc-t3730" which initializes wl12xx device.

This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it.

Unfortunately, there is still one machine that uses a board file in combination with this wl12xx: da850-evm calls wl12xx_set_platform_data() from a legacy board file without DT.

We have DT support for this file as well, but if I remember correctly, it is somewhat incomplete and we don't want to remove the file yet. Adding Sekhar and Kevin to Cc here for confirmation.

Oops I forgot about the omap3-sbc-t3730, so yes we have to keep the platform data a little bit longer. But nothing stopping us moving all the other ones to use a proper device tree based configuration.

Quoted 5 lines · On Tuesday 10 March 2015 07:28:05 Tony Lindgren wrote:

Oops I forgot about the omap3-sbc-t3730, so yes we have to keep the platform data a little bit longer. But nothing stopping us moving all the other ones to use a proper device tree based configuration.

For all I can tell, the t3730 board file does not support wl12xx at the moment, only the dts file does.

If we do what Sekhar suggested and drop wl12xx support from the DA850 EVM board file, we can fix all wlcore users.

Quoted 22 lines · On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote:

Quoted 21 lines · Tony Lindgren

Quoted 21 lines

Quoted 4 lines

I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start.

not sure i'm following - can you elaborate?

i'll summarize the way i see it. please correct me if i'm wrong.

both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings.

for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver)

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

do you have something else in mind?

I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts.

Right, that was my idea.

Quoted 3 lines

I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for "compulab,omap3-sbc-t3730" which initializes wl12xx device.

This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it.

Unfortunately, there is still one machine that uses a board file in combination with this wl12xx: da850-evm calls wl12xx_set_platform_data() from a legacy board file without DT.

Quoted 30 lines · Arnd Bergmann

Quoted 22 lines · On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote:

Quoted 21 lines · Tony Lindgren

Quoted 21 lines

Quoted 4 lines

I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start.

not sure i'm following - can you elaborate?

i'll summarize the way i see it. please correct me if i'm wrong.

both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings.

for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver)

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

do you have something else in mind?

I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts.

Right, that was my idea.

Quoted 8 lines

Quoted 3 lines

I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for "compulab,omap3-sbc-t3730" which initializes wl12xx device.

This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it.

i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution)

the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well.

Quoted 44 lines · On Tuesday 10 March 2015 16:31:33 Eliad Peller wrote:

Quoted 30 lines · Arnd Bergmann

Quoted 22 lines · On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote:

Quoted 21 lines · Tony Lindgren

Quoted 21 lines

Quoted 4 lines

I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start.

not sure i'm following - can you elaborate?

i'll summarize the way i see it. please correct me if i'm wrong.

both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings.

for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver)

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

do you have something else in mind?

I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts.

Right, that was my idea.

Quoted 8 lines

Quoted 3 lines

I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for "compulab,omap3-sbc-t3730" which initializes wl12xx device.

This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it.

i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution)

Quoted 4 lines

the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well.

What is missing for that binding then? I keep getting confused here, but I thought that they share the implementation that looks at the platform data.

Quoted 45 lines · Arnd Bergmann

Quoted 44 lines · On Tuesday 10 March 2015 16:31:33 Eliad Peller wrote:

Quoted 30 lines · Arnd Bergmann

Quoted 22 lines · On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote:

Quoted 21 lines · Tony Lindgren

Quoted 21 lines

Quoted 4 lines

I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start.

not sure i'm following - can you elaborate?

i'll summarize the way i see it. please correct me if i'm wrong.

both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings.

for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver)

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

do you have something else in mind?

I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts.

Right, that was my idea.

Quoted 8 lines

Quoted 3 lines

I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for "compulab,omap3-sbc-t3730" which initializes wl12xx device.

This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it.

i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution)

Quoted 9 lines

Quoted 4 lines

the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well.

What is missing for that binding then? I keep getting confused here, but I thought that they share the implementation that looks at the platform data.

they both get the same wl12xx_platform_data struct, but only wl12xx cares about the clocks-related fields.
the bindings i added parses only the irq.

(Luca tried previously to upstream wl12xx DT support along with the required clock DT changes, but got some rejections, mainly wrt. clock stuff.
e.g. http://thread.gmane.org/gmane.linux.kernel/1520752
that's why i preferred starting with "easier" wl18xx bindings only)

Quoted 59 lines · Eliad Peller

Quoted 45 lines · Arnd Bergmann

Quoted 44 lines · On Tuesday 10 March 2015 16:31:33 Eliad Peller wrote:

Quoted 30 lines · Arnd Bergmann

Quoted 22 lines · On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote:

Quoted 21 lines · Tony Lindgren

Quoted 21 lines

Quoted 4 lines

I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start.

not sure i'm following - can you elaborate?

i'll summarize the way i see it. please correct me if i'm wrong.

both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings.

for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver)

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

do you have something else in mind?

I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts.

Right, that was my idea.

Quoted 8 lines

Quoted 3 lines

I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for "compulab,omap3-sbc-t3730" which initializes wl12xx device.

This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it.

i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution)

Quoted 9 lines

Quoted 4 lines

the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well.

What is missing for that binding then? I keep getting confused here, but I thought that they share the implementation that looks at the platform data.

they both get the same wl12xx_platform_data struct, but only wl12xx cares about the clocks-related fields.
the bindings i added parses only the irq.

Quoted 5 lines

(Luca tried previously to upstream wl12xx DT support along with the required clock DT changes, but got some rejections, mainly wrt. clock stuff.
e.g. http://thread.gmane.org/gmane.linux.kernel/1520752
that's why i preferred starting with "easier" wl18xx bindings only)

I believe we did not have clock bindings back then, now it's simple to get the clock. If it's some internal clock to the wl12xx, then that's a different story, it should be just hidden behind a compatible flag then.

Quoted 66 lines · Tony Lindgren

Quoted 59 lines · Eliad Peller

Quoted 45 lines · Arnd Bergmann

Quoted 44 lines · On Tuesday 10 March 2015 16:31:33 Eliad Peller wrote:

Quoted 30 lines · Arnd Bergmann

Quoted 22 lines · On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote:

Quoted 21 lines · Tony Lindgren

Quoted 21 lines

Quoted 4 lines

I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start.

not sure i'm following - can you elaborate?

i'll summarize the way i see it. please correct me if i'm wrong.

both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings.

for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver)

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

do you have something else in mind?

I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts.

Right, that was my idea.

Quoted 8 lines

Quoted 3 lines

I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for "compulab,omap3-sbc-t3730" which initializes wl12xx device.

This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it.

i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution)

Quoted 9 lines

Quoted 4 lines

the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well.

What is missing for that binding then? I keep getting confused here, but I thought that they share the implementation that looks at the platform data.

they both get the same wl12xx_platform_data struct, but only wl12xx cares about the clocks-related fields.
the bindings i added parses only the irq.

Quoted 5 lines

(Luca tried previously to upstream wl12xx DT support along with the required clock DT changes, but got some rejections, mainly wrt. clock stuff.
e.g. http://thread.gmane.org/gmane.linux.kernel/1520752
that's why i preferred starting with "easier" wl18xx bindings only)

Quoted 5 lines

I believe we did not have clock bindings back then, now it's simple to get the clock. If it's some internal clock to the wl12xx, then that's a different story, it should be just hidden behind a compatible flag then.

i'm really not familiar with the common clock framework, but there still doesn't seem to be a way to determine whether a clock is XTAL or not (which is what Luca's patch was about). should we use compatible flag in such case? i'm not sure it sounds right.

anyway, i'd really prefer, if possible, starting with the wl18xx bindings, and defer the wl12xx complications to later on. (i also need to find some wl12xx card in order to actually test the patches once i'll have them...)

we do have to make sure these wl18xx bindings are future-compatible with the wl12xx ones, but i think the current bindings are pretty much standard (and are actually a subset of the bindings needed by wl12xx), so it should be fine.

Quoted 10 lines · Eliad Peller

i'm really not familiar with the common clock framework, but there still doesn't seem to be a way to determine whether a clock is XTAL or not (which is what Luca's patch was about). should we use compatible flag in such case? i'm not sure it sounds right.

anyway, i'd really prefer, if possible, starting with the wl18xx bindings, and defer the wl12xx complications to later on. (i also need to find some wl12xx card in order to actually test the patches once i'll have them...)

Quoted 4 lines

we do have to make sure these wl18xx bindings are future-compatible with the wl12xx ones, but i think the current bindings are pretty much standard (and are actually a subset of the bindings needed by wl12xx), so it should be fine.

Well how about add just the parsing of the clock and assume it's always WL12XX_REFCLOCK_38 for now as that's the only thing we're currently using. Then we can add a property or compatible value if using something else as needed.

Quoted 5 lines

we do have to make sure these wl18xx bindings are future-compatible with the wl12xx ones, but i think the current bindings are pretty much standard (and are actually a subset of the bindings needed by wl12xx), so it should be fine.

Well how about add just the parsing of the clock and assume it's always WL12XX_REFCLOCK_38 for now as that's the only thing we're currently using. Then we can add a property or compatible value if using something else as needed.

Regards,

Tony

Hello Tony,

Quoted 11 lines · Tony Lindgren

Quoted 5 lines

we do have to make sure these wl18xx bindings are future-compatible with the wl12xx ones, but i think the current bindings are pretty much standard (and are actually a subset of the bindings needed by wl12xx), so it should be fine.

Well how about add just the parsing of the clock and assume it's always WL12XX_REFCLOCK_38 for now as that's the only thing we're currently using. Then we can add a property or compatible value if using something else as needed.

What do you mean by parsing here? IIUC there isn't a clock driver for these clocks and are setup directly in the drivers/net/wireless/ti/wl12xx/main.c driver.

So you can't make the WLAN chip dev node a consumer of these clocks by adding a phandle to a clock provider and clock specifiers since there isn't a provider to be referenced in the DT. Or did I misunderstand?

Quoted 8 lines · On Wednesday 11 March 2015 02:00:59 Javier Martinez Canillas wrote:

What do you mean by parsing here? IIUC there isn't a clock driver for these clocks and are setup directly in the drivers/net/wireless/ti/wl12xx/main.c driver.

So you can't make the WLAN chip dev node a consumer of these clocks by adding a phandle to a clock provider and clock specifiers since there isn't a provider to be referenced in the DT. Or did I misunderstand?

As I understand it, the clock signal is provided by an external oscillator,

which we can easily model in DT, and then you call clk_get_rate on that.

If there is no external clock provider for this chip and the clocks are provided by the device itself, then all we need is a clock-frequency property in the device node.

Arnd
--

I think that patch [0] should not be needed since for external clocks, the IP providing the clocks should have its own clock driver and for internal clocks, a property should be used instead as you said.

Quoted 4 lines

If there is no external clock provider for this chip and the clocks are provided by the device itself, then all we need is a clock-frequency property in the device node.

Agreed, IIUC Luciano wanted to expose the internal clocks by registering in the common clock framework but if those clocks are not really accessible from outside the wlan chip, then I also think that a device node property should be used instead.

Quoted 9 lines · Javier Martinez Canillas

I think that patch [0] should not be needed since for external clocks, the IP providing the clocks should have its own clock driver and for internal clocks, a property should be used instead as you said.

Quoted 4 lines

If there is no external clock provider for this chip and the clocks are provided by the device itself, then all we need is a clock-frequency property in the device node.

Quoted 5 lines

Agreed, IIUC Luciano wanted to expose the internal clocks by registering in the common clock framework but if those clocks are not really accessible from outside the wlan chip, then I also think that a device node property should be used instead.

how should i describe multiple clock-frequency properties (there are 2 relevant clocks) in this case?

does something like the following makes sense?

wlcore: wlcore at 2 {
    ...
    refclock: refclock {
        compatible = "fixed-clock";
        #clock-cells = <0>;
        clock-frequency = <38400000>;
    };
}

Hello Arnd,

Quoted 6 lines · Arnd Bergmann

Quoted 5 lines · On Wednesday 11 March 2015 02:00:59 Javier Martinez Canillas wrote:

What do you mean by parsing here? IIUC there isn't a clock driver for these clocks and are setup directly in the drivers/net/wireless/ti/wl12xx/main.c driver.

Quoted 5 lines

Quoted 3 lines

So you can't make the WLAN chip dev node a consumer of these clocks by adding a phandle to a clock provider and clock specifiers since there isn't a provider to be referenced in the DT. Or did I misunderstand?

As I understand it, the clock signal is provided by an external oscillator,

According to [0], it seems the chip can be connected to both external oscillators or internal clocks provided by the chip itself.

Quoted 2 lines

which we can easily model in DT, and then you call clk_get_rate on that.

Right, my point wast that this can be done only if the external oscillator have a proper clock driver / provider which I don't think is the case here. Most of this stuff predates the common clock framework.

Or at least Luciano Coelho had a patch on his series to make the wilink driver a clock provider itself by registering the refclock and tcxoclock clocks [0].

Luciano also had patches for:

* Adding the clock provider dev node in the DTS [1]
* Have a table to map the clock rate with the FW configuration values [2]
* Getting the clock from DT and the rate as you said to configure the firmware accordingly [3]

I think that patch [0] should not be needed since for external clocks, the IP providing the clocks should have its own clock driver and for internal clocks, a property should be used instead as you said.

Quoted 4 lines

If there is no external clock provider for this chip and the clocks are provided by the device itself, then all we need is a clock-frequency property in the device node.

Agreed, IIUC Luciano wanted to expose the internal clocks by registering in the common clock framework but if those clocks are not really accessible from outside the wlan chip, then I also think that a device node property should be used instead.

Quoted 13 lines · On Wednesday 11 March 2015 12:34:03 Javier Martinez Canillas wrote:

Hello Arnd,

Quoted 10 lines · Arnd Bergmann

Quoted 8 lines · On Wednesday 11 March 2015 02:00:59 Javier Martinez Canillas wrote:

What do you mean by parsing here? IIUC there isn't a clock driver for these clocks and are setup directly in the drivers/net/wireless/ti/wl12xx/main.c driver.

So you can't make the WLAN chip dev node a consumer of these clocks by adding a phandle to a clock provider and clock specifiers since there isn't a provider to be referenced in the DT. Or did I misunderstand?

Quoted 4 lines

Quoted 1 line

As I understand it, the clock signal is provided by an external oscillator,

According to [0], it seems the chip can be connected to both external oscillators or internal clocks provided by the chip itself.

I see, that part wasn't clear to me.

Quoted 9 lines

Quoted 4 lines

If there is no external clock provider for this chip and the clocks are provided by the device itself, then all we need is a clock-frequency property in the device node.

Agreed, IIUC Luciano wanted to expose the internal clocks by registering in the common clock framework but if those clocks are not really accessible from outside the wlan chip, then I also think that a device node property should be used instead.

If I understand this right, that measn we can easily distinguish between an external XTAL clock and the internal clock by they way they are described in the DT: for the internal clock, we just provide a clock-frequency property, while the external clock would be referenced through a clocks property.

Right now it seems that all boards in mainline with a WiLink6 part are using internal clocks. So as a first step I think that adding an optional refclock-frequency and tcxoclock-frequency properties should be enough.

It would be good if the driver supports getting the refclock and tcxoclock from an external provider in case a board gets these from external clocks but that can be done as a followup if there are boards in the future using that design.

But please bear in mind that I'm not familiar with the clock handling in WiLink6 since the WiLink8 part used in the IGEP boards does not need these clocks and I only looked at Luciano's previous patches and the WiLink today driver today. So it would be good if Eliad can double check my assumptions to see if those are correct.

Quoted 11 lines · On Wednesday 11 March 2015 14:07:11 Javier Martinez Canillas wrote:

Right now it seems that all boards in mainline with a WiLink6 part are using internal clocks. So as a first step I think that adding an optional refclock-frequency and tcxoclock-frequency properties should be enough.

It would be good if the driver supports getting the refclock and tcxoclock from an external provider in case a board gets these from external clocks but that can be done as a followup if there are boards in the future using that design.

Quoted 5 lines

But please bear in mind that I'm not familiar with the clock handling in WiLink6 since the WiLink8 part used in the IGEP boards does not need these clocks and I only looked at Luciano's previous patches and the WiLink today driver today. So it would be good if Eliad can double check my assumptions to see if those are correct.

Sounds good. I'd also be fine with not implementing the case for external clocks in the code until we need (and can test) it, but I think it should be specified in the binding from the start.

Quoted 12 lines · Arnd Bergmann

Quoted 11 lines · On Wednesday 11 March 2015 14:07:11 Javier Martinez Canillas wrote:

Right now it seems that all boards in mainline with a WiLink6 part are using internal clocks. So as a first step I think that adding an optional refclock-frequency and tcxoclock-frequency properties should be enough.

It would be good if the driver supports getting the refclock and tcxoclock from an external provider in case a board gets these from external clocks but that can be done as a followup if there are boards in the future using that design.

Quoted 6 lines

Quoted 5 lines

But please bear in mind that I'm not familiar with the clock handling in WiLink6 since the WiLink8 part used in the IGEP boards does not need these clocks and I only looked at Luciano's previous patches and the WiLink today driver today. So it would be good if Eliad can double check my assumptions to see if those are correct.

Quoted 4 lines

Sounds good. I'd also be fine with not implementing the case for external clocks in the code until we need (and can test) it, but I think it should be specified in the binding from the start.

Agreed.

Quoted 13 lines · Javier Martinez Canillas

Quoted 12 lines · Arnd Bergmann

Quoted 11 lines · On Wednesday 11 March 2015 14:07:11 Javier Martinez Canillas wrote:

Right now it seems that all boards in mainline with a WiLink6 part are using internal clocks. So as a first step I think that adding an optional refclock-frequency and tcxoclock-frequency properties should be enough.

It would be good if the driver supports getting the refclock and tcxoclock from an external provider in case a board gets these from external clocks but that can be done as a followup if there are boards in the future using that design.

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

But please bear in mind that I'm not familiar with the clock handling in WiLink6 since the WiLink8 part used in the IGEP boards does not need these clocks and I only looked at Luciano's previous patches and the WiLink today driver today. So it would be good if Eliad can double check my assumptions to see if those are correct.

sounds right. that's what i know as well.

Quoted 6 lines

Quoted 4 lines

Sounds good. I'd also be fine with not implementing the case for external clocks in the code until we need (and can test) it, but I think it should be specified in the binding from the start.

Agreed.

great. so i'll implement the internal clocks case only.

Quoted 46 lines · On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote:

Quoted 45 lines · Tony Lindgren

Quoted 44 lines · Eliad Peller

Quoted 11 lines · Arnd Bergmann

Quoted 10 lines · On Monday 09 March 2015 17:36:42 Eliad Peller wrote:

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +4 -0

@@ -64,4 +64,13 @@
        vmmc-supply = <&lbep5clwmc_wlen>;
        bus-width = <4>;
        non-removable;
+
+       #address-cells = <1>;
+       #size-cells = <0>;
+       wlcore: wlcore at 2 {

Quoted 9 lines

arch/arm/boot/dts/omap3-igep0030-rev-g.dts

Quoted 6 lines

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +5 -0

+               compatible = "ti,wl1835";
+               reg = <2>;
+               interrupt-parent = <&gpio5>;
+               interrupts = <8 IRQ_TYPE_NONE>;
+       };
 

Why IRQ_TYPE_NONE?

i simply mirrored the current board file (which only sets the irq number).

Quoted 4 lines

I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start.

not sure i'm following - can you elaborate?

i'll summarize the way i see it. please correct me if i'm wrong.

both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings.

for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver)

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

do you have something else in mind?

I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts.

Right, that was my idea.

Quoted 3 lines

I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for "compulab,omap3-sbc-t3730" which initializes wl12xx device.

This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it.

Unfortunately, there is still one machine that uses a board file in combination with this wl12xx: da850-evm calls wl12xx_set_platform_data() from a legacy board file without DT.

We have DT support for this file as well, but if I remember correctly, it is somewhat incomplete and we don't want to remove the file yet. Adding Sekhar and Kevin to Cc here for confirmation.

Quoted 11 lines · Arnd Bergmann

Quoted 10 lines · On Monday 09 March 2015 17:36:42 Eliad Peller wrote:

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +4 -0

@@ -64,4 +64,13 @@
        vmmc-supply = <&lbep5clwmc_wlen>;
        bus-width = <4>;
        non-removable;
+
+       #address-cells = <1>;
+       #size-cells = <0>;
+       wlcore: wlcore at 2 {

Quoted 9 lines

arch/arm/boot/dts/omap3-igep0030-rev-g.dts

Quoted 6 lines

arch/arm/boot/dts/omap3-igep0030-rev-g.dts +5 -0

+               compatible = "ti,wl1835";
+               reg = <2>;
+               interrupt-parent = <&gpio5>;
+               interrupts = <8 IRQ_TYPE_NONE>;
+       };
 

Why IRQ_TYPE_NONE?

i simply mirrored the current board file (which only sets the irq number).

Add wl18xx (wilink8) bindings to omap3-igep0030-rev-g and omap3-igep0020-rev-f dts files, instead of defining the platform data through the pdata-quirks.

The patch was compile-tested only.

Signed-off-by Eliad Peller <[email protected]>

---
 arch/arm/boot/dts/omap3-igep0020-rev-f.dts | 9 +++++++++
 arch/arm/boot/dts/omap3-igep0030-rev-g.dts | 9 +++++++++
 arch/arm/mach-omap2/pdata-quirks.c         | 2 --
 3 files changed, 18 insertions(+), 2 deletions(-)

arch/arm/boot/dts/omap3-igep0020-rev-f.dts +0 -0

index cc8bd0c..8e5b44e 100644
@@ -42,4 +42,13 @@
        vmmc-supply = <&lbep5clwmc_wlen>;

arch/arm/boot/dts/omap3-igep0020-rev-f.dts line 42

Quoted 4 lines · Eliad Peller

arch/arm/boot/dts/omap3-igep0020-rev-f.dts +0 -0

@@ -42,4 +42,13 @@
        vmmc-supply = <&lbep5clwmc_wlen>;

Hello Eliad,

Quoted 5 lines · Javier Martinez Canillas

arch/arm/boot/dts/omap3-igep0020-rev-f.dts line 42

Quoted 4 lines · Eliad Peller

arch/arm/boot/dts/omap3-igep0020-rev-f.dts +0 -0

@@ -42,4 +42,13 @@
        vmmc-supply = <&lbep5clwmc_wlen>;

Something I forgot to mention is that this vmmc-supply is a DT hack since the WLAN SDIO chip needs a WL_EN signal to be asserted as a part of the chip power sequencing.

But there wasn't a DT binding to describe that so most boards use the same hack which is to define a fake fixed-regulator with an enable GPIO just to toggle that pin.

But now the MMC subsystem has support for power sequence providers and the pwrseq-simple driver (Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt) has support for reset GPIOs so you may want to change that as well.

Quoted 5 lines · Javier Martinez Canillas

But there wasn't a DT binding to describe that so most boards use the same hack which is to define a fake fixed-regulator with an enable GPIO just to toggle that pin.

Quoted 5 lines

But now the MMC subsystem has support for power sequence providers and the pwrseq-simple driver (Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt) has support for reset GPIOs so you may want to change that as well.

interesting. i wasn't aware of it.
but that's for a different time i guess :)

thanks,
Eliad.

Hello Eliad,

Quoted 4 lines · Eliad Peller

Add wl18xx (wilink8) bindings to omap3-igep0030-rev-g and omap3-igep0020-rev-f dts files, instead of defining the platform data through the pdata-quirks.

Quoted 2 lines

The patch was compile-tested only.

You should look at MAINTAINERS or use ./scripts/get_maintainer.pl to know who should be cc'ed. Specially for this kind of patches where you are not able to test on the actual platform. Added Enric to cc as well since he also maintains the IGEP boards an has access to the hw too.

Quoted 16 lines

 arch/arm/boot/dts/omap3-igep0020-rev-f.dts | 9 +++++++++
 arch/arm/boot/dts/omap3-igep0030-rev-g.dts | 9 +++++++++
 arch/arm/mach-omap2/pdata-quirks.c         | 2 --
 3 files changed, 18 insertions(+), 2 deletions(-)

arch/arm/boot/dts/omap3-igep0020-rev-f.dts +3 -0

index cc8bd0c..8e5b44e 100644
@@ -42,4 +42,13 @@
        vmmc-supply = <&lbep5clwmc_wlen>;
        bus-width = <4>;
        non-removable;
+
+       #address-cells = <1>;
+       #size-cells = <0>;

arch/arm/boot/dts/omap3-igep0020-rev-f.dts line 52

Quoted 5 lines

arch/arm/boot/dts/omap3-igep0020-rev-f.dts +5 -0

+       wlcore: wlcore at 2 {
+               compatible = "ti,wl1835";
+               reg = <2>;
+               interrupt-parent = <&gpio6>;
+               interrupts = <17 IRQ_TYPE_NONE>;

As Arnd said, it seems this should be IRQ_TYPE_LEVEL_HIGH to match what the platform code is currently doing.