[PATCH 0/5] net: rfkill: gpio: replace platform data with build-in property

[PATCH 0/5] net: rfkill: gpio: replace platform data with build-in property

[Heikki Krogerus]

The first patch adds a few helper macros for build-in property
creation and the second makes it possible to get the rfkill type
index based on name. The rest deal with rfkill-gpio.

Cheers,


Heikki Krogerus (5):
  device property: helper macros for property entry creation
  net: rfkill: add rfkill_find_type function
  net: rfkill: gpio: get the name and type from device property
  ARM: tegra: use build-in device properties with rfkill_gpio
  net: rfkill: gpio: remove rfkill_gpio_platform_data

 arch/arm/mach-tegra/board-paz00.c | 17 +++++++-----
 include/linux/property.h          | 35 ++++++++++++++++++++++++
 include/linux/rfkill-gpio.h       | 37 -------------------------
 include/linux/rfkill.h            | 15 +++++++++++
 net/rfkill/Kconfig                |  3 +--
 net/rfkill/core.c                 | 57 ++++++++++++++++++++-------------------
 net/rfkill/rfkill-gpio.c          | 24 ++++++++---------
 7 files changed, 101 insertions(+), 87 deletions(-)
 delete mode 100644 include/linux/rfkill-gpio.h

-- 
2.4.6

[PATCH 1/5] device property: helper macros for property entry creation

[Heikki Krogerus]

Marcos for easier creation of build-in property entries.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 include/linux/property.h | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/include/linux/property.h b/include/linux/property.h
index 76ebde9..204d899 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -152,6 +152,41 @@ struct property_entry {
 	} value;
 };
 
+#define PROP_ENTRY_U8(_name_, _val_) { \
+	.name = _name_, \
+	.type = DEV_PROP_U8, \
+	.nval = 1, \
+	.value.u8_data = _val_, \
+}
+
+#define PROP_ENTRY_U16(_name_, _val_) { \
+	.name = _name_, \
+	.type = DEV_PROP_U16, \
+	.nval = 1, \
+	.value.u16_data = _val_, \
+}
+
+#define PROP_ENTRY_U32(_name_, _val_) { \
+	.name = _name_, \
+	.type = DEV_PROP_U32, \
+	.nval = 1, \
+	.value.u32_data = _val_, \
+}
+
+#define PROP_ENTRY_U64(_name_, _val_) { \
+	.name = _name_, \
+	.type = DEV_PROP_U64, \
+	.nval = 1, \
+	.value.u64_data = _val_, \
+}
+
+#define PROP_ENTRY_STRING(_name_, _val_) { \
+	.name = _name_, \
+	.type = DEV_PROP_STRING, \
+	.nval = 1, \
+	.value.str = (const char **)_val_, \
+}
+
 /**
  * struct property_set - Collection of "built-in" device properties.
  * @fwnode: Handle to be pointed to by the fwnode field of struct device.
-- 
2.4.6

[PATCH 2/5] net: rfkill: add rfkill_find_type function

[Heikki Krogerus]

Helper for finding the type based on name. Useful if the
type needs to be determined based on device property.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 include/linux/rfkill.h | 15 +++++++++++++
 net/rfkill/core.c      | 57 +++++++++++++++++++++++++-------------------------
 2 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h
index d901078..02f563c 100644
--- a/include/linux/rfkill.h
+++ b/include/linux/rfkill.h
@@ -212,6 +212,15 @@ void rfkill_set_states(struct rfkill *rfkill, bool sw, bool hw);
  * @rfkill: rfkill struct to query
  */
 bool rfkill_blocked(struct rfkill *rfkill);
+
+/**
+ * rfkill_find_type - Helpper for finding rfkill type by name
+ * @name: the name of the type
+ *
+ * Returns enum rfkill_type that conrresponds the name.
+ */
+enum rfkill_type rfkill_find_type(const char *name);
+
 #else /* !RFKILL */
 static inline struct rfkill * __must_check
 rfkill_alloc(const char *name,
@@ -268,6 +277,12 @@ static inline bool rfkill_blocked(struct rfkill *rfkill)
 {
 	return false;
 }
+
+static inline enum rfkill_type rfkill_find_type(const char *name)
+{
+	return 0;
+}
+
 #endif /* RFKILL || RFKILL_MODULE */
 
 
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index f12149a..53d7a97e 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -574,6 +574,33 @@ void rfkill_set_states(struct rfkill *rfkill, bool sw, bool hw)
 }
 EXPORT_SYMBOL(rfkill_set_states);
 
+static const char *rfkill_types[NUM_RFKILL_TYPES] = {
+	[RFKILL_TYPE_WLAN]	= "wlan",
+	[RFKILL_TYPE_BLUETOOTH]	= "bluetooth",
+	[RFKILL_TYPE_UWB]	= "ultrawideband",
+	[RFKILL_TYPE_WIMAX]	= "wimax",
+	[RFKILL_TYPE_WWAN]	= "wwan",
+	[RFKILL_TYPE_GPS]	= "gps",
+	[RFKILL_TYPE_FM]	= "fm",
+	[RFKILL_TYPE_NFC]	= "nfc",
+};
+
+enum rfkill_type rfkill_find_type(const char *name)
+{
+	int i;
+
+	BUILD_BUG_ON(NUM_RFKILL_TYPES != RFKILL_TYPE_NFC + 1);
+
+	if (!name)
+		return RFKILL_TYPE_ALL;
+
+	for (i = 1; i < NUM_RFKILL_TYPES; i++)
+		if (!strcmp(name, rfkill_types[i]))
+			return i;
+	return RFKILL_TYPE_ALL;
+}
+EXPORT_SYMBOL(rfkill_find_type);
+
 static ssize_t name_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
@@ -583,38 +610,12 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(name);
 
-static const char *rfkill_get_type_str(enum rfkill_type type)
-{
-	BUILD_BUG_ON(NUM_RFKILL_TYPES != RFKILL_TYPE_NFC + 1);
-
-	switch (type) {
-	case RFKILL_TYPE_WLAN:
-		return "wlan";
-	case RFKILL_TYPE_BLUETOOTH:
-		return "bluetooth";
-	case RFKILL_TYPE_UWB:
-		return "ultrawideband";
-	case RFKILL_TYPE_WIMAX:
-		return "wimax";
-	case RFKILL_TYPE_WWAN:
-		return "wwan";
-	case RFKILL_TYPE_GPS:
-		return "gps";
-	case RFKILL_TYPE_FM:
-		return "fm";
-	case RFKILL_TYPE_NFC:
-		return "nfc";
-	default:
-		BUG();
-	}
-}
-
 static ssize_t type_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
 	struct rfkill *rfkill = to_rfkill(dev);
 
-	return sprintf(buf, "%s\n", rfkill_get_type_str(rfkill->type));
+	return sprintf(buf, "%s\n", rfkill_types[rfkill->type]);
 }
 static DEVICE_ATTR_RO(type);
 
@@ -760,7 +761,7 @@ static int rfkill_dev_uevent(struct device *dev, struct kobj_uevent_env *env)
 	if (error)
 		return error;
 	error = add_uevent_var(env, "RFKILL_TYPE=%s",
-			       rfkill_get_type_str(rfkill->type));
+			       rfkill_types[rfkill->type]);
 	if (error)
 		return error;
 	spin_lock_irqsave(&rfkill->lock, flags);
-- 
2.4.6

[PATCH 3/5] net: rfkill: gpio: get the name and type from device property

[Heikki Krogerus]

This prepares the driver for removal of platform data.

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

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index d5d58d9..07323c3 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -81,7 +81,6 @@ static int rfkill_gpio_acpi_probe(struct device *dev,
 	if (!id)
 		return -ENODEV;
 
-	rfkill->name = dev_name(dev);
 	rfkill->type = (unsigned)id->driver_data;
 
 	return acpi_dev_add_driver_gpios(ACPI_COMPANION(dev),
@@ -93,12 +92,21 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 	struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
 	struct rfkill_gpio_data *rfkill;
 	struct gpio_desc *gpio;
+	const char *type_name;
 	int ret;
 
 	rfkill = devm_kzalloc(&pdev->dev, sizeof(*rfkill), GFP_KERNEL);
 	if (!rfkill)
 		return -ENOMEM;
 
+	device_property_read_string(&pdev->dev, "name", &rfkill->name);
+	device_property_read_string(&pdev->dev, "type", &type_name);
+
+	if (!rfkill->name)
+		rfkill->name = dev_name(&pdev->dev);
+
+	rfkill->type = rfkill_find_type(type_name);
+
 	if (ACPI_HANDLE(&pdev->dev)) {
 		ret = rfkill_gpio_acpi_probe(&pdev->dev, rfkill);
 		if (ret)
@@ -124,10 +132,8 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 
 	rfkill->shutdown_gpio = gpio;
 
-	/* Make sure at-least one of the GPIO is defined and that
-	 * a name is specified for this instance
-	 */
-	if ((!rfkill->reset_gpio && !rfkill->shutdown_gpio) || !rfkill->name) {
+	/* Make sure at-least one GPIO is defined for this instance */
+	if (!rfkill->reset_gpio && !rfkill->shutdown_gpio) {
 		dev_err(&pdev->dev, "invalid platform data\n");
 		return -EINVAL;
 	}
-- 
2.4.6

[PATCH 4/5] ARM: tegra: use build-in device properties with rfkill_gpio

[Heikki Krogerus]

Pass the rfkill name and type to the device with properties
instead of driver specific platform data.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
CC: Alexandre Courbot <gnurou@gmail.com>
CC: Thierry Reding <thierry.reding@gmail.com>
CC: Stephen Warren <swarren@wwwdotorg.org>
---
 arch/arm/mach-tegra/board-paz00.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
index fbe74c6..5d8d3a4 100644
--- a/arch/arm/mach-tegra/board-paz00.c
+++ b/arch/arm/mach-tegra/board-paz00.c
@@ -17,23 +17,25 @@
  *
  */
 
+#include <linux/property.h>
 #include <linux/gpio/machine.h>
 #include <linux/platform_device.h>
-#include <linux/rfkill-gpio.h>
 
 #include "board.h"
 
-static struct rfkill_gpio_platform_data wifi_rfkill_platform_data = {
-	.name	= "wifi_rfkill",
-	.type	= RFKILL_TYPE_WLAN,
+static struct property_entry wifi_rfkill_prop[] = {
+	PROP_ENTRY_STRING("name", "wifi_rfkill"),
+	PROP_ENTRY_STRING("type", "wlan"),
+	{ },
+};
+
+static struct property_set wifi_rfkill_pset = {
+	.properties = wifi_rfkill_prop,
 };
 
 static struct platform_device wifi_rfkill_device = {
 	.name	= "rfkill_gpio",
 	.id	= -1,
-	.dev	= {
-		.platform_data = &wifi_rfkill_platform_data,
-	},
 };
 
 static struct gpiod_lookup_table wifi_gpio_lookup = {
@@ -47,6 +49,7 @@ static struct gpiod_lookup_table wifi_gpio_lookup = {
 
 void __init tegra_paz00_wifikill_init(void)
 {
+	device_add_property_set(&wifi_rfkill_device.dev, &wifi_rfkill_pset);
 	gpiod_add_lookup_table(&wifi_gpio_lookup);
 	platform_device_register(&wifi_rfkill_device);
 }
-- 
2.4.6

[PATCH 5/5] net: rfkill: gpio: remove rfkill_gpio_platform_data

[Heikki Krogerus]

No more users for it.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 include/linux/rfkill-gpio.h | 37 -------------------------------------
 net/rfkill/Kconfig          |  3 +--
 net/rfkill/rfkill-gpio.c    |  8 --------
 3 files changed, 1 insertion(+), 47 deletions(-)
 delete mode 100644 include/linux/rfkill-gpio.h

diff --git a/include/linux/rfkill-gpio.h b/include/linux/rfkill-gpio.h
deleted file mode 100644
index 20bcb55..0000000
--- a/include/linux/rfkill-gpio.h
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * 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
- */
-
-struct rfkill_gpio_platform_data {
-	char			*name;
-	enum rfkill_type	type;
-};
-
-#endif /* __RFKILL_GPIO_H */
diff --git a/net/rfkill/Kconfig b/net/rfkill/Kconfig
index 4c10e7e..6320890 100644
--- a/net/rfkill/Kconfig
+++ b/net/rfkill/Kconfig
@@ -40,5 +40,4 @@ config RFKILL_GPIO
 	default n
 	help
 	  If you say yes here you get support of a generic gpio RFKILL
-	  driver. The platform should fill in the appropriate fields in the
-	  rfkill_gpio_platform_data structure and pass that to the driver.
+	  driver.
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 07323c3..69d92e1 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -27,8 +27,6 @@
 #include <linux/acpi.h>
 #include <linux/gpio/consumer.h>
 
-#include <linux/rfkill-gpio.h>
-
 struct rfkill_gpio_data {
 	const char		*name;
 	enum rfkill_type	type;
@@ -89,7 +87,6 @@ static int rfkill_gpio_acpi_probe(struct device *dev,
 
 static int rfkill_gpio_probe(struct platform_device *pdev)
 {
-	struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
 	struct rfkill_gpio_data *rfkill;
 	struct gpio_desc *gpio;
 	const char *type_name;
@@ -111,11 +108,6 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 		ret = rfkill_gpio_acpi_probe(&pdev->dev, rfkill);
 		if (ret)
 			return ret;
-	} else if (pdata) {
-		rfkill->name = pdata->name;
-		rfkill->type = pdata->type;
-	} else {
-		return -ENODEV;
 	}
 
 	rfkill->clk = devm_clk_get(&pdev->dev, NULL);
-- 
2.4.6

Re: [PATCH 1/5] device property: helper macros for property entry creation

[Andy Shevchenko]

On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:
> Marcos for easier creation of build-in property entries.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  include/linux/property.h | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 76ebde9..204d899 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -152,6 +152,41 @@ struct property_entry {
>  	} value;
>  };
>  
> +#define PROP_ENTRY_U8(_name_, _val_) { \

PROP_ prefix is too generic.
Maybe DEVPROP_ ? At least for the latter no records in the current
sources.

> +	.name = _name_, \
> +	.type = DEV_PROP_U8, \
> +	.nval = 1, \
> +	.value.u8_data = _val_, \
> +}
> +
> +#define PROP_ENTRY_U16(_name_, _val_) { \
> +	.name = _name_, \
> +	.type = DEV_PROP_U16, \
> +	.nval = 1, \
> +	.value.u16_data = _val_, \
> +}
> +
> +#define PROP_ENTRY_U32(_name_, _val_) { \
> +	.name = _name_, \
> +	.type = DEV_PROP_U32, \
> +	.nval = 1, \
> +	.value.u32_data = _val_, \
> +}
> +
> +#define PROP_ENTRY_U64(_name_, _val_) { \
> +	.name = _name_, \
> +	.type = DEV_PROP_U64, \
> +	.nval = 1, \
> +	.value.u64_data = _val_, \
> +}
> +
> +#define PROP_ENTRY_STRING(_name_, _val_) { \

…_STRING_ARRAY I can notice.

> +	.name = _name_, \
> +	.type = DEV_PROP_STRING, \
> +	.nval = 1, \
> +	.value.str = (const char **)_val_, \
> +}
> +
>  /**
>   * struct property_set - Collection of "built-in" device properties.
>   * @fwnode: Handle to be pointed to by the fwnode field of struct 
> device.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH 2/5] net: rfkill: add rfkill_find_type function

[Andy Shevchenko]

On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:
> Helper for finding the type based on name. Useful if the
> type needs to be determined based on device property.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  include/linux/rfkill.h | 15 +++++++++++++
>  net/rfkill/core.c      | 57 +++++++++++++++++++++++++---------------
> ----------
>  2 files changed, 44 insertions(+), 28 deletions(-)
> 
> diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h
> index d901078..02f563c 100644
> --- a/include/linux/rfkill.h
> +++ b/include/linux/rfkill.h
> @@ -212,6 +212,15 @@ void rfkill_set_states(struct rfkill *rfkill, 
> bool sw, bool hw);
>   * @rfkill: rfkill struct to query
>   */
>  bool rfkill_blocked(struct rfkill *rfkill);
> +
> +/**
> + * rfkill_find_type - Helpper for finding rfkill type by name
> + * @name: the name of the type
> + *
> + * Returns enum rfkill_type that conrresponds the name.
> + */
> +enum rfkill_type rfkill_find_type(const char *name);
> +
>  #else /* !RFKILL */
>  static inline struct rfkill * __must_check
>  rfkill_alloc(const char *name,
> @@ -268,6 +277,12 @@ static inline bool rfkill_blocked(struct rfkill 
> *rfkill)
>  {
>  	return false;
>  }
> +
> +static inline enum rfkill_type rfkill_find_type(const char *name)
> +{
> +	return 0;

Hmm… Besides 0 is implicitly casted to enum type the issue with enums
that you rather have to supply existing enum entry. I would suggest to
add RFKILL_TYPE_UNKNOWN if _ALL is reserved for some use cases.

> +}
> +
>  #endif /* RFKILL || RFKILL_MODULE */
>  
>  
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index f12149a..53d7a97e 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -574,6 +574,33 @@ void rfkill_set_states(struct rfkill *rfkill, 
> bool sw, bool hw)
>  }
>  EXPORT_SYMBOL(rfkill_set_states);
>  
> +static const char *rfkill_types[NUM_RFKILL_TYPES] = {
> +	[RFKILL_TYPE_WLAN]	= "wlan",
> +	[RFKILL_TYPE_BLUETOOTH]	= "bluetooth",
> +	[RFKILL_TYPE_UWB]	= "ultrawideband",
> +	[RFKILL_TYPE_WIMAX]	= "wimax",
> +	[RFKILL_TYPE_WWAN]	= "wwan",
> +	[RFKILL_TYPE_GPS]	= "gps",
> +	[RFKILL_TYPE_FM]	= "fm",
> +	[RFKILL_TYPE_NFC]	= "nfc",
> +};
> +
> +enum rfkill_type rfkill_find_type(const char *name)
> +{
> +	int i;
> +
> +	BUILD_BUG_ON(NUM_RFKILL_TYPES != RFKILL_TYPE_NFC + 1);
> +
> +	if (!name)
> +		return RFKILL_TYPE_ALL;
> +
> +	for (i = 1; i < NUM_RFKILL_TYPES; i++)
> +		if (!strcmp(name, rfkill_types[i]))
> +			return i;
> +	return RFKILL_TYPE_ALL;
> +}
> +EXPORT_SYMBOL(rfkill_find_type);
> +
>  static ssize_t name_show(struct device *dev, struct device_attribute 
> *attr,
>  			 char *buf)
>  {
> @@ -583,38 +610,12 @@ static ssize_t name_show(struct device *dev, 
> struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(name);
>  
> -static const char *rfkill_get_type_str(enum rfkill_type type)
> -{
> -	BUILD_BUG_ON(NUM_RFKILL_TYPES != RFKILL_TYPE_NFC + 1);
> -
> -	switch (type) {
> -	case RFKILL_TYPE_WLAN:
> -		return "wlan";
> -	case RFKILL_TYPE_BLUETOOTH:
> -		return "bluetooth";
> -	case RFKILL_TYPE_UWB:
> -		return "ultrawideband";
> -	case RFKILL_TYPE_WIMAX:
> -		return "wimax";
> -	case RFKILL_TYPE_WWAN:
> -		return "wwan";
> -	case RFKILL_TYPE_GPS:
> -		return "gps";
> -	case RFKILL_TYPE_FM:
> -		return "fm";
> -	case RFKILL_TYPE_NFC:
> -		return "nfc";
> -	default:
> -		BUG();
> -	}
> -}
> -
>  static ssize_t type_show(struct device *dev, struct device_attribute 
> *attr,
>  			 char *buf)
>  {
>  	struct rfkill *rfkill = to_rfkill(dev);
>  
> -	return sprintf(buf, "%s\n", rfkill_get_type_str(rfkill
> ->type));
> +	return sprintf(buf, "%s\n", rfkill_types[rfkill->type]);
>  }
>  static DEVICE_ATTR_RO(type);
>  
> @@ -760,7 +761,7 @@ static int rfkill_dev_uevent(struct device *dev, 
> struct kobj_uevent_env *env)
>  	if (error)
>  		return error;
>  	error = add_uevent_var(env, "RFKILL_TYPE=%s",
> -			       rfkill_get_type_str(rfkill->type));
> +			       rfkill_types[rfkill->type]);
>  	if (error)
>  		return error;
>  	spin_lock_irqsave(&rfkill->lock, flags);

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH 1/5] device property: helper macros for property entry creation

[Shevchenko, Andriy]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1221 bytes --]

On Wed, 2015-08-05 at 17:02 +0300, Andy Shevchenko wrote:
> On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:

[]

> > +#define PROP_ENTRY_STRING(_name_, _val_) { \
> 
> …_STRING_ARRAY I can notice.

s / can / can't /

> 
> > +	.name = _name_, \
> > +	.type = DEV_PROP_STRING, \
> > +	.nval = 1, \
> > +	.value.str = (const char **)_val_, \
> > +}
> > +
> >  /**
> >   * struct property_set - Collection of "built-in" device 
> > properties.
> >   * @fwnode: Handle to be pointed to by the fwnode field of struct 
> > device.
> 

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 5/5] net: rfkill: gpio: remove rfkill_gpio_platform_data

[Andy Shevchenko]

On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:
> No more users for it.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  include/linux/rfkill-gpio.h | 37 -----------------------------------
> --
>  net/rfkill/Kconfig          |  3 +--
>  net/rfkill/rfkill-gpio.c    |  8 --------
>  3 files changed, 1 insertion(+), 47 deletions(-)
>  delete mode 100644 include/linux/rfkill-gpio.h
> 
> diff --git a/include/linux/rfkill-gpio.h b/include/linux/rfkill
> -gpio.h
> deleted file mode 100644
> index 20bcb55..0000000
> --- a/include/linux/rfkill-gpio.h
> +++ /dev/null
> @@ -1,37 +0,0 @@
> -/*
> - * 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
> - */
> -
> -struct rfkill_gpio_platform_data {
> -	char			*name;
> -	enum rfkill_type	type;
> -};
> -
> -#endif /* __RFKILL_GPIO_H */
> diff --git a/net/rfkill/Kconfig b/net/rfkill/Kconfig
> index 4c10e7e..6320890 100644
> --- a/net/rfkill/Kconfig
> +++ b/net/rfkill/Kconfig
> @@ -40,5 +40,4 @@ config RFKILL_GPIO
>  	default n
>  	help
>  	  If you say yes here you get support of a generic gpio 
> RFKILL
> -	  driver. The platform should fill in the appropriate fields 
> in the
> -	  rfkill_gpio_platform_data structure and pass that to the 
> driver.
> +	  driver.
> diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
> index 07323c3..69d92e1 100644
> --- a/net/rfkill/rfkill-gpio.c
> +++ b/net/rfkill/rfkill-gpio.c
> @@ -27,8 +27,6 @@
>  #include <linux/acpi.h>
>  #include <linux/gpio/consumer.h>
>  
> -#include <linux/rfkill-gpio.h>
> -
>  struct rfkill_gpio_data {
>  	const char		*name;
>  	enum rfkill_type	type;
> @@ -89,7 +87,6 @@ static int rfkill_gpio_acpi_probe(struct device 
> *dev,
>  
>  static int rfkill_gpio_probe(struct platform_device *pdev)
>  {
> -	struct rfkill_gpio_platform_data *pdata = pdev
> ->dev.platform_data;
>  	struct rfkill_gpio_data *rfkill;
>  	struct gpio_desc *gpio;
>  	const char *type_name;
> @@ -111,11 +108,6 @@ static int rfkill_gpio_probe(struct 
> platform_device *pdev)
>  		ret = rfkill_gpio_acpi_probe(&pdev->dev, rfkill);
>  		if (ret)
>  			return ret;
> -	} else if (pdata) {
> -		rfkill->name = pdata->name;
> -		rfkill->type = pdata->type;
> -	} else {
> -		return -ENODEV;

Shouldn't we leave the error path and modify to check if we have device
property set set?

>  	}
>  
>  	rfkill->clk = devm_clk_get(&pdev->dev, NULL);

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH 5/5] net: rfkill: gpio: remove rfkill_gpio_platform_data

[Heikki Krogerus]

On Wed, Aug 05, 2015 at 05:15:28PM +0300, Andy Shevchenko wrote:
> On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:
> > No more users for it.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  include/linux/rfkill-gpio.h | 37 -----------------------------------
> > --
> >  net/rfkill/Kconfig          |  3 +--
> >  net/rfkill/rfkill-gpio.c    |  8 --------
> >  3 files changed, 1 insertion(+), 47 deletions(-)
> >  delete mode 100644 include/linux/rfkill-gpio.h
> > 
> > diff --git a/include/linux/rfkill-gpio.h b/include/linux/rfkill
> > -gpio.h
> > deleted file mode 100644
> > index 20bcb55..0000000
> > --- a/include/linux/rfkill-gpio.h
> > +++ /dev/null
> > @@ -1,37 +0,0 @@
> > -/*
> > - * 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
> > - */
> > -
> > -struct rfkill_gpio_platform_data {
> > -	char			*name;
> > -	enum rfkill_type	type;
> > -};
> > -
> > -#endif /* __RFKILL_GPIO_H */
> > diff --git a/net/rfkill/Kconfig b/net/rfkill/Kconfig
> > index 4c10e7e..6320890 100644
> > --- a/net/rfkill/Kconfig
> > +++ b/net/rfkill/Kconfig
> > @@ -40,5 +40,4 @@ config RFKILL_GPIO
> >  	default n
> >  	help
> >  	  If you say yes here you get support of a generic gpio 
> > RFKILL
> > -	  driver. The platform should fill in the appropriate fields 
> > in the
> > -	  rfkill_gpio_platform_data structure and pass that to the 
> > driver.
> > +	  driver.
> > diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
> > index 07323c3..69d92e1 100644
> > --- a/net/rfkill/rfkill-gpio.c
> > +++ b/net/rfkill/rfkill-gpio.c
> > @@ -27,8 +27,6 @@
> >  #include <linux/acpi.h>
> >  #include <linux/gpio/consumer.h>
> >  
> > -#include <linux/rfkill-gpio.h>
> > -
> >  struct rfkill_gpio_data {
> >  	const char		*name;
> >  	enum rfkill_type	type;
> > @@ -89,7 +87,6 @@ static int rfkill_gpio_acpi_probe(struct device 
> > *dev,
> >  
> >  static int rfkill_gpio_probe(struct platform_device *pdev)
> >  {
> > -	struct rfkill_gpio_platform_data *pdata = pdev
> > ->dev.platform_data;
> >  	struct rfkill_gpio_data *rfkill;
> >  	struct gpio_desc *gpio;
> >  	const char *type_name;
> > @@ -111,11 +108,6 @@ static int rfkill_gpio_probe(struct 
> > platform_device *pdev)
> >  		ret = rfkill_gpio_acpi_probe(&pdev->dev, rfkill);
> >  		if (ret)
> >  			return ret;
> > -	} else if (pdata) {
> > -		rfkill->name = pdata->name;
> > -		rfkill->type = pdata->type;
> > -	} else {
> > -		return -ENODEV;
> 
> Shouldn't we leave the error path and modify to check if we have device
> property set set?

We already check them before this point. After this ACPI will be the
only "special" case where we know the needed information does not come
from device property and needs separate handling. Otherwise, if the
device properties are not set, we cracefully fail.


Thanks,

-- 
heikki

Re: [PATCH 1/5] device property: helper macros for property entry creation

[Heikki Krogerus]

On Wed, Aug 05, 2015 at 05:02:18PM +0300, Andy Shevchenko wrote:
> On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:
> > Marcos for easier creation of build-in property entries.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  include/linux/property.h | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/include/linux/property.h b/include/linux/property.h
> > index 76ebde9..204d899 100644
> > --- a/include/linux/property.h
> > +++ b/include/linux/property.h
> > @@ -152,6 +152,41 @@ struct property_entry {
> >  	} value;
> >  };
> >  
> > +#define PROP_ENTRY_U8(_name_, _val_) { \
> 
> PROP_ prefix is too generic.
> Maybe DEVPROP_ ? At least for the latter no records in the current
> sources.

I disagree with that. IMO this kind of macros should ideally resemble
the structure name they are used to fill (struct property_entry in
this case). And there are already definitions for DEV_PROP_* to
describe the types, so using something like DEVPROP_* here is just
confusing.

If PROP_ENTRY_* is really not good enough, we can change them
PROPERTY_ENTRY_*. But is PROP_ENTRY_* really so bad?

Rafael, what is your opinion?


Thanks,

-- 
heikki

Re: [PATCH 2/5] net: rfkill: add rfkill_find_type function

[Heikki Krogerus]

On Wed, Aug 05, 2015 at 05:07:29PM +0300, Andy Shevchenko wrote:
> On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:
> > Helper for finding the type based on name. Useful if the
> > type needs to be determined based on device property.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  include/linux/rfkill.h | 15 +++++++++++++
> >  net/rfkill/core.c      | 57 +++++++++++++++++++++++++---------------
> > ----------
> >  2 files changed, 44 insertions(+), 28 deletions(-)
> > 
> > diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h
> > index d901078..02f563c 100644
> > --- a/include/linux/rfkill.h
> > +++ b/include/linux/rfkill.h
> > @@ -212,6 +212,15 @@ void rfkill_set_states(struct rfkill *rfkill, 
> > bool sw, bool hw);
> >   * @rfkill: rfkill struct to query
> >   */
> >  bool rfkill_blocked(struct rfkill *rfkill);
> > +
> > +/**
> > + * rfkill_find_type - Helpper for finding rfkill type by name
> > + * @name: the name of the type
> > + *
> > + * Returns enum rfkill_type that conrresponds the name.
> > + */
> > +enum rfkill_type rfkill_find_type(const char *name);
> > +
> >  #else /* !RFKILL */
> >  static inline struct rfkill * __must_check
> >  rfkill_alloc(const char *name,
> > @@ -268,6 +277,12 @@ static inline bool rfkill_blocked(struct rfkill 
> > *rfkill)
> >  {
> >  	return false;
> >  }
> > +
> > +static inline enum rfkill_type rfkill_find_type(const char *name)
> > +{
> > +	return 0;
> 
> Hmm… Besides 0 is implicitly casted to enum type the issue with enums
> that you rather have to supply existing enum entry. I would suggest to
> add RFKILL_TYPE_UNKNOWN if _ALL is reserved for some use cases.

Why would you add a new type just for this? You do realize it would
require adding specific handling all over the place? RFKILL_TYPE_ALL
(0) is already handled as an invalid type. Confused?

I'll change this and return RFKILL_TYPE_ALL instead of 0.


Thanks,

-- 
heikki

Re: [PATCH 2/5] net: rfkill: add rfkill_find_type function

[Andy Shevchenko]

On Thu, 2015-08-06 at 11:30 +0300, Heikki Krogerus wrote:
> > > 
> On Wed, Aug 05, 2015 at 05:07:29PM +0300, Andy Shevchenko wrote:
> > On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:


> > > +static inline enum rfkill_type rfkill_find_type(const char
> > > *name)
> > > +{
> > > +	return 0;
> > 
> > Hmm… Besides 0 is implicitly casted to enum type the issue with 
> > enums
> > that you rather have to supply existing enum entry. I would suggest 
> > to
> > add RFKILL_TYPE_UNKNOWN if _ALL is reserved for some use cases.
> 
> Why would you add a new type just for this? You do realize it would
> require adding specific handling all over the place? RFKILL_TYPE_ALL
> (0) is already handled as an invalid type.

It was my thought as well (see *if* in my previous comment).

>  Confused?

A bit, yes.

> 
> I'll change this and return RFKILL_TYPE_ALL instead of 0.

Excellent!

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH 2/5] net: rfkill: add rfkill_find_type function

[Sergei Shtylyov]

Hello.

On 8/5/2015 4:39 PM, Heikki Krogerus wrote:

> Helper for finding the type based on name. Useful if the
> type needs to be determined based on device property.

> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>   include/linux/rfkill.h | 15 +++++++++++++
>   net/rfkill/core.c      | 57 +++++++++++++++++++++++++-------------------------
>   2 files changed, 44 insertions(+), 28 deletions(-)

> diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h
> index d901078..02f563c 100644
> --- a/include/linux/rfkill.h
> +++ b/include/linux/rfkill.h
[...]
> @@ -268,6 +277,12 @@ static inline bool rfkill_blocked(struct rfkill *rfkill)
>   {
>   	return false;
>   }
> +
> +static inline enum rfkill_type rfkill_find_type(const char *name)
> +{
> +	return 0;
 > +}
 > +

    Shouldn't it return some member of 'enum rfkill_type' instead?

[...]

MBR, Sergei

Re: [PATCH 1/5] device property: helper macros for property entry creation

[Rafael J. Wysocki]

On Thursday, August 06, 2015 10:48:48 AM Heikki Krogerus wrote:
> On Wed, Aug 05, 2015 at 05:02:18PM +0300, Andy Shevchenko wrote:
> > On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:
> > > Marcos for easier creation of build-in property entries.
> > > 
> > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > ---
> > >  include/linux/property.h | 35 +++++++++++++++++++++++++++++++++++
> > >  1 file changed, 35 insertions(+)
> > > 
> > > diff --git a/include/linux/property.h b/include/linux/property.h
> > > index 76ebde9..204d899 100644
> > > --- a/include/linux/property.h
> > > +++ b/include/linux/property.h
> > > @@ -152,6 +152,41 @@ struct property_entry {
> > >  	} value;
> > >  };
> > >  
> > > +#define PROP_ENTRY_U8(_name_, _val_) { \
> > 
> > PROP_ prefix is too generic.
> > Maybe DEVPROP_ ? At least for the latter no records in the current
> > sources.
> 
> I disagree with that. IMO this kind of macros should ideally resemble
> the structure name they are used to fill (struct property_entry in
> this case). And there are already definitions for DEV_PROP_* to
> describe the types, so using something like DEVPROP_* here is just
> confusing.
> 
> If PROP_ENTRY_* is really not good enough, we can change them
> PROPERTY_ENTRY_*. But is PROP_ENTRY_* really so bad?
> 
> Rafael, what is your opinion?

I would prefer PROPERTY_ENTRY_ to be honest.  It's not like we need to save
characters here.

Thanks,
Rafael

Re: [PATCH 2/5] net: rfkill: add rfkill_find_type function

[Johannes Berg]

On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:
> 
> +static const char *rfkill_types[NUM_RFKILL_TYPES] = {
> +	[RFKILL_TYPE_WLAN]	= "wlan",
> +	[RFKILL_TYPE_BLUETOOTH]	= "bluetooth",
> +	[RFKILL_TYPE_UWB]	= "ultrawideband",
> +	[RFKILL_TYPE_WIMAX]	= "wimax",
> +	[RFKILL_TYPE_WWAN]	= "wwan",
> +	[RFKILL_TYPE_GPS]	= "gps",
> +	[RFKILL_TYPE_FM]	= "fm",
> +	[RFKILL_TYPE_NFC]	= "nfc",
> +};
> +
> +enum rfkill_type rfkill_find_type(const char *name)
> +{
> +	int i;
> +
> +	BUILD_BUG_ON(NUM_RFKILL_TYPES != RFKILL_TYPE_NFC + 1);
> 
That BUILD_BUG_ON() is now less useful - previously it pointed to the
code that needed to change, now you're left wondering if you don't look
up since it isn't quite that obvious from the code what this does.

Something like

	BUILD_BUG_ON(rfkill_types[NUM_RFKILL_TYPES - 1] == NULL);

would be better. As we only add here, that would be safe enough - I've
done something similar in the past that a bit more complicated.

With that and the static inline fixed (which maybe you could even
remove) I'm fine with all these rfkill patches, but I'm not sure how to
merge them since they affect all kinds of other trees. If desired, I
can apply them, but an ACK from the tegra maintainer would be good :)

johannes

Re: [PATCH 2/5] net: rfkill: add rfkill_find_type function

[Heikki Krogerus]

Hi,

On Thu, Aug 13, 2015 at 11:27:46AM +0200, Johannes Berg wrote:
> On Wed, 2015-08-05 at 16:39 +0300, Heikki Krogerus wrote:
> > 
> > +static const char *rfkill_types[NUM_RFKILL_TYPES] = {
> > +	[RFKILL_TYPE_WLAN]	= "wlan",
> > +	[RFKILL_TYPE_BLUETOOTH]	= "bluetooth",
> > +	[RFKILL_TYPE_UWB]	= "ultrawideband",
> > +	[RFKILL_TYPE_WIMAX]	= "wimax",
> > +	[RFKILL_TYPE_WWAN]	= "wwan",
> > +	[RFKILL_TYPE_GPS]	= "gps",
> > +	[RFKILL_TYPE_FM]	= "fm",
> > +	[RFKILL_TYPE_NFC]	= "nfc",
> > +};
> > +
> > +enum rfkill_type rfkill_find_type(const char *name)
> > +{
> > +	int i;
> > +
> > +	BUILD_BUG_ON(NUM_RFKILL_TYPES != RFKILL_TYPE_NFC + 1);
> > 
> That BUILD_BUG_ON() is now less useful - previously it pointed to the
> code that needed to change, now you're left wondering if you don't look
> up since it isn't quite that obvious from the code what this does.
> 
> Something like
> 
> 	BUILD_BUG_ON(rfkill_types[NUM_RFKILL_TYPES - 1] == NULL);
> 
> would be better. As we only add here, that would be safe enough - I've
> done something similar in the past that a bit more complicated.

OK, I'll change it.

> With that and the static inline fixed (which maybe you could even
> remove) I'm fine with all these rfkill patches, but I'm not sure how to
> merge them since they affect all kinds of other trees. If desired, I
> can apply them, but an ACK from the tegra maintainer would be good :)

Andy and Mika are preparing some changes to the device property
handling. I'll wait for their proposal and prepare next version these
after that.


Thanks,

-- 
heikki