[PATCH 0/5] rfkill-gpio: ACPI support

[PATCH 0/5] rfkill-gpio: ACPI support

[Heikki Krogerus]

Hi,

The first patches prepare the driver for the support. The last patch
can then add the support quite easily. With these patches, adding DT
support later will be quite straight forward if someone needs it.


Heikki Krogerus (5):
  net: rfkill: gpio: convert to resource managed allocation
  net: rfkill: gpio: clean up clock handling
  net: rfkill: gpio: spinlock-safe GPIO access
  net: rfkill: gpio: prepare for DT and ACPI support
  net: rfkill: gpio: add ACPI support

 net/rfkill/Kconfig       |   2 +-
 net/rfkill/rfkill-gpio.c | 211 ++++++++++++++++++++++-------------------------
 2 files changed, 99 insertions(+), 114 deletions(-)

-- 
1.8.4.rc3

[PATCH 1/5] net: rfkill: gpio: convert to resource managed allocation

[Heikki Krogerus]

And remove now unneeded resource freeing.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 net/rfkill/rfkill-gpio.c | 75 +++++++++++++-----------------------------------
 1 file changed, 20 insertions(+), 55 deletions(-)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index fb076cd..0705806 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -97,7 +97,7 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	rfkill = kzalloc(sizeof(*rfkill), GFP_KERNEL);
+	rfkill = devm_kzalloc(&pdev->dev, sizeof(*rfkill), GFP_KERNEL);
 	if (!rfkill)
 		return -ENOMEM;
 
@@ -105,89 +105,65 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 		ret = pdata->gpio_runtime_setup(pdev);
 		if (ret) {
 			pr_warn("%s: can't set up gpio\n", __func__);
-			goto fail_alloc;
+			return ret;
 		}
 	}
 
 	rfkill->pdata = pdata;
 
 	len = strlen(pdata->name);
-	rfkill->reset_name = kzalloc(len + 7, GFP_KERNEL);
-	if (!rfkill->reset_name) {
-		ret = -ENOMEM;
-		goto fail_alloc;
-	}
+	rfkill->reset_name = devm_kzalloc(&pdev->dev, len + 7, GFP_KERNEL);
+	if (!rfkill->reset_name)
+		return -ENOMEM;
 
-	rfkill->shutdown_name = kzalloc(len + 10, GFP_KERNEL);
-	if (!rfkill->shutdown_name) {
-		ret = -ENOMEM;
-		goto fail_reset_name;
-	}
+	rfkill->shutdown_name = devm_kzalloc(&pdev->dev, len + 10, GFP_KERNEL);
+	if (!rfkill->shutdown_name)
+		return -ENOMEM;
 
 	snprintf(rfkill->reset_name, len + 6 , "%s_reset", pdata->name);
 	snprintf(rfkill->shutdown_name, len + 9, "%s_shutdown", pdata->name);
 
 	if (pdata->power_clk_name) {
-		rfkill->pwr_clk = clk_get(&pdev->dev, pdata->power_clk_name);
+		rfkill->pwr_clk = devm_clk_get(&pdev->dev,
+						pdata->power_clk_name);
 		if (IS_ERR(rfkill->pwr_clk)) {
 			pr_warn("%s: can't find pwr_clk.\n", __func__);
-			ret = PTR_ERR(rfkill->pwr_clk);
-			goto fail_shutdown_name;
+			return PTR_ERR(rfkill->pwr_clk);
 		}
 	}
 
 	if (gpio_is_valid(pdata->reset_gpio)) {
-		ret = gpio_request(pdata->reset_gpio, rfkill->reset_name);
+		ret = devm_gpio_request(&pdev->dev, pdata->reset_gpio,
+					rfkill->reset_name);
 		if (ret) {
 			pr_warn("%s: failed to get reset gpio.\n", __func__);
-			goto fail_clock;
+			return ret;
 		}
 	}
 
 	if (gpio_is_valid(pdata->shutdown_gpio)) {
-		ret = gpio_request(pdata->shutdown_gpio, rfkill->shutdown_name);
+		ret = devm_gpio_request(&pdev->dev, pdata->shutdown_gpio,
+					rfkill->shutdown_name);
 		if (ret) {
 			pr_warn("%s: failed to get shutdown gpio.\n", __func__);
-			goto fail_reset;
+			return ret;
 		}
 	}
 
 	rfkill->rfkill_dev = rfkill_alloc(pdata->name, &pdev->dev, pdata->type,
 					  &rfkill_gpio_ops, rfkill);
-	if (!rfkill->rfkill_dev) {
-		ret = -ENOMEM;
-		goto fail_shutdown;
-	}
+	if (!rfkill->rfkill_dev)
+		return -ENOMEM;
 
 	ret = rfkill_register(rfkill->rfkill_dev);
 	if (ret < 0)
-		goto fail_rfkill;
+		return ret;
 
 	platform_set_drvdata(pdev, rfkill);
 
 	dev_info(&pdev->dev, "%s device registered.\n", pdata->name);
 
 	return 0;
-
-fail_rfkill:
-	rfkill_destroy(rfkill->rfkill_dev);
-fail_shutdown:
-	if (gpio_is_valid(pdata->shutdown_gpio))
-		gpio_free(pdata->shutdown_gpio);
-fail_reset:
-	if (gpio_is_valid(pdata->reset_gpio))
-		gpio_free(pdata->reset_gpio);
-fail_clock:
-	if (rfkill->pwr_clk)
-		clk_put(rfkill->pwr_clk);
-fail_shutdown_name:
-	kfree(rfkill->shutdown_name);
-fail_reset_name:
-	kfree(rfkill->reset_name);
-fail_alloc:
-	kfree(rfkill);
-
-	return ret;
 }
 
 static int rfkill_gpio_remove(struct platform_device *pdev)
@@ -199,17 +175,6 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
 		pdata->gpio_runtime_close(pdev);
 	rfkill_unregister(rfkill->rfkill_dev);
 	rfkill_destroy(rfkill->rfkill_dev);
-	if (gpio_is_valid(rfkill->pdata->shutdown_gpio))
-		gpio_free(rfkill->pdata->shutdown_gpio);
-	if (gpio_is_valid(rfkill->pdata->reset_gpio))
-		gpio_free(rfkill->pdata->reset_gpio);
-	if (rfkill->pwr_clk && PWR_CLK_ENABLED(rfkill))
-		clk_disable(rfkill->pwr_clk);
-	if (rfkill->pwr_clk)
-		clk_put(rfkill->pwr_clk);
-	kfree(rfkill->shutdown_name);
-	kfree(rfkill->reset_name);
-	kfree(rfkill);
 
 	return 0;
 }
-- 
1.8.4.rc3

[PATCH 2/5] net: rfkill: gpio: clean up clock handling

[Heikki Krogerus]

Use a simple flag to see the state of the clock, and make
the clock available even without a name. Also, get rid of
HAVE_CLK dependency.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 net/rfkill/Kconfig       |  2 +-
 net/rfkill/rfkill-gpio.c | 35 ++++++++---------------------------
 2 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/net/rfkill/Kconfig b/net/rfkill/Kconfig
index 78efe89..4c10e7e 100644
--- a/net/rfkill/Kconfig
+++ b/net/rfkill/Kconfig
@@ -36,7 +36,7 @@ config RFKILL_REGULATOR
 
 config RFKILL_GPIO
 	tristate "GPIO RFKILL driver"
-	depends on RFKILL && GPIOLIB && HAVE_CLK
+	depends on RFKILL && GPIOLIB
 	default n
 	help
 	  If you say yes here you get support of a generic gpio RFKILL
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 0705806..1d104e7 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -27,24 +27,13 @@
 
 #include <linux/rfkill-gpio.h>
 
-enum rfkill_gpio_clk_state {
-	UNSPECIFIED = 0,
-	PWR_ENABLED,
-	PWR_DISABLED
-};
-
-#define PWR_CLK_SET(_RF, _EN) \
-	((_RF)->pwr_clk_enabled = (!(_EN) ? PWR_ENABLED : PWR_DISABLED))
-#define PWR_CLK_ENABLED(_RF) ((_RF)->pwr_clk_enabled == PWR_ENABLED)
-#define PWR_CLK_DISABLED(_RF) ((_RF)->pwr_clk_enabled != PWR_ENABLED)
-
 struct rfkill_gpio_data {
 	struct rfkill_gpio_platform_data	*pdata;
 	struct rfkill				*rfkill_dev;
 	char					*reset_name;
 	char					*shutdown_name;
-	enum rfkill_gpio_clk_state		pwr_clk_enabled;
-	struct clk				*pwr_clk;
+	struct clk				*clk;
+	bool					clk_enabled;
 };
 
 static int rfkill_gpio_set_power(void *data, bool blocked)
@@ -56,19 +45,18 @@ static int rfkill_gpio_set_power(void *data, bool blocked)
 			gpio_direction_output(rfkill->pdata->shutdown_gpio, 0);
 		if (gpio_is_valid(rfkill->pdata->reset_gpio))
 			gpio_direction_output(rfkill->pdata->reset_gpio, 0);
-		if (rfkill->pwr_clk && PWR_CLK_ENABLED(rfkill))
-			clk_disable(rfkill->pwr_clk);
+		if (!IS_ERR(rfkill->clk) && rfkill->clk_enabled)
+			clk_disable(rfkill->clk);
 	} else {
-		if (rfkill->pwr_clk && PWR_CLK_DISABLED(rfkill))
-			clk_enable(rfkill->pwr_clk);
+		if (!IS_ERR(rfkill->clk) && !rfkill->clk_enabled)
+			clk_enable(rfkill->clk);
 		if (gpio_is_valid(rfkill->pdata->reset_gpio))
 			gpio_direction_output(rfkill->pdata->reset_gpio, 1);
 		if (gpio_is_valid(rfkill->pdata->shutdown_gpio))
 			gpio_direction_output(rfkill->pdata->shutdown_gpio, 1);
 	}
 
-	if (rfkill->pwr_clk)
-		PWR_CLK_SET(rfkill, blocked);
+	rfkill->clk_enabled = blocked;
 
 	return 0;
 }
@@ -123,14 +111,7 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 	snprintf(rfkill->reset_name, len + 6 , "%s_reset", pdata->name);
 	snprintf(rfkill->shutdown_name, len + 9, "%s_shutdown", pdata->name);
 
-	if (pdata->power_clk_name) {
-		rfkill->pwr_clk = devm_clk_get(&pdev->dev,
-						pdata->power_clk_name);
-		if (IS_ERR(rfkill->pwr_clk)) {
-			pr_warn("%s: can't find pwr_clk.\n", __func__);
-			return PTR_ERR(rfkill->pwr_clk);
-		}
-	}
+	rfkill->clk = devm_clk_get(&pdev->dev, pdata->power_clk_name);
 
 	if (gpio_is_valid(pdata->reset_gpio)) {
 		ret = devm_gpio_request(&pdev->dev, pdata->reset_gpio,
-- 
1.8.4.rc3

[PATCH 3/5] net: rfkill: gpio: spinlock-safe GPIO access

[Heikki Krogerus]

This sets the direction of the gpio once when it's requested,
and uses the spinlock-safe gpio_set_state() to change the
state.

Signed-off-by: Heikki Krogerus <heikki.krogerus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 net/rfkill/rfkill-gpio.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 1d104e7..aa4ac10 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -42,18 +42,18 @@ static int rfkill_gpio_set_power(void *data, bool blocked)
 
 	if (blocked) {
 		if (gpio_is_valid(rfkill->pdata->shutdown_gpio))
-			gpio_direction_output(rfkill->pdata->shutdown_gpio, 0);
+			gpio_set_value(rfkill->pdata->shutdown_gpio, 0);
 		if (gpio_is_valid(rfkill->pdata->reset_gpio))
-			gpio_direction_output(rfkill->pdata->reset_gpio, 0);
+			gpio_set_value(rfkill->pdata->reset_gpio, 0);
 		if (!IS_ERR(rfkill->clk) && rfkill->clk_enabled)
 			clk_disable(rfkill->clk);
 	} else {
 		if (!IS_ERR(rfkill->clk) && !rfkill->clk_enabled)
 			clk_enable(rfkill->clk);
 		if (gpio_is_valid(rfkill->pdata->reset_gpio))
-			gpio_direction_output(rfkill->pdata->reset_gpio, 1);
+			gpio_set_value(rfkill->pdata->reset_gpio, 1);
 		if (gpio_is_valid(rfkill->pdata->shutdown_gpio))
-			gpio_direction_output(rfkill->pdata->shutdown_gpio, 1);
+			gpio_set_value(rfkill->pdata->shutdown_gpio, 1);
 	}
 
 	rfkill->clk_enabled = blocked;
@@ -114,8 +114,8 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 	rfkill->clk = devm_clk_get(&pdev->dev, pdata->power_clk_name);
 
 	if (gpio_is_valid(pdata->reset_gpio)) {
-		ret = devm_gpio_request(&pdev->dev, pdata->reset_gpio,
-					rfkill->reset_name);
+		ret = devm_gpio_request_one(&pdev->dev, pdata->reset_gpio,
+					    0, rfkill->reset_name);
 		if (ret) {
 			pr_warn("%s: failed to get reset gpio.\n", __func__);
 			return ret;
@@ -123,8 +123,8 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 	}
 
 	if (gpio_is_valid(pdata->shutdown_gpio)) {
-		ret = devm_gpio_request(&pdev->dev, pdata->shutdown_gpio,
-					rfkill->shutdown_name);
+		ret = devm_gpio_request_one(&pdev->dev, pdata->shutdown_gpio,
+					    0, rfkill->shutdown_name);
 		if (ret) {
 			pr_warn("%s: failed to get shutdown gpio.\n", __func__);
 			return ret;
-- 
1.8.4.rc3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] net: rfkill: gpio: add ACPI support

[Heikki Krogerus]

Including ACPI ID for Broadcom GPS receiver BCM4752.

Signed-off-by: Heikki Krogerus <heikki.krogerus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 net/rfkill/rfkill-gpio.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 2dd78c6..5620d3c 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -24,6 +24,8 @@
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/slab.h>
+#include <linux/acpi.h>
+#include <linux/acpi_gpio.h>
 
 #include <linux/rfkill-gpio.h>
 
@@ -70,6 +72,23 @@ static const struct rfkill_ops rfkill_gpio_ops = {
 	.set_block = rfkill_gpio_set_power,
 };
 
+static int rfkill_gpio_acpi_probe(struct device *dev,
+				  struct rfkill_gpio_data *rfkill)
+{
+	const struct acpi_device_id *id;
+
+	id = acpi_match_device(dev->driver->acpi_match_table, dev);
+	if (!id)
+		return -ENODEV;
+
+	rfkill->name = dev_name(dev);
+	rfkill->type = (unsigned)id->driver_data;
+	rfkill->reset_gpio = acpi_get_gpio_by_index(dev, 0, NULL);
+	rfkill->shutdown_gpio = acpi_get_gpio_by_index(dev, 1, NULL);
+
+	return 0;
+}
+
 static int rfkill_gpio_probe(struct platform_device *pdev)
 {
 	struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
@@ -82,7 +101,11 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 	if (!rfkill)
 		return -ENOMEM;
 
-	if (pdata) {
+	if (ACPI_HANDLE(&pdev->dev)) {
+		ret = rfkill_gpio_acpi_probe(&pdev->dev, rfkill);
+		if (ret)
+			return ret;
+	} else if (pdata) {
 		clk_name = pdata->power_clk_name;
 		rfkill->name = pdata->name;
 		rfkill->type = pdata->type;
@@ -170,12 +193,18 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct acpi_device_id rfkill_acpi_match[] = {
+	{ "BCM4752", RFKILL_TYPE_GPS },
+	{ },
+};
+
 static struct platform_driver rfkill_gpio_driver = {
 	.probe = rfkill_gpio_probe,
 	.remove = rfkill_gpio_remove,
 	.driver = {
 		.name = "rfkill_gpio",
 		.owner = THIS_MODULE,
+		.acpi_match_table = ACPI_PTR(rfkill_acpi_match),
 	},
 };
 
-- 
1.8.4.rc3

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] net: rfkill: gpio: add ACPI support

[Rafael J. Wysocki]

On Wednesday, October 16, 2013 01:53:43 PM Heikki Krogerus wrote:
> Including ACPI ID for Broadcom GPS receiver BCM4752.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  net/rfkill/rfkill-gpio.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
> index 2dd78c6..5620d3c 100644
> --- a/net/rfkill/rfkill-gpio.c
> +++ b/net/rfkill/rfkill-gpio.c
> @@ -24,6 +24,8 @@
>  #include <linux/platform_device.h>
>  #include <linux/clk.h>
>  #include <linux/slab.h>
> +#include <linux/acpi.h>
> +#include <linux/acpi_gpio.h>
>  
>  #include <linux/rfkill-gpio.h>
>  
> @@ -70,6 +72,23 @@ static const struct rfkill_ops rfkill_gpio_ops = {
>  	.set_block = rfkill_gpio_set_power,
>  };
>  
> +static int rfkill_gpio_acpi_probe(struct device *dev,
> +				  struct rfkill_gpio_data *rfkill)
> +{
> +	const struct acpi_device_id *id;
> +
> +	id = acpi_match_device(dev->driver->acpi_match_table, dev);
> +	if (!id)
> +		return -ENODEV;
> +
> +	rfkill->name = dev_name(dev);
> +	rfkill->type = (unsigned)id->driver_data;
> +	rfkill->reset_gpio = acpi_get_gpio_by_index(dev, 0, NULL);
> +	rfkill->shutdown_gpio = acpi_get_gpio_by_index(dev, 1, NULL);
> +
> +	return 0;
> +}
> +
>  static int rfkill_gpio_probe(struct platform_device *pdev)
>  {
>  	struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
> @@ -82,7 +101,11 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
>  	if (!rfkill)
>  		return -ENOMEM;
>  
> -	if (pdata) {
> +	if (ACPI_HANDLE(&pdev->dev)) {
> +		ret = rfkill_gpio_acpi_probe(&pdev->dev, rfkill);
> +		if (ret)
> +			return ret;
> +	} else if (pdata) {
>  		clk_name = pdata->power_clk_name;
>  		rfkill->name = pdata->name;
>  		rfkill->type = pdata->type;
> @@ -170,12 +193,18 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct acpi_device_id rfkill_acpi_match[] = {
> +	{ "BCM4752", RFKILL_TYPE_GPS },
> +	{ },
> +};
> +
>  static struct platform_driver rfkill_gpio_driver = {
>  	.probe = rfkill_gpio_probe,
>  	.remove = rfkill_gpio_remove,
>  	.driver = {
>  		.name = "rfkill_gpio",
>  		.owner = THIS_MODULE,
> +		.acpi_match_table = ACPI_PTR(rfkill_acpi_match),
>  	},
>  };

Looks good to me.

Has Mika seen this?

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 5/5] net: rfkill: gpio: add ACPI support

[Mika Westerberg]

On Wed, Oct 16, 2013 at 10:55:01PM +0200, Rafael J. Wysocki wrote:
> On Wednesday, October 16, 2013 01:53:43 PM Heikki Krogerus wrote:
> > Including ACPI ID for Broadcom GPS receiver BCM4752.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  net/rfkill/rfkill-gpio.c | 31 ++++++++++++++++++++++++++++++-
> >  1 file changed, 30 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
> > index 2dd78c6..5620d3c 100644
> > --- a/net/rfkill/rfkill-gpio.c
> > +++ b/net/rfkill/rfkill-gpio.c
> > @@ -24,6 +24,8 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/clk.h>
> >  #include <linux/slab.h>
> > +#include <linux/acpi.h>
> > +#include <linux/acpi_gpio.h>
> >  
> >  #include <linux/rfkill-gpio.h>
> >  
> > @@ -70,6 +72,23 @@ static const struct rfkill_ops rfkill_gpio_ops = {
> >  	.set_block = rfkill_gpio_set_power,
> >  };
> >  
> > +static int rfkill_gpio_acpi_probe(struct device *dev,
> > +				  struct rfkill_gpio_data *rfkill)
> > +{
> > +	const struct acpi_device_id *id;
> > +
> > +	id = acpi_match_device(dev->driver->acpi_match_table, dev);
> > +	if (!id)
> > +		return -ENODEV;
> > +
> > +	rfkill->name = dev_name(dev);
> > +	rfkill->type = (unsigned)id->driver_data;
> > +	rfkill->reset_gpio = acpi_get_gpio_by_index(dev, 0, NULL);
> > +	rfkill->shutdown_gpio = acpi_get_gpio_by_index(dev, 1, NULL);
> > +
> > +	return 0;
> > +}
> > +
> >  static int rfkill_gpio_probe(struct platform_device *pdev)
> >  {
> >  	struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
> > @@ -82,7 +101,11 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
> >  	if (!rfkill)
> >  		return -ENOMEM;
> >  
> > -	if (pdata) {
> > +	if (ACPI_HANDLE(&pdev->dev)) {
> > +		ret = rfkill_gpio_acpi_probe(&pdev->dev, rfkill);
> > +		if (ret)
> > +			return ret;
> > +	} else if (pdata) {
> >  		clk_name = pdata->power_clk_name;
> >  		rfkill->name = pdata->name;
> >  		rfkill->type = pdata->type;
> > @@ -170,12 +193,18 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >  
> > +static const struct acpi_device_id rfkill_acpi_match[] = {
> > +	{ "BCM4752", RFKILL_TYPE_GPS },
> > +	{ },
> > +};
> > +
> >  static struct platform_driver rfkill_gpio_driver = {
> >  	.probe = rfkill_gpio_probe,
> >  	.remove = rfkill_gpio_remove,
> >  	.driver = {
> >  		.name = "rfkill_gpio",
> >  		.owner = THIS_MODULE,
> > +		.acpi_match_table = ACPI_PTR(rfkill_acpi_match),
> >  	},
> >  };
> 
> Looks good to me.
> 
> Has Mika seen this?

Yes, saw it now and looks good to me as well.

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

for the whole series, for what it's worth.

Re: [PATCH 5/5] net: rfkill: gpio: add ACPI support

[Rafael J. Wysocki]

On Thursday, October 17, 2013 10:44:26 AM Mika Westerberg wrote:
> On Wed, Oct 16, 2013 at 10:55:01PM +0200, Rafael J. Wysocki wrote:
> > On Wednesday, October 16, 2013 01:53:43 PM Heikki Krogerus wrote:
> > > Including ACPI ID for Broadcom GPS receiver BCM4752.
> > > 
> > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > ---
> > >  net/rfkill/rfkill-gpio.c | 31 ++++++++++++++++++++++++++++++-
> > >  1 file changed, 30 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
> > > index 2dd78c6..5620d3c 100644
> > > --- a/net/rfkill/rfkill-gpio.c
> > > +++ b/net/rfkill/rfkill-gpio.c
> > > @@ -24,6 +24,8 @@
> > >  #include <linux/platform_device.h>
> > >  #include <linux/clk.h>
> > >  #include <linux/slab.h>
> > > +#include <linux/acpi.h>
> > > +#include <linux/acpi_gpio.h>
> > >  
> > >  #include <linux/rfkill-gpio.h>
> > >  
> > > @@ -70,6 +72,23 @@ static const struct rfkill_ops rfkill_gpio_ops = {
> > >  	.set_block = rfkill_gpio_set_power,
> > >  };
> > >  
> > > +static int rfkill_gpio_acpi_probe(struct device *dev,
> > > +				  struct rfkill_gpio_data *rfkill)
> > > +{
> > > +	const struct acpi_device_id *id;
> > > +
> > > +	id = acpi_match_device(dev->driver->acpi_match_table, dev);
> > > +	if (!id)
> > > +		return -ENODEV;
> > > +
> > > +	rfkill->name = dev_name(dev);
> > > +	rfkill->type = (unsigned)id->driver_data;
> > > +	rfkill->reset_gpio = acpi_get_gpio_by_index(dev, 0, NULL);
> > > +	rfkill->shutdown_gpio = acpi_get_gpio_by_index(dev, 1, NULL);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  static int rfkill_gpio_probe(struct platform_device *pdev)
> > >  {
> > >  	struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
> > > @@ -82,7 +101,11 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
> > >  	if (!rfkill)
> > >  		return -ENOMEM;
> > >  
> > > -	if (pdata) {
> > > +	if (ACPI_HANDLE(&pdev->dev)) {
> > > +		ret = rfkill_gpio_acpi_probe(&pdev->dev, rfkill);
> > > +		if (ret)
> > > +			return ret;
> > > +	} else if (pdata) {
> > >  		clk_name = pdata->power_clk_name;
> > >  		rfkill->name = pdata->name;
> > >  		rfkill->type = pdata->type;
> > > @@ -170,12 +193,18 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
> > >  	return 0;
> > >  }
> > >  
> > > +static const struct acpi_device_id rfkill_acpi_match[] = {
> > > +	{ "BCM4752", RFKILL_TYPE_GPS },
> > > +	{ },
> > > +};
> > > +
> > >  static struct platform_driver rfkill_gpio_driver = {
> > >  	.probe = rfkill_gpio_probe,
> > >  	.remove = rfkill_gpio_remove,
> > >  	.driver = {
> > >  		.name = "rfkill_gpio",
> > >  		.owner = THIS_MODULE,
> > > +		.acpi_match_table = ACPI_PTR(rfkill_acpi_match),
> > >  	},
> > >  };
> > 
> > Looks good to me.
> > 
> > Has Mika seen this?
> 
> Yes, saw it now and looks good to me as well.
> 
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> for the whole series, for what it's worth.

OK, so

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

for the ACPI-related changes.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH 4/5] net: rfkill: gpio: prepare for DT and ACPI support

[Heikki Krogerus]

This will add the relevant values like the gpios and the
type in rfkill_gpio_platform_data to the rfkill_gpio_data
structure. It will allow those values to be easily picked
from DT and ACPI tables later.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 net/rfkill/rfkill-gpio.c | 92 +++++++++++++++++++++++++++---------------------
 1 file changed, 51 insertions(+), 41 deletions(-)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index aa4ac10..2dd78c6 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -28,12 +28,17 @@
 #include <linux/rfkill-gpio.h>
 
 struct rfkill_gpio_data {
-	struct rfkill_gpio_platform_data	*pdata;
-	struct rfkill				*rfkill_dev;
-	char					*reset_name;
-	char					*shutdown_name;
-	struct clk				*clk;
-	bool					clk_enabled;
+	const char		*name;
+	enum rfkill_type	type;
+	int			reset_gpio;
+	int			shutdown_gpio;
+
+	struct rfkill		*rfkill_dev;
+	char			*reset_name;
+	char			*shutdown_name;
+	struct clk		*clk;
+
+	bool			clk_enabled;
 };
 
 static int rfkill_gpio_set_power(void *data, bool blocked)
@@ -41,19 +46,19 @@ static int rfkill_gpio_set_power(void *data, bool blocked)
 	struct rfkill_gpio_data *rfkill = data;
 
 	if (blocked) {
-		if (gpio_is_valid(rfkill->pdata->shutdown_gpio))
-			gpio_set_value(rfkill->pdata->shutdown_gpio, 0);
-		if (gpio_is_valid(rfkill->pdata->reset_gpio))
-			gpio_set_value(rfkill->pdata->reset_gpio, 0);
+		if (gpio_is_valid(rfkill->shutdown_gpio))
+			gpio_set_value(rfkill->shutdown_gpio, 0);
+		if (gpio_is_valid(rfkill->reset_gpio))
+			gpio_set_value(rfkill->reset_gpio, 0);
 		if (!IS_ERR(rfkill->clk) && rfkill->clk_enabled)
 			clk_disable(rfkill->clk);
 	} else {
 		if (!IS_ERR(rfkill->clk) && !rfkill->clk_enabled)
 			clk_enable(rfkill->clk);
-		if (gpio_is_valid(rfkill->pdata->reset_gpio))
-			gpio_set_value(rfkill->pdata->reset_gpio, 1);
-		if (gpio_is_valid(rfkill->pdata->shutdown_gpio))
-			gpio_set_value(rfkill->pdata->shutdown_gpio, 1);
+		if (gpio_is_valid(rfkill->reset_gpio))
+			gpio_set_value(rfkill->reset_gpio, 1);
+		if (gpio_is_valid(rfkill->shutdown_gpio))
+			gpio_set_value(rfkill->shutdown_gpio, 1);
 	}
 
 	rfkill->clk_enabled = blocked;
@@ -67,29 +72,35 @@ static const struct rfkill_ops rfkill_gpio_ops = {
 
 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 rfkill_gpio_data *rfkill;
+	const char *clk_name = NULL;
 	int ret = 0;
 	int len = 0;
 
-	if (!pdata) {
-		pr_warn("%s: No platform data specified\n", __func__);
-		return -EINVAL;
+	rfkill = devm_kzalloc(&pdev->dev, sizeof(*rfkill), GFP_KERNEL);
+	if (!rfkill)
+		return -ENOMEM;
+
+	if (pdata) {
+		clk_name = pdata->power_clk_name;
+		rfkill->name = pdata->name;
+		rfkill->type = pdata->type;
+		rfkill->reset_gpio = pdata->reset_gpio;
+		rfkill->shutdown_gpio = pdata->shutdown_gpio;
+	} else {
+		return -ENODEV;
 	}
 
 	/* make sure at-least one of the GPIO is defined and that
 	 * a name is specified for this instance */
-	if (!pdata->name || (!gpio_is_valid(pdata->reset_gpio) &&
-		!gpio_is_valid(pdata->shutdown_gpio))) {
+	if ((!gpio_is_valid(rfkill->reset_gpio) &&
+	     !gpio_is_valid(rfkill->shutdown_gpio)) || !rfkill->name) {
 		pr_warn("%s: invalid platform data\n", __func__);
 		return -EINVAL;
 	}
 
-	rfkill = devm_kzalloc(&pdev->dev, sizeof(*rfkill), GFP_KERNEL);
-	if (!rfkill)
-		return -ENOMEM;
-
-	if (pdata->gpio_runtime_setup) {
+	if (pdata && pdata->gpio_runtime_setup) {
 		ret = pdata->gpio_runtime_setup(pdev);
 		if (ret) {
 			pr_warn("%s: can't set up gpio\n", __func__);
@@ -97,9 +108,7 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 		}
 	}
 
-	rfkill->pdata = pdata;
-
-	len = strlen(pdata->name);
+	len = strlen(rfkill->name);
 	rfkill->reset_name = devm_kzalloc(&pdev->dev, len + 7, GFP_KERNEL);
 	if (!rfkill->reset_name)
 		return -ENOMEM;
@@ -108,13 +117,13 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 	if (!rfkill->shutdown_name)
 		return -ENOMEM;
 
-	snprintf(rfkill->reset_name, len + 6 , "%s_reset", pdata->name);
-	snprintf(rfkill->shutdown_name, len + 9, "%s_shutdown", pdata->name);
+	snprintf(rfkill->reset_name, len + 6 , "%s_reset", rfkill->name);
+	snprintf(rfkill->shutdown_name, len + 9, "%s_shutdown", rfkill->name);
 
-	rfkill->clk = devm_clk_get(&pdev->dev, pdata->power_clk_name);
+	rfkill->clk = devm_clk_get(&pdev->dev, clk_name);
 
-	if (gpio_is_valid(pdata->reset_gpio)) {
-		ret = devm_gpio_request_one(&pdev->dev, pdata->reset_gpio,
+	if (gpio_is_valid(rfkill->reset_gpio)) {
+		ret = devm_gpio_request_one(&pdev->dev, rfkill->reset_gpio,
 					    0, rfkill->reset_name);
 		if (ret) {
 			pr_warn("%s: failed to get reset gpio.\n", __func__);
@@ -122,8 +131,8 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (gpio_is_valid(pdata->shutdown_gpio)) {
-		ret = devm_gpio_request_one(&pdev->dev, pdata->shutdown_gpio,
+	if (gpio_is_valid(rfkill->shutdown_gpio)) {
+		ret = devm_gpio_request_one(&pdev->dev, rfkill->shutdown_gpio,
 					    0, rfkill->shutdown_name);
 		if (ret) {
 			pr_warn("%s: failed to get shutdown gpio.\n", __func__);
@@ -131,8 +140,9 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 		}
 	}
 
-	rfkill->rfkill_dev = rfkill_alloc(pdata->name, &pdev->dev, pdata->type,
-					  &rfkill_gpio_ops, rfkill);
+	rfkill->rfkill_dev = rfkill_alloc(rfkill->name, &pdev->dev,
+					  rfkill->type, &rfkill_gpio_ops,
+					  rfkill);
 	if (!rfkill->rfkill_dev)
 		return -ENOMEM;
 
@@ -142,7 +152,7 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, rfkill);
 
-	dev_info(&pdev->dev, "%s device registered.\n", pdata->name);
+	dev_info(&pdev->dev, "%s device registered.\n", rfkill->name);
 
 	return 0;
 }
@@ -152,7 +162,7 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
 	struct rfkill_gpio_data *rfkill = platform_get_drvdata(pdev);
 	struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
 
-	if (pdata->gpio_runtime_close)
+	if (pdata && pdata->gpio_runtime_close)
 		pdata->gpio_runtime_close(pdev);
 	rfkill_unregister(rfkill->rfkill_dev);
 	rfkill_destroy(rfkill->rfkill_dev);
@@ -164,8 +174,8 @@ static struct platform_driver rfkill_gpio_driver = {
 	.probe = rfkill_gpio_probe,
 	.remove = rfkill_gpio_remove,
 	.driver = {
-		   .name = "rfkill_gpio",
-		   .owner = THIS_MODULE,
+		.name = "rfkill_gpio",
+		.owner = THIS_MODULE,
 	},
 };
 
-- 
1.8.4.rc3

Re: [PATCH 0/5] rfkill-gpio: ACPI support

[Rhyland Klein]

On 10/16/2013 6:53 AM, Heikki Krogerus wrote:
> Hi,
> 
> The first patches prepare the driver for the support. The last patch
> can then add the support quite easily. With these patches, adding DT
> support later will be quite straight forward if someone needs it.
> 
> 
> Heikki Krogerus (5):
>   net: rfkill: gpio: convert to resource managed allocation
>   net: rfkill: gpio: clean up clock handling
>   net: rfkill: gpio: spinlock-safe GPIO access
>   net: rfkill: gpio: prepare for DT and ACPI support
>   net: rfkill: gpio: add ACPI support
> 
>  net/rfkill/Kconfig       |   2 +-
>  net/rfkill/rfkill-gpio.c | 211 ++++++++++++++++++++++-------------------------
>  2 files changed, 99 insertions(+), 114 deletions(-)
> 

Strictly speaking, duplicating the pdata fields into the
rfkill_gpio_data structure isn't really necessary. Many drivers simply
have the dt parsing or in this case ACPI parsing generate a
platform_data structure which would then be saved in the
rfkill_gpio_data structure.

In this case, since it is only a few fields, I am not too worried and I
am fine either way.

Acked-by: Rhyland Klein <rklein@nvidia.com>

-- 
nvpublic

Re: [PATCH 0/5] rfkill-gpio: ACPI support

[Johannes Berg]

On Wed, 2013-10-16 at 13:53 +0300, Heikki Krogerus wrote:
> Hi,
> 
> The first patches prepare the driver for the support. The last patch
> can then add the support quite easily. With these patches, adding DT
> support later will be quite straight forward if someone needs it.

Applied. I'm totally relying on all the reviewers though, since I have
very little idea of what's going on. If anyone else wants to maintain
the rfkill-gpio driver I'd welcome that :-)

johannes