Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH v3 2/3] HID: quirks: Fix keyboard + touchpad on Lenovo Miix 630 for DT
From: Jeffrey Hugo @ 2019-04-18 14:43 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Benjamin Tissoires, Rob Herring, Mark Rutland, Jiri Kosina,
	Lee Jones, Bjorn Andersson, Dmitry Torokhov, agross, David Brown,
	open list:HID CORE LAYER, devicetree, linux-arm-msm, lkml
In-Reply-To: <a47672ed-fc25-dd07-0ddf-2deca08283ef@redhat.com>

On Thu, Apr 18, 2019 at 3:51 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 18-04-19 11:34, Benjamin Tissoires wrote:
> > On Mon, Apr 15, 2019 at 6:11 PM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
> >>
> >> Following up on commit 2bafa1e96254 ("HID: quirks: Fix keyboard + touchpad
> >> on Lenovo Miix 630"), the devicetree (DT) identifier for the combo keyboard
> >> + touchpad device is "elan,combo400-i2c", which differs from the ACPI ID,
> >> thus if we want the quirk to work properly when booting via DT instead of
> >> ACPI, we need to key off the DT id as well.
> >>
> >> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> >> ---
> >>   drivers/hid/hid-quirks.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> >> index 77ffba48cc73..00c08f8318b8 100644
> >> --- a/drivers/hid/hid-quirks.c
> >> +++ b/drivers/hid/hid-quirks.c
> >> @@ -997,7 +997,8 @@ bool hid_ignore(struct hid_device *hdev)
> >>                          return true;
> >>                  /* Same with product id 0x0400 */
> >>                  if (hdev->product == 0x0400 &&
> >> -                   strncmp(hdev->name, "QTEC0001", 8) != 0)
> >> +                   (strncmp(hdev->name, "QTEC0001", 8) != 0 ||
> >> +                    strncmp(hdev->name, "elan,combo400-i2c", 17) != 0))
> >
> > I think we are taking the problem the wrong way here.
> >
> > When I first introduced 6ccfe64, I thought 0x0400 would be reserved
> > for the elan_i2c touchpads only. But it turns out we are deliberately
> > disabling valid HID touchpads hoping that they would be picked up by
> > elan_i2c when elan_i2c has its own whitelist of devices.
> >
> > How about we turn this into list with the matching ones from elan_i2c:
> > if ((hdev->product == 0x0400 || hdev->product == 0x0401) &&
> >     (strncmp(hdev->name, "ELAN0000", 8) == 0 ||
> >      strncmp(hdev->name, "ELAN0100", 8) == 0 ||
> >      ...
> >      strncmp(hdev->name, "ELAN1000", 8) == 0))
> >        return true;
> >
> > So next time we need to force binding a HID touchpad to elan_i2c, we
> > can just blacklist here and whitelist it in elan_i2c.
>
> This indeed sounds like a better way forward with this.

This sounds good to me.  Let me implement it and test with the Miix
630.  Thanks for the suggestion.

^ permalink raw reply

* Re: [PATCH v3 09/26] compat_ioctl: move drivers to compat_ptr_ioctl
From: Stefan Hajnoczi @ 2019-04-18 11:10 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Alexander Viro, linux-fsdevel, y2038, linux-kernel,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jason Gunthorpe,
	Sudip Mukherjee, Peter Huewe, Jason Gunthorpe, Stefan Richter,
	Jiri Kosina, Benjamin Tissoires, Alexander Shishkin,
	Maxime Coquelin, Alexandre Torgue, Tomas Winkler,
	Artem Bityutskiy, Richard Weinberger
In-Reply-To: <20190416202013.4034148-10-arnd@arndb.de>

[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]

On Tue, Apr 16, 2019 at 10:19:47PM +0200, Arnd Bergmann wrote:
> Each of these drivers has a copy of the same trivial helper function to
> convert the pointer argument and then call the native ioctl handler.
> 
> We now have a generic implementation of that, so use it.
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/char/ppdev.c              | 12 +---------
>  drivers/char/tpm/tpm_vtpm_proxy.c | 12 +---------
>  drivers/firewire/core-cdev.c      | 12 +---------
>  drivers/hid/usbhid/hiddev.c       | 11 +--------
>  drivers/hwtracing/stm/core.c      | 12 +---------
>  drivers/misc/mei/main.c           | 22 +----------------
>  drivers/mtd/ubi/cdev.c            | 36 +++-------------------------
>  drivers/net/tap.c                 | 12 +---------
>  drivers/staging/pi433/pi433_if.c  | 12 +---------
>  drivers/usb/core/devio.c          | 16 +------------
>  drivers/vfio/vfio.c               | 39 +++----------------------------
>  drivers/vhost/net.c               | 12 +---------
>  drivers/vhost/scsi.c              | 12 +---------
>  drivers/vhost/test.c              | 12 +---------
>  drivers/vhost/vsock.c             | 12 +---------
>  fs/fat/file.c                     | 13 +----------
>  16 files changed, 20 insertions(+), 237 deletions(-)

The vhost parts look good:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply

* Re: [PATCH v3 2/3] HID: quirks: Fix keyboard + touchpad on Lenovo Miix 630 for DT
From: Hans de Goede @ 2019-04-18  9:51 UTC (permalink / raw)
  To: Benjamin Tissoires, Jeffrey Hugo
  Cc: Rob Herring, mark.rutland, Jiri Kosina, Lee Jones,
	bjorn.andersson, Dmitry Torokhov, agross, david.brown,
	open list:HID CORE LAYER, devicetree, linux-arm-msm, lkml
In-Reply-To: <CAO-hwJKeACBPW52ej6mVeG4aVDYvGF8zEMtegonVwFjT-ZoLOQ@mail.gmail.com>

Hi,

On 18-04-19 11:34, Benjamin Tissoires wrote:
> On Mon, Apr 15, 2019 at 6:11 PM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
>>
>> Following up on commit 2bafa1e96254 ("HID: quirks: Fix keyboard + touchpad
>> on Lenovo Miix 630"), the devicetree (DT) identifier for the combo keyboard
>> + touchpad device is "elan,combo400-i2c", which differs from the ACPI ID,
>> thus if we want the quirk to work properly when booting via DT instead of
>> ACPI, we need to key off the DT id as well.
>>
>> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
>> ---
>>   drivers/hid/hid-quirks.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
>> index 77ffba48cc73..00c08f8318b8 100644
>> --- a/drivers/hid/hid-quirks.c
>> +++ b/drivers/hid/hid-quirks.c
>> @@ -997,7 +997,8 @@ bool hid_ignore(struct hid_device *hdev)
>>                          return true;
>>                  /* Same with product id 0x0400 */
>>                  if (hdev->product == 0x0400 &&
>> -                   strncmp(hdev->name, "QTEC0001", 8) != 0)
>> +                   (strncmp(hdev->name, "QTEC0001", 8) != 0 ||
>> +                    strncmp(hdev->name, "elan,combo400-i2c", 17) != 0))
> 
> I think we are taking the problem the wrong way here.
> 
> When I first introduced 6ccfe64, I thought 0x0400 would be reserved
> for the elan_i2c touchpads only. But it turns out we are deliberately
> disabling valid HID touchpads hoping that they would be picked up by
> elan_i2c when elan_i2c has its own whitelist of devices.
> 
> How about we turn this into list with the matching ones from elan_i2c:
> if ((hdev->product == 0x0400 || hdev->product == 0x0401) &&
>     (strncmp(hdev->name, "ELAN0000", 8) == 0 ||
>      strncmp(hdev->name, "ELAN0100", 8) == 0 ||
>      ...
>      strncmp(hdev->name, "ELAN1000", 8) == 0))
>        return true;
> 
> So next time we need to force binding a HID touchpad to elan_i2c, we
> can just blacklist here and whitelist it in elan_i2c.

This indeed sounds like a better way forward with this.

Regards,

Hans

^ permalink raw reply

* Re: [PATCH v3 1/3] dt-bindings: input: add Elan 400 combo keyboard/touchpad over i2c
From: Benjamin Tissoires @ 2019-04-18  9:35 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: Rob Herring, mark.rutland, Lee Jones, bjorn.andersson,
	Dmitry Torokhov, agross, David Brown, Jiri Kosina,
	open list:HID CORE LAYER, devicetree, linux-arm-msm, lkml
In-Reply-To: <20190415161055.16372-1-jeffrey.l.hugo@gmail.com>

On Mon, Apr 15, 2019 at 6:11 PM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
>
> The Elan 400 combo keyboard/touchpad over i2c device is a distinct device
> from the Elan 400 standalone touchpad device.  The combo device has been
> found in the Lenovo Miix 630 and HP Envy x2 laptops.
>
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---

With my comments in 2/3, I wonder if you need this patch at all then.

Cheers,
Benjamin

>  .../devicetree/bindings/input/elan,combo400-i2c.txt   | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
>
> diff --git a/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt b/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
> new file mode 100644
> index 000000000000..fb700a29148d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
> @@ -0,0 +1,11 @@
> +Elantech 0400 I2C combination Keyboard/Touchpad
> +
> +This binding describes an Elan device with pid 0x0400, that is a combination
> +keyboard + touchpad device.  This binding does not cover an Elan device with
> +pid 0x0400 that is solely a standalone touchpad device.
> +
> +Required properties:
> +- compatible: should be "elan,combo400-i2c"
> +
> +This binding is compatible with the HID over I2C binding, which is specified
> +in hid-over-i2c.txt in this directory.
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH v3 2/3] HID: quirks: Fix keyboard + touchpad on Lenovo Miix 630 for DT
From: Benjamin Tissoires @ 2019-04-18  9:34 UTC (permalink / raw)
  To: Jeffrey Hugo, Hans de Goede
  Cc: Rob Herring, mark.rutland, Jiri Kosina, Lee Jones,
	bjorn.andersson, Dmitry Torokhov, agross, david.brown,
	open list:HID CORE LAYER, devicetree, linux-arm-msm, lkml
In-Reply-To: <20190415161108.16419-1-jeffrey.l.hugo@gmail.com>

On Mon, Apr 15, 2019 at 6:11 PM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
>
> Following up on commit 2bafa1e96254 ("HID: quirks: Fix keyboard + touchpad
> on Lenovo Miix 630"), the devicetree (DT) identifier for the combo keyboard
> + touchpad device is "elan,combo400-i2c", which differs from the ACPI ID,
> thus if we want the quirk to work properly when booting via DT instead of
> ACPI, we need to key off the DT id as well.
>
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---
>  drivers/hid/hid-quirks.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 77ffba48cc73..00c08f8318b8 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -997,7 +997,8 @@ bool hid_ignore(struct hid_device *hdev)
>                         return true;
>                 /* Same with product id 0x0400 */
>                 if (hdev->product == 0x0400 &&
> -                   strncmp(hdev->name, "QTEC0001", 8) != 0)
> +                   (strncmp(hdev->name, "QTEC0001", 8) != 0 ||
> +                    strncmp(hdev->name, "elan,combo400-i2c", 17) != 0))

I think we are taking the problem the wrong way here.

When I first introduced 6ccfe64, I thought 0x0400 would be reserved
for the elan_i2c touchpads only. But it turns out we are deliberately
disabling valid HID touchpads hoping that they would be picked up by
elan_i2c when elan_i2c has its own whitelist of devices.

How about we turn this into list with the matching ones from elan_i2c:
if ((hdev->product == 0x0400 || hdev->product == 0x0401) &&
   (strncmp(hdev->name, "ELAN0000", 8) == 0 ||
    strncmp(hdev->name, "ELAN0100", 8) == 0 ||
    ...
    strncmp(hdev->name, "ELAN1000", 8) == 0))
      return true;

So next time we need to force binding a HID touchpad to elan_i2c, we
can just blacklist here and whitelist it in elan_i2c.

Cheers,
Benjamin

>                         return true;
>                 break;
>         }
> --
> 2.17.1
>

^ permalink raw reply

* Input: synaptics-rmi4: fix possible double free
From: Pan Bian @ 2019-04-18  1:58 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Wei Yongjun, Nick Dyer, Nick Desaulniers, Kees Cook, linux-input,
	linux-kernel, Pan Bian

The RMI4 function structure has been released in rmi_register_function
if error occurs. However, it will be released again in the function
rmi_create_function, which may result in a double-free bug.

Signed-off-by: Pan Bian <bianpan2016@163.com>
---
 drivers/input/rmi4/rmi_driver.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index fc3ab93..7fb358f 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -860,7 +860,7 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
 
 	error = rmi_register_function(fn);
 	if (error)
-		goto err_put_fn;
+		return error;
 
 	if (pdt->function_number == 0x01)
 		data->f01_container = fn;
@@ -870,10 +870,6 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
 	list_add_tail(&fn->node, &data->function_list);
 
 	return RMI_SCAN_CONTINUE;
-
-err_put_fn:
-	put_device(&fn->dev);
-	return error;
 }
 
 void rmi_enable_irq(struct rmi_device *rmi_dev, bool clear_wake)
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH v2] Input: uinput: Avoid Object-Already-Free with a global lock
From: dmitry.torokhov @ 2019-04-18  1:43 UTC (permalink / raw)
  To: Mukesh Ojha
  Cc: linux-input, linux-kernel, Gaurav Kohli, Peter Hutterer,
	Martin Kepplinger, Paul E. McKenney
In-Reply-To: <7299a6db-38b7-75c7-633a-00d2257eba45@codeaurora.org>

Hi Mukesh,

On Mon, Apr 15, 2019 at 03:35:51PM +0530, Mukesh Ojha wrote:
> 
> Hi Dmitry,
> 
> Can you please have a look at this patch ? as this seems to reproducing
> quite frequently
> 
> Thanks,
> Mukesh
> 
> On 4/10/2019 1:29 PM, Mukesh Ojha wrote:
> > uinput_destroy_device() gets called from two places. In one place,
> > uinput_ioctl_handler() where it is protected under a lock
> > udev->mutex but there is no protection on udev device from freeing
> > inside uinput_release().

uinput_release() should be called when last file handle to the uinput
instance is being dropped, so there should be no other users and thus we
can't be racing with anyone.

> > 
> > This can result in Object-Already-Free case where uinput parent
> > device already got freed while a child being inserted inside it.
> > That result in a double free case for parent while kernfs_put()
> > being done for child in a failure path of adding a node.

Can you please describe scenario in more detail? How do you free the
parent device while child input device is being registered?

Thanks.

- 
Dmitry

^ permalink raw reply

* Re: [PATCH v3 1/3] dt-bindings: input: add GPIO controllable vibrator
From: Rob Herring @ 2019-04-18  1:35 UTC (permalink / raw)
  To: Luca Weiss
  Cc: Dmitry Torokhov, Rob Herring, Mark Rutland, Mauro Carvalho Chehab,
	Pascal PAILLET-LME, Coly Li, Lee Jones, Xiaotong Lu, Brian Masney,
	Rob Herring, Baolin Wang, David Brown,
	open list:ARM/QUALCOMM SUPPORT,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
In-Reply-To: <15255825.75VNnaJK3T@g550jk>

On Wed, Apr 17, 2019 at 11:02 AM Luca Weiss <luca@z3ntu.xyz> wrote:
>
> On Freitag, 12. April 2019 17:06:23 CEST Luca Weiss wrote:
> > Provide a simple driver for GPIO controllable vibrators.
> > It will be used by the Fairphone 2.
> >
> > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > ---
> >  .../bindings/input/gpio-vibrator.txt          | 20 +++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/input/gpio-vibrator.txt
> >
> > diff --git a/Documentation/devicetree/bindings/input/gpio-vibrator.txt
> > b/Documentation/devicetree/bindings/input/gpio-vibrator.txt new file mode
> > 100644
> > index 000000000000..93e5a8e7622d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/gpio-vibrator.txt
> > @@ -0,0 +1,20 @@
> > +* GPIO vibrator device tree bindings
> > +
> > +Registers a GPIO device as vibrator, where the vibration motor just has the
> > +capability to turn on or off. If the device is connected to a pwm, you
> > should +use the pwm-vibrator driver instead.
> > +
> > +Required properties:
> > +- compatible: should contain "gpio-vibrator"
> > +- enable-gpios: Should contain a GPIO handle
> > +
> > +Optional properties:
> > +- vcc-supply: Phandle for the regulator supplying power
> > +
> > +Example from Fairphone 2:
> > +
> > +vibrator {
> > +     compatible = "gpio-vibrator";
> > +     enable-gpios = <&msmgpio 86 GPIO_ACTIVE_HIGH>;
> > +     vcc-supply = <&pm8941_l18>;
> > +};
>
> I see that the yaml based device tree binding docs seem to be the new hotness?
> Is there any "policy" / preference about new drivers?

Not required yet, but welcomed. It's still a trickle so we can work
out any issues and in some cases the common bindings still need to be
done. I'm starting to ask subsystem maintainers to require DT schemas
though.

This one looks straightforward to use the schema.

Rob

^ permalink raw reply

* Re: [PATCH v9 07/11] power: supply: max77650: add support for battery charger
From: Sebastian Reichel @ 2019-04-17 21:05 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
	Jacek Anaszewski, Pavel Machek, Lee Jones, Liam Girdwood,
	Greg Kroah-Hartman, linux-kernel, linux-gpio, devicetree,
	linux-input, linux-leds, linux-pm, Bartosz Golaszewski
In-Reply-To: <20190411124212.7612-8-brgl@bgdev.pl>

[-- Attachment #1: Type: text/plain, Size: 13584 bytes --]

Hi,

On Thu, Apr 11, 2019 at 02:42:08PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Add basic support for the battery charger for max77650 PMIC.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/power/supply/Kconfig            |   7 +
>  drivers/power/supply/Makefile           |   1 +
>  drivers/power/supply/max77650-charger.c | 368 ++++++++++++++++++++++++
>  3 files changed, 376 insertions(+)
>  create mode 100644 drivers/power/supply/max77650-charger.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index e901b9879e7e..0230c96fa94d 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -499,6 +499,13 @@ config CHARGER_DETECTOR_MAX14656
>  	  Revision 1.2 and can be found e.g. in Kindle 4/5th generation
>  	  readers and certain LG devices.
>  
> +config CHARGER_MAX77650
> +	tristate "Maxim MAX77650 battery charger driver"
> +	depends on MFD_MAX77650
> +	help
> +	  Say Y to enable support for the battery charger control of MAX77650
> +	  PMICs.
> +
>  config CHARGER_MAX77693
>  	tristate "Maxim MAX77693 battery charger driver"
>  	depends on MFD_MAX77693
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index b731c2a9b695..b73eb8c5c1a9 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -70,6 +70,7 @@ obj-$(CONFIG_CHARGER_MANAGER)	+= charger-manager.o
>  obj-$(CONFIG_CHARGER_LTC3651)	+= ltc3651-charger.o
>  obj-$(CONFIG_CHARGER_MAX14577)	+= max14577_charger.o
>  obj-$(CONFIG_CHARGER_DETECTOR_MAX14656)	+= max14656_charger_detector.o
> +obj-$(CONFIG_CHARGER_MAX77650)	+= max77650-charger.o
>  obj-$(CONFIG_CHARGER_MAX77693)	+= max77693_charger.o
>  obj-$(CONFIG_CHARGER_MAX8997)	+= max8997_charger.o
>  obj-$(CONFIG_CHARGER_MAX8998)	+= max8998_charger.o
> diff --git a/drivers/power/supply/max77650-charger.c b/drivers/power/supply/max77650-charger.c
> new file mode 100644
> index 000000000000..e34714cb05ec
> --- /dev/null
> +++ b/drivers/power/supply/max77650-charger.c
> @@ -0,0 +1,368 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (C) 2018 BayLibre SAS
> +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> +//
> +// Battery charger driver for MAXIM 77650/77651 charger/power-supply.
> +
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/max77650.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +
> +#define MAX77650_CHARGER_ENABLED		BIT(0)
> +#define MAX77650_CHARGER_DISABLED		0x00
> +#define MAX77650_CHARGER_CHG_EN_MASK		BIT(0)
> +
> +#define MAX77650_CHG_DETAILS_MASK		GENMASK(7, 4)
> +#define MAX77650_CHG_DETAILS_BITS(_reg) \
> +		(((_reg) & MAX77650_CHG_DETAILS_MASK) >> 4)
> +
> +/* Charger is OFF. */
> +#define MAX77650_CHG_OFF			0x00
> +/* Charger is in prequalification mode. */
> +#define MAX77650_CHG_PREQ			0x01
> +/* Charger is in fast-charge constant current mode. */
> +#define MAX77650_CHG_ON_CURR			0x02
> +/* Charger is in JEITA modified fast-charge constant-current mode. */
> +#define MAX77650_CHG_ON_CURR_JEITA		0x03
> +/* Charger is in fast-charge constant-voltage mode. */
> +#define MAX77650_CHG_ON_VOLT			0x04
> +/* Charger is in JEITA modified fast-charge constant-voltage mode. */
> +#define MAX77650_CHG_ON_VOLT_JEITA		0x05
> +/* Charger is in top-off mode. */
> +#define MAX77650_CHG_ON_TOPOFF			0x06
> +/* Charger is in JEITA modified top-off mode. */
> +#define MAX77650_CHG_ON_TOPOFF_JEITA		0x07
> +/* Charger is done. */
> +#define MAX77650_CHG_DONE			0x08
> +/* Charger is JEITA modified done. */
> +#define MAX77650_CHG_DONE_JEITA			0x09
> +/* Charger is suspended due to a prequalification timer fault. */
> +#define MAX77650_CHG_SUSP_PREQ_TIM_FAULT	0x0a
> +/* Charger is suspended due to a fast-charge timer fault. */
> +#define MAX77650_CHG_SUSP_FAST_CHG_TIM_FAULT	0x0b
> +/* Charger is suspended due to a battery temperature fault. */
> +#define MAX77650_CHG_SUSP_BATT_TEMP_FAULT	0x0c
> +
> +#define MAX77650_CHGIN_DETAILS_MASK		GENMASK(3, 2)
> +#define MAX77650_CHGIN_DETAILS_BITS(_reg) \
> +		(((_reg) & MAX77650_CHGIN_DETAILS_MASK) >> 2)
> +
> +#define MAX77650_CHGIN_UNDERVOLTAGE_LOCKOUT	0x00
> +#define MAX77650_CHGIN_OVERVOLTAGE_LOCKOUT	0x01
> +#define MAX77650_CHGIN_OKAY			0x11
> +
> +#define MAX77650_CHARGER_CHG_MASK	BIT(1)
> +#define MAX77650_CHARGER_CHG_CHARGING(_reg) \
> +		(((_reg) & MAX77650_CHARGER_CHG_MASK) > 1)
> +
> +#define MAX77650_CHARGER_VCHGIN_MIN_MASK	0xc0
> +#define MAX77650_CHARGER_VCHGIN_MIN_SHIFT(_val)	((_val) << 5)
> +
> +#define MAX77650_CHARGER_ICHGIN_LIM_MASK	0x1c
> +#define MAX77650_CHARGER_ICHGIN_LIM_SHIFT(_val)	((_val) << 2)
> +
> +struct max77650_charger_data {
> +	struct regmap *map;
> +	struct device *dev;
> +};
> +
> +static enum power_supply_property max77650_charger_properties[] = {
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_ONLINE,
> +	POWER_SUPPLY_PROP_CHARGE_TYPE
> +};
> +
> +static const unsigned int max77650_charger_vchgin_min_table[] = {
> +	4000000, 4100000, 4200000, 4300000, 4400000, 4500000, 4600000, 4700000
> +};
> +
> +static const unsigned int max77650_charger_ichgin_lim_table[] = {
> +	95000, 190000, 285000, 380000, 475000
> +};
> +
> +static int max77650_charger_set_vchgin_min(struct max77650_charger_data *chg,
> +					   unsigned int val)
> +{
> +	int i, rv;
> +
> +	for (i = 0; i < ARRAY_SIZE(max77650_charger_vchgin_min_table); i++) {
> +		if (val == max77650_charger_vchgin_min_table[i]) {
> +			rv = regmap_update_bits(chg->map,
> +					MAX77650_REG_CNFG_CHG_B,
> +					MAX77650_CHARGER_VCHGIN_MIN_MASK,
> +					MAX77650_CHARGER_VCHGIN_MIN_SHIFT(i));
> +			if (rv)
> +				return rv;
> +
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int max77650_charger_set_ichgin_lim(struct max77650_charger_data *chg,
> +					   unsigned int val)
> +{
> +	int i, rv;
> +
> +	for (i = 0; i < ARRAY_SIZE(max77650_charger_ichgin_lim_table); i++) {
> +		if (val == max77650_charger_ichgin_lim_table[i]) {
> +			rv = regmap_update_bits(chg->map,
> +					MAX77650_REG_CNFG_CHG_B,
> +					MAX77650_CHARGER_ICHGIN_LIM_MASK,
> +					MAX77650_CHARGER_ICHGIN_LIM_SHIFT(i));
> +			if (rv)
> +				return rv;
> +
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int max77650_charger_enable(struct max77650_charger_data *chg)
> +{
> +	int rv;
> +
> +	rv = regmap_update_bits(chg->map,
> +				MAX77650_REG_CNFG_CHG_B,
> +				MAX77650_CHARGER_CHG_EN_MASK,
> +				MAX77650_CHARGER_ENABLED);
> +	if (rv)
> +		dev_err(chg->dev, "unable to enable the charger: %d\n", rv);
> +
> +	return rv;
> +}
> +
> +static int max77650_charger_disable(struct max77650_charger_data *chg)
> +{
> +	int rv;
> +
> +	rv = regmap_update_bits(chg->map,
> +				MAX77650_REG_CNFG_CHG_B,
> +				MAX77650_CHARGER_CHG_EN_MASK,
> +				MAX77650_CHARGER_DISABLED);
> +	if (rv)
> +		dev_err(chg->dev, "unable to disable the charger: %d\n", rv);
> +
> +	return rv;
> +}
> +
> +static irqreturn_t max77650_charger_check_status(int irq, void *data)
> +{
> +	struct max77650_charger_data *chg = data;
> +	int rv, reg;
> +
> +	rv = regmap_read(chg->map, MAX77650_REG_STAT_CHG_B, &reg);
> +	if (rv) {
> +		dev_err(chg->dev,
> +			"unable to read the charger status: %d\n", rv);
> +		return IRQ_HANDLED;
> +	}
> +
> +	switch (MAX77650_CHGIN_DETAILS_BITS(reg)) {
> +	case MAX77650_CHGIN_UNDERVOLTAGE_LOCKOUT:
> +		dev_err(chg->dev, "undervoltage lockout detected, disabling charger\n");
> +		max77650_charger_disable(chg);
> +		break;
> +	case MAX77650_CHGIN_OVERVOLTAGE_LOCKOUT:
> +		dev_err(chg->dev, "overvoltage lockout detected, disabling charger\n");
> +		max77650_charger_disable(chg);
> +		break;
> +	case MAX77650_CHGIN_OKAY:
> +		max77650_charger_enable(chg);
> +		break;
> +	default:
> +		/* May be 0x10 - debouncing */
> +		break;
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int max77650_charger_get_property(struct power_supply *psy,
> +					 enum power_supply_property psp,
> +					 union power_supply_propval *val)
> +{
> +	struct max77650_charger_data *chg = power_supply_get_drvdata(psy);
> +	int rv, reg;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_STATUS:
> +		rv = regmap_read(chg->map, MAX77650_REG_STAT_CHG_B, &reg);
> +		if (rv)
> +			return rv;
> +
> +		if (MAX77650_CHARGER_CHG_CHARGING(reg)) {
> +			val->intval = POWER_SUPPLY_STATUS_CHARGING;
> +			break;
> +		}
> +
> +		switch (MAX77650_CHG_DETAILS_BITS(reg)) {
> +		case MAX77650_CHG_OFF:
> +		case MAX77650_CHG_SUSP_PREQ_TIM_FAULT:
> +		case MAX77650_CHG_SUSP_FAST_CHG_TIM_FAULT:
> +		case MAX77650_CHG_SUSP_BATT_TEMP_FAULT:
> +			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
> +			break;
> +		case MAX77650_CHG_PREQ:
> +		case MAX77650_CHG_ON_CURR:
> +		case MAX77650_CHG_ON_CURR_JEITA:
> +		case MAX77650_CHG_ON_VOLT:
> +		case MAX77650_CHG_ON_VOLT_JEITA:
> +		case MAX77650_CHG_ON_TOPOFF:
> +		case MAX77650_CHG_ON_TOPOFF_JEITA:
> +			val->intval = POWER_SUPPLY_STATUS_CHARGING;
> +			break;
> +		case MAX77650_CHG_DONE:
> +			val->intval = POWER_SUPPLY_STATUS_FULL;
> +			break;
> +		default:
> +			val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
> +		}
> +		break;
> +	case POWER_SUPPLY_PROP_ONLINE:
> +		rv = regmap_read(chg->map, MAX77650_REG_STAT_CHG_B, &reg);
> +		if (rv)
> +			return rv;
> +
> +		val->intval = MAX77650_CHARGER_CHG_CHARGING(reg);
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_TYPE:
> +		rv = regmap_read(chg->map, MAX77650_REG_STAT_CHG_B, &reg);
> +		if (rv)
> +			return rv;
> +
> +		if (!MAX77650_CHARGER_CHG_CHARGING(reg)) {
> +			val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
> +			break;
> +		}
> +
> +		switch (MAX77650_CHG_DETAILS_BITS(reg)) {
> +		case MAX77650_CHG_PREQ:
> +		case MAX77650_CHG_ON_CURR:
> +		case MAX77650_CHG_ON_CURR_JEITA:
> +		case MAX77650_CHG_ON_VOLT:
> +		case MAX77650_CHG_ON_VOLT_JEITA:
> +			val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
> +			break;
> +		case MAX77650_CHG_ON_TOPOFF:
> +		case MAX77650_CHG_ON_TOPOFF_JEITA:
> +			val->intval = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> +			break;
> +		default:
> +			val->intval = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct power_supply_desc max77650_battery_desc = {
> +	.name		= "max77650",
> +	.type		= POWER_SUPPLY_TYPE_USB,
> +	.get_property	= max77650_charger_get_property,
> +	.properties	= max77650_charger_properties,
> +	.num_properties	= ARRAY_SIZE(max77650_charger_properties),
> +};
> +
> +static int max77650_charger_probe(struct platform_device *pdev)
> +{
> +	struct power_supply_config pscfg = {};
> +	struct max77650_charger_data *chg;
> +	struct power_supply *battery;
> +	struct device *dev, *parent;
> +	int rv, chg_irq, chgin_irq;
> +	unsigned int prop;
> +
> +	dev = &pdev->dev;
> +	parent = dev->parent;
> +
> +	chg = devm_kzalloc(dev, sizeof(*chg), GFP_KERNEL);
> +	if (!chg)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, chg);
> +
> +	chg->map = dev_get_regmap(parent, NULL);
> +	if (!chg->map)
> +		return -ENODEV;
> +
> +	chg->dev = dev;
> +
> +	pscfg.of_node = dev->of_node;
> +	pscfg.drv_data = chg;
> +
> +	chg_irq = platform_get_irq_byname(pdev, "CHG");
> +	if (chg_irq < 0)
> +		return chg_irq;
> +
> +	chgin_irq = platform_get_irq_byname(pdev, "CHGIN");
> +	if (chgin_irq < 0)
> +		return chgin_irq;
> +
> +	rv = devm_request_any_context_irq(dev, chg_irq,
> +					  max77650_charger_check_status,
> +					  IRQF_ONESHOT, "chg", chg);
> +	if (rv < 0)
> +		return rv;
> +
> +	rv = devm_request_any_context_irq(dev, chgin_irq,
> +					  max77650_charger_check_status,
> +					  IRQF_ONESHOT, "chgin", chg);
> +	if (rv < 0)
> +		return rv;
> +
> +	battery = devm_power_supply_register(dev,
> +					     &max77650_battery_desc, &pscfg);
> +	if (IS_ERR(battery))
> +		return PTR_ERR(battery);
> +
> +	rv = of_property_read_u32(dev->of_node,
> +				  "input-voltage-min-microvolt", &prop);
> +	if (rv == 0) {
> +		rv = max77650_charger_set_vchgin_min(chg, prop);
> +		if (rv)
> +			return rv;
> +	}
> +
> +	rv = of_property_read_u32(dev->of_node,
> +				  "input-current-limit-microamp", &prop);
> +	if (rv == 0) {
> +		rv = max77650_charger_set_ichgin_lim(chg, prop);
> +		if (rv)
> +			return rv;
> +	}
> +
> +	return max77650_charger_enable(chg);
> +}
> +
> +static int max77650_charger_remove(struct platform_device *pdev)
> +{
> +	struct max77650_charger_data *chg = platform_get_drvdata(pdev);
> +
> +	return max77650_charger_disable(chg);
> +}
> +
> +static struct platform_driver max77650_charger_driver = {
> +	.driver = {
> +		.name = "max77650-charger",
> +	},
> +	.probe = max77650_charger_probe,
> +	.remove = max77650_charger_remove,
> +};
> +module_platform_driver(max77650_charger_driver);
> +
> +MODULE_DESCRIPTION("MAXIM 77650/77651 charger driver");
> +MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.21.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v9 02/11] dt-bindings: power: supply: add DT bindings for max77650
From: Sebastian Reichel @ 2019-04-17 20:30 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
	Jacek Anaszewski, Pavel Machek, Lee Jones, Liam Girdwood,
	Greg Kroah-Hartman, linux-kernel, linux-gpio, devicetree,
	linux-input, linux-leds, linux-pm, Bartosz Golaszewski
In-Reply-To: <20190411124212.7612-3-brgl@bgdev.pl>

[-- Attachment #1: Type: text/plain, Size: 1860 bytes --]

Hi,

On Thu, Apr 11, 2019 at 02:42:03PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Add the DT binding document for the battery charger module of max77650.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  .../power/supply/max77650-charger.txt         | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/max77650-charger.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/max77650-charger.txt b/Documentation/devicetree/bindings/power/supply/max77650-charger.txt
> new file mode 100644
> index 000000000000..821a2240e70f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/max77650-charger.txt
> @@ -0,0 +1,28 @@
> +Battery charger driver for MAX77650 PMIC from Maxim Integrated.
> +
> +This module is part of the MAX77650 MFD device. For more details
> +see Documentation/devicetree/bindings/mfd/max77650.txt.
> +
> +The charger is represented as a sub-node of the PMIC node on the device tree.
> +
> +Required properties:
> +--------------------
> +- compatible:		Must be "maxim,max77650-charger"
> +
> +Optional properties:
> +--------------------
> +- input-voltage-min-microvolt:	Minimum CHGIN regulation voltage. Must be one
> +				of: 4000000, 4100000, 4200000, 4300000,
> +				4400000, 4500000, 4600000, 4700000.
> +- input-current-limit-microamp:	CHGIN input current limit (in microamps). Must
> +				be one of: 95000, 190000, 285000, 380000,
> +				475000.

The binding looks good to me.

> +Example:
> +--------
> +
> +	charger {
> +		compatible = "maxim,max77650-charger";
> +		min-microvolt = <4200000>;
> +		curr-lim-microamp = <285000>;
> +	};

The example does not match the binding.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 1/3] dt-bindings: input: add GPIO controllable vibrator
From: Luca Weiss @ 2019-04-17 16:02 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Rob Herring, Mark Rutland, Mauro Carvalho Chehab,
	Pascal PAILLET-LME, Coly Li, Lee Jones, Xiaotong Lu, Brian Masney,
	Rob Herring, Baolin Wang, David Brown,
	open list:ARM/QUALCOMM SUPPORT,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
In-Reply-To: <20190412150625.28384-1-luca@z3ntu.xyz>

On Freitag, 12. April 2019 17:06:23 CEST Luca Weiss wrote:
> Provide a simple driver for GPIO controllable vibrators.
> It will be used by the Fairphone 2.
> 
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
>  .../bindings/input/gpio-vibrator.txt          | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/input/gpio-vibrator.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/gpio-vibrator.txt
> b/Documentation/devicetree/bindings/input/gpio-vibrator.txt new file mode
> 100644
> index 000000000000..93e5a8e7622d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/gpio-vibrator.txt
> @@ -0,0 +1,20 @@
> +* GPIO vibrator device tree bindings
> +
> +Registers a GPIO device as vibrator, where the vibration motor just has the
> +capability to turn on or off. If the device is connected to a pwm, you
> should +use the pwm-vibrator driver instead.
> +
> +Required properties:
> +- compatible: should contain "gpio-vibrator"
> +- enable-gpios: Should contain a GPIO handle
> +
> +Optional properties:
> +- vcc-supply: Phandle for the regulator supplying power
> +
> +Example from Fairphone 2:
> +
> +vibrator {
> +	compatible = "gpio-vibrator";
> +	enable-gpios = <&msmgpio 86 GPIO_ACTIVE_HIGH>;
> +	vcc-supply = <&pm8941_l18>;
> +};

I see that the yaml based device tree binding docs seem to be the new hotness? 
Is there any "policy" / preference about new drivers?

Luca

^ permalink raw reply

* Re: [PATCH v3 00/26] compat_ioctl: cleanups
From: Arnd Bergmann @ 2019-04-17  9:26 UTC (permalink / raw)
  To: dgilbert
  Cc: linux-nvme, linux-iio, linux-remoteproc,
	Linux Fbdev development list, dri-devel, Platform Driver, IDE-ML,
	linux-mtd, sparclinux, linux1394-devel, driverdevel, linux-s390,
	linux-scsi, Bluez mailing list, y2038 Mailman List, qat-linux,
	amd-gfx, open list:HID CORE LAYER, Marcel Holtmann,
	Linux Media Mailing List, linux-rtc, ALSA
In-Reply-To: <c379ed8d-1f33-e391-a1b5-810da77605f2@interlog.com>

On Wed, Apr 17, 2019 at 12:33 AM Douglas Gilbert <dgilbert@interlog.com> wrote:
>
> On 2019-04-16 4:19 p.m., Arnd Bergmann wrote:
> > Hi Al,
> >
> > It took me way longer than I had hoped to revisit this series, see
> > https://lore.kernel.org/lkml/20180912150142.157913-1-arnd@arndb.de/
> > for the previously posted version.
> >
> > I've come to the point where all conversion handlers and most
> > COMPATIBLE_IOCTL() entries are gone from this file, but for
> > now, this series only has the parts that have either been reviewed
> > previously, or that are simple enough to include.
> >
> > The main missing piece is the SG_IO/SG_GET_REQUEST_TABLE conversion.
> > I'll post the patches I made for that later, as they need more
> > testing and review from the scsi maintainers.
>
> Perhaps you could look at the document in this url:
>      http://sg.danny.cz/sg/sg_v40.html
>
> It is work-in-progress to modernize the SCSI generic driver. It
> extends ioctl(sg_fd, SG_IO, &pt_obj) to additionally accept the sg v4
> interface as defined in include/uapi/linux/bsg.h . Currently only the
> bsg driver uses the sg v4 interface. Since struct sg_io_v4 is all
> explicitly sized integers, I'm guessing it is immune "compat" problems.
> [I can see no reference to bsg nor struct sg_io_v4 in the current
> fs/compat_ioctl.c file.]

Ok, I've taken a brief look at your series now. Unfortunately it clashes
quite hard with my series, but it's probably for the better to have your
stuff get merged first.

A few (unsorted) comments from going through your patches:

- the added ioctls are all compatible when using the v4 structures
  and mostly don't need handlers for compat mode, but they need to be
  called from .compat_ioctl to actually be usable in compat mode.
  With my patches you get that.
- One exception for the v4 layout is the use of iovec pointers, as
  'struct iovec' is incompatible. We should probably merge the
  generic compat_import_iovec() into import_iovec() with a
  'in_compat_syscall()' check, which would be helpful in general.
  bsg.c does not iovec, so it is not affected by this at the moment,
  maybe it would be better to stay compatible with that and also
  not support them in sg.c?
- Is there a need for the new sg_ioctl_iosubmit/sg_ioctl_ioreceive
  to support the v3 structures? Those are /not/ compatible, so
  you need extra code to handle the v3-compat layout as well.
  Supporting only v4 would simplify this.
- the lack of changeset descriptions is a bit irritating and makes
  it much harder to understand what you are doing.
- try to keep patches that move code around separate from those
  that change it in any other way, for better reviewing.
- in "sg: preparation for request sharing", you seem to inadvertently
  change the size of "struct sg_extended_info", making it 4 bytes
  longer by adding two members.
- You should never use IS_ERR_OR_NULL() in normal code, that
  is just a sign of a bad API. Make each function have consistent
  error behavior.
- The "#if 0  /* temporary to shorten big patch */" trick breaks
  bisection, that is probably worse than the larger patch.
- The split access_ok()/__copy_from_user() has fallen out of
  favor because it has caused too many bugs in the past, just
  use the combined copy_from_user() instead.
- ktime_to_ns(ktime_get_with_offset(TK_OFFS_BOOT)) followed
  by a 64-bit division won't work on 32-bit machines, use
  ktime_get_boottime_ts64() instead.

> Other additions described in the that document are these new ioctls:
>    - SG_IOSUBMIT    ultimately to replace write(sg_fd, ...)
>    - SG_IORECEIVE              to replace read(sg_fd, ...)
>    - SG_IOABORT     abort SCSI cmd in progress; new functionality
>    - SG_SET_GET_EXTENDED   has associated struct sg_extended_info
>
> The first three take a pointer to a struct sg_io_hdr (v3 interface) or
> a struct sg_io_v4 object. Both objects start with a 32 bit integer:
> 'S' identifies the v3 interface while 'Q' identifies the v4 interface.

I think the magic character was a mistake in the original design,
just like versioned interfaces in general. If you are extending an
interface in an incompatible way, the normal way would be to
have separate command codes, like SG_IORECEIVE_V3
and SG_IORECEIVE_V4, if you absolutely have to maintain
compatiblity with the old interface (which I think you don't in
case of SG_IORECEIVE).

For SG_IO, I can see  why you want to support both the v3
and v4 structures plus the compat-v3 version, but I'd try to keep
them as separate as possible, and do something like

static int  sg_ctl_sg_io(struct file *filp, struct sg_device *sdp,
struct sg_fd *sfp,
                                 void __user *p)
{
       int ret;

      ret = sg_io_v4(filp, sdp, sfp, (struct sg_io_v4 __user *)p);

      if (ret != -ENOIOCTLCMD || !S_ENABLED(CONFIG_SG_IO_V3))
            return ret;

     if (in_compat_syscall())
           ret = sg_io_compat_(filp, sdp, sfp, (struct
compat_sg_io_hdr __user *)p);
     else
           ret = sg_io_v3(filp, sdp, sfp, (struct sg_io_hdr __user *)p);
}

In my patch series, I combined the latter two cases and used a shared
get_sg_io_hdr()/put_sg_io_hdr() helper as well as a wrapper for the iovec
issue.

> The SG_SET_GET_EXTENDED ioctl takes a pointer to a struct
> sg_extended_info object which contains explicitly sized integers so it
> may also be immune from "compat" problems. The ioctls section (13) of
> that document referenced above has a table showing how many "sets and
> gets" are hiding in the SG_SET_GET_EXTENDED ioctl.

Agreed, SG_SET_GET_EXTENDED looks fine to me from a compat
perspective.

I've uploaded my patches to
git://git.kernel.org:/pub/scm/linux/kernel/git/arnd/playground.git
compat-ioctl-v3
This contains both the series I posted here, and my scsi ioctl rework.

Maybe you can take the bits you need from that to handle the v3-compat
structures and integrate it into your series?

     Arnd
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH] xpad.c: Send necessary control transfer to certain Xbox controllers
From: Mike Salvatore @ 2019-04-16 23:31 UTC (permalink / raw)
  To: dmitry.torokhov, linux-input
  Cc: ramzeto, marcus.folkesson, flibitijibibo, aicommander,
	leosperling97, gottox, frtherien, linux-kernel, Mike Salvatore,
	mike.s.salvatore
In-Reply-To: <eff45453-f105-d713-1e77-a8fa9850c8fa@canonical.com>


[-- Attachment #1.1: Type: text/plain, Size: 7344 bytes --]

All,

I took another look at this patch and realized it was missing a #include.
I've added the appropriate #include to a new version of the patch.

Thanks,
Mike Salvatore



From 6fbf80fa3d5bc39fa054350a663080e1380046f8 Mon Sep 17 00:00:00 2001
From: Mike Salvatore <mike.salvatore@canonical.com>
Date: Wed, 13 Mar 2019 22:11:37 -0400
Subject: [PATCH] Input: xpad - send control init message to certain Xbox
 controllers

The Xbox controller with idVendor == 0x045e and idProduct == 0x028e
requires that a specific control transfer be sent from the host to the
device before the device will send data to the host.

This patch introduces an xboxone_control_packet struct and a mechanism
for sending control packets to devices that require them at
initialization.

Signed-off-by: Mike Salvatore <mike.salvatore@canonical.com>
---
 drivers/input/joystick/xpad.c | 57 +++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index cfc8b94527b9..756df325bfa6 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -81,6 +81,7 @@
 #include <linux/slab.h>
 #include <linux/stat.h>
 #include <linux/module.h>
+#include <linux/usb/ch9.h>
 #include <linux/usb/input.h>
 #include <linux/usb/quirks.h>
 
@@ -460,6 +461,25 @@ struct xboxone_init_packet {
 		.len		= ARRAY_SIZE(_data),	\
 	}
 
+struct xboxone_control_packet {
+	u16 idVendor;
+	u16 idProduct;
+	struct usb_ctrlrequest ctrlrequest;
+};
+
+#define XBOXONE_CONTROL_PKT(_vid, _pid, _reqtype, _req, _value, _index, _len)  \
+	{						\
+		.idVendor	= (_vid),		\
+		.idProduct	= (_pid),		\
+		.ctrlrequest    = {			\
+			.bRequestType	= (_reqtype),	\
+			.bRequest	= (_req),	\
+			.wValue		= (_value),	\
+			.wIndex		= (_index),	\
+			.wLength	= (_len),	\
+		},					\
+	}
+
 
 /*
  * This packet is required for all Xbox One pads with 2015
@@ -537,6 +557,13 @@ static const struct xboxone_init_packet xboxone_init_packets[] = {
 	XBOXONE_INIT_PKT(0x24c6, 0x543a, xboxone_rumbleend_init),
 };
 
+static const struct xboxone_control_packet xboxone_control_packets[] = {
+	XBOXONE_CONTROL_PKT(0x045e, 0x028e,
+			USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_INTERFACE,
+			USB_REQ_CLEAR_FEATURE,
+			USB_DEVICE_REMOTE_WAKEUP, 0, 0),
+};
+
 struct xpad_output_packet {
 	u8 data[XPAD_PKT_LEN];
 	u8 len;
@@ -1119,6 +1146,31 @@ static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad,
 	return error;
 }
 
+static int xpad_init_control_msg(struct usb_xpad *xpad)
+{
+	struct usb_device *udev = xpad->udev;
+	size_t i;
+
+	for (i = 0; i < ARRAY_SIZE(xboxone_control_packets); i++) {
+		u16 idVendor = xboxone_control_packets[i].idVendor;
+		u16 idProduct = xboxone_control_packets[i].idProduct;
+
+		if (le16_to_cpu(udev->descriptor.idVendor) == idVendor
+		    && le16_to_cpu(udev->descriptor.idProduct) == idProduct) {
+			const struct usb_ctrlrequest *ctrlrequest =
+				&(xboxone_control_packets[i].ctrlrequest);
+
+			return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+				ctrlrequest->bRequest,
+				ctrlrequest->bRequestType, ctrlrequest->wValue,
+				ctrlrequest->wIndex, NULL, ctrlrequest->wLength,
+				2 * HZ);
+		}
+	}
+
+	return 0;
+}
+
 static void xpad_stop_output(struct usb_xpad *xpad)
 {
 	if (xpad->xtype != XTYPE_UNKNOWN) {
@@ -1839,6 +1891,11 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 		if (error)
 			goto err_deinit_output;
 	}
+
+	error = xpad_init_control_msg(xpad);
+	if (error)
+		goto err_deinit_output;
+
 	return 0;
 
 err_deinit_output:
-- 
2.17.1



On 3/23/19 12:46 PM, Mike Salvatore wrote:
> From 3051524e62d68b920019bcb50a713e736fcf4234 Mon Sep 17 00:00:00 2001
> From: Mike Salvatore <mike.salvatore@canonical.com>
> Date: Wed, 13 Mar 2019 22:11:37 -0400
> Subject: [PATCH] Input: xpad - send control init message to certain Xbox
>  controllers
> 
> The Xbox controller with idVendor == 0x045e and idProduct == 0x028e
> requires that a specific control transfer be sent from the host to the
> device before the device will send data to the host.
> 
> This patch introduces an xboxone_control_packet struct and a mechanism
> for sending control packets to devices that require them at
> initialization.
> 
> Signed-off-by: Mike Salvatore <mike.salvatore@canonical.com>
> ---
>  drivers/input/joystick/xpad.c | 56 +++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index cfc8b94527b9..f45522b9ff1f 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -460,6 +460,25 @@ struct xboxone_init_packet {
>  		.len		= ARRAY_SIZE(_data),	\
>  	}
>  
> +struct xboxone_control_packet {
> +	u16 idVendor;
> +	u16 idProduct;
> +	struct usb_ctrlrequest ctrlrequest;
> +};
> +
> +#define XBOXONE_CONTROL_PKT(_vid, _pid, _reqtype, _req, _value, _index, _len)  \
> +	{						\
> +		.idVendor	= (_vid),		\
> +		.idProduct	= (_pid),		\
> +		.ctrlrequest    = {			\
> +			.bRequestType	= (_reqtype),	\
> +			.bRequest	= (_req),	\
> +			.wValue		= (_value),	\
> +			.wIndex		= (_index),	\
> +			.wLength	= (_len),	\
> +		},					\
> +	}
> +
>  
>  /*
>   * This packet is required for all Xbox One pads with 2015
> @@ -537,6 +556,13 @@ static const struct xboxone_init_packet xboxone_init_packets[] = {
>  	XBOXONE_INIT_PKT(0x24c6, 0x543a, xboxone_rumbleend_init),
>  };
>  
> +static const struct xboxone_control_packet xboxone_control_packets[] = {
> +	XBOXONE_CONTROL_PKT(0x045e, 0x028e,
> +			USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_INTERFACE,
> +			USB_REQ_CLEAR_FEATURE,
> +			USB_DEVICE_REMOTE_WAKEUP, 0, 0),
> +};
> +
>  struct xpad_output_packet {
>  	u8 data[XPAD_PKT_LEN];
>  	u8 len;
> @@ -1119,6 +1145,31 @@ static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad,
>  	return error;
>  }
>  
> +static int xpad_init_control_msg(struct usb_xpad *xpad)
> +{
> +	struct usb_device *udev = xpad->udev;
> +	size_t i;
> +
> +	for (i = 0; i < ARRAY_SIZE(xboxone_control_packets); i++) {
> +		u16 idVendor = xboxone_control_packets[i].idVendor;
> +		u16 idProduct = xboxone_control_packets[i].idProduct;
> +
> +		if (le16_to_cpu(udev->descriptor.idVendor) == idVendor
> +		    && le16_to_cpu(udev->descriptor.idProduct) == idProduct) {
> +			const struct usb_ctrlrequest *ctrlrequest =
> +				&(xboxone_control_packets[i].ctrlrequest);
> +
> +			return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> +				ctrlrequest->bRequest,
> +				ctrlrequest->bRequestType, ctrlrequest->wValue,
> +				ctrlrequest->wIndex, NULL, ctrlrequest->wLength,
> +				2 * HZ);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static void xpad_stop_output(struct usb_xpad *xpad)
>  {
>  	if (xpad->xtype != XTYPE_UNKNOWN) {
> @@ -1839,6 +1890,11 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  		if (error)
>  			goto err_deinit_output;
>  	}
> +
> +	error = xpad_init_control_msg(xpad);
> +	if (error)
> +		goto err_deinit_output;
> +
>  	return 0;
>  
>  err_deinit_output:
> -- 2.17.1


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related

* Re: [PATCH v3 00/26] compat_ioctl: cleanups
From: Douglas Gilbert @ 2019-04-16 22:33 UTC (permalink / raw)
  To: Arnd Bergmann, Alexander Viro
  Cc: linux-fsdevel, y2038, linux-kernel, David S. Miller,
	Greg Kroah-Hartman, Karsten Keil, James E.J. Bottomley,
	Martin K. Petersen, Marcel Holtmann, netdev, devel,
	linux-integrity, qat-linux, linux-crypto, linux-media, dri-devel,
	linux1394-devel, amd-gfx, linux-input, linux-usb,
	linux-arm-kernel, linux-ide, linux-iio, linux-rdma
In-Reply-To: <20190416202013.4034148-1-arnd@arndb.de>

On 2019-04-16 4:19 p.m., Arnd Bergmann wrote:
> Hi Al,
> 
> It took me way longer than I had hoped to revisit this series, see
> https://lore.kernel.org/lkml/20180912150142.157913-1-arnd@arndb.de/
> for the previously posted version.
> 
> I've come to the point where all conversion handlers and most
> COMPATIBLE_IOCTL() entries are gone from this file, but for
> now, this series only has the parts that have either been reviewed
> previously, or that are simple enough to include.
> 
> The main missing piece is the SG_IO/SG_GET_REQUEST_TABLE conversion.
> I'll post the patches I made for that later, as they need more
> testing and review from the scsi maintainers.

Perhaps you could look at the document in this url:
     http://sg.danny.cz/sg/sg_v40.html

It is work-in-progress to modernize the SCSI generic driver. It
extends ioctl(sg_fd, SG_IO, &pt_obj) to additionally accept the sg v4
interface as defined in include/uapi/linux/bsg.h . Currently only the
bsg driver uses the sg v4 interface. Since struct sg_io_v4 is all
explicitly sized integers, I'm guessing it is immune "compat" problems.
[I can see no reference to bsg nor struct sg_io_v4 in the current
fs/compat_ioctl.c file.]

Other additions described in the that document are these new ioctls:
   - SG_IOSUBMIT    ultimately to replace write(sg_fd, ...)
   - SG_IORECEIVE              to replace read(sg_fd, ...)
   - SG_IOABORT     abort SCSI cmd in progress; new functionality
   - SG_SET_GET_EXTENDED   has associated struct sg_extended_info

The first three take a pointer to a struct sg_io_hdr (v3 interface) or
a struct sg_io_v4 object. Both objects start with a 32 bit integer:
'S' identifies the v3 interface while 'Q' identifies the v4 interface.

The SG_SET_GET_EXTENDED ioctl takes a pointer to a struct
sg_extended_info object which contains explicitly sized integers so it
may also be immune from "compat" problems. The ioctls section (13) of
that document referenced above has a table showing how many "sets and
gets" are hiding in the SG_SET_GET_EXTENDED ioctl.

BTW No change is proposed for this case:
     ioctl(normal_block_device, SG_IO, &sg_v3_obj)
which is handled by block/scsi_ioctl.c


This would be a good time for me to address any "compat" concerns in the
proposed sg driver update.

Doug Gilbert


> I hope you can still take these for the coming merge window, unless
> new problems come up.
> 
>        Arnd
> 
> Arnd Bergmann (26):
>    compat_ioctl: pppoe: fix PPPOEIOCSFWD handling
>    compat_ioctl: move simple ppp command handling into driver
>    compat_ioctl: avoid unused function warning for do_ioctl
>    compat_ioctl: move PPPIOCSCOMPRESS32 to ppp-generic.c
>    compat_ioctl: move PPPIOCSPASS32/PPPIOCSACTIVE32 to ppp_generic.c
>    compat_ioctl: handle PPPIOCGIDLE for 64-bit time_t
>    compat_ioctl: move rtc handling into rtc-dev.c
>    compat_ioctl: add compat_ptr_ioctl()
>    compat_ioctl: move drivers to compat_ptr_ioctl
>    compat_ioctl: use correct compat_ptr() translation in drivers
>    ceph: fix compat_ioctl for ceph_dir_operations
>    compat_ioctl: move more drivers to compat_ptr_ioctl
>    compat_ioctl: move tape handling into drivers
>    compat_ioctl: move ATYFB_CLK handling to atyfb driver
>    compat_ioctl: move isdn/capi ioctl translation into driver
>    compat_ioctl: move rfcomm handlers into driver
>    compat_ioctl: move hci_sock handlers into driver
>    compat_ioctl: remove HCIUART handling
>    compat_ioctl: remove HIDIO translation
>    compat_ioctl: remove translation for sound ioctls
>    compat_ioctl: remove IGNORE_IOCTL()
>    compat_ioctl: remove /dev/random commands
>    compat_ioctl: remove joystick ioctl translation
>    compat_ioctl: remove PCI ioctl translation
>    compat_ioctl: remove /dev/raw ioctl translation
>    compat_ioctl: remove last RAID handling code
> 
>   Documentation/networking/ppp_generic.txt    |   2 +
>   arch/um/drivers/hostaudio_kern.c            |   1 +
>   drivers/android/binder.c                    |   2 +-
>   drivers/char/ppdev.c                        |  12 +-
>   drivers/char/random.c                       |   1 +
>   drivers/char/tpm/tpm_vtpm_proxy.c           |  12 +-
>   drivers/crypto/qat/qat_common/adf_ctl_drv.c |   2 +-
>   drivers/dma-buf/dma-buf.c                   |   4 +-
>   drivers/dma-buf/sw_sync.c                   |   2 +-
>   drivers/dma-buf/sync_file.c                 |   2 +-
>   drivers/firewire/core-cdev.c                |  12 +-
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c    |   2 +-
>   drivers/hid/hidraw.c                        |   4 +-
>   drivers/hid/usbhid/hiddev.c                 |  11 +-
>   drivers/hwtracing/stm/core.c                |  12 +-
>   drivers/ide/ide-tape.c                      |  31 +-
>   drivers/iio/industrialio-core.c             |   2 +-
>   drivers/infiniband/core/uverbs_main.c       |   4 +-
>   drivers/isdn/capi/capi.c                    |  31 +
>   drivers/isdn/i4l/isdn_ppp.c                 |  14 +-
>   drivers/media/rc/lirc_dev.c                 |   4 +-
>   drivers/mfd/cros_ec_dev.c                   |   4 +-
>   drivers/misc/cxl/flash.c                    |   8 +-
>   drivers/misc/genwqe/card_dev.c              |  23 +-
>   drivers/misc/mei/main.c                     |  22 +-
>   drivers/misc/vmw_vmci/vmci_host.c           |   2 +-
>   drivers/mtd/ubi/cdev.c                      |  36 +-
>   drivers/net/ppp/ppp_generic.c               |  99 +++-
>   drivers/net/ppp/pppoe.c                     |   7 +
>   drivers/net/ppp/pptp.c                      |   3 +
>   drivers/net/tap.c                           |  12 +-
>   drivers/nvdimm/bus.c                        |   4 +-
>   drivers/nvme/host/core.c                    |   2 +-
>   drivers/pci/switch/switchtec.c              |   2 +-
>   drivers/platform/x86/wmi.c                  |   2 +-
>   drivers/rpmsg/rpmsg_char.c                  |   4 +-
>   drivers/rtc/dev.c                           |  13 +-
>   drivers/rtc/rtc-vr41xx.c                    |  10 +
>   drivers/s390/char/tape_char.c               |  41 +-
>   drivers/sbus/char/display7seg.c             |   2 +-
>   drivers/sbus/char/envctrl.c                 |   4 +-
>   drivers/scsi/3w-xxxx.c                      |   4 +-
>   drivers/scsi/cxlflash/main.c                |   2 +-
>   drivers/scsi/esas2r/esas2r_main.c           |   2 +-
>   drivers/scsi/megaraid/megaraid_mm.c         |  28 +-
>   drivers/scsi/osst.c                         |  34 +-
>   drivers/scsi/pmcraid.c                      |   4 +-
>   drivers/scsi/st.c                           |  35 +-
>   drivers/staging/android/ion/ion.c           |   4 +-
>   drivers/staging/pi433/pi433_if.c            |  12 +-
>   drivers/staging/vme/devices/vme_user.c      |   2 +-
>   drivers/tee/tee_core.c                      |   2 +-
>   drivers/usb/class/cdc-wdm.c                 |   2 +-
>   drivers/usb/class/usbtmc.c                  |   4 +-
>   drivers/usb/core/devio.c                    |  16 +-
>   drivers/usb/gadget/function/f_fs.c          |  12 +-
>   drivers/vfio/vfio.c                         |  39 +-
>   drivers/vhost/net.c                         |  12 +-
>   drivers/vhost/scsi.c                        |  12 +-
>   drivers/vhost/test.c                        |  12 +-
>   drivers/vhost/vsock.c                       |  12 +-
>   drivers/video/fbdev/aty/atyfb_base.c        |  12 +-
>   drivers/virt/fsl_hypervisor.c               |   2 +-
>   fs/btrfs/super.c                            |   2 +-
>   fs/ceph/dir.c                               |   1 +
>   fs/ceph/file.c                              |   2 +-
>   fs/compat_ioctl.c                           | 602 +-------------------
>   fs/fat/file.c                               |  13 +-
>   fs/fuse/dev.c                               |   2 +-
>   fs/notify/fanotify/fanotify_user.c          |   2 +-
>   fs/userfaultfd.c                            |   2 +-
>   include/linux/fs.h                          |   7 +
>   include/linux/if_pppox.h                    |   2 +
>   include/linux/mtio.h                        |  58 ++
>   include/uapi/linux/ppp-ioctl.h              |   2 +
>   include/uapi/linux/ppp_defs.h               |  14 +
>   net/bluetooth/hci_sock.c                    |  21 +-
>   net/bluetooth/rfcomm/sock.c                 |  14 +-
>   net/l2tp/l2tp_ppp.c                         |   3 +
>   net/rfkill/core.c                           |   2 +-
>   sound/core/oss/pcm_oss.c                    |   4 +
>   sound/oss/dmasound/dmasound_core.c          |   2 +
>   82 files changed, 452 insertions(+), 1034 deletions(-)
>   create mode 100644 include/linux/mtio.h
> 

^ permalink raw reply

* Re: [PATCH v3 09/26] compat_ioctl: move drivers to compat_ptr_ioctl
From: Jiri Kosina @ 2019-04-16 20:31 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Alexander Viro, linux-fsdevel, y2038, linux-kernel,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jason Gunthorpe,
	Sudip Mukherjee, Peter Huewe, Jason Gunthorpe, Stefan Richter,
	Benjamin Tissoires, Alexander Shishkin, Maxime Coquelin,
	Alexandre Torgue, Tomas Winkler, Artem Bityutskiy,
	Richard Weinberger, David Woodhouse
In-Reply-To: <20190416202013.4034148-10-arnd@arndb.de>

On Tue, 16 Apr 2019, Arnd Bergmann wrote:

> Each of these drivers has a copy of the same trivial helper function to
> convert the pointer argument and then call the native ioctl handler.
> 
> We now have a generic implementation of that, so use it.
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/char/ppdev.c              | 12 +---------
>  drivers/char/tpm/tpm_vtpm_proxy.c | 12 +---------
>  drivers/firewire/core-cdev.c      | 12 +---------
>  drivers/hid/usbhid/hiddev.c       | 11 +--------

For hiddev.c:

Reviewed-by: Jiri Kosina <jkosina@suse.cz> 

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* [PATCH v3 12/26] compat_ioctl: move more drivers to compat_ptr_ioctl
From: Arnd Bergmann @ 2019-04-16 20:25 UTC (permalink / raw)
  To: Alexander Viro
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, Daniel Vetter,
	linux-pci-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Bjorn Andersson,
	sparclinux-u79uwXL29TY76Z2rM5mHXA, Mauro Carvalho Chehab,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, y2038-cunTk1MwBs8s++Sfvej+rw,
	qat-linux-ral2JQCrhuEAvxtiuMwx3w,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Jason Gunthorpe,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Darren Hart,
	linux-media-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann,
	linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
	linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jonathan Cameron,
	David Sterba, ceph-devel-u79uwXL29TY76Z2rM5mHXA,
	platform-driver-x86-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
	linux-usb-fy+rA21nqHI
In-Reply-To: <20190416202013.4034148-1-arnd-r2nGTMty4D4@public.gmane.org>

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.

Acked-by: Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Acked-by: Daniel Vetter <daniel.vetter-/w4YWyX8dFk@public.gmane.org>
Acked-by: Mauro Carvalho Chehab <mchehab+samsung-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Acked-by: David Sterba <dsterba-IBi9RG/b67k@public.gmane.org>
Acked-by: Darren Hart (VMware) <dvhart-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Acked-by: Jonathan Cameron <Jonathan.Cameron-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Acked-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
 drivers/android/binder.c                    | 2 +-
 drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +-
 drivers/dma-buf/dma-buf.c                   | 4 +---
 drivers/dma-buf/sw_sync.c                   | 2 +-
 drivers/dma-buf/sync_file.c                 | 2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c    | 2 +-
 drivers/hid/hidraw.c                        | 4 +---
 drivers/iio/industrialio-core.c             | 2 +-
 drivers/infiniband/core/uverbs_main.c       | 4 ++--
 drivers/media/rc/lirc_dev.c                 | 4 +---
 drivers/mfd/cros_ec_dev.c                   | 4 +---
 drivers/misc/vmw_vmci/vmci_host.c           | 2 +-
 drivers/nvdimm/bus.c                        | 4 ++--
 drivers/nvme/host/core.c                    | 2 +-
 drivers/pci/switch/switchtec.c              | 2 +-
 drivers/platform/x86/wmi.c                  | 2 +-
 drivers/rpmsg/rpmsg_char.c                  | 4 ++--
 drivers/sbus/char/display7seg.c             | 2 +-
 drivers/sbus/char/envctrl.c                 | 4 +---
 drivers/scsi/3w-xxxx.c                      | 4 +---
 drivers/scsi/cxlflash/main.c                | 2 +-
 drivers/scsi/esas2r/esas2r_main.c           | 2 +-
 drivers/scsi/pmcraid.c                      | 4 +---
 drivers/staging/android/ion/ion.c           | 4 +---
 drivers/staging/vme/devices/vme_user.c      | 2 +-
 drivers/tee/tee_core.c                      | 2 +-
 drivers/usb/class/cdc-wdm.c                 | 2 +-
 drivers/usb/class/usbtmc.c                  | 4 +---
 drivers/virt/fsl_hypervisor.c               | 2 +-
 fs/btrfs/super.c                            | 2 +-
 fs/ceph/dir.c                               | 2 +-
 fs/ceph/file.c                              | 2 +-
 fs/fuse/dev.c                               | 2 +-
 fs/notify/fanotify/fanotify_user.c          | 2 +-
 fs/userfaultfd.c                            | 2 +-
 net/rfkill/core.c                           | 2 +-
 36 files changed, 39 insertions(+), 57 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 4b9c7ca492e6..48109ade7234 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5998,7 +5998,7 @@ const struct file_operations binder_fops = {
 	.owner = THIS_MODULE,
 	.poll = binder_poll,
 	.unlocked_ioctl = binder_ioctl,
-	.compat_ioctl = binder_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.mmap = binder_mmap,
 	.open = binder_open,
 	.flush = binder_flush,
diff --git a/drivers/crypto/qat/qat_common/adf_ctl_drv.c b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
index abc7a7f64d64..ef0e482ee04f 100644
--- a/drivers/crypto/qat/qat_common/adf_ctl_drv.c
+++ b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
@@ -68,7 +68,7 @@ static long adf_ctl_ioctl(struct file *fp, unsigned int cmd, unsigned long arg);
 static const struct file_operations adf_ctl_ops = {
 	.owner = THIS_MODULE,
 	.unlocked_ioctl = adf_ctl_ioctl,
-	.compat_ioctl = adf_ctl_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 struct adf_ctl_drv_info {
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 7c858020d14b..0cb336fe6324 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -325,9 +325,7 @@ static const struct file_operations dma_buf_fops = {
 	.llseek		= dma_buf_llseek,
 	.poll		= dma_buf_poll,
 	.unlocked_ioctl	= dma_buf_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= dma_buf_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 };
 
 /*
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 32dcf7b4c935..411de6a8a0ad 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -419,5 +419,5 @@ const struct file_operations sw_sync_debugfs_fops = {
 	.open           = sw_sync_debugfs_open,
 	.release        = sw_sync_debugfs_release,
 	.unlocked_ioctl = sw_sync_ioctl,
-	.compat_ioctl	= sw_sync_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 };
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 4f6305ca52c8..0949f91eb85f 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -488,5 +488,5 @@ static const struct file_operations sync_file_fops = {
 	.release = sync_file_release,
 	.poll = sync_file_poll,
 	.unlocked_ioctl = sync_file_ioctl,
-	.compat_ioctl = sync_file_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 083bd8114db1..5d6ac7885aa7 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -49,7 +49,7 @@ static const char kfd_dev_name[] = "kfd";
 static const struct file_operations kfd_fops = {
 	.owner = THIS_MODULE,
 	.unlocked_ioctl = kfd_ioctl,
-	.compat_ioctl = kfd_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.open = kfd_open,
 	.mmap = kfd_mmap,
 };
diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index 9fc51eff1079..e7284d38b66d 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -476,9 +476,7 @@ static const struct file_operations hidraw_ops = {
 	.release =      hidraw_release,
 	.unlocked_ioctl = hidraw_ioctl,
 	.fasync =	hidraw_fasync,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl   = hidraw_ioctl,
-#endif
+	.compat_ioctl   = compat_ptr_ioctl,
 	.llseek =	noop_llseek,
 };
 
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 4700fd5d8c90..eed1bea257b4 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1635,7 +1635,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 = compat_ptr_ioctl,
 };
 
 static int iio_check_unique_scan_index(struct iio_dev *indio_dev)
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 70b7d80431a9..ac4321d7c800 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -1120,7 +1120,7 @@ static const struct file_operations uverbs_fops = {
 	.release = ib_uverbs_close,
 	.llseek	 = no_llseek,
 	.unlocked_ioctl = ib_uverbs_ioctl,
-	.compat_ioctl = ib_uverbs_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static const struct file_operations uverbs_mmap_fops = {
@@ -1131,7 +1131,7 @@ static const struct file_operations uverbs_mmap_fops = {
 	.release = ib_uverbs_close,
 	.llseek	 = no_llseek,
 	.unlocked_ioctl = ib_uverbs_ioctl,
-	.compat_ioctl = ib_uverbs_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static struct ib_client uverbs_client = {
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index f862f1b7f996..9ccc7e9cbc8e 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -730,9 +730,7 @@ static const struct file_operations lirc_fops = {
 	.owner		= THIS_MODULE,
 	.write		= ir_lirc_transmit_ir,
 	.unlocked_ioctl	= ir_lirc_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= ir_lirc_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 	.read		= ir_lirc_read,
 	.poll		= ir_lirc_poll,
 	.open		= ir_lirc_open,
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index d275deaecb12..4a602a40d75c 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -251,9 +251,7 @@ static const struct file_operations fops = {
 	.release = ec_device_release,
 	.read = ec_device_read,
 	.unlocked_ioctl = ec_device_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl = ec_device_ioctl,
-#endif
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static void cros_ec_class_release(struct device *dev)
diff --git a/drivers/misc/vmw_vmci/vmci_host.c b/drivers/misc/vmw_vmci/vmci_host.c
index 997f92543dd4..5bb406dabe85 100644
--- a/drivers/misc/vmw_vmci/vmci_host.c
+++ b/drivers/misc/vmw_vmci/vmci_host.c
@@ -969,7 +969,7 @@ static const struct file_operations vmuser_fops = {
 	.release	= vmci_host_close,
 	.poll		= vmci_host_poll,
 	.unlocked_ioctl	= vmci_host_unlocked_ioctl,
-	.compat_ioctl	= vmci_host_unlocked_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 };
 
 static struct miscdevice vmci_host_miscdev = {
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index 7bbff0af29b2..065ebd584482 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -1167,7 +1167,7 @@ static const struct file_operations nvdimm_bus_fops = {
 	.owner = THIS_MODULE,
 	.open = nd_open,
 	.unlocked_ioctl = nd_ioctl,
-	.compat_ioctl = nd_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.llseek = noop_llseek,
 };
 
@@ -1175,7 +1175,7 @@ static const struct file_operations nvdimm_fops = {
 	.owner = THIS_MODULE,
 	.open = nd_open,
 	.unlocked_ioctl = nvdimm_ioctl,
-	.compat_ioctl = nvdimm_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.llseek = noop_llseek,
 };
 
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 2c43e12b70af..560929bee5ce 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2739,7 +2739,7 @@ static const struct file_operations nvme_dev_fops = {
 	.owner		= THIS_MODULE,
 	.open		= nvme_dev_open,
 	.unlocked_ioctl	= nvme_dev_ioctl,
-	.compat_ioctl	= nvme_dev_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 };
 
 static ssize_t nvme_sysfs_reset(struct device *dev,
diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index e22766c79fe9..3a54b4b616e2 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1006,7 +1006,7 @@ static const struct file_operations switchtec_fops = {
 	.read = switchtec_dev_read,
 	.poll = switchtec_dev_poll,
 	.unlocked_ioctl = switchtec_dev_ioctl,
-	.compat_ioctl = switchtec_dev_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static void link_event_work(struct work_struct *work)
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 7b26b6ccf1a0..dded9cef42f4 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -889,7 +889,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	= compat_ptr_ioctl,
 };
 
 static int wmi_dev_probe(struct device *dev)
diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index eea5ebbb5119..507bfe163883 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -290,7 +290,7 @@ static const struct file_operations rpmsg_eptdev_fops = {
 	.write_iter = rpmsg_eptdev_write_iter,
 	.poll = rpmsg_eptdev_poll,
 	.unlocked_ioctl = rpmsg_eptdev_ioctl,
-	.compat_ioctl = rpmsg_eptdev_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static ssize_t name_show(struct device *dev, struct device_attribute *attr,
@@ -451,7 +451,7 @@ static const struct file_operations rpmsg_ctrldev_fops = {
 	.open = rpmsg_ctrldev_open,
 	.release = rpmsg_ctrldev_release,
 	.unlocked_ioctl = rpmsg_ctrldev_ioctl,
-	.compat_ioctl = rpmsg_ctrldev_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static void rpmsg_ctrldev_release_device(struct device *dev)
diff --git a/drivers/sbus/char/display7seg.c b/drivers/sbus/char/display7seg.c
index a36e4cf1841d..c9f60656f54d 100644
--- a/drivers/sbus/char/display7seg.c
+++ b/drivers/sbus/char/display7seg.c
@@ -155,7 +155,7 @@ static long d7s_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 static const struct file_operations d7s_fops = {
 	.owner =		THIS_MODULE,
 	.unlocked_ioctl =	d7s_ioctl,
-	.compat_ioctl =		d7s_ioctl,
+	.compat_ioctl =		compat_ptr_ioctl,
 	.open =			d7s_open,
 	.release =		d7s_release,
 	.llseek = noop_llseek,
diff --git a/drivers/sbus/char/envctrl.c b/drivers/sbus/char/envctrl.c
index 1a6e7224017c..dd2dfa85fc68 100644
--- a/drivers/sbus/char/envctrl.c
+++ b/drivers/sbus/char/envctrl.c
@@ -714,9 +714,7 @@ static const struct file_operations envctrl_fops = {
 	.owner =		THIS_MODULE,
 	.read =			envctrl_read,
 	.unlocked_ioctl =	envctrl_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl =		envctrl_ioctl,
-#endif
+	.compat_ioctl =		compat_ptr_ioctl,
 	.open =			envctrl_open,
 	.release =		envctrl_release,
 	.llseek =		noop_llseek,
diff --git a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c
index 2b1e0d503020..fb6444d0409c 100644
--- a/drivers/scsi/3w-xxxx.c
+++ b/drivers/scsi/3w-xxxx.c
@@ -1049,9 +1049,7 @@ static int tw_chrdev_open(struct inode *inode, struct file *file)
 static const struct file_operations tw_fops = {
 	.owner		= THIS_MODULE,
 	.unlocked_ioctl	= tw_chrdev_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl   = tw_chrdev_ioctl,
-#endif
+	.compat_ioctl   = compat_ptr_ioctl,
 	.open		= tw_chrdev_open,
 	.release	= NULL,
 	.llseek		= noop_llseek,
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 7096810fd222..e13d5de1d76e 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -3589,7 +3589,7 @@ static const struct file_operations cxlflash_chr_fops = {
 	.owner          = THIS_MODULE,
 	.open           = cxlflash_chr_open,
 	.unlocked_ioctl	= cxlflash_chr_ioctl,
-	.compat_ioctl	= cxlflash_chr_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 };
 
 /**
diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
index fdbda5c05aa0..80c5a235d193 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -613,7 +613,7 @@ static int __init esas2r_init(void)
 
 /* Handle ioctl calls to "/proc/scsi/esas2r/ATTOnode" */
 static const struct file_operations esas2r_proc_fops = {
-	.compat_ioctl	= esas2r_proc_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 	.unlocked_ioctl = esas2r_proc_ioctl,
 };
 
diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index e338d7a4f571..c0a1a1218c56 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -3988,9 +3988,7 @@ static const struct file_operations pmcraid_fops = {
 	.open = pmcraid_chr_open,
 	.fasync = pmcraid_chr_fasync,
 	.unlocked_ioctl = pmcraid_chr_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl = pmcraid_chr_ioctl,
-#endif
+	.compat_ioctl = compat_ptr_ioctl,
 	.llseek = noop_llseek,
 };
 
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 92c2914239e3..1663c163edca 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -567,9 +567,7 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 static const struct file_operations ion_fops = {
 	.owner          = THIS_MODULE,
 	.unlocked_ioctl = ion_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= ion_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 };
 
 static int debug_shrink_set(void *data, u64 val)
diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c
index 6a33aaa1a49f..fd0ea4dbcb91 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -494,7 +494,7 @@ static const struct file_operations vme_user_fops = {
 	.write = vme_user_write,
 	.llseek = vme_user_llseek,
 	.unlocked_ioctl = vme_user_unlocked_ioctl,
-	.compat_ioctl = vme_user_unlocked_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.mmap = vme_user_mmap,
 };
 
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 17c64fccbb10..eb97acf09868 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -684,7 +684,7 @@ static const struct file_operations tee_fops = {
 	.open = tee_open,
 	.release = tee_release,
 	.unlocked_ioctl = tee_ioctl,
-	.compat_ioctl = tee_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static void tee_release_device(struct device *dev)
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 9e9caff905d5..d48c032580d0 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -724,7 +724,7 @@ static const struct file_operations wdm_fops = {
 	.release =	wdm_release,
 	.poll =		wdm_poll,
 	.unlocked_ioctl = wdm_ioctl,
-	.compat_ioctl = wdm_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.llseek =	noop_llseek,
 };
 
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 4942122b2346..bbd0308b13f5 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -2220,9 +2220,7 @@ static const struct file_operations fops = {
 	.release	= usbtmc_release,
 	.flush		= usbtmc_flush,
 	.unlocked_ioctl	= usbtmc_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= usbtmc_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 	.fasync         = usbtmc_fasync,
 	.poll           = usbtmc_poll,
 	.llseek		= default_llseek,
diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
index 8ba726e600e9..fbf02bf60f62 100644
--- a/drivers/virt/fsl_hypervisor.c
+++ b/drivers/virt/fsl_hypervisor.c
@@ -703,7 +703,7 @@ static const struct file_operations fsl_hv_fops = {
 	.poll = fsl_hv_poll,
 	.read = fsl_hv_read,
 	.unlocked_ioctl = fsl_hv_ioctl,
-	.compat_ioctl = fsl_hv_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static struct miscdevice fsl_hv_misc_dev = {
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 120e4340792a..162ea4b6b417 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2307,7 +2307,7 @@ static const struct super_operations btrfs_super_ops = {
 static const struct file_operations btrfs_ctl_fops = {
 	.open = btrfs_control_open,
 	.unlocked_ioctl	 = btrfs_control_ioctl,
-	.compat_ioctl = btrfs_control_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.owner	 = THIS_MODULE,
 	.llseek = noop_llseek,
 };
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 7c060cb22aa3..a493b957713f 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1785,7 +1785,7 @@ const struct file_operations ceph_dir_fops = {
 	.open = ceph_open,
 	.release = ceph_release,
 	.unlocked_ioctl = ceph_ioctl,
-	.compat_ioctl = ceph_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.fsync = ceph_fsync,
 	.lock = ceph_lock,
 	.flock = ceph_flock,
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 9f53c3d99304..9b5fe7eee3c1 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -2112,7 +2112,7 @@ const struct file_operations ceph_file_fops = {
 	.splice_read = generic_file_splice_read,
 	.splice_write = iter_file_splice_write,
 	.unlocked_ioctl = ceph_ioctl,
-	.compat_ioctl	= ceph_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 	.fallocate	= ceph_fallocate,
 	.copy_file_range = ceph_copy_file_range,
 };
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 9971a35cf1ef..dcdb26068b71 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2354,7 +2354,7 @@ const struct file_operations fuse_dev_operations = {
 	.release	= fuse_dev_release,
 	.fasync		= fuse_dev_fasync,
 	.unlocked_ioctl = fuse_dev_ioctl,
-	.compat_ioctl   = fuse_dev_ioctl,
+	.compat_ioctl   = compat_ptr_ioctl,
 };
 EXPORT_SYMBOL_GPL(fuse_dev_operations);
 
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index a90bb19dcfa2..a55aa029a308 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -523,7 +523,7 @@ static const struct file_operations fanotify_fops = {
 	.fasync		= NULL,
 	.release	= fanotify_release,
 	.unlocked_ioctl	= fanotify_ioctl,
-	.compat_ioctl	= fanotify_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 	.llseek		= noop_llseek,
 };
 
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 89800fc7dc9d..f93dcf8c996f 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1901,7 +1901,7 @@ static const struct file_operations userfaultfd_fops = {
 	.poll		= userfaultfd_poll,
 	.read		= userfaultfd_read,
 	.unlocked_ioctl = userfaultfd_ioctl,
-	.compat_ioctl	= userfaultfd_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 	.llseek		= noop_llseek,
 };
 
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index abca57040f37..3b2f6ea44397 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -1323,7 +1323,7 @@ static const struct file_operations rfkill_fops = {
 	.release	= rfkill_fop_release,
 #ifdef CONFIG_RFKILL_INPUT
 	.unlocked_ioctl	= rfkill_fop_ioctl,
-	.compat_ioctl	= rfkill_fop_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 #endif
 	.llseek		= no_llseek,
 };
-- 
2.20.0

^ permalink raw reply related

* [PATCH v3 09/26] compat_ioctl: move drivers to compat_ptr_ioctl
From: Arnd Bergmann @ 2019-04-16 20:19 UTC (permalink / raw)
  To: Alexander Viro
  Cc: linux-fsdevel, y2038, linux-kernel, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jason Gunthorpe,
	Sudip Mukherjee, Peter Huewe, Jason Gunthorpe, Stefan Richter,
	Jiri Kosina, Benjamin Tissoires, Alexander Shishkin,
	Maxime Coquelin, Alexandre Torgue, Tomas Winkler,
	Artem Bityutskiy, Richard Weinberger, David
In-Reply-To: <20190416202013.4034148-1-arnd@arndb.de>

Each of these drivers has a copy of the same trivial helper function to
convert the pointer argument and then call the native ioctl handler.

We now have a generic implementation of that, so use it.

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jason Gunthorpe <jgg@mellanox.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/char/ppdev.c              | 12 +---------
 drivers/char/tpm/tpm_vtpm_proxy.c | 12 +---------
 drivers/firewire/core-cdev.c      | 12 +---------
 drivers/hid/usbhid/hiddev.c       | 11 +--------
 drivers/hwtracing/stm/core.c      | 12 +---------
 drivers/misc/mei/main.c           | 22 +----------------
 drivers/mtd/ubi/cdev.c            | 36 +++-------------------------
 drivers/net/tap.c                 | 12 +---------
 drivers/staging/pi433/pi433_if.c  | 12 +---------
 drivers/usb/core/devio.c          | 16 +------------
 drivers/vfio/vfio.c               | 39 +++----------------------------
 drivers/vhost/net.c               | 12 +---------
 drivers/vhost/scsi.c              | 12 +---------
 drivers/vhost/test.c              | 12 +---------
 drivers/vhost/vsock.c             | 12 +---------
 fs/fat/file.c                     | 13 +----------
 16 files changed, 20 insertions(+), 237 deletions(-)

diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
index 1ae77b41050a..e96c8d9623e0 100644
--- a/drivers/char/ppdev.c
+++ b/drivers/char/ppdev.c
@@ -674,14 +674,6 @@ static long pp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	return ret;
 }
 
-#ifdef CONFIG_COMPAT
-static long pp_compat_ioctl(struct file *file, unsigned int cmd,
-			    unsigned long arg)
-{
-	return pp_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static int pp_open(struct inode *inode, struct file *file)
 {
 	unsigned int minor = iminor(inode);
@@ -790,9 +782,7 @@ static const struct file_operations pp_fops = {
 	.write		= pp_write,
 	.poll		= pp_poll,
 	.unlocked_ioctl	= pp_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl   = pp_compat_ioctl,
-#endif
+	.compat_ioctl   = compat_ptr_ioctl,
 	.open		= pp_open,
 	.release	= pp_release,
 };
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index d74f3de74ae6..fb845f0a430b 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -675,20 +675,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 = compat_ptr_ioctl,
 	.llseek = noop_llseek,
 };
 
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 16a7045736a9..fb934680fdd3 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1659,14 +1659,6 @@ static long fw_device_op_ioctl(struct file *file,
 	return dispatch_ioctl(file->private_data, cmd, (void __user *)arg);
 }
 
-#ifdef CONFIG_COMPAT
-static long fw_device_op_compat_ioctl(struct file *file,
-				      unsigned int cmd, unsigned long arg)
-{
-	return dispatch_ioctl(file->private_data, cmd, compat_ptr(arg));
-}
-#endif
-
 static int fw_device_op_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct client *client = file->private_data;
@@ -1808,7 +1800,5 @@ const struct file_operations fw_device_ops = {
 	.mmap		= fw_device_op_mmap,
 	.release	= fw_device_op_release,
 	.poll		= fw_device_op_poll,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= fw_device_op_compat_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 };
diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index a746017fac17..ef4a1cd389d6 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -855,13 +855,6 @@ static long hiddev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	return r;
 }
 
-#ifdef CONFIG_COMPAT
-static long hiddev_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
-	return hiddev_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static const struct file_operations hiddev_fops = {
 	.owner =	THIS_MODULE,
 	.read =		hiddev_read,
@@ -871,9 +864,7 @@ static const struct file_operations hiddev_fops = {
 	.release =	hiddev_release,
 	.unlocked_ioctl =	hiddev_ioctl,
 	.fasync =	hiddev_fasync,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= hiddev_compat_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 	.llseek		= noop_llseek,
 };
 
diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index c7ba8acfd4d5..454da259f144 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -840,23 +840,13 @@ stm_char_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	return err;
 }
 
-#ifdef CONFIG_COMPAT
-static long
-stm_char_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
-	return stm_char_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
-}
-#else
-#define stm_char_compat_ioctl	NULL
-#endif
-
 static const struct file_operations stm_fops = {
 	.open		= stm_char_open,
 	.release	= stm_char_release,
 	.write		= stm_char_write,
 	.mmap		= stm_char_mmap,
 	.unlocked_ioctl	= stm_char_ioctl,
-	.compat_ioctl	= stm_char_compat_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 	.llseek		= no_llseek,
 };
 
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 87281b3695e6..cc6af92cdef0 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -535,24 +535,6 @@ static long mei_ioctl(struct file *file, unsigned int cmd, unsigned long data)
 	return rets;
 }
 
-/**
- * mei_compat_ioctl - the compat IOCTL function
- *
- * @file: pointer to file structure
- * @cmd: ioctl command
- * @data: pointer to mei message structure
- *
- * Return: 0 on success , <0 on error
- */
-#ifdef CONFIG_COMPAT
-static long mei_compat_ioctl(struct file *file,
-			unsigned int cmd, unsigned long data)
-{
-	return mei_ioctl(file, cmd, (unsigned long)compat_ptr(data));
-}
-#endif
-
-
 /**
  * mei_poll - the poll function
  *
@@ -855,9 +837,7 @@ static const struct file_operations mei_fops = {
 	.owner = THIS_MODULE,
 	.read = mei_read,
 	.unlocked_ioctl = mei_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl = mei_compat_ioctl,
-#endif
+	.compat_ioctl = compat_ptr_ioctl,
 	.open = mei_open,
 	.release = mei_release,
 	.write = mei_write,
diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c
index 947a8adbc799..265d34fa3efa 100644
--- a/drivers/mtd/ubi/cdev.c
+++ b/drivers/mtd/ubi/cdev.c
@@ -1091,36 +1091,6 @@ static long ctrl_cdev_ioctl(struct file *file, unsigned int cmd,
 	return err;
 }
 
-#ifdef CONFIG_COMPAT
-static long vol_cdev_compat_ioctl(struct file *file, unsigned int cmd,
-				  unsigned long arg)
-{
-	unsigned long translated_arg = (unsigned long)compat_ptr(arg);
-
-	return vol_cdev_ioctl(file, cmd, translated_arg);
-}
-
-static long ubi_cdev_compat_ioctl(struct file *file, unsigned int cmd,
-				  unsigned long arg)
-{
-	unsigned long translated_arg = (unsigned long)compat_ptr(arg);
-
-	return ubi_cdev_ioctl(file, cmd, translated_arg);
-}
-
-static long ctrl_cdev_compat_ioctl(struct file *file, unsigned int cmd,
-				   unsigned long arg)
-{
-	unsigned long translated_arg = (unsigned long)compat_ptr(arg);
-
-	return ctrl_cdev_ioctl(file, cmd, translated_arg);
-}
-#else
-#define vol_cdev_compat_ioctl  NULL
-#define ubi_cdev_compat_ioctl  NULL
-#define ctrl_cdev_compat_ioctl NULL
-#endif
-
 /* UBI volume character device operations */
 const struct file_operations ubi_vol_cdev_operations = {
 	.owner          = THIS_MODULE,
@@ -1131,7 +1101,7 @@ const struct file_operations ubi_vol_cdev_operations = {
 	.write          = vol_cdev_write,
 	.fsync		= vol_cdev_fsync,
 	.unlocked_ioctl = vol_cdev_ioctl,
-	.compat_ioctl   = vol_cdev_compat_ioctl,
+	.compat_ioctl   = compat_ptr_ioctl,
 };
 
 /* UBI character device operations */
@@ -1139,13 +1109,13 @@ const struct file_operations ubi_cdev_operations = {
 	.owner          = THIS_MODULE,
 	.llseek         = no_llseek,
 	.unlocked_ioctl = ubi_cdev_ioctl,
-	.compat_ioctl   = ubi_cdev_compat_ioctl,
+	.compat_ioctl   = compat_ptr_ioctl,
 };
 
 /* UBI control character device operations */
 const struct file_operations ubi_ctrl_cdev_operations = {
 	.owner          = THIS_MODULE,
 	.unlocked_ioctl = ctrl_cdev_ioctl,
-	.compat_ioctl   = ctrl_cdev_compat_ioctl,
+	.compat_ioctl   = compat_ptr_ioctl,
 	.llseek		= no_llseek,
 };
diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index 2ea9b4976f4a..ebe425e65992 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -1123,14 +1123,6 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
 	}
 }
 
-#ifdef CONFIG_COMPAT
-static long tap_compat_ioctl(struct file *file, unsigned int cmd,
-			     unsigned long arg)
-{
-	return tap_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static const struct file_operations tap_fops = {
 	.owner		= THIS_MODULE,
 	.open		= tap_open,
@@ -1140,9 +1132,7 @@ static const struct file_operations tap_fops = {
 	.poll		= tap_poll,
 	.llseek		= no_llseek,
 	.unlocked_ioctl	= tap_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= tap_compat_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 };
 
 static int tap_get_user_xdp(struct tap_queue *q, struct xdp_buff *xdp)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index b2314636dc89..ab7dfc7c2917 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -935,16 +935,6 @@ pi433_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	return retval;
 }
 
-#ifdef CONFIG_COMPAT
-static long
-pi433_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
-{
-	return pi433_ioctl(filp, cmd, (unsigned long)compat_ptr(arg));
-}
-#else
-#define pi433_compat_ioctl NULL
-#endif /* CONFIG_COMPAT */
-
 /*-------------------------------------------------------------------------*/
 
 static int pi433_open(struct inode *inode, struct file *filp)
@@ -1101,7 +1091,7 @@ static const struct file_operations pi433_fops = {
 	.write =	pi433_write,
 	.read =		pi433_read,
 	.unlocked_ioctl = pi433_ioctl,
-	.compat_ioctl = pi433_compat_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.open =		pi433_open,
 	.release =	pi433_release,
 	.llseek =	no_llseek,
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index fa783531ee88..d75052b36584 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -2568,18 +2568,6 @@ static long usbdev_ioctl(struct file *file, unsigned int cmd,
 	return ret;
 }
 
-#ifdef CONFIG_COMPAT
-static long usbdev_compat_ioctl(struct file *file, unsigned int cmd,
-			unsigned long arg)
-{
-	int ret;
-
-	ret = usbdev_do_ioctl(file, cmd, compat_ptr(arg));
-
-	return ret;
-}
-#endif
-
 /* No kernel lock - fine */
 static __poll_t usbdev_poll(struct file *file,
 				struct poll_table_struct *wait)
@@ -2603,9 +2591,7 @@ const struct file_operations usbdev_file_operations = {
 	.read =		  usbdev_read,
 	.poll =		  usbdev_poll,
 	.unlocked_ioctl = usbdev_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl =   usbdev_compat_ioctl,
-#endif
+	.compat_ioctl =   compat_ptr_ioctl,
 	.mmap =           usbdev_mmap,
 	.open =		  usbdev_open,
 	.release =	  usbdev_release,
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index a3030cdf3c18..a5efe82584a5 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1200,15 +1200,6 @@ static long vfio_fops_unl_ioctl(struct file *filep,
 	return ret;
 }
 
-#ifdef CONFIG_COMPAT
-static long vfio_fops_compat_ioctl(struct file *filep,
-				   unsigned int cmd, unsigned long arg)
-{
-	arg = (unsigned long)compat_ptr(arg);
-	return vfio_fops_unl_ioctl(filep, cmd, arg);
-}
-#endif	/* CONFIG_COMPAT */
-
 static int vfio_fops_open(struct inode *inode, struct file *filep)
 {
 	struct vfio_container *container;
@@ -1291,9 +1282,7 @@ static const struct file_operations vfio_fops = {
 	.read		= vfio_fops_read,
 	.write		= vfio_fops_write,
 	.unlocked_ioctl	= vfio_fops_unl_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= vfio_fops_compat_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 	.mmap		= vfio_fops_mmap,
 };
 
@@ -1572,15 +1561,6 @@ static long vfio_group_fops_unl_ioctl(struct file *filep,
 	return ret;
 }
 
-#ifdef CONFIG_COMPAT
-static long vfio_group_fops_compat_ioctl(struct file *filep,
-					 unsigned int cmd, unsigned long arg)
-{
-	arg = (unsigned long)compat_ptr(arg);
-	return vfio_group_fops_unl_ioctl(filep, cmd, arg);
-}
-#endif	/* CONFIG_COMPAT */
-
 static int vfio_group_fops_open(struct inode *inode, struct file *filep)
 {
 	struct vfio_group *group;
@@ -1636,9 +1616,7 @@ static int vfio_group_fops_release(struct inode *inode, struct file *filep)
 static const struct file_operations vfio_group_fops = {
 	.owner		= THIS_MODULE,
 	.unlocked_ioctl	= vfio_group_fops_unl_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= vfio_group_fops_compat_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 	.open		= vfio_group_fops_open,
 	.release	= vfio_group_fops_release,
 };
@@ -1703,24 +1681,13 @@ static int vfio_device_fops_mmap(struct file *filep, struct vm_area_struct *vma)
 	return device->ops->mmap(device->device_data, vma);
 }
 
-#ifdef CONFIG_COMPAT
-static long vfio_device_fops_compat_ioctl(struct file *filep,
-					  unsigned int cmd, unsigned long arg)
-{
-	arg = (unsigned long)compat_ptr(arg);
-	return vfio_device_fops_unl_ioctl(filep, cmd, arg);
-}
-#endif	/* CONFIG_COMPAT */
-
 static const struct file_operations vfio_device_fops = {
 	.owner		= THIS_MODULE,
 	.release	= vfio_device_fops_release,
 	.read		= vfio_device_fops_read,
 	.write		= vfio_device_fops_write,
 	.unlocked_ioctl	= vfio_device_fops_unl_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= vfio_device_fops_compat_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 	.mmap		= vfio_device_fops_mmap,
 };
 
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index df51a35cf537..1642b3573230 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1765,14 +1765,6 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
 	}
 }
 
-#ifdef CONFIG_COMPAT
-static long vhost_net_compat_ioctl(struct file *f, unsigned int ioctl,
-				   unsigned long arg)
-{
-	return vhost_net_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static ssize_t vhost_net_chr_read_iter(struct kiocb *iocb, struct iov_iter *to)
 {
 	struct file *file = iocb->ki_filp;
@@ -1808,9 +1800,7 @@ static const struct file_operations vhost_net_fops = {
 	.write_iter     = vhost_net_chr_write_iter,
 	.poll           = vhost_net_chr_poll,
 	.unlocked_ioctl = vhost_net_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl   = vhost_net_compat_ioctl,
-#endif
+	.compat_ioctl   = compat_ptr_ioctl,
 	.open           = vhost_net_open,
 	.llseek		= noop_llseek,
 };
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 618fb6461017..f9b14c39d89b 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1721,21 +1721,11 @@ vhost_scsi_ioctl(struct file *f,
 	}
 }
 
-#ifdef CONFIG_COMPAT
-static long vhost_scsi_compat_ioctl(struct file *f, unsigned int ioctl,
-				unsigned long arg)
-{
-	return vhost_scsi_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static const struct file_operations vhost_scsi_fops = {
 	.owner          = THIS_MODULE,
 	.release        = vhost_scsi_release,
 	.unlocked_ioctl = vhost_scsi_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= vhost_scsi_compat_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 	.open           = vhost_scsi_open,
 	.llseek		= noop_llseek,
 };
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index 40589850eb33..61d4d98c8f70 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -298,21 +298,11 @@ static long vhost_test_ioctl(struct file *f, unsigned int ioctl,
 	}
 }
 
-#ifdef CONFIG_COMPAT
-static long vhost_test_compat_ioctl(struct file *f, unsigned int ioctl,
-				   unsigned long arg)
-{
-	return vhost_test_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static const struct file_operations vhost_test_fops = {
 	.owner          = THIS_MODULE,
 	.release        = vhost_test_release,
 	.unlocked_ioctl = vhost_test_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl   = vhost_test_compat_ioctl,
-#endif
+	.compat_ioctl   = compat_ptr_ioctl,
 	.open           = vhost_test_open,
 	.llseek		= noop_llseek,
 };
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index bb5fc0e9fbc2..9a86202678b6 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -716,23 +716,13 @@ static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
 	}
 }
 
-#ifdef CONFIG_COMPAT
-static long vhost_vsock_dev_compat_ioctl(struct file *f, unsigned int ioctl,
-					 unsigned long arg)
-{
-	return vhost_vsock_dev_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static const struct file_operations vhost_vsock_fops = {
 	.owner          = THIS_MODULE,
 	.open           = vhost_vsock_dev_open,
 	.release        = vhost_vsock_dev_release,
 	.llseek		= noop_llseek,
 	.unlocked_ioctl = vhost_vsock_dev_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl   = vhost_vsock_dev_compat_ioctl,
-#endif
+	.compat_ioctl   = compat_ptr_ioctl,
 };
 
 static struct miscdevice vhost_vsock_misc = {
diff --git a/fs/fat/file.c b/fs/fat/file.c
index b3bed32946b1..f173d9261115 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -171,15 +171,6 @@ long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	}
 }
 
-#ifdef CONFIG_COMPAT
-static long fat_generic_compat_ioctl(struct file *filp, unsigned int cmd,
-				      unsigned long arg)
-
-{
-	return fat_generic_ioctl(filp, cmd, (unsigned long)compat_ptr(arg));
-}
-#endif
-
 static int fat_file_release(struct inode *inode, struct file *filp)
 {
 	if ((filp->f_mode & FMODE_WRITE) &&
@@ -209,9 +200,7 @@ const struct file_operations fat_file_operations = {
 	.mmap		= generic_file_mmap,
 	.release	= fat_file_release,
 	.unlocked_ioctl	= fat_generic_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= fat_generic_compat_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 	.fsync		= fat_file_fsync,
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
-- 
2.20.0

^ permalink raw reply related

* [PATCH v3 00/26] compat_ioctl: cleanups
From: Arnd Bergmann @ 2019-04-16 20:19 UTC (permalink / raw)
  To: Alexander Viro
  Cc: linux-fsdevel, y2038, linux-kernel, Arnd Bergmann,
	David S. Miller, Greg Kroah-Hartman, Karsten Keil,
	James E.J. Bottomley, Martin K. Petersen, Marcel Holtmann, netdev,
	devel, linux-integrity, qat-linux, linux-crypto, linux-media,
	dri-devel, linux1394-devel, amd-gfx, linux-input, linux-usb,
	linux-arm-kernel, linux-ide, linux-iio

Hi Al,

It took me way longer than I had hoped to revisit this series, see
https://lore.kernel.org/lkml/20180912150142.157913-1-arnd@arndb.de/
for the previously posted version.

I've come to the point where all conversion handlers and most
COMPATIBLE_IOCTL() entries are gone from this file, but for
now, this series only has the parts that have either been reviewed
previously, or that are simple enough to include.

The main missing piece is the SG_IO/SG_GET_REQUEST_TABLE conversion.
I'll post the patches I made for that later, as they need more
testing and review from the scsi maintainers.

I hope you can still take these for the coming merge window, unless
new problems come up.

      Arnd

Arnd Bergmann (26):
  compat_ioctl: pppoe: fix PPPOEIOCSFWD handling
  compat_ioctl: move simple ppp command handling into driver
  compat_ioctl: avoid unused function warning for do_ioctl
  compat_ioctl: move PPPIOCSCOMPRESS32 to ppp-generic.c
  compat_ioctl: move PPPIOCSPASS32/PPPIOCSACTIVE32 to ppp_generic.c
  compat_ioctl: handle PPPIOCGIDLE for 64-bit time_t
  compat_ioctl: move rtc handling into rtc-dev.c
  compat_ioctl: add compat_ptr_ioctl()
  compat_ioctl: move drivers to compat_ptr_ioctl
  compat_ioctl: use correct compat_ptr() translation in drivers
  ceph: fix compat_ioctl for ceph_dir_operations
  compat_ioctl: move more drivers to compat_ptr_ioctl
  compat_ioctl: move tape handling into drivers
  compat_ioctl: move ATYFB_CLK handling to atyfb driver
  compat_ioctl: move isdn/capi ioctl translation into driver
  compat_ioctl: move rfcomm handlers into driver
  compat_ioctl: move hci_sock handlers into driver
  compat_ioctl: remove HCIUART handling
  compat_ioctl: remove HIDIO translation
  compat_ioctl: remove translation for sound ioctls
  compat_ioctl: remove IGNORE_IOCTL()
  compat_ioctl: remove /dev/random commands
  compat_ioctl: remove joystick ioctl translation
  compat_ioctl: remove PCI ioctl translation
  compat_ioctl: remove /dev/raw ioctl translation
  compat_ioctl: remove last RAID handling code

 Documentation/networking/ppp_generic.txt    |   2 +
 arch/um/drivers/hostaudio_kern.c            |   1 +
 drivers/android/binder.c                    |   2 +-
 drivers/char/ppdev.c                        |  12 +-
 drivers/char/random.c                       |   1 +
 drivers/char/tpm/tpm_vtpm_proxy.c           |  12 +-
 drivers/crypto/qat/qat_common/adf_ctl_drv.c |   2 +-
 drivers/dma-buf/dma-buf.c                   |   4 +-
 drivers/dma-buf/sw_sync.c                   |   2 +-
 drivers/dma-buf/sync_file.c                 |   2 +-
 drivers/firewire/core-cdev.c                |  12 +-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c    |   2 +-
 drivers/hid/hidraw.c                        |   4 +-
 drivers/hid/usbhid/hiddev.c                 |  11 +-
 drivers/hwtracing/stm/core.c                |  12 +-
 drivers/ide/ide-tape.c                      |  31 +-
 drivers/iio/industrialio-core.c             |   2 +-
 drivers/infiniband/core/uverbs_main.c       |   4 +-
 drivers/isdn/capi/capi.c                    |  31 +
 drivers/isdn/i4l/isdn_ppp.c                 |  14 +-
 drivers/media/rc/lirc_dev.c                 |   4 +-
 drivers/mfd/cros_ec_dev.c                   |   4 +-
 drivers/misc/cxl/flash.c                    |   8 +-
 drivers/misc/genwqe/card_dev.c              |  23 +-
 drivers/misc/mei/main.c                     |  22 +-
 drivers/misc/vmw_vmci/vmci_host.c           |   2 +-
 drivers/mtd/ubi/cdev.c                      |  36 +-
 drivers/net/ppp/ppp_generic.c               |  99 +++-
 drivers/net/ppp/pppoe.c                     |   7 +
 drivers/net/ppp/pptp.c                      |   3 +
 drivers/net/tap.c                           |  12 +-
 drivers/nvdimm/bus.c                        |   4 +-
 drivers/nvme/host/core.c                    |   2 +-
 drivers/pci/switch/switchtec.c              |   2 +-
 drivers/platform/x86/wmi.c                  |   2 +-
 drivers/rpmsg/rpmsg_char.c                  |   4 +-
 drivers/rtc/dev.c                           |  13 +-
 drivers/rtc/rtc-vr41xx.c                    |  10 +
 drivers/s390/char/tape_char.c               |  41 +-
 drivers/sbus/char/display7seg.c             |   2 +-
 drivers/sbus/char/envctrl.c                 |   4 +-
 drivers/scsi/3w-xxxx.c                      |   4 +-
 drivers/scsi/cxlflash/main.c                |   2 +-
 drivers/scsi/esas2r/esas2r_main.c           |   2 +-
 drivers/scsi/megaraid/megaraid_mm.c         |  28 +-
 drivers/scsi/osst.c                         |  34 +-
 drivers/scsi/pmcraid.c                      |   4 +-
 drivers/scsi/st.c                           |  35 +-
 drivers/staging/android/ion/ion.c           |   4 +-
 drivers/staging/pi433/pi433_if.c            |  12 +-
 drivers/staging/vme/devices/vme_user.c      |   2 +-
 drivers/tee/tee_core.c                      |   2 +-
 drivers/usb/class/cdc-wdm.c                 |   2 +-
 drivers/usb/class/usbtmc.c                  |   4 +-
 drivers/usb/core/devio.c                    |  16 +-
 drivers/usb/gadget/function/f_fs.c          |  12 +-
 drivers/vfio/vfio.c                         |  39 +-
 drivers/vhost/net.c                         |  12 +-
 drivers/vhost/scsi.c                        |  12 +-
 drivers/vhost/test.c                        |  12 +-
 drivers/vhost/vsock.c                       |  12 +-
 drivers/video/fbdev/aty/atyfb_base.c        |  12 +-
 drivers/virt/fsl_hypervisor.c               |   2 +-
 fs/btrfs/super.c                            |   2 +-
 fs/ceph/dir.c                               |   1 +
 fs/ceph/file.c                              |   2 +-
 fs/compat_ioctl.c                           | 602 +-------------------
 fs/fat/file.c                               |  13 +-
 fs/fuse/dev.c                               |   2 +-
 fs/notify/fanotify/fanotify_user.c          |   2 +-
 fs/userfaultfd.c                            |   2 +-
 include/linux/fs.h                          |   7 +
 include/linux/if_pppox.h                    |   2 +
 include/linux/mtio.h                        |  58 ++
 include/uapi/linux/ppp-ioctl.h              |   2 +
 include/uapi/linux/ppp_defs.h               |  14 +
 net/bluetooth/hci_sock.c                    |  21 +-
 net/bluetooth/rfcomm/sock.c                 |  14 +-
 net/l2tp/l2tp_ppp.c                         |   3 +
 net/rfkill/core.c                           |   2 +-
 sound/core/oss/pcm_oss.c                    |   4 +
 sound/oss/dmasound/dmasound_core.c          |   2 +
 82 files changed, 452 insertions(+), 1034 deletions(-)
 create mode 100644 include/linux/mtio.h

-- 
2.20.0

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Karsten Keil <isdn@linux-pingi.de>
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: devel@driverdev.osuosl.org
Cc: linux-integrity@vger.kernel.org
Cc: qat-linux@intel.com
Cc: linux-crypto@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux1394-devel@lists.sourceforge.net
Cc: amd-gfx@lists.freedesktop.org
Cc: linux-input@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-ide@vger.kernel.org
Cc: linux-iio@vger.kernel.org
Cc: linux-rdma@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mtd@lists.infradead.org
Cc: linux-ppp@vger.kernel.org
Cc: linux-nvme@lists.infradead.org
Cc: platform-driver-x86@vger.kernel.org
Cc: linux-remoteproc@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: linux-s390@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-btrfs@vger.kernel.org
Cc: ceph-devel@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-bluetooth@vger.kernel.org
Cc: linux-wireless@vger.kernel.org
Cc: alsa-devel@alsa-project.org
Cc: y2038@lists.linaro.org

^ permalink raw reply

* [PATCH 4/4] input: keyboard: gpio-keys-polled: skip oftree code when CONFIG_OF disabled
From: Enrico Weigelt, metux IT consult @ 2019-04-16 19:57 UTC (permalink / raw)
  To: linux-kernel; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <1555444645-15156-1-git-send-email-info@metux.net>

we don't need to build in oftree probing stuff when oftree isn't
enabled at all.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/input/keyboard/gpio_keys_polled.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 3f773b2..fbccb89 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -147,6 +147,7 @@ static void gpio_keys_polled_close(struct input_polled_dev *dev)
 static struct gpio_keys_platform_data *
 gpio_keys_polled_get_devtree_pdata(struct device *dev)
 {
+#ifdef CONFIG_OF
 	struct gpio_keys_platform_data *pdata;
 	struct gpio_keys_button *button;
 	struct fwnode_handle *child;
@@ -200,6 +201,9 @@ static void gpio_keys_polled_close(struct input_polled_dev *dev)
 	}
 
 	return pdata;
+#else /* CONFIG_OF */
+	return ERR_PTR(-ENOENT);
+#endif /* CONFIG_OF */
 }
 
 static void gpio_keys_polled_set_abs_params(struct input_dev *input,
@@ -226,7 +230,7 @@ static void gpio_keys_polled_set_abs_params(struct input_dev *input,
 	{ .compatible = "gpio-keys-polled", },
 	{ },
 };
-MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);
+MODULE_DEVICE_TABLE_OF(gpio_keys_polled_of_match);
 
 static struct gpio_desc *gpio_keys_polled_get_gpiod_fwnode(
 	struct device *dev,
@@ -452,7 +456,9 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 	.probe	= gpio_keys_polled_probe,
 	.driver	= {
 		.name	= DRV_NAME,
+#ifdef CONFIG_OF
 		.of_match_table = gpio_keys_polled_of_match,
+#endif /* CONFIG_OF */
 	},
 };
 module_platform_driver(gpio_keys_polled_driver);
-- 
1.9.1

^ permalink raw reply related

* [PATCH 3/4] input: keyboard: gpio_keys_polled: use gpio lookup table
From: Enrico Weigelt, metux IT consult @ 2019-04-16 19:57 UTC (permalink / raw)
  To: linux-kernel; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <1555444645-15156-1-git-send-email-info@metux.net>

Support the recently introduced gpio lookup tables for
attaching to gpio lines. So, harcoded gpio numbers aren't
needed anymore.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/input/keyboard/gpio_keys_polled.c | 167 +++++++++++++++++++++---------
 1 file changed, 119 insertions(+), 48 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 3312186..3f773b2 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -24,6 +24,7 @@
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
 #include <linux/gpio_keys.h>
 #include <linux/property.h>
 
@@ -227,6 +228,119 @@ static void gpio_keys_polled_set_abs_params(struct input_dev *input,
 };
 MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);
 
+static struct gpio_desc *gpio_keys_polled_get_gpiod_fwnode(
+	struct device *dev,
+	int idx,
+	const char *desc)
+{
+	struct gpio_desc *gpiod;
+	struct fwnode_handle *child;
+	int x;
+
+	/* get the idx'th child node */
+	child = device_get_next_child_node(dev, NULL);
+	while (child && x) {
+		child = device_get_next_child_node(dev, child);
+		x--;
+	}
+
+	if (!child) {
+		dev_err(dev, "missing oftree child node #%d\n", idx);
+		return ERR_PTR(-EINVAL);
+	}
+
+	gpiod = devm_fwnode_get_gpiod_from_child(dev,
+						 NULL,
+						 child,
+						 GPIOD_IN,
+						 desc);
+	if (IS_ERR(gpiod)) {
+		if (PTR_ERR(gpiod) != -EPROBE_DEFER)
+			dev_err(dev,
+				"failed to get gpio: %ld\n",
+				PTR_ERR(gpiod));
+		fwnode_handle_put(child);
+		return gpiod;
+	}
+
+	return gpiod;
+}
+
+static struct gpio_desc *gpio_keys_polled_get_gpiod_legacy(
+	struct device *dev,
+	int idx,
+	const struct gpio_keys_button *button)
+{
+	/*
+	 * Legacy GPIO number so request the GPIO here and
+	 * convert it to descriptor.
+	 */
+	unsigned int flags = GPIOF_IN;
+	struct gpio_desc *gpiod;
+	int error;
+
+	dev_info(dev, "hardcoded gpio IDs are deprecated.\n");
+
+	if (button->active_low)
+		flags |= GPIOF_ACTIVE_LOW;
+
+	error = devm_gpio_request_one(dev, button->gpio,
+			flags, button->desc ? : DRV_NAME);
+	if (error) {
+		dev_err(dev,
+			"unable to claim gpio %u, err=%d\n",
+			button->gpio, error);
+		return ERR_PTR(error);
+	}
+
+	gpiod = gpio_to_desc(button->gpio);
+	if (!gpiod) {
+		dev_err(dev,
+			"unable to convert gpio %u to descriptor\n",
+			button->gpio);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return gpiod;
+}
+
+static struct gpio_desc *gpio_keys_polled_get_gpiod(
+	struct device *dev,
+	int idx,
+	const struct gpio_keys_button *button)
+{
+	struct gpio_desc *gpiod = NULL;
+	int error;
+
+	/* No legacy static platform data - use oftree */
+	if (!dev_get_platdata(dev)) {
+		return gpio_keys_polled_get_gpiod_fwnode(
+			dev, idx, button->desc);
+	}
+
+	gpiod = devm_gpiod_get_index(dev, NULL, idx, GPIOF_IN);
+
+	if (!IS_ERR(gpiod)) {
+		dev_info(dev, "picked gpiod idx %d from gpio table\n", idx);
+		gpiod_set_consumer_name(gpiod, button->desc ? : DRV_NAME);
+		return gpiod;
+	}
+
+	if (PTR_ERR(gpiod) != -ENOENT) {
+		dev_err(dev, "failed fetching gpiod #%d: %d\n",
+			idx, PTR_ERR(gpiod));
+		return gpiod;
+	}
+
+	/* Use legacy gpio id, if defined */
+	if (gpio_is_valid(button->gpio)) {
+		return gpio_keys_polled_get_gpiod_legacy(
+			dev, idx, button);
+	}
+
+	return ERR_PTR(-ENOENT);
+}
+
 static int gpio_keys_polled_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -291,57 +405,14 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 
 		if (button->wakeup) {
 			dev_err(dev, DRV_NAME " does not support wakeup\n");
-			fwnode_handle_put(child);
 			return -EINVAL;
 		}
 
-		if (!dev_get_platdata(dev)) {
-			/* No legacy static platform data */
-			child = device_get_next_child_node(dev, child);
-			if (!child) {
-				dev_err(dev, "missing child device node\n");
-				return -EINVAL;
-			}
-
-			bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev,
-								NULL, child,
-								GPIOD_IN,
-								button->desc);
-			if (IS_ERR(bdata->gpiod)) {
-				error = PTR_ERR(bdata->gpiod);
-				if (error != -EPROBE_DEFER)
-					dev_err(dev,
-						"failed to get gpio: %d\n",
-						error);
-				fwnode_handle_put(child);
-				return error;
-			}
-		} else if (gpio_is_valid(button->gpio)) {
-			/*
-			 * Legacy GPIO number so request the GPIO here and
-			 * convert it to descriptor.
-			 */
-			unsigned flags = GPIOF_IN;
-
-			if (button->active_low)
-				flags |= GPIOF_ACTIVE_LOW;
-
-			error = devm_gpio_request_one(dev, button->gpio,
-					flags, button->desc ? : DRV_NAME);
-			if (error) {
-				dev_err(dev,
-					"unable to claim gpio %u, err=%d\n",
-					button->gpio, error);
-				return error;
-			}
-
-			bdata->gpiod = gpio_to_desc(button->gpio);
-			if (!bdata->gpiod) {
-				dev_err(dev,
-					"unable to convert gpio %u to descriptor\n",
-					button->gpio);
-				return -EINVAL;
-			}
+		bdata->gpiod = gpio_keys_polled_get_gpiod(dev, i, button);
+
+		if (IS_ERR(bdata->gpiod)) {
+			dev_err(dev, "failed to fetch gpiod #%d\n", i);
+			return PTR_ERR(bdata->gpiod);
 		}
 
 		bdata->last_state = -1;
-- 
1.9.1

^ permalink raw reply related

* [PATCH 2/4] input: keyboard: gpio-keys-polled: use input name from pdata if available
From: Enrico Weigelt, metux IT consult @ 2019-04-16 19:57 UTC (permalink / raw)
  To: linux-kernel; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <1555444645-15156-1-git-send-email-info@metux.net>

Instead of hardcoding the input name to the driver name
('gpio-keys-polled'), allow the passing a name via platform data
('name' field was already present), but default to old behaviour
in case of NULL.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/input/keyboard/gpio_keys_polled.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index edc7262..3312186 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -272,7 +272,7 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 
 	input = poll_dev->input;
 
-	input->name = pdev->name;
+	input->name = (pdata->name ? pdata->name : pdev->name);
 	input->phys = DRV_NAME"/input0";
 
 	input->id.bustype = BUS_HOST;
-- 
1.9.1

^ permalink raw reply related

* [PATCH 1/4] mod_devicetable: helper macro for declaring oftree module device table
From: Enrico Weigelt, metux IT consult @ 2019-04-16 19:57 UTC (permalink / raw)
  To: linux-kernel; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <1555444645-15156-1-git-send-email-info@metux.net>

Little helper macro that declares an oftree module device table,
if CONFIG_OF is enabled. Otherwise it's just noop.

This is also helpful if some drivers can be built w/ or w/o
oftree support.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 include/linux/mod_devicetable.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 448621c..853e301 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -245,6 +245,15 @@ struct of_device_id {
 	const void *data;
 };
 
+/*
+ * macro for adding the of module device table only if CONFIG_OF enabled
+ */
+#ifdef CONFIG_OF
+#define MODULE_DEVICE_TABLE_OF(name)	MODULE_DEVICE_TABLE(of,name)
+#else
+#define MODULE_DEVICE_TABLE_OF(name)
+#endif /* CONFIG_OF */
+
 /* VIO */
 struct vio_device_id {
 	char type[32];
-- 
1.9.1

^ permalink raw reply related

* gpio-keys-polled improvements
From: Enrico Weigelt, metux IT consult @ 2019-04-16 19:57 UTC (permalink / raw)
  To: linux-kernel; +Cc: dmitry.torokhov, linux-input

Hello folks,


here're some improvements for the gpio-keys-polled driver.

The first patch isn't in the driver itself, but adds a little module helper
for conditionally declaring oftree module table if oftree is enabled
(only needed for the last patch)


have fun.

--mtx

^ permalink raw reply

* Re: [RFC v3] iio: input-bridge: optionally bridge iio acceleometers to create a /dev/input interface
From: H. Nikolaus Schaller @ 2019-04-16 19:33 UTC (permalink / raw)
  To: Bastien Nocera
  Cc: Jonathan Cameron, Dmitry Torokhov, Eric Piel, linux-input,
	letux-kernel, kernel, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, linux-kernel, linux-iio
In-Reply-To: <0189da8d91652dd3ecf729b03029ab9db5a24f99.camel@hadess.net>

Hi Bastien,

> Am 16.04.2019 um 18:04 schrieb Bastien Nocera <hadess@hadess.net>:
> 
> Having written a "bridge" myself (I called it a "proxy"[1]), I have a
> few comments.
> 
> [1]: https://github.com/hadess/iio-sensor-proxy

Nice work!

> 
> Let's start with the easy ones ;) there's a typo in the subject line.
> 
> The subject line also says "optionally" but there doesn't seem to be
> any ways to disable the feature if it's shipped by the kernel used.

Well, the "optionally" refers to that this can be completely disabled
by a Kconfig option. Maybe it is the wrong wording for this and should
be changed.

> 
> On Mon, 2019-04-15 at 23:05 +0200, H. Nikolaus Schaller wrote:
>> Some user spaces (e.g. some Android devices) use /dev/input/event*
>> for handling
>> the 3D position of the device with respect to the center of gravity
>> (earth).
>> This can be used for gaming input, auto-rotation of screens etc.
>> 
>> This interface should be the standard for such use cases because it
>> is an abstraction
>> of how orientation data is acquired from sensor chips. Sensor chips
>> may be connected
>> through different interfaces and in different positions. They may
>> also have different
>> parameters. And, if a chip is replaced by a different one, the values
>> reported by
>> the device position interface should remain the same, provided the
>> device tree reflects
>> the changed chip.
> 
> I don't understand this section of the commit message. The IIO drivers
> are already that abstraction interface, no?

IIO is also some abstraction but a different one than input accelerometers.

IIO reports physical measurement data in some standardized way reporting value,
scale, units, type of measurement. But this has no inherent purpose.

Accelerator input events are something different. They report the orientation
of a handheld device in space relative to center of earth. They may be implemented
through iio drivers but do not need to.

You will find several non-iio accelerometer drivers in drivers/input/misc and
elsewhere.

> 
>> This did initially lead to input accelerometer drivers like
>> drivers/input/misc/bma150.c
>> or drivers/misc/lis3lv02d/
>> 
>> But nowadays, new accelerometer chips mostly get iio drivers and
>> rarely input drivers.
>> 
>> Therefore we need something like a protocol stack which bridges raw
>> data and input devices.
>> It can be seen as a similar layering like TCP/IP vs. bare Ethernet.
>> Or keyboard
>> input events vs. raw gpio or raw USB access.
> 
> This can be done in user-space, reading the data from the IIO driver,
> and using uinput to feed it back. Why is doing this at the kernel level
> better?

Well, I'd estimate that >80% of the current kernel could be done in user-space
(but not at the same speed/quality).

E.g. TCP could most likely be done by directly accessing the Ethernet layer and
providing other processes access through named pipes instead of sockets.

But usually a user-space daemon feeding things back into the kernel is slower
(because it is scheduled differently) and needs more resources for running the
process and IPC and is less protected against hickups and deadlocks.

Two more aspects come to my mind from reading your project page:

a) "It requires libgudev and systemd"
b) "Note that a number of kernel bugs will prevent it from working correctly"

a) this makes quite significant assumptions about the user-space while a kernel
   driver can be kept independent of this
b) if it is in-kernel it will be kept in sync with kernel changes and such bugs
   are less likely

> 
>> This patch bridges the gap between raw iio data and the input device
>> abstraction
>> so that accelerometer measurements can additionally be presented as
>> X/Y/Z accelerometer
>> channels (INPUT_PROP_ACCELEROMETER) through /dev/input/event*.
>> 
>> There are no special requirements or changes needed for an iio
>> driver.
> 
> The user-space daemon I wrote supports both IIO drivers and input
> drivers for accelerometers. How do I know from user-space whether a
> device is proxied or not?

Since my proposal does not stop direct iio access, I assume that your
daemon will simply continue to work as is.

> 
>> There is no need to define a mapping (e.g. in device tree).
>> 
>> This driver simply collects the first 3 accelerometer channels as X,
>> Y and Z.
>> If only 1 or 2 channels are available, they are used for X and Y
>> only. Additional
>> channels are ignored.
> 
> In what cases are 2 dimensional accelerometers used?

I don't know. This is just a description that there will be a graceful
behavior, if some accelerometer has less or more than 3 channels. So
the driver will segfault or panic the kernel...

> 
>> Scaling is done automatically so that 1g is represented by value 256
>> and
>> range is assumed to be -511 .. +511 which gives a reasonable
>> precision as an
>> input device.
>> 
>> If a mount-matrix is provided by the iio driver, it is also taken
>> into account
>> so that the input event automatically gets the correct orientation
>> with respect
>> to the device.
>> 
>> If this extension is not configured into the kernel it takes no
>> resources (except
>> source code).
>> 
>> If it is configured, but there is no accelerometer, there is only a
>> tiny penalty
>> for scanning for accelerometer channels once during probe of each iio
>> device.
>> 
>> If it runs, the driver polls the device(s) once every 100 ms. A mode
>> where the
>> iio device defines the update rate is not implemented and for further
>> study.
>> 
>> If there is no user-space client, polling is not running.
> 
> Is the bridge going to modify the IIO device's settings behind other
> possible consumer's backs, such as threshold values, and triggers?

No. The bridge only transforms values.

For other parameters it takes what the iio device has been initialized to
Or if they are changed dynamically through the iio API, it uses new parameters.

> 
>> The driver is capable to handle multiple iio accelerometers and they
>> are
>> presented by unique /dev/input/event* files. The iio chip name is
>> used to define
>> the input device name so that it can be identified (e.g. by udev
>> rules or evtest).
> 
> As you can probably guess, I'm not overly enthusiastic about this piece
> of code. If it had existed 5 years ago, I probably wouldn't have
> written iio-sensor-proxy, but then somebody else would have had to for
> the rest of the IIO sensors that can be consumed.

I have looked into your work and get the impression that my proposal is
not contradicting your work.

You still need mechanisms to rotate e.g. the display if the device is
rotated. This is not done by this kernel code.

This is a similar situation that a keyboard driver reports key event
codes but does not draw glyphs.

The bridge provides a different (and well established) higher-level API
for accelerometers than iio.

So you may check if it simplifies your code by using this higher-layer interface
(input event). From my rough check it appears to me that you can for example
remove the mount-matrix handling from your code because it would be done by
this bridge.

> To me, this bridge has all the drawbacks of a simple user-space
> implementation using uinput, without much of the benefits of being an
> exclusive user of the IIO accelerometers, such as being able to change
> the update rate, or using triggers depending on the usage.
> 
> What am I missing? Why shouldn't this live in user-space?

That's a pretty biased question...

It's a matter of philosophy whether you want a microkernel + user-space daemons
or a kernel that prebakes many things to make user-space daemons easier or
even superfluous.

In the other extreme you could even get rid of iio and directly access the
sensors through /dev/i2c from user-space daemon.

So I can't give you a technical answer for that and since I am not a
maintainer I can't answer it at all.

BR and thanks,
Nikolaus

^ permalink raw reply

* Re: [RFC v3] iio: input-bridge: optionally bridge iio acceleometers to create a /dev/input interface
From: Bastien Nocera @ 2019-04-16 16:04 UTC (permalink / raw)
  To: H. Nikolaus Schaller, Jonathan Cameron, Dmitry Torokhov
  Cc: Eric Piel, linux-input, letux-kernel, kernel, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, linux-kernel,
	linux-iio
In-Reply-To: <d52cf9ee5944c90c69f6e74a46d844cef51e487e.1555362312.git.hns@goldelico.com>

Having written a "bridge" myself (I called it a "proxy"[1]), I have a
few comments.

[1]: https://github.com/hadess/iio-sensor-proxy

Let's start with the easy ones ;) there's a typo in the subject line.

The subject line also says "optionally" but there doesn't seem to be
any ways to disable the feature if it's shipped by the kernel used.

On Mon, 2019-04-15 at 23:05 +0200, H. Nikolaus Schaller wrote:
> Some user spaces (e.g. some Android devices) use /dev/input/event*
> for handling
> the 3D position of the device with respect to the center of gravity
> (earth).
> This can be used for gaming input, auto-rotation of screens etc.
> 
> This interface should be the standard for such use cases because it
> is an abstraction
> of how orientation data is acquired from sensor chips. Sensor chips
> may be connected
> through different interfaces and in different positions. They may
> also have different
> parameters. And, if a chip is replaced by a different one, the values
> reported by
> the device position interface should remain the same, provided the
> device tree reflects
> the changed chip.

I don't understand this section of the commit message. The IIO drivers
are already that abstraction interface, no?

> This did initially lead to input accelerometer drivers like
> drivers/input/misc/bma150.c
> or drivers/misc/lis3lv02d/
> 
> But nowadays, new accelerometer chips mostly get iio drivers and
> rarely input drivers.
> 
> Therefore we need something like a protocol stack which bridges raw
> data and input devices.
> It can be seen as a similar layering like TCP/IP vs. bare Ethernet.
> Or keyboard
> input events vs. raw gpio or raw USB access.

This can be done in user-space, reading the data from the IIO driver,
and using uinput to feed it back. Why is doing this at the kernel level
better?

> This patch bridges the gap between raw iio data and the input device
> abstraction
> so that accelerometer measurements can additionally be presented as
> X/Y/Z accelerometer
> channels (INPUT_PROP_ACCELEROMETER) through /dev/input/event*.
> 
> There are no special requirements or changes needed for an iio
> driver.

The user-space daemon I wrote supports both IIO drivers and input
drivers for accelerometers. How do I know from user-space whether a
device is proxied or not?

> There is no need to define a mapping (e.g. in device tree).
> 
> This driver simply collects the first 3 accelerometer channels as X,
> Y and Z.
> If only 1 or 2 channels are available, they are used for X and Y
> only. Additional
> channels are ignored.

In what cases are 2 dimensional accelerometers used?

> Scaling is done automatically so that 1g is represented by value 256
> and
> range is assumed to be -511 .. +511 which gives a reasonable
> precision as an
> input device.
> 
> If a mount-matrix is provided by the iio driver, it is also taken
> into account
> so that the input event automatically gets the correct orientation
> with respect
> to the device.
>
> If this extension is not configured into the kernel it takes no
> resources (except
> source code).
> 
> If it is configured, but there is no accelerometer, there is only a
> tiny penalty
> for scanning for accelerometer channels once during probe of each iio
> device.
> 
> If it runs, the driver polls the device(s) once every 100 ms. A mode
> where the
> iio device defines the update rate is not implemented and for further
> study.
> 
> If there is no user-space client, polling is not running.

Is the bridge going to modify the IIO device's settings behind other
possible consumer's backs, such as threshold values, and triggers?

> The driver is capable to handle multiple iio accelerometers and they
> are
> presented by unique /dev/input/event* files. The iio chip name is
> used to define
> the input device name so that it can be identified (e.g. by udev
> rules or evtest).

As you can probably guess, I'm not overly enthusiastic about this piece
of code. If it had existed 5 years ago, I probably wouldn't have
written iio-sensor-proxy, but then somebody else would have had to for
the rest of the IIO sensors that can be consumed.

To me, this bridge has all the drawbacks of a simple user-space
implementation using uinput, without much of the benefits of being an
exclusive user of the IIO accelerometers, such as being able to change
the update rate, or using triggers depending on the usage.

What am I missing? Why shouldn't this live in user-space?

Cheers

^ permalink raw reply


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