This adds device tree support for rfkill-gpio. The optional platform
paramters gpio_runtime_close and gpio_runtime_setup are not implemented.
Cc: [email protected]
Cc: "John W. Linville" <[email protected]>
Cc: Johannes Berg <[email protected]>
Cc: Rhyland Klein <[email protected]>
Signed-off-by: Marc Dietrich <[email protected]>
---
include/linux/rfkill-gpio.h | 2 +-
net/rfkill/rfkill-gpio.c | 60 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/include/linux/rfkill-gpio.h b/include/linux/rfkill-gpio.h
index 4d09f6e..76a9674 100644
--- a/include/linux/rfkill-gpio.h
+++ b/include/linux/rfkill-gpio.h
@@ -35,7 +35,7 @@
*/
struct rfkill_gpio_platform_data {
- char *name;
+ const char *name;
int reset_gpio;
int shutdown_gpio;
const char *power_clk_name;
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 865adb6..6b4c5e8 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -21,6 +21,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/rfkill.h>
+#include <linux/of_gpio.h>
#include <linux/platform_device.h>
#include <linux/clk.h>
#include <linux/slab.h>
@@ -77,13 +78,70 @@ static const struct rfkill_ops rfkill_gpio_ops = {
.set_block = rfkill_gpio_set_power,
};
+#ifdef CONFIG_OF
+static struct rfkill_gpio_platform_data * __devinit
+ rfkill_gpio_parse_pdata(struct platform_device *pdev)
+{
+ struct rfkill_gpio_platform_data *pdata, *rfkill;
+ struct device_node *np = pdev->dev.of_node, *child;
+ int count = 0;
+
+ for_each_child_of_node(np, child)
+ count++;
+ if (!count)
+ return NULL;
+
+ pdata = devm_kzalloc(&pdev->dev,
+ sizeof(struct rfkill_gpio_platform_data) *
+ count, GFP_KERNEL);
+ if (!pdata)
+ return NULL;
+
+ rfkill = pdata;
+
+ for_each_child_of_node(np, child) {
+ of_property_read_string(child, "label", &rfkill->name);
+ if (!rfkill->name)
+ rfkill->name = child->name;
+ rfkill->reset_gpio = of_get_named_gpio(child, "reset-gpio", 0);
+ rfkill->shutdown_gpio = of_get_named_gpio(child,
+ "shutdown-gpio", 0);
+ of_property_read_u32(child, "type", &rfkill->type);
+ of_property_read_string(child, "clock",
+ &rfkill->power_clk_name);
+
+ rfkill += sizeof(struct rfkill_gpio_platform_data);
+ }
+
+ return pdata;
+}
+
+static const struct of_device_id of_rfkill_gpio_match[] = {
+ { .compatible = "rfkill-gpio", },
+ {},
+};
+
+#else
+static inline struct rfkill_gpio_platform_data
+ *rfkill_gpio_parse_pdata(struct platform_device *pdev)
+{
+ return NULL;
+}
+
+#define of_rfkill_gpio_match NULL
+#endif
+
static int rfkill_gpio_probe(struct platform_device *pdev)
{
struct rfkill_gpio_data *rfkill;
struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
+ struct device_node *np = pdev->dev.of_node;
int ret = 0;
int len = 0;
+ if (np)
+ pdata = rfkill_gpio_parse_pdata(pdev);
+
if (!pdata) {
pr_warn("%s: No platform data specified\n", __func__);
return -EINVAL;
@@ -210,13 +268,13 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
return 0;
}
-
static struct platform_driver rfkill_gpio_driver = {
.probe = rfkill_gpio_probe,
.remove = __devexit_p(rfkill_gpio_remove),
.driver = {
.name = "rfkill_gpio",
.owner = THIS_MODULE,
+ .of_match_table = of_rfkill_gpio_match,
},
};
--
1.7.5.4
Hi,
On Sun, Feb 5, 2012 at 9:18 AM, Marc Dietrich <[email protected]> wrote:
> Add device tree bindings information for rfkill gpio switches.
>
> Cc: [email protected]
> Cc: "John W. Linville" <[email protected]>
> Cc: Johannes Berg <[email protected]>
> Cc: Rhyland Klein <[email protected]>
> Cc: Grant Likely <[email protected]>
> Signed-off-by: Marc Dietrich <[email protected]>
Please cc [email protected] on device tree bindings;
not everyone monitors the linux mailing lists.
> diff --git a/Documentation/devicetree/bindings/gpio/rfkill.txt b/Documentation/devicetree/bindings/gpio/rfkill.txt
> new file mode 100644
> index 0000000..22bf22a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/rfkill.txt
> @@ -0,0 +1,38 @@
> +RFKILL switches connected to GPIO lines
> +
> +Required properties:
> +- compatible : should be "rfkill-gpio".
> +
> +Each rfkill switch is represented as a sub-node of the rfkill-gpio device.
> +Each node has a label property which represents the name of the corresponding
> +rfkill device.
> +
> +RFKILL sub-node properties:
> +- label : ?(optional) The label for this rfkill switch. ?If omitted, the label is
> + ?taken from the node name (excluding the unit address).
> +- reset-gpio, shutdown-gpio : ?Should specify the rfkill gpios for reset and
> + ?shutdown (see "Specifying GPIO information for devices" in
> + ?Documentation/devicetree/booting-without-of.txt).
> +- type : enumerated type of the gpio (see include/linux/rfkill.h).
> +- clock : (optional) name of the clock name associated with the rfkill switch
> + ?(see include/linux/rfkill-gpio.h)
Sorry, but this is going about things the wrong way.
A device tree binding is meant to describe hardware. For example, the
type and clock properties are clearly linux specifics that shouldn't
be expressed that way in the device tree.
instead, think about what you actually need to provide for a driver to
do its work. Does the wifi device have a gpio to control power (and
one for providing reset outside of the standard interface)? Well, then
that should probably be defined in the node that describes the device
-- i.e. under the sdhci node in question here.
That doesn't fit the rfkill device/driver model perfectly, since it
doesn't have something to bind against, so the rfkill platform device
needs to be instantiated from somewhere else. It's possible that best
match is to have the wifi driver handle it, which should be OK, I
think?
-Olof
Hi Olof,
Am Sonntag, 5. Februar 2012, 14:00:25 schrieb Olof Johansson:
> Hi,
>
> On Sun, Feb 5, 2012 at 9:18 AM, Marc Dietrich <[email protected]> wrote:
> > Add device tree bindings information for rfkill gpio switches.
> >
> > Cc: [email protected]
> > Cc: "John W. Linville" <[email protected]>
> > Cc: Johannes Berg <[email protected]>
> > Cc: Rhyland Klein <[email protected]>
> > Cc: Grant Likely <[email protected]>
> > Signed-off-by: Marc Dietrich <[email protected]>
>
> Please cc [email protected] on device tree bindings;
> not everyone monitors the linux mailing lists.
done.
> > diff --git a/Documentation/devicetree/bindings/gpio/rfkill.txt
> > b/Documentation/devicetree/bindings/gpio/rfkill.txt new file mode 100644
> > index 0000000..22bf22a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/rfkill.txt
> > @@ -0,0 +1,38 @@
> > +RFKILL switches connected to GPIO lines
> > +
> > +Required properties:
> > +- compatible : should be "rfkill-gpio".
> > +
> > +Each rfkill switch is represented as a sub-node of the rfkill-gpio device.
> > +Each node has a label property which represents the name of the corresponding
> > +rfkill device.
> > +
> > +RFKILL sub-node properties:
> > +- label : (optional) The label for this rfkill switch. If omitted, the label
> > is + taken from the node name (excluding the unit address).
> > +- reset-gpio, shutdown-gpio : Should specify the rfkill gpios for reset and
> > + shutdown (see "Specifying GPIO information for devices" in
> > + Documentation/devicetree/booting-without-of.txt).
> > +- type : enumerated type of the gpio (see include/linux/rfkill.h).
> > +- clock : (optional) name of the clock name associated with the rfkill switch
> > + (see include/linux/rfkill-gpio.h)
>
> Sorry, but this is going about things the wrong way.
>
> A device tree binding is meant to describe hardware. For example, the
> type and clock properties are clearly linux specifics that shouldn't
> be expressed that way in the device tree.
>
> instead, think about what you actually need to provide for a driver to
> do its work. Does the wifi device have a gpio to control power (and
> one for providing reset outside of the standard interface)? Well, then
> that should probably be defined in the node that describes the device
> -- i.e. under the sdhci node in question here.
well, that doesn't work in case of paz00 because nearly all devices (including wifi)
are connected via usb. Only the rfkill switch is controlled via the gpios.
> That doesn't fit the rfkill device/driver model perfectly, since it
> doesn't have something to bind against, so the rfkill platform device
> needs to be instantiated from somewhere else. It's possible that best
> match is to have the wifi driver handle it, which should be OK, I
> think?
You are right, but in case of usb devices which are not instantiated via device tree
there is no parent. It doesn't make much sense for me to put it under the usb
controller because it is independent to it. One may argue that the rfkill switch is a
separate device, which is controlled via gpio lines. The wifi driver itself has no
way to modify the gpios itself, it can only read out its status. I'm a bit lost here,
maybe you have a better solution.
Thanks
Marc
Add device tree bindings information for rfkill gpio switches.
Cc: [email protected]
Cc: "John W. Linville" <[email protected]>
Cc: Johannes Berg <[email protected]>
Cc: Rhyland Klein <[email protected]>
Cc: Grant Likely <[email protected]>
Signed-off-by: Marc Dietrich <[email protected]>
---
Documentation/devicetree/bindings/gpio/rfkill.txt | 38 +++++++++++++++++++++
1 files changed, 38 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/rfkill.txt
diff --git a/Documentation/devicetree/bindings/gpio/rfkill.txt b/Documentation/devicetree/bindings/gpio/rfkill.txt
new file mode 100644
index 0000000..22bf22a
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/rfkill.txt
@@ -0,0 +1,38 @@
+RFKILL switches connected to GPIO lines
+
+Required properties:
+- compatible : should be "rfkill-gpio".
+
+Each rfkill switch is represented as a sub-node of the rfkill-gpio device.
+Each node has a label property which represents the name of the corresponding
+rfkill device.
+
+RFKILL sub-node properties:
+- label : (optional) The label for this rfkill switch. If omitted, the label is
+ taken from the node name (excluding the unit address).
+- reset-gpio, shutdown-gpio : Should specify the rfkill gpios for reset and
+ shutdown (see "Specifying GPIO information for devices" in
+ Documentation/devicetree/booting-without-of.txt).
+- type : enumerated type of the gpio (see include/linux/rfkill.h).
+- clock : (optional) name of the clock name associated with the rfkill switch
+ (see include/linux/rfkill-gpio.h)
+
+Examples:
+
+rfkill-switches {
+ compatible = "rfkill-gpio";
+
+ wifi {
+ label = "wifi";
+ reset-gpio = <&gpio 25 0>; /* Active high */
+ shutdown-gpio = <&gpio 85 0>; /* Active high */
+ type = <1>;
+ };
+
+ bt {
+ label = "bluetooth";
+ reset-gpio = <&gpio 17 0>; /* Active high */
+ shutdown-gpio = <&gpio 35 0>; /* Active high */
+ type = <1>;
+ };
+};
--
1.7.5.4
Am 13.02.2012 20:36, schrieb Olof Johansson:
> On Mon, Feb 13, 2012 at 11:25 AM, Rhyland Klein<[email protected]> wrote:
>> On Sun, 2012-02-12 at 11:13 -0800, Marc Dietrich wrote:
>>> This adds device tree support for rfkill-gpio. The optional platform
>>> paramters gpio_runtime_close and gpio_runtime_setup are not implemented.
>>>
>>> Cc: [email protected]
>>> Cc: "John W. Linville"<[email protected]>
>>> Cc: Johannes Berg<[email protected]>
>>> Cc: Rhyland Klein<[email protected]>
>>> Signed-off-by: Marc Dietrich<[email protected]>
>>> +
>>> static int rfkill_gpio_probe(struct platform_device *pdev)
>>> {
>>> struct rfkill_gpio_data *rfkill;
>>> struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
>>> + struct device_node *np = pdev->dev.of_node;
>>> int ret = 0;
>>> int len = 0;
>>>
>>> + if (np)
>>> + pdata = rfkill_gpio_parse_pdata(pdev);
>>> +
>> The only concern I have is the precedence of devicetree settings vs
>> platform data settings? If there is pdata passed in from board file
>> initialization, and there is a device tree (a corner case but I think a
>> valid one) then I believe the order would be that defined pdata would
>> override the devicetree settings. That way if someone wanted to make a
>> quick update, they wouldn't need to change the boot loader as well.
> Yes, that is how other drivers tend to be coded -- only of pdata is
> null will the driver try to fill in from the DT.
If the device tree from the bootloader does not contain the rfkill
information the resources from platform_data will be used (so no need to
update the bootloader). If the bootloader contains rfkill information,
why shouldn't it be used?
Marc
On Mon, Feb 13, 2012 at 11:25 AM, Rhyland Klein <[email protected]> wrote:
> On Sun, 2012-02-12 at 11:13 -0800, Marc Dietrich wrote:
>> This adds device tree support for rfkill-gpio. The optional platform
>> paramters gpio_runtime_close and gpio_runtime_setup are not implemented.
>>
>> Cc: [email protected]
>> Cc: "John W. Linville" <[email protected]>
>> Cc: Johannes Berg <[email protected]>
>> Cc: Rhyland Klein <[email protected]>
>> Signed-off-by: Marc Dietrich <[email protected]>
>> +
>> ?static int rfkill_gpio_probe(struct platform_device *pdev)
>> ?{
>> ? ? ? struct rfkill_gpio_data *rfkill;
>> ? ? ? struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
>> + ? ? struct device_node *np = pdev->dev.of_node;
>> ? ? ? int ret = 0;
>> ? ? ? int len = 0;
>>
>> + ? ? if (np)
>> + ? ? ? ? ? ? pdata = rfkill_gpio_parse_pdata(pdev);
>> +
>
> The only concern I have is the precedence of devicetree settings vs
> platform data settings? If there is pdata passed in from board file
> initialization, and there is a device tree (a corner case but I think a
> valid one) then I believe the order would be that defined pdata would
> override the devicetree settings. That way if someone wanted to make a
> quick update, they wouldn't need to change the boot loader as well.
Yes, that is how other drivers tend to be coded -- only of pdata is
null will the driver try to fill in from the DT.
-Olof
* Marc Dietrich wrote:
> +#ifdef CONFIG_OF
[...]
> +#else
[...]
> +#define of_rfkill_gpio_match NULL
This can be dropped, see below.
> +#endif
> +
> static int rfkill_gpio_probe(struct platform_device *pdev)
> {
> struct rfkill_gpio_data *rfkill;
> struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
> + struct device_node *np = pdev->dev.of_node;
> int ret = 0;
> int len = 0;
>
> + if (np)
> + pdata = rfkill_gpio_parse_pdata(pdev);
> +
> if (!pdata) {
> pr_warn("%s: No platform data specified\n", __func__);
> return -EINVAL;
> @@ -210,13 +268,13 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
>
> return 0;
> }
> -
Are you removing this line on purpose?
> static struct platform_driver rfkill_gpio_driver = {
> .probe = rfkill_gpio_probe,
> .remove = __devexit_p(rfkill_gpio_remove),
> .driver = {
> .name = "rfkill_gpio",
> .owner = THIS_MODULE,
> + .of_match_table = of_rfkill_gpio_match,
I think the canonical way to do this is of_match_ptr() and leave out the
definition to NULL in the !CONFIG_OF above.
Thierry
Am Sonntag, 5. Februar 2012, 20:59:24 schrieb Thierry Reding:
> * Marc Dietrich wrote:
> > +#ifdef CONFIG_OF
>
> [...]
>
> > +#else
>
> [...]
>
> > +#define of_rfkill_gpio_match NULL
>
> This can be dropped, see below.
>
> > +#endif
> > +
> >
> > static int rfkill_gpio_probe(struct platform_device *pdev)
> > {
> >
> > struct rfkill_gpio_data *rfkill;
> > struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
> >
> > + struct device_node *np = pdev->dev.of_node;
> >
> > int ret = 0;
> > int len = 0;
> >
> > + if (np)
> > + pdata = rfkill_gpio_parse_pdata(pdev);
> > +
> >
> > if (!pdata) {
> >
> > pr_warn("%s: No platform data specified\n", __func__);
> > return -EINVAL;
> >
> > @@ -210,13 +268,13 @@ static int rfkill_gpio_remove(struct platform_device
> > *pdev)>
> > return 0;
> >
> > }
> >
> > -
>
> Are you removing this line on purpose?
of course not ;-)
> > static struct platform_driver rfkill_gpio_driver = {
> >
> > .probe = rfkill_gpio_probe,
> > .remove = __devexit_p(rfkill_gpio_remove),
> > .driver = {
> >
> > .name = "rfkill_gpio",
> > .owner = THIS_MODULE,
> >
> > + .of_match_table = of_rfkill_gpio_match,
>
> I think the canonical way to do this is of_match_ptr() and leave out the
> definition to NULL in the !CONFIG_OF above.
yes, the OF api is changing in such a fast way, that it is getting hard to track it,
especially if there are many drivers which use different generations of it.
I'll fix it up in the next iteration.
Thanks
Marc