Add DT support for WLS1271 SPI driver.
Fix oops by adding a power callback.
Uri Mashiach (3):
wlcore/wl12xx: spi: fix NULL pointer dereference (Oops)
wlcore/wl12xx: spi: add device tree support
wlcore/wl12xx: spi: add wifi support to cm-t335
.../bindings/net/wireless/ti,wlcore,spi.txt | 35 +++++++
arch/arm/boot/dts/am335x-cm-t335.dts | 58 +++++++++++-
drivers/net/wireless/ti/wlcore/spi.c | 104 +++++++++++++++++++--
3 files changed, 189 insertions(+), 8 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
--
2.5.0
Add DT support for the wl1271 SPI WiFi.
Add documentation file for the wl1271 SPI WiFi.
Signed-off-by: Uri Mashiach <[email protected]>
Acked-by: Igor Grinberg <[email protected]>
---
.../bindings/net/wireless/ti,wlcore,spi.txt | 35 +++++++++++
drivers/net/wireless/ti/wlcore/spi.c | 67 +++++++++++++++++++---
2 files changed, 95 insertions(+), 7 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
new file mode 100644
index 0000000..a3e7eb7
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
@@ -0,0 +1,35 @@
+* Texas Instruments wl1271 wireless lan controller
+
+The wl1271 chip can be connected via SPI or via SDIO. This
+document describes the binding for the SPI connected chip.
+
+Required properties:
+- compatible : Should be "ti,wl1271"
+- reg : Chip select address of device
+- spi-max-frequency : Maximum SPI clocking speed of device in Hz
+- ref-clock-frequency : Reference clock frequency
+- interrupts : Should contain interrupt line and trigger type
+- interrupt-parent : Should be the phandle for the interrupt controller
+ that services interrupts for this device
+- vwlan-supply : Point the node of the regulator that controls the wl1271 chip WLAN_EN pin
+
+Optional properties:
+- clock-xtal : boolean, clock is generated from XTAL
+
+- Please consult Documentation/devicetree/bindings/spi/spi-bus.txt
+ for optional SPI connection related properties,
+
+Examples:
+
+&spi1 {
+ wl1271@1 {
+ compatible = "ti,wl1271";
+
+ reg = <1>;
+ spi-max-frequency = <48000000>;
+ clock-xtal;
+ interrupt-parent = <&gpio3>;
+ interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
+ wifi-supply = <&wifi_fixed>;
+ };
+};
diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
index d3a4bcb..7281f5a 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -30,6 +30,7 @@
#include <linux/spi/spi.h>
#include <linux/wl12xx.h>
#include <linux/platform_device.h>
+#include <linux/of_irq.h>
#include <linux/regulator/consumer.h>
#include "wlcore.h"
@@ -357,6 +358,54 @@ static struct wl1271_if_operations spi_ops = {
.set_block_size = NULL,
};
+#ifdef CONFIG_OF
+static const struct of_device_id wlcore_spi_of_match_table[] = {
+ { .compatible = "ti,wl1271" },
+ { }
+};
+
+/**
+ * wlcore_probe_of - DT node parsing.
+ * @spi: SPI slave device parameters.
+ * @res: resource parameters.
+ * @glue: wl12xx SPI bus to slave device glue parameters.
+ * @pdev_data: wlcore device parameters
+ */
+static int wlcore_probe_of(struct spi_device *spi, struct resource *res,
+ struct wl12xx_spi_glue *glue,
+ struct wlcore_platdev_data *pdev_data)
+{
+ struct device_node *dt_node = spi->dev.of_node;
+ int ret;
+
+ ret = of_irq_to_resource(dt_node, 0, &res[0]);
+ if (spi->irq != ret) {
+ dev_err(glue->dev, "can't get interrupt resource\n");
+ return -EINVAL;
+ }
+
+ if (of_find_property(dt_node, "clock-xtal", NULL))
+ pdev_data->ref_clock_xtal = true;
+
+ ret = of_property_read_u32(dt_node, "ref-clock-frequency",
+ &pdev_data->ref_clock_freq);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(glue->dev,
+ "can't get reference clock frequency (%d)\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+#else /* CONFIG_OF */
+static int wlcore_probe_of(struct spi_device *spi, struct resource *res,
+ struct wl12xx_spi_glue *glue,
+ struct wlcore_platdev_data *pdev_data)
+{
+ return -ENODATA;
+}
+#endif /* CONFIG_OF */
+
static int wl1271_probe(struct spi_device *spi)
{
struct wl12xx_spi_glue *glue;
@@ -365,8 +414,7 @@ static int wl1271_probe(struct spi_device *spi)
int ret;
memset(&pdev_data, 0x00, sizeof(pdev_data));
-
- /* TODO: add DT parsing when needed */
+ memset(res, 0x00, sizeof(res));
pdev_data.if_ops = &spi_ops;
@@ -390,6 +438,13 @@ static int wl1271_probe(struct spi_device *spi)
return PTR_ERR(glue->reg);
}
+ ret = wlcore_probe_of(spi, &res[0], glue, &pdev_data);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(glue->dev,
+ "can't get device tree parameters (%d)\n", ret);
+ return ret;
+ }
+
ret = spi_setup(spi);
if (ret < 0) {
dev_err(glue->dev, "spi_setup failed\n");
@@ -404,10 +459,6 @@ static int wl1271_probe(struct spi_device *spi)
glue->core->dev.parent = &spi->dev;
- memset(res, 0x00, sizeof(res));
-
- res[0].start = spi->irq;
- res[0].flags = IORESOURCE_IRQ;
res[0].name = "irq";
ret = platform_device_add_resources(glue->core, res, ARRAY_SIZE(res));
@@ -445,10 +496,12 @@ static int wl1271_remove(struct spi_device *spi)
return 0;
}
-
static struct spi_driver wl1271_spi_driver = {
.driver = {
.name = "wl1271_spi",
+#ifdef CONFIG_OF
+ .of_match_table = of_match_ptr(wlcore_spi_of_match_table),
+#endif /* CONFIG_OF */
},
.probe = wl1271_probe,
--
2.5.0
Hi Grygorii,
On 12/23/2015 12:08 PM, Grygorii Strashko wrote:
> On 12/23/2015 10:35 AM, Uri Mashiach wrote:
>> The power function uses a consumer regulator access to update the WiFi
>> enable GPIO value.
>>
>> Fix the below Oops when trying to modprobe wlcore_spi.
>> The oops occurs because the wl1271_power_{off,on}()
>> function doesn't check the power() function pointer.
>>
>> [ 23.401447] Unable to handle kernel NULL pointer dereference at
>> virtual address 00000000
>> [ 23.409954] pgd = c0004000
>> [ 23.412922] [00000000] *pgd=00000000
>> [ 23.416693] Internal error: Oops: 80000007 [#1] SMP ARM
>> [ 23.422168] Modules linked in: wl12xx wlcore mac80211 cfg80211
>> musb_dsps musb_hdrc usbcore usb_common snd_soc_simple_card evdev joydev
>> omap_rng wlcore_spi snd_soc_tlv320aic23_i2c rng_core snd_soc_tlv320aic23
>> c_can_platform c_can can_dev snd_soc_davinci_mcasp snd_soc_edma
>> snd_soc_omap omap_wdt musb_am335x cpufreq_dt thermal_sys hwmon
>> [ 23.453253] CPU: 0 PID: 36 Comm: kworker/0:2 Not tainted
>> 4.2.0-00002-g951efee-dirty #233
>> [ 23.461720] Hardware name: Generic AM33XX (Flattened Device Tree)
>> [ 23.468123] Workqueue: events request_firmware_work_func
>> [ 23.473690] task: de32efc0 ti: de4ee000 task.ti: de4ee000
>> [ 23.479341] PC is at 0x0
>> [ 23.482112] LR is at wl12xx_set_power_on+0x28/0x124 [wlcore]
>
> Why can't you just add proper check in wl1271_power_on/wl1271_power_off() instead?
>
Will be done in v2.
Thanks,
Uri
Hi Grygorii,
On 12/23/2015 12:25 PM, Grygorii Strashko wrote:
> On 12/23/2015 10:35 AM, Uri Mashiach wrote:
>> Add DT support for the wl1271 SPI WiFi.
>>
>> Add documentation file for the wl1271 SPI WiFi.
>>
>> Signed-off-by: Uri Mashiach <[email protected]>
>> Acked-by: Igor Grinberg <[email protected]>
>> ---
>> .../bindings/net/wireless/ti,wlcore,spi.txt | 35 +++++++++++
>> drivers/net/wireless/ti/wlcore/spi.c | 67 +++++++++++++++++++---
>> 2 files changed, 95 insertions(+), 7 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
>> new file mode 100644
>> index 0000000..a3e7eb7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
>> @@ -0,0 +1,35 @@
>> +* Texas Instruments wl1271 wireless lan controller
>> +
>> +The wl1271 chip can be connected via SPI or via SDIO. This
>> +document describes the binding for the SPI connected chip.
>> +
>> +Required properties:
>> +- compatible : Should be "ti,wl1271"
>> +- reg : Chip select address of device
>> +- spi-max-frequency : Maximum SPI clocking speed of device in Hz
>> +- ref-clock-frequency : Reference clock frequency
>> +- interrupts : Should contain interrupt line and trigger type
>
> It's good to describe here number of IRQ lines your device have and IRQ types (edge/level)
>
>> +- interrupt-parent : Should be the phandle for the interrupt controller
> interrupt-parent is not required (also interrupts-extended can be used)
>
>> + that services interrupts for this device
>> +- vwlan-supply : Point the node of the regulator that controls the wl1271 chip WLAN_EN pin
>
> Pls, use gpio if this is required just to toggle WLAN_EN pin.
>
> [ti,] power-gpio or en-gpio
>
>
>> +
>> +Optional properties:
>> +- clock-xtal : boolean, clock is generated from XTAL
>> +
>> +- Please consult Documentation/devicetree/bindings/spi/spi-bus.txt
>> + for optional SPI connection related properties,just
>> +
>> +Examples:
>> +
>> +&spi1 {
>> + wl1271@1 {
>> + compatible = "ti,wl1271";
>> +
>> + reg = <1>;
>> + spi-max-frequency = <48000000>;
>> + clock-xtal;
>> + interrupt-parent = <&gpio3>;
>> + interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
>> + wifi-supply = <&wifi_fixed>;
>> + };
>> +};
>> diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
>> index d3a4bcb..7281f5a 100644
>> --- a/drivers/net/wireless/ti/wlcore/spi.c
>> +++ b/drivers/net/wireless/ti/wlcore/spi.c
>> @@ -30,6 +30,7 @@
>> #include <linux/spi/spi.h>
>> #include <linux/wl12xx.h>
>> #include <linux/platform_device.h>
>> +#include <linux/of_irq.h>
>> #include <linux/regulator/consumer.h>
>>
>> #include "wlcore.h"
>> @@ -357,6 +358,54 @@ static struct wl1271_if_operations spi_ops = {
>> .set_block_size = NULL,
>> };
>>
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id wlcore_spi_of_match_table[] = {
>> + { .compatible = "ti,wl1271" },
>> + { }
>> +};
>> +
>> +/**
>> + * wlcore_probe_of - DT node parsing.
>> + * @spi: SPI slave device parameters.
>> + * @res: resource parameters.
>> + * @glue: wl12xx SPI bus to slave device glue parameters.
>> + * @pdev_data: wlcore device parameters
>> + */
>> +static int wlcore_probe_of(struct spi_device *spi, struct resource *res,
>> + struct wl12xx_spi_glue *glue,
>> + struct wlcore_platdev_data *pdev_data)
>> +{
>> + struct device_node *dt_node = spi->dev.of_node;
>> + int ret;
>> +
>> + ret = of_irq_to_resource(dt_node, 0, &res[0]);
>> + if (spi->irq != ret) {
>> + dev_err(glue->dev, "can't get interrupt resource\n");
>> + return -EINVAL;
>> + }
>
> SPI core will parse IRQ for you (see below)
>
The IRQ type should be stored in res[0].flags.
Is there an alternative to of_irq_to_resource() for getting the IRQ type?
>> +
>> + if (of_find_property(dt_node, "clock-xtal", NULL))
>> + pdev_data->ref_clock_xtal = true;
>> +
>> + ret = of_property_read_u32(dt_node, "ref-clock-frequency",
>> + &pdev_data->ref_clock_freq);
>> + if (IS_ERR_VALUE(ret)) {
>> + dev_err(glue->dev,
>> + "can't get reference clock frequency (%d)\n", ret);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +#else /* CONFIG_OF */
>> +static int wlcore_probe_of(struct spi_device *spi, struct resource *res,
>> + struct wl12xx_spi_glue *glue,
>> + struct wlcore_platdev_data *pdev_data)
>> +{
>> + return -ENODATA;
>> +}
>> +#endif /* CONFIG_OF */
>> +
>> static int wl1271_probe(struct spi_device *spi)
>> {
>> struct wl12xx_spi_glue *glue;
>> @@ -365,8 +414,7 @@ static int wl1271_probe(struct spi_device *spi)
>> int ret;
>>
>> memset(&pdev_data, 0x00, sizeof(pdev_data));
>> -
>> - /* TODO: add DT parsing when needed */
>> + memset(res, 0x00, sizeof(res));
>>
>> pdev_data.if_ops = &spi_ops;
>>
>> @@ -390,6 +438,13 @@ static int wl1271_probe(struct spi_device *spi)
>> return PTR_ERR(glue->reg);
>> }
>>
>> + ret = wlcore_probe_of(spi, &res[0], glue, &pdev_data);
>> + if (IS_ERR_VALUE(ret)) {
>> + dev_err(glue->dev,
>> + "can't get device tree parameters (%d)\n", ret);
>> + return ret;
>
> So, wl1271_probe will fail always if CONFIG_OF=n. Right?
>
>
>> + }
>> +
>> ret = spi_setup(spi);
>> if (ret < 0) {
>> dev_err(glue->dev, "spi_setup failed\n");
>> @@ -404,10 +459,6 @@ static int wl1271_probe(struct spi_device *spi)
>>
>> glue->core->dev.parent = &spi->dev;
>>
>> - memset(res, 0x00, sizeof(res));
>> -
>> - res[0].start = spi->irq;
>> - res[0].flags = IORESOURCE_IRQ;
>> res[0].name = "irq";
>
> Above code might be left unchaenged.
>
>
>>
>> ret = platform_device_add_resources(glue->core, res, ARRAY_SIZE(res));
>> @@ -445,10 +496,12 @@ static int wl1271_remove(struct spi_device *spi)
>> return 0;
>> }
>>
>> -
>> static struct spi_driver wl1271_spi_driver = {
>> .driver = {
>> .name = "wl1271_spi",
>> +#ifdef CONFIG_OF
>> + .of_match_table = of_match_ptr(wlcore_spi_of_match_table),
>> +#endif /* CONFIG_OF */
>
> These ifdefs are not needed - see of_match_ptr() definition
>
>> },
>>
>> .probe = wl1271_probe,
>>
>
>
--
Thanks,
Uri
The power function uses a consumer regulator access to update the WiFi
enable GPIO value.
Fix the below Oops when trying to modprobe wlcore_spi.
The oops occurs because the wl1271_power_{off,on}()
function doesn't check the power() function pointer.
[ 23.401447] Unable to handle kernel NULL pointer dereference at
virtual address 00000000
[ 23.409954] pgd = c0004000
[ 23.412922] [00000000] *pgd=00000000
[ 23.416693] Internal error: Oops: 80000007 [#1] SMP ARM
[ 23.422168] Modules linked in: wl12xx wlcore mac80211 cfg80211
musb_dsps musb_hdrc usbcore usb_common snd_soc_simple_card evdev joydev
omap_rng wlcore_spi snd_soc_tlv320aic23_i2c rng_core snd_soc_tlv320aic23
c_can_platform c_can can_dev snd_soc_davinci_mcasp snd_soc_edma
snd_soc_omap omap_wdt musb_am335x cpufreq_dt thermal_sys hwmon
[ 23.453253] CPU: 0 PID: 36 Comm: kworker/0:2 Not tainted
4.2.0-00002-g951efee-dirty #233
[ 23.461720] Hardware name: Generic AM33XX (Flattened Device Tree)
[ 23.468123] Workqueue: events request_firmware_work_func
[ 23.473690] task: de32efc0 ti: de4ee000 task.ti: de4ee000
[ 23.479341] PC is at 0x0
[ 23.482112] LR is at wl12xx_set_power_on+0x28/0x124 [wlcore]
[ 23.488074] pc : [<00000000>] lr : [<bf2581f0>] psr: 60000013
[ 23.488074] sp : de4efe50 ip : 00000002 fp : 00000000
[ 23.500162] r10: de7cdd00 r9 : dc848800 r8 : bf27af00
[ 23.505663] r7 : bf27a1a8 r6 : dcbd8a80 r5 : dce0e2e0 r4 :
dce0d2e0
[ 23.512536] r3 : 00000000 r2 : 00000000 r1 : 00000001 r0 :
dc848810
[ 23.519412] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM
Segment kernel
[ 23.527109] Control: 10c5387d Table: 9cb78019 DAC: 00000015
[ 23.533160] Process kworker/0:2 (pid: 36, stack limit = 0xde4ee218)
[ 23.539760] Stack: (0xde4efe50 to 0xde4f0000)
[...]
[ 23.665030] [<bf2581f0>] (wl12xx_set_power_on [wlcore]) from
[<bf25f7ac>] (wlcore_nvs_cb+0x118/0xa4c [wlcore])
[ 23.675604] [<bf25f7ac>] (wlcore_nvs_cb [wlcore]) from [<c04387ec>]
(request_firmware_work_func+0x30/0x58)
[ 23.685784] [<c04387ec>] (request_firmware_work_func) from
[<c0058e2c>] (process_one_work+0x1b4/0x4b4)
[ 23.695591] [<c0058e2c>] (process_one_work) from [<c0059168>]
(worker_thread+0x3c/0x4a4)
[ 23.704124] [<c0059168>] (worker_thread) from [<c005ee68>]
(kthread+0xd4/0xf0)
[ 23.711747] [<c005ee68>] (kthread) from [<c000f598>]
(ret_from_fork+0x14/0x3c)
[ 23.719357] Code: bad PC value
[ 23.722760] ---[ end trace 981be8510db9b3a9 ]---
Fix this by adding a power() function implementation.
Signed-off-by: Uri Mashiach <[email protected]>
Acked-by: Igor Grinberg <[email protected]>
---
drivers/net/wireless/ti/wlcore/spi.c | 37 ++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
index 44f059f..d3a4bcb 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -30,6 +30,7 @@
#include <linux/spi/spi.h>
#include <linux/wl12xx.h>
#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
#include "wlcore.h"
#include "wl12xx_80211.h"
@@ -81,6 +82,7 @@
struct wl12xx_spi_glue {
struct device *dev;
struct platform_device *core;
+ struct regulator *reg; /* Power regulator */
};
static void wl12xx_spi_reset(struct device *child)
@@ -318,11 +320,40 @@ static int __must_check wl12xx_spi_raw_write(struct device *child, int addr,
return 0;
}
+/**
+ * wl12xx_spi_set_power - power on/off the wl12xx unit
+ * @child: wl12xx device handle.
+ * @enable: true/false to power on/off the unit.
+ *
+ * use the WiFi enable regulator to enable/disable the WiFi unit.
+ */
+static int wl12xx_spi_set_power(struct device *child, bool enable)
+{
+ int ret = 0;
+ struct wl12xx_spi_glue *glue = dev_get_drvdata(child->parent);
+
+ WARN_ON(!glue->reg);
+
+ /* Update regulator state */
+ if (enable) {
+ ret = regulator_enable(glue->reg);
+ if (ret)
+ dev_err(child, "Power enable failure\n");
+ } else {
+ ret = regulator_disable(glue->reg);
+ if (ret)
+ dev_err(child, "Power disable failure\n");
+ }
+
+ return ret;
+}
+
static struct wl1271_if_operations spi_ops = {
.read = wl12xx_spi_raw_read,
.write = wl12xx_spi_raw_write,
.reset = wl12xx_spi_reset,
.init = wl12xx_spi_init,
+ .power = wl12xx_spi_set_power,
.set_block_size = NULL,
};
@@ -353,6 +384,12 @@ static int wl1271_probe(struct spi_device *spi)
* comes from the board-peripherals file */
spi->bits_per_word = 32;
+ glue->reg = devm_regulator_get(&spi->dev, "vwlan");
+ if (IS_ERR(glue->reg)) {
+ dev_err(glue->dev, "can't get regulator\n");
+ return PTR_ERR(glue->reg);
+ }
+
ret = spi_setup(spi);
if (ret < 0) {
dev_err(glue->dev, "spi_setup failed\n");
--
2.5.0
On 12/23/2015 10:35 AM, Uri Mashiach wrote:
> The power function uses a consumer regulator access to update the WiFi
> enable GPIO value.
>
> Fix the below Oops when trying to modprobe wlcore_spi.
> The oops occurs because the wl1271_power_{off,on}()
> function doesn't check the power() function pointer.
>
> [ 23.401447] Unable to handle kernel NULL pointer dereference at
> virtual address 00000000
> [ 23.409954] pgd = c0004000
> [ 23.412922] [00000000] *pgd=00000000
> [ 23.416693] Internal error: Oops: 80000007 [#1] SMP ARM
> [ 23.422168] Modules linked in: wl12xx wlcore mac80211 cfg80211
> musb_dsps musb_hdrc usbcore usb_common snd_soc_simple_card evdev joydev
> omap_rng wlcore_spi snd_soc_tlv320aic23_i2c rng_core snd_soc_tlv320aic23
> c_can_platform c_can can_dev snd_soc_davinci_mcasp snd_soc_edma
> snd_soc_omap omap_wdt musb_am335x cpufreq_dt thermal_sys hwmon
> [ 23.453253] CPU: 0 PID: 36 Comm: kworker/0:2 Not tainted
> 4.2.0-00002-g951efee-dirty #233
> [ 23.461720] Hardware name: Generic AM33XX (Flattened Device Tree)
> [ 23.468123] Workqueue: events request_firmware_work_func
> [ 23.473690] task: de32efc0 ti: de4ee000 task.ti: de4ee000
> [ 23.479341] PC is at 0x0
> [ 23.482112] LR is at wl12xx_set_power_on+0x28/0x124 [wlcore]
Why can't you just add proper check in wl1271_power_on/wl1271_power_off() instead?
--
regards,
-grygorii
* Uri Mashiach <[email protected]> [151223 06:38]:
> On 12/23/2015 12:25 PM, Grygorii Strashko wrote:
>
> >>+ that services interrupts for this device
> >>+- vwlan-supply : Point the node of the regulator that controls the wl1271 chip WLAN_EN pin
> >
> >Pls, use gpio if this is required just to toggle WLAN_EN pin.
> >
> >[ti,] power-gpio or en-gpio
> >
> >
>
> The controller's power might need toggling in some board implementation.
> Maybe GPIO and regulator should be implemented?
Yes a regulator for wlcore is best here because to properly control it
and the wait needed.
Regards,
Tony
On Wed, Dec 23, 2015 at 10:35:43AM +0200, Uri Mashiach wrote:
> Add DT support for the wl1271 SPI WiFi.
>
> Add documentation file for the wl1271 SPI WiFi.
>
> Signed-off-by: Uri Mashiach <[email protected]>
> Acked-by: Igor Grinberg <[email protected]>
> ---
> .../bindings/net/wireless/ti,wlcore,spi.txt | 35 +++++++++++
For the binding:
Acked-by: Rob Herring <[email protected]>
Hi Grygorii,
On 12/23/2015 12:25 PM, Grygorii Strashko wrote:
> On 12/23/2015 10:35 AM, Uri Mashiach wrote:
>> Add DT support for the wl1271 SPI WiFi.
>>
>> Add documentation file for the wl1271 SPI WiFi.
>>
>> Signed-off-by: Uri Mashiach <[email protected]>
>> Acked-by: Igor Grinberg <[email protected]>
>> ---
>> .../bindings/net/wireless/ti,wlcore,spi.txt | 35 +++++++++++
>> drivers/net/wireless/ti/wlcore/spi.c | 67 +++++++++++++++++++---
>> 2 files changed, 95 insertions(+), 7 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
>> new file mode 100644
>> index 0000000..a3e7eb7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
>> @@ -0,0 +1,35 @@
>> +* Texas Instruments wl1271 wireless lan controller
>> +
>> +The wl1271 chip can be connected via SPI or via SDIO. This
>> +document describes the binding for the SPI connected chip.
>> +
>> +Required properties:
>> +- compatible : Should be "ti,wl1271"
>> +- reg : Chip select address of device
>> +- spi-max-frequency : Maximum SPI clocking speed of device in Hz
>> +- ref-clock-frequency : Reference clock frequency
>> +- interrupts : Should contain interrupt line and trigger type
>
> It's good to describe here number of IRQ lines your device have and IRQ types (edge/level)
>
Maybe IRQ type should be provided by dts file
(IRQ_TYPE_LEVEL_HIGH/IRQ_TYPE_EDGE_RISING...)
to enable support for additional controllers?
>> +- interrupt-parent : Should be the phandle for the interrupt controller
> interrupt-parent is not required (also interrupts-extended can be used)
>
Will be done in v2.
>> + that services interrupts for this device
>> +- vwlan-supply : Point the node of the regulator that controls the wl1271 chip WLAN_EN pin
>
> Pls, use gpio if this is required just to toggle WLAN_EN pin.
>
> [ti,] power-gpio or en-gpio
>
>
The controller's power might need toggling in some board implementation.
Maybe GPIO and regulator should be implemented?
>> +
>> +Optional properties:
>> +- clock-xtal : boolean, clock is generated from XTAL
>> +
>> +- Please consult Documentation/devicetree/bindings/spi/spi-bus.txt
>> + for optional SPI connection related properties,just
>> +
>> +Examples:
>> +
>> +&spi1 {
>> + wl1271@1 {
>> + compatible = "ti,wl1271";
>> +
>> + reg = <1>;
>> + spi-max-frequency = <48000000>;
>> + clock-xtal;
>> + interrupt-parent = <&gpio3>;
>> + interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
>> + wifi-supply = <&wifi_fixed>;
>> + };
>> +};
>> diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
>> index d3a4bcb..7281f5a 100644
>> --- a/drivers/net/wireless/ti/wlcore/spi.c
>> +++ b/drivers/net/wireless/ti/wlcore/spi.c
>> @@ -30,6 +30,7 @@
>> #include <linux/spi/spi.h>
>> #include <linux/wl12xx.h>
>> #include <linux/platform_device.h>
>> +#include <linux/of_irq.h>
>> #include <linux/regulator/consumer.h>
>>
>> #include "wlcore.h"
>> @@ -357,6 +358,54 @@ static struct wl1271_if_operations spi_ops = {
>> .set_block_size = NULL,
>> };
>>
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id wlcore_spi_of_match_table[] = {
>> + { .compatible = "ti,wl1271" },
>> + { }
>> +};
>> +
>> +/**
>> + * wlcore_probe_of - DT node parsing.
>> + * @spi: SPI slave device parameters.
>> + * @res: resource parameters.
>> + * @glue: wl12xx SPI bus to slave device glue parameters.
>> + * @pdev_data: wlcore device parameters
>> + */
>> +static int wlcore_probe_of(struct spi_device *spi, struct resource *res,
>> + struct wl12xx_spi_glue *glue,
>> + struct wlcore_platdev_data *pdev_data)
>> +{
>> + struct device_node *dt_node = spi->dev.of_node;
>> + int ret;
>> +
>> + ret = of_irq_to_resource(dt_node, 0, &res[0]);
>> + if (spi->irq != ret) {
>> + dev_err(glue->dev, "can't get interrupt resource\n");
>> + return -EINVAL;
>> + }
>
> SPI core will parse IRQ for you (see below)
>
>> +
>> + if (of_find_property(dt_node, "clock-xtal", NULL))
>> + pdev_data->ref_clock_xtal = true;
>> +
>> + ret = of_property_read_u32(dt_node, "ref-clock-frequency",
>> + &pdev_data->ref_clock_freq);
>> + if (IS_ERR_VALUE(ret)) {
>> + dev_err(glue->dev,
>> + "can't get reference clock frequency (%d)\n", ret);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +#else /* CONFIG_OF */
>> +static int wlcore_probe_of(struct spi_device *spi, struct resource *res,
>> + struct wl12xx_spi_glue *glue,
>> + struct wlcore_platdev_data *pdev_data)
>> +{
>> + return -ENODATA;
>> +}
>> +#endif /* CONFIG_OF */
>> +
>> static int wl1271_probe(struct spi_device *spi)
>> {
>> struct wl12xx_spi_glue *glue;
>> @@ -365,8 +414,7 @@ static int wl1271_probe(struct spi_device *spi)
>> int ret;
>>
>> memset(&pdev_data, 0x00, sizeof(pdev_data));
>> -
>> - /* TODO: add DT parsing when needed */
>> + memset(res, 0x00, sizeof(res));
>>
>> pdev_data.if_ops = &spi_ops;
>>
>> @@ -390,6 +438,13 @@ static int wl1271_probe(struct spi_device *spi)
>> return PTR_ERR(glue->reg);
>> }
>>
>> + ret = wlcore_probe_of(spi, &res[0], glue, &pdev_data);
>> + if (IS_ERR_VALUE(ret)) {
>> + dev_err(glue->dev,
>> + "can't get device tree parameters (%d)\n", ret);
>> + return ret;
>
> So, wl1271_probe will fail always if CONFIG_OF=n. Right?
>
>
Is a support for CONFIG_OF=n needed?
>> + }
>> +
>> ret = spi_setup(spi);
>> if (ret < 0) {
>> dev_err(glue->dev, "spi_setup failed\n");
>> @@ -404,10 +459,6 @@ static int wl1271_probe(struct spi_device *spi)
>>
>> glue->core->dev.parent = &spi->dev;
>>
>> - memset(res, 0x00, sizeof(res));
>> -
>> - res[0].start = spi->irq;
>> - res[0].flags = IORESOURCE_IRQ;
>> res[0].name = "irq";
>
> Above code might be left unchaenged.
>
>
Will be done in v2.
>>
>> ret = platform_device_add_resources(glue->core, res, ARRAY_SIZE(res));
>> @@ -445,10 +496,12 @@ static int wl1271_remove(struct spi_device *spi)
>> return 0;
>> }
>>
>> -
>> static struct spi_driver wl1271_spi_driver = {
>> .driver = {
>> .name = "wl1271_spi",
>> +#ifdef CONFIG_OF
>> + .of_match_table = of_match_ptr(wlcore_spi_of_match_table),
>> +#endif /* CONFIG_OF */
>
> These ifdefs are not needed - see of_match_ptr() definition
>
>> },
>>
>> .probe = wl1271_probe,
>>
>
>
--
Regards,
Uri
On 12/23/2015 10:35 AM, Uri Mashiach wrote:
> Add DT support for the wl1271 SPI WiFi.
>
> Add documentation file for the wl1271 SPI WiFi.
>
> Signed-off-by: Uri Mashiach <[email protected]>
> Acked-by: Igor Grinberg <[email protected]>
> ---
> .../bindings/net/wireless/ti,wlcore,spi.txt | 35 +++++++++++
> drivers/net/wireless/ti/wlcore/spi.c | 67 +++++++++++++++++++---
> 2 files changed, 95 insertions(+), 7 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
> new file mode 100644
> index 0000000..a3e7eb7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
> @@ -0,0 +1,35 @@
> +* Texas Instruments wl1271 wireless lan controller
> +
> +The wl1271 chip can be connected via SPI or via SDIO. This
> +document describes the binding for the SPI connected chip.
> +
> +Required properties:
> +- compatible : Should be "ti,wl1271"
> +- reg : Chip select address of device
> +- spi-max-frequency : Maximum SPI clocking speed of device in Hz
> +- ref-clock-frequency : Reference clock frequency
> +- interrupts : Should contain interrupt line and trigger type
It's good to describe here number of IRQ lines your device have and IRQ types (edge/level)
> +- interrupt-parent : Should be the phandle for the interrupt controller
interrupt-parent is not required (also interrupts-extended can be used)
> + that services interrupts for this device
> +- vwlan-supply : Point the node of the regulator that controls the wl1271 chip WLAN_EN pin
Pls, use gpio if this is required just to toggle WLAN_EN pin.
[ti,] power-gpio or en-gpio
> +
> +Optional properties:
> +- clock-xtal : boolean, clock is generated from XTAL
> +
> +- Please consult Documentation/devicetree/bindings/spi/spi-bus.txt
> + for optional SPI connection related properties,just
> +
> +Examples:
> +
> +&spi1 {
> + wl1271@1 {
> + compatible = "ti,wl1271";
> +
> + reg = <1>;
> + spi-max-frequency = <48000000>;
> + clock-xtal;
> + interrupt-parent = <&gpio3>;
> + interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
> + wifi-supply = <&wifi_fixed>;
> + };
> +};
> diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
> index d3a4bcb..7281f5a 100644
> --- a/drivers/net/wireless/ti/wlcore/spi.c
> +++ b/drivers/net/wireless/ti/wlcore/spi.c
> @@ -30,6 +30,7 @@
> #include <linux/spi/spi.h>
> #include <linux/wl12xx.h>
> #include <linux/platform_device.h>
> +#include <linux/of_irq.h>
> #include <linux/regulator/consumer.h>
>
> #include "wlcore.h"
> @@ -357,6 +358,54 @@ static struct wl1271_if_operations spi_ops = {
> .set_block_size = NULL,
> };
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id wlcore_spi_of_match_table[] = {
> + { .compatible = "ti,wl1271" },
> + { }
> +};
> +
> +/**
> + * wlcore_probe_of - DT node parsing.
> + * @spi: SPI slave device parameters.
> + * @res: resource parameters.
> + * @glue: wl12xx SPI bus to slave device glue parameters.
> + * @pdev_data: wlcore device parameters
> + */
> +static int wlcore_probe_of(struct spi_device *spi, struct resource *res,
> + struct wl12xx_spi_glue *glue,
> + struct wlcore_platdev_data *pdev_data)
> +{
> + struct device_node *dt_node = spi->dev.of_node;
> + int ret;
> +
> + ret = of_irq_to_resource(dt_node, 0, &res[0]);
> + if (spi->irq != ret) {
> + dev_err(glue->dev, "can't get interrupt resource\n");
> + return -EINVAL;
> + }
SPI core will parse IRQ for you (see below)
> +
> + if (of_find_property(dt_node, "clock-xtal", NULL))
> + pdev_data->ref_clock_xtal = true;
> +
> + ret = of_property_read_u32(dt_node, "ref-clock-frequency",
> + &pdev_data->ref_clock_freq);
> + if (IS_ERR_VALUE(ret)) {
> + dev_err(glue->dev,
> + "can't get reference clock frequency (%d)\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +#else /* CONFIG_OF */
> +static int wlcore_probe_of(struct spi_device *spi, struct resource *res,
> + struct wl12xx_spi_glue *glue,
> + struct wlcore_platdev_data *pdev_data)
> +{
> + return -ENODATA;
> +}
> +#endif /* CONFIG_OF */
> +
> static int wl1271_probe(struct spi_device *spi)
> {
> struct wl12xx_spi_glue *glue;
> @@ -365,8 +414,7 @@ static int wl1271_probe(struct spi_device *spi)
> int ret;
>
> memset(&pdev_data, 0x00, sizeof(pdev_data));
> -
> - /* TODO: add DT parsing when needed */
> + memset(res, 0x00, sizeof(res));
>
> pdev_data.if_ops = &spi_ops;
>
> @@ -390,6 +438,13 @@ static int wl1271_probe(struct spi_device *spi)
> return PTR_ERR(glue->reg);
> }
>
> + ret = wlcore_probe_of(spi, &res[0], glue, &pdev_data);
> + if (IS_ERR_VALUE(ret)) {
> + dev_err(glue->dev,
> + "can't get device tree parameters (%d)\n", ret);
> + return ret;
So, wl1271_probe will fail always if CONFIG_OF=n. Right?
> + }
> +
> ret = spi_setup(spi);
> if (ret < 0) {
> dev_err(glue->dev, "spi_setup failed\n");
> @@ -404,10 +459,6 @@ static int wl1271_probe(struct spi_device *spi)
>
> glue->core->dev.parent = &spi->dev;
>
> - memset(res, 0x00, sizeof(res));
> -
> - res[0].start = spi->irq;
> - res[0].flags = IORESOURCE_IRQ;
> res[0].name = "irq";
Above code might be left unchaenged.
>
> ret = platform_device_add_resources(glue->core, res, ARRAY_SIZE(res));
> @@ -445,10 +496,12 @@ static int wl1271_remove(struct spi_device *spi)
> return 0;
> }
>
> -
> static struct spi_driver wl1271_spi_driver = {
> .driver = {
> .name = "wl1271_spi",
> +#ifdef CONFIG_OF
> + .of_match_table = of_match_ptr(wlcore_spi_of_match_table),
> +#endif /* CONFIG_OF */
These ifdefs are not needed - see of_match_ptr() definition
> },
>
> .probe = wl1271_probe,
>
--
regards,
-grygorii
Device tree modifications:
- Pinmux for SPI0 and WiFi GPIOs.
- SPI0 node with wlcore as a child node.
Cc: Tony Lindgren <[email protected]>
Signed-off-by: Uri Mashiach <[email protected]>
Acked-by: Igor Grinberg <[email protected]>
---
Prerequisites:
This patch applies on Tony's omap-for-v4.5/dt branch.
arch/arm/boot/dts/am335x-cm-t335.dts | 58 +++++++++++++++++++++++++++++++++++-
1 file changed, 57 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/am335x-cm-t335.dts b/arch/arm/boot/dts/am335x-cm-t335.dts
index 42e9b66..41242a5 100644
--- a/arch/arm/boot/dts/am335x-cm-t335.dts
+++ b/arch/arm/boot/dts/am335x-cm-t335.dts
@@ -11,6 +11,7 @@
/dts-v1/;
#include "am33xx.dtsi"
+#include <dt-bindings/interrupt-controller/irq.h>
/ {
model = "CompuLab CM-T335";
@@ -40,6 +41,15 @@
regulator-max-microvolt = <3300000>;
};
+ /* Regulator for WiFi */
+ vwlan_fixed: fixedregulator@2 {
+ compatible = "regulator-fixed";
+ regulator-name = "vwlan_fixed";
+ gpio = <&gpio0 20 GPIO_ACTIVE_HIGH>; /* gpio0_20 */
+ enable-active-high;
+ regulator-boot-off;
+ };
+
backlight {
compatible = "pwm-backlight";
pwms = <&ecap0 0 50000 0>;
@@ -50,7 +60,10 @@
&am33xx_pinmux {
pinctrl-names = "default";
- pinctrl-0 = <&bluetooth_pins>;
+ pinctrl-0 = <
+ &bluetooth_pins
+ &wifi_pins
+ >;
i2c0_pins: pinmux_i2c0_pins {
pinctrl-single,pins = <
@@ -223,6 +236,21 @@
>;
};
+ spi0_pins: pinmux_spi0_pins {
+ pinctrl-single,pins = <
+ /* spi0_sclk.spi0_sclk */
+ AM33XX_IOPAD(0x950, PIN_INPUT | MUX_MODE0)
+ /* spi0_d0.spi0_d0 */
+ AM33XX_IOPAD(0x954, PIN_OUTPUT_PULLUP | MUX_MODE0)
+ /* spi0_d1.spi0_d1 */
+ AM33XX_IOPAD(0x958, PIN_INPUT | MUX_MODE0)
+ /* spi0_cs0.spi0_cs0 */
+ AM33XX_IOPAD(0x95C, PIN_OUTPUT | MUX_MODE0)
+ /* spi0_cs1.spi0_cs1 */
+ AM33XX_IOPAD(0x960, PIN_OUTPUT | MUX_MODE0)
+ >;
+ };
+
/* wl1271 bluetooth */
bluetooth_pins: pinmux_bluetooth_pins {
pinctrl-single,pins = <
@@ -230,6 +258,16 @@
AM33XX_IOPAD(0x9b0, PIN_OUTPUT_PULLUP | MUX_MODE7)
>;
};
+
+ /* wl1271 WiFi */
+ wifi_pins: pinmux_wifi_pins {
+ pinctrl-single,pins = <
+ /* EMU1.gpio3_8 - WiFi IRQ */
+ AM33XX_IOPAD(0x9e8, PIN_INPUT_PULLUP | MUX_MODE7)
+ /* XDMA_EVENT_INTR1.gpio0_20 - WiFi enable */
+ AM33XX_IOPAD(0x9b4, PIN_OUTPUT | MUX_MODE7)
+ >;
+ };
};
&uart0 {
@@ -394,3 +432,21 @@ status = "okay";
pinctrl-names = "default";
pinctrl-0 = <&mmc1_pins>;
};
+
+&spi0 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi0_pins>;
+ ti,pindir-d0-out-d1-in = <1>;
+ /* WLS1271 WiFi */
+ wlcore: wlcore@1 {
+ compatible = "ti,wl1271";
+ reg = <1>;
+ spi-max-frequency = <48000000>;
+ clock-xtal;
+ ref-clock-frequency = <38400000>;
+ interrupt-parent = <&gpio3>;
+ interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
+ vwlan-supply = <&vwlan_fixed>;
+ };
+};
--
2.5.0