diff mbox

net: rfkill-regulator: Add devicetree support.

Message ID 1391802529-29861-1-git-send-email-marek@goldelico.com
State Superseded, archived
Headers show

Commit Message

Marek Belisko Feb. 7, 2014, 7:48 p.m. UTC
Signed-off-by: NeilBrown <neilb@suse.de>
Signed-off-by: Marek Belisko <marek@goldelico.com>
---
Based on Neil's patch and extend for documentation and bindings include.

 .../bindings/net/rfkill/rfkill-relugator.txt       | 28 ++++++++++++++++
 include/dt-bindings/net/rfkill-regulator.h         | 23 +++++++++++++
 net/rfkill/rfkill-regulator.c                      | 38 ++++++++++++++++++++++
 3 files changed, 89 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
 create mode 100644 include/dt-bindings/net/rfkill-regulator.h

Comments

Bill Fink Feb. 8, 2014, 6:22 a.m. UTC | #1
On Fri,  7 Feb 2014, Marek Belisko wrote:

> Signed-off-by: NeilBrown <neilb@suse.de>
> Signed-off-by: Marek Belisko <marek@goldelico.com>
> ---
> Based on Neil's patch and extend for documentation and bindings include.
> 
>  .../bindings/net/rfkill/rfkill-relugator.txt       | 28 ++++++++++++++++

                                  ^^^^^^^^^
                                  Typo in file name.

					-Bill



>  include/dt-bindings/net/rfkill-regulator.h         | 23 +++++++++++++
>  net/rfkill/rfkill-regulator.c                      | 38 ++++++++++++++++++++++
>  3 files changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
>  create mode 100644 include/dt-bindings/net/rfkill-regulator.h
> 
> diff --git a/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt b/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
> new file mode 100644
> index 0000000..cdb7dd7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
> @@ -0,0 +1,28 @@
> +Regulator consumer for rfkill devices
> +
> +Required properties:
> +- compatible   : Must be "rfkill-regulator".
> +- label  : Name of rfkill device.
> +- type  : Type of rfkill device.
> +
> +Possible values (defined in include/dt-bindings/net/rfkill-regulator.h):
> +	RFKILL_TYPE_ALL
> +	RFKILL_TYPE_WLAN
> +	RFKILL_TYPE_BLUETOOTH
> +	RFKILL_TYPE_UWB
> +	RFKILL_TYPE_WIMAX
> +	RFKILL_TYPE_WWAN
> +	RFKILL_TYPE_GPS
> +	RFKILL_TYPE_FM
> +	RFKILL_TYPE_NFC
> +
> +- vrfkill-supply - regulator device.
> +
> +Example:
> +	gps-rfkill {
> +		compatible = "rfkill-regulator";
> +		label = "GPS";
> +		type = <RFKILL_TYPE_GPS>;
> +		vrfkill-supply = <&reg>;
> +	};
> +
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Belisko Marek Feb. 8, 2014, 8:42 p.m. UTC | #2
On Sat, Feb 8, 2014 at 7:22 AM, Bill Fink <billfink@mindspring.com> wrote:
> On Fri,  7 Feb 2014, Marek Belisko wrote:
>
>> Signed-off-by: NeilBrown <neilb@suse.de>
>> Signed-off-by: Marek Belisko <marek@goldelico.com>
>> ---
>> Based on Neil's patch and extend for documentation and bindings include.
>>
>>  .../bindings/net/rfkill/rfkill-relugator.txt       | 28 ++++++++++++++++
>
>                                   ^^^^^^^^^
>                                   Typo in file name.
Ah. Right. Thanks.
>
>                                         -Bill
>
>
>
>>  include/dt-bindings/net/rfkill-regulator.h         | 23 +++++++++++++
I also think this should be renamed to rfkill.h because same defines
could be used for rfkill-gpio driver.
>>  net/rfkill/rfkill-regulator.c                      | 38 ++++++++++++++++++++++
>>  3 files changed, 89 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
>>  create mode 100644 include/dt-bindings/net/rfkill-regulator.h
>>
>> diff --git a/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt b/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
>> new file mode 100644
>> index 0000000..cdb7dd7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
>> @@ -0,0 +1,28 @@
>> +Regulator consumer for rfkill devices
>> +
>> +Required properties:
>> +- compatible   : Must be "rfkill-regulator".
>> +- label  : Name of rfkill device.
>> +- type  : Type of rfkill device.
>> +
>> +Possible values (defined in include/dt-bindings/net/rfkill-regulator.h):
>> +     RFKILL_TYPE_ALL
>> +     RFKILL_TYPE_WLAN
>> +     RFKILL_TYPE_BLUETOOTH
>> +     RFKILL_TYPE_UWB
>> +     RFKILL_TYPE_WIMAX
>> +     RFKILL_TYPE_WWAN
>> +     RFKILL_TYPE_GPS
>> +     RFKILL_TYPE_FM
>> +     RFKILL_TYPE_NFC
>> +
>> +- vrfkill-supply - regulator device.
>> +
>> +Example:
>> +     gps-rfkill {
>> +             compatible = "rfkill-regulator";
>> +             label = "GPS";
>> +             type = <RFKILL_TYPE_GPS>;
>> +             vrfkill-supply = <&reg>;
>> +     };
>> +

BR,

marek
Johannes Berg Feb. 10, 2014, 8:27 a.m. UTC | #3
On Fri, 2014-02-07 at 20:48 +0100, Marek Belisko wrote:

> +#define RFKILL_TYPE_ALL		(0)
> +#define RFKILL_TYPE_WLAN	(1)
> +#define RFKILL_TYPE_BLUETOOTH	(2)
> +#define RFKILL_TYPE_UWB		(3)
> +#define RFKILL_TYPE_WIMAX	(4)
> +#define RFKILL_TYPE_WWAN	(5)
> +#define RFKILL_TYPE_GPS		(6)
> +#define RFKILL_TYPE_FM		(7)
> +#define RFKILL_TYPE_NFC		(8)

This seems like a bad idea since there's an enum elsewhere in userspace
API already.

johannes

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
H. Nikolaus Schaller Feb. 10, 2014, 8:54 a.m. UTC | #4
Am 10.02.2014 um 09:27 schrieb Johannes Berg:

> On Fri, 2014-02-07 at 20:48 +0100, Marek Belisko wrote:
> 
>> +#define RFKILL_TYPE_ALL		(0)
>> +#define RFKILL_TYPE_WLAN	(1)
>> +#define RFKILL_TYPE_BLUETOOTH	(2)
>> +#define RFKILL_TYPE_UWB		(3)
>> +#define RFKILL_TYPE_WIMAX	(4)
>> +#define RFKILL_TYPE_WWAN	(5)
>> +#define RFKILL_TYPE_GPS		(6)
>> +#define RFKILL_TYPE_FM		(7)
>> +#define RFKILL_TYPE_NFC		(8)
> 
> This seems like a bad idea since there's an enum elsewhere in userspace
> API already.

Yes,
you are right. It is defined in include/uapi/linux/rfkill.h

Tnx,
Nikolaus

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland Feb. 10, 2014, 10:18 a.m. UTC | #5
On Fri, Feb 07, 2014 at 07:48:49PM +0000, Marek Belisko wrote:
> Signed-off-by: NeilBrown <neilb@suse.de>
> Signed-off-by: Marek Belisko <marek@goldelico.com>
> ---
> Based on Neil's patch and extend for documentation and bindings include.
> 
>  .../bindings/net/rfkill/rfkill-relugator.txt       | 28 ++++++++++++++++
>  include/dt-bindings/net/rfkill-regulator.h         | 23 +++++++++++++
>  net/rfkill/rfkill-regulator.c                      | 38 ++++++++++++++++++++++
>  3 files changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
>  create mode 100644 include/dt-bindings/net/rfkill-regulator.h
> 
> diff --git a/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt b/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
> new file mode 100644
> index 0000000..cdb7dd7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
> @@ -0,0 +1,28 @@
> +Regulator consumer for rfkill devices

What exactly is an "rfkill" device? How is it used? How does it relate
to other devices in the DT?

To me, this looks like a leak of a Linux abstraction.

> +
> +Required properties:
> +- compatible   : Must be "rfkill-regulator".
> +- label  : Name of rfkill device.

What's this for? Why does this need a label in the DT? Surely this can
be implied by the relationship to a particular radio device?

> +- type  : Type of rfkill device.
> +
> +Possible values (defined in include/dt-bindings/net/rfkill-regulator.h):
> +	RFKILL_TYPE_ALL
> +	RFKILL_TYPE_WLAN
> +	RFKILL_TYPE_BLUETOOTH
> +	RFKILL_TYPE_UWB
> +	RFKILL_TYPE_WIMAX
> +	RFKILL_TYPE_WWAN
> +	RFKILL_TYPE_GPS
> +	RFKILL_TYPE_FM
> +	RFKILL_TYPE_NFC

What do these mean? Why can these not be implied by a relationship to
any devices of these particular types?

> +
> +- vrfkill-supply - regulator device.

Why isn't this described on the radio revice node? It's a supply to the
radio, not to the rfkill concept.

> +
> +Example:
> +	gps-rfkill {
> +		compatible = "rfkill-regulator";
> +		label = "GPS";
> +		type = <RFKILL_TYPE_GPS>;
> +		vrfkill-supply = <&reg>;
> +	};

Why is this not bound to the particular GPS device in some way?

What if I have more than one of any of the types of device this
supports, which device is this expected to control?

Why is it described as a separate device in the device tree at all?

I do not think this binding is the right way to describe this.

Thanks,
Mark.

> +
> diff --git a/include/dt-bindings/net/rfkill-regulator.h b/include/dt-bindings/net/rfkill-regulator.h
> new file mode 100644
> index 0000000..ae32273
> --- /dev/null
> +++ b/include/dt-bindings/net/rfkill-regulator.h
> @@ -0,0 +1,23 @@
> +/*
> + * This header provides macros for rfkill-regulator bindings.
> + *
> + * Copyright (C) 2014 Marek Belisko <marek@goldelico.com>
> + *
> + * GPLv2 only
> + */
> +
> +#ifndef __DT_BINDINGS_RFKILL_REGULATOR_H__
> +#define __DT_BINDINGS_RFKILL_REGULATOR_H__
> +
> +
> +#define RFKILL_TYPE_ALL		(0)
> +#define RFKILL_TYPE_WLAN	(1)
> +#define RFKILL_TYPE_BLUETOOTH	(2)
> +#define RFKILL_TYPE_UWB		(3)
> +#define RFKILL_TYPE_WIMAX	(4)
> +#define RFKILL_TYPE_WWAN	(5)
> +#define RFKILL_TYPE_GPS		(6)
> +#define RFKILL_TYPE_FM		(7)
> +#define RFKILL_TYPE_NFC		(8)
> +
> +#endif /* __DT_BINDINGS_RFKILL_REGULATOR_H__ */
> diff --git a/net/rfkill/rfkill-regulator.c b/net/rfkill/rfkill-regulator.c
> index cf5b145..a04aff8 100644
> --- a/net/rfkill/rfkill-regulator.c
> +++ b/net/rfkill/rfkill-regulator.c
> @@ -19,6 +19,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/rfkill.h>
>  #include <linux/rfkill-regulator.h>
> +#include <linux/of_platform.h>
>  
>  struct rfkill_regulator_data {
>  	struct rfkill *rf_kill;
> @@ -57,6 +58,31 @@ static struct rfkill_ops rfkill_regulator_ops = {
>  	.set_block = rfkill_regulator_set_block,
>  };
>  
> +#ifdef CONFIG_OF
> +static struct rfkill_regulator_platform_data *
> +rfkill_regulator_parse_pdata(struct device *dev)
> +{
> +	struct rfkill_regulator_platform_data *pdata;
> +	struct device_node *np = dev->of_node;
> +	u32 num;
> +	if (!np)
> +		return NULL;
> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return NULL;
> +	if (of_property_read_u32(np, "type", &num) == 0)
> +		pdata->type = num;
> +	of_property_read_string(np, "label", &pdata->name);
> +	return pdata;
> +}
> +#else
> +static inline struct rfkill_regulator_platform_data *
> +rfkill_regulator_parse_pdata(struct device *dev)
> +{
> +	return NULL;
> +}
> +#endif
> +
>  static int rfkill_regulator_probe(struct platform_device *pdev)
>  {
>  	struct rfkill_regulator_platform_data *pdata = pdev->dev.platform_data;
> @@ -65,6 +91,9 @@ static int rfkill_regulator_probe(struct platform_device *pdev)
>  	struct rfkill *rf_kill;
>  	int ret = 0;
>  
> +	if (!pdata)
> +		pdata = rfkill_regulator_parse_pdata(&pdev->dev);
> +
>  	if (pdata == NULL) {
>  		dev_err(&pdev->dev, "no platform data\n");
>  		return -ENODEV;
> @@ -137,12 +166,21 @@ static int rfkill_regulator_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id rfkill_regulator_match[] = {
> +	{.compatible = "rfkill-regulator"},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, rfkill_regulator_match);
> +#endif
> +
>  static struct platform_driver rfkill_regulator_driver = {
>  	.probe = rfkill_regulator_probe,
>  	.remove = rfkill_regulator_remove,
>  	.driver = {
>  		.name = "rfkill-regulator",
>  		.owner = THIS_MODULE,
> +		.of_match_table = of_match_ptr(rfkill_regulator_match),
>  	},
>  };
>  
> -- 
> 1.8.3.2
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Belisko Marek Feb. 10, 2014, 8:05 p.m. UTC | #6
On Mon, Feb 10, 2014 at 9:54 AM, Dr. H. Nikolaus Schaller
<hns@goldelico.com> wrote:
> Am 10.02.2014 um 09:27 schrieb Johannes Berg:
>
>> On Fri, 2014-02-07 at 20:48 +0100, Marek Belisko wrote:
>>
>>> +#define RFKILL_TYPE_ALL             (0)
>>> +#define RFKILL_TYPE_WLAN    (1)
>>> +#define RFKILL_TYPE_BLUETOOTH       (2)
>>> +#define RFKILL_TYPE_UWB             (3)
>>> +#define RFKILL_TYPE_WIMAX   (4)
>>> +#define RFKILL_TYPE_WWAN    (5)
>>> +#define RFKILL_TYPE_GPS             (6)
>>> +#define RFKILL_TYPE_FM              (7)
>>> +#define RFKILL_TYPE_NFC             (8)
>>
>> This seems like a bad idea since there's an enum elsewhere in userspace
>> API already.
I know this is duplicate but in device tree we cannot use enums
(AFAIU include/dt-bindings are defines used in device tree files)
>
> Yes,
> you are right. It is defined in include/uapi/linux/rfkill.h
>
> Tnx,
> Nikolaus
>

BR,

marek
Belisko Marek Feb. 10, 2014, 9:35 p.m. UTC | #7
On Mon, Feb 10, 2014 at 11:18 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Feb 07, 2014 at 07:48:49PM +0000, Marek Belisko wrote:
>> Signed-off-by: NeilBrown <neilb@suse.de>
>> Signed-off-by: Marek Belisko <marek@goldelico.com>
>> ---
>> Based on Neil's patch and extend for documentation and bindings include.
>>
>>  .../bindings/net/rfkill/rfkill-relugator.txt       | 28 ++++++++++++++++
>>  include/dt-bindings/net/rfkill-regulator.h         | 23 +++++++++++++
>>  net/rfkill/rfkill-regulator.c                      | 38 ++++++++++++++++++++++
>>  3 files changed, 89 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
>>  create mode 100644 include/dt-bindings/net/rfkill-regulator.h
>>
>> diff --git a/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt b/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
>> new file mode 100644
>> index 0000000..cdb7dd7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
>> @@ -0,0 +1,28 @@
>> +Regulator consumer for rfkill devices
>
> What exactly is an "rfkill" device? How is it used? How does it relate
> to other devices in the DT?
>
> To me, this looks like a leak of a Linux abstraction.
>
>> +
>> +Required properties:
>> +- compatible   : Must be "rfkill-regulator".
>> +- label  : Name of rfkill device.
>
> What's this for? Why does this need a label in the DT? Surely this can
> be implied by the relationship to a particular radio device?
This label is used by rfkill (converted to pdata->name in probe
function) and used for displaying.
Maybe label isn't correct name for that purpose.
>
>> +- type  : Type of rfkill device.
>> +
>> +Possible values (defined in include/dt-bindings/net/rfkill-regulator.h):
>> +     RFKILL_TYPE_ALL
>> +     RFKILL_TYPE_WLAN
>> +     RFKILL_TYPE_BLUETOOTH
>> +     RFKILL_TYPE_UWB
>> +     RFKILL_TYPE_WIMAX
>> +     RFKILL_TYPE_WWAN
>> +     RFKILL_TYPE_GPS
>> +     RFKILL_TYPE_FM
>> +     RFKILL_TYPE_NFC
>
> What do these mean? Why can these not be implied by a relationship to
> any devices of these particular types?
I did platform data -> DT mapping 1 : 1. Maybe we don't need to export
those to separate
include file and only use raw number instead.
>
>> +
>> +- vrfkill-supply - regulator device.
>
> Why isn't this described on the radio revice node? It's a supply to the
> radio, not to the rfkill concept.
rfkill-regulator in probe check for vrfkill regulator so I've added it
to description as without that rfkill-regulator doesn't make sense.
>
>> +
>> +Example:
>> +     gps-rfkill {
>> +             compatible = "rfkill-regulator";
>> +             label = "GPS";
>> +             type = <RFKILL_TYPE_GPS>;
>> +             vrfkill-supply = <&reg>;
>> +     };
>
> Why is this not bound to the particular GPS device in some way?
We can do something like:
gps-device {
    compatible = "my-desired-gps";
    <other device properties>
    rfkill = <&gps-rfkill>;
};
>
> What if I have more than one of any of the types of device this
> supports, which device is this expected to control?
rfkill-regulator is linked with regulator so if you have another
device it is probably controlled
with another regulator.

>
> Why is it described as a separate device in the device tree at all?
>
> I do not think this binding is the right way to describe this.
Some time ago was posted rfkill-gpio DT binding conversion and was
using the nearly the same bindings as
we propose and there was no issue with that.

>
> Thanks,
> Mark.
>
>> +
>> diff --git a/include/dt-bindings/net/rfkill-regulator.h b/include/dt-bindings/net/rfkill-regulator.h
>> new file mode 100644
>> index 0000000..ae32273
>> --- /dev/null
>> +++ b/include/dt-bindings/net/rfkill-regulator.h
>> @@ -0,0 +1,23 @@
>> +/*
>> + * This header provides macros for rfkill-regulator bindings.
>> + *
>> + * Copyright (C) 2014 Marek Belisko <marek@goldelico.com>
>> + *
>> + * GPLv2 only
>> + */
>> +
>> +#ifndef __DT_BINDINGS_RFKILL_REGULATOR_H__
>> +#define __DT_BINDINGS_RFKILL_REGULATOR_H__
>> +
>> +
>> +#define RFKILL_TYPE_ALL              (0)
>> +#define RFKILL_TYPE_WLAN     (1)
>> +#define RFKILL_TYPE_BLUETOOTH        (2)
>> +#define RFKILL_TYPE_UWB              (3)
>> +#define RFKILL_TYPE_WIMAX    (4)
>> +#define RFKILL_TYPE_WWAN     (5)
>> +#define RFKILL_TYPE_GPS              (6)
>> +#define RFKILL_TYPE_FM               (7)
>> +#define RFKILL_TYPE_NFC              (8)
>> +
>> +#endif /* __DT_BINDINGS_RFKILL_REGULATOR_H__ */
>> diff --git a/net/rfkill/rfkill-regulator.c b/net/rfkill/rfkill-regulator.c
>> index cf5b145..a04aff8 100644
>> --- a/net/rfkill/rfkill-regulator.c
>> +++ b/net/rfkill/rfkill-regulator.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/regulator/consumer.h>
>>  #include <linux/rfkill.h>
>>  #include <linux/rfkill-regulator.h>
>> +#include <linux/of_platform.h>
>>
>>  struct rfkill_regulator_data {
>>       struct rfkill *rf_kill;
>> @@ -57,6 +58,31 @@ static struct rfkill_ops rfkill_regulator_ops = {
>>       .set_block = rfkill_regulator_set_block,
>>  };
>>
>> +#ifdef CONFIG_OF
>> +static struct rfkill_regulator_platform_data *
>> +rfkill_regulator_parse_pdata(struct device *dev)
>> +{
>> +     struct rfkill_regulator_platform_data *pdata;
>> +     struct device_node *np = dev->of_node;
>> +     u32 num;
>> +     if (!np)
>> +             return NULL;
>> +     pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> +     if (!pdata)
>> +             return NULL;
>> +     if (of_property_read_u32(np, "type", &num) == 0)
>> +             pdata->type = num;
>> +     of_property_read_string(np, "label", &pdata->name);
>> +     return pdata;
>> +}
>> +#else
>> +static inline struct rfkill_regulator_platform_data *
>> +rfkill_regulator_parse_pdata(struct device *dev)
>> +{
>> +     return NULL;
>> +}
>> +#endif
>> +
>>  static int rfkill_regulator_probe(struct platform_device *pdev)
>>  {
>>       struct rfkill_regulator_platform_data *pdata = pdev->dev.platform_data;
>> @@ -65,6 +91,9 @@ static int rfkill_regulator_probe(struct platform_device *pdev)
>>       struct rfkill *rf_kill;
>>       int ret = 0;
>>
>> +     if (!pdata)
>> +             pdata = rfkill_regulator_parse_pdata(&pdev->dev);
>> +
>>       if (pdata == NULL) {
>>               dev_err(&pdev->dev, "no platform data\n");
>>               return -ENODEV;
>> @@ -137,12 +166,21 @@ static int rfkill_regulator_remove(struct platform_device *pdev)
>>       return 0;
>>  }
>>
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id rfkill_regulator_match[] = {
>> +     {.compatible = "rfkill-regulator"},
>> +     {}
>> +};
>> +MODULE_DEVICE_TABLE(of, rfkill_regulator_match);
>> +#endif
>> +
>>  static struct platform_driver rfkill_regulator_driver = {
>>       .probe = rfkill_regulator_probe,
>>       .remove = rfkill_regulator_remove,
>>       .driver = {
>>               .name = "rfkill-regulator",
>>               .owner = THIS_MODULE,
>> +             .of_match_table = of_match_ptr(rfkill_regulator_match),
>>       },
>>  };
>>
>> --
>> 1.8.3.2
>>
>>

BR,

marek
Marc Dietrich Feb. 11, 2014, 8:42 a.m. UTC | #8
Am Montag, 10. Februar 2014, 10:18:42 schrieb Mark Rutland:
> On Fri, Feb 07, 2014 at 07:48:49PM +0000, Marek Belisko wrote:
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > Signed-off-by: Marek Belisko <marek@goldelico.com>
> > ---
> > Based on Neil's patch and extend for documentation and bindings include.
> > 
> >  .../bindings/net/rfkill/rfkill-relugator.txt       | 28 ++++++++++++++++
> >  include/dt-bindings/net/rfkill-regulator.h         | 23 +++++++++++++
> >  net/rfkill/rfkill-regulator.c                      | 38
> >  ++++++++++++++++++++++ 3 files changed, 89 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt create
> >  mode 100644 include/dt-bindings/net/rfkill-regulator.h
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
> > b/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt new
> > file mode 100644
> > index 0000000..cdb7dd7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
> > @@ -0,0 +1,28 @@
> > +Regulator consumer for rfkill devices
> 
> What exactly is an "rfkill" device? How is it used? How does it relate
> to other devices in the DT?
> 
> To me, this looks like a leak of a Linux abstraction.
> 
> > +
> > +Required properties:
> > +- compatible   : Must be "rfkill-regulator".
> > +- label  : Name of rfkill device.
> 
> What's this for? Why does this need a label in the DT? Surely this can
> be implied by the relationship to a particular radio device?
> 
> > +- type  : Type of rfkill device.
> > +
> > +Possible values (defined in include/dt-bindings/net/rfkill-regulator.h):
> > +	RFKILL_TYPE_ALL
> > +	RFKILL_TYPE_WLAN
> > +	RFKILL_TYPE_BLUETOOTH
> > +	RFKILL_TYPE_UWB
> > +	RFKILL_TYPE_WIMAX
> > +	RFKILL_TYPE_WWAN
> > +	RFKILL_TYPE_GPS
> > +	RFKILL_TYPE_FM
> > +	RFKILL_TYPE_NFC
> 
> What do these mean? Why can these not be implied by a relationship to
> any devices of these particular types?

This problem comes up from time to time. rfkill-gpio has a similar problem, 
btw. You can of course list an rfkill property inside the device node where it 
is connected to, but what are you going to to if there is no device node, e.g. 
the device is enumerated on a bus (e.g. usb, pci, ...)?

In such cases it may be better to define a standalone rfkill node similar to 
the "backlight" device, even if it just contains a gpio or regulator.

Marc

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt b/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
new file mode 100644
index 0000000..cdb7dd7
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/rfkill/rfkill-relugator.txt
@@ -0,0 +1,28 @@ 
+Regulator consumer for rfkill devices
+
+Required properties:
+- compatible   : Must be "rfkill-regulator".
+- label  : Name of rfkill device.
+- type  : Type of rfkill device.
+
+Possible values (defined in include/dt-bindings/net/rfkill-regulator.h):
+	RFKILL_TYPE_ALL
+	RFKILL_TYPE_WLAN
+	RFKILL_TYPE_BLUETOOTH
+	RFKILL_TYPE_UWB
+	RFKILL_TYPE_WIMAX
+	RFKILL_TYPE_WWAN
+	RFKILL_TYPE_GPS
+	RFKILL_TYPE_FM
+	RFKILL_TYPE_NFC
+
+- vrfkill-supply - regulator device.
+
+Example:
+	gps-rfkill {
+		compatible = "rfkill-regulator";
+		label = "GPS";
+		type = <RFKILL_TYPE_GPS>;
+		vrfkill-supply = <&reg>;
+	};
+
diff --git a/include/dt-bindings/net/rfkill-regulator.h b/include/dt-bindings/net/rfkill-regulator.h
new file mode 100644
index 0000000..ae32273
--- /dev/null
+++ b/include/dt-bindings/net/rfkill-regulator.h
@@ -0,0 +1,23 @@ 
+/*
+ * This header provides macros for rfkill-regulator bindings.
+ *
+ * Copyright (C) 2014 Marek Belisko <marek@goldelico.com>
+ *
+ * GPLv2 only
+ */
+
+#ifndef __DT_BINDINGS_RFKILL_REGULATOR_H__
+#define __DT_BINDINGS_RFKILL_REGULATOR_H__
+
+
+#define RFKILL_TYPE_ALL		(0)
+#define RFKILL_TYPE_WLAN	(1)
+#define RFKILL_TYPE_BLUETOOTH	(2)
+#define RFKILL_TYPE_UWB		(3)
+#define RFKILL_TYPE_WIMAX	(4)
+#define RFKILL_TYPE_WWAN	(5)
+#define RFKILL_TYPE_GPS		(6)
+#define RFKILL_TYPE_FM		(7)
+#define RFKILL_TYPE_NFC		(8)
+
+#endif /* __DT_BINDINGS_RFKILL_REGULATOR_H__ */
diff --git a/net/rfkill/rfkill-regulator.c b/net/rfkill/rfkill-regulator.c
index cf5b145..a04aff8 100644
--- a/net/rfkill/rfkill-regulator.c
+++ b/net/rfkill/rfkill-regulator.c
@@ -19,6 +19,7 @@ 
 #include <linux/regulator/consumer.h>
 #include <linux/rfkill.h>
 #include <linux/rfkill-regulator.h>
+#include <linux/of_platform.h>
 
 struct rfkill_regulator_data {
 	struct rfkill *rf_kill;
@@ -57,6 +58,31 @@  static struct rfkill_ops rfkill_regulator_ops = {
 	.set_block = rfkill_regulator_set_block,
 };
 
+#ifdef CONFIG_OF
+static struct rfkill_regulator_platform_data *
+rfkill_regulator_parse_pdata(struct device *dev)
+{
+	struct rfkill_regulator_platform_data *pdata;
+	struct device_node *np = dev->of_node;
+	u32 num;
+	if (!np)
+		return NULL;
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+	if (of_property_read_u32(np, "type", &num) == 0)
+		pdata->type = num;
+	of_property_read_string(np, "label", &pdata->name);
+	return pdata;
+}
+#else
+static inline struct rfkill_regulator_platform_data *
+rfkill_regulator_parse_pdata(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
 static int rfkill_regulator_probe(struct platform_device *pdev)
 {
 	struct rfkill_regulator_platform_data *pdata = pdev->dev.platform_data;
@@ -65,6 +91,9 @@  static int rfkill_regulator_probe(struct platform_device *pdev)
 	struct rfkill *rf_kill;
 	int ret = 0;
 
+	if (!pdata)
+		pdata = rfkill_regulator_parse_pdata(&pdev->dev);
+
 	if (pdata == NULL) {
 		dev_err(&pdev->dev, "no platform data\n");
 		return -ENODEV;
@@ -137,12 +166,21 @@  static int rfkill_regulator_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id rfkill_regulator_match[] = {
+	{.compatible = "rfkill-regulator"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, rfkill_regulator_match);
+#endif
+
 static struct platform_driver rfkill_regulator_driver = {
 	.probe = rfkill_regulator_probe,
 	.remove = rfkill_regulator_remove,
 	.driver = {
 		.name = "rfkill-regulator",
 		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(rfkill_regulator_match),
 	},
 };