2011-05-07 00:40:28

by Rhyland Klein

[permalink] [raw]
Subject: [PATCH] add support for rfkill gpio devices

This is a rewrite of the bt_rfkill driver that Anantha first sent out
in mid April. I tried to take into account all comments, and ended up with
what I hope to be a simple yet useful base generic gpio rfkill driver.

Please feel free to point out areas that could use changes as I am new to
the rfkill subsystem and rfkill devices in general.

Rhyland Klein (1):
net: rfkill: add generic gpio rfkill driver

include/linux/rfkill-gpio.h | 45 ++++++++++
net/rfkill/Kconfig | 7 ++
net/rfkill/Makefile | 1 +
net/rfkill/rfkill-gpio.c | 199 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 252 insertions(+), 0 deletions(-)
create mode 100644 include/linux/rfkill-gpio.h
create mode 100644 net/rfkill/rfkill-gpio.c

--
1.7.5


2011-05-07 00:42:40

by Rhyland Klein

[permalink] [raw]
Subject: [PATCH] net: rfkill: add generic gpio rfkill driver

This adds a new generic gpio rfkill driver to support rfkill switches
which are controlled by gpios. The driver also supports passing in
data about the clock for the radio, so that when rfkill is blocking,
it can disable the clock.

This driver assumes platform data is passed from the board files to
configure it for specific devices.

Change-Id: I10bdce04c9d6e0489ad208ebbc6e78ab4424f4a8
From: Anantha Idapalapati <[email protected]>
Signed-off-by: Rhyland Klein <[email protected]>
---
include/linux/rfkill-gpio.h | 45 ++++++++++
net/rfkill/Kconfig | 7 ++
net/rfkill/Makefile | 1 +
net/rfkill/rfkill-gpio.c | 199 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 252 insertions(+), 0 deletions(-)
create mode 100644 include/linux/rfkill-gpio.h
create mode 100644 net/rfkill/rfkill-gpio.c

diff --git a/include/linux/rfkill-gpio.h b/include/linux/rfkill-gpio.h
new file mode 100644
index 0000000..c930881
--- /dev/null
+++ b/include/linux/rfkill-gpio.h
@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2011, NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+
+#ifndef __RFKILL_GPIO_H
+#define __RFKILL_GPIO_H
+
+#include <linux/types.h>
+#include <linux/rfkill.h>
+
+/**
+ * struct rfkill_gpio_platform_data - platform data for rfkill gpio device.
+ * for unused gpio's, the expected value is -1.
+ * @name: name for the gpio rf kill instance
+ * @reset_gpio: GPIO which is used for reseting rfkill switch
+ * @shutdown_gpio: GPIO which is used for shutdown of rfkill switch
+ * @power_clk_name: [optional] name of clk to turn off while blocked
+ * @blocked: block rf enabled (default is true)
+ */
+
+struct rfkill_gpio_platform_data {
+ char *name;
+ int reset_gpio;
+ int shutdown_gpio;
+ char *power_clk_name;
+ bool blocked;
+ enum rfkill_type type;
+};
+
+#endif /* __RFKILL_GPIO_H */
diff --git a/net/rfkill/Kconfig b/net/rfkill/Kconfig
index 48464ca..6313747 100644
--- a/net/rfkill/Kconfig
+++ b/net/rfkill/Kconfig
@@ -33,3 +33,10 @@ config RFKILL_REGULATOR

To compile this driver as a module, choose M here: the module will
be called rfkill-regulator.
+
+config RFKILL_GPIO
+ bool "GPIO RFKILL driver"
+ depends on RFKILL && GPIOLIB
+ help
+ If you say yes here you get support of a generic gpio RFKILL
+ driver. Platform needs to define the resources required
diff --git a/net/rfkill/Makefile b/net/rfkill/Makefile
index d9a5a58..3117687 100644
--- a/net/rfkill/Makefile
+++ b/net/rfkill/Makefile
@@ -6,3 +6,4 @@ rfkill-y += core.o
rfkill-$(CONFIG_RFKILL_INPUT) += input.o
obj-$(CONFIG_RFKILL) += rfkill.o
obj-$(CONFIG_RFKILL_REGULATOR) += rfkill-regulator.o
+obj-$(CONFIG_RFKILL_GPIO) += rfkill-gpio.o
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
new file mode 100644
index 0000000..c311820
--- /dev/null
+++ b/net/rfkill/rfkill-gpio.c
@@ -0,0 +1,199 @@
+/*
+ * Copyright (c) 2011, NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#include <linux/gpio.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/rfkill.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/slab.h>
+
+#include <linux/rfkill-gpio.h>
+
+/* length of names for gpios */
+#define GPIO_NAME_LEN 40
+
+struct rfkill_gpio_data {
+ struct rfkill_gpio_platform_data *pdata;
+ struct rfkill *rfkill_dev;
+ char reset_name[GPIO_NAME_LEN];
+ char shutdown_name[GPIO_NAME_LEN];
+ struct clk *pwr_clk;
+};
+
+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_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)
+ clk_disable(rfkill->pwr_clk);
+ } else {
+ if (rfkill->pwr_clk)
+ clk_enable(rfkill->pwr_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);
+ }
+
+ return 0;
+}
+
+static const struct rfkill_ops rfkill_gpio_ops = {
+ .set_block = rfkill_gpio_set_power,
+};
+
+static int rfkill_gpio_probe(struct platform_device *pdev)
+{
+ struct rfkill_gpio_data *rfkill;
+ struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
+ int ret = 0;
+
+ if (!pdata) {
+ pr_warn("%s: No platform data specified\n", __func__);
+ return -EINVAL;
+ }
+
+ /* 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))) {
+ pr_warn("%s: invalid platform data\n", __func__);
+ return -EINVAL;
+ }
+
+ rfkill = kzalloc(sizeof(*rfkill), GFP_KERNEL);
+ if (!rfkill)
+ return -ENOMEM;
+
+ rfkill->pdata = pdata;
+
+ snprintf(rfkill->reset_name, GPIO_NAME_LEN, "%s_reset", pdata->name);
+ snprintf(rfkill->shutdown_name, GPIO_NAME_LEN,
+ "%s_shutdown", pdata->name);
+
+ if (pdata->power_clk_name) {
+ rfkill->pwr_clk = clk_get(&pdev->dev, pdata->power_clk_name);
+ if (IS_ERR(rfkill->pwr_clk)) {
+ pr_warn("%s: can't find pwr_clk.\n", __func__);
+ goto fail_alloc;
+ }
+ }
+
+ if (gpio_is_valid(pdata->reset_gpio)) {
+ ret = gpio_request(pdata->reset_gpio, rfkill->reset_name);
+ if (ret) {
+ pr_warn("%s: failed to get reset gpio.\n", __func__);
+ goto fail_clock;
+ }
+ }
+
+ if (gpio_is_valid(pdata->shutdown_gpio)) {
+ ret = gpio_request(pdata->shutdown_gpio, rfkill->shutdown_name);
+ if (ret) {
+ pr_warn("%s: failed to get shutdown gpio.\n", __func__);
+ goto fail_reset;
+ }
+ }
+
+ rfkill->rfkill_dev = rfkill_alloc(pdata->name, &pdev->dev, pdata->type,
+ &rfkill_gpio_ops, rfkill);
+ if (!rfkill->rfkill_dev)
+ goto fail_shutdown;
+
+ /* setup initial state */
+ rfkill_gpio_set_power(rfkill, pdata->blocked);
+ rfkill_set_states(rfkill->rfkill_dev, pdata->blocked, false);
+
+ ret = rfkill_register(rfkill->rfkill_dev);
+ if (ret < 0)
+ goto fail_rfkill;
+
+ platform_set_drvdata(pdev, rfkill);
+
+ 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 && !pdata->blocked)
+ clk_disable(rfkill->pwr_clk);
+ if (rfkill->pwr_clk)
+ clk_put(rfkill->pwr_clk);
+fail_alloc:
+ kfree(rfkill);
+
+ return ret;
+}
+
+static int rfkill_gpio_remove(struct platform_device *pdev)
+{
+ struct rfkill_gpio_data *rfkill = platform_get_drvdata(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 && !rfkill->pdata->blocked)
+ clk_disable(rfkill->pwr_clk);
+ if (rfkill->pwr_clk)
+ clk_put(rfkill->pwr_clk);
+ kfree(rfkill);
+
+ 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,
+ },
+};
+
+static int __init rfkill_gpio_init(void)
+{
+ return platform_driver_register(&rfkill_gpio_driver);
+}
+
+static void __exit rfkill_gpio_exit(void)
+{
+ platform_driver_unregister(&rfkill_gpio_driver);
+}
+
+module_init(rfkill_gpio_init);
+module_exit(rfkill_gpio_exit);
+
+MODULE_DESCRIPTION("gpio rfkill");
+MODULE_AUTHOR("NVIDIA");
+MODULE_LICENSE("GPL");
--
1.7.5

2011-05-07 10:01:53

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] net: rfkill: add generic gpio rfkill driver

On Fri, 2011-05-06 at 17:40 -0700, Rhyland Klein wrote:

[snip]

Looks nice! A few comments below:

> + * @blocked: block rf enabled (default is true)

(see below)

> +struct rfkill_gpio_platform_data {
> + char *name;

> + char *power_clk_name;

const?

> + bool blocked;

The comments say this is default "true", but I don't think that makes
sense since this struct would be defined somewhere else and be static
const so zero-initialised. Also, are you sure that even having this
makes sense? (see below)

> +config RFKILL_GPIO
> + bool "GPIO RFKILL driver"
> + depends on RFKILL && GPIOLIB
> + help
> + If you say yes here you get support of a generic gpio RFKILL
> + driver. Platform needs to define the resources required

That appears to be cut short?


> +#define GPIO_NAME_LEN 40

Is there really no definition yet? Maybe it would make sense to
dynamically allocate?

> +struct rfkill_gpio_data {
> + struct rfkill_gpio_platform_data *pdata;
> + struct rfkill *rfkill_dev;
> + char reset_name[GPIO_NAME_LEN];
> + char shutdown_name[GPIO_NAME_LEN];
> + struct clk *pwr_clk;

That last line is a little oddly formatted.

> + /* setup initial state */
> + rfkill_gpio_set_power(rfkill, pdata->blocked);
> + rfkill_set_states(rfkill->rfkill_dev, pdata->blocked, false);
> +
> + ret = rfkill_register(rfkill->rfkill_dev);

So if you do this, rfkill core will set the persistent thing. We
discovered a while ago (and I mentioned that in the original thread)
there will be quirky behaviour. I would recommend leaving this out and
just relying on the rfkill core to call set_block() a little after
rfkill_register() to sync the state of this device with the desired
system state.

johannes