2015-12-24 15:36:01

by Uri Mashiach

[permalink] [raw]
Subject: [PATCH v2 0/3] wlcore/wl12xx: spi: add wifi support to cm-t335

Add DT support for WLS1271 SPI driver.
Add power operation function to the WLS1271 SPI driver.

Uri Mashiach (3):
wlcore/wl12xx: spi: add power operation function
wlcore/wl12xx: spi: add device tree support
wlcore/wl12xx: spi: add wifi support to cm-t335

.../bindings/net/wireless/ti,wlcore,spi.txt | 34 ++++++++
arch/arm/boot/dts/am335x-cm-t335.dts | 57 +++++++++++++-
drivers/net/wireless/ti/wlcore/spi.c | 92 +++++++++++++++++++++-
3 files changed, 178 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt

--
2.5.0



2015-12-24 16:33:23

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] wlcore/wl12xx: spi: add wifi support to cm-t335

Hello Uri,

On Thu, Dec 24, 2015 at 12:35 PM, Uri Mashiach
<[email protected]> wrote:
> 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]>
> ---
> v1 -> v2: replace interrupts and interrupt-parent with interrupts-extended.
>
> arch/arm/boot/dts/am335x-cm-t335.dts | 57 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 56 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..31f8371 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
> + >;

The pinctrl lines should be in the device node that needs the pin
muxing (unless there isn't a device node) so I think is better if you
mofe the pinctrl-0 = <&wifi_pins> to the wlcore dev node.

Best regards,
Javier

2015-12-28 12:01:23

by Grygorii Strashko

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] wlcore/wl12xx: spi: add power operation function

On 12/27/2015 07:14 PM, Uri Mashiach wrote:
> Hello Grygorii,
>
> On 12/24/2015 06:32 PM, Grygorii Strashko wrote:
>> On 12/24/2015 05:35 PM, Uri Mashiach wrote:
>>> The power function uses a consumer regulator access to update the WiFi
>>> enable GPIO value.
>>>
>>> Signed-off-by: Uri Mashiach <[email protected]>
>>> ---
>>> v1 -> v2: oops fix was removed to a separate fix.
>>>
>>> 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
>>>
>>
>> [...]
>>
>>> +
>>> 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)) {
>>
>> It will be more correct to handle -EPROBE_DEFER here also. Like:
>> if (PTR_ERR(glue->reg) == -EPROBE_DEFER)
>> return PTR_ERR(glue->reg);
>>
>
> It seems that the IS_ERR(glue->reg) condition covers the EPROBE_DEFER case.

True. But this is not an error and it's common practice do not print
any log messages in this case from driver :)

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


--
regards,
-grygorii

2015-12-24 17:21:58

by Uri Mashiach

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] wlcore/wl12xx: spi: add device tree support

Hello Javier,

On 12/24/2015 06:25 PM, Javier Martinez Canillas wrote:
> Hello Uri,
>
> On Thu, Dec 24, 2015 at 12:35 PM, Uri Mashiach
> <[email protected]> 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]>
>> ---
>> v1 -> v2: update interrupt documentation.
>> replace interrupts and interrupt-parent with interrupts-extended.
>> IRQ parameters retrieved from spi_device instead of DT.
>> remove redundant #ifdef CONFIG_OF
>>
>> .../bindings/net/wireless/ti,wlcore,spi.txt | 34 +++++++++++++
>> drivers/net/wireless/ti/wlcore/spi.c | 55 ++++++++++++++++++++--
>> 2 files changed, 85 insertions(+), 4 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..502c27e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
>> @@ -0,0 +1,34 @@
>> +* 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-extended : Should contain parameters for 1 interrupt line.
>> + Interrupt parameters: parent, line number, type.
>> +- vwlan-supply : Point the node of the regulator that powers/enable the wl1271 chip
>> +
>> +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;
>> + ref-clock-frequency = <38400000>;
>> + interrupts-extended = <&gpio3 8 IRQ_TYPE_LEVEL_HIGH>;
>> + vwlan-supply = <&vwlan_fixed>;
>> + };
>> +};
>> diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
>> index d3a4bcb..e9e8d54 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,46 @@ 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" },
>> + { }
>> +};
>
> Could you please add a MODULE_DEVICE_TABLE(of, wlcore_spi_of_match_table) here?
>

Will be added in v3.

>> +
>> +/**
>> + * 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 wl12xx_spi_glue *glue,
>> + struct wlcore_platdev_data *pdev_data)
>> +{
>> + struct device_node *dt_node = spi->dev.of_node;
>> + int ret;
>> +
>> + 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 wl12xx_spi_glue *glue,
>> + struct wlcore_platdev_data *pdev_data)
>> +{
>> + return -ENODATA;
>> +}
>> +#endif /* CONFIG_OF */
>> +
>
> I don't understand what's the point of making the driver to be built
> with !CONFIG_OF if the probe function is going to fail anyways. If
> this driver really need then is better to remove the ifdefery and make
> the Kconfig symbol to depend on OF.
>

Will be done in v3.

> Best regards,
> Javier
>

Thanks,
Uri

2015-12-24 15:36:10

by Uri Mashiach

[permalink] [raw]
Subject: [PATCH v2 2/3] wlcore/wl12xx: spi: add device tree support

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]>
---
v1 -> v2: update interrupt documentation.
replace interrupts and interrupt-parent with interrupts-extended.
IRQ parameters retrieved from spi_device instead of DT.
remove redundant #ifdef CONFIG_OF

.../bindings/net/wireless/ti,wlcore,spi.txt | 34 +++++++++++++
drivers/net/wireless/ti/wlcore/spi.c | 55 ++++++++++++++++++++--
2 files changed, 85 insertions(+), 4 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..502c27e
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
@@ -0,0 +1,34 @@
+* 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-extended : Should contain parameters for 1 interrupt line.
+ Interrupt parameters: parent, line number, type.
+- vwlan-supply : Point the node of the regulator that powers/enable the wl1271 chip
+
+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;
+ ref-clock-frequency = <38400000>;
+ interrupts-extended = <&gpio3 8 IRQ_TYPE_LEVEL_HIGH>;
+ vwlan-supply = <&vwlan_fixed>;
+ };
+};
diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
index d3a4bcb..e9e8d54 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,46 @@ 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 wl12xx_spi_glue *glue,
+ struct wlcore_platdev_data *pdev_data)
+{
+ struct device_node *dt_node = spi->dev.of_node;
+ int ret;
+
+ 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 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;
@@ -366,8 +407,6 @@ static int wl1271_probe(struct spi_device *spi)

memset(&pdev_data, 0x00, sizeof(pdev_data));

- /* TODO: add DT parsing when needed */
-
pdev_data.if_ops = &spi_ops;

glue = devm_kzalloc(&spi->dev, sizeof(*glue), GFP_KERNEL);
@@ -390,6 +429,13 @@ static int wl1271_probe(struct spi_device *spi)
return PTR_ERR(glue->reg);
}

+ ret = wlcore_probe_of(spi, 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");
@@ -407,7 +453,8 @@ static int wl1271_probe(struct spi_device *spi)
memset(res, 0x00, sizeof(res));

res[0].start = spi->irq;
- res[0].flags = IORESOURCE_IRQ;
+ res[0].flags = IORESOURCE_IRQ |
+ irqd_get_trigger_type(irq_get_irq_data(spi->irq));
res[0].name = "irq";

ret = platform_device_add_resources(glue->core, res, ARRAY_SIZE(res));
@@ -445,10 +492,10 @@ static int wl1271_remove(struct spi_device *spi)
return 0;
}

-
static struct spi_driver wl1271_spi_driver = {
.driver = {
.name = "wl1271_spi",
+ .of_match_table = of_match_ptr(wlcore_spi_of_match_table),
},

.probe = wl1271_probe,
--
2.5.0


2015-12-24 16:34:51

by Grygorii Strashko

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] wlcore/wl12xx: spi: add power operation function

On 12/24/2015 05:35 PM, Uri Mashiach wrote:
> The power function uses a consumer regulator access to update the WiFi
> enable GPIO value.
>
> Signed-off-by: Uri Mashiach <[email protected]>
> ---
> v1 -> v2: oops fix was removed to a separate fix.
>
> 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
>

[...]

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

It will be more correct to handle -EPROBE_DEFER here also. Like:
if (PTR_ERR(glue->reg) == -EPROBE_DEFER)
return PTR_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");
>


--
regards,
-grygorii

2015-12-27 17:14:42

by Uri Mashiach

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] wlcore/wl12xx: spi: add power operation function

Hello Grygorii,

On 12/24/2015 06:32 PM, Grygorii Strashko wrote:
> On 12/24/2015 05:35 PM, Uri Mashiach wrote:
>> The power function uses a consumer regulator access to update the WiFi
>> enable GPIO value.
>>
>> Signed-off-by: Uri Mashiach <[email protected]>
>> ---
>> v1 -> v2: oops fix was removed to a separate fix.
>>
>> 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
>>
>
> [...]
>
>> +
>> 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)) {
>
> It will be more correct to handle -EPROBE_DEFER here also. Like:
> if (PTR_ERR(glue->reg) == -EPROBE_DEFER)
> return PTR_ERR(glue->reg);
>

It seems that the IS_ERR(glue->reg) condition covers the EPROBE_DEFER case.

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

--
Thanks,
Uri

2015-12-24 15:36:06

by Uri Mashiach

[permalink] [raw]
Subject: [PATCH v2 1/3] wlcore/wl12xx: spi: add power operation function

The power function uses a consumer regulator access to update the WiFi
enable GPIO value.

Signed-off-by: Uri Mashiach <[email protected]>
---
v1 -> v2: oops fix was removed to a separate fix.

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


2015-12-28 14:16:15

by Uri Mashiach

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] wlcore/wl12xx: spi: add power operation function

Hi Grygorii,

On 12/28/2015 01:59 PM, Grygorii Strashko wrote:
> On 12/27/2015 07:14 PM, Uri Mashiach wrote:
>> Hello Grygorii,
>>
>> On 12/24/2015 06:32 PM, Grygorii Strashko wrote:
>>> On 12/24/2015 05:35 PM, Uri Mashiach wrote:
>>>> The power function uses a consumer regulator access to update the WiFi
>>>> enable GPIO value.
>>>>
>>>> Signed-off-by: Uri Mashiach <[email protected]>
>>>> ---
>>>> v1 -> v2: oops fix was removed to a separate fix.
>>>>
>>>> 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
>>>>
>>>
>>> [...]
>>>
>>>> +
>>>> 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)) {
>>>
>>> It will be more correct to handle -EPROBE_DEFER here also. Like:
>>> if (PTR_ERR(glue->reg) == -EPROBE_DEFER)
>>> return PTR_ERR(glue->reg);
>>>
>>
>> It seems that the IS_ERR(glue->reg) condition covers the EPROBE_DEFER case.
>
> True. But this is not an error and it's common practice do not print
> any log messages in this case from driver :)
>

Sorry, already posted v3.
Will be in v4.

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

--
Thanks,
Uri

2015-12-24 15:36:12

by Uri Mashiach

[permalink] [raw]
Subject: [PATCH v2 3/3] wlcore/wl12xx: spi: add wifi support to cm-t335

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]>
---
v1 -> v2: replace interrupts and interrupt-parent with interrupts-extended.

arch/arm/boot/dts/am335x-cm-t335.dts | 57 +++++++++++++++++++++++++++++++++++-
1 file changed, 56 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..31f8371 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,20 @@ 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>;
+ interrupts-extended = <&gpio3 8 IRQ_TYPE_LEVEL_HIGH>;
+ vwlan-supply = <&vwlan_fixed>;
+ };
+};
--
2.5.0


2015-12-24 16:25:43

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] wlcore/wl12xx: spi: add device tree support

Hello Uri,

On Thu, Dec 24, 2015 at 12:35 PM, Uri Mashiach
<[email protected]> 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]>
> ---
> v1 -> v2: update interrupt documentation.
> replace interrupts and interrupt-parent with interrupts-extended.
> IRQ parameters retrieved from spi_device instead of DT.
> remove redundant #ifdef CONFIG_OF
>
> .../bindings/net/wireless/ti,wlcore,spi.txt | 34 +++++++++++++
> drivers/net/wireless/ti/wlcore/spi.c | 55 ++++++++++++++++++++--
> 2 files changed, 85 insertions(+), 4 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..502c27e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
> @@ -0,0 +1,34 @@
> +* 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-extended : Should contain parameters for 1 interrupt line.
> + Interrupt parameters: parent, line number, type.
> +- vwlan-supply : Point the node of the regulator that powers/enable the wl1271 chip
> +
> +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;
> + ref-clock-frequency = <38400000>;
> + interrupts-extended = <&gpio3 8 IRQ_TYPE_LEVEL_HIGH>;
> + vwlan-supply = <&vwlan_fixed>;
> + };
> +};
> diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
> index d3a4bcb..e9e8d54 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,46 @@ 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" },
> + { }
> +};

Could you please add a MODULE_DEVICE_TABLE(of, wlcore_spi_of_match_table) here?

> +
> +/**
> + * 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 wl12xx_spi_glue *glue,
> + struct wlcore_platdev_data *pdev_data)
> +{
> + struct device_node *dt_node = spi->dev.of_node;
> + int ret;
> +
> + 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 wl12xx_spi_glue *glue,
> + struct wlcore_platdev_data *pdev_data)
> +{
> + return -ENODATA;
> +}
> +#endif /* CONFIG_OF */
> +

I don't understand what's the point of making the driver to be built
with !CONFIG_OF if the probe function is going to fail anyways. If
this driver really need then is better to remove the ifdefery and make
the Kconfig symbol to depend on OF.

Best regards,
Javier

2015-12-24 16:34:18

by Grygorii Strashko

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] wlcore/wl12xx: spi: add device tree support

On 12/24/2015 05:35 PM, 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]>
> ---
> v1 -> v2: update interrupt documentation.
> replace interrupts and interrupt-parent with interrupts-extended.
> IRQ parameters retrieved from spi_device instead of DT.
> remove redundant #ifdef CONFIG_OF
>
> .../bindings/net/wireless/ti,wlcore,spi.txt | 34 +++++++++++++
> drivers/net/wireless/ti/wlcore/spi.c | 55 ++++++++++++++++++++--
> 2 files changed, 85 insertions(+), 4 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..502c27e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
> @@ -0,0 +1,34 @@
> +* 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-extended : Should contain parameters for 1 interrupt line.
> + Interrupt parameters: parent, line number, type.
> +- vwlan-supply : Point the node of the regulator that powers/enable the wl1271 chip
> +
> +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;
> + ref-clock-frequency = <38400000>;
> + interrupts-extended = <&gpio3 8 IRQ_TYPE_LEVEL_HIGH>;
> + vwlan-supply = <&vwlan_fixed>;
> + };
> +};
> diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
> index d3a4bcb..e9e8d54 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,46 @@ 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 wl12xx_spi_glue *glue,
> + struct wlcore_platdev_data *pdev_data)
> +{
> + struct device_node *dt_node = spi->dev.of_node;
> + int ret;
> +
> + 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 wl12xx_spi_glue *glue,
> + struct wlcore_platdev_data *pdev_data)
> +{
> + return -ENODATA;
> +}
> +#endif /* CONFIG_OF */

My question for your v1 was related to all above ifdefs.
If CONFIG_OF=n is not going to be supported then all this ifdefs
can be dropped and proper dependency can be added to Kconfig instead.

> +
> static int wl1271_probe(struct spi_device *spi)
> {
> struct wl12xx_spi_glue *glue;
> @@ -366,8 +407,6 @@ static int wl1271_probe(struct spi_device *spi)
>
> memset(&pdev_data, 0x00, sizeof(pdev_data));
>
> - /* TODO: add DT parsing when needed */
> -
> pdev_data.if_ops = &spi_ops;
>
> glue = devm_kzalloc(&spi->dev, sizeof(*glue), GFP_KERNEL);
> @@ -390,6 +429,13 @@ static int wl1271_probe(struct spi_device *spi)
> return PTR_ERR(glue->reg);
> }
>
> + ret = wlcore_probe_of(spi, 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");
> @@ -407,7 +453,8 @@ static int wl1271_probe(struct spi_device *spi)
> memset(res, 0x00, sizeof(res));
>
> res[0].start = spi->irq;
> - res[0].flags = IORESOURCE_IRQ;
> + res[0].flags = IORESOURCE_IRQ |
> + irqd_get_trigger_type(irq_get_irq_data(spi->irq));


irq_get_trigger_type()?

> res[0].name = "irq";
>
> ret = platform_device_add_resources(glue->core, res, ARRAY_SIZE(res));
> @@ -445,10 +492,10 @@ static int wl1271_remove(struct spi_device *spi)
> return 0;
> }
>
> -
> static struct spi_driver wl1271_spi_driver = {
> .driver = {
> .name = "wl1271_spi",
> + .of_match_table = of_match_ptr(wlcore_spi_of_match_table),
> },
>
> .probe = wl1271_probe,
>

Sorry for delayed replay.
--
regards,
-grygorii

2015-12-24 17:24:21

by Uri Mashiach

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] wlcore/wl12xx: spi: add device tree support

Hi Grygorii,

On 12/24/2015 06:32 PM, Grygorii Strashko wrote:
> On 12/24/2015 05:35 PM, 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]>
>> ---
>> v1 -> v2: update interrupt documentation.
>> replace interrupts and interrupt-parent with interrupts-extended.
>> IRQ parameters retrieved from spi_device instead of DT.
>> remove redundant #ifdef CONFIG_OF
>>
>> .../bindings/net/wireless/ti,wlcore,spi.txt | 34 +++++++++++++
>> drivers/net/wireless/ti/wlcore/spi.c | 55 ++++++++++++++++++++--
>> 2 files changed, 85 insertions(+), 4 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..502c27e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
>> @@ -0,0 +1,34 @@
>> +* 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-extended : Should contain parameters for 1 interrupt line.
>> + Interrupt parameters: parent, line number, type.
>> +- vwlan-supply : Point the node of the regulator that powers/enable the wl1271 chip
>> +
>> +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;
>> + ref-clock-frequency = <38400000>;
>> + interrupts-extended = <&gpio3 8 IRQ_TYPE_LEVEL_HIGH>;
>> + vwlan-supply = <&vwlan_fixed>;
>> + };
>> +};
>> diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
>> index d3a4bcb..e9e8d54 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,46 @@ 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 wl12xx_spi_glue *glue,
>> + struct wlcore_platdev_data *pdev_data)
>> +{
>> + struct device_node *dt_node = spi->dev.of_node;
>> + int ret;
>> +
>> + 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 wl12xx_spi_glue *glue,
>> + struct wlcore_platdev_data *pdev_data)
>> +{
>> + return -ENODATA;
>> +}
>> +#endif /* CONFIG_OF */
>
> My question for your v1 was related to all above ifdefs.
> If CONFIG_OF=n is not going to be supported then all this ifdefs
> can be dropped and proper dependency can be added to Kconfig instead.
>

Will be done in v3.

>> +
>> static int wl1271_probe(struct spi_device *spi)
>> {
>> struct wl12xx_spi_glue *glue;
>> @@ -366,8 +407,6 @@ static int wl1271_probe(struct spi_device *spi)
>>
>> memset(&pdev_data, 0x00, sizeof(pdev_data));
>>
>> - /* TODO: add DT parsing when needed */
>> -
>> pdev_data.if_ops = &spi_ops;
>>
>> glue = devm_kzalloc(&spi->dev, sizeof(*glue), GFP_KERNEL);
>> @@ -390,6 +429,13 @@ static int wl1271_probe(struct spi_device *spi)
>> return PTR_ERR(glue->reg);
>> }
>>
>> + ret = wlcore_probe_of(spi, 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");
>> @@ -407,7 +453,8 @@ static int wl1271_probe(struct spi_device *spi)
>> memset(res, 0x00, sizeof(res));
>>
>> res[0].start = spi->irq;
>> - res[0].flags = IORESOURCE_IRQ;
>> + res[0].flags = IORESOURCE_IRQ |
>> + irqd_get_trigger_type(irq_get_irq_data(spi->irq));
>
>
> irq_get_trigger_type()?
>

Will be replaced in v3.

>> res[0].name = "irq";
>>
>> ret = platform_device_add_resources(glue->core, res, ARRAY_SIZE(res));
>> @@ -445,10 +492,10 @@ static int wl1271_remove(struct spi_device *spi)
>> return 0;
>> }
>>
>> -
>> static struct spi_driver wl1271_spi_driver = {
>> .driver = {
>> .name = "wl1271_spi",
>> + .of_match_table = of_match_ptr(wlcore_spi_of_match_table),
>> },
>>
>> .probe = wl1271_probe,
>>
>
> Sorry for delayed replay.
>

--
Thanks,
Uri