cron job: media_tree daily build: ERRORS

[Hans Verkuil]

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Tue May  2 05:00:19 CEST 2017
media-tree git hash:3622d3e77ecef090b5111e3c5423313f11711dfa
media_build git hash:   1af19680bde3e227d64d99ff5fdc43eb343a3b28
v4l-utils git hash: 847bf8d62cd6b11defc1e4c3b30b68d3c66876e0
gcc version:i686-linux-gcc (GCC) 6.3.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.9.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.14.9-i686: WARNINGS
linux-3.15.2-i686: WARNINGS
linux-3.16.7-i686: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9-i686: OK
linux-4.10.1-i686: OK
linux-4.11-rc1-i686: OK
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9-x86_64: WARNINGS
linux-4.10.1-x86_64: WARNINGS
linux-4.11-rc1-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html

Re: [PATCH 1/2] em28xx: allow setting the eeprom bus at cards struct

[Mauro Carvalho Chehab]

Hi Frank,

Em Mon, 1 May 2017 16:11:51 +0200
Frank Schäfer  escreveu:

> Am 01.05.2017 um 13:38 schrieb Mauro Carvalho Chehab:
> > Right now, all devices use bus 0 for eeprom. However, newer
> > versions of Terratec H6 use a different buffer for eeprom.
> >
> > So, add support to use a different I2C address for eeprom.  
> 
> Has this been tested ?
> Did you read my reply to the previous patch version ?:
> See http://www.spinics.net/lists/linux-media/msg114860.html
> 
> I doubt it will work. At least not for the device from the thread in the
> Kodi-forum.

Yes. Someone at IRC were complaining about this device (his nick is
buxy81). According with the tests he did, with both patches his
device is now working.

That's said, it would be great if he could provide us more details
about the tests he did, with the logs enabled, in order for us to see
if the eeprom contents is properly read.


Thanks,
Mauro

Re: [PATCH 2/6] rc-core: cleanup rc_register_device

[David Härdeman]

On Mon, May 01, 2017 at 05:49:53PM +0100, Sean Young wrote:
>On Thu, Apr 27, 2017 at 10:34:03PM +0200, David Härdeman wrote:
>> The device core infrastructure is based on the presumption that
>> once a driver calls device_add(), it must be ready to accept
>> userspace interaction.
>> 
>> This requires splitting rc_setup_rx_device() into two functions
>> and reorganizing rc_register_device() so that as much work
>> as possible is performed before calling device_add().
>> 
>
>With this patch applied, I'm no longer getting any scancodes from
>my rc devices.
>
>David, please can you test your patches before submitting. I have to go
>over them meticulously because I cannot assume you've tested them.

I did test this patch and I just redid the tests, both with rc-loopback
and with a mceusb receiver. I'm seeing scancodes on the input device as
well as pulse-space readings on the lirc device in both tests.

I did the tests with only this patch applied and the lirc-use-after-free
(v3). What hardware did you test with?

Meanwhile, I'll try rebasing my patches to the latest version of the
media-master git tree and test again.

-- 
David Härdeman

Re: [PATCH v2 3/3] libv4l-codecparsers: add GStreamer mpeg2 parser

[Nicolas Dufresne]

Le vendredi 28 avril 2017 à 17:02 +0200, Hugues Fruchet a écrit :
> Add the mpeg2 codecparser backend glue which will
> call the GStreamer parsing functions.
> 
> Signed-off-by: Hugues Fruchet 
> ---
>  configure.ac|  21 ++
>  lib/libv4l-codecparsers/Makefile.am |  14 +-
>  lib/libv4l-codecparsers/libv4l-cparsers-mpeg2.c | 375
> 
>  lib/libv4l-codecparsers/libv4l-cparsers.c   |   4 +
>  4 files changed, 413 insertions(+), 1 deletion(-)
>  create mode 100644 lib/libv4l-codecparsers/libv4l-cparsers-mpeg2.c
> 
> diff --git a/configure.ac b/configure.ac
> index 9ce7392..ce43f18 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -273,6 +273,25 @@ fi
>  
>  AC_SUBST([JPEG_LIBS])
>  
> +# Check for GStreamer codecparsers
> +
> +gst_codecparsers_pkgconfig=false
> +PKG_CHECK_MODULES([GST], [gstreamer-1.0 >= 1.8.0],
> [gst_pkgconfig=true], [gst_pkgconfig=false])
> +if test "x$gst_pkgconfig" = "xfalse"; then
> +   AC_MSG_WARN(GStreamer library is not available)
> +else
> +   PKG_CHECK_MODULES([GST_BASE], [gstreamer-base-1.0 >= 1.8.0],
> [gst_base_pkgconfig=true], [gst_base_pkgconfig=false])
> +   if test "x$gst_base_pkgconfig" = "xfalse"; then
> +  AC_MSG_WARN(GStreamer base library is not available)
> +   else
> +  PKG_CHECK_MODULES(GST_CODEC_PARSERS, [gstreamer-codecparsers-
> 1.0 >= 1.8.0], [gst_codecparsers_pkgconfig=true], 

You should only check for the codecparser library. The rest are
dependencies which will be pulled automatically by pkg-config. If for
some reason you needed multiple libs, that don't depend on each others,
notice the S in PKG_CHECK_MODULES. You can do a single invocation.

> [gst_codecparsers_pkgconfig=false])
> +  if test "x$gst_codecparsers_pkgconfig" = "xfalse"; then
> + AC_MSG_WARN(GStreamer codecparser library is not available)
> +  fi
> +   fi
> +fi
> +AM_CONDITIONAL([HAVE_GST_CODEC_PARSERS], [test
> x$gst_codecparsers_pkgconfig = xtrue])
> +
>  # Check for pthread
>  
>  AS_IF([test x$enable_shared != xno],
> @@ -477,6 +496,7 @@ AM_COND_IF([WITH_V4L2_CTL_LIBV4L],
> [USE_V4L2_CTL="yes"], [USE_V4L2_CTL="no"])
>  AM_COND_IF([WITH_V4L2_CTL_STREAM_TO], [USE_V4L2_CTL="yes"],
> [USE_V4L2_CTL="no"])
>  AM_COND_IF([WITH_V4L2_COMPLIANCE_LIBV4L],
> [USE_V4L2_COMPLIANCE="yes"], [USE_V4L2_COMPLIANCE="no"])
>  AS_IF([test "x$alsa_pkgconfig" = "xtrue"], [USE_ALSA="yes"],
> [USE_ALSA="no"])
> +AS_IF([test "x$gst_codecparsers_pkgconfig" = "xtrue"],
> [USE_GST_CODECPARSERS="yes"], [USE_GST_CODECPARSERS="no"])
>  
>  AC_OUTPUT
>  
> @@ -497,6 +517,7 @@ compile time options summary
>  pthread : $have_pthread
>  QT version  : $QT_VERSION
>  ALSA support: $USE_ALSA
> +GST codecparsers: $USE_GST_CODECPARSERS
>  
>  build dynamic libs  : $enable_shared
>  build static libs   : $enable_static
> diff --git a/lib/libv4l-codecparsers/Makefile.am b/lib/libv4l-
> codecparsers/Makefile.am
> index a9d6c8b..61f4730 100644
> --- a/lib/libv4l-codecparsers/Makefile.am
> +++ b/lib/libv4l-codecparsers/Makefile.am
> @@ -1,9 +1,21 @@
>  if WITH_V4L_PLUGINS
> +if HAVE_GST_CODEC_PARSERS
> +
>  libv4l2plugin_LTLIBRARIES = libv4l-codecparsers.la
> -endif
>  
>  libv4l_codecparsers_la_SOURCES = libv4l-cparsers.c libv4l-cparsers.h
>  
>  libv4l_codecparsers_la_CPPFLAGS = $(CFLAG_VISIBILITY)
> -I$(top_srcdir)/lib/libv4l2/ -I$(top_srcdir)/lib/libv4lconvert/
>  libv4l_codecparsers_la_LDFLAGS = -avoid-version -module -shared
> -export-dynamic -lpthread
>  libv4l_codecparsers_la_LIBADD = ../libv4l2/libv4l2.la
> +
> +# GStreamer codecparsers library
> +libv4l_codecparsers_la_CFLAGS = $(GST_CFLAGS) -DGST_USE_UNSTABLE_API
> +libv4l_codecparsers_la_LDFLAGS += $(GST_LIB_LDFLAGS)
> +libv4l_codecparsers_la_LIBADD += $(GLIB_LIBS) $(GST_LIBS)
> $(GST_BASE_LIBS) $(GST_CODEC_PARSERS_LIBS) $(NULL)
> +
> +# MPEG-2 parser back-end
> +libv4l_codecparsers_la_SOURCES += libv4l-cparsers-mpeg2.c
> +
> +endif
> +endif
> diff --git a/lib/libv4l-codecparsers/libv4l-cparsers-mpeg2.c
> b/lib/libv4l-codecparsers/libv4l-cparsers-mpeg2.c
> new file mode 100644
> index 000..3456b73
> --- /dev/null
> +++ b/lib/libv4l-codecparsers/libv4l-cparsers-mpeg2.c
> @@ -0,0 +1,375 @@
> +/*
> + * libv4l-cparsers-mpeg2.c
> + *
> + * Copyright (C) STMicroelectronics SA 2017
> + * Authors: Hugues Fruchet 
> + *  Tifaine Inguere 
> + *  for STMicroelectronics.
> + *
> + * This program is free software; you can redistribute it and/or
> modify
> + * it under the terms of the GNU Lesser General Public License as
> published by
> + * the Free Software Foundation; either version 2.1 of the License,
> or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR 

Re: [PATCH] [RFC] rc-core: report protocol information to userspace

[Sean Young]

On Mon, May 01, 2017 at 02:49:57PM +0200, David Härdeman wrote:
> On Mon, May 01, 2017 at 11:38:30AM +0100, Sean Young wrote:
> >On Sat, Apr 29, 2017 at 12:52:12PM +0200, David Härdeman wrote:
> >> Whether we decide to go for any new keytable ioctl():s or not in rc-core, 
> >> we
> >> should provide the protocol information of keypresses to userspace.
> >> 
> >> Note that this means that the RC_TYPE_* definitions become part of the
> >> userspace <-> kernel API/ABI (meaning a full patch should maybe move those
> >> defines under include/uapi).
> >> 
> >> This would also need to be ack:ed by the input maintainers.
> >
> >This was already NACKed in the past.
> >
> >http://www.spinics.net/lists/linux-input/msg46941.html
> >
> 
> Didn't know that, thanks for the pointer. I still think we should
> revisit this though. Even if we don't add protocol-aware EVIOC[SG]KEY_V2
> ioctls, that information is useful for a configuration tool when
> creating keymaps for a new remote.
> 
> And examining the parent hardware device (as Dmitry seemed to suggest)
> doesn't help with protocol identification.
> 
> Another option if we don't want to touch the input layer would be to
> export the last_* members from struct rc_dev in sysfs (and I'm guessing
> a timestamp would be necessary then). Seems like a lot of work to
> accomplish what would otherwise be a one-line change in the input layer
> though (one-line since I'm assuming we could provide the protocol
> defines in a separate header, other than input-event-codes.h as the
> protocols are subsystem-specific).

So I have some patches for reading and writing scancodes, which will
give you a u64 scancode, protocol and other bits of information. This
can also be used for transmit where the IR encoders will be used.

http://www.spinics.net/lists/linux-media/msg109836.html

I wanted to make sure that these patches are also sufficient for sending
scancodes for the lirc_zilog driver before merging, which is what I'm
working on right now.


Sean

Re: [PATCH 2/6] rc-core: cleanup rc_register_device

[Sean Young]

On Thu, Apr 27, 2017 at 10:34:03PM +0200, David Härdeman wrote:
> The device core infrastructure is based on the presumption that
> once a driver calls device_add(), it must be ready to accept
> userspace interaction.
> 
> This requires splitting rc_setup_rx_device() into two functions
> and reorganizing rc_register_device() so that as much work
> as possible is performed before calling device_add().
> 

With this patch applied, I'm no longer getting any scancodes from
my rc devices.

David, please can you test your patches before submitting. I have to go
over them meticulously because I cannot assume you've tested them.

Thanks
Sean

> Signed-off-by: David Härdeman 
> ---
>  drivers/media/rc/rc-core-priv.h |2 +
>  drivers/media/rc/rc-ir-raw.c|   34 --
>  drivers/media/rc/rc-main.c  |   75 
> +--
>  3 files changed, 73 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
> index 0455b273c2fc..b3e7cac2c3ee 100644
> --- a/drivers/media/rc/rc-core-priv.h
> +++ b/drivers/media/rc/rc-core-priv.h
> @@ -263,7 +263,9 @@ int ir_raw_gen_pl(struct ir_raw_event **ev, unsigned int 
> max,
>   * Routines from rc-raw.c to be used internally and by decoders
>   */
>  u64 ir_raw_get_allowed_protocols(void);
> +int ir_raw_event_prepare(struct rc_dev *dev);
>  int ir_raw_event_register(struct rc_dev *dev);
> +void ir_raw_event_free(struct rc_dev *dev);
>  void ir_raw_event_unregister(struct rc_dev *dev);
>  int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler);
>  void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler);
> diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
> index 90f66dc7c0d7..ae7785c4fbe7 100644
> --- a/drivers/media/rc/rc-ir-raw.c
> +++ b/drivers/media/rc/rc-ir-raw.c
> @@ -486,14 +486,18 @@ EXPORT_SYMBOL(ir_raw_encode_scancode);
>  /*
>   * Used to (un)register raw event clients
>   */
> -int ir_raw_event_register(struct rc_dev *dev)
> +int ir_raw_event_prepare(struct rc_dev *dev)
>  {
> - int rc;
> - struct ir_raw_handler *handler;
> + static bool raw_init; /* 'false' default value, raw decoders loaded? */
>  
>   if (!dev)
>   return -EINVAL;
>  
> + if (!raw_init) {
> + request_module("ir-lirc-codec");
> + raw_init = true;
> + }
> +
>   dev->raw = kzalloc(sizeof(*dev->raw), GFP_KERNEL);
>   if (!dev->raw)
>   return -ENOMEM;
> @@ -502,6 +506,13 @@ int ir_raw_event_register(struct rc_dev *dev)
>   dev->change_protocol = change_protocol;
>   INIT_KFIFO(dev->raw->kfifo);
>  
> + return 0;
> +}
> +
> +int ir_raw_event_register(struct rc_dev *dev)
> +{
> + struct ir_raw_handler *handler;
> +
>   /*
>* raw transmitters do not need any event registration
>* because the event is coming from userspace
> @@ -510,10 +521,8 @@ int ir_raw_event_register(struct rc_dev *dev)
>   dev->raw->thread = kthread_run(ir_raw_event_thread, dev->raw,
>  "rc%u", dev->minor);
>  
> - if (IS_ERR(dev->raw->thread)) {
> - rc = PTR_ERR(dev->raw->thread);
> - goto out;
> - }
> + if (IS_ERR(dev->raw->thread))
> + return PTR_ERR(dev->raw->thread);
>   }
>  
>   mutex_lock(_raw_handler_lock);
> @@ -524,11 +533,15 @@ int ir_raw_event_register(struct rc_dev *dev)
>   mutex_unlock(_raw_handler_lock);
>  
>   return 0;
> +}
> +
> +void ir_raw_event_free(struct rc_dev *dev)
> +{
> + if (!dev)
> + return;
>  
> -out:
>   kfree(dev->raw);
>   dev->raw = NULL;
> - return rc;
>  }
>  
>  void ir_raw_event_unregister(struct rc_dev *dev)
> @@ -547,8 +560,7 @@ void ir_raw_event_unregister(struct rc_dev *dev)
>   handler->raw_unregister(dev);
>   mutex_unlock(_raw_handler_lock);
>  
> - kfree(dev->raw);
> - dev->raw = NULL;
> + ir_raw_event_free(dev);
>  }
>  
>  /*
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index 802e559cc30e..44189366f232 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -1663,7 +1663,7 @@ struct rc_dev *devm_rc_allocate_device(struct device 
> *dev,
>  }
>  EXPORT_SYMBOL_GPL(devm_rc_allocate_device);
>  
> -static int rc_setup_rx_device(struct rc_dev *dev)
> +static int rc_prepare_rx_device(struct rc_dev *dev)
>  {
>   int rc;
>   struct rc_map *rc_map;
> @@ -1708,10 +1708,22 @@ static int rc_setup_rx_device(struct rc_dev *dev)
>   dev->input_dev->phys = dev->input_phys;
>   dev->input_dev->name = dev->input_name;
>  
> + return 0;
> +
> +out_table:
> + ir_free_table(>rc_map);
> +
> + return rc;
> +}
> +
> +static int rc_setup_rx_device(struct rc_dev *dev)
> +{
> + int rc;
> +
>   /* rc_open will be 

Re: [PATCH] ir-lirc-codec: let lirc_dev handle the lirc_buffer (v3)

[Sean Young]

On Mon, May 01, 2017 at 03:32:34PM +0200, David Härdeman wrote:
> ir_lirc_register() currently creates its own lirc_buffer before
> passing the lirc_driver to lirc_register_driver().
> 
> When a module is later unloaded, ir_lirc_unregister() gets called
> which performs a call to lirc_unregister_driver() and then free():s
> the lirc_buffer.
> 
> The problem is that:
> 
> a) there can still be a userspace app holding an open lirc fd
>when lirc_unregister_driver() returns; and
> 
> b) the lirc_buffer contains "wait_queue_head_t wait_poll" which
>is potentially used as long as any userspace app is still around.
> 
> The result is an oops which can be triggered quite easily by a
> userspace app monitoring its lirc fd using epoll() and not closing
> the fd promptly on device removal.
> 
> The minimalistic fix is to let lirc_dev create the lirc_buffer since
> lirc_dev will then also free the buffer once it believes it is safe to
> do so.
> 
> Version 2: make sure that the allocated buffer is communicated back to
> ir-lirc-codec so that ir_lirc_decode() can use it.
> 
> Version 3: set chunk_size and buffer_size in ir-lirc-codec.

Great, this version works fine.

> CC: sta...@vger.kernel.org

I don't think this change make sense before
"74c839b [media] lirc: use refcounting for lirc devices", so I've removed
the stable cc.

Thanks,
Sean

> Signed-off-by: David Härdeman 
> ---
>  drivers/media/rc/ir-lirc-codec.c |   25 +++--
>  drivers/media/rc/lirc_dev.c  |   13 -
>  2 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/rc/ir-lirc-codec.c 
> b/drivers/media/rc/ir-lirc-codec.c
> index de85f1d7ce43..8f0669c9894c 100644
> --- a/drivers/media/rc/ir-lirc-codec.c
> +++ b/drivers/media/rc/ir-lirc-codec.c
> @@ -354,7 +354,6 @@ static const struct file_operations lirc_fops = {
>  static int ir_lirc_register(struct rc_dev *dev)
>  {
>   struct lirc_driver *drv;
> - struct lirc_buffer *rbuf;
>   int rc = -ENOMEM;
>   unsigned long features = 0;
>  
> @@ -362,19 +361,12 @@ static int ir_lirc_register(struct rc_dev *dev)
>   if (!drv)
>   return rc;
>  
> - rbuf = kzalloc(sizeof(struct lirc_buffer), GFP_KERNEL);
> - if (!rbuf)
> - goto rbuf_alloc_failed;
> -
> - rc = lirc_buffer_init(rbuf, sizeof(int), LIRCBUF_SIZE);
> - if (rc)
> - goto rbuf_init_failed;
> -
>   if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
>   features |= LIRC_CAN_REC_MODE2;
>   if (dev->rx_resolution)
>   features |= LIRC_CAN_GET_REC_RESOLUTION;
>   }
> +
>   if (dev->tx_ir) {
>   features |= LIRC_CAN_SEND_PULSE;
>   if (dev->s_tx_mask)
> @@ -403,10 +395,12 @@ static int ir_lirc_register(struct rc_dev *dev)
>   drv->minor = -1;
>   drv->features = features;
>   drv->data = >raw->lirc;
> - drv->rbuf = rbuf;
> + drv->rbuf = NULL;
>   drv->set_use_inc = _lirc_open;
>   drv->set_use_dec = _lirc_close;
>   drv->code_length = sizeof(struct ir_raw_event) * 8;
> + drv->chunk_size = sizeof(int);
> + drv->buffer_size = LIRCBUF_SIZE;
>   drv->fops = _fops;
>   drv->dev = >dev;
>   drv->rdev = dev;
> @@ -415,19 +409,15 @@ static int ir_lirc_register(struct rc_dev *dev)
>   drv->minor = lirc_register_driver(drv);
>   if (drv->minor < 0) {
>   rc = -ENODEV;
> - goto lirc_register_failed;
> + goto out;
>   }
>  
>   dev->raw->lirc.drv = drv;
>   dev->raw->lirc.dev = dev;
>   return 0;
>  
> -lirc_register_failed:
> -rbuf_init_failed:
> - kfree(rbuf);
> -rbuf_alloc_failed:
> +out:
>   kfree(drv);
> -
>   return rc;
>  }
>  
> @@ -436,9 +426,8 @@ static int ir_lirc_unregister(struct rc_dev *dev)
>   struct lirc_codec *lirc = >raw->lirc;
>  
>   lirc_unregister_driver(lirc->drv->minor);
> - lirc_buffer_free(lirc->drv->rbuf);
> - kfree(lirc->drv->rbuf);
>   kfree(lirc->drv);
> + lirc->drv = NULL;
>  
>   return 0;
>  }
> diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
> index 8d60c9f00df9..42704552b005 100644
> --- a/drivers/media/rc/lirc_dev.c
> +++ b/drivers/media/rc/lirc_dev.c
> @@ -52,6 +52,7 @@ struct irctl {
>  
>   struct mutex irctl_lock;
>   struct lirc_buffer *buf;
> + bool buf_internal;
>   unsigned int chunk_size;
>  
>   struct device dev;
> @@ -83,7 +84,7 @@ static void lirc_release(struct device *ld)
>  
>   put_device(ir->dev.parent);
>  
> - if (ir->buf != ir->d.rbuf) {
> + if (ir->buf_internal) {
>   lirc_buffer_free(ir->buf);
>   kfree(ir->buf);
>   }
> @@ -198,6 +199,7 @@ static int lirc_allocate_buffer(struct irctl *ir)
>  
>   if (d->rbuf) {
>   ir->buf = d->rbuf;
> + ir->buf_internal = false;
>   } else {
>   ir->buf = 

[PATCH 6/7] rc-core: cx231xx - leave the internals of rc_dev alone

[David Härdeman]

Just some debug statements to change.

Signed-off-by: David Härdeman 
---
 drivers/media/usb/cx231xx/cx231xx-input.c |5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-input.c 
b/drivers/media/usb/cx231xx/cx231xx-input.c
index 6e80f3c573f3..eecf074b0a48 100644
--- a/drivers/media/usb/cx231xx/cx231xx-input.c
+++ b/drivers/media/usb/cx231xx/cx231xx-input.c
@@ -30,7 +30,7 @@ static int get_key_isdbt(struct IR_i2c *ir, enum rc_type 
*protocol,
int rc;
u8  cmd, scancode;
 
-   dev_dbg(>rc->input_dev->dev, "%s\n", __func__);
+   dev_dbg(>rc->dev, "%s\n", __func__);
 
/* poll IR chip */
rc = i2c_master_recv(ir->c, , 1);
@@ -48,8 +48,7 @@ static int get_key_isdbt(struct IR_i2c *ir, enum rc_type 
*protocol,
 
scancode = bitrev8(cmd);
 
-   dev_dbg(>rc->input_dev->dev, "cmd %02x, scan = %02x\n",
-   cmd, scancode);
+   dev_dbg(>rc->dev, "cmd %02x, scan = %02x\n", cmd, scancode);
 
*protocol = RC_TYPE_OTHER;
*pscancode = scancode;

[PATCH 7/7] rc-core: tm6000 - leave the internals of rc_dev alone

[David Härdeman]

Not sure what the driver is trying to do, however, IR handling seems incomplete
ATM so deleting the offending parts shouldn't affect functionality

Signed-off-by: David Härdeman 
---
 drivers/media/usb/tm6000/tm6000-input.c |4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/usb/tm6000/tm6000-input.c 
b/drivers/media/usb/tm6000/tm6000-input.c
index 39c15bb2b20c..1a033f57fcc1 100644
--- a/drivers/media/usb/tm6000/tm6000-input.c
+++ b/drivers/media/usb/tm6000/tm6000-input.c
@@ -63,7 +63,6 @@ struct tm6000_IR {
u8  wait:1;
u8  pwled:2;
u8  submit_urb:1;
-   u16 key_addr;
struct urb  *int_urb;
 
/* IR device properties */
@@ -321,9 +320,6 @@ static int tm6000_ir_change_protocol(struct rc_dev *rc, u64 
*rc_type)
 
dprintk(2, "%s\n",__func__);
 
-   if ((rc->rc_map.scan) && (*rc_type == RC_BIT_NEC))
-   ir->key_addr = ((rc->rc_map.scan[0].scancode >> 8) & 0x);
-
ir->rc_type = *rc_type;
 
tm6000_ir_config(ir);

[PATCH 3/7] rc-core: img-nec-decoder - leave the internals of rc_dev alone

[David Härdeman]

Obvious fix, leave repeat handling to rc-core

Signed-off-by: David Härdeman 
---
 drivers/media/rc/ir-nec-decoder.c |   10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/media/rc/ir-nec-decoder.c 
b/drivers/media/rc/ir-nec-decoder.c
index 3ce850314dca..75b9137f6faf 100644
--- a/drivers/media/rc/ir-nec-decoder.c
+++ b/drivers/media/rc/ir-nec-decoder.c
@@ -88,13 +88,9 @@ static int ir_nec_decode(struct rc_dev *dev, struct 
ir_raw_event ev)
data->state = STATE_BIT_PULSE;
return 0;
} else if (eq_margin(ev.duration, NEC_REPEAT_SPACE, NEC_UNIT / 
2)) {
-   if (!dev->keypressed) {
-   IR_dprintk(1, "Discarding last key repeat: 
event after key up\n");
-   } else {
-   rc_repeat(dev);
-   IR_dprintk(1, "Repeat last key\n");
-   data->state = STATE_TRAILER_PULSE;
-   }
+   rc_repeat(dev);
+   IR_dprintk(1, "Repeat last key\n");
+   data->state = STATE_TRAILER_PULSE;
return 0;
}
 

[PATCH 5/7] rc-core: ir-raw - leave the internals of rc_dev alone

[David Härdeman]

Replace the REP_DELAY value with a static value, which makes more sense.
Automatic repeat handling in the input layer has no relevance for the drivers
idea of "a long time".

Signed-off-by: David Härdeman 
---
 drivers/media/rc/rc-ir-raw.c |4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index ae7785c4fbe7..967ab9531e0a 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -102,20 +102,18 @@ int ir_raw_event_store_edge(struct rc_dev *dev, enum 
raw_event_type type)
s64 delta; /* ns */
DEFINE_IR_RAW_EVENT(ev);
int rc = 0;
-   int delay;
 
if (!dev->raw)
return -EINVAL;
 
now = ktime_get();
delta = ktime_to_ns(ktime_sub(now, dev->raw->last_event));
-   delay = MS_TO_NS(dev->input_dev->rep[REP_DELAY]);
 
/* Check for a long duration since last event or if we're
 * being called for the first time, note that delta can't
 * possibly be negative.
 */
-   if (delta > delay || !dev->raw->last_type)
+   if (delta > MS_TO_NS(500) || !dev->raw->last_type)
type |= IR_START_EVENT;
else
ev.duration = delta;

[PATCH 2/7] rc-core: img-ir - leave the internals of rc_dev alone

[David Härdeman]

Changing the protocol does not imply that the keymap changes.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/img-ir/img-ir-hw.c |4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/rc/img-ir/img-ir-hw.c 
b/drivers/media/rc/img-ir/img-ir-hw.c
index 431d33b36fb0..8d1439622533 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -702,10 +702,6 @@ static void img_ir_set_protocol(struct img_ir_priv *priv, 
u64 proto)
 {
struct rc_dev *rdev = priv->hw.rdev;
 
-   spin_lock_irq(>rc_map.lock);
-   rdev->rc_map.rc_type = __ffs64(proto);
-   spin_unlock_irq(>rc_map.lock);
-
mutex_lock(>lock);
rdev->enabled_protocols = proto;
rdev->allowed_wakeup_protocols = proto;

[PATCH 0/7] rc: don't poke around in rc_dev internals

[David Härdeman]

The following patch series fixes up various drivers which go
poking around in struct rc_dev for no good reason.

---

David Härdeman (7):
  rc-core: ati_remote - leave the internals of rc_dev alone
  rc-core: img-ir - leave the internals of rc_dev alone
  rc-core: img-nec-decoder - leave the internals of rc_dev alone
  rc-core: sanyo - leave the internals of rc_dev alone
  rc-core: ir-raw - leave the internals of rc_dev alone
  rc-core: cx231xx - leave the internals of rc_dev alone
  rc-core: tm6000 - leave the internals of rc_dev alone


 drivers/media/rc/ati_remote.c |3 ---
 drivers/media/rc/img-ir/img-ir-hw.c   |4 
 drivers/media/rc/ir-nec-decoder.c |   10 +++---
 drivers/media/rc/ir-sanyo-decoder.c   |   10 +++---
 drivers/media/rc/rc-ir-raw.c  |4 +---
 drivers/media/usb/cx231xx/cx231xx-input.c |5 ++---
 drivers/media/usb/tm6000/tm6000-input.c   |4 
 7 files changed, 9 insertions(+), 31 deletions(-)

--
David Härdeman

[PATCH 1/7] rc-core: ati_remote - leave the internals of rc_dev alone

[David Härdeman]

The REP_DELAY setting on the input device is independent of hardware. This
change should not change how to driver works (as it does a keydown/keyup and
has no real repeat handling).

Signed-off-by: David Härdeman 
---
 drivers/media/rc/ati_remote.c |3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c
index 9cf3e69de16a..a4c6ad4f67c1 100644
--- a/drivers/media/rc/ati_remote.c
+++ b/drivers/media/rc/ati_remote.c
@@ -904,9 +904,6 @@ static int ati_remote_probe(struct usb_interface *interface,
if (err)
goto exit_kill_urbs;
 
-   /* use our delay for rc_dev */
-   ati_remote->rdev->input_dev->rep[REP_DELAY] = repeat_delay;
-
/* Set up and register mouse input device */
if (mouse) {
input_dev = input_allocate_device();

[PATCH 4/7] rc-core: sanyo - leave the internals of rc_dev alone

[David Härdeman]

Leave repeat handling to rc-core.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/ir-sanyo-decoder.c |   10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/media/rc/ir-sanyo-decoder.c 
b/drivers/media/rc/ir-sanyo-decoder.c
index 520bb77dcb62..e6a906a34f90 100644
--- a/drivers/media/rc/ir-sanyo-decoder.c
+++ b/drivers/media/rc/ir-sanyo-decoder.c
@@ -110,13 +110,9 @@ static int ir_sanyo_decode(struct rc_dev *dev, struct 
ir_raw_event ev)
break;
 
if (!data->count && geq_margin(ev.duration, SANYO_REPEAT_SPACE, 
SANYO_UNIT / 2)) {
-   if (!dev->keypressed) {
-   IR_dprintk(1, "SANYO discarding last key 
repeat: event after key up\n");
-   } else {
-   rc_repeat(dev);
-   IR_dprintk(1, "SANYO repeat last key\n");
-   data->state = STATE_INACTIVE;
-   }
+   rc_repeat(dev);
+   IR_dprintk(1, "SANYO repeat last key\n");
+   data->state = STATE_INACTIVE;
return 0;
}
 

[PATCH 12/16] lirc_dev: return POLLHUP and POLLERR when device is gone

[David Härdeman]

Most drivers return both values when the device is gone.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 57d21201ff93..e44e0b23b883 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -374,7 +374,7 @@ unsigned int lirc_dev_fop_poll(struct file *file, 
poll_table *wait)
}
 
if (!ir->attached)
-   return POLLERR;
+   return POLLHUP | POLLERR;
 
if (ir->buf) {
poll_wait(file, >buf->wait_poll, wait);

[PATCH 14/16] lirc_dev: cleanup includes

[David Härdeman]

Remove superfluous includes and defines.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |   12 +---
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 7db7d4c57991..4ba6c7e2d41b 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -15,20 +15,11 @@
  *
  */
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
 #include 
-#include 
 #include 
-#include 
 #include 
-#include 
 #include 
-#include 
 #include 
-#include 
-#include 
-#include 
 #include 
 #include 
 #include 
@@ -37,7 +28,6 @@
 #include 
 #include 
 
-#define IRCTL_DEV_NAME "BaseRemoteCtl"
 #define LOGHEAD"lirc_dev (%s[%d]): "
 
 static dev_t lirc_base_dev;
@@ -545,7 +535,7 @@ static int __init lirc_dev_init(void)
}
 
retval = alloc_chrdev_region(_base_dev, 0, LIRC_MAX_DEVICES,
-IRCTL_DEV_NAME);
+"BaseRemoteCtl");
if (retval) {
class_destroy(lirc_class);
pr_err("alloc_chrdev_region failed\n");

[PATCH 16/16] lirc_dev: cleanup header

[David Härdeman]

Remove some stuff from lirc_dev.h which is not used anywhere.

Signed-off-by: David Härdeman 
---
 include/media/lirc_dev.h |   11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index 11f455a34090..af738d522dec 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -9,10 +9,6 @@
 #ifndef _LINUX_LIRC_DEV_H
 #define _LINUX_LIRC_DEV_H
 
-#define BUFLEN16
-
-#define mod(n, div) ((n) % (div))
-
 #include 
 #include 
 #include 
@@ -20,6 +16,8 @@
 #include 
 #include 
 
+#define BUFLEN16
+
 struct lirc_buffer {
wait_queue_head_t wait_poll;
spinlock_t fifo_lock;
@@ -89,11 +87,6 @@ static inline int lirc_buffer_empty(struct lirc_buffer *buf)
return !lirc_buffer_len(buf);
 }
 
-static inline int lirc_buffer_available(struct lirc_buffer *buf)
-{
-   return buf->size - (lirc_buffer_len(buf) / buf->chunk_size);
-}
-
 static inline unsigned int lirc_buffer_read(struct lirc_buffer *buf,
unsigned char *dest)
 {

[PATCH 15/16] lirc_dev: remove name from struct lirc_driver

[David Härdeman]

The name is only used for a few debug messages and the name of the parent
device as well as the name of the lirc device (e.g. "lirc0") are sufficient
anyway.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/ir-lirc-codec.c|2 --
 drivers/media/rc/lirc_dev.c |   25 -
 drivers/staging/media/lirc/lirc_zilog.c |1 -
 include/media/lirc_dev.h|3 ---
 4 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index 2c1221a61ea1..74ce27f92901 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -380,8 +380,6 @@ static int ir_lirc_register(struct rc_dev *dev)
if (dev->max_timeout)
features |= LIRC_CAN_SET_REC_TIMEOUT;
 
-   snprintf(drv->name, sizeof(drv->name), "ir-lirc-codec (%s)",
-dev->driver_name);
drv->features = features;
drv->data = >raw->lirc;
drv->rbuf = NULL;
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 4ba6c7e2d41b..10783ef75a25 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -28,8 +28,6 @@
 #include 
 #include 
 
-#define LOGHEAD"lirc_dev (%s[%d]): "
-
 static dev_t lirc_base_dev;
 
 /**
@@ -160,7 +158,6 @@ lirc_register_driver(struct lirc_driver *d)
return -EBADRQC;
}
 
-   d->name[sizeof(d->name)-1] = '\0';
if (d->features == 0)
d->features = LIRC_CAN_REC_LIRCCODE;
 
@@ -207,8 +204,7 @@ lirc_register_driver(struct lirc_driver *d)
if (err)
goto out_cdev;
 
-   dev_info(ir->d.dev, "lirc_dev: driver %s registered at minor = %d\n",
-ir->d.name, ir->d.minor);
+   dev_info(ir->d.dev, "lirc device registered as lirc%d\n", minor);
 
d->lirc_internal = ir;
return 0;
@@ -242,13 +238,11 @@ lirc_unregister_driver(struct lirc_driver *d)
 
mutex_lock(>mutex);
 
-   dev_dbg(ir->d.dev, "lirc_dev: driver %s unregistered from minor = %d\n",
-   ir->d.name, ir->d.minor);
+   dev_dbg(>dev, "unregistered\n");
 
ir->dead = true;
if (ir->users) {
-   dev_dbg(ir->d.dev, LOGHEAD "releasing opened driver\n",
-   ir->d.name, ir->d.minor);
+   dev_dbg(>dev, "releasing opened driver\n");
wake_up_interruptible(>buf->wait_poll);
}
 
@@ -278,7 +272,7 @@ int lirc_dev_fop_open(struct inode *inode, struct file 
*file)
 
mutex_unlock(>mutex);
 
-   dev_dbg(ir->d.dev, LOGHEAD "open called\n", ir->d.name, ir->d.minor);
+   dev_dbg(>dev, "open called\n");
 
if (ir->d.rdev) {
retval = rc_open(ir->d.rdev);
@@ -332,8 +326,7 @@ unsigned int lirc_dev_fop_poll(struct file *file, 
poll_table *wait)
else
ret = POLLIN | POLLRDNORM;
 
-   dev_dbg(ir->d.dev, LOGHEAD "poll result = %d\n",
-   ir->d.name, ir->d.minor, ret);
+   dev_dbg(>dev, "poll result = %d\n", ret);
 
return ret;
 }
@@ -345,12 +338,10 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
__u32 mode;
int result = 0;
 
-   dev_dbg(ir->d.dev, LOGHEAD "ioctl called (0x%x)\n",
-   ir->d.name, ir->d.minor, cmd);
+   dev_dbg(>dev, "ioctl called (0x%x)\n", cmd);
 
if (ir->dead) {
-   dev_err(ir->d.dev, LOGHEAD "ioctl result = -ENODEV\n",
-   ir->d.name, ir->d.minor);
+   dev_err(>dev, "ioctl result = -ENODEV\n");
return -ENODEV;
}
 
@@ -428,7 +419,7 @@ ssize_t lirc_dev_fop_read(struct file *file,
if (!LIRC_CAN_REC(ir->d.features))
return -EINVAL;
 
-   dev_dbg(ir->d.dev, LOGHEAD "read called\n", ir->d.name, ir->d.minor);
+   dev_dbg(>dev, "read called\n");
 
buf = kzalloc(ir->chunk_size, GFP_KERNEL);
if (!buf)
diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index ffb70dee4547..131d87a04aab 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -1377,7 +1377,6 @@ static const struct file_operations lirc_fops = {
 };
 
 static struct lirc_driver lirc_template = {
-   .name   = "lirc_zilog",
.code_length= 13,
.buffer_size= BUFLEN / 2,
.chunk_size = 2,
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index f7629ff116a9..11f455a34090 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -120,8 +120,6 @@ static inline unsigned int lirc_buffer_write(struct 
lirc_buffer *buf,
 /**
  * struct lirc_driver - Defines the parameters on a LIRC driver
  *
- * @name:  this string will be used for logs
- *
  * @minor: indicates minor device (/dev/lirc) number for
  * 

[PATCH 13/16] lirc_dev: use an ida instead of a hand-rolled array to keep track of minors

[David Härdeman]

Using the kernel ida facilities, we can avoid a lot of unnecessary code and at 
the same
time get rid of lirc_dev_lock in favour of per-device locks (the irctls array 
was used
throughout lirc_dev so this patch necessarily touches a lot of code).

Signed-off-by: David Härdeman 
---
 drivers/media/rc/ir-lirc-codec.c|9 -
 drivers/media/rc/lirc_dev.c |  258 ---
 drivers/staging/media/lirc/lirc_zilog.c |   16 +-
 include/media/lirc_dev.h|   14 --
 4 files changed, 115 insertions(+), 182 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index a30af91710fe..2c1221a61ea1 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -382,7 +382,6 @@ static int ir_lirc_register(struct rc_dev *dev)
 
snprintf(drv->name, sizeof(drv->name), "ir-lirc-codec (%s)",
 dev->driver_name);
-   drv->minor = -1;
drv->features = features;
drv->data = >raw->lirc;
drv->rbuf = NULL;
@@ -394,11 +393,9 @@ static int ir_lirc_register(struct rc_dev *dev)
drv->rdev = dev;
drv->owner = THIS_MODULE;
 
-   drv->minor = lirc_register_driver(drv);
-   if (drv->minor < 0) {
-   rc = -ENODEV;
+   rc = lirc_register_driver(drv);
+   if (rc < 0)
goto out;
-   }
 
dev->raw->lirc.drv = drv;
dev->raw->lirc.dev = dev;
@@ -413,7 +410,7 @@ static int ir_lirc_unregister(struct rc_dev *dev)
 {
struct lirc_codec *lirc = >raw->lirc;
 
-   lirc_unregister_driver(lirc->drv->minor);
+   lirc_unregister_driver(lirc->drv);
kfree(lirc->drv);
lirc->drv = NULL;
 
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index e44e0b23b883..7db7d4c57991 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -31,23 +31,35 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
 #include 
 
 #define IRCTL_DEV_NAME "BaseRemoteCtl"
-#define NOPLUG -1
 #define LOGHEAD"lirc_dev (%s[%d]): "
 
 static dev_t lirc_base_dev;
 
+/**
+ * struct irctl - lirc per-device structure
+ * @d: internal copy of the  lirc_driver for the device
+ * @dead:  if the device has gone away
+ * @users: the number of users of the lirc chardev (currently 
limited to 1)
+ * @mutex: synchronizes open()/close()/ioctl()/etc calls
+ * @buf:   used to store lirc RX data
+ * @buf_internal:  whether @buf was allocated internally or not
+ * @chunk_size:@buf stores and read() returns data chunks of 
this size
+ * @dev:   the  device for the lirc device
+ * @cdev:  the  device for the lirc chardev
+ */
 struct irctl {
struct lirc_driver d;
-   int attached;
-   int open;
+   bool dead;
+   unsigned users;
 
-   struct mutex irctl_lock;
+   struct mutex mutex;
struct lirc_buffer *buf;
bool buf_internal;
unsigned int chunk_size;
@@ -56,9 +68,9 @@ struct irctl {
struct cdev cdev;
 };
 
-static DEFINE_MUTEX(lirc_dev_lock);
-
-static struct irctl *irctls[MAX_IRCTL_DEVICES];
+/* Used to keep track of allocated lirc devices */
+#define LIRC_MAX_DEVICES 256
+static DEFINE_IDA(lirc_ida);
 
 /* Only used for sysfs but defined to void otherwise */
 static struct class *lirc_class;
@@ -72,9 +84,6 @@ static void lirc_release(struct device *ld)
kfree(ir->buf);
}
 
-   mutex_lock(_dev_lock);
-   irctls[ir->d.minor] = NULL;
-   mutex_unlock(_dev_lock);
kfree(ir);
 }
 
@@ -117,7 +126,18 @@ static int lirc_allocate_buffer(struct irctl *ir)
return err;
 }
 
-int lirc_register_driver(struct lirc_driver *d)
+/**
+ * lirc_register_driver() - create a new lirc device by registering a driver
+ * @d: the  lirc_driver to register
+ *
+ * This function allocates and registers a lirc device for a given
+ * _driver. The _driver structure is updated to contain
+ * information about the allocated device (minor, buffer, etc).
+ *
+ * Return: zero on success or a negative error value.
+ */
+int
+lirc_register_driver(struct lirc_driver *d)
 {
struct irctl *ir;
int minor;
@@ -138,12 +158,6 @@ int lirc_register_driver(struct lirc_driver *d)
return -EINVAL;
}
 
-   if (d->minor >= MAX_IRCTL_DEVICES) {
-   dev_err(d->dev, "minor must be between 0 and %d!\n",
-   MAX_IRCTL_DEVICES - 1);
-   return -EBADRQC;
-   }
-
if (d->code_length < 1 || d->code_length > (BUFLEN * 8)) {
dev_err(d->dev, "code length must be less than %d bits\n",
BUFLEN * 8);
@@ -156,49 +170,30 @@ int lirc_register_driver(struct lirc_driver *d)
return 

[PATCH 11/16] lirc_dev: remove unused module parameter

[David Härdeman]

The "debug" parameter isn't actually used anywhere.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 34bd3f8bf30d..57d21201ff93 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -36,8 +36,6 @@
 #include 
 #include 
 
-static bool debug;
-
 #define IRCTL_DEV_NAME "BaseRemoteCtl"
 #define NOPLUG -1
 #define LOGHEAD"lirc_dev (%s[%d]): "
@@ -625,6 +623,3 @@ module_exit(lirc_dev_exit);
 MODULE_DESCRIPTION("LIRC base driver module");
 MODULE_AUTHOR("Artur Lipowski");
 MODULE_LICENSE("GPL");
-
-module_param(debug, bool, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(debug, "Enable debugging messages");

[PATCH 10/16] lirc_dev: remove superfluous get/put_device() calls

[David Härdeman]

device_add() and friends alredy manage the references to the parent device so
these calls aren't necessary.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 574f4dd416b8..34bd3f8bf30d 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -69,8 +69,6 @@ static void lirc_release(struct device *ld)
 {
struct irctl *ir = container_of(ld, struct irctl, dev);
 
-   put_device(ir->dev.parent);
-
if (ir->buf_internal) {
lirc_buffer_free(ir->buf);
kfree(ir->buf);
@@ -230,8 +228,6 @@ int lirc_register_driver(struct lirc_driver *d)
 
mutex_unlock(_dev_lock);
 
-   get_device(ir->dev.parent);
-
dev_info(ir->d.dev, "lirc_dev: driver %s registered at minor = %d\n",
 ir->d.name, ir->d.minor);
 

[PATCH 08/16] lirc_zilog: remove module parameter minor

[David Härdeman]

Remove the "minor" module parameter in preparation for the next patch.

Signed-off-by: David Härdeman 
---
 drivers/staging/media/lirc/lirc_zilog.c |   16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index 8d8fd8b164e2..59e05aa1bc55 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -156,7 +156,6 @@ static struct mutex tx_data_lock;
 /* module parameters */
 static bool debug; /* debug output */
 static bool tx_only;   /* only handle the IR Tx function */
-static int minor = -1; /* minor number */
 
 
 /* struct IR reference counting */
@@ -184,10 +183,11 @@ static void release_ir_device(struct kref *ref)
 * ir->open_count ==  0 - happens on final close()
 * ir_lock, tx_ref_lock, rx_ref_lock, all released
 */
-   if (ir->l.minor >= 0 && ir->l.minor < MAX_IRCTL_DEVICES) {
+   if (ir->l.minor >= 0) {
lirc_unregister_driver(ir->l.minor);
-   ir->l.minor = MAX_IRCTL_DEVICES;
+   ir->l.minor = -1;
}
+
if (kfifo_initialized(>rbuf.fifo))
lirc_buffer_free(>rbuf);
list_del(>list);
@@ -1597,12 +1597,11 @@ static int ir_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
}
 
/* register with lirc */
-   ir->l.minor = minor; /* module option: user requested minor number */
ir->l.minor = lirc_register_driver(>l);
-   if (ir->l.minor < 0 || ir->l.minor >= MAX_IRCTL_DEVICES) {
+   if (ir->l.minor < 0) {
dev_err(tx->ir->l.dev,
-   "%s: \"minor\" must be between 0 and %d (%d)!\n",
-   __func__, MAX_IRCTL_DEVICES-1, ir->l.minor);
+   "%s: lirc_register_driver() failed: %i\n",
+   __func__, ir->l.minor);
ret = -EBADRQC;
goto out_put_xx;
}
@@ -1673,9 +1672,6 @@ MODULE_LICENSE("GPL");
 /* for compat with old name, which isn't all that accurate anymore */
 MODULE_ALIAS("lirc_pvr150");
 
-module_param(minor, int, 0444);
-MODULE_PARM_DESC(minor, "Preferred minor device number");
-
 module_param(debug, bool, 0644);
 MODULE_PARM_DESC(debug, "Enable debugging messages");
 

[PATCH 09/16] lirc_dev: remove lirc_irctl_init() and lirc_cdev_add()

[David Härdeman]

These two functions only make the logic in lirc_register_driver() harder to
follow.

(Note that almost no other driver calls kobject_set_name() on their cdev so I
simply removed that part).

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |   44 ---
 1 file changed, 12 insertions(+), 32 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index fcc88a09b108..574f4dd416b8 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -65,15 +65,6 @@ static struct irctl *irctls[MAX_IRCTL_DEVICES];
 /* Only used for sysfs but defined to void otherwise */
 static struct class *lirc_class;
 
-/*  helper function
- *  initializes the irctl structure
- */
-static void lirc_irctl_init(struct irctl *ir)
-{
-   mutex_init(>irctl_lock);
-   ir->d.minor = NOPLUG;
-}
-
 static void lirc_release(struct device *ld)
 {
struct irctl *ir = container_of(ld, struct irctl, dev);
@@ -91,27 +82,6 @@ static void lirc_release(struct device *ld)
kfree(ir);
 }
 
-static int lirc_cdev_add(struct irctl *ir)
-{
-   struct lirc_driver *d = >d;
-   struct cdev *cdev;
-   int retval;
-
-   cdev = >cdev;
-
-   if (!d->fops)
-   return -EINVAL;
-
-   cdev_init(cdev, d->fops);
-   cdev->owner = d->owner;
-   retval = kobject_set_name(>kobj, "lirc%d", d->minor);
-   if (retval)
-   return retval;
-
-   cdev->kobj.parent = >dev.kobj;
-   return cdev_add(cdev, ir->dev.devt, 1);
-}
-
 static int lirc_allocate_buffer(struct irctl *ir)
 {
int err = 0;
@@ -167,6 +137,11 @@ int lirc_register_driver(struct lirc_driver *d)
return -EINVAL;
}
 
+   if (!d->fops) {
+   pr_err("fops pointer not filled in!\n");
+   return -EINVAL;
+   }
+
if (d->minor >= MAX_IRCTL_DEVICES) {
dev_err(d->dev, "minor must be between 0 and %d!\n",
MAX_IRCTL_DEVICES - 1);
@@ -210,7 +185,8 @@ int lirc_register_driver(struct lirc_driver *d)
err = -ENOMEM;
goto out_lock;
}
-   lirc_irctl_init(ir);
+
+   mutex_init(>irctl_lock);
irctls[minor] = ir;
d->minor = minor;
 
@@ -238,7 +214,11 @@ int lirc_register_driver(struct lirc_driver *d)
ir->dev.release = lirc_release;
dev_set_name(>dev, "lirc%d", ir->d.minor);
 
-   err = lirc_cdev_add(ir);
+   cdev_init(>cdev, d->fops);
+   ir->cdev.owner = ir->d.owner;
+   ir->cdev.kobj.parent = >dev.kobj;
+
+   err = cdev_add(>cdev, ir->dev.devt, 1);
if (err)
goto out_free_dev;
 

[PATCH 07/16] lirc_dev: merge lirc_register_driver() and lirc_allocate_driver()

[David Härdeman]

Merging the two means that lirc_allocate_buffer() is called before device_add()
and cdev_add() which makes more sense. This also simplifies the locking
slightly because lirc_allocate_buffer() will always be called with lirc_dev_lock
held.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |   41 +
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 29eecddbd371..fcc88a09b108 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -120,8 +120,6 @@ static int lirc_allocate_buffer(struct irctl *ir)
unsigned int buffer_size;
struct lirc_driver *d = >d;
 
-   mutex_lock(_dev_lock);
-
bytes_in_key = BITS_TO_LONGS(d->code_length) +
(d->code_length % 8 ? 1 : 0);
buffer_size = d->buffer_size ? d->buffer_size : BUFLEN / bytes_in_key;
@@ -145,16 +143,15 @@ static int lirc_allocate_buffer(struct irctl *ir)
}
 
ir->buf_internal = true;
+   d->rbuf = ir->buf;
}
ir->chunk_size = ir->buf->chunk_size;
 
 out:
-   mutex_unlock(_dev_lock);
-
return err;
 }
 
-static int lirc_allocate_driver(struct lirc_driver *d)
+int lirc_register_driver(struct lirc_driver *d)
 {
struct irctl *ir;
int minor;
@@ -225,6 +222,15 @@ static int lirc_allocate_driver(struct lirc_driver *d)
 
ir->d = *d;
 
+   if (LIRC_CAN_REC(d->features)) {
+   err = lirc_allocate_buffer(irctls[minor]);
+   if (err) {
+   kfree(ir);
+   goto out_lock;
+   }
+   d->rbuf = ir->buf;
+   }
+
device_initialize(>dev);
ir->dev.devt = MKDEV(MAJOR(lirc_base_dev), ir->d.minor);
ir->dev.class = lirc_class;
@@ -248,7 +254,9 @@ static int lirc_allocate_driver(struct lirc_driver *d)
 
dev_info(ir->d.dev, "lirc_dev: driver %s registered at minor = %d\n",
 ir->d.name, ir->d.minor);
+
return minor;
+
 out_cdev:
cdev_del(>cdev);
 out_free_dev:
@@ -258,29 +266,6 @@ static int lirc_allocate_driver(struct lirc_driver *d)
 
return err;
 }
-
-int lirc_register_driver(struct lirc_driver *d)
-{
-   int minor, err = 0;
-
-   minor = lirc_allocate_driver(d);
-   if (minor < 0)
-   return minor;
-
-   if (LIRC_CAN_REC(d->features)) {
-   err = lirc_allocate_buffer(irctls[minor]);
-   if (err)
-   lirc_unregister_driver(minor);
-   else
-   /*
-* This is kind of a hack but ir-lirc-codec needs
-* access to the buffer that lirc_dev allocated.
-*/
-   d->rbuf = irctls[minor]->buf;
-   }
-
-   return err ? err : minor;
-}
 EXPORT_SYMBOL(lirc_register_driver);
 
 int lirc_unregister_driver(int minor)

[PATCH 06/16] lirc_dev: make fops mandatory

[David Härdeman]

Every caller of lirc_register_driver() passes their own fops and there are no
users of lirc_dev_fop_write() in the kernel tree. Thus we can make fops
mandatory and remove lirc_dev_fop_write().

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |   41 +
 include/media/lirc_dev.h|3 ---
 2 files changed, 5 insertions(+), 39 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index fb487c39b834..29eecddbd371 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -91,17 +91,6 @@ static void lirc_release(struct device *ld)
kfree(ir);
 }
 
-static const struct file_operations lirc_dev_fops = {
-   .owner  = THIS_MODULE,
-   .read   = lirc_dev_fop_read,
-   .write  = lirc_dev_fop_write,
-   .poll   = lirc_dev_fop_poll,
-   .unlocked_ioctl = lirc_dev_fop_ioctl,
-   .open   = lirc_dev_fop_open,
-   .release= lirc_dev_fop_close,
-   .llseek = noop_llseek,
-};
-
 static int lirc_cdev_add(struct irctl *ir)
 {
struct lirc_driver *d = >d;
@@ -110,13 +99,11 @@ static int lirc_cdev_add(struct irctl *ir)
 
cdev = >cdev;
 
-   if (d->fops) {
-   cdev_init(cdev, d->fops);
-   cdev->owner = d->owner;
-   } else {
-   cdev_init(cdev, _dev_fops);
-   cdev->owner = THIS_MODULE;
-   }
+   if (!d->fops)
+   return -EINVAL;
+
+   cdev_init(cdev, d->fops);
+   cdev->owner = d->owner;
retval = kobject_set_name(>kobj, "lirc%d", d->minor);
if (retval)
return retval;
@@ -640,24 +627,6 @@ void *lirc_get_pdata(struct file *file)
 EXPORT_SYMBOL(lirc_get_pdata);
 
 
-ssize_t lirc_dev_fop_write(struct file *file, const char __user *buffer,
-  size_t length, loff_t *ppos)
-{
-   struct irctl *ir = irctls[iminor(file_inode(file))];
-
-   if (!ir) {
-   pr_err("called with invalid irctl\n");
-   return -ENODEV;
-   }
-
-   if (!ir->attached)
-   return -ENODEV;
-
-   return -EINVAL;
-}
-EXPORT_SYMBOL(lirc_dev_fop_write);
-
-
 static int __init lirc_dev_init(void)
 {
int retval;
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index 01649b009922..1f327e25a9be 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -210,7 +210,4 @@ unsigned int lirc_dev_fop_poll(struct file *file, 
poll_table *wait);
 long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long 
arg);
 ssize_t lirc_dev_fop_read(struct file *file, char __user *buffer, size_t 
length,
  loff_t *ppos);
-ssize_t lirc_dev_fop_write(struct file *file, const char __user *buffer,
-  size_t length, loff_t *ppos);
-
 #endif

[PATCH 05/16] lirc_dev: clarify error handling

[David Härdeman]

out_sysfs is misleading, sysfs only comes into play after device_add(). Also,
calling device_init() before the rest of struct dev is filled out is clearer.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 5c2b009b6d50..fb487c39b834 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -238,16 +238,16 @@ static int lirc_allocate_driver(struct lirc_driver *d)
 
ir->d = *d;
 
+   device_initialize(>dev);
ir->dev.devt = MKDEV(MAJOR(lirc_base_dev), ir->d.minor);
ir->dev.class = lirc_class;
ir->dev.parent = d->dev;
ir->dev.release = lirc_release;
dev_set_name(>dev, "lirc%d", ir->d.minor);
-   device_initialize(>dev);
 
err = lirc_cdev_add(ir);
if (err)
-   goto out_sysfs;
+   goto out_free_dev;
 
ir->attached = 1;
 
@@ -264,7 +264,7 @@ static int lirc_allocate_driver(struct lirc_driver *d)
return minor;
 out_cdev:
cdev_del(>cdev);
-out_sysfs:
+out_free_dev:
put_device(>dev);
 out_lock:
mutex_unlock(_dev_lock);

[PATCH 04/16] lirc_dev: remove sampling kthread

[David Härdeman]

There are no drivers which use this functionality.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |   94 +--
 drivers/staging/media/lirc/lirc_zilog.c |1 
 include/media/lirc_dev.h|   16 -
 3 files changed, 2 insertions(+), 109 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 7f13ed479e1c..5c2b009b6d50 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -28,7 +28,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -57,9 +56,6 @@ struct irctl {
 
struct device dev;
struct cdev cdev;
-
-   struct task_struct *task;
-   long jiffies_to_wait;
 };
 
 static DEFINE_MUTEX(lirc_dev_lock);
@@ -95,59 +91,6 @@ static void lirc_release(struct device *ld)
kfree(ir);
 }
 
-/*  helper function
- *  reads key codes from driver and puts them into buffer
- *  returns 0 on success
- */
-static int lirc_add_to_buf(struct irctl *ir)
-{
-   int res;
-   int got_data = -1;
-
-   if (!ir->d.add_to_buf)
-   return 0;
-
-   /*
-* service the device as long as it is returning
-* data and we have space
-*/
-   do {
-   got_data++;
-   res = ir->d.add_to_buf(ir->d.data, ir->buf);
-   } while (!res);
-
-   if (res == -ENODEV)
-   kthread_stop(ir->task);
-
-   return got_data ? 0 : res;
-}
-
-/* main function of the polling thread
- */
-static int lirc_thread(void *irctl)
-{
-   struct irctl *ir = irctl;
-
-   do {
-   if (ir->open) {
-   if (ir->jiffies_to_wait) {
-   set_current_state(TASK_INTERRUPTIBLE);
-   schedule_timeout(ir->jiffies_to_wait);
-   }
-   if (kthread_should_stop())
-   break;
-   if (!lirc_add_to_buf(ir))
-   wake_up_interruptible(>buf->wait_poll);
-   } else {
-   set_current_state(TASK_INTERRUPTIBLE);
-   schedule();
-   }
-   } while (!kthread_should_stop());
-
-   return 0;
-}
-
-
 static const struct file_operations lirc_dev_fops = {
.owner  = THIS_MODULE,
.read   = lirc_dev_fop_read,
@@ -252,18 +195,8 @@ static int lirc_allocate_driver(struct lirc_driver *d)
return -EBADRQC;
}
 
-   if (d->sample_rate) {
-   if (2 > d->sample_rate || HZ < d->sample_rate) {
-   dev_err(d->dev, "invalid %d sample rate\n",
-   d->sample_rate);
-   return -EBADRQC;
-   }
-   if (!d->add_to_buf) {
-   dev_err(d->dev, "add_to_buf not set\n");
-   return -EBADRQC;
-   }
-   } else if (!d->rbuf && !(d->fops && d->fops->read &&
-   d->fops->poll && d->fops->unlocked_ioctl)) {
+   if (!d->rbuf && !(d->fops && d->fops->read &&
+ d->fops->poll && d->fops->unlocked_ioctl)) {
dev_err(d->dev, "undefined read, poll, ioctl\n");
return -EBADRQC;
}
@@ -312,22 +245,6 @@ static int lirc_allocate_driver(struct lirc_driver *d)
dev_set_name(>dev, "lirc%d", ir->d.minor);
device_initialize(>dev);
 
-   if (d->sample_rate) {
-   ir->jiffies_to_wait = HZ / d->sample_rate;
-
-   /* try to fire up polling thread */
-   ir->task = kthread_run(lirc_thread, (void *)ir, "lirc_dev");
-   if (IS_ERR(ir->task)) {
-   dev_err(d->dev, "cannot run thread for minor = %d\n",
-   d->minor);
-   err = -ECHILD;
-   goto out_sysfs;
-   }
-   } else {
-   /* it means - wait for external event in task queue */
-   ir->jiffies_to_wait = 0;
-   }
-
err = lirc_cdev_add(ir);
if (err)
goto out_sysfs;
@@ -404,10 +321,6 @@ int lirc_unregister_driver(int minor)
return -ENOENT;
}
 
-   /* end up polling thread */
-   if (ir->task)
-   kthread_stop(ir->task);
-
dev_dbg(ir->d.dev, "lirc_dev: driver %s unregistered from minor = %d\n",
ir->d.name, ir->d.minor);
 
@@ -470,9 +383,6 @@ int lirc_dev_fop_open(struct inode *inode, struct file 
*file)
if (ir->buf)
lirc_buffer_clear(ir->buf);
 
-   if (ir->task)
-   wake_up_process(ir->task);
-
ir->open++;
 
nonseekable_open(inode, file);
diff --git a/drivers/staging/media/lirc/lirc_zilog.c 

[PATCH 02/16] lirc_dev: remove unused set_use_inc/set_use_dec

[David Härdeman]

Since there are no users of this functionality, it can be removed altogether.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/ir-lirc-codec.c |2 --
 drivers/media/rc/lirc_dev.c  |   24 ++--
 include/media/lirc_dev.h |6 --
 3 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index fc58745b26b8..a30af91710fe 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -386,8 +386,6 @@ static int ir_lirc_register(struct rc_dev *dev)
drv->features = features;
drv->data = >raw->lirc;
drv->rbuf = NULL;
-   drv->set_use_inc = NULL;
-   drv->set_use_dec = NULL;
drv->code_length = sizeof(struct ir_raw_event) * 8;
drv->chunk_size = sizeof(int);
drv->buffer_size = LIRCBUF_SIZE;
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 42704552b005..05f600bd6c67 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -418,12 +418,6 @@ int lirc_unregister_driver(int minor)
wake_up_interruptible(>buf->wait_poll);
}
 
-   mutex_lock(>irctl_lock);
-
-   if (ir->d.set_use_dec)
-   ir->d.set_use_dec(ir->d.data);
-
-   mutex_unlock(>irctl_lock);
mutex_unlock(_dev_lock);
 
device_del(>dev);
@@ -473,17 +467,13 @@ int lirc_dev_fop_open(struct inode *inode, struct file 
*file)
goto error;
}
 
+   if (ir->buf)
+   lirc_buffer_clear(ir->buf);
+
+   if (ir->task)
+   wake_up_process(ir->task);
+
ir->open++;
-   if (ir->d.set_use_inc)
-   retval = ir->d.set_use_inc(ir->d.data);
-   if (retval) {
-   ir->open--;
-   } else {
-   if (ir->buf)
-   lirc_buffer_clear(ir->buf);
-   if (ir->task)
-   wake_up_process(ir->task);
-   }
 
 error:
nonseekable_open(inode, file);
@@ -508,8 +498,6 @@ int lirc_dev_fop_close(struct inode *inode, struct file 
*file)
rc_close(ir->d.rdev);
 
ir->open--;
-   if (ir->d.set_use_dec)
-   ir->d.set_use_dec(ir->d.data);
if (!ret)
mutex_unlock(_dev_lock);
 
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index cec7d35602d1..71c1c11950fe 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -165,10 +165,6 @@ static inline unsigned int lirc_buffer_write(struct 
lirc_buffer *buf,
  * have to write to the buffer by other means, like irq's
  * (see also lirc_serial.c).
  *
- * @set_use_inc:   set_use_inc will be called after device is opened
- *
- * @set_use_dec:   set_use_dec will be called after device is closed
- *
  * @rdev:  Pointed to struct rc_dev associated with the LIRC
  * device.
  *
@@ -198,8 +194,6 @@ struct lirc_driver {
int max_timeout;
int (*add_to_buf)(void *data, struct lirc_buffer *buf);
struct lirc_buffer *rbuf;
-   int (*set_use_inc)(void *data);
-   void (*set_use_dec)(void *data);
struct rc_dev *rdev;
const struct file_operations *fops;
struct device *dev;

[PATCH 01/16] lirc_dev: remove pointless functions

[David Härdeman]

drv->set_use_inc and drv->set_use_dec are already optional so we can remove all
dummy functions.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/ir-lirc-codec.c|   14 ++
 drivers/staging/media/lirc/lirc_zilog.c |   11 ---
 2 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index 8f0669c9894c..fc58745b26b8 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -327,16 +327,6 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int 
cmd,
return ret;
 }
 
-static int ir_lirc_open(void *data)
-{
-   return 0;
-}
-
-static void ir_lirc_close(void *data)
-{
-   return;
-}
-
 static const struct file_operations lirc_fops = {
.owner  = THIS_MODULE,
.write  = ir_lirc_transmit_ir,
@@ -396,8 +386,8 @@ static int ir_lirc_register(struct rc_dev *dev)
drv->features = features;
drv->data = >raw->lirc;
drv->rbuf = NULL;
-   drv->set_use_inc = _lirc_open;
-   drv->set_use_dec = _lirc_close;
+   drv->set_use_inc = NULL;
+   drv->set_use_dec = NULL;
drv->code_length = sizeof(struct ir_raw_event) * 8;
drv->chunk_size = sizeof(int);
drv->buffer_size = LIRCBUF_SIZE;
diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index e4a533b6beb3..436cf1b6a70a 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -497,15 +497,6 @@ static int lirc_thread(void *arg)
return 0;
 }
 
-static int set_use_inc(void *data)
-{
-   return 0;
-}
-
-static void set_use_dec(void *data)
-{
-}
-
 /* safe read of a uint32 (always network byte order) */
 static int read_uint32(unsigned char **data,
 unsigned char *endp, unsigned int *val)
@@ -1396,8 +1387,6 @@ static struct lirc_driver lirc_template = {
.buffer_size= BUFLEN / 2,
.sample_rate= 0, /* tell lirc_dev to not start its own kthread */
.chunk_size = 2,
-   .set_use_inc= set_use_inc,
-   .set_use_dec= set_use_dec,
.fops   = _fops,
.owner  = THIS_MODULE,
 };

[PATCH 00/16] lirc_dev spring cleaning

[David Härdeman]

lirc_dev has lots of functionality which is unused and the code isn't exactly
up-to-date with current kernel practices. This patchset removes the unused bits
and also simplifies the locking by moving lirc_dev over to only use
per-device mutexes rather than a big lirc lock in addition to per-device locks.

I think this is about as much as can be done right now before lirc_zilog is
either removed or ported to rc-core.

---

David Härdeman (16):
  lirc_dev: remove pointless functions
  lirc_dev: remove unused set_use_inc/set_use_dec
  lirc_dev: correct error handling
  lirc_dev: remove sampling kthread
  lirc_dev: clarify error handling
  lirc_dev: make fops mandatory
  lirc_dev: merge lirc_register_driver() and lirc_allocate_driver()
  lirc_zilog: remove module parameter minor
  lirc_dev: remove lirc_irctl_init() and lirc_cdev_add()
  lirc_dev: remove superfluous get/put_device() calls
  lirc_dev: remove unused module parameter
  lirc_dev: return POLLHUP and POLLERR when device is gone
  lirc_dev: use an ida instead of a hand-rolled array to keep track of 
minors
  lirc_dev: cleanup includes
  lirc_dev: remove name from struct lirc_driver
  lirc_dev: cleanup header


 drivers/media/rc/ir-lirc-codec.c|   23 -
 drivers/media/rc/lirc_dev.c |  516 ---
 drivers/staging/media/lirc/lirc_zilog.c |   33 --
 include/media/lirc_dev.h|   53 ---
 4 files changed, 149 insertions(+), 476 deletions(-)

--
David Härdeman

[PATCH 03/16] lirc_dev: correct error handling

[David Härdeman]

If an error is generated, nonseekable_open() shouldn't be called.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 05f600bd6c67..7f13ed479e1c 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -431,7 +431,7 @@ EXPORT_SYMBOL(lirc_unregister_driver);
 int lirc_dev_fop_open(struct inode *inode, struct file *file)
 {
struct irctl *ir;
-   int retval = 0;
+   int retval;
 
if (iminor(inode) >= MAX_IRCTL_DEVICES) {
pr_err("open result for %d is -ENODEV\n", iminor(inode));
@@ -475,9 +475,11 @@ int lirc_dev_fop_open(struct inode *inode, struct file 
*file)
 
ir->open++;
 
-error:
nonseekable_open(inode, file);
 
+   return 0;
+
+error:
return retval;
 }
 EXPORT_SYMBOL(lirc_dev_fop_open);

Re: [PATCH 1/2] em28xx: allow setting the eeprom bus at cards struct

[Devin Heitmueller]

On Mon, May 1, 2017 at 10:11 AM, Frank Schäfer
 wrote:
>
> Am 01.05.2017 um 13:38 schrieb Mauro Carvalho Chehab:
>> Right now, all devices use bus 0 for eeprom. However, newer
>> versions of Terratec H6 use a different buffer for eeprom.
>>
>> So, add support to use a different I2C address for eeprom.
>
> Has this been tested ?
> Did you read my reply to the previous patch version ?:
> See http://www.spinics.net/lists/linux-media/msg114860.html
>
> I doubt it will work. At least not for the device from the thread in the
> Kodi-forum.

Based on what I know about the Empia 2874/2884 design, I would be
absolutely shocked if the eeprom was really on the second I2C bus.
The boot code in ROM requires the eeprom to be on bus 0 in order to
find the 8051 microcode to be executed.  This is a documented hardware
design requirement.

I have seen designs where the first bus is accessible through an I2C
gate on a demodulator on the second bus.  This creates a multi-master
situation and I have no idea why anyone would ever do this.  However
it does explain a situation where the EEPROM could be optionally
accessed via the second bus (if the I2C gate on the demod was open at
the time).

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH 1/2] em28xx: allow setting the eeprom bus at cards struct

[Frank Schäfer]

Am 01.05.2017 um 13:38 schrieb Mauro Carvalho Chehab:
> Right now, all devices use bus 0 for eeprom. However, newer
> versions of Terratec H6 use a different buffer for eeprom.
>
> So, add support to use a different I2C address for eeprom.

Has this been tested ?
Did you read my reply to the previous patch version ?:
See http://www.spinics.net/lists/linux-media/msg114860.html

I doubt it will work. At least not for the device from the thread in the
Kodi-forum.

Regards,
Frank

> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/usb/em28xx/em28xx-cards.c | 1 +
>  drivers/media/usb/em28xx/em28xx-i2c.c   | 5 +
>  drivers/media/usb/em28xx/em28xx.h   | 4 +++-
>  3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c 
> b/drivers/media/usb/em28xx/em28xx-cards.c
> index a12b599a1fa2..c7754303e88e 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -2669,6 +2669,7 @@ static inline void em28xx_set_model(struct em28xx *dev)
>  
>   /* Should be initialized early, for I2C to work */
>   dev->def_i2c_bus = dev->board.def_i2c_bus;
> + dev->eeprom_i2c_bus = dev->board.eeprom_i2c_bus;
>  }
>  
>  /* Wait until AC97_RESET reports the expected value reliably before 
> proceeding.
> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c 
> b/drivers/media/usb/em28xx/em28xx-i2c.c
> index 8c472d5adb50..df0ab4b6f18f 100644
> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
> @@ -665,8 +665,6 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned 
> bus,
>   *eedata = NULL;
>   *eedata_len = 0;
>  
> - /* EEPROM is always on i2c bus 0 on all known devices. */
> -
>   dev->i2c_client[bus].addr = 0xa0 >> 1;
>  
>   /* Check if board has eeprom */
> @@ -975,8 +973,7 @@ int em28xx_i2c_register(struct em28xx *dev, unsigned bus,
>   dev->i2c_client[bus] = em28xx_client_template;
>   dev->i2c_client[bus].adapter = >i2c_adap[bus];
>  
> - /* Up to now, all eeproms are at bus 0 */
> - if (!bus) {
> + if (bus == dev->eeprom_i2c_bus) {
>   retval = em28xx_i2c_eeprom(dev, bus, >eedata, 
> >eedata_len);
>   if ((retval < 0) && (retval != -ENODEV)) {
>   dev_err(>intf->dev,
> diff --git a/drivers/media/usb/em28xx/em28xx.h 
> b/drivers/media/usb/em28xx/em28xx.h
> index e8d97d5ec161..8117536343ab 100644
> --- a/drivers/media/usb/em28xx/em28xx.h
> +++ b/drivers/media/usb/em28xx/em28xx.h
> @@ -440,7 +440,8 @@ struct em28xx_board {
>   int vchannels;
>   int tuner_type;
>   int tuner_addr;
> - unsigned def_i2c_bus;   /* Default I2C bus */
> + unsigned def_i2c_bus;   /* Default I2C bus */
> + unsigned eeprom_i2c_bus;/* EEPROM I2C bus */
>  
>   /* i2c flags */
>   unsigned int tda9887_conf;
> @@ -643,6 +644,7 @@ struct em28xx {
>  
>   unsigned char eeprom_addrwidth_16bit:1;
>   unsigned def_i2c_bus;   /* Default I2C bus */
> + unsigned eeprom_i2c_bus;/* EEPROM I2C bus */
>   unsigned cur_i2c_bus;   /* Current I2C bus */
>   struct rt_mutex i2c_bus_lock;
>  

[PATCH] ir-lirc-codec: let lirc_dev handle the lirc_buffer (v3)

[David Härdeman]

ir_lirc_register() currently creates its own lirc_buffer before
passing the lirc_driver to lirc_register_driver().

When a module is later unloaded, ir_lirc_unregister() gets called
which performs a call to lirc_unregister_driver() and then free():s
the lirc_buffer.

The problem is that:

a) there can still be a userspace app holding an open lirc fd
   when lirc_unregister_driver() returns; and

b) the lirc_buffer contains "wait_queue_head_t wait_poll" which
   is potentially used as long as any userspace app is still around.

The result is an oops which can be triggered quite easily by a
userspace app monitoring its lirc fd using epoll() and not closing
the fd promptly on device removal.

The minimalistic fix is to let lirc_dev create the lirc_buffer since
lirc_dev will then also free the buffer once it believes it is safe to
do so.

Version 2: make sure that the allocated buffer is communicated back to
ir-lirc-codec so that ir_lirc_decode() can use it.

Version 3: set chunk_size and buffer_size in ir-lirc-codec.

CC: sta...@vger.kernel.org
Signed-off-by: David Härdeman 
---
 drivers/media/rc/ir-lirc-codec.c |   25 +++--
 drivers/media/rc/lirc_dev.c  |   13 -
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index de85f1d7ce43..8f0669c9894c 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -354,7 +354,6 @@ static const struct file_operations lirc_fops = {
 static int ir_lirc_register(struct rc_dev *dev)
 {
struct lirc_driver *drv;
-   struct lirc_buffer *rbuf;
int rc = -ENOMEM;
unsigned long features = 0;
 
@@ -362,19 +361,12 @@ static int ir_lirc_register(struct rc_dev *dev)
if (!drv)
return rc;
 
-   rbuf = kzalloc(sizeof(struct lirc_buffer), GFP_KERNEL);
-   if (!rbuf)
-   goto rbuf_alloc_failed;
-
-   rc = lirc_buffer_init(rbuf, sizeof(int), LIRCBUF_SIZE);
-   if (rc)
-   goto rbuf_init_failed;
-
if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
features |= LIRC_CAN_REC_MODE2;
if (dev->rx_resolution)
features |= LIRC_CAN_GET_REC_RESOLUTION;
}
+
if (dev->tx_ir) {
features |= LIRC_CAN_SEND_PULSE;
if (dev->s_tx_mask)
@@ -403,10 +395,12 @@ static int ir_lirc_register(struct rc_dev *dev)
drv->minor = -1;
drv->features = features;
drv->data = >raw->lirc;
-   drv->rbuf = rbuf;
+   drv->rbuf = NULL;
drv->set_use_inc = _lirc_open;
drv->set_use_dec = _lirc_close;
drv->code_length = sizeof(struct ir_raw_event) * 8;
+   drv->chunk_size = sizeof(int);
+   drv->buffer_size = LIRCBUF_SIZE;
drv->fops = _fops;
drv->dev = >dev;
drv->rdev = dev;
@@ -415,19 +409,15 @@ static int ir_lirc_register(struct rc_dev *dev)
drv->minor = lirc_register_driver(drv);
if (drv->minor < 0) {
rc = -ENODEV;
-   goto lirc_register_failed;
+   goto out;
}
 
dev->raw->lirc.drv = drv;
dev->raw->lirc.dev = dev;
return 0;
 
-lirc_register_failed:
-rbuf_init_failed:
-   kfree(rbuf);
-rbuf_alloc_failed:
+out:
kfree(drv);
-
return rc;
 }
 
@@ -436,9 +426,8 @@ static int ir_lirc_unregister(struct rc_dev *dev)
struct lirc_codec *lirc = >raw->lirc;
 
lirc_unregister_driver(lirc->drv->minor);
-   lirc_buffer_free(lirc->drv->rbuf);
-   kfree(lirc->drv->rbuf);
kfree(lirc->drv);
+   lirc->drv = NULL;
 
return 0;
 }
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 8d60c9f00df9..42704552b005 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -52,6 +52,7 @@ struct irctl {
 
struct mutex irctl_lock;
struct lirc_buffer *buf;
+   bool buf_internal;
unsigned int chunk_size;
 
struct device dev;
@@ -83,7 +84,7 @@ static void lirc_release(struct device *ld)
 
put_device(ir->dev.parent);
 
-   if (ir->buf != ir->d.rbuf) {
+   if (ir->buf_internal) {
lirc_buffer_free(ir->buf);
kfree(ir->buf);
}
@@ -198,6 +199,7 @@ static int lirc_allocate_buffer(struct irctl *ir)
 
if (d->rbuf) {
ir->buf = d->rbuf;
+   ir->buf_internal = false;
} else {
ir->buf = kmalloc(sizeof(struct lirc_buffer), GFP_KERNEL);
if (!ir->buf) {
@@ -208,8 +210,11 @@ static int lirc_allocate_buffer(struct irctl *ir)
err = lirc_buffer_init(ir->buf, chunk_size, buffer_size);
if (err) {
kfree(ir->buf);
+   ir->buf = NULL;
goto out;
}
+
+   

Re: [PATCH] ir-lirc-codec: let lirc_dev handle the lirc_buffer (v2)

[David Härdeman]

On Mon, May 01, 2017 at 01:22:21PM +0100, Sean Young wrote:
>On Sat, Apr 29, 2017 at 11:22:28PM +0200, David Härdeman wrote:
>> ir_lirc_register() currently creates its own lirc_buffer before
>> passing the lirc_driver to lirc_register_driver().
>> 
>> When a module is later unloaded, ir_lirc_unregister() gets called
>> which performs a call to lirc_unregister_driver() and then free():s
>> the lirc_buffer.
>> 
>> The problem is that:
>> 
>> a) there can still be a userspace app holding an open lirc fd
>>when lirc_unregister_driver() returns; and
>> 
>> b) the lirc_buffer contains "wait_queue_head_t wait_poll" which
>>is potentially used as long as any userspace app is still around.
>> 
>> The result is an oops which can be triggered quite easily by a
>> userspace app monitoring its lirc fd using epoll() and not closing
>> the fd promptly on device removal.
>
>You're right, the rbuf is freed too early. Good catch! I missed this one.
>
>However, when I test your patch it does not work.
>
>[sean@bigcore bin]$ ./ir-ctl -d /dev/lirc1 -r
>/dev/lirc1: read returned 2 bytes
>
>The lirc_buffer is no longer has a chunk size of 4.

Thanks. I just tested that /dev/lirc0 returned something, but not the
actual data which was returned. I'll spin a v3.

-- 
David Härdeman

Re: [PATCH] [RFC] rc-core: report protocol information to userspace

[David Härdeman]

On Mon, May 01, 2017 at 11:38:30AM +0100, Sean Young wrote:
>On Sat, Apr 29, 2017 at 12:52:12PM +0200, David Härdeman wrote:
>> Whether we decide to go for any new keytable ioctl():s or not in rc-core, we
>> should provide the protocol information of keypresses to userspace.
>> 
>> Note that this means that the RC_TYPE_* definitions become part of the
>> userspace <-> kernel API/ABI (meaning a full patch should maybe move those
>> defines under include/uapi).
>> 
>> This would also need to be ack:ed by the input maintainers.
>
>This was already NACKed in the past.
>
>http://www.spinics.net/lists/linux-input/msg46941.html
>

Didn't know that, thanks for the pointer. I still think we should
revisit this though. Even if we don't add protocol-aware EVIOC[SG]KEY_V2
ioctls, that information is useful for a configuration tool when
creating keymaps for a new remote.

And examining the parent hardware device (as Dmitry seemed to suggest)
doesn't help with protocol identification.

Another option if we don't want to touch the input layer would be to
export the last_* members from struct rc_dev in sysfs (and I'm guessing
a timestamp would be necessary then). Seems like a lot of work to
accomplish what would otherwise be a one-line change in the input layer
though (one-line since I'm assuming we could provide the protocol
defines in a separate header, other than input-event-codes.h as the
protocols are subsystem-specific).

-- 
David Härdeman

Re: [PATCH] rc-core: export the hardware type to sysfs

[David Härdeman]

On Mon, May 01, 2017 at 11:36:13AM +0100, Sean Young wrote:
>On Sat, Apr 29, 2017 at 12:03:29PM +0200, David Härdeman wrote:
>> Exporting the hardware type makes it possible for userspace applications
>> to know what to expect from the hardware.
>> 
>> This makes it possible to write more user-friendly userspace apps.
>
>This duplicates lirc features (LIRC_GET_FEATURES ioctl); the one exception
>is that the scancode-only devices which have no lirc device, but there
>are patches which change that.

The intention was to let userspace have a way of knowing whether to
expect any lirc device to show up at all. If some class of devices can't
have lirc devices (looking at the patch you linked to that'd still be
CEC?) then I think it's still useful?

-- 
David Härdeman

Re: [PATCH] ir-lirc-codec: let lirc_dev handle the lirc_buffer (v2)

[Sean Young]

On Sat, Apr 29, 2017 at 11:22:28PM +0200, David Härdeman wrote:
> ir_lirc_register() currently creates its own lirc_buffer before
> passing the lirc_driver to lirc_register_driver().
> 
> When a module is later unloaded, ir_lirc_unregister() gets called
> which performs a call to lirc_unregister_driver() and then free():s
> the lirc_buffer.
> 
> The problem is that:
> 
> a) there can still be a userspace app holding an open lirc fd
>when lirc_unregister_driver() returns; and
> 
> b) the lirc_buffer contains "wait_queue_head_t wait_poll" which
>is potentially used as long as any userspace app is still around.
> 
> The result is an oops which can be triggered quite easily by a
> userspace app monitoring its lirc fd using epoll() and not closing
> the fd promptly on device removal.

You're right, the rbuf is freed too early. Good catch! I missed this one.

However, when I test your patch it does not work.

[sean@bigcore bin]$ ./ir-ctl -d /dev/lirc1 -r
/dev/lirc1: read returned 2 bytes

The lirc_buffer is no longer has a chunk size of 4.

Thanks,

Sean

> 
> The minimalistic fix is to let lirc_dev create the lirc_buffer since
> lirc_dev will then also free the buffer once it believes it is safe to
> do so.
> 
> Version 2: make sure that the allocated buffer is communicated back to
> ir-lirc-codec so that ir_lirc_decode() can use it.
> 
> CC: sta...@vger.kernel.org
> Signed-off-by: David Härdeman 
> ---
>  drivers/media/rc/ir-lirc-codec.c |   23 +--
>  drivers/media/rc/lirc_dev.c  |   13 -
>  2 files changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/rc/ir-lirc-codec.c 
> b/drivers/media/rc/ir-lirc-codec.c
> index de85f1d7ce43..7b961357d333 100644
> --- a/drivers/media/rc/ir-lirc-codec.c
> +++ b/drivers/media/rc/ir-lirc-codec.c
> @@ -354,7 +354,6 @@ static const struct file_operations lirc_fops = {
>  static int ir_lirc_register(struct rc_dev *dev)
>  {
>   struct lirc_driver *drv;
> - struct lirc_buffer *rbuf;
>   int rc = -ENOMEM;
>   unsigned long features = 0;
>  
> @@ -362,19 +361,12 @@ static int ir_lirc_register(struct rc_dev *dev)
>   if (!drv)
>   return rc;
>  
> - rbuf = kzalloc(sizeof(struct lirc_buffer), GFP_KERNEL);
> - if (!rbuf)
> - goto rbuf_alloc_failed;
> -
> - rc = lirc_buffer_init(rbuf, sizeof(int), LIRCBUF_SIZE);
> - if (rc)
> - goto rbuf_init_failed;
> -
>   if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
>   features |= LIRC_CAN_REC_MODE2;
>   if (dev->rx_resolution)
>   features |= LIRC_CAN_GET_REC_RESOLUTION;
>   }
> +
>   if (dev->tx_ir) {
>   features |= LIRC_CAN_SEND_PULSE;
>   if (dev->s_tx_mask)
> @@ -403,7 +395,7 @@ static int ir_lirc_register(struct rc_dev *dev)
>   drv->minor = -1;
>   drv->features = features;
>   drv->data = >raw->lirc;
> - drv->rbuf = rbuf;
> + drv->rbuf = NULL;
>   drv->set_use_inc = _lirc_open;
>   drv->set_use_dec = _lirc_close;
>   drv->code_length = sizeof(struct ir_raw_event) * 8;
> @@ -415,19 +407,15 @@ static int ir_lirc_register(struct rc_dev *dev)
>   drv->minor = lirc_register_driver(drv);
>   if (drv->minor < 0) {
>   rc = -ENODEV;
> - goto lirc_register_failed;
> + goto out;
>   }
>  
>   dev->raw->lirc.drv = drv;
>   dev->raw->lirc.dev = dev;
>   return 0;
>  
> -lirc_register_failed:
> -rbuf_init_failed:
> - kfree(rbuf);
> -rbuf_alloc_failed:
> +out:
>   kfree(drv);
> -
>   return rc;
>  }
>  
> @@ -436,9 +424,8 @@ static int ir_lirc_unregister(struct rc_dev *dev)
>   struct lirc_codec *lirc = >raw->lirc;
>  
>   lirc_unregister_driver(lirc->drv->minor);
> - lirc_buffer_free(lirc->drv->rbuf);
> - kfree(lirc->drv->rbuf);
>   kfree(lirc->drv);
> + lirc->drv = NULL;
>  
>   return 0;
>  }
> diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
> index 8d60c9f00df9..42704552b005 100644
> --- a/drivers/media/rc/lirc_dev.c
> +++ b/drivers/media/rc/lirc_dev.c
> @@ -52,6 +52,7 @@ struct irctl {
>  
>   struct mutex irctl_lock;
>   struct lirc_buffer *buf;
> + bool buf_internal;
>   unsigned int chunk_size;
>  
>   struct device dev;
> @@ -83,7 +84,7 @@ static void lirc_release(struct device *ld)
>  
>   put_device(ir->dev.parent);
>  
> - if (ir->buf != ir->d.rbuf) {
> + if (ir->buf_internal) {
>   lirc_buffer_free(ir->buf);
>   kfree(ir->buf);
>   }
> @@ -198,6 +199,7 @@ static int lirc_allocate_buffer(struct irctl *ir)
>  
>   if (d->rbuf) {
>   ir->buf = d->rbuf;
> + ir->buf_internal = false;
>   } else {
>   ir->buf = kmalloc(sizeof(struct lirc_buffer), GFP_KERNEL);
>   if (!ir->buf) {
> @@ -208,8 +210,11 @@ static int lirc_allocate_buffer(struct irctl 

[PATCH 2/2] em28xx: add support for new revisions of Terratec H6

[Mauro Carvalho Chehab]

There's a new version of Terratec H6 with uses USB ID
0ccd:10b2. This version is similar to the old one (with is
supported via the HTC entry), except that this one has the
eeprom on the second bus.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/em28xx/em28xx-cards.c | 19 +++
 drivers/media/usb/em28xx/em28xx-dvb.c   |  1 +
 drivers/media/usb/em28xx/em28xx.h   |  1 +
 3 files changed, 21 insertions(+)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c 
b/drivers/media/usb/em28xx/em28xx-cards.c
index c7754303e88e..b788ae0d5646 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -1193,6 +1193,23 @@ struct em28xx_board em28xx_boards[] = {
.i2c_speed= EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_400_KHZ,
},
+   [EM2884_BOARD_TERRATEC_H6] = {
+   .name = "Terratec Cinergy H6",
+   .has_dvb  = 1,
+   .ir_codes = RC_MAP_NEC_TERRATEC_CINERGY_XS,
+#if 0
+   .tuner_type   = TUNER_PHILIPS_TDA8290,
+   .tuner_addr   = 0x41,
+   .dvb_gpio = terratec_h5_digital, /* FIXME: probably wrong */
+   .tuner_gpio   = terratec_h5_gpio,
+#else
+   .tuner_type   = TUNER_ABSENT,
+#endif
+   .def_i2c_bus  = 1,
+   .eeprom_i2c_bus  = 1,
+   .i2c_speed= EM28XX_I2C_CLK_WAIT_ENABLE |
+   EM28XX_I2C_FREQ_400_KHZ,
+   },
[EM2884_BOARD_HAUPPAUGE_WINTV_HVR_930C] = {
.name = "Hauppauge WinTV HVR 930C",
.has_dvb  = 1,
@@ -2496,6 +2513,8 @@ struct usb_device_id em28xx_id_table[] = {
.driver_info = EM2884_BOARD_TERRATEC_H5 },
{ USB_DEVICE(0x0ccd, 0x10b6),   /* H5 Rev. 3 */
.driver_info = EM2884_BOARD_TERRATEC_H5 },
+   { USB_DEVICE(0x0ccd, 0x10b2),   /* H6 */
+   .driver_info = EM2884_BOARD_TERRATEC_H6 },
{ USB_DEVICE(0x0ccd, 0x0084),
.driver_info = EM2860_BOARD_TERRATEC_AV350 },
{ USB_DEVICE(0x0ccd, 0x0096),
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c 
b/drivers/media/usb/em28xx/em28xx-dvb.c
index 82edd37f0d73..4a7db623fe29 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -1522,6 +1522,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
break;
case EM2884_BOARD_ELGATO_EYETV_HYBRID_2008:
case EM2884_BOARD_CINERGY_HTC_STICK:
+   case EM2884_BOARD_TERRATEC_H6:
terratec_htc_stick_init(dev);
 
/* attach demodulator */
diff --git a/drivers/media/usb/em28xx/em28xx.h 
b/drivers/media/usb/em28xx/em28xx.h
index 8117536343ab..a333ca954129 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -148,6 +148,7 @@
 #define EM28178_BOARD_PLEX_PX_BCUD98
 #define EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_DVB  99
 #define EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_01595 100
+#define EM2884_BOARD_TERRATEC_H6 101
 
 /* Limits minimum and default number of buffers */
 #define EM28XX_MIN_BUF 4
-- 
2.9.3

[PATCH 1/2] em28xx: allow setting the eeprom bus at cards struct

[Mauro Carvalho Chehab]

Right now, all devices use bus 0 for eeprom. However, newer
versions of Terratec H6 use a different buffer for eeprom.

So, add support to use a different I2C address for eeprom.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/em28xx/em28xx-cards.c | 1 +
 drivers/media/usb/em28xx/em28xx-i2c.c   | 5 +
 drivers/media/usb/em28xx/em28xx.h   | 4 +++-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c 
b/drivers/media/usb/em28xx/em28xx-cards.c
index a12b599a1fa2..c7754303e88e 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2669,6 +2669,7 @@ static inline void em28xx_set_model(struct em28xx *dev)
 
/* Should be initialized early, for I2C to work */
dev->def_i2c_bus = dev->board.def_i2c_bus;
+   dev->eeprom_i2c_bus = dev->board.eeprom_i2c_bus;
 }
 
 /* Wait until AC97_RESET reports the expected value reliably before proceeding.
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c 
b/drivers/media/usb/em28xx/em28xx-i2c.c
index 8c472d5adb50..df0ab4b6f18f 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -665,8 +665,6 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned 
bus,
*eedata = NULL;
*eedata_len = 0;
 
-   /* EEPROM is always on i2c bus 0 on all known devices. */
-
dev->i2c_client[bus].addr = 0xa0 >> 1;
 
/* Check if board has eeprom */
@@ -975,8 +973,7 @@ int em28xx_i2c_register(struct em28xx *dev, unsigned bus,
dev->i2c_client[bus] = em28xx_client_template;
dev->i2c_client[bus].adapter = >i2c_adap[bus];
 
-   /* Up to now, all eeproms are at bus 0 */
-   if (!bus) {
+   if (bus == dev->eeprom_i2c_bus) {
retval = em28xx_i2c_eeprom(dev, bus, >eedata, 
>eedata_len);
if ((retval < 0) && (retval != -ENODEV)) {
dev_err(>intf->dev,
diff --git a/drivers/media/usb/em28xx/em28xx.h 
b/drivers/media/usb/em28xx/em28xx.h
index e8d97d5ec161..8117536343ab 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -440,7 +440,8 @@ struct em28xx_board {
int vchannels;
int tuner_type;
int tuner_addr;
-   unsigned def_i2c_bus;   /* Default I2C bus */
+   unsigned def_i2c_bus;   /* Default I2C bus */
+   unsigned eeprom_i2c_bus;/* EEPROM I2C bus */
 
/* i2c flags */
unsigned int tda9887_conf;
@@ -643,6 +644,7 @@ struct em28xx {
 
unsigned char eeprom_addrwidth_16bit:1;
unsigned def_i2c_bus;   /* Default I2C bus */
+   unsigned eeprom_i2c_bus;/* EEPROM I2C bus */
unsigned cur_i2c_bus;   /* Current I2C bus */
struct rt_mutex i2c_bus_lock;
 
-- 
2.9.3

Re: [PATCH] [RFC] rc-core: report protocol information to userspace

[Sean Young]

On Sat, Apr 29, 2017 at 12:52:12PM +0200, David Härdeman wrote:
> Whether we decide to go for any new keytable ioctl():s or not in rc-core, we
> should provide the protocol information of keypresses to userspace.
> 
> Note that this means that the RC_TYPE_* definitions become part of the
> userspace <-> kernel API/ABI (meaning a full patch should maybe move those
> defines under include/uapi).
> 
> This would also need to be ack:ed by the input maintainers.

This was already NACKed in the past.

http://www.spinics.net/lists/linux-input/msg46941.html

> ---
>  drivers/media/rc/rc-main.c |1 +
>  include/uapi/linux/input-event-codes.h |1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index e0f9b322ab02..a38c1f3569ee 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -773,6 +773,7 @@ static void ir_do_keydown(struct rc_dev *dev, enum 
> rc_type protocol,
>   if (new_event && dev->keypressed)
>   ir_do_keyup(dev, false);
>  
> + input_event(dev->input_dev, EV_MSC, MSC_PROTOCOL, protocol);
>   input_event(dev->input_dev, EV_MSC, MSC_SCAN, scancode);
>  
>   if (new_event && keycode != KEY_RESERVED) {
> diff --git a/include/uapi/linux/input-event-codes.h 
> b/include/uapi/linux/input-event-codes.h
> index 3af60ee69053..1a8c3554cbcb 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -794,6 +794,7 @@
>  #define MSC_RAW  0x03
>  #define MSC_SCAN 0x04
>  #define MSC_TIMESTAMP0x05
> +#define MSC_PROTOCOL 0x06
>  #define MSC_MAX  0x07
>  #define MSC_CNT  (MSC_MAX+1)
>  

Re: [PATCH] rc-core: export the hardware type to sysfs

[Sean Young]

On Sat, Apr 29, 2017 at 12:03:29PM +0200, David Härdeman wrote:
> Exporting the hardware type makes it possible for userspace applications
> to know what to expect from the hardware.
> 
> This makes it possible to write more user-friendly userspace apps.

This duplicates lirc features (LIRC_GET_FEATURES ioctl); the one exception
is that the scancode-only devices which have no lirc device, but there
are patches which change that.

https://patchwork.linuxtv.org/patch/39593/


Sean

> 
> Note that the size of sysfs_groups[] in struct rc_dev is not changed
> by this patch because it was already large enough for one more group.
> 
> Signed-off-by: David Härdeman 
> ---
>  drivers/media/rc/rc-main.c |   43 +++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index 85f95441b85b..e0f9b322ab02 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -1294,6 +1294,38 @@ static ssize_t store_protocols(struct device *device,
>  }
>  
>  /**
> + * show_hwtype() - shows the hardware type in sysfs
> + * @device:  the  device descriptor
> + * @attr:the  device_attribute
> + * @buf: a pointer to the output buffer
> + *
> + * This callback function is used to get the hardware type of an rc device.
> + * It is triggered by reading /sys/class/rc/rc?/hwtype.
> + *
> + * Return: the number of bytes read or a negative error code.
> + */
> +static ssize_t show_hwtype(struct device *device,
> +struct device_attribute *attr,
> +char *buf)
> +{
> + struct rc_dev *dev = to_rc_dev(device);
> +
> + switch (dev->driver_type) {
> + case RC_DRIVER_SCANCODE:
> + return sprintf(buf, "scancode\n");
> + case RC_DRIVER_IR_RAW_TX:
> + return sprintf(buf, "ir-tx\n");
> + case RC_DRIVER_IR_RAW:
> + if (dev->tx_ir)
> + return sprintf(buf, "ir-tx-rx\n");
> + else
> + return sprintf(buf, "ir-rx\n");
> + default:
> + return sprintf(buf, "\n");
> + }
> +}
> +
> +/**
>   * show_filter() - shows the current scancode filter value or mask
>   * @device:  the device descriptor
>   * @attr:the device attribute struct
> @@ -1613,6 +1645,7 @@ static int rc_dev_uevent(struct device *device, struct 
> kobj_uevent_env *env)
>   * Static device attribute struct with the sysfs attributes for IR's
>   */
>  static DEVICE_ATTR(protocols, 0644, show_protocols, store_protocols);
> +static DEVICE_ATTR(hwtype, 0444, show_hwtype, NULL);
>  static DEVICE_ATTR(wakeup_protocols, 0644, show_wakeup_protocols,
>  store_wakeup_protocols);
>  static RC_FILTER_ATTR(filter, S_IRUGO|S_IWUSR,
> @@ -1633,6 +1666,15 @@ static struct attribute_group rc_dev_protocol_attr_grp 
> = {
>   .attrs  = rc_dev_protocol_attrs,
>  };
>  
> +static struct attribute *rc_dev_hwtype_attrs[] = {
> + _attr_hwtype.attr,
> + NULL,
> +};
> +
> +static struct attribute_group rc_dev_hwtype_attr_grp = {
> + .attrs = rc_dev_hwtype_attrs,
> +};
> +
>  static struct attribute *rc_dev_filter_attrs[] = {
>   _attr_filter.attr.attr,
>   _attr_filter_mask.attr.attr,
> @@ -1863,6 +1905,7 @@ int rc_register_device(struct rc_dev *dev)
>   dev->sysfs_groups[attr++] = _dev_filter_attr_grp;
>   if (dev->s_wakeup_filter)
>   dev->sysfs_groups[attr++] = _dev_wakeup_filter_attr_grp;
> + dev->sysfs_groups[attr++] = _dev_hwtype_attr_grp;
>   dev->sysfs_groups[attr++] = NULL;
>  
>   if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {

Re: [PATCH 0/5] RFC: ADV748x HDMI/Analog video receiver

[Simon Horman]

On Fri, Apr 28, 2017 at 09:47:05AM +0100, Kieran Bingham wrote:
> Hi Simon,
> 
> On 28/04/17 08:09, Simon Horman wrote:
> > On Thu, Apr 27, 2017 at 07:25:59PM +0100, Kieran Bingham wrote:
> >> From: Kieran Bingham 
> >>
> >> This is an RFC for the Analog Devices ADV748x driver, and follows on from a
> >> previous posting by Niklas Söderlund [0] of an earlier incarnation of this
> >> driver.
> > 
> > ...
> > 
> >> This series presents the following patches:
> >>
> >>  [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous
> >>  [PATCH 2/5] rcar-vin: Match sources against ports if specified.
> >>  [PATCH 3/5] media: i2c: adv748x: add adv748x driver
> >>  [PATCH 4/5] arm64: dts: r8a7795: salvator-x: enable VIN, CSI and ADV7482
> >>  [PATCH 5/5] arm64: dts: r8a7796: salvator-x: enable VIN, CSI and ADV7482
> > 
> > I am marking the above dts patches as "RFC" and do not plan to apply them
> > unless you ping me or repost them.
> 
> Yes, sorry - the whole series was supposed to be marked as RFC, but I didn't
> think about it - and apparently only applied the tag to the cover letter.
> 
> Apologies for any confusion.

It was clear enough, though an tag RFC in every patch would be better.
In any case I was referring to how I have handled these patches in
patchwork.

Apologies for any confusion.

> > Assuming they don't cause any
> > regressions I would be happy to consider applying them as soon as their
> > dependencies are accepted.
> 
> Does that mean you've done a cursory glance over the content ? :-)

Yes, I did take a quick glance.

> In this instance, the port numbers need to revert back to a zero-base,
> but I would appreciate an eye on how and where I've put the
> representation of the physical hdmi/cvbs connectors. Having modified
> plenty of DT, but not actually submitted much - I still feel 'new' at it
> - so I'm sure I may not have followed the standards quite right yet.

Assuming you are talking about where in the DT file the hdmi and cvbs nodes
should go, I think this is somewhat arbitrary so long as they are within
the top-level node - what you have looks good to me.

> The dts patches are based heavily on the previous posting by Niklas, but I 
> have
> extended to put the extra hdmi and cvbs links in.
> 
> Regards
> --
> Kieran
> 

Re: [PATCH v2 0/3] r8a7793 Gose video input support

[Simon Horman]

On Fri, Apr 28, 2017 at 11:40:20AM +0300, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Friday 28 Apr 2017 07:16:24 Simon Horman wrote:
> > On Wed, Apr 26, 2017 at 06:56:06PM +0300, Laurent Pinchart wrote:
> > > On Tuesday 21 Feb 2017 01:42:15 Laurent Pinchart wrote:
> > >> On Thursday 20 Oct 2016 10:49:11 Simon Horman wrote:
> > >>> On Tue, Oct 18, 2016 at 05:02:20PM +0200, Ulrich Hecht wrote:
> >  Hi!
> >  
> >  This is a by-the-datasheet implementation of analog and digital video
> >  input on the Gose board.
> >  
> >  I have tried to address all concerns raised by reviewers, with the
> >  exception of the composite input patch, which has been left as is for
> >  now.
> >  
> >  CU
> >  Uli
> >  
> >  
> >  Changes since v1:
> >  - r8a7793.dtsi: added VIN2
> >  - modeled HDMI decoder input/output and connector
> >  - added "renesas,rcar-gen2-vin" compat strings
> >  - removed unnecessary "remote" node and aliases
> >  - set ADV7612 interrupt to GP4_2
> >  
> >  Ulrich Hecht (3):
> >    ARM: dts: r8a7793: Enable VIN0-VIN2
> > >>> 
> > >>> I have queued up the above patch with Laurent and Geert's tags.
> > >>> 
> >    ARM: dts: gose: add HDMI input
> >    ARM: dts: gose: add composite video input
> > >>> 
> > >>> Please address the review of the above two patches and repost.
> > >> 
> > >> Could you please do so ? Feedback on 2/3 should be easy to handle. For
> > >> 3/3, you might need to ping the DT maintainers.
> > > 
> > > Ping. These are the only two patches that block
> > > 
> > > VIN,v4.12,public,ulrich,Gen2 VIN integration
> > 
> > Sorry, I'm unsure how these slipped through the cracks.
> > I now have them queued up locally for v4.13 and I plan to push this morning.
> 
> Please don't, the ping was for Ulrich, he needs to address review comments on 
> patches 2/3 and 3/3.

Oh, sorry. I will dequeue them.