[PATCH 1/2] Documentation: devicetree: Add bindings info for rfkill-regulator

[PATCH 1/2] Documentation: devicetree: Add bindings info for rfkill-regulator

[Paul Cercueil]

This document gives information about how to write a devicetree
node that corresponds to the rfkill-regulator driver.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 .../devicetree/bindings/net/rfkill-regulator.txt       | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/rfkill-regulator.txt

diff --git a/Documentation/devicetree/bindings/net/rfkill-regulator.txt b/Documentation/devicetree/bindings/net/rfkill-regulator.txt
new file mode 100644
index 0000000..aac2fe1
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/rfkill-regulator.txt
@@ -0,0 +1,18 @@
+Device tree bindings for the rfkill-regulator driver
+
+Required properties:
+  - compatible:	should be "rfkill-regulator"
+  - rfkill-name:	the name of this rfkill device
+  - rfkill-type:	the type of this rfkill device;
+			must correspond to a valid rfkill_type from <uapi/linux/rfkill.h>
+  - vrfkill-supply:	phandle to a regulator
+
+Example:
+
+	wlan-rfkill {
+		compatible = "rfkill-regulator";
+		rfkill-name = "WLAN power switch";
+		rfkill-type = <1>;
+
+		vrfkill-supply = <&wlan_power>;
+	};
-- 
2.9.3

[PATCH 2/2] net: rfkill-regulator: Add devicetree support

[Paul Cercueil]

This commit adds the support for probing the rfkill-regulator
from devicetree.

Information about the devicetree bindings can be found here:
Documentation/devicetree/bindings/net/rfkill-regulator.txt

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 net/rfkill/rfkill-regulator.c | 43 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/net/rfkill/rfkill-regulator.c b/net/rfkill/rfkill-regulator.c
index 50cd26a..29ded4c 100644
--- a/net/rfkill/rfkill-regulator.c
+++ b/net/rfkill/rfkill-regulator.c
@@ -13,6 +13,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
@@ -60,24 +61,38 @@ static struct rfkill_ops rfkill_regulator_ops = {
 static int rfkill_regulator_probe(struct platform_device *pdev)
 {
 	struct rfkill_regulator_platform_data *pdata = pdev->dev.platform_data;
+	struct device_node *node = pdev->dev.of_node;
 	struct rfkill_regulator_data *rfkill_data;
 	struct regulator *vcc;
 	struct rfkill *rf_kill;
+	const char *name;
+	u32 type;
 	int ret = 0;
 
-	if (pdata == NULL) {
+	if (pdata) {
+		if (!pdata->name || !pdata->type) {
+			dev_err(&pdev->dev, "invalid name or type in platform data\n");
+			return -EINVAL;
+		}
+
+		name = pdata->name;
+		type = pdata->type;
+	} else if (node) {
+		ret = of_property_read_u32(node, "rfkill-type", &type);
+		if (ret < 0)
+			return ret;
+
+		ret = of_property_read_string(node, "rfkill-name", &name);
+		if (ret < 0)
+			return ret;
+	} else {
 		dev_err(&pdev->dev, "no platform data\n");
 		return -ENODEV;
 	}
 
-	if (pdata->name == NULL || pdata->type == 0) {
-		dev_err(&pdev->dev, "invalid name or type in platform data\n");
-		return -EINVAL;
-	}
-
 	vcc = regulator_get_exclusive(&pdev->dev, "vrfkill");
 	if (IS_ERR(vcc)) {
-		dev_err(&pdev->dev, "Cannot get vcc for %s\n", pdata->name);
+		dev_err(&pdev->dev, "Cannot get vcc for %s\n", name);
 		ret = PTR_ERR(vcc);
 		goto out;
 	}
@@ -88,9 +103,8 @@ static int rfkill_regulator_probe(struct platform_device *pdev)
 		goto err_data_alloc;
 	}
 
-	rf_kill = rfkill_alloc(pdata->name, &pdev->dev,
-				pdata->type,
-				&rfkill_regulator_ops, rfkill_data);
+	rf_kill = rfkill_alloc(name, &pdev->dev, type,
+			       &rfkill_regulator_ops, rfkill_data);
 	if (rf_kill == NULL) {
 		ret = -ENOMEM;
 		goto err_rfkill_alloc;
@@ -110,7 +124,7 @@ static int rfkill_regulator_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, rfkill_data);
-	dev_info(&pdev->dev, "%s initialized\n", pdata->name);
+	dev_info(&pdev->dev, "%s initialized\n", name);
 
 	return 0;
 
@@ -137,11 +151,18 @@ static int rfkill_regulator_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id rfkill_regulator_of_match[] = {
+	{ .compatible = "rfkill-regulator" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rfkill_regulator_of_match);
+
 static struct platform_driver rfkill_regulator_driver = {
 	.probe = rfkill_regulator_probe,
 	.remove = rfkill_regulator_remove,
 	.driver = {
 		.name = "rfkill-regulator",
+		.of_match_table = of_match_ptr(rfkill_regulator_of_match),
 	},
 };
 
-- 
2.9.3

Re: [PATCH 1/2] Documentation: devicetree: Add bindings info for rfkill-regulator

[Rob Herring]

On Tue, Nov 01, 2016 at 11:58:39AM +0100, Paul Cercueil wrote:
> This document gives information about how to write a devicetree
> node that corresponds to the rfkill-regulator driver.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  .../devicetree/bindings/net/rfkill-regulator.txt       | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/rfkill-regulator.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/rfkill-regulator.txt b/Documentation/devicetree/bindings/net/rfkill-regulator.txt
> new file mode 100644
> index 0000000..aac2fe1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/rfkill-regulator.txt
> @@ -0,0 +1,18 @@
> +Device tree bindings for the rfkill-regulator driver
> +
> +Required properties:
> +  - compatible:	should be "rfkill-regulator"
> +  - rfkill-name:	the name of this rfkill device
> +  - rfkill-type:	the type of this rfkill device;
> +			must correspond to a valid rfkill_type from <uapi/linux/rfkill.h>
> +  - vrfkill-supply:	phandle to a regulator

My understanding is it is generally felt that using the regulator enable 
GPIO commonly found on WiFi chips for rfkill is an abuse of rfkill as it 
is more that just an RF disable. From a DT standpoint, this seems like 
creating a binding for what a Linux driver wants. Instead, I think this 
should be either a GPIO or GPIO regulator and the driver for the WiFi 
chip should decide whether or not to register that as an rfkill driver.

Rob

Re: [PATCH 1/2] Documentation: devicetree: Add bindings info for rfkill-regulator

[Johannes Berg]

> My understanding is it is generally felt that using the regulator
> enable GPIO commonly found on WiFi chips for rfkill is an abuse of
> rfkill as it is more that just an RF disable. From a DT standpoint,
> this seems like creating a binding for what a Linux driver wants.
> Instead, I think this should be either a GPIO or GPIO regulator and
> the driver for the WiFi chip should decide whether or not to register
> that as an rfkill driver.

Sadly, there are two ways to use rfkill right now:

1) the more common, and correct, way of having rfkill be a control tied
to a specific wireless interface (wifi, BT, FM, GPS, NFC, ...), to both
report the hardware button state that might be tied to it, and to
control - centrally - the software state.

2) the platform way, which some ACPI based platforms do, which register
an rfkill instance, which often allows controlling in software the
hardware line that then toggles the hardware rfkill on the WiFi NIC.


It's not clear to me what this patch is trying to achieve. It seems a
bit like something else entirely, which would be using it to toggle the
power for a wifi device? I agree that doesn't seem appropriate, and
instead the driver could bind to the regulator and disable it when wifi
gets disabled (by rfkill or simply by taking all interfaces down.)


In fact, given that there are no in-tree users, I'm tempted to remove
the rfkill-regulator entirely. Thoughts?

johannes