Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 02/20] Input: iforce - introduce transport ops
From: Dmitry Torokhov @ 2018-09-18  0:47 UTC (permalink / raw)
  To: linux-input, Tim Schumacher; +Cc: linux-kernel
In-Reply-To: <20180918004732.9875-1-dmitry.torokhov@gmail.com>

In order to tease apart the driver into core and transport modules, let's
introduce transport operations and make "xmit" the very first one such
operation.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 .../input/joystick/iforce/iforce-packets.c    | 20 ++-----------------
 drivers/input/joystick/iforce/iforce-serio.c  |  9 +++++++--
 drivers/input/joystick/iforce/iforce-usb.c    | 15 ++++++++++++--
 drivers/input/joystick/iforce/iforce.h        | 13 ++++++------
 4 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/drivers/input/joystick/iforce/iforce-packets.c b/drivers/input/joystick/iforce/iforce-packets.c
index 91893c751524..b8ca9bdfdef8 100644
--- a/drivers/input/joystick/iforce/iforce-packets.c
+++ b/drivers/input/joystick/iforce/iforce-packets.c
@@ -91,25 +91,9 @@ int iforce_send_packet(struct iforce *iforce, u16 cmd, unsigned char* data)
 /*
  * If necessary, start the transmission
  */
-	switch (iforce->bus) {
-
-#ifdef CONFIG_JOYSTICK_IFORCE_232
-		case IFORCE_232:
-		if (empty)
-			iforce_serial_xmit(iforce);
-		break;
-#endif
-#ifdef CONFIG_JOYSTICK_IFORCE_USB
-		case IFORCE_USB:
+	if (empty)
+		iforce->xport_ops->xmit(iforce);
 
-		if (iforce->usbdev && empty &&
-			!test_and_set_bit(IFORCE_XMIT_RUNNING, iforce->xmit_flags)) {
-
-			iforce_usb_xmit(iforce);
-		}
-		break;
-#endif
-	}
 	return 0;
 }
 
diff --git a/drivers/input/joystick/iforce/iforce-serio.c b/drivers/input/joystick/iforce/iforce-serio.c
index f4ba4a751fe0..c9469209f994 100644
--- a/drivers/input/joystick/iforce/iforce-serio.c
+++ b/drivers/input/joystick/iforce/iforce-serio.c
@@ -23,7 +23,7 @@
 
 #include "iforce.h"
 
-void iforce_serial_xmit(struct iforce *iforce)
+static void iforce_serio_xmit(struct iforce *iforce)
 {
 	unsigned char cs;
 	int i;
@@ -67,11 +67,15 @@ void iforce_serial_xmit(struct iforce *iforce)
 	spin_unlock_irqrestore(&iforce->xmit_lock, flags);
 }
 
+static const struct iforce_xport_ops iforce_serio_xport_ops = {
+	.xmit		= iforce_serio_xmit,
+};
+
 static void iforce_serio_write_wakeup(struct serio *serio)
 {
 	struct iforce *iforce = serio_get_drvdata(serio);
 
-	iforce_serial_xmit(iforce);
+	iforce_serio_xmit(iforce);
 }
 
 static irqreturn_t iforce_serio_irq(struct serio *serio,
@@ -129,6 +133,7 @@ static int iforce_serio_connect(struct serio *serio, struct serio_driver *drv)
 	if (!iforce)
 		return -ENOMEM;
 
+	iforce->xport_ops = &iforce_serio_xport_ops;
 	iforce->bus = IFORCE_232;
 	iforce->serio = serio;
 
diff --git a/drivers/input/joystick/iforce/iforce-usb.c b/drivers/input/joystick/iforce/iforce-usb.c
index 78073259c9a1..d4f7f34db9a0 100644
--- a/drivers/input/joystick/iforce/iforce-usb.c
+++ b/drivers/input/joystick/iforce/iforce-usb.c
@@ -23,7 +23,7 @@
 
 #include "iforce.h"
 
-void iforce_usb_xmit(struct iforce *iforce)
+static void __iforce_usb_xmit(struct iforce *iforce)
 {
 	int n, c;
 	unsigned long flags;
@@ -69,6 +69,16 @@ void iforce_usb_xmit(struct iforce *iforce)
 	spin_unlock_irqrestore(&iforce->xmit_lock, flags);
 }
 
+static void iforce_usb_xmit(struct iforce *iforce)
+{
+	if (!test_and_set_bit(IFORCE_XMIT_RUNNING, iforce->xmit_flags))
+		__iforce_usb_xmit(iforce);
+}
+
+static const struct iforce_xport_ops iforce_usb_xport_ops = {
+	.xmit		= iforce_usb_xmit,
+};
+
 static void iforce_usb_irq(struct urb *urb)
 {
 	struct iforce *iforce = urb->context;
@@ -113,7 +123,7 @@ static void iforce_usb_out(struct urb *urb)
 		return;
 	}
 
-	iforce_usb_xmit(iforce);
+	__iforce_usb_xmit(iforce);
 
 	wake_up(&iforce->wait);
 }
@@ -155,6 +165,7 @@ static int iforce_usb_probe(struct usb_interface *intf,
 	if (!(iforce->ctrl = usb_alloc_urb(0, GFP_KERNEL)))
 		goto fail;
 
+	iforce->xport_ops = &iforce_usb_xport_ops;
 	iforce->bus = IFORCE_USB;
 	iforce->usbdev = dev;
 	iforce->intf = intf;
diff --git a/drivers/input/joystick/iforce/iforce.h b/drivers/input/joystick/iforce/iforce.h
index 0e9d01f8bcb6..2fea3be751ed 100644
--- a/drivers/input/joystick/iforce/iforce.h
+++ b/drivers/input/joystick/iforce/iforce.h
@@ -93,9 +93,16 @@ struct iforce_device {
 	signed short *ff;
 };
 
+struct iforce;
+
+struct iforce_xport_ops {
+	void (*xmit)(struct iforce *iforce);
+};
+
 struct iforce {
 	struct input_dev *dev;		/* Input device interface */
 	struct iforce_device *type;
+	const struct iforce_xport_ops *xport_ops;
 	int bus;
 
 	unsigned char data[IFORCE_MAX_LENGTH];
@@ -141,12 +148,6 @@ struct iforce {
 
 
 /* Public functions */
-/* iforce-serio.c */
-void iforce_serial_xmit(struct iforce *iforce);
-
-/* iforce-usb.c */
-void iforce_usb_xmit(struct iforce *iforce);
-
 /* iforce-main.c */
 int iforce_init_device(struct iforce *iforce);
 
-- 
2.19.0.397.gdd90340f6a-goog

^ permalink raw reply related

* [PATCH 01/20] Input: iforce - remove "being used" silliness
From: Dmitry Torokhov @ 2018-09-18  0:47 UTC (permalink / raw)
  To: linux-input, Tim Schumacher; +Cc: linux-kernel

The kernel is supposed to handle multiple devices, static flags
in packet handling code will never work.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

This is a random assortment of iforce patches that I made a few weeks back.

Tim, I do not have hardware, so I was bound to screw it up, but if you
have some time I'd appreciate if you try them out (and if I indeed broke
things if you could identify issues that would be even more awesome).

Thanks!


 drivers/input/joystick/iforce/iforce-packets.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/input/joystick/iforce/iforce-packets.c b/drivers/input/joystick/iforce/iforce-packets.c
index c10169f4554e..91893c751524 100644
--- a/drivers/input/joystick/iforce/iforce-packets.c
+++ b/drivers/input/joystick/iforce/iforce-packets.c
@@ -149,12 +149,6 @@ void iforce_process_packet(struct iforce *iforce, u16 cmd, unsigned char *data)
 {
 	struct input_dev *dev = iforce->dev;
 	int i;
-	static int being_used = 0;
-
-	if (being_used)
-		dev_warn(&iforce->dev->dev,
-			 "re-entrant call to iforce_process %d\n", being_used);
-	being_used++;
 
 #ifdef CONFIG_JOYSTICK_IFORCE_232
 	if (HI(iforce->expect_packet) == HI(cmd)) {
@@ -165,10 +159,8 @@ void iforce_process_packet(struct iforce *iforce, u16 cmd, unsigned char *data)
 #endif
 	wake_up(&iforce->wait);
 
-	if (!iforce->type) {
-		being_used--;
+	if (!iforce->type)
 		return;
-	}
 
 	switch (HI(cmd)) {
 
@@ -233,7 +225,6 @@ void iforce_process_packet(struct iforce *iforce, u16 cmd, unsigned char *data)
 			}
 			break;
 	}
-	being_used--;
 }
 
 int iforce_get_id_packet(struct iforce *iforce, char *packet)
-- 
2.19.0.397.gdd90340f6a-goog

^ permalink raw reply related

* Re: [PATCH 2/2] input: atakbd.c - fix Atari CapsLock behaviour
From: Dmitry Torokhov @ 2018-09-17 22:36 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Michael Schmitz, linux-input, debian-68k, linux-m68k
In-Reply-To: <87in33hnqj.fsf@igel.home>

On Mon, Sep 17, 2018 at 10:29:08PM +0200, Andreas Schwab wrote:
> On Sep 17 2018, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > On Fri, Sep 07, 2018 at 11:40:43AM +1200, Michael Schmitz wrote:
> >> The CapsLock key on Atari keyboards is not a toggle, it does send the
> >> normal make and break scancodes.
> >> 
> >> Drop the CapsLock toggle handling code, which did cause the CapsLock
> >> key to merely act as a Shift key.
> >> 
> >> Tested-by: Michael Schmitz <schmitzmic@gmail.com>
> >> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
> >
> > Andreas, I have no idea how Atari keyboards work, can you add your
> > signed-off-by (or NAK) to this one as well please?
> 
> The old atakbd driver never had that special handling.
> 
> Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH 2/2] input: atakbd.c - fix Atari CapsLock behaviour
From: Andreas Schwab @ 2018-09-17 20:29 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Michael Schmitz, linux-input, debian-68k, linux-m68k
In-Reply-To: <20180917194310.GA171258@dtor-ws>

On Sep 17 2018, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Fri, Sep 07, 2018 at 11:40:43AM +1200, Michael Schmitz wrote:
>> The CapsLock key on Atari keyboards is not a toggle, it does send the
>> normal make and break scancodes.
>> 
>> Drop the CapsLock toggle handling code, which did cause the CapsLock
>> key to merely act as a Shift key.
>> 
>> Tested-by: Michael Schmitz <schmitzmic@gmail.com>
>> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
>
> Andreas, I have no idea how Atari keyboards work, can you add your
> signed-off-by (or NAK) to this one as well please?

The old atakbd driver never had that special handling.

Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>

Andreas.

^ permalink raw reply

* Re: [PATCH 2/2] input: atakbd.c - fix Atari CapsLock behaviour
From: Dmitry Torokhov @ 2018-09-17 19:43 UTC (permalink / raw)
  To: Michael Schmitz, Andreas Schwab; +Cc: linux-input, debian-68k, linux-m68k
In-Reply-To: <1536277243-3821-3-git-send-email-schmitzmic@gmail.com>

On Fri, Sep 07, 2018 at 11:40:43AM +1200, Michael Schmitz wrote:
> The CapsLock key on Atari keyboards is not a toggle, it does send the
> normal make and break scancodes.
> 
> Drop the CapsLock toggle handling code, which did cause the CapsLock
> key to merely act as a Shift key.
> 
> Tested-by: Michael Schmitz <schmitzmic@gmail.com>
> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>

Andreas, I have no idea how Atari keyboards work, can you add your
signed-off-by (or NAK) to this one as well please?

> 
> ---
>  drivers/input/keyboard/atakbd.c |   10 ++--------
>  1 files changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/input/keyboard/atakbd.c b/drivers/input/keyboard/atakbd.c
> index e989574..6caee80 100644
> --- a/drivers/input/keyboard/atakbd.c
> +++ b/drivers/input/keyboard/atakbd.c
> @@ -185,14 +185,8 @@ static void atakbd_interrupt(unsigned char scancode, char down)
>  
>  		scancode = atakbd_keycode[scancode];
>  
> -		if (scancode == KEY_CAPSLOCK) {	/* CapsLock is a toggle switch key on Amiga */
> -			input_report_key(atakbd_dev, scancode, 1);
> -			input_report_key(atakbd_dev, scancode, 0);
> -			input_sync(atakbd_dev);
> -		} else {
> -			input_report_key(atakbd_dev, scancode, down);
> -			input_sync(atakbd_dev);
> -		}
> +		input_report_key(atakbd_dev, scancode, down);
> +		input_sync(atakbd_dev);
>  	} else				/* scancodes >= 0xf3 are mouse data, most likely */
>  		printk(KERN_INFO "atakbd: unhandled scancode %x\n", scancode);
>  
> -- 
> 1.7.0.4
> 

^ permalink raw reply

* [RFC/PATCH 5/5] RFC: ARM: simone: Hacked in keys
From: Dmitry Torokhov @ 2018-09-17 18:16 UTC (permalink / raw)
  To: Linus Walleij, Rafael J . Wysocki
  Cc: linux-input, linux-gpio, linux-kernel, Andy Shevchenko
In-Reply-To: <20180917181603.125492-1-dmitry.torokhov@gmail.com>

From: Linus Walleij <linus.walleij@linaro.org>

This serves as an illustration of how to use the gpio-keys
in board files using static device properties and machine GPIO descriptor
tables.

It is a hack for the joystick connector on the entirely
boardfile-based SIM.ONE. It will probably not be applied.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 arch/arm/mach-ep93xx/simone.c | 60 +++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/arch/arm/mach-ep93xx/simone.c b/arch/arm/mach-ep93xx/simone.c
index 41aa57581356..b998d6772158 100644
--- a/arch/arm/mach-ep93xx/simone.c
+++ b/arch/arm/mach-ep93xx/simone.c
@@ -24,7 +24,11 @@
 #include <linux/spi/mmc_spi.h>
 #include <linux/platform_data/video-ep93xx.h>
 #include <linux/platform_data/spi-ep93xx.h>
+#include <linux/gpio/machine.h>
 #include <linux/gpio.h>
+#include <linux/input.h>
+#include <linux/gpio_keys.h>
+#include <linux/property.h>
 
 #include <mach/hardware.h>
 #include <mach/gpio-ep93xx.h>
@@ -34,6 +38,47 @@
 
 #include "soc.h"
 
+static const struct property_entry simone_key_enter_props[] __initconst = {
+	PROPERTY_ENTRY_U32("linux,code",	KEY_ENTER),
+	PROPERTY_ENTRY_STRING("label",		"enter"),
+	PROPERTY_ENTRY_STRING("gpios",		"enter-gpios"),
+	{ }
+};
+
+static const struct property_entry simone_key_up_props[] __initconst = {
+	PROPERTY_ENTRY_U32("linux,code",	KEY_UP),
+	PROPERTY_ENTRY_STRING("label",		"up"),
+	PROPERTY_ENTRY_STRING("gpios",		"up-gpios"),
+	{ }
+};
+
+static const struct property_entry simone_key_up_props[] __initconst = {
+	PROPERTY_ENTRY_U32("linux,code",	KEY_LEFT),
+	PROPERTY_ENTRY_STRING("label",		"left"),
+	PROPERTY_ENTRY_STRING("gpios",		"left-gpios"),
+	{ }
+};
+
+static const struct property_entry simone_key_props[] __initconst = {
+	/* There are no properties at device level on this device */
+	{ }
+};
+
+static struct gpiod_lookup_table simone_keys_gpiod_table = {
+	.dev_id = "gpio-keys",
+	.table = {
+		/* Use local offsets on gpiochip/port "B" */
+		GPIO_LOOKUP_IDX("B", 0, "enter-gpios", 0, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("B", 1, "up-gpios", 1, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("B", 2, "left-gpios", 2, GPIO_ACTIVE_LOW),
+	},
+};
+
+static struct platform_device simone_keys_device = {
+	.name = "gpio-keys",
+	.id = -1,
+};
+
 static struct ep93xx_eth_data __initdata simone_eth_data = {
 	.phy_id		= 1,
 };
@@ -107,6 +152,21 @@ static void __init simone_init_machine(void)
 			    ARRAY_SIZE(simone_i2c_board_info));
 	ep93xx_register_spi(&simone_spi_info, simone_spi_devices,
 			    ARRAY_SIZE(simone_spi_devices));
+
+	gpiod_add_lookup_table(&simone_keys_gpiod_table);
+	device_add_properties(&simone_keys_device.dev,
+			      simone_keys_device_props);
+	device_add_child_properties(&simone_keys_device.dev,
+				    dev_fwnode(&simone_keys_device.dev),
+				    simone_key_enter_props);
+	device_add_child_properties(&simone_keys_device.dev,
+				    dev_fwnode(&simone_keys_device.dev),
+				    simone_key_up_props);
+	device_add_child_properties(&simone_keys_device.dev,
+				    dev_fwnode(&simone_keys_device.dev),
+				    simone_key_left_props);
+	platform_device_register(&simone_keys_device);
+
 	simone_register_audio();
 }
 
-- 
2.19.0.397.gdd90340f6a-goog

^ permalink raw reply related

* [RFC/PATCH 4/5] gpiolib: add support for fetching descriptors from static properties
From: Dmitry Torokhov @ 2018-09-17 18:16 UTC (permalink / raw)
  To: Linus Walleij, Rafael J . Wysocki
  Cc: linux-input, linux-gpio, linux-kernel, Andy Shevchenko
In-Reply-To: <20180917181603.125492-1-dmitry.torokhov@gmail.com>

Now that static device properties understand notion of child nodes, let's
teach gpiolib to tie such children and machine GPIO descriptor tables.
We will continue using a single table for entire device, but instead of
using connection ID as a lookup key in the GPIO descriptor table directly,
we will perform additional translation: fwnode_get_named_gpiod() when
dealing with property_set-backed fwnodes will try parsing string property
with name matching connection ID and use result of the lookup as the key in
the table:

static const struct property_entry dev_child1_props[] __initconst = {
	...
	PROPERTY_ENTRY_STRING("gpios",		"child-1-gpios"),
	{ }
};

static struct gpiod_lookup_table dev_gpiod_table = {
	.dev_id = "some-device",
	.table = {
		...
		GPIO_LOOKUP_IDX("B", 1, "child-1-gpios", 1, GPIO_ACTIVE_LOW),
		...
	},
};

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/gpio/gpiolib.c | 109 +++++++++++++++++++++++++++++------------
 1 file changed, 79 insertions(+), 30 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e11a3bb03820..f0e51d34a1c1 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3959,39 +3959,44 @@ struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
 }
 EXPORT_SYMBOL(gpiod_get_from_of_node);
 
-/**
- * fwnode_get_named_gpiod - obtain a GPIO from firmware node
- * @fwnode:	handle of the firmware node
- * @propname:	name of the firmware property representing the GPIO
- * @index:	index of the GPIO to obtain for the consumer
- * @dflags:	GPIO initialization flags
- * @label:	label to attach to the requested GPIO
- *
- * This function can be used for drivers that get their configuration
- * from opaque firmware.
- *
- * The function properly finds the corresponding GPIO using whatever is the
- * underlying firmware interface and then makes sure that the GPIO
- * descriptor is requested before it is returned to the caller.
- *
- * Returns:
- * On successful request the GPIO pin is configured in accordance with
- * provided @dflags.
- *
- * In case of error an ERR_PTR() is returned.
- */
-struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
-					 const char *propname, int index,
-					 enum gpiod_flags dflags,
-					 const char *label)
+static struct gpio_desc *pset_node_get_gpiod(struct fwnode_handle *fwnode,
+					     const char *propname, int index,
+					     enum gpio_lookup_flags *flags)
 {
-	struct gpio_desc *desc = ERR_PTR(-ENODEV);
-	unsigned long lflags = 0;
-	int ret;
+	struct property_set *pset;
+	const char *con_id;
 
-	if (!fwnode)
+	pset = to_pset_node(fwnode);
+	if (!pset)
 		return ERR_PTR(-EINVAL);
 
+	if (fwnode_property_read_string(fwnode, propname, &con_id)) {
+		/*
+		 * We could not find string mapping property name to
+		 * entry in gpio lookup table. Let's see if we are
+		 * dealing with firmware node corresponding to the
+		 * device (and not a child node): for such nodes we can
+		 * try doing lookup directly with property name.
+		 */
+		if (pset->parent)
+			return ERR_PTR(-ENOENT);
+
+		con_id = propname;
+	}
+
+	return gpiod_find(pset->dev, con_id, index, flags);
+}
+
+static struct gpio_desc *__fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
+						  const char *propname,
+						  int index,
+						  enum gpiod_flags dflags,
+						  const char *label)
+{
+	struct gpio_desc *desc = ERR_PTR(-ENODEV);
+	enum gpio_lookup_flags lflags = 0;
+	int ret;
+
 	if (is_of_node(fwnode)) {
 		desc = gpiod_get_from_of_node(to_of_node(fwnode),
 					      propname, index,
@@ -4009,9 +4014,12 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 
 		if (info.polarity == GPIO_ACTIVE_LOW)
 			lflags |= GPIO_ACTIVE_LOW;
+	} else if (is_pset_node(fwnode)) {
+		desc = pset_node_get_gpiod(fwnode, propname, index, &lflags);
+		if (IS_ERR(desc))
+			return desc;
 	}
 
-	/* Currently only ACPI takes this path */
 	ret = gpiod_request(desc, label);
 	if (ret)
 		return ERR_PTR(ret);
@@ -4024,6 +4032,47 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 
 	return desc;
 }
+
+/**
+ * fwnode_get_named_gpiod - obtain a GPIO from firmware node
+ * @fwnode:	handle of the firmware node
+ * @propname:	name of the firmware property representing the GPIO
+ * @index:	index of the GPIO to obtain for the consumer
+ * @dflags:	GPIO initialization flags
+ * @label:	label to attach to the requested GPIO
+ *
+ * This function can be used for drivers that get their configuration
+ * from opaque firmware.
+ *
+ * The function properly finds the corresponding GPIO using whatever is the
+ * underlying firmware interface and then makes sure that the GPIO
+ * descriptor is requested before it is returned to the caller.
+ *
+ * Returns:
+ * On successful request the GPIO pin is configured in accordance with
+ * provided @dflags.
+ *
+ * In case of error an ERR_PTR() is returned.
+ */
+struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
+					 const char *propname, int index,
+					 enum gpiod_flags dflags,
+					 const char *label)
+{
+	struct gpio_desc *desc;
+
+	if (!fwnode)
+		return ERR_PTR(-EINVAL);
+
+	desc = __fwnode_get_named_gpiod(fwnode, propname, index, dflags, label);
+	if (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT &&
+	    !IS_ERR_OR_NULL(fwnode->secondary)) {
+		desc = __fwnode_get_named_gpiod(fwnode->secondary,
+						propname, index, dflags, label);
+	}
+
+	return desc;
+}
 EXPORT_SYMBOL_GPL(fwnode_get_named_gpiod);
 
 /**
-- 
2.19.0.397.gdd90340f6a-goog

^ permalink raw reply related

* [RFC/PATCH 3/5] device property: export property_set structure
From: Dmitry Torokhov @ 2018-09-17 18:16 UTC (permalink / raw)
  To: Linus Walleij, Rafael J . Wysocki
  Cc: linux-input, linux-gpio, linux-kernel, Andy Shevchenko
In-Reply-To: <20180917181603.125492-1-dmitry.torokhov@gmail.com>

Future gpiolib will need to handle property sets properly, and this we
need to export property_set, is_pset_node() and to_pset_node().

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/base/pset_property.c | 31 ++-----------------------------
 include/linux/property.h     | 29 +++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/drivers/base/pset_property.c b/drivers/base/pset_property.c
index 63f2377aefe8..7118528816a4 100644
--- a/drivers/base/pset_property.c
+++ b/drivers/base/pset_property.c
@@ -14,34 +14,6 @@
 #include <linux/property.h>
 #include <linux/slab.h>
 
-struct property_set {
-	struct device *dev;
-	struct fwnode_handle fwnode;
-	const struct property_entry *properties;
-
-	struct property_set *parent;
-	/* Entry in parent->children list */
-	struct list_head child_node;
-	struct list_head children;
-};
-
-static const struct fwnode_operations pset_fwnode_ops;
-
-static inline bool is_pset_node(const struct fwnode_handle *fwnode)
-{
-	return !IS_ERR_OR_NULL(fwnode) && fwnode->ops == &pset_fwnode_ops;
-}
-
-#define to_pset_node(__fwnode)						\
-	({								\
-		typeof(__fwnode) __to_pset_node_fwnode = __fwnode;	\
-									\
-		is_pset_node(__to_pset_node_fwnode) ?			\
-			container_of(__to_pset_node_fwnode,		\
-				     struct property_set, fwnode) :	\
-			NULL;						\
-	})
-
 static const struct property_entry *
 pset_prop_get(const struct property_set *pset, const char *name)
 {
@@ -323,13 +295,14 @@ pset_fwnode_get_next_subnode(const struct fwnode_handle *fwnode,
 	return &next->fwnode;
 }
 
-static const struct fwnode_operations pset_fwnode_ops = {
+const struct fwnode_operations pset_fwnode_ops = {
 	.property_present = pset_fwnode_property_present,
 	.property_read_int_array = pset_fwnode_read_int_array,
 	.property_read_string_array = pset_fwnode_property_read_string_array,
 	.get_parent = pset_fwnode_get_parent,
 	.get_next_child_node = pset_fwnode_get_next_subnode,
 };
+EXPORT_SYMBOL_GPL(pset_fwnode_ops);
 
 static void property_entry_free_data(const struct property_entry *p)
 {
diff --git a/include/linux/property.h b/include/linux/property.h
index bb1cf4f30770..957f75d10cf9 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -13,6 +13,7 @@
 #ifndef _LINUX_PROPERTY_H_
 #define _LINUX_PROPERTY_H_
 
+#include <linux/err.h>
 #include <linux/fwnode.h>
 #include <linux/types.h>
 
@@ -281,6 +282,34 @@ device_add_child_properties(struct device *dev,
 			    const struct property_entry *properties);
 void device_remove_properties(struct device *dev);
 
+struct property_set {
+	struct device *dev;
+	struct fwnode_handle fwnode;
+	const struct property_entry *properties;
+
+	struct property_set *parent;
+	/* Entry in parent->children list */
+	struct list_head child_node;
+	struct list_head children;
+};
+
+extern const struct fwnode_operations pset_fwnode_ops;
+
+static inline bool is_pset_node(const struct fwnode_handle *fwnode)
+{
+	return !IS_ERR_OR_NULL(fwnode) && fwnode->ops == &pset_fwnode_ops;
+}
+
+#define to_pset_node(__fwnode)						\
+	({								\
+		typeof(__fwnode) __to_pset_node_fwnode = __fwnode;	\
+									\
+		is_pset_node(__to_pset_node_fwnode) ?			\
+			container_of(__to_pset_node_fwnode,		\
+				     struct property_set, fwnode) :	\
+			NULL;						\
+	})
+
 bool device_dma_supported(struct device *dev);
 
 enum dev_dma_attr device_get_dma_attr(struct device *dev);
-- 
2.19.0.397.gdd90340f6a-goog

^ permalink raw reply related

* [RFC/PATCH 2/5] device property: introduce notion of subnodes for legacy boards
From: Dmitry Torokhov @ 2018-09-17 18:16 UTC (permalink / raw)
  To: Linus Walleij, Rafael J . Wysocki
  Cc: linux-input, linux-gpio, linux-kernel, Andy Shevchenko
In-Reply-To: <20180917181603.125492-1-dmitry.torokhov@gmail.com>

Several drivers rely on having notion of sub-nodes when describing
hardware, let's allow static board-defined properties also have it.

The board files will then attach properties to devices in the following
fashion:

	device_add_properties(&board_platform_device.dev,
			      main_device_props);
	device_add_child_properties(&board_platform_device.dev,
				    dev_fwnode(&board_platform_device.dev),
				    child1_device_props);
	device_add_child_properties(&board_platform_device.dev,
				    dev_fwnode(&board_platform_device.dev),
				    child2_device_props);
	...
	platform_device_register(&board_platform_device.dev);

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/base/pset_property.c | 110 +++++++++++++++++++++++++++++++----
 include/linux/property.h     |   4 ++
 2 files changed, 102 insertions(+), 12 deletions(-)

diff --git a/drivers/base/pset_property.c b/drivers/base/pset_property.c
index 08ecc13080ae..63f2377aefe8 100644
--- a/drivers/base/pset_property.c
+++ b/drivers/base/pset_property.c
@@ -18,6 +18,11 @@ struct property_set {
 	struct device *dev;
 	struct fwnode_handle fwnode;
 	const struct property_entry *properties;
+
+	struct property_set *parent;
+	/* Entry in parent->children list */
+	struct list_head child_node;
+	struct list_head children;
 };
 
 static const struct fwnode_operations pset_fwnode_ops;
@@ -283,10 +288,47 @@ pset_fwnode_property_read_string_array(const struct fwnode_handle *fwnode,
 					   val, nval);
 }
 
+struct fwnode_handle *pset_fwnode_get_parent(const struct fwnode_handle *fwnode)
+{
+	struct property_set *pset = to_pset_node(fwnode);
+
+	return pset ? &pset->parent->fwnode : NULL;
+}
+
+struct fwnode_handle *
+pset_fwnode_get_next_subnode(const struct fwnode_handle *fwnode,
+			     struct fwnode_handle *child)
+{
+	const struct property_set *pset = to_pset_node(fwnode);
+	struct property_set *first_child;
+	struct property_set *next;
+
+	if (!pset)
+		return NULL;
+
+	if (list_empty(&pset->children))
+		return NULL;
+
+	first_child = list_first_entry(&pset->children, struct property_set,
+				       child_node);
+
+	if (child) {
+		next = list_next_entry(to_pset_node(child), child_node);
+		if (next == first_child)
+			return NULL;
+	} else {
+		next = first_child;
+	}
+
+	return &next->fwnode;
+}
+
 static const struct fwnode_operations pset_fwnode_ops = {
 	.property_present = pset_fwnode_property_present,
 	.property_read_int_array = pset_fwnode_read_int_array,
 	.property_read_string_array = pset_fwnode_property_read_string_array,
+	.get_parent = pset_fwnode_get_parent,
+	.get_next_child_node = pset_fwnode_get_next_subnode,
 };
 
 static void property_entry_free_data(const struct property_entry *p)
@@ -439,24 +481,31 @@ EXPORT_SYMBOL_GPL(property_entries_free);
  */
 static void pset_free_set(struct property_set *pset)
 {
+	struct property_set *child, *next;
+
 	if (!pset)
 		return;
 
+	list_for_each_entry_safe(child, next, &pset->children, child_node) {
+		list_del(&child->child_node);
+		pset_free_set(child);
+	}
+
 	property_entries_free(pset->properties);
 	kfree(pset);
 }
 
 /**
- * pset_copy_set - copies property set
- * @pset: Property set to copy
+ * pset_create_set - creates property set.
+ * @src: property entries for the set.
  *
- * This function takes a deep copy of the given property set and returns
- * pointer to the copy. Call device_free_property_set() to free resources
- * allocated in this function.
+ * This function takes a deep copy of the given property entries and creates
+ * property set. Call pset_free_set() to free resources allocated in this
+ * function.
  *
  * Return: Pointer to the new property set or error pointer.
  */
-static struct property_set *pset_copy_set(const struct property_set *pset)
+static struct property_set *pset_create_set(const struct property_entry *src)
 {
 	struct property_entry *properties;
 	struct property_set *p;
@@ -465,7 +514,11 @@ static struct property_set *pset_copy_set(const struct property_set *pset)
 	if (!p)
 		return ERR_PTR(-ENOMEM);
 
-	properties = property_entries_dup(pset->properties);
+	INIT_LIST_HEAD(&p->child_node);
+	INIT_LIST_HEAD(&p->children);
+	p->fwnode.ops = &pset_fwnode_ops;
+
+	properties = property_entries_dup(src);
 	if (IS_ERR(properties)) {
 		kfree(p);
 		return ERR_CAST(properties);
@@ -521,20 +574,53 @@ EXPORT_SYMBOL_GPL(device_remove_properties);
 int device_add_properties(struct device *dev,
 			  const struct property_entry *properties)
 {
-	struct property_set *p, pset;
+	struct property_set *p;
 
 	if (!properties)
 		return -EINVAL;
 
-	pset.properties = properties;
-
-	p = pset_copy_set(&pset);
+	p = pset_create_set(properties);
 	if (IS_ERR(p))
 		return PTR_ERR(p);
 
-	p->fwnode.ops = &pset_fwnode_ops;
 	set_secondary_fwnode(dev, &p->fwnode);
 	p->dev = dev;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(device_add_properties);
+
+/**
+ * device_add_child_properties - Add a collection of properties to a device object.
+ * @dev: Device to add properties to.
+ * @properties: Collection of properties to add.
+ *
+ * Associate a collection of device properties represented by @properties as a
+ * child of given @parent firmware node.  The function takes a copy of
+ * @properties.
+ */
+struct fwnode_handle *
+device_add_child_properties(struct device *dev,
+			    struct fwnode_handle *parent,
+			    const struct property_entry *properties)
+{
+	struct property_set *p;
+	struct property_set *parent_pset;
+
+	if (!properties)
+		return ERR_PTR(-EINVAL);
+
+	parent_pset = to_pset_node(parent);
+	if (!parent_pset)
+		return ERR_PTR(-EINVAL);
+
+	p = pset_create_set(properties);
+	if (IS_ERR(p))
+		return ERR_CAST(p);
+
+	p->dev = dev;
+	p->parent = parent_pset;
+	list_add_tail(&p->child_node, &parent_pset->children);
+
+	return &p->fwnode;
+}
+EXPORT_SYMBOL_GPL(device_add_child_properties);
diff --git a/include/linux/property.h b/include/linux/property.h
index ac8a1ebc4c1b..bb1cf4f30770 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -275,6 +275,10 @@ void property_entries_free(const struct property_entry *properties);
 
 int device_add_properties(struct device *dev,
 			  const struct property_entry *properties);
+struct fwnode_handle *
+device_add_child_properties(struct device *dev,
+			    struct fwnode_handle *parent,
+			    const struct property_entry *properties);
 void device_remove_properties(struct device *dev);
 
 bool device_dma_supported(struct device *dev);
-- 
2.19.0.397.gdd90340f6a-goog

^ permalink raw reply related

* [RFC/PATCH 1/5] device property: split generic properties and property sets
From: Dmitry Torokhov @ 2018-09-17 18:15 UTC (permalink / raw)
  To: Linus Walleij, Rafael J . Wysocki
  Cc: linux-input, linux-gpio, linux-kernel, Andy Shevchenko
In-Reply-To: <20180917181603.125492-1-dmitry.torokhov@gmail.com>

drivers/base/property.c became a bit of a kitchen sink, with code handling
generic device/fwnode properties intermingled with code handling static
properties (pset). Let's split them apart to not pollute each other.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/base/Makefile        |   4 +-
 drivers/base/property.c      | 525 ----------------------------------
 drivers/base/pset_property.c | 540 +++++++++++++++++++++++++++++++++++
 3 files changed, 542 insertions(+), 527 deletions(-)
 create mode 100644 drivers/base/pset_property.c

diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 704f44295810..5f72cd5ab8ba 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -5,8 +5,8 @@ obj-y			:= component.o core.o bus.o dd.o syscore.o \
 			   driver.o class.o platform.o \
 			   cpu.o firmware.o init.o map.o devres.o \
 			   attribute_container.o transport_class.o \
-			   topology.o container.o property.o cacheinfo.o \
-			   devcon.o
+			   topology.o container.o cacheinfo.o \
+			   property.o pset_property.o devcon.o
 obj-$(CONFIG_DEVTMPFS)	+= devtmpfs.o
 obj-y			+= power/
 obj-$(CONFIG_ISA_BUS_API)	+= isa.o
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 240ab5230ff6..d57df94e396c 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -18,236 +18,6 @@
 #include <linux/etherdevice.h>
 #include <linux/phy.h>
 
-struct property_set {
-	struct device *dev;
-	struct fwnode_handle fwnode;
-	const struct property_entry *properties;
-};
-
-static const struct fwnode_operations pset_fwnode_ops;
-
-static inline bool is_pset_node(const struct fwnode_handle *fwnode)
-{
-	return !IS_ERR_OR_NULL(fwnode) && fwnode->ops == &pset_fwnode_ops;
-}
-
-#define to_pset_node(__fwnode)						\
-	({								\
-		typeof(__fwnode) __to_pset_node_fwnode = __fwnode;	\
-									\
-		is_pset_node(__to_pset_node_fwnode) ?			\
-			container_of(__to_pset_node_fwnode,		\
-				     struct property_set, fwnode) :	\
-			NULL;						\
-	})
-
-static const struct property_entry *
-pset_prop_get(const struct property_set *pset, const char *name)
-{
-	const struct property_entry *prop;
-
-	if (!pset || !pset->properties)
-		return NULL;
-
-	for (prop = pset->properties; prop->name; prop++)
-		if (!strcmp(name, prop->name))
-			return prop;
-
-	return NULL;
-}
-
-static const void *property_get_pointer(const struct property_entry *prop)
-{
-	switch (prop->type) {
-	case DEV_PROP_U8:
-		if (prop->is_array)
-			return prop->pointer.u8_data;
-		return &prop->value.u8_data;
-	case DEV_PROP_U16:
-		if (prop->is_array)
-			return prop->pointer.u16_data;
-		return &prop->value.u16_data;
-	case DEV_PROP_U32:
-		if (prop->is_array)
-			return prop->pointer.u32_data;
-		return &prop->value.u32_data;
-	case DEV_PROP_U64:
-		if (prop->is_array)
-			return prop->pointer.u64_data;
-		return &prop->value.u64_data;
-	case DEV_PROP_STRING:
-		if (prop->is_array)
-			return prop->pointer.str;
-		return &prop->value.str;
-	default:
-		return NULL;
-	}
-}
-
-static void property_set_pointer(struct property_entry *prop, const void *pointer)
-{
-	switch (prop->type) {
-	case DEV_PROP_U8:
-		if (prop->is_array)
-			prop->pointer.u8_data = pointer;
-		else
-			prop->value.u8_data = *((u8 *)pointer);
-		break;
-	case DEV_PROP_U16:
-		if (prop->is_array)
-			prop->pointer.u16_data = pointer;
-		else
-			prop->value.u16_data = *((u16 *)pointer);
-		break;
-	case DEV_PROP_U32:
-		if (prop->is_array)
-			prop->pointer.u32_data = pointer;
-		else
-			prop->value.u32_data = *((u32 *)pointer);
-		break;
-	case DEV_PROP_U64:
-		if (prop->is_array)
-			prop->pointer.u64_data = pointer;
-		else
-			prop->value.u64_data = *((u64 *)pointer);
-		break;
-	case DEV_PROP_STRING:
-		if (prop->is_array)
-			prop->pointer.str = pointer;
-		else
-			prop->value.str = pointer;
-		break;
-	default:
-		break;
-	}
-}
-
-static const void *pset_prop_find(const struct property_set *pset,
-				  const char *propname, size_t length)
-{
-	const struct property_entry *prop;
-	const void *pointer;
-
-	prop = pset_prop_get(pset, propname);
-	if (!prop)
-		return ERR_PTR(-EINVAL);
-	pointer = property_get_pointer(prop);
-	if (!pointer)
-		return ERR_PTR(-ENODATA);
-	if (length > prop->length)
-		return ERR_PTR(-EOVERFLOW);
-	return pointer;
-}
-
-static int pset_prop_read_u8_array(const struct property_set *pset,
-				   const char *propname,
-				   u8 *values, size_t nval)
-{
-	const void *pointer;
-	size_t length = nval * sizeof(*values);
-
-	pointer = pset_prop_find(pset, propname, length);
-	if (IS_ERR(pointer))
-		return PTR_ERR(pointer);
-
-	memcpy(values, pointer, length);
-	return 0;
-}
-
-static int pset_prop_read_u16_array(const struct property_set *pset,
-				    const char *propname,
-				    u16 *values, size_t nval)
-{
-	const void *pointer;
-	size_t length = nval * sizeof(*values);
-
-	pointer = pset_prop_find(pset, propname, length);
-	if (IS_ERR(pointer))
-		return PTR_ERR(pointer);
-
-	memcpy(values, pointer, length);
-	return 0;
-}
-
-static int pset_prop_read_u32_array(const struct property_set *pset,
-				    const char *propname,
-				    u32 *values, size_t nval)
-{
-	const void *pointer;
-	size_t length = nval * sizeof(*values);
-
-	pointer = pset_prop_find(pset, propname, length);
-	if (IS_ERR(pointer))
-		return PTR_ERR(pointer);
-
-	memcpy(values, pointer, length);
-	return 0;
-}
-
-static int pset_prop_read_u64_array(const struct property_set *pset,
-				    const char *propname,
-				    u64 *values, size_t nval)
-{
-	const void *pointer;
-	size_t length = nval * sizeof(*values);
-
-	pointer = pset_prop_find(pset, propname, length);
-	if (IS_ERR(pointer))
-		return PTR_ERR(pointer);
-
-	memcpy(values, pointer, length);
-	return 0;
-}
-
-static int pset_prop_count_elems_of_size(const struct property_set *pset,
-					 const char *propname, size_t length)
-{
-	const struct property_entry *prop;
-
-	prop = pset_prop_get(pset, propname);
-	if (!prop)
-		return -EINVAL;
-
-	return prop->length / length;
-}
-
-static int pset_prop_read_string_array(const struct property_set *pset,
-				       const char *propname,
-				       const char **strings, size_t nval)
-{
-	const struct property_entry *prop;
-	const void *pointer;
-	size_t array_len, length;
-
-	/* Find out the array length. */
-	prop = pset_prop_get(pset, propname);
-	if (!prop)
-		return -EINVAL;
-
-	if (!prop->is_array)
-		/* The array length for a non-array string property is 1. */
-		array_len = 1;
-	else
-		/* Find the length of an array. */
-		array_len = pset_prop_count_elems_of_size(pset, propname,
-							  sizeof(const char *));
-
-	/* Return how many there are if strings is NULL. */
-	if (!strings)
-		return array_len;
-
-	array_len = min(nval, array_len);
-	length = array_len * sizeof(*strings);
-
-	pointer = pset_prop_find(pset, propname, length);
-	if (IS_ERR(pointer))
-		return PTR_ERR(pointer);
-
-	memcpy(strings, pointer, length);
-
-	return array_len;
-}
-
 struct fwnode_handle *dev_fwnode(struct device *dev)
 {
 	return IS_ENABLED(CONFIG_OF) && dev->of_node ?
@@ -255,51 +25,6 @@ struct fwnode_handle *dev_fwnode(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(dev_fwnode);
 
-static bool pset_fwnode_property_present(const struct fwnode_handle *fwnode,
-					 const char *propname)
-{
-	return !!pset_prop_get(to_pset_node(fwnode), propname);
-}
-
-static int pset_fwnode_read_int_array(const struct fwnode_handle *fwnode,
-				      const char *propname,
-				      unsigned int elem_size, void *val,
-				      size_t nval)
-{
-	const struct property_set *node = to_pset_node(fwnode);
-
-	if (!val)
-		return pset_prop_count_elems_of_size(node, propname, elem_size);
-
-	switch (elem_size) {
-	case sizeof(u8):
-		return pset_prop_read_u8_array(node, propname, val, nval);
-	case sizeof(u16):
-		return pset_prop_read_u16_array(node, propname, val, nval);
-	case sizeof(u32):
-		return pset_prop_read_u32_array(node, propname, val, nval);
-	case sizeof(u64):
-		return pset_prop_read_u64_array(node, propname, val, nval);
-	}
-
-	return -ENXIO;
-}
-
-static int
-pset_fwnode_property_read_string_array(const struct fwnode_handle *fwnode,
-				       const char *propname,
-				       const char **val, size_t nval)
-{
-	return pset_prop_read_string_array(to_pset_node(fwnode), propname,
-					   val, nval);
-}
-
-static const struct fwnode_operations pset_fwnode_ops = {
-	.property_present = pset_fwnode_property_present,
-	.property_read_int_array = pset_fwnode_read_int_array,
-	.property_read_string_array = pset_fwnode_property_read_string_array,
-};
-
 /**
  * device_property_present - check if a property of a device is present
  * @dev: Device whose property is being checked
@@ -759,256 +484,6 @@ int fwnode_property_get_reference_args(const struct fwnode_handle *fwnode,
 }
 EXPORT_SYMBOL_GPL(fwnode_property_get_reference_args);
 
-static void property_entry_free_data(const struct property_entry *p)
-{
-	const void *pointer = property_get_pointer(p);
-	size_t i, nval;
-
-	if (p->is_array) {
-		if (p->type == DEV_PROP_STRING && p->pointer.str) {
-			nval = p->length / sizeof(const char *);
-			for (i = 0; i < nval; i++)
-				kfree(p->pointer.str[i]);
-		}
-		kfree(pointer);
-	} else if (p->type == DEV_PROP_STRING) {
-		kfree(p->value.str);
-	}
-	kfree(p->name);
-}
-
-static int property_copy_string_array(struct property_entry *dst,
-				      const struct property_entry *src)
-{
-	const char **d;
-	size_t nval = src->length / sizeof(*d);
-	int i;
-
-	d = kcalloc(nval, sizeof(*d), GFP_KERNEL);
-	if (!d)
-		return -ENOMEM;
-
-	for (i = 0; i < nval; i++) {
-		d[i] = kstrdup(src->pointer.str[i], GFP_KERNEL);
-		if (!d[i] && src->pointer.str[i]) {
-			while (--i >= 0)
-				kfree(d[i]);
-			kfree(d);
-			return -ENOMEM;
-		}
-	}
-
-	dst->pointer.str = d;
-	return 0;
-}
-
-static int property_entry_copy_data(struct property_entry *dst,
-				    const struct property_entry *src)
-{
-	const void *pointer = property_get_pointer(src);
-	const void *new;
-	int error;
-
-	if (src->is_array) {
-		if (!src->length)
-			return -ENODATA;
-
-		if (src->type == DEV_PROP_STRING) {
-			error = property_copy_string_array(dst, src);
-			if (error)
-				return error;
-			new = dst->pointer.str;
-		} else {
-			new = kmemdup(pointer, src->length, GFP_KERNEL);
-			if (!new)
-				return -ENOMEM;
-		}
-	} else if (src->type == DEV_PROP_STRING) {
-		new = kstrdup(src->value.str, GFP_KERNEL);
-		if (!new && src->value.str)
-			return -ENOMEM;
-	} else {
-		new = pointer;
-	}
-
-	dst->length = src->length;
-	dst->is_array = src->is_array;
-	dst->type = src->type;
-
-	property_set_pointer(dst, new);
-
-	dst->name = kstrdup(src->name, GFP_KERNEL);
-	if (!dst->name)
-		goto out_free_data;
-
-	return 0;
-
-out_free_data:
-	property_entry_free_data(dst);
-	return -ENOMEM;
-}
-
-/**
- * property_entries_dup - duplicate array of properties
- * @properties: array of properties to copy
- *
- * This function creates a deep copy of the given NULL-terminated array
- * of property entries.
- */
-struct property_entry *
-property_entries_dup(const struct property_entry *properties)
-{
-	struct property_entry *p;
-	int i, n = 0;
-
-	while (properties[n].name)
-		n++;
-
-	p = kcalloc(n + 1, sizeof(*p), GFP_KERNEL);
-	if (!p)
-		return ERR_PTR(-ENOMEM);
-
-	for (i = 0; i < n; i++) {
-		int ret = property_entry_copy_data(&p[i], &properties[i]);
-		if (ret) {
-			while (--i >= 0)
-				property_entry_free_data(&p[i]);
-			kfree(p);
-			return ERR_PTR(ret);
-		}
-	}
-
-	return p;
-}
-EXPORT_SYMBOL_GPL(property_entries_dup);
-
-/**
- * property_entries_free - free previously allocated array of properties
- * @properties: array of properties to destroy
- *
- * This function frees given NULL-terminated array of property entries,
- * along with their data.
- */
-void property_entries_free(const struct property_entry *properties)
-{
-	const struct property_entry *p;
-
-	for (p = properties; p->name; p++)
-		property_entry_free_data(p);
-
-	kfree(properties);
-}
-EXPORT_SYMBOL_GPL(property_entries_free);
-
-/**
- * pset_free_set - releases memory allocated for copied property set
- * @pset: Property set to release
- *
- * Function takes previously copied property set and releases all the
- * memory allocated to it.
- */
-static void pset_free_set(struct property_set *pset)
-{
-	if (!pset)
-		return;
-
-	property_entries_free(pset->properties);
-	kfree(pset);
-}
-
-/**
- * pset_copy_set - copies property set
- * @pset: Property set to copy
- *
- * This function takes a deep copy of the given property set and returns
- * pointer to the copy. Call device_free_property_set() to free resources
- * allocated in this function.
- *
- * Return: Pointer to the new property set or error pointer.
- */
-static struct property_set *pset_copy_set(const struct property_set *pset)
-{
-	struct property_entry *properties;
-	struct property_set *p;
-
-	p = kzalloc(sizeof(*p), GFP_KERNEL);
-	if (!p)
-		return ERR_PTR(-ENOMEM);
-
-	properties = property_entries_dup(pset->properties);
-	if (IS_ERR(properties)) {
-		kfree(p);
-		return ERR_CAST(properties);
-	}
-
-	p->properties = properties;
-	return p;
-}
-
-/**
- * device_remove_properties - Remove properties from a device object.
- * @dev: Device whose properties to remove.
- *
- * The function removes properties previously associated to the device
- * secondary firmware node with device_add_properties(). Memory allocated
- * to the properties will also be released.
- */
-void device_remove_properties(struct device *dev)
-{
-	struct fwnode_handle *fwnode;
-	struct property_set *pset;
-
-	fwnode = dev_fwnode(dev);
-	if (!fwnode)
-		return;
-	/*
-	 * Pick either primary or secondary node depending which one holds
-	 * the pset. If there is no real firmware node (ACPI/DT) primary
-	 * will hold the pset.
-	 */
-	pset = to_pset_node(fwnode);
-	if (pset) {
-		set_primary_fwnode(dev, NULL);
-	} else {
-		pset = to_pset_node(fwnode->secondary);
-		if (pset && dev == pset->dev)
-			set_secondary_fwnode(dev, NULL);
-	}
-	if (pset && dev == pset->dev)
-		pset_free_set(pset);
-}
-EXPORT_SYMBOL_GPL(device_remove_properties);
-
-/**
- * device_add_properties - Add a collection of properties to a device object.
- * @dev: Device to add properties to.
- * @properties: Collection of properties to add.
- *
- * Associate a collection of device properties represented by @properties with
- * @dev as its secondary firmware node. The function takes a copy of
- * @properties.
- */
-int device_add_properties(struct device *dev,
-			  const struct property_entry *properties)
-{
-	struct property_set *p, pset;
-
-	if (!properties)
-		return -EINVAL;
-
-	pset.properties = properties;
-
-	p = pset_copy_set(&pset);
-	if (IS_ERR(p))
-		return PTR_ERR(p);
-
-	p->fwnode.ops = &pset_fwnode_ops;
-	set_secondary_fwnode(dev, &p->fwnode);
-	p->dev = dev;
-	return 0;
-}
-EXPORT_SYMBOL_GPL(device_add_properties);
-
 /**
  * fwnode_get_next_parent - Iterate to the node's parent
  * @fwnode: Firmware whose parent is retrieved
diff --git a/drivers/base/pset_property.c b/drivers/base/pset_property.c
new file mode 100644
index 000000000000..08ecc13080ae
--- /dev/null
+++ b/drivers/base/pset_property.c
@@ -0,0 +1,540 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Handling of device properties defined in legacy board files.
+ *
+ * Copyright (C) 2014, Intel Corporation
+ * Authors: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+ *          Mika Westerberg <mika.westerberg@linux.intel.com>
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+
+struct property_set {
+	struct device *dev;
+	struct fwnode_handle fwnode;
+	const struct property_entry *properties;
+};
+
+static const struct fwnode_operations pset_fwnode_ops;
+
+static inline bool is_pset_node(const struct fwnode_handle *fwnode)
+{
+	return !IS_ERR_OR_NULL(fwnode) && fwnode->ops == &pset_fwnode_ops;
+}
+
+#define to_pset_node(__fwnode)						\
+	({								\
+		typeof(__fwnode) __to_pset_node_fwnode = __fwnode;	\
+									\
+		is_pset_node(__to_pset_node_fwnode) ?			\
+			container_of(__to_pset_node_fwnode,		\
+				     struct property_set, fwnode) :	\
+			NULL;						\
+	})
+
+static const struct property_entry *
+pset_prop_get(const struct property_set *pset, const char *name)
+{
+	const struct property_entry *prop;
+
+	if (!pset || !pset->properties)
+		return NULL;
+
+	for (prop = pset->properties; prop->name; prop++)
+		if (!strcmp(name, prop->name))
+			return prop;
+
+	return NULL;
+}
+
+static const void *property_get_pointer(const struct property_entry *prop)
+{
+	switch (prop->type) {
+	case DEV_PROP_U8:
+		if (prop->is_array)
+			return prop->pointer.u8_data;
+		return &prop->value.u8_data;
+	case DEV_PROP_U16:
+		if (prop->is_array)
+			return prop->pointer.u16_data;
+		return &prop->value.u16_data;
+	case DEV_PROP_U32:
+		if (prop->is_array)
+			return prop->pointer.u32_data;
+		return &prop->value.u32_data;
+	case DEV_PROP_U64:
+		if (prop->is_array)
+			return prop->pointer.u64_data;
+		return &prop->value.u64_data;
+	case DEV_PROP_STRING:
+		if (prop->is_array)
+			return prop->pointer.str;
+		return &prop->value.str;
+	default:
+		return NULL;
+	}
+}
+
+static void property_set_pointer(struct property_entry *prop, const void *pointer)
+{
+	switch (prop->type) {
+	case DEV_PROP_U8:
+		if (prop->is_array)
+			prop->pointer.u8_data = pointer;
+		else
+			prop->value.u8_data = *((u8 *)pointer);
+		break;
+	case DEV_PROP_U16:
+		if (prop->is_array)
+			prop->pointer.u16_data = pointer;
+		else
+			prop->value.u16_data = *((u16 *)pointer);
+		break;
+	case DEV_PROP_U32:
+		if (prop->is_array)
+			prop->pointer.u32_data = pointer;
+		else
+			prop->value.u32_data = *((u32 *)pointer);
+		break;
+	case DEV_PROP_U64:
+		if (prop->is_array)
+			prop->pointer.u64_data = pointer;
+		else
+			prop->value.u64_data = *((u64 *)pointer);
+		break;
+	case DEV_PROP_STRING:
+		if (prop->is_array)
+			prop->pointer.str = pointer;
+		else
+			prop->value.str = pointer;
+		break;
+	default:
+		break;
+	}
+}
+
+static const void *pset_prop_find(const struct property_set *pset,
+				  const char *propname, size_t length)
+{
+	const struct property_entry *prop;
+	const void *pointer;
+
+	prop = pset_prop_get(pset, propname);
+	if (!prop)
+		return ERR_PTR(-EINVAL);
+	pointer = property_get_pointer(prop);
+	if (!pointer)
+		return ERR_PTR(-ENODATA);
+	if (length > prop->length)
+		return ERR_PTR(-EOVERFLOW);
+	return pointer;
+}
+
+static int pset_prop_read_u8_array(const struct property_set *pset,
+				   const char *propname,
+				   u8 *values, size_t nval)
+{
+	const void *pointer;
+	size_t length = nval * sizeof(*values);
+
+	pointer = pset_prop_find(pset, propname, length);
+	if (IS_ERR(pointer))
+		return PTR_ERR(pointer);
+
+	memcpy(values, pointer, length);
+	return 0;
+}
+
+static int pset_prop_read_u16_array(const struct property_set *pset,
+				    const char *propname,
+				    u16 *values, size_t nval)
+{
+	const void *pointer;
+	size_t length = nval * sizeof(*values);
+
+	pointer = pset_prop_find(pset, propname, length);
+	if (IS_ERR(pointer))
+		return PTR_ERR(pointer);
+
+	memcpy(values, pointer, length);
+	return 0;
+}
+
+static int pset_prop_read_u32_array(const struct property_set *pset,
+				    const char *propname,
+				    u32 *values, size_t nval)
+{
+	const void *pointer;
+	size_t length = nval * sizeof(*values);
+
+	pointer = pset_prop_find(pset, propname, length);
+	if (IS_ERR(pointer))
+		return PTR_ERR(pointer);
+
+	memcpy(values, pointer, length);
+	return 0;
+}
+
+static int pset_prop_read_u64_array(const struct property_set *pset,
+				    const char *propname,
+				    u64 *values, size_t nval)
+{
+	const void *pointer;
+	size_t length = nval * sizeof(*values);
+
+	pointer = pset_prop_find(pset, propname, length);
+	if (IS_ERR(pointer))
+		return PTR_ERR(pointer);
+
+	memcpy(values, pointer, length);
+	return 0;
+}
+
+static int pset_prop_count_elems_of_size(const struct property_set *pset,
+					 const char *propname, size_t length)
+{
+	const struct property_entry *prop;
+
+	prop = pset_prop_get(pset, propname);
+	if (!prop)
+		return -EINVAL;
+
+	return prop->length / length;
+}
+
+static int pset_prop_read_string_array(const struct property_set *pset,
+				       const char *propname,
+				       const char **strings, size_t nval)
+{
+	const struct property_entry *prop;
+	const void *pointer;
+	size_t array_len, length;
+
+	/* Find out the array length. */
+	prop = pset_prop_get(pset, propname);
+	if (!prop)
+		return -EINVAL;
+
+	if (!prop->is_array)
+		/* The array length for a non-array string property is 1. */
+		array_len = 1;
+	else
+		/* Find the length of an array. */
+		array_len = pset_prop_count_elems_of_size(pset, propname,
+							  sizeof(const char *));
+
+	/* Return how many there are if strings is NULL. */
+	if (!strings)
+		return array_len;
+
+	array_len = min(nval, array_len);
+	length = array_len * sizeof(*strings);
+
+	pointer = pset_prop_find(pset, propname, length);
+	if (IS_ERR(pointer))
+		return PTR_ERR(pointer);
+
+	memcpy(strings, pointer, length);
+
+	return array_len;
+}
+
+static bool pset_fwnode_property_present(const struct fwnode_handle *fwnode,
+					 const char *propname)
+{
+	return !!pset_prop_get(to_pset_node(fwnode), propname);
+}
+
+static int pset_fwnode_read_int_array(const struct fwnode_handle *fwnode,
+				      const char *propname,
+				      unsigned int elem_size, void *val,
+				      size_t nval)
+{
+	const struct property_set *node = to_pset_node(fwnode);
+
+	if (!val)
+		return pset_prop_count_elems_of_size(node, propname, elem_size);
+
+	switch (elem_size) {
+	case sizeof(u8):
+		return pset_prop_read_u8_array(node, propname, val, nval);
+	case sizeof(u16):
+		return pset_prop_read_u16_array(node, propname, val, nval);
+	case sizeof(u32):
+		return pset_prop_read_u32_array(node, propname, val, nval);
+	case sizeof(u64):
+		return pset_prop_read_u64_array(node, propname, val, nval);
+	}
+
+	return -ENXIO;
+}
+
+static int
+pset_fwnode_property_read_string_array(const struct fwnode_handle *fwnode,
+				       const char *propname,
+				       const char **val, size_t nval)
+{
+	return pset_prop_read_string_array(to_pset_node(fwnode), propname,
+					   val, nval);
+}
+
+static const struct fwnode_operations pset_fwnode_ops = {
+	.property_present = pset_fwnode_property_present,
+	.property_read_int_array = pset_fwnode_read_int_array,
+	.property_read_string_array = pset_fwnode_property_read_string_array,
+};
+
+static void property_entry_free_data(const struct property_entry *p)
+{
+	const void *pointer = property_get_pointer(p);
+	size_t i, nval;
+
+	if (p->is_array) {
+		if (p->type == DEV_PROP_STRING && p->pointer.str) {
+			nval = p->length / sizeof(const char *);
+			for (i = 0; i < nval; i++)
+				kfree(p->pointer.str[i]);
+		}
+		kfree(pointer);
+	} else if (p->type == DEV_PROP_STRING) {
+		kfree(p->value.str);
+	}
+	kfree(p->name);
+}
+
+static int property_copy_string_array(struct property_entry *dst,
+				      const struct property_entry *src)
+{
+	const char **d;
+	size_t nval = src->length / sizeof(*d);
+	int i;
+
+	d = kcalloc(nval, sizeof(*d), GFP_KERNEL);
+	if (!d)
+		return -ENOMEM;
+
+	for (i = 0; i < nval; i++) {
+		d[i] = kstrdup(src->pointer.str[i], GFP_KERNEL);
+		if (!d[i] && src->pointer.str[i]) {
+			while (--i >= 0)
+				kfree(d[i]);
+			kfree(d);
+			return -ENOMEM;
+		}
+	}
+
+	dst->pointer.str = d;
+	return 0;
+}
+
+static int property_entry_copy_data(struct property_entry *dst,
+				    const struct property_entry *src)
+{
+	const void *pointer = property_get_pointer(src);
+	const void *new;
+	int error;
+
+	if (src->is_array) {
+		if (!src->length)
+			return -ENODATA;
+
+		if (src->type == DEV_PROP_STRING) {
+			error = property_copy_string_array(dst, src);
+			if (error)
+				return error;
+			new = dst->pointer.str;
+		} else {
+			new = kmemdup(pointer, src->length, GFP_KERNEL);
+			if (!new)
+				return -ENOMEM;
+		}
+	} else if (src->type == DEV_PROP_STRING) {
+		new = kstrdup(src->value.str, GFP_KERNEL);
+		if (!new && src->value.str)
+			return -ENOMEM;
+	} else {
+		new = pointer;
+	}
+
+	dst->length = src->length;
+	dst->is_array = src->is_array;
+	dst->type = src->type;
+
+	property_set_pointer(dst, new);
+
+	dst->name = kstrdup(src->name, GFP_KERNEL);
+	if (!dst->name)
+		goto out_free_data;
+
+	return 0;
+
+out_free_data:
+	property_entry_free_data(dst);
+	return -ENOMEM;
+}
+
+/**
+ * property_entries_dup - duplicate array of properties
+ * @properties: array of properties to copy
+ *
+ * This function creates a deep copy of the given NULL-terminated array
+ * of property entries.
+ */
+struct property_entry *
+property_entries_dup(const struct property_entry *properties)
+{
+	struct property_entry *p;
+	int i, n = 0;
+
+	while (properties[n].name)
+		n++;
+
+	p = kcalloc(n + 1, sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return ERR_PTR(-ENOMEM);
+
+	for (i = 0; i < n; i++) {
+		int ret = property_entry_copy_data(&p[i], &properties[i]);
+		if (ret) {
+			while (--i >= 0)
+				property_entry_free_data(&p[i]);
+			kfree(p);
+			return ERR_PTR(ret);
+		}
+	}
+
+	return p;
+}
+EXPORT_SYMBOL_GPL(property_entries_dup);
+
+/**
+ * property_entries_free - free previously allocated array of properties
+ * @properties: array of properties to destroy
+ *
+ * This function frees given NULL-terminated array of property entries,
+ * along with their data.
+ */
+void property_entries_free(const struct property_entry *properties)
+{
+	const struct property_entry *p;
+
+	for (p = properties; p->name; p++)
+		property_entry_free_data(p);
+
+	kfree(properties);
+}
+EXPORT_SYMBOL_GPL(property_entries_free);
+
+/**
+ * pset_free_set - releases memory allocated for copied property set
+ * @pset: Property set to release
+ *
+ * Function takes previously copied property set and releases all the
+ * memory allocated to it.
+ */
+static void pset_free_set(struct property_set *pset)
+{
+	if (!pset)
+		return;
+
+	property_entries_free(pset->properties);
+	kfree(pset);
+}
+
+/**
+ * pset_copy_set - copies property set
+ * @pset: Property set to copy
+ *
+ * This function takes a deep copy of the given property set and returns
+ * pointer to the copy. Call device_free_property_set() to free resources
+ * allocated in this function.
+ *
+ * Return: Pointer to the new property set or error pointer.
+ */
+static struct property_set *pset_copy_set(const struct property_set *pset)
+{
+	struct property_entry *properties;
+	struct property_set *p;
+
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return ERR_PTR(-ENOMEM);
+
+	properties = property_entries_dup(pset->properties);
+	if (IS_ERR(properties)) {
+		kfree(p);
+		return ERR_CAST(properties);
+	}
+
+	p->properties = properties;
+	return p;
+}
+
+/**
+ * device_remove_properties - Remove properties from a device object.
+ * @dev: Device whose properties to remove.
+ *
+ * The function removes properties previously associated to the device
+ * secondary firmware node with device_add_properties(). Memory allocated
+ * to the properties will also be released.
+ */
+void device_remove_properties(struct device *dev)
+{
+	struct fwnode_handle *fwnode;
+	struct property_set *pset;
+
+	fwnode = dev_fwnode(dev);
+	if (!fwnode)
+		return;
+	/*
+	 * Pick either primary or secondary node depending which one holds
+	 * the pset. If there is no real firmware node (ACPI/DT) primary
+	 * will hold the pset.
+	 */
+	pset = to_pset_node(fwnode);
+	if (pset) {
+		set_primary_fwnode(dev, NULL);
+	} else {
+		pset = to_pset_node(fwnode->secondary);
+		if (pset && dev == pset->dev)
+			set_secondary_fwnode(dev, NULL);
+	}
+	if (pset && dev == pset->dev)
+		pset_free_set(pset);
+}
+EXPORT_SYMBOL_GPL(device_remove_properties);
+
+/**
+ * device_add_properties - Add a collection of properties to a device object.
+ * @dev: Device to add properties to.
+ * @properties: Collection of properties to add.
+ *
+ * Associate a collection of device properties represented by @properties with
+ * @dev as its secondary firmware node. The function takes a copy of
+ * @properties.
+ */
+int device_add_properties(struct device *dev,
+			  const struct property_entry *properties)
+{
+	struct property_set *p, pset;
+
+	if (!properties)
+		return -EINVAL;
+
+	pset.properties = properties;
+
+	p = pset_copy_set(&pset);
+	if (IS_ERR(p))
+		return PTR_ERR(p);
+
+	p->fwnode.ops = &pset_fwnode_ops;
+	set_secondary_fwnode(dev, &p->fwnode);
+	p->dev = dev;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(device_add_properties);
-- 
2.19.0.397.gdd90340f6a-goog

^ permalink raw reply related

* [RFC/PATCH 0/5] Support children for legacy device properties
From: Dmitry Torokhov @ 2018-09-17 18:15 UTC (permalink / raw)
  To: Linus Walleij, Rafael J . Wysocki
  Cc: linux-input, linux-gpio, linux-kernel, Andy Shevchenko

The generic device properties APIs are very helpful as they allow abstracting
away details of the platform (whether it is ACPI, device tree, or legacy board
file), so that individual driver does not need separate code paths to support
all variants. However there are drivers that currently can not use generic
device properties API as they need notion of children properties, for example
gpio_keys driver, that expects every button to be described as a sub-node of
main device.

This patch series introduces notion of sub-nodes for static properties and ties
it up with GPIO lookup tables so that they are usable with sub-nodes as well.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH V2] input: egalax_ts: add system wakeup support
From: Dmitry Torokhov @ 2018-09-17 18:09 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Anson Huang, Fabio Estevam, linux-input, linux-kernel,
	NXP Linux Team
In-Reply-To: <CAOMZO5DzYBz3U=RT-HzzK5KMMnYipceCWh9eYuYRmBctkRZ_=w@mail.gmail.com>

On Thu, Sep 06, 2018 at 01:46:32PM -0300, Fabio Estevam wrote:
> On Thu, Sep 6, 2018 at 12:24 AM, Anson Huang <Anson.Huang@nxp.com> wrote:
> > This patch adds wakeup function support for egalax touch
> > screen, if "wakeup-source" is added to device tree's egalax
> > touch screen node, the wakeup function will be enabled, and
> > egalax touch screen will be able to wakeup system from suspend.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> 
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 2/2] dt-bindings: egalax-ts: add support for wakeup event action
From: Dmitry Torokhov @ 2018-09-17 18:07 UTC (permalink / raw)
  To: Rob Herring
  Cc: Anson Huang, mark.rutland, marco.franchi, fabio.estevam,
	linux-input, devicetree, linux-kernel, Linux-imx
In-Reply-To: <5b9f3f5d.1c69fb81.bca6d.cdee@mx.google.com>

On Sun, Sep 16, 2018 at 05:54:33PM -0500, Rob Herring wrote:
> On Wed, Sep 05, 2018 at 05:26:47PM +0800, Anson Huang wrote:
> > Add support for wakeup event action, this would allow the device
> > to configure whether to be a wakeup source of system suspend.
> > 
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/input/touchscreen/egalax-ts.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/egalax-ts.txt b/Documentation/devicetree/bindings/input/touchscreen/egalax-ts.txt
> > index 92fb262..296a9cd 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/egalax-ts.txt
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/egalax-ts.txt
> > @@ -6,6 +6,7 @@ Required properties:
> >  - interrupts: touch controller interrupt
> >  - wakeup-gpios: the gpio pin to be used for waking up the controller
> >    and also used as irq pin
> 
> If the wakeup gpio is the same as the irq, then this should be removed 
> since wakeup-source replaces it.

No, it does not: it is GPIO that we need to toggle to wake up the
controller, not host, and we do not have generic API to map from IRQ to
GPIO.

This is also existing binding...

> 
> > +- wakeup-source: boolean, touch screen can wake-up the system
> >  
> >  Example:
> >  
> > @@ -15,4 +16,5 @@ Example:
> >  		interrupt-parent = <&gpio1>;
> >  		interrupts = <9 2>;
> >  		wakeup-gpios = <&gpio1 9 0>;
> > +		wakeup-source;
> >  	};
> > -- 
> > 2.7.4
> > 
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
From: Jonathan Cameron @ 2018-09-17  9:33 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	platform-driver-x86-u79uwXL29TY76Z2rM5mHXA,
	sparclinux-u79uwXL29TY76Z2rM5mHXA,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	qat-linux-ral2JQCrhuEAvxtiuMwx3w,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	ceph-devel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, David S. Miller,
	linux-btrfs-u79uwXL29TY76Z2rM5mHXA,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn
In-Reply-To: <20180912151134.436719-1-arnd-r2nGTMty4D4@public.gmane.org>

On Wed, 12 Sep 2018 17:08:52 +0200
Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:

> The .ioctl and .compat_ioctl file operations have the same prototype so
> they can both point to the same function, which works great almost all
> the time when all the commands are compatible.
> 
> One exception is the s390 architecture, where a compat pointer is only
> 31 bit wide, and converting it into a 64-bit pointer requires calling
> compat_ptr(). Most drivers here will ever run in s390, but since we now
> have a generic helper for it, it's easy enough to use it consistently.
> 
> I double-checked all these drivers to ensure that all ioctl arguments
> are used as pointers or are ignored, but are not interpreted as integer
> values.
> 
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> ---

For IIO part.

Acked-by: Jonathan Cameron <Jonathan.Cameron-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>

Thanks,
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index a062cfddc5af..22844b94b0e9 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1630,7 +1630,7 @@ static const struct file_operations iio_buffer_fileops = {
>  	.owner = THIS_MODULE,
>  	.llseek = noop_llseek,
>  	.unlocked_ioctl = iio_ioctl,
> -	.compat_ioctl = iio_ioctl,
> +	.compat_ioctl = generic_compat_ioctl_ptrarg,
>  };
>  

^ permalink raw reply

* Re: [PATCH v2 02/17] compat_ioctl: move drivers to generic_compat_ioctl_ptrarg
From: Jarkko Sakkinen @ 2018-09-16 19:07 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: kvm, Alexander Shishkin, virtualization, Benjamin Tissoires,
	linux-mtd, Peter Huewe, linux1394-devel, devel, Jason Gunthorpe,
	Marek Vasut, linux-input, Tomas Winkler, Jiri Kosina,
	Alex Williamson, viro, OGAWA Hirofumi, Artem Bityutskiy,
	Greg Kroah-Hartman, linux-usb, linux-kernel, Sudip Mukherjee,
	Stefan Richter, netdev, linux-fsdevel
In-Reply-To: <20180912150142.157913-2-arnd@arndb.de>

On Wed, Sep 12, 2018 at 05:01:03PM +0200, Arnd Bergmann wrote:
> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> index 87a0ce47f201..a170f5ca7416 100644
> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> @@ -678,20 +678,10 @@ static long vtpmx_fops_ioctl(struct file *f, unsigned int ioctl,
>  	}
>  }
>  
> -#ifdef CONFIG_COMPAT
> -static long vtpmx_fops_compat_ioctl(struct file *f, unsigned int ioctl,
> -					  unsigned long arg)
> -{
> -	return vtpmx_fops_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
> -}
> -#endif
> -
>  static const struct file_operations vtpmx_fops = {
>  	.owner = THIS_MODULE,
>  	.unlocked_ioctl = vtpmx_fops_ioctl,
> -#ifdef CONFIG_COMPAT
> -	.compat_ioctl = vtpmx_fops_compat_ioctl,
> -#endif
> +	.compat_ioctl = generic_compat_ioctl_ptrarg,
>  	.llseek = noop_llseek,
>  };

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

^ permalink raw reply

* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
From: Al Viro @ 2018-09-14 20:57 UTC (permalink / raw)
  To: Darren Hart
  Cc: Arnd Bergmann, linux-fsdevel, Greg Kroah-Hartman, David S. Miller,
	devel, linux-kernel, qat-linux, linux-crypto, linux-media,
	dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio,
	linux-rdma, linux-nvdimm, linux-nvme, linux-pci,
	platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi,
	linux-usb, linux-fbdev, linuxppc-dev, linux-btrfs
In-Reply-To: <20180914203506.GE35251@wrath>

On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote:
 
> Acked-by: Darren Hart (VMware) <dvhart@infradead.org>
> 
> As for a longer term solution, would it be possible to init fops in such
> a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg
> so we don't have to duplicate this boilerplate for every ioctl fops
> structure?

	Bad idea, that...  Because several years down the road somebody will add
an ioctl that takes an unsigned int for argument.  Without so much as looking
at your magical mystery macro being used to initialize file_operations.

	FWIW, I would name that helper in more blunt way - something like
compat_ioctl_only_compat_pointer_ioctls_here()...

^ permalink raw reply

* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
From: Darren Hart @ 2018-09-14 20:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: viro, linux-fsdevel, Greg Kroah-Hartman, David S. Miller, devel,
	linux-kernel, qat-linux, linux-crypto, linux-media, dri-devel,
	linaro-mm-sig, amd-gfx, linux-input, linux-iio, linux-rdma,
	linux-nvdimm, linux-nvme, linux-pci, platform-driver-x86,
	linux-remoteproc, sparclinux, linux-scsi, linux-usb, linux-fbdev,
	linuxppc-dev, linux-btrfs
In-Reply-To: <20180912151134.436719-1-arnd@arndb.de>

On Wed, Sep 12, 2018 at 05:08:52PM +0200, Arnd Bergmann wrote:
> The .ioctl and .compat_ioctl file operations have the same prototype so
> they can both point to the same function, which works great almost all
> the time when all the commands are compatible.
> 
> One exception is the s390 architecture, where a compat pointer is only
> 31 bit wide, and converting it into a 64-bit pointer requires calling
> compat_ptr(). Most drivers here will ever run in s390, but since we now
> have a generic helper for it, it's easy enough to use it consistently.
> 
> I double-checked all these drivers to ensure that all ioctl arguments
> are used as pointers or are ignored, but are not interpreted as integer
> values.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
...
>  drivers/platform/x86/wmi.c                  | 2 +-
...
>  static void link_event_work(struct work_struct *work)
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index 04791ea5d97b..e4d0697e07d6 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -886,7 +886,7 @@ static const struct file_operations wmi_fops = {
>  	.read		= wmi_char_read,
>  	.open		= wmi_char_open,
>  	.unlocked_ioctl	= wmi_ioctl,
> -	.compat_ioctl	= wmi_ioctl,
> +	.compat_ioctl	= generic_compat_ioctl_ptrarg,
>  };

For platform/drivers/x86:

Acked-by: Darren Hart (VMware) <dvhart@infradead.org>

As for a longer term solution, would it be possible to init fops in such
a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg
so we don't have to duplicate this boilerplate for every ioctl fops
structure?

-- 
Darren Hart
VMware Open Source Technology Center

^ permalink raw reply

* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
From: David Sterba @ 2018-09-14 14:23 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	platform-driver-x86-u79uwXL29TY76Z2rM5mHXA,
	sparclinux-u79uwXL29TY76Z2rM5mHXA,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	qat-linux-ral2JQCrhuEAvxtiuMwx3w,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	ceph-devel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, David S. Miller,
	linux-btrfs-u79uwXL29TY76Z2rM5mHXA,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn
In-Reply-To: <20180912151134.436719-1-arnd-r2nGTMty4D4@public.gmane.org>

On Wed, Sep 12, 2018 at 05:08:52PM +0200, Arnd Bergmann wrote:
> The .ioctl and .compat_ioctl file operations have the same prototype so
> they can both point to the same function, which works great almost all
> the time when all the commands are compatible.
> 
> One exception is the s390 architecture, where a compat pointer is only
> 31 bit wide, and converting it into a 64-bit pointer requires calling
> compat_ptr(). Most drivers here will ever run in s390, but since we now
> have a generic helper for it, it's easy enough to use it consistently.
> 
> I double-checked all these drivers to ensure that all ioctl arguments
> are used as pointers or are ignored, but are not interpreted as integer
> values.
> 
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> ---

>  fs/btrfs/super.c                            | 2 +-

Acked-by: David Sterba <dsterba-IBi9RG/b67k@public.gmane.org>

^ permalink raw reply

* RE: [PATCH] hyper-v: Fix wakeup from suspend-to-idle
From: KY Srinivasan @ 2018-09-13 12:58 UTC (permalink / raw)
  To: Rafael J. Wysocki, vkuznets
  Cc: Linux PM, Rafael J. Wysocki, Haiyang Zhang, Stephen Hemminger,
	Jiri Kosina, Dmitry Torokhov, linux-input@vger.kernel.org,
	Linux Kernel Mailing List
In-Reply-To: <CAJZ5v0jmNFaH3JUVS9W_1ofXcD=8U8Y9enPi+j+9U=6cBVxfJQ@mail.gmail.com>



> -----Original Message-----
> From: Rafael J. Wysocki <rafael@kernel.org>
> Sent: Wednesday, September 12, 2018 11:55 PM
> To: vkuznets <vkuznets@redhat.com>
> Cc: Linux PM <linux-pm@vger.kernel.org>; Rafael J. Wysocki
> <rjw@rjwysocki.net>; KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; Stephen Hemminger
> <sthemmin@microsoft.com>; Jiri Kosina <jikos@kernel.org>; Dmitry
> Torokhov <dmitry.torokhov@gmail.com>; linux-input@vger.kernel.org;
> Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH] hyper-v: Fix wakeup from suspend-to-idle
> 
> On Wed, Sep 12, 2018 at 6:11 PM Vitaly Kuznetsov <vkuznets@redhat.com>
> wrote:
> >
> > It makes little sense but still possible to put Hyper-V guests into
> > suspend-to-idle state. To wake them up two wakeup sources were
> registered
> > in the past: hyperv-keyboard and hid-hyperv. However, since
> > commit eed4d47efe95 ("ACPI / sleep: Ignore spurious SCI wakeups from
> > suspend-to-idle") pm_wakeup_event() from these devices is ignored.
> Switch
> > to pm_wakeup_hard_event() API as these devices are actually the only
> > possible way to wakeup Hyper-V guests.
> >
> > Fixes: eed4d47efe95 (ACPI / sleep: Ignore spurious SCI wakeups from
> suspend-to-idle)
> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> 
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: K. Y. Srinivasan <kys@microsoft.com>

> 
> > ---
> >  drivers/hid/hid-hyperv.c              | 2 +-
> >  drivers/input/serio/hyperv-keyboard.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> > index b372854cf38d..704049e62d58 100644
> > --- a/drivers/hid/hid-hyperv.c
> > +++ b/drivers/hid/hid-hyperv.c
> > @@ -309,7 +309,7 @@ static void mousevsc_on_receive(struct hv_device
> *device,
> >                 hid_input_report(input_dev->hid_device, HID_INPUT_REPORT,
> >                                  input_dev->input_buf, len, 1);
> >
> > -               pm_wakeup_event(&input_dev->device->device, 0);
> > +               pm_wakeup_hard_event(&input_dev->device->device);
> >
> >                 break;
> >         default:
> > diff --git a/drivers/input/serio/hyperv-keyboard.c
> b/drivers/input/serio/hyperv-keyboard.c
> > index 47a0e81a2989..a8b9be3e28db 100644
> > --- a/drivers/input/serio/hyperv-keyboard.c
> > +++ b/drivers/input/serio/hyperv-keyboard.c
> > @@ -177,7 +177,7 @@ static void hv_kbd_on_receive(struct hv_device
> *hv_dev,
> >                  * state because the Enter-UP can trigger a wakeup at once.
> >                  */
> >                 if (!(info & IS_BREAK))
> > -                       pm_wakeup_event(&hv_dev->device, 0);
> > +                       pm_wakeup_hard_event(&hv_dev->device);
> >
> >                 break;
> >
> > --
> > 2.14.4
> >

^ permalink raw reply

* [PATCH v2] HID: logitech: fix a used uninitialized GCC warning
From: zhong jiang @ 2018-09-13  7:41 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel

Fix the following compile warning:

drivers/hid/hid-logitech-hidpp.c: In function 'hi_res_scroll_enable':
drivers/hid/hid-logitech-hidpp.c:2714:54: warning: 'multiplier' may be used uninitialized in this function [-Wmaybe-uninitialized]
  hidpp->vertical_wheel_counter.resolution_multiplier = multiplier;

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
v1->v2:
     According to Benjamin's suggestion, To initialize the value
and remove the duplicated assignement. 

 drivers/hid/hid-logitech-hidpp.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 5f0c080..f012808 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1231,7 +1231,6 @@ static int hidpp_hrw_get_wheel_capability(struct hidpp_device *hidpp,
 	*multiplier = response.fap.params[0];
 	return 0;
 return_default:
-	*multiplier = 8;
 	hid_warn(hidpp->hid_dev,
 		 "Couldn't get wheel multiplier (error %d), assuming %d.\n",
 		 ret, *multiplier);
@@ -2696,7 +2695,7 @@ static int hi_res_scroll_look_up_microns(__u32 product_id)
 static int hi_res_scroll_enable(struct hidpp_device *hidpp)
 {
 	int ret;
-	u8 multiplier;
+	u8 multiplier = 8;
 
 	if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2121) {
 		ret = hidpp_hrw_set_wheel_mode(hidpp, false, true, false);
@@ -2704,10 +2703,9 @@ static int hi_res_scroll_enable(struct hidpp_device *hidpp)
 	} else if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2120) {
 		ret = hidpp_hrs_set_highres_scrolling_mode(hidpp, true,
 							   &multiplier);
-	} else /* if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_1P0) */ {
+	} else /* if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_1P0) */
 		ret = hidpp10_enable_scrolling_acceleration(hidpp);
-		multiplier = 8;
-	}
+
 	if (ret)
 		return ret;
 
-- 
1.7.12.4

^ permalink raw reply related

* Re: [PATCH] HID: logitech: fix a used uninitialized GCC warning
From: zhong jiang @ 2018-09-13  7:39 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Jiri Kosina, HID CORE LAYER, lkml
In-Reply-To: <CAO-hwJJAGdUokt4rByNFsit1bY8t3ZpV3CFh-ES7-GNukZTBLw@mail.gmail.com>

On 2018/9/13 15:01, Benjamin Tissoires wrote:
> On Thu, Sep 13, 2018 at 6:04 AM zhong jiang <zhongjiang@huawei.com> wrote:
>> Fix the following compile warning:
>>
>> drivers/hid/hid-logitech-hidpp.c: In function ‘hi_res_scroll_enable’:
>> drivers/hid/hid-logitech-hidpp.c:2714:54: warning: ‘multiplier’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>>   hidpp->vertical_wheel_counter.resolution_multiplier = multiplier;
>>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> ---
>>  drivers/hid/hid-logitech-hidpp.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
>> index 5f0c080..83c43dd 100644
>> --- a/drivers/hid/hid-logitech-hidpp.c
>> +++ b/drivers/hid/hid-logitech-hidpp.c
>> @@ -2696,7 +2696,7 @@ static int hi_res_scroll_look_up_microns(__u32 product_id)
>>  static int hi_res_scroll_enable(struct hidpp_device *hidpp)
>>  {
>>         int ret;
>> -       u8 multiplier;
>> +       u8 uninitialized_var(multiplier);
> I think your patch is correct (multiplier will be set given the code
> path), but IMO, it feels terribly wrong to explicitly remove this
> warning this way. The problem is that if someone else adds a new piece
> of code, we might miss the fact that multiplier is not set and we
> might show garbage in the hid_info call.
Make sense.
>
> Why don't you initialize the value to 8 as in the 'else' statement and
> remove the now duplicated assignement in this else statement?
Will do in v2.  Thank you for your suggestion.

Thanks,
zhong jiang
> Cheers,
> Benjamin
>
>>         if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2121) {
>>                 ret = hidpp_hrw_set_wheel_mode(hidpp, false, true, false);
>> --
>> 1.7.12.4
>>
> .
>

^ permalink raw reply

* Re: [PATCH] HID: logitech: fix a used uninitialized GCC warning
From: Benjamin Tissoires @ 2018-09-13  7:01 UTC (permalink / raw)
  To: zhongjiang; +Cc: Jiri Kosina, open list:HID CORE LAYER, lkml
In-Reply-To: <1536810739-27342-1-git-send-email-zhongjiang@huawei.com>

On Thu, Sep 13, 2018 at 6:04 AM zhong jiang <zhongjiang@huawei.com> wrote:
>
> Fix the following compile warning:
>
> drivers/hid/hid-logitech-hidpp.c: In function ‘hi_res_scroll_enable’:
> drivers/hid/hid-logitech-hidpp.c:2714:54: warning: ‘multiplier’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>   hidpp->vertical_wheel_counter.resolution_multiplier = multiplier;
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
>  drivers/hid/hid-logitech-hidpp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 5f0c080..83c43dd 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -2696,7 +2696,7 @@ static int hi_res_scroll_look_up_microns(__u32 product_id)
>  static int hi_res_scroll_enable(struct hidpp_device *hidpp)
>  {
>         int ret;
> -       u8 multiplier;
> +       u8 uninitialized_var(multiplier);

I think your patch is correct (multiplier will be set given the code
path), but IMO, it feels terribly wrong to explicitly remove this
warning this way. The problem is that if someone else adds a new piece
of code, we might miss the fact that multiplier is not set and we
might show garbage in the hid_info call.

Why don't you initialize the value to 8 as in the 'else' statement and
remove the now duplicated assignement in this else statement?

Cheers,
Benjamin

>
>         if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2121) {
>                 ret = hidpp_hrw_set_wheel_mode(hidpp, false, true, false);
> --
> 1.7.12.4
>

^ permalink raw reply

* Re: [PATCH] hyper-v: Fix wakeup from suspend-to-idle
From: Rafael J. Wysocki @ 2018-09-13  6:55 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Linux PM, Rafael J. Wysocki, kys, haiyangz, sthemmin, Jiri Kosina,
	Dmitry Torokhov, linux-input, Linux Kernel Mailing List
In-Reply-To: <20180912161101.2634-1-vkuznets@redhat.com>

On Wed, Sep 12, 2018 at 6:11 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> It makes little sense but still possible to put Hyper-V guests into
> suspend-to-idle state. To wake them up two wakeup sources were registered
> in the past: hyperv-keyboard and hid-hyperv. However, since
> commit eed4d47efe95 ("ACPI / sleep: Ignore spurious SCI wakeups from
> suspend-to-idle") pm_wakeup_event() from these devices is ignored. Switch
> to pm_wakeup_hard_event() API as these devices are actually the only
> possible way to wakeup Hyper-V guests.
>
> Fixes: eed4d47efe95 (ACPI / sleep: Ignore spurious SCI wakeups from suspend-to-idle)
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

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

> ---
>  drivers/hid/hid-hyperv.c              | 2 +-
>  drivers/input/serio/hyperv-keyboard.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> index b372854cf38d..704049e62d58 100644
> --- a/drivers/hid/hid-hyperv.c
> +++ b/drivers/hid/hid-hyperv.c
> @@ -309,7 +309,7 @@ static void mousevsc_on_receive(struct hv_device *device,
>                 hid_input_report(input_dev->hid_device, HID_INPUT_REPORT,
>                                  input_dev->input_buf, len, 1);
>
> -               pm_wakeup_event(&input_dev->device->device, 0);
> +               pm_wakeup_hard_event(&input_dev->device->device);
>
>                 break;
>         default:
> diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c
> index 47a0e81a2989..a8b9be3e28db 100644
> --- a/drivers/input/serio/hyperv-keyboard.c
> +++ b/drivers/input/serio/hyperv-keyboard.c
> @@ -177,7 +177,7 @@ static void hv_kbd_on_receive(struct hv_device *hv_dev,
>                  * state because the Enter-UP can trigger a wakeup at once.
>                  */
>                 if (!(info & IS_BREAK))
> -                       pm_wakeup_event(&hv_dev->device, 0);
> +                       pm_wakeup_hard_event(&hv_dev->device);
>
>                 break;
>
> --
> 2.14.4
>

^ permalink raw reply

* [PATCH] HID: logitech: fix a used uninitialized GCC warning
From: zhong jiang @ 2018-09-13  3:52 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel

Fix the following compile warning:

drivers/hid/hid-logitech-hidpp.c: In function ‘hi_res_scroll_enable’:
drivers/hid/hid-logitech-hidpp.c:2714:54: warning: ‘multiplier’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  hidpp->vertical_wheel_counter.resolution_multiplier = multiplier;

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/hid/hid-logitech-hidpp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 5f0c080..83c43dd 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -2696,7 +2696,7 @@ static int hi_res_scroll_look_up_microns(__u32 product_id)
 static int hi_res_scroll_enable(struct hidpp_device *hidpp)
 {
 	int ret;
-	u8 multiplier;
+	u8 uninitialized_var(multiplier);
 
 	if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2121) {
 		ret = hidpp_hrw_set_wheel_mode(hidpp, false, true, false);
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH V2] hid: hid-core: Fix a sleep-in-atomic-context bug in __hid_request()
From: Jia-Ju Bai @ 2018-09-13  3:34 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Jia-Ju Bai

hid_alloc_report_buf() has to be called with GFP_ATOMIC in 
__hid_request(), because there are the following callchains 
leading to __hid_request() being an atomic context:

picolcd_send_and_wait (acquire a spinlock)
  hid_hw_request
    __hid_request
      hid_alloc_report_buf(GFP_KERNEL)

picolcd_reset (acquire a spinlock)
  hid_hw_request
    __hid_request
      hid_alloc_report_buf(GFP_KERNEL)

lg4ff_play (acquire a spinlock)
  hid_hw_request
    __hid_request
      hid_alloc_report_buf(GFP_KERNEL)

lg4ff_set_autocenter_ffex (acquire a spinlock)
  hid_hw_request
    __hid_request
      hid_alloc_report_buf(GFP_KERNEL)

This bug is found by my static analysis tool DSAC.

Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
v2:
* Make the description more human readable.
  Thanks Jiri for good advice.
---
 drivers/hid/hid-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 3942ee61bd1c..c886af00c8c9 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1442,7 +1442,7 @@ void __hid_request(struct hid_device *hid, struct hid_report *report,
 	int ret;
 	u32 len;
 
-	buf = hid_alloc_report_buf(report, GFP_KERNEL);
+	buf = hid_alloc_report_buf(report, GFP_ATOMIC);
 	if (!buf)
 		return;
 
-- 
2.17.0

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox