Re: [PATCH 4/4] wl1251: spi: add device tree support
On Oct 28, 2013, at 5:38 PM, Tomasz Figa wrote: On Monday 28 of October 2013 01:37:34 Kumar Gala wrote: On Oct 27, 2013, at 11:14 AM, Sebastian Reichel wrote: Add device tree support for the spi variant of wl1251 and document the binding. Signed-off-by: Sebastian Reichel s...@debian.org --- .../devicetree/bindings/net/wireless/ti,wl1251.txt | 36 ++ drivers/net/wireless/ti/wl1251/spi.c | 23 ++ 2 files changed, 53 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt new file mode 100644 index 000..5f8a154 --- /dev/null +++ b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt @@ -0,0 +1,36 @@ +* Texas Instruments wl1251 controller + +The wl1251 chip can be connected via SPI or via SDIO. The linux +kernel currently only supports device tree for the SPI variant. + From the binding I have no idea what this chip actually does, also we don't normally reference linux kernel support in bindings specs (so please remove it). However, what would expect the SDIO binding to look like? Or more specifically, how would you distinguish the SPI vs SDIO binding/connection? I'm wondering if the compatible should be something like ti,wl1251-spi and than the sdio can be ti,wl1251-sdio Well, you can easily distinguish an SDIO device from an SPI device by its parent node, but... The binding for SDIO might require different set of properties (other than ones inherited from generic SDIO or SPI bindings) than one for SPI. So probably different compatible values might be justified. Did we already have such case before? (maybe some I2C + SPI devices?) +Required properties: +- compatible : Should be ti,wl1251 reg is not listed as a required prop. It is implied by SPI bindings, but it might be a good idea to have this stated here as well. +- interrupts : Should contain interrupt line +- interrupt-parent : Should be the phandle for the interrupt + controller that services interrupts for this device +- vio-supply : phandle to regulator providing VIO +- power-gpio : GPIO connected to chip's PMEN pin should be vendor prefixed: ti,power-gpio Hmm, out of curiosity, is it a rule for this kind of properties? I can see both cases with and without prefixes when grepping for -gpios in Documentation/devicetree/bindings. We should really have such things written down somewhere. Agreed, it should be part of the various docs we are suppose to produce for review and binding creation guidelines. +- For additional required properties on SPI, please consult + Documentation/devicetree/bindings/spi/spi-bus.txt + +Optional properties: +- ti,use-eeprom : If found, configuration will be loaded from eeprom. can you be a bit more specific on what cfg will be loaded. Also, is this property a boolean, if so how do I know which eeprom the cfg is loaded from (is it one that is directly connected to the wl1251? Maybe one from ti,has-eeprom or ti,config-eeprom would be better name for this property? Probably, ti,wl1251-has-eeprom or something like that would be better. However, I'm not going to get too caught up on names of properties. - k -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] wl1251: spi: add device tree support
On Oct 27, 2013, at 11:14 AM, Sebastian Reichel wrote:
Add device tree support for the spi variant of wl1251
and document the binding.
Signed-off-by: Sebastian Reichel s...@debian.org
---
.../devicetree/bindings/net/wireless/ti,wl1251.txt | 36 ++
drivers/net/wireless/ti/wl1251/spi.c | 23 ++
2 files changed, 53 insertions(+), 6 deletions(-)
create mode 100644
Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
new file mode 100644
index 000..5f8a154
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
@@ -0,0 +1,36 @@
+* Texas Instruments wl1251 controller
+
+The wl1251 chip can be connected via SPI or via SDIO. The linux
+kernel currently only supports device tree for the SPI variant.
+
From the binding I have no idea what this chip actually does, also we don't
normally reference linux kernel support in bindings specs (so please remove
it).
However, what would expect the SDIO binding to look like? Or more
specifically, how would you distinguish the SPI vs SDIO binding/connection?
I'm wondering if the compatible should be something like ti,wl1251-spi and
than the sdio can be ti,wl1251-sdio
+Required properties:
+- compatible : Should be ti,wl1251
reg is not listed as a required prop.
+- interrupts : Should contain interrupt line
+- interrupt-parent : Should be the phandle for the interrupt
+ controller that services interrupts for this device
+- vio-supply : phandle to regulator providing VIO
+- power-gpio : GPIO connected to chip's PMEN pin
should be vendor prefixed: ti,power-gpio
+- For additional required properties on SPI, please consult
+ Documentation/devicetree/bindings/spi/spi-bus.txt
+
+Optional properties:
+- ti,use-eeprom : If found, configuration will be loaded from eeprom.
can you be a bit more specific on what cfg will be loaded. Also, is this
property a boolean, if so how do I know which eeprom the cfg is loaded from (is
it one that is directly connected to the wl1251?
+
+Examples:
+
+spi1 {
+ wl1251_spi@0 {
+ compatible = ti,wl1251;
+
+ reg = 0;
+ spi-max-frequency = 4800;
+ spi-cpol;
+ spi-cpha;
+
+ interrupt-parent = gpio2;
+ interrupts = 10 IRQ_TYPE_NONE; /* gpio line 42 */
+
+ vio-supply = vio;
+ power-gpio = gpio3 23 GPIO_ACTIVE_HIGH; /* 87 */
+ };
+};
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] wl1251: spi: add device tree support
On Mon, Oct 28, 2013 at 8:37 AM, Kumar Gala ga...@codeaurora.org wrote: On Oct 27, 2013, at 11:14 AM, Sebastian Reichel wrote: +++ b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt @@ -0,0 +1,36 @@ +* Texas Instruments wl1251 controller + +The wl1251 chip can be connected via SPI or via SDIO. The linux +kernel currently only supports device tree for the SPI variant. + From the binding I have no idea what this chip actually does, also we don't normally reference linux kernel support in bindings specs (so please remove it). However, what would expect the SDIO binding to look like? Or more specifically, how would you distinguish the SPI vs SDIO binding/connection? I'm wondering if the compatible should be something like ti,wl1251-spi and than the sdio can be ti,wl1251-sdio When connected to SDIO, it doesn't act as standard SDIO device and can't be probed (standard SDIO registers missing), so information has to come some other way. That used to partially come through platform_data and partially through a callback from mmc subsystem (see pandora_wl1251_init_card() in arch/arm/mach-omap2/board-omap3pandora.c). I don't know much about DT, but maybe the information that comes from SDIO registers on normal SDIO devices should come through DT in this case too? I don't really know how that should be integrated with mmc subsystem though.. +Required properties: +- compatible : Should be ti,wl1251 reg is not listed as a required prop. +- interrupts : Should contain interrupt line +- interrupt-parent : Should be the phandle for the interrupt + controller that services interrupts for this device +- vio-supply : phandle to regulator providing VIO +- power-gpio : GPIO connected to chip's PMEN pin should be vendor prefixed: ti,power-gpio +- For additional required properties on SPI, please consult + Documentation/devicetree/bindings/spi/spi-bus.txt + +Optional properties: +- ti,use-eeprom : If found, configuration will be loaded from eeprom. can you be a bit more specific on what cfg will be loaded. Also, is this property a boolean, if so how do I know which eeprom the cfg is loaded from (is it one that is directly connected to the wl1251? wl1251 is a wifi chip that can have an optional eeprom connected to it to store things like calibration stuff and MAC address, and that eeprom is usually inside a single module with some additional radio related chips. If the eeprom is connected (like the module on pandora board has), the driver can issue command to the firmware running on chip to load that data on it's startup, alternatively the driver can load calibration from other storage (like it happens on N900). Gražvydas -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] wl1251: spi: add device tree support
On Oct 28, 2013, at 12:15 PM, Grazvydas Ignotas wrote: On Mon, Oct 28, 2013 at 8:37 AM, Kumar Gala ga...@codeaurora.org wrote: On Oct 27, 2013, at 11:14 AM, Sebastian Reichel wrote: +++ b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt @@ -0,0 +1,36 @@ +* Texas Instruments wl1251 controller + +The wl1251 chip can be connected via SPI or via SDIO. The linux +kernel currently only supports device tree for the SPI variant. + From the binding I have no idea what this chip actually does, also we don't normally reference linux kernel support in bindings specs (so please remove it). However, what would expect the SDIO binding to look like? Or more specifically, how would you distinguish the SPI vs SDIO binding/connection? I'm wondering if the compatible should be something like ti,wl1251-spi and than the sdio can be ti,wl1251-sdio When connected to SDIO, it doesn't act as standard SDIO device and can't be probed (standard SDIO registers missing), so information has to come some other way. That used to partially come through platform_data and partially through a callback from mmc subsystem (see pandora_wl1251_init_card() in arch/arm/mach-omap2/board-omap3pandora.c). I don't know much about DT, but maybe the information that comes from SDIO registers on normal SDIO devices should come through DT in this case too? I don't really know how that should be integrated with mmc subsystem though.. Ok, my point is still valid that we can use a different compatible for the SDIO case even if its no standard SDIO vs the SPI case. +Required properties: +- compatible : Should be ti,wl1251 reg is not listed as a required prop. +- interrupts : Should contain interrupt line +- interrupt-parent : Should be the phandle for the interrupt + controller that services interrupts for this device +- vio-supply : phandle to regulator providing VIO +- power-gpio : GPIO connected to chip's PMEN pin should be vendor prefixed: ti,power-gpio +- For additional required properties on SPI, please consult + Documentation/devicetree/bindings/spi/spi-bus.txt + +Optional properties: +- ti,use-eeprom : If found, configuration will be loaded from eeprom. can you be a bit more specific on what cfg will be loaded. Also, is this property a boolean, if so how do I know which eeprom the cfg is loaded from (is it one that is directly connected to the wl1251? wl1251 is a wifi chip that can have an optional eeprom connected to it to store things like calibration stuff and MAC address, and that eeprom is usually inside a single module with some additional radio related chips. If the eeprom is connected (like the module on pandora board has), the driver can issue command to the firmware running on chip to load that data on it's startup, alternatively the driver can load calibration from other storage (like it happens on N900). So sounds like a boolean. I think just adding that its a boolean and maybe something like configuration (calibration data, MAC addr, etc..) - k Gražvydas -- To unsubscribe from this list: send the line unsubscribe devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] wl1251: spi: add device tree support
On Monday 28 of October 2013 01:37:34 Kumar Gala wrote: On Oct 27, 2013, at 11:14 AM, Sebastian Reichel wrote: Add device tree support for the spi variant of wl1251 and document the binding. Signed-off-by: Sebastian Reichel s...@debian.org --- .../devicetree/bindings/net/wireless/ti,wl1251.txt | 36 ++ drivers/net/wireless/ti/wl1251/spi.c | 23 ++ 2 files changed, 53 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt new file mode 100644 index 000..5f8a154 --- /dev/null +++ b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt @@ -0,0 +1,36 @@ +* Texas Instruments wl1251 controller + +The wl1251 chip can be connected via SPI or via SDIO. The linux +kernel currently only supports device tree for the SPI variant. + From the binding I have no idea what this chip actually does, also we don't normally reference linux kernel support in bindings specs (so please remove it). However, what would expect the SDIO binding to look like? Or more specifically, how would you distinguish the SPI vs SDIO binding/connection? I'm wondering if the compatible should be something like ti,wl1251-spi and than the sdio can be ti,wl1251-sdio Well, you can easily distinguish an SDIO device from an SPI device by its parent node, but... The binding for SDIO might require different set of properties (other than ones inherited from generic SDIO or SPI bindings) than one for SPI. So probably different compatible values might be justified. Did we already have such case before? (maybe some I2C + SPI devices?) +Required properties: +- compatible : Should be ti,wl1251 reg is not listed as a required prop. It is implied by SPI bindings, but it might be a good idea to have this stated here as well. +- interrupts : Should contain interrupt line +- interrupt-parent : Should be the phandle for the interrupt + controller that services interrupts for this device +- vio-supply : phandle to regulator providing VIO +- power-gpio : GPIO connected to chip's PMEN pin should be vendor prefixed: ti,power-gpio Hmm, out of curiosity, is it a rule for this kind of properties? I can see both cases with and without prefixes when grepping for -gpios in Documentation/devicetree/bindings. We should really have such things written down somewhere. +- For additional required properties on SPI, please consult + Documentation/devicetree/bindings/spi/spi-bus.txt + +Optional properties: +- ti,use-eeprom : If found, configuration will be loaded from eeprom. can you be a bit more specific on what cfg will be loaded. Also, is this property a boolean, if so how do I know which eeprom the cfg is loaded from (is it one that is directly connected to the wl1251? Maybe one from ti,has-eeprom or ti,config-eeprom would be better name for this property? Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] wl1251: spi: add device tree support
Add device tree support for the spi variant of wl1251
and document the binding.
Signed-off-by: Sebastian Reichel s...@debian.org
---
.../devicetree/bindings/net/wireless/ti,wl1251.txt | 36 ++
drivers/net/wireless/ti/wl1251/spi.c | 23 ++
2 files changed, 53 insertions(+), 6 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
new file mode 100644
index 000..5f8a154
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
@@ -0,0 +1,36 @@
+* Texas Instruments wl1251 controller
+
+The wl1251 chip can be connected via SPI or via SDIO. The linux
+kernel currently only supports device tree for the SPI variant.
+
+Required properties:
+- compatible : Should be ti,wl1251
+- interrupts : Should contain interrupt line
+- interrupt-parent : Should be the phandle for the interrupt
+ controller that services interrupts for this device
+- vio-supply : phandle to regulator providing VIO
+- power-gpio : GPIO connected to chip's PMEN pin
+- For additional required properties on SPI, please consult
+ Documentation/devicetree/bindings/spi/spi-bus.txt
+
+Optional properties:
+- ti,use-eeprom : If found, configuration will be loaded from eeprom.
+
+Examples:
+
+spi1 {
+ wl1251_spi@0 {
+ compatible = ti,wl1251;
+
+ reg = 0;
+ spi-max-frequency = 4800;
+ spi-cpol;
+ spi-cpha;
+
+ interrupt-parent = gpio2;
+ interrupts = 10 IRQ_TYPE_NONE; /* gpio line 42 */
+
+ vio-supply = vio;
+ power-gpio = gpio3 23 GPIO_ACTIVE_HIGH; /* 87 */
+ };
+};
diff --git a/drivers/net/wireless/ti/wl1251/spi.c
b/drivers/net/wireless/ti/wl1251/spi.c
index efea57a..ee6ce4c 100644
--- a/drivers/net/wireless/ti/wl1251/spi.c
+++ b/drivers/net/wireless/ti/wl1251/spi.c
@@ -27,6 +27,8 @@
#include linux/spi/spi.h
#include linux/wl12xx.h
#include linux/gpio.h
+#include linux/of.h
+#include linux/of_gpio.h
#include linux/regulator/consumer.h
#include wl1251.h
@@ -240,13 +242,13 @@ static const struct wl1251_if_operations wl1251_spi_ops =
{
static int wl1251_spi_probe(struct spi_device *spi)
{
- struct wl1251_platform_data *pdata;
+ struct wl1251_platform_data *pdata = spi-dev.platform_data;
+ struct device_node *np = spi-dev.of_node;
struct ieee80211_hw *hw;
struct wl1251 *wl;
int ret;
- pdata = spi-dev.platform_data;
- if (!pdata) {
+ if (!np !pdata) {
wl1251_error(no platform data);
return -ENODEV;
}
@@ -273,7 +275,18 @@ static int wl1251_spi_probe(struct spi_device *spi)
goto out_free;
}
- wl-power_gpio = pdata-power_gpio;
+ if (np) {
+ wl-use_eeprom = of_property_read_bool(np, ti,use-eeprom);
+ wl-power_gpio = of_get_named_gpio(np, power-gpio, 0);
+ } else if (pdata) {
+ wl-power_gpio = pdata-power_gpio;
+ wl-use_eeprom = pdata-use_eeprom;
+ }
+
+ if (wl-power_gpio == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto out_free;
+ }
if (gpio_is_valid(wl-power_gpio)) {
ret = devm_gpio_request_one(spi-dev, wl-power_gpio,
@@ -295,8 +308,6 @@ static int wl1251_spi_probe(struct spi_device *spi)
goto out_free;
}
- wl-use_eeprom = pdata-use_eeprom;
-
irq_set_status_flags(wl-irq, IRQ_NOAUTOEN);
ret = devm_request_irq(spi-dev, wl-irq, wl1251_irq, 0,
DRIVER_NAME, wl);
--
1.8.4.rc3
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
