Re: [PATCH v3 04/26] media: lirc_zilog: remove receiver

[Sean Young]

Hi Devin, Andy,

On Wed, Oct 11, 2017 at 10:25:34PM -0400, Devin Heitmueller wrote:
> Hi Andy,
> 
> > 5. Rx and IR Learn both use the same external hardware.  Not
> > coordinating Rx with Learn mode in the same driver, will prevent Learn
> > operation from working.  That is, if Learn mode is ever implemented.
> > (Once upon a time, I was planning on doing that.  But I have no time
> > for that anymore.)
> 
> There's not really any infrastructure in Linux that maps to the
> Zilog's "learning mode" functionality.  Usually I would just tell
> users to do the learning under Windows and send me the resulting .ini
> file (which we could then add to the database).
> 
> I had planned on getting rid of the database entirely and just
> converting an MCE compatible pulse train to the blasting format
> required by the Zilog firmware (using the awesome work you sent me
> privately), but the fact of the matter is that nobody cares and MCEUSB
> devices are $20 online.

I wouldn't mind working on that, if I had the blasting format. :)

Note that you can have IR Rx and Tx on a gpio port, so it can get even
cheaper than $20. The hauppauge solution with a z8 microcontroller
with it's non-obvious firmware and i2c format seem a bit ludricous.

> > I'm glad someone remembers all this stuff.  I'm assuming you had more
> > pain with this than I ever did.
> 
> This would be a safe assumption.  I probably put about a month's worth
> of engineering into driver work for the Zilog, which seems
> extraordinary given how simple something like an IR blaster/receiver
> is supposed to be.  I guess that's the fun of proving out a new
> hardware design as opposed to just making something work under Linux
> that is already known to work under Windows.
> 
> > I never owned an HD-PVR.
> 
> I'm sure I have a spare or two if you really want one (not that you
> have the time to muck with such things nowadays).  :-)
> 
> The HD-PVR was a bit of a weird case compared to devices like ivtv and
> cx18 because it was technically multi-master (I2C commands came both
> from the host and from the onboard SOC).  Hence you could have weird
> cases where one would block the other at unexpected times.  I2C
> commands to the Zilog would hold the bus which would delay the onboard
> firmware from issuing commands to the video decoder (fun timing
> issues).  There was also some weird edge case I don't recall the
> details of that prompted them to add an I2C gate in later board
> revisions.

Interesting.

Thanks

Sean

Re: [PATCH v3 04/26] media: lirc_zilog: remove receiver

[Sean Young]

On Wed, Oct 11, 2017 at 08:25:12PM -0400, Devin Heitmueller wrote:
> > There's an ir_lock mutex in the driver to prevent simultaneous access to 
> > the Rx and Tx functions of the z8.  Accessing Rx and Tx functions of the 
> > chip together can cause it to do the wrong thing (sometimes hang?), IIRC.
> >
> > I'll see if I can dig up my old disassembly of the z8's firmware to see if 
> > that interlock is strictly necessary.
> >
> > Yes I know ir-kbd-i2c is in mainline, but as I said, I had reasons for 
> > avoiding it when using Tx operation of the chip.  It's been 7 years, and 
> > I'm getting too old to remember the reasons. :P
> 
> Yeah, you definitely don't want to be issuing requests to the Rx and
> Tx addresses at the same time.  Very bad things will happen.

Right, ok. That's good to know. I'll have to merge the Tx code with 
ir-kbd-i2c or port lirc_zilog Rx to rc-core.

> Also, what is the polling interval for ir-kbd-i2c?

lirc_zilog polls every 260ms and ir-kbd-i2c polls every 100ms.

>  If it's too high
> you can wedge the I2C bus (depending on the hardware design).
> Likewise, many people disable IR RX entirely (which is trivial to do
> with lirc_zilog through a modprobe optoin).

Yes, lirc_zilog has a tx_only option.

>  This is because early
> versions of the HDPVR had issues where polling too often can interfere
> with traffic that configures the component receiver chip.  This was
> improved in later versions of the design, but many people found it was
> just easiest to disable RX since they don't use it (i.e. they would
> use the blaster for controlling their STB but use a separate IR
> receiver).

That's very interesting.

> Are you testing with video capture actively in use?  If you're testing
> the IR interface without an active HD capture in progress you're
> likely to miss some of these edge cases (in particular those which
> would hang the encoder).

I had not, but I'll do that now.

> I'm not against the removal of duplicate code in general, but you have
> to tread carefully because there are plenty of non-obvious edge cases
> to consider.

Absolutely, thank you for your insights. 

Rather than relying on ir-kbd-i2c for receive, I can port lirc_zilog
to rc-core and leave much of it in place.

Thanks

Sean

Re: [PATCH v3 04/26] media: lirc_zilog: remove receiver

[Devin Heitmueller]

Hi Andy,

> 5. Rx and IR Learn both use the same external hardware.  Not
> coordinating Rx with Learn mode in the same driver, will prevent Learn
> operation from working.  That is, if Learn mode is ever implemented.
> (Once upon a time, I was planning on doing that.  But I have no time
> for that anymore.)

There's not really any infrastructure in Linux that maps to the
Zilog's "learning mode" functionality.  Usually I would just tell
users to do the learning under Windows and send me the resulting .ini
file (which we could then add to the database).

I had planned on getting rid of the database entirely and just
converting an MCE compatible pulse train to the blasting format
required by the Zilog firmware (using the awesome work you sent me
privately), but the fact of the matter is that nobody cares and MCEUSB
devices are $20 online.

> I'm glad someone remembers all this stuff.  I'm assuming you had more
> pain with this than I ever did.

This would be a safe assumption.  I probably put about a month's worth
of engineering into driver work for the Zilog, which seems
extraordinary given how simple something like an IR blaster/receiver
is supposed to be.  I guess that's the fun of proving out a new
hardware design as opposed to just making something work under Linux
that is already known to work under Windows.

> I never owned an HD-PVR.

I'm sure I have a spare or two if you really want one (not that you
have the time to muck with such things nowadays).  :-)

The HD-PVR was a bit of a weird case compared to devices like ivtv and
cx18 because it was technically multi-master (I2C commands came both
from the host and from the onboard SOC).  Hence you could have weird
cases where one would block the other at unexpected times.  I2C
commands to the Zilog would hold the bus which would delay the onboard
firmware from issuing commands to the video decoder (fun timing
issues).  There was also some weird edge case I don't recall the
details of that prompted them to add an I2C gate in later board
revisions.

Devin

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

Re: [PATCH v3 04/26] media: lirc_zilog: remove receiver

[Andy Walls]

Hi Sean and Devin:

On Wed, 2017-10-11 at 20:25 -0400, Devin Heitmueller wrote:
> > There's an ir_lock mutex in the driver to prevent simultaneous
> > access to the Rx and Tx functions of the z8.  Accessing Rx and Tx
> > functions of the chip together can cause it to do the wrong thing
> > (sometimes hang?), IIRC.
> > 
> > I'll see if I can dig up my old disassembly of the z8's firmware to
> > see if that interlock is strictly necessary.

Following up on this:

1. The I2C bus command and response buffers inside the Z8 appear to be
independent and non-overlapping (ignore the garbage assembly
mnemonics):

; I2C data buffer addresses
01bf srp   #%00   ; 01 00 ; Buffer pointer I2C addr E0 (70w) IR Tx  - 160 bytes
01c1 add   r0,@r0 ; 03 00 ; Buffer pointer I2C addr E1 (70r) IR Tx  -   4 bytes
01c3 add   r1,@r5 ; 03 15 ; Buffer pointer I2C addr E2 (71w) IR Rx  -   5 bytes
01c5 add   r1,@r0 ; 03 10 ; Buffer pointer I2C addr E3 (71r) IR Rx  -   5 bytes
01c7 add   r0,@r10; 03 0a ; Buffer pointer I2C addr E4 (72w) IR PB1 -   4 bytes
01c9 add   r1,@r10; 03 1a ; Buffer pointer I2C addr E5 (72r) IR PB1 -   4 bytes
01cb add   r0,@r5 ; 03 05 ; Buffer pointer I2C addr E6 (73w) IR Lrn -   1 bytes
01cd add   r0,r0  ; 02 00 ; Buffer pointer I2C addr E7 (73r) IR Lrn - 256 bytes

and the I2C handling routines appear to do the right thing in waiting
for "full" buffers before operating on them.

2. Z8 Port B is used by both Tx and Rx functions, but the functions
each only use 1 line of the I/O port and they use different lines.

3. Z8 Port C is unused.

4. Z8 Port A is used by both Tx functions, Rx functions, the I2C
interface.  If something inside the Z8 screws up the I2C bus comms with
the chip, it's likely the errant misconfiguration of Port A during
certain Rx and Tx operations.

5. Rx and IR Learn both use the same external hardware.  Not
coordinating Rx with Learn mode in the same driver, will prevent Learn
operation from working.  That is, if Learn mode is ever implemented. 
(Once upon a time, I was planning on doing that.  But I have no time
for that anymore.)  

> > Yes I know ir-kbd-i2c is in mainline, but as I said, I had reasons
> > for avoiding it when using Tx operation of the chip.  It's been 7
> > years, and I'm getting too old to remember the reasons. :P
> 
> Yeah, you definitely don't want to be issuing requests to the Rx and
> Tx addresses at the same time.  Very bad things will happen.
> 
> Also, what is the polling interval for ir-kbd-i2c?  If it's too high
> you can wedge the I2C bus (depending on the hardware design).
> Likewise, many people disable IR RX entirely (which is trivial to do
> with lirc_zilog through a modprobe optoin).  This is because early
> versions of the HDPVR had issues where polling too often can
> interfere
> with traffic that configures the component receiver chip.  This was
> improved in later versions of the design, but many people found it
> was
> just easiest to disable RX since they don't use it (i.e. they would
> use the blaster for controlling their STB but use a separate IR
> receiver).
> 
> Are you testing with video capture actively in use?  If you're
> testing
> the IR interface without an active HD capture in progress you're
> likely to miss some of these edge cases (in particular those which
> would hang the encoder).

I'm glad someone remembers all this stuff.  I'm assuming you had more
pain with this than I ever did.  I never owned an HD-PVR.

Regards,
Andy

> I'm not against the removal of duplicate code in general, but you
> have
> to tread carefully because there are plenty of non-obvious edge cases
> to consider.
> 
> Devin
> 

Re: [PATCH v3 04/26] media: lirc_zilog: remove receiver

[Devin Heitmueller]

> There's an ir_lock mutex in the driver to prevent simultaneous access to the 
> Rx and Tx functions of the z8.  Accessing Rx and Tx functions of the chip 
> together can cause it to do the wrong thing (sometimes hang?), IIRC.
>
> I'll see if I can dig up my old disassembly of the z8's firmware to see if 
> that interlock is strictly necessary.
>
> Yes I know ir-kbd-i2c is in mainline, but as I said, I had reasons for 
> avoiding it when using Tx operation of the chip.  It's been 7 years, and I'm 
> getting too old to remember the reasons. :P

Yeah, you definitely don't want to be issuing requests to the Rx and
Tx addresses at the same time.  Very bad things will happen.

Also, what is the polling interval for ir-kbd-i2c?  If it's too high
you can wedge the I2C bus (depending on the hardware design).
Likewise, many people disable IR RX entirely (which is trivial to do
with lirc_zilog through a modprobe optoin).  This is because early
versions of the HDPVR had issues where polling too often can interfere
with traffic that configures the component receiver chip.  This was
improved in later versions of the design, but many people found it was
just easiest to disable RX since they don't use it (i.e. they would
use the blaster for controlling their STB but use a separate IR
receiver).

Are you testing with video capture actively in use?  If you're testing
the IR interface without an active HD capture in progress you're
likely to miss some of these edge cases (in particular those which
would hang the encoder).

I'm not against the removal of duplicate code in general, but you have
to tread carefully because there are plenty of non-obvious edge cases
to consider.

Devin

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

Re: [PATCH v3 04/26] media: lirc_zilog: remove receiver

[Andy Walls]

On October 11, 2017 5:02:37 PM EDT, Sean Young  wrote:
>On Wed, Oct 11, 2017 at 03:43:16PM -0400, Andy Walls wrote:
>> On Tue, 2017-10-10 at 08:17 +0100, Sean Young wrote:
>> > The ir-kbd-i2c module already handles this very well.
>> 
>> Hi Sean:
>> 
>> It's been years, but my recollection is that although ir-kdb-i2c
>might
>> handle receive well, but since the 4 i2c addresses (1 Rx, 1 Tx, 1 IR
>Tx
>> code learning, 1 custom Tx code) are all handled by the same Zilog
>> microcontroller internally, that splitting the Rx and Tx
>functionality
>> between two modules became problematic.
>> 
>> Believe me, if i could have leveraged ir-kdb-i2c back when I ported
>> this, I would have.  I think it's a very bad idea to remove the
>> receiver from lirc_zilog.
>> 
>> The cx18 and ivtv drivers both use lirc_zilog IIRC.  Have you looked
>at
>> the changes required to have the first I2C address used by one i2c
>> module, and the next one (or three) I2C addresses used by another i2c
>> module?
>
>This is already the case.
>
>In current kernels you can use ir-kbd-i2c for Rx and lirc_zilog for Tx,
>this works fine. In fact, the lirc_zilog Rx code produces lirccodes,
>(not mode2) and ir-kbd-i2c produces keycodes through rc-core, so
>ir-kbd-i2c
>(also mainline, not staging) is much more likely to be used for Rx. For
>Tx
>you have to use lirc_zilog.
>
>I haven't heard of any reports of problems (or observed this myself)
>when
>using these two modules together.
>
>As you point out, both drivers sending i2c commands to the same z8f0811
>
>with its z8 encore hand-coded i2c implementation. But they're using
>different
>addresses.
>
>So removing the lirc_zilog Rx doesn't make things worse, in fact, it
>just
>removes some code which is unlikely to be used.
>
>It's the hdpvr, ivtv, cx18 and pvrusb2 drivers which have a Tx
>interface. I
>have hdpvr and ivtv hardware to test.
>
>
>Sean
>
>> 
>> -Andy
>> 
>> 
>> > 
>> > Signed-off-by: Sean Young 
>> > ---
>> >  drivers/staging/media/lirc/lirc_zilog.c | 901
>+++---
>> > --
>> >  1 file changed, 76 insertions(+), 825 deletions(-)
>> > 
>> > diff --git a/drivers/staging/media/lirc/lirc_zilog.c
>> > b/drivers/staging/media/lirc/lirc_zilog.c
>> > index 6bd0717bf76e..757b3fc247ac 100644
>> > --- a/drivers/staging/media/lirc/lirc_zilog.c
>> > +++ b/drivers/staging/media/lirc/lirc_zilog.c
>> > @@ -64,28 +64,7 @@
>> >  /* Max transfer size done by I2C transfer functions */
>> >  #define MAX_XFER_SIZE  64
>> >  
>> > -struct IR;
>> > -
>> > -struct IR_rx {
>> > -  struct kref ref;
>> > -  struct IR *ir;
>> > -
>> > -  /* RX device */
>> > -  struct mutex client_lock;
>> > -  struct i2c_client *c;
>> > -
>> > -  /* RX polling thread data */
>> > -  struct task_struct *task;
>> > -
>> > -  /* RX read data */
>> > -  unsigned char b[3];
>> > -  bool hdpvr_data_fmt;
>> > -};
>> > -
>> >  struct IR_tx {
>> > -  struct kref ref;
>> > -  struct IR *ir;
>> > -
>> >/* TX device */
>> >struct mutex client_lock;
>> >struct i2c_client *c;
>> > @@ -93,39 +72,9 @@ struct IR_tx {
>> >/* TX additional actions needed */
>> >int need_boot;
>> >bool post_tx_ready_poll;
>> > -};
>> > -
>> > -struct IR {
>> > -  struct kref ref;
>> > -  struct list_head list;
>> > -
>> > -  /* FIXME spinlock access to l->features */
>> >struct lirc_dev *l;
>> > -  struct lirc_buffer rbuf;
>> > -
>> > -  struct mutex ir_lock;
>> > -  atomic_t open_count;
>> > -
>> > -  struct device *dev;
>> > -  struct i2c_adapter *adapter;
>> > -
>> > -  spinlock_t rx_ref_lock; /* struct IR_rx kref get()/put() */
>> > -  struct IR_rx *rx;
>> > -
>> > -  spinlock_t tx_ref_lock; /* struct IR_tx kref get()/put() */
>> > -  struct IR_tx *tx;
>> >  };
>> >  
>> > -/* IR transceiver instance object list */
>> > -/*
>> > - * This lock is used for the following:
>> > - * a. ir_devices_list access, insertions, deletions
>> > - * b. struct IR kref get()s and put()s
>> > - * c. serialization of ir_probe() for the two i2c_clients for a Z8
>> > - */
>> > -static DEFINE_MUTEX(ir_devices_lock);
>> > -static LIST_HEAD(ir_devices_list);
>> > -
>> >  /* Block size for IR transmitter */
>> >  #define TX_BLOCK_SIZE 99
>> >  
>> > @@ -153,348 +102,8 @@ struct tx_data_struct {
>> >  static struct tx_data_struct *tx_data;
>> >  static struct mutex tx_data_lock;
>> >  
>> > -
>> >  /* module parameters */
>> >  static bool debug;/* debug output */
>> > -static bool tx_only;  /* only handle the IR Tx function */
>> > -
>> > -
>> > -/* struct IR reference counting */
>> > -static struct IR *get_ir_device(struct IR *ir, bool
>> > ir_devices_lock_held)
>> > -{
>> > -  if (ir_devices_lock_held) {
>> > -  kref_get(>ref);
>> > -  } else {
>> > -  mutex_lock(_devices_lock);
>> > -  kref_get(>ref);
>> > -  mutex_unlock(_devices_lock);
>> > -  }
>> > -  return ir;
>> > -}
>> > -
>> > -static void release_ir_device(struct kref *ref)
>> > -{
>> > -  

Re: [PATCH v3 04/26] media: lirc_zilog: remove receiver

[Sean Young]

On Wed, Oct 11, 2017 at 03:43:16PM -0400, Andy Walls wrote:
> On Tue, 2017-10-10 at 08:17 +0100, Sean Young wrote:
> > The ir-kbd-i2c module already handles this very well.
> 
> Hi Sean:
> 
> It's been years, but my recollection is that although ir-kdb-i2c might
> handle receive well, but since the 4 i2c addresses (1 Rx, 1 Tx, 1 IR Tx
> code learning, 1 custom Tx code) are all handled by the same Zilog
> microcontroller internally, that splitting the Rx and Tx functionality
> between two modules became problematic.
> 
> Believe me, if i could have leveraged ir-kdb-i2c back when I ported
> this, I would have.  I think it's a very bad idea to remove the
> receiver from lirc_zilog.
> 
> The cx18 and ivtv drivers both use lirc_zilog IIRC.  Have you looked at
> the changes required to have the first I2C address used by one i2c
> module, and the next one (or three) I2C addresses used by another i2c
> module?

This is already the case.

In current kernels you can use ir-kbd-i2c for Rx and lirc_zilog for Tx,
this works fine. In fact, the lirc_zilog Rx code produces lirccodes,
(not mode2) and ir-kbd-i2c produces keycodes through rc-core, so ir-kbd-i2c
(also mainline, not staging) is much more likely to be used for Rx. For Tx
you have to use lirc_zilog.

I haven't heard of any reports of problems (or observed this myself) when
using these two modules together.

As you point out, both drivers sending i2c commands to the same z8f0811 
with its z8 encore hand-coded i2c implementation. But they're using different
addresses.

So removing the lirc_zilog Rx doesn't make things worse, in fact, it just
removes some code which is unlikely to be used.

It's the hdpvr, ivtv, cx18 and pvrusb2 drivers which have a Tx interface. I
have hdpvr and ivtv hardware to test.


Sean

> 
> -Andy
> 
> 
> > 
> > Signed-off-by: Sean Young 
> > ---
> >  drivers/staging/media/lirc/lirc_zilog.c | 901 +++---
> > --
> >  1 file changed, 76 insertions(+), 825 deletions(-)
> > 
> > diff --git a/drivers/staging/media/lirc/lirc_zilog.c
> > b/drivers/staging/media/lirc/lirc_zilog.c
> > index 6bd0717bf76e..757b3fc247ac 100644
> > --- a/drivers/staging/media/lirc/lirc_zilog.c
> > +++ b/drivers/staging/media/lirc/lirc_zilog.c
> > @@ -64,28 +64,7 @@
> >  /* Max transfer size done by I2C transfer functions */
> >  #define MAX_XFER_SIZE  64
> >  
> > -struct IR;
> > -
> > -struct IR_rx {
> > -   struct kref ref;
> > -   struct IR *ir;
> > -
> > -   /* RX device */
> > -   struct mutex client_lock;
> > -   struct i2c_client *c;
> > -
> > -   /* RX polling thread data */
> > -   struct task_struct *task;
> > -
> > -   /* RX read data */
> > -   unsigned char b[3];
> > -   bool hdpvr_data_fmt;
> > -};
> > -
> >  struct IR_tx {
> > -   struct kref ref;
> > -   struct IR *ir;
> > -
> > /* TX device */
> > struct mutex client_lock;
> > struct i2c_client *c;
> > @@ -93,39 +72,9 @@ struct IR_tx {
> > /* TX additional actions needed */
> > int need_boot;
> > bool post_tx_ready_poll;
> > -};
> > -
> > -struct IR {
> > -   struct kref ref;
> > -   struct list_head list;
> > -
> > -   /* FIXME spinlock access to l->features */
> > struct lirc_dev *l;
> > -   struct lirc_buffer rbuf;
> > -
> > -   struct mutex ir_lock;
> > -   atomic_t open_count;
> > -
> > -   struct device *dev;
> > -   struct i2c_adapter *adapter;
> > -
> > -   spinlock_t rx_ref_lock; /* struct IR_rx kref get()/put() */
> > -   struct IR_rx *rx;
> > -
> > -   spinlock_t tx_ref_lock; /* struct IR_tx kref get()/put() */
> > -   struct IR_tx *tx;
> >  };
> >  
> > -/* IR transceiver instance object list */
> > -/*
> > - * This lock is used for the following:
> > - * a. ir_devices_list access, insertions, deletions
> > - * b. struct IR kref get()s and put()s
> > - * c. serialization of ir_probe() for the two i2c_clients for a Z8
> > - */
> > -static DEFINE_MUTEX(ir_devices_lock);
> > -static LIST_HEAD(ir_devices_list);
> > -
> >  /* Block size for IR transmitter */
> >  #define TX_BLOCK_SIZE  99
> >  
> > @@ -153,348 +102,8 @@ struct tx_data_struct {
> >  static struct tx_data_struct *tx_data;
> >  static struct mutex tx_data_lock;
> >  
> > -
> >  /* module parameters */
> >  static bool debug; /* debug output */
> > -static bool tx_only;   /* only handle the IR Tx function */
> > -
> > -
> > -/* struct IR reference counting */
> > -static struct IR *get_ir_device(struct IR *ir, bool
> > ir_devices_lock_held)
> > -{
> > -   if (ir_devices_lock_held) {
> > -   kref_get(>ref);
> > -   } else {
> > -   mutex_lock(_devices_lock);
> > -   kref_get(>ref);
> > -   mutex_unlock(_devices_lock);
> > -   }
> > -   return ir;
> > -}
> > -
> > -static void release_ir_device(struct kref *ref)
> > -{
> > -   struct IR *ir = container_of(ref, struct IR, ref);
> > -
> > -   /*
> > -* Things should be in this state by now:
> > -* ir->rx set to NULL and deallocated - happens before ir-
> > >rx->ir put()
> > - 

Re: [PATCH v3 04/26] media: lirc_zilog: remove receiver

[Andy Walls]

On Tue, 2017-10-10 at 08:17 +0100, Sean Young wrote:
> The ir-kbd-i2c module already handles this very well.

Hi Sean:

It's been years, but my recollection is that although ir-kdb-i2c might
handle receive well, but since the 4 i2c addresses (1 Rx, 1 Tx, 1 IR Tx
code learning, 1 custom Tx code) are all handled by the same Zilog
microcontroller internally, that splitting the Rx and Tx functionality
between two modules became problematic.

Believe me, if i could have leveraged ir-kdb-i2c back when I ported
this, I would have.  I think it's a very bad idea to remove the
receiver from lirc_zilog.

The cx18 and ivtv drivers both use lirc_zilog IIRC.  Have you looked at
the changes required to have the first I2C address used by one i2c
module, and the next one (or three) I2C addresses used by another i2c
module?

-Andy


> 
> Signed-off-by: Sean Young 
> ---
>  drivers/staging/media/lirc/lirc_zilog.c | 901 +++---
> --
>  1 file changed, 76 insertions(+), 825 deletions(-)
> 
> diff --git a/drivers/staging/media/lirc/lirc_zilog.c
> b/drivers/staging/media/lirc/lirc_zilog.c
> index 6bd0717bf76e..757b3fc247ac 100644
> --- a/drivers/staging/media/lirc/lirc_zilog.c
> +++ b/drivers/staging/media/lirc/lirc_zilog.c
> @@ -64,28 +64,7 @@
>  /* Max transfer size done by I2C transfer functions */
>  #define MAX_XFER_SIZE  64
>  
> -struct IR;
> -
> -struct IR_rx {
> - struct kref ref;
> - struct IR *ir;
> -
> - /* RX device */
> - struct mutex client_lock;
> - struct i2c_client *c;
> -
> - /* RX polling thread data */
> - struct task_struct *task;
> -
> - /* RX read data */
> - unsigned char b[3];
> - bool hdpvr_data_fmt;
> -};
> -
>  struct IR_tx {
> - struct kref ref;
> - struct IR *ir;
> -
>   /* TX device */
>   struct mutex client_lock;
>   struct i2c_client *c;
> @@ -93,39 +72,9 @@ struct IR_tx {
>   /* TX additional actions needed */
>   int need_boot;
>   bool post_tx_ready_poll;
> -};
> -
> -struct IR {
> - struct kref ref;
> - struct list_head list;
> -
> - /* FIXME spinlock access to l->features */
>   struct lirc_dev *l;
> - struct lirc_buffer rbuf;
> -
> - struct mutex ir_lock;
> - atomic_t open_count;
> -
> - struct device *dev;
> - struct i2c_adapter *adapter;
> -
> - spinlock_t rx_ref_lock; /* struct IR_rx kref get()/put() */
> - struct IR_rx *rx;
> -
> - spinlock_t tx_ref_lock; /* struct IR_tx kref get()/put() */
> - struct IR_tx *tx;
>  };
>  
> -/* IR transceiver instance object list */
> -/*
> - * This lock is used for the following:
> - * a. ir_devices_list access, insertions, deletions
> - * b. struct IR kref get()s and put()s
> - * c. serialization of ir_probe() for the two i2c_clients for a Z8
> - */
> -static DEFINE_MUTEX(ir_devices_lock);
> -static LIST_HEAD(ir_devices_list);
> -
>  /* Block size for IR transmitter */
>  #define TX_BLOCK_SIZE99
>  
> @@ -153,348 +102,8 @@ struct tx_data_struct {
>  static struct tx_data_struct *tx_data;
>  static struct mutex tx_data_lock;
>  
> -
>  /* module parameters */
>  static bool debug;   /* debug output */
> -static bool tx_only; /* only handle the IR Tx function */
> -
> -
> -/* struct IR reference counting */
> -static struct IR *get_ir_device(struct IR *ir, bool
> ir_devices_lock_held)
> -{
> - if (ir_devices_lock_held) {
> - kref_get(>ref);
> - } else {
> - mutex_lock(_devices_lock);
> - kref_get(>ref);
> - mutex_unlock(_devices_lock);
> - }
> - return ir;
> -}
> -
> -static void release_ir_device(struct kref *ref)
> -{
> - struct IR *ir = container_of(ref, struct IR, ref);
> -
> - /*
> -  * Things should be in this state by now:
> -  * ir->rx set to NULL and deallocated - happens before ir-
> >rx->ir put()
> -  * ir->rx->task kthread stopped - happens before ir->rx->ir
> put()
> -  * ir->tx set to NULL and deallocated - happens before ir-
> >tx->ir put()
> -  * ir->open_count ==  0 - happens on final close()
> -  * ir_lock, tx_ref_lock, rx_ref_lock, all released
> -  */
> - if (ir->l)
> - lirc_unregister_device(ir->l);
> -
> - if (kfifo_initialized(>rbuf.fifo))
> - lirc_buffer_free(>rbuf);
> - list_del(>list);
> - kfree(ir);
> -}
> -
> -static int put_ir_device(struct IR *ir, bool ir_devices_lock_held)
> -{
> - int released;
> -
> - if (ir_devices_lock_held)
> - return kref_put(>ref, release_ir_device);
> -
> - mutex_lock(_devices_lock);
> - released = kref_put(>ref, release_ir_device);
> - mutex_unlock(_devices_lock);
> -
> - return released;
> -}
> -
> -/* struct IR_rx reference counting */
> -static struct IR_rx *get_ir_rx(struct IR *ir)
> -{
> - struct IR_rx *rx;
> -
> - spin_lock(>rx_ref_lock);
> - rx = ir->rx;
> - if (rx)
> - kref_get(>ref);
> - spin_unlock(>rx_ref_lock);