[GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Andy Walls]

Mauro,

Please pull the one ir-kbd-i2c change and multiple lirc_zilog changes
for 2.6.38.

The one ir-kbd-i2c change is to put back a case to have ir-kbd-i2c set
defaults for I2C client address 0x71.  I know I was the one who
recommend that ir-kbd-i2c not do this, but I discovered pvrusb2 and bttv
rely on it for the moment - Mea culpa.

The lirc_zilog changes are tested to work with both Tx and Rx with an
HVR-1600.  I don't want to continue much further on lirc_zilog changes,
unitl a few things happen:

1. I have developed, and have had tested, a patch for the pvrusb2 driver
to allow the in kernel lirc_zilog to bind to a Z8 on a pvrusb2 supported
device.

2. Jarrod finishes his changes related to the Z8 chip for hdpvr and they
are pulled into media_tree.git branch.

3. I hear from Jean, or whomever really cares about ir-kbd-i2c, if
adding some new fields for struct IR_i2c_init_data is acceptable.
Specifically, I'd like to add a transceiver_lock mutex, a transceiver
reset callback, and a data pointer for that reset callback.
(Only lirc_zilog would use the reset callback and data pointer.)

4. I find spare time ever again.

Anyway, here's the patchset reference...


The following changes since commit 0a97a683049d83deaf636d18316358065417d87b:

  [media] cpia2: convert .ioctl to .unlocked_ioctl (2011-01-06 11:34:41 -0200)

are available in the git repository at:
  ssh://linuxtv.org/git/awalls/media_tree.git z8

Andy Walls (11):
      lirc_zilog: Reword debug message in ir_probe()
      lirc_zilog: Remove disable_tx module parameter
      lirc_zilog: Split struct IR into structs IR, IR_tx, and IR_rx
      lirc_zilog: Don't make private copies of i2c clients
      lirc_zilog: Extensive rework of ir_probe()/ir_remove()
      lirc_zilog: Update IR Rx polling kthread start/stop and some printks
      lirc_zilog: Remove unneeded tests for existence of the IR Tx function
      lirc_zilog: Remove useless struct i2c_driver.command function
      lirc_zilog: Add Andy Walls to copyright notice and authors list
      lirc_zilog: Update TODO.lirc_zilog
      ir-kbd-i2c: Add back defaults setting for Zilog Z8's at addr 0x71

 drivers/media/video/ir-kbd-i2c.c     |    6 +
 drivers/staging/lirc/TODO.lirc_zilog |   36 ++-
 drivers/staging/lirc/lirc_zilog.c    |  650 ++++++++++++++++++----------------
 3 files changed, 389 insertions(+), 303 deletions(-)


Regards,
Andy

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Andy Walls]

On Sun, 2011-01-16 at 14:20 -0500, Andy Walls wrote:
> Mauro,
> 
> Please pull the one ir-kbd-i2c change and multiple lirc_zilog changes
> for 2.6.38.
> 
> The one ir-kbd-i2c change is to put back a case to have ir-kbd-i2c set
> defaults for I2C client address 0x71.  I know I was the one who
> recommend that ir-kbd-i2c not do this, but I discovered pvrusb2 and bttv
> rely on it for the moment - Mea culpa.
> 
> The lirc_zilog changes are tested to work with both Tx and Rx with an
> HVR-1600.  I don't want to continue much further on lirc_zilog changes,
> unitl a few things happen:
> 
> 1. I have developed, and have had tested, a patch for the pvrusb2 driver
> to allow the in kernel lirc_zilog to bind to a Z8 on a pvrusb2 supported
> device.

Mauro,

I have developed a patch for pvrusb2 and Mike Isely provided his Ack.  I
have added it to my "z8" branch and this pull request.

> 2. Jarrod finishes his changes related to the Z8 chip for hdpvr and they
> are pulled into media_tree.git branch.
> 
> 3. I hear from Jean, or whomever really cares about ir-kbd-i2c, if
> adding some new fields for struct IR_i2c_init_data is acceptable.
> Specifically, I'd like to add a transceiver_lock mutex, a transceiver
> reset callback, and a data pointer for that reset callback.
> (Only lirc_zilog would use the reset callback and data pointer.)
> 
> 4. I find spare time ever again.

Here's the updated changeset information:

The following changes since commit 0a97a683049d83deaf636d18316358065417d87b:

  [media] cpia2: convert .ioctl to .unlocked_ioctl (2011-01-06 11:34:41 -0200)

are available in the git repository at:
  ssh://linuxtv.org/git/awalls/media_tree.git z8

Andy Walls (12):
      lirc_zilog: Reword debug message in ir_probe()
      lirc_zilog: Remove disable_tx module parameter
      lirc_zilog: Split struct IR into structs IR, IR_tx, and IR_rx
      lirc_zilog: Don't make private copies of i2c clients
      lirc_zilog: Extensive rework of ir_probe()/ir_remove()
      lirc_zilog: Update IR Rx polling kthread start/stop and some printks
      lirc_zilog: Remove unneeded tests for existence of the IR Tx function
      lirc_zilog: Remove useless struct i2c_driver.command function
      lirc_zilog: Add Andy Walls to copyright notice and authors list
      lirc_zilog: Update TODO.lirc_zilog
      ir-kbd-i2c: Add back defaults setting for Zilog Z8's at addr 0x71
      pvrusb2: Provide more information about IR units to lirc_zilog and ir-kbd-i2c

 drivers/media/video/ir-kbd-i2c.c                   |    6 +
 drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h |    2 +
 drivers/media/video/pvrusb2/pvrusb2-i2c-core.c     |   62 ++-
 drivers/staging/lirc/TODO.lirc_zilog               |   36 +-
 drivers/staging/lirc/lirc_zilog.c                  |  650 +++++++++++---------
 5 files changed, 434 insertions(+), 322 deletions(-)


Regards,
Andy

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jarod Wilson]

On Jan 16, 2011, at 10:29 PM, Andy Walls wrote:

> On Sun, 2011-01-16 at 14:20 -0500, Andy Walls wrote:
>> Mauro,
>> 
>> Please pull the one ir-kbd-i2c change and multiple lirc_zilog changes
>> for 2.6.38.
>> 
>> The one ir-kbd-i2c change is to put back a case to have ir-kbd-i2c set
>> defaults for I2C client address 0x71.  I know I was the one who
>> recommend that ir-kbd-i2c not do this, but I discovered pvrusb2 and bttv
>> rely on it for the moment - Mea culpa.
>> 
>> The lirc_zilog changes are tested to work with both Tx and Rx with an
>> HVR-1600.  I don't want to continue much further on lirc_zilog changes,
>> unitl a few things happen:
>> 
>> 1. I have developed, and have had tested, a patch for the pvrusb2 driver
>> to allow the in kernel lirc_zilog to bind to a Z8 on a pvrusb2 supported
>> device.
> 
> Mauro,
> 
> I have developed a patch for pvrusb2 and Mike Isely provided his Ack.  I
> have added it to my "z8" branch and this pull request.

I've finally got around to trying it out with the HVR-1950 I've got here,
and it does do the trick for ir-kbd-i2c (albeit I never see proper key
repeats, only alternating press/release key events). Not working with
lirc_zilog yet, it fails to load, due to an -EIO ret to one of the
i2c_master_send() calls in lirc_zilog during probe of the TX side. Haven't
looked into it any more than that yet.


>> 2. Jarrod finishes his changes related to the Z8 chip for hdpvr and they
>> are pulled into media_tree.git branch.

They're in now. Still need to tweak the ir-kbd-i2c usage by hdpvr a bit,
but at least I'm close to having other hardware to compare and contrast
with, behavior-wise.


>> 3. I hear from Jean, or whomever really cares about ir-kbd-i2c, if
>> adding some new fields for struct IR_i2c_init_data is acceptable.
>> Specifically, I'd like to add a transceiver_lock mutex, a transceiver
>> reset callback, and a data pointer for that reset callback.
>> (Only lirc_zilog would use the reset callback and data pointer.)
>> 
>> 4. I find spare time ever again.

Ha, I feel your pain... ;)

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Andy Walls]

On Wed, 2011-01-19 at 00:20 -0500, Jarod Wilson wrote:
> On Jan 16, 2011, at 10:29 PM, Andy Walls wrote:
> 
> > On Sun, 2011-01-16 at 14:20 -0500, Andy Walls wrote:
> >> Mauro,
> >> 
> >> Please pull the one ir-kbd-i2c change and multiple lirc_zilog changes
> >> for 2.6.38.
> >> 
> >> The one ir-kbd-i2c change is to put back a case to have ir-kbd-i2c set
> >> defaults for I2C client address 0x71.  I know I was the one who
> >> recommend that ir-kbd-i2c not do this, but I discovered pvrusb2 and bttv
> >> rely on it for the moment - Mea culpa.
> >> 
> >> The lirc_zilog changes are tested to work with both Tx and Rx with an
> >> HVR-1600.  I don't want to continue much further on lirc_zilog changes,
> >> unitl a few things happen:
> >> 
> >> 1. I have developed, and have had tested, a patch for the pvrusb2 driver
> >> to allow the in kernel lirc_zilog to bind to a Z8 on a pvrusb2 supported
> >> device.
> > 
> > Mauro,
> > 
> > I have developed a patch for pvrusb2 and Mike Isely provided his Ack.  I
> > have added it to my "z8" branch and this pull request.
> 
> I've finally got around to trying it out with the HVR-1950 I've got here,
> and it does do the trick for ir-kbd-i2c (albeit I never see proper key
> repeats, only alternating press/release key events).

Yay, a small victory.

I explicitly set the polling interval to 260 ms in pvrusb2, based on
your hdpvr findings and lirc_zilog code.  I guess you can try tweaking
that back to 100 ms.


>  Not working with
> lirc_zilog yet, it fails to load, due to an -EIO ret to one of the
> i2c_master_send() calls in lirc_zilog during probe of the TX side. Haven't
> looked into it any more than that yet.

Well technically lirc_zilog doesn't "probe" anymore.  It relies on the
bridge driver telling it the truth.

But yes, lirc_zilog is overly sensitive to anything not being right
during lirc_zilog.c:ir_probe().

BTW, does your HVR-1950 have a blaster?  The simple code I added to
pvrusb2 doesn't try to detect a unit on address 0x70.  It always assumes
the Z8 is Tx capable.

With the cx18 and ivtv cards, the Hauppauge EEPROM indicates whether a
blaster is present.  For those bridge drivers, I can communicate that
information to the IR modules.

For the hdpvr and pvrusb2, my short term plan was to always assume a Z8
could Tx, and make lirc_zilog not barf when it couldn't talk to a Tx
unit.  I notice that Mike had written some short, simple IR unit
detection code here for other reasons:

http://git.linuxtv.org/media_tree.git?a=blob;f=drivers/media/video/pvrusb2/pvrusb2-i2c-core.c;h=ccc884948f34b385563ccbf548c5f80b33cd4f08;hb=refs/heads/staging/for_2.6.38-rc1#l661
http://git.linuxtv.org/media_tree.git?a=blob;f=drivers/media/video/pvrusb2/pvrusb2-i2c-core.c;h=ccc884948f34b385563ccbf548c5f80b33cd4f08;hb=refs/heads/staging/for_2.6.38-rc1#l542

For debugging, you might want to hack in a probe of address 0x70 for
your HVR-1950, to ensure the Tx side responds in the bridge driver. 

Regards,
Andy

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jean Delvare]

On Wed, 19 Jan 2011 07:21:58 -0500, Andy Walls wrote:
> For debugging, you might want to hack in a probe of address 0x70 for
> your HVR-1950, to ensure the Tx side responds in the bridge driver. 

... keeping in mind that the Z8 doesn't seem to like quick writes, so
short reads should be used for probing purpose.

-- 
Jean Delvare

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Andy Walls]

On Wed, 2011-01-19 at 13:40 +0100, Jean Delvare wrote:
> On Wed, 19 Jan 2011 07:21:58 -0500, Andy Walls wrote:
> > For debugging, you might want to hack in a probe of address 0x70 for
> > your HVR-1950, to ensure the Tx side responds in the bridge driver. 
> 
> ... keeping in mind that the Z8 doesn't seem to like quick writes, so
> short reads should be used for probing purpose.
> 

Noted.  Thanks.

Actually, I think that might be due to the controller in the USB
connected devices (hdpvr and pvrusb2).  The PCI connected devices, like
cx18 cards, don't have a problem with the Z8, the default I2C probe
method, and i2c-algo-bit.
(A good example of why only bridge drivers should do any required
probing.)


Looking at the code in pvrusb2, it appears to already use a 0 length
read for a probe:

http://git.linuxtv.org/media_tree.git?a=blob;f=drivers/media/video/pvrusb2/pvrusb2-i2c-core.c;h=ccc884948f34b385563ccbf548c5f80b33cd4f08;hb=refs/heads/staging/for_2.6.38-rc1#l542

Regards,
Andy

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Mike Isely]

On Wed, 19 Jan 2011, Andy Walls wrote:

> On Wed, 2011-01-19 at 13:40 +0100, Jean Delvare wrote:
> > On Wed, 19 Jan 2011 07:21:58 -0500, Andy Walls wrote:
> > > For debugging, you might want to hack in a probe of address 0x70 for
> > > your HVR-1950, to ensure the Tx side responds in the bridge driver. 
> > 
> > ... keeping in mind that the Z8 doesn't seem to like quick writes, so
> > short reads should be used for probing purpose.
> > 
> 
> Noted.  Thanks.
> 
> Actually, I think that might be due to the controller in the USB
> connected devices (hdpvr and pvrusb2).  The PCI connected devices, like
> cx18 cards, don't have a problem with the Z8, the default I2C probe
> method, and i2c-algo-bit.
> (A good example of why only bridge drivers should do any required
> probing.)
> 
> 
> Looking at the code in pvrusb2, it appears to already use a 0 length
> read for a probe:
> 
> http://git.linuxtv.org/media_tree.git?a=blob;f=drivers/media/video/pvrusb2/pvrusb2-i2c-core.c;h=ccc884948f34b385563ccbf548c5f80b33cd4f08;hb=refs/heads/staging/for_2.6.38-rc1#l542

Yes but that function is used in two places: (1) If a bus scan is 
performed during initialization (normally it isn't), and (2) it is 
called once ONLY for a 24xxx device (targeting 0x71) in order to 
determine if it is dealing with the MCE variant.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Mike Isely]

On Wed, 19 Jan 2011, Andy Walls wrote:

> On Wed, 2011-01-19 at 00:20 -0500, Jarod Wilson wrote:
> 
> 
> >  Not working with
> > lirc_zilog yet, it fails to load, due to an -EIO ret to one of the
> > i2c_master_send() calls in lirc_zilog during probe of the TX side. Haven't
> > looked into it any more than that yet.
> 
> Well technically lirc_zilog doesn't "probe" anymore.  It relies on the
> bridge driver telling it the truth.

The bridge driver (pvrusb2) still does one probe if it's a 24xxx device: 
It probes 0x71 in order to determine if it is dealing with an MCE 
variant device.  Hauppauge did not change the USB ID when they released 
the 24xxx MCE variant (which has the IR blaster, thus the zilog device).  
The only way to tell the two devices apart is by discovering the 
existence of the zilog device - and the bridge driver needs to do this 
in order to properly disable its "emulated" I2C IR receiver which would 
otherwise be needed for the non-MCE device.

Based on the discussion here, could that probe be a source of trouble on 
the 24XXX MCE device?

This probing behavior does not happen for HVR-1950 (or HVR-1900) since 
there's only one possible IR configuration there.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Andy Walls]

On Wed, 2011-01-19 at 07:20 -0600, Mike Isely wrote:
> On Wed, 19 Jan 2011, Andy Walls wrote:
> 
> > On Wed, 2011-01-19 at 00:20 -0500, Jarod Wilson wrote:
> > 
> > 
> > >  Not working with
> > > lirc_zilog yet, it fails to load, due to an -EIO ret to one of the
> > > i2c_master_send() calls in lirc_zilog during probe of the TX side. Haven't
> > > looked into it any more than that yet.
> > 
> > Well technically lirc_zilog doesn't "probe" anymore.  It relies on the
> > bridge driver telling it the truth.
> 
> The bridge driver (pvrusb2) still does one probe if it's a 24xxx device: 
> It probes 0x71 in order to determine if it is dealing with an MCE 
> variant device.  Hauppauge did not change the USB ID when they released 
> the 24xxx MCE variant (which has the IR blaster, thus the zilog device).  
> The only way to tell the two devices apart is by discovering the 
> existence of the zilog device - and the bridge driver needs to do this 
> in order to properly disable its "emulated" I2C IR receiver which would 
> otherwise be needed for the non-MCE device.
> 
> Based on the discussion here, could that probe be a source of trouble on 
> the 24XXX MCE device?

IMO, No. I think it is needed and just fine.

As I understand it, the rules/guidelines for I2C probing are now
something like this:

1. I2C device driver modules (ir-kbd-i2c, lirc_zilog, etc.) should not
do hardware probes at all.  They are to assume the bridge or platform
drivers verified the I2C slave hardware's existence somehow.

2. Bridge drivers (pvrusb, hdpvr, cx18, ivtv, etc.) should not ask the
I2C subsystem to probe hardware that it knows for sure exists, or knows
for sure does not exist.  Just add the I2C device or not.

3. Bridge drivers should generally ask the I2C subsystem to probe for
hardware that _may_ exist.

4. If the default I2C subsystem hardware probe method doesn't work on a
particular hardware unit, the bridge driver may perform its own hardware
probe or provide a custom hardware probe method to the I2C subsystem.
hdpvr and pvrusb2 currently do the former.


> This probing behavior does not happen for HVR-1950 (or HVR-1900) since 
> there's only one possible IR configuration there.

So the HVR-1950 only has Z8's capable of both Tx and Rx?  No HVR-1950
has an Rx only Z8 unit?

Regards,
Andy

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jean Delvare]

Hi Andy,

On Wed, 19 Jan 2011 08:38:02 -0500, Andy Walls wrote:
> As I understand it, the rules/guidelines for I2C probing are now
> something like this:
> 
> 1. I2C device driver modules (ir-kbd-i2c, lirc_zilog, etc.) should not
> do hardware probes at all.  They are to assume the bridge or platform
> drivers verified the I2C slave hardware's existence somehow.
> 
> 2. Bridge drivers (pvrusb, hdpvr, cx18, ivtv, etc.) should not ask the
> I2C subsystem to probe hardware that it knows for sure exists, or knows
> for sure does not exist.  Just add the I2C device or not.
> 
> 3. Bridge drivers should generally ask the I2C subsystem to probe for
> hardware that _may_ exist.
> 
> 4. If the default I2C subsystem hardware probe method doesn't work on a
> particular hardware unit, the bridge driver may perform its own hardware
> probe or provide a custom hardware probe method to the I2C subsystem.
> hdpvr and pvrusb2 currently do the former.

Yes, that's exactly how things are supposed to work now. And hopefully
it makes sense and helps you all write cleaner code (that was the
intent at least.)

-- 
Jean Delvare

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jarod Wilson]

On Jan 19, 2011, at 8:50 AM, Jean Delvare wrote:

> Hi Andy,
> 
> On Wed, 19 Jan 2011 08:38:02 -0500, Andy Walls wrote:
>> As I understand it, the rules/guidelines for I2C probing are now
>> something like this:
>> 
>> 1. I2C device driver modules (ir-kbd-i2c, lirc_zilog, etc.) should not
>> do hardware probes at all.  They are to assume the bridge or platform
>> drivers verified the I2C slave hardware's existence somehow.
>> 
>> 2. Bridge drivers (pvrusb, hdpvr, cx18, ivtv, etc.) should not ask the
>> I2C subsystem to probe hardware that it knows for sure exists, or knows
>> for sure does not exist.  Just add the I2C device or not.
>> 
>> 3. Bridge drivers should generally ask the I2C subsystem to probe for
>> hardware that _may_ exist.
>> 
>> 4. If the default I2C subsystem hardware probe method doesn't work on a
>> particular hardware unit, the bridge driver may perform its own hardware
>> probe or provide a custom hardware probe method to the I2C subsystem.
>> hdpvr and pvrusb2 currently do the former.
> 
> Yes, that's exactly how things are supposed to work now. And hopefully
> it makes sense and helps you all write cleaner code (that was the
> intent at least.)

One more i2c question...

Am I correct in assuming that since the zilog is a single device, which
can be accessed via two different addresses (0x70 for tx, 0x71 for rx),
that i2c_new_device() just once with both addresses in i2c_board_info
is correct, vs. calling i2c_new_device() once for each address?

At least, I'm reasonably sure that was the key to making the hdpvr IR
behave with lirc_zilog, and after lunch, I should know if that's also
the case for pvrusb2 devices w/a zilog IR chip.

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jarod Wilson]

On Jan 19, 2011, at 12:12 PM, Jarod Wilson wrote:

> On Jan 19, 2011, at 8:50 AM, Jean Delvare wrote:
> 
>> Hi Andy,
>> 
>> On Wed, 19 Jan 2011 08:38:02 -0500, Andy Walls wrote:
>>> As I understand it, the rules/guidelines for I2C probing are now
>>> something like this:
>>> 
>>> 1. I2C device driver modules (ir-kbd-i2c, lirc_zilog, etc.) should not
>>> do hardware probes at all.  They are to assume the bridge or platform
>>> drivers verified the I2C slave hardware's existence somehow.
>>> 
>>> 2. Bridge drivers (pvrusb, hdpvr, cx18, ivtv, etc.) should not ask the
>>> I2C subsystem to probe hardware that it knows for sure exists, or knows
>>> for sure does not exist.  Just add the I2C device or not.
>>> 
>>> 3. Bridge drivers should generally ask the I2C subsystem to probe for
>>> hardware that _may_ exist.
>>> 
>>> 4. If the default I2C subsystem hardware probe method doesn't work on a
>>> particular hardware unit, the bridge driver may perform its own hardware
>>> probe or provide a custom hardware probe method to the I2C subsystem.
>>> hdpvr and pvrusb2 currently do the former.
>> 
>> Yes, that's exactly how things are supposed to work now. And hopefully
>> it makes sense and helps you all write cleaner code (that was the
>> intent at least.)
> 
> One more i2c question...
> 
> Am I correct in assuming that since the zilog is a single device, which
> can be accessed via two different addresses (0x70 for tx, 0x71 for rx),
> that i2c_new_device() just once with both addresses in i2c_board_info
> is correct, vs. calling i2c_new_device() once for each address?
> 
> At least, I'm reasonably sure that was the key to making the hdpvr IR
> behave with lirc_zilog, and after lunch, I should know if that's also
> the case for pvrusb2 devices w/a zilog IR chip.

Actually, in looking at things closer and talking to Andy on irc, it
looks like this:

static struct i2c_board_info pvr2_xcvr_i2c_board_info = {
        I2C_BOARD_INFO("ir_tx_z8f0811_haup", Z8F0811_IR_TX_I2C_ADDR),
        I2C_BOARD_INFO("ir_rx_z8f0811_haup", Z8F0811_IR_RX_I2C_ADDR),
};

Expands to:

static struct i2c_board_info pvr2_xcvr_i2c_board_info = {
        .type = "ir_tx_z8f0811_haup",
	.addr = Z8F0811_IR_TX_I2C_ADDR,
        .type = "ir_rx_z8f0811_haup",
	.addr = Z8F0811_IR_RX_I2C_ADDR,
};

In which case, we're actually only registering 0x71 -- i2c_new_device()
certainly only appears to act on a single address, anyway.

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jean Delvare]

On Wed, 19 Jan 2011 12:12:49 -0500, Jarod Wilson wrote:
> On Jan 19, 2011, at 8:50 AM, Jean Delvare wrote:
> 
> > Hi Andy,
> > 
> > On Wed, 19 Jan 2011 08:38:02 -0500, Andy Walls wrote:
> >> As I understand it, the rules/guidelines for I2C probing are now
> >> something like this:
> >> 
> >> 1. I2C device driver modules (ir-kbd-i2c, lirc_zilog, etc.) should not
> >> do hardware probes at all.  They are to assume the bridge or platform
> >> drivers verified the I2C slave hardware's existence somehow.
> >> 
> >> 2. Bridge drivers (pvrusb, hdpvr, cx18, ivtv, etc.) should not ask the
> >> I2C subsystem to probe hardware that it knows for sure exists, or knows
> >> for sure does not exist.  Just add the I2C device or not.
> >> 
> >> 3. Bridge drivers should generally ask the I2C subsystem to probe for
> >> hardware that _may_ exist.
> >> 
> >> 4. If the default I2C subsystem hardware probe method doesn't work on a
> >> particular hardware unit, the bridge driver may perform its own hardware
> >> probe or provide a custom hardware probe method to the I2C subsystem.
> >> hdpvr and pvrusb2 currently do the former.
> > 
> > Yes, that's exactly how things are supposed to work now. And hopefully
> > it makes sense and helps you all write cleaner code (that was the
> > intent at least.)
> 
> One more i2c question...
> 
> Am I correct in assuming that since the zilog is a single device, which
> can be accessed via two different addresses (0x70 for tx, 0x71 for rx),
> that i2c_new_device() just once with both addresses in i2c_board_info
> is correct, vs. calling i2c_new_device() once for each address?

Preliminary technical nitpicking: you can't actually pass two addresses
in i2c_board_info, so the second address has to be passed as platform
data.

I am sorry if you expected an authoritative answer, but... both options
are actually possible.

If you use a single call to i2c_new_device(), you'll have a single
i2c_client to start with, and you'll have to instantiate the second one
in the probe function using i2c_new_dummy().

If you instead decide to call i2c_new_device() twice, there will be two
calls to the probe function (which can be the same one in a single
driver, or two different ones in separate drivers, at your option.) If
any synchronization is needed between the two i2c_clients, you have to
use the bridge driver as a relay, as Andy proposed doing already.

Really, both are possible, and the two options aren't that different in
the end. I can't think of anything that can be done with one that
couldn't be achieved with the other.

> At least, I'm reasonably sure that was the key to making the hdpvr IR
> behave with lirc_zilog, and after lunch, I should know if that's also
> the case for pvrusb2 devices w/a zilog IR chip.

-- 
Jean Delvare

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jarod Wilson]

On Jan 19, 2011, at 12:43 PM, Jean Delvare wrote:

> On Wed, 19 Jan 2011 12:12:49 -0500, Jarod Wilson wrote:
>> On Jan 19, 2011, at 8:50 AM, Jean Delvare wrote:
>> 
>>> Hi Andy,
>>> 
>>> On Wed, 19 Jan 2011 08:38:02 -0500, Andy Walls wrote:
>>>> As I understand it, the rules/guidelines for I2C probing are now
>>>> something like this:
>>>> 
>>>> 1. I2C device driver modules (ir-kbd-i2c, lirc_zilog, etc.) should not
>>>> do hardware probes at all.  They are to assume the bridge or platform
>>>> drivers verified the I2C slave hardware's existence somehow.
>>>> 
>>>> 2. Bridge drivers (pvrusb, hdpvr, cx18, ivtv, etc.) should not ask the
>>>> I2C subsystem to probe hardware that it knows for sure exists, or knows
>>>> for sure does not exist.  Just add the I2C device or not.
>>>> 
>>>> 3. Bridge drivers should generally ask the I2C subsystem to probe for
>>>> hardware that _may_ exist.
>>>> 
>>>> 4. If the default I2C subsystem hardware probe method doesn't work on a
>>>> particular hardware unit, the bridge driver may perform its own hardware
>>>> probe or provide a custom hardware probe method to the I2C subsystem.
>>>> hdpvr and pvrusb2 currently do the former.
>>> 
>>> Yes, that's exactly how things are supposed to work now. And hopefully
>>> it makes sense and helps you all write cleaner code (that was the
>>> intent at least.)
>> 
>> One more i2c question...
>> 
>> Am I correct in assuming that since the zilog is a single device, which
>> can be accessed via two different addresses (0x70 for tx, 0x71 for rx),
>> that i2c_new_device() just once with both addresses in i2c_board_info
>> is correct, vs. calling i2c_new_device() once for each address?
> 
> Preliminary technical nitpicking: you can't actually pass two addresses
> in i2c_board_info, so the second address has to be passed as platform
> data.
> 
> I am sorry if you expected an authoritative answer, but... both options
> are actually possible.
> 
> If you use a single call to i2c_new_device(), you'll have a single
> i2c_client to start with, and you'll have to instantiate the second one
> in the probe function using i2c_new_dummy().
> 
> If you instead decide to call i2c_new_device() twice, there will be two
> calls to the probe function (which can be the same one in a single
> driver, or two different ones in separate drivers, at your option.) If
> any synchronization is needed between the two i2c_clients, you have to
> use the bridge driver as a relay, as Andy proposed doing already.
> 
> Really, both are possible, and the two options aren't that different in
> the end. I can't think of anything that can be done with one that
> couldn't be achieved with the other.

Yeah, see my follow-up mail. The code in hdpvr-i2c.c is clearly a bit
off now, and only worked in my testing, as at the time, I was using
an older lirc_zilog from prior to Andy's changes that still used the
old style binding and probing directly in the driver. I'm working on
fixing up hdpvr-i2c further right now, and will do some more prodding
with pvrusb2, the code for which looks correct with two i2c_new_device()
calls in it, one for each address, so I just need to figure out why
lirc_zilog is getting an -EIO trying to get tx brought up.

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jarod Wilson]

On Jan 19, 2011, at 3:08 PM, Jarod Wilson wrote:

> On Jan 19, 2011, at 12:43 PM, Jean Delvare wrote:
> 
>> Preliminary technical nitpicking: you can't actually pass two addresses
>> in i2c_board_info, so the second address has to be passed as platform
>> data.
>> 
>> I am sorry if you expected an authoritative answer, but... both options
>> are actually possible.
>> 
>> If you use a single call to i2c_new_device(), you'll have a single
>> i2c_client to start with, and you'll have to instantiate the second one
>> in the probe function using i2c_new_dummy().
>> 
>> If you instead decide to call i2c_new_device() twice, there will be two
>> calls to the probe function (which can be the same one in a single
>> driver, or two different ones in separate drivers, at your option.) If
>> any synchronization is needed between the two i2c_clients, you have to
>> use the bridge driver as a relay, as Andy proposed doing already.
>> 
>> Really, both are possible, and the two options aren't that different in
>> the end. I can't think of anything that can be done with one that
>> couldn't be achieved with the other.
> 
> Yeah, see my follow-up mail. The code in hdpvr-i2c.c is clearly a bit
> off now, and only worked in my testing, as at the time, I was using
> an older lirc_zilog from prior to Andy's changes that still used the
> old style binding and probing directly in the driver. I'm working on
> fixing up hdpvr-i2c further right now, and will do some more prodding
> with pvrusb2, the code for which looks correct with two i2c_new_device()
> calls in it, one for each address, so I just need to figure out why
> lirc_zilog is getting an -EIO trying to get tx brought up.

So as we were discussing on irc today, the -EIO is within lirc_zilog's
send_boot_data() function. The firmware is loaded, and then we send the
z8 a command to activate the firmware, immediately follow by an attempt
to read the firmware version. The z8 is still busy when we do that, and
throwing in a simple mdelay() remedies the problem for both the hvr-1950
and the hdpvr -- tried 100 initially, and all the way down to 20 still
worked, didn't try any lower.

And I definitely horked up the hdpvr i2c a bit, but have a follow-up
patch that goes back to doing the right thing with two i2c_new_device()
calls, which I've successfully tested with the latest lirc_zilog plus
mdelay patch.

Will post patches tomorrow though, its already past my bed time.

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jarod Wilson]

On Jan 19, 2011, at 11:45 PM, Jarod Wilson wrote:

> So as we were discussing on irc today, the -EIO is within lirc_zilog's
> send_boot_data() function. The firmware is loaded, and then we send the
> z8 a command to activate the firmware, immediately follow by an attempt
> to read the firmware version. The z8 is still busy when we do that, and
> throwing in a simple mdelay() remedies the problem for both the hvr-1950
> and the hdpvr -- tried 100 initially, and all the way down to 20 still
> worked, didn't try any lower.
> 
> And I definitely horked up the hdpvr i2c a bit, but have a follow-up
> patch that goes back to doing the right thing with two i2c_new_device()
> calls, which I've successfully tested with the latest lirc_zilog plus
> mdelay patch.
> 
> Will post patches tomorrow though, its already past my bed time.

D'oh. Forgot to mention: while lirc_zilog tx binds to the hvr-1950, it
doesn't actually work, I get -EIO when trying to transmit, iirc. So that
is on the list of things to poke at some tomorrow as well.

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Andy Walls]

On Wed, 2011-01-19 at 23:45 -0500, Jarod Wilson wrote:
> On Jan 19, 2011, at 3:08 PM, Jarod Wilson wrote:

> > I'm working on
> > fixing up hdpvr-i2c further right now, and will do some more prodding
> > with pvrusb2, the code for which looks correct with two i2c_new_device()
> > calls in it, one for each address, so I just need to figure out why
> > lirc_zilog is getting an -EIO trying to get tx brought up.
> 
> So as we were discussing on irc today, the -EIO is within lirc_zilog's
> send_boot_data() function. The firmware is loaded, and then we send the
> z8 a command to activate the firmware, immediately follow by an attempt
> to read the firmware version. The z8 is still busy when we do that, and
> throwing in a simple mdelay() remedies the problem for both the hvr-1950
> and the hdpvr -- tried 100 initially, and all the way down to 20 still
> worked, didn't try any lower.

The Z8 on my HVR-1600 is using a 18.432 MHz crystal for its clock.

The Z8 CPU Fetch and Execution units are running with a pipeline depth
of 1: 1 insn being executed while another 1 insn is being fetched.  Most
Z8 fetch or execution cycle counts are in the range of 2-4 cycles.  So
let's just assume an insn takes 4 cycles to execute.

	18.432 MHz * 20 ms = 368,640 cycles 
	368,640 cycles / 4 cycles/insn = 92,160 insns

20 ms is ~90k instructions, and seems like too long a delay to be just
for Z8 latency. 

I find it interesting that for the HVR-1600 the delay isn't needed at
all.

I'm wondering if there might also be some Linux/USB latency in getting
commands shoved over to the HVR-1950's controller (or maybe latency in
the HVR-1950's controller too), for which this delay is really
accounting.  I suppose in kernel tracing can be used to find the latency
on shoving things across the USB and watching for any Ack from the
HVR-1950 controller.  An experiment for some other day, I guess.



> And I definitely horked up the hdpvr i2c a bit, but have a follow-up
> patch that goes back to doing the right thing with two i2c_new_device()
> calls, which I've successfully tested with the latest lirc_zilog plus
> mdelay patch.

Yay!

Regards,
Andy

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jarod Wilson]

On Jan 20, 2011, at 8:22 AM, Andy Walls wrote:

> On Wed, 2011-01-19 at 23:45 -0500, Jarod Wilson wrote:
>> On Jan 19, 2011, at 3:08 PM, Jarod Wilson wrote:
> 
>>> I'm working on
>>> fixing up hdpvr-i2c further right now, and will do some more prodding
>>> with pvrusb2, the code for which looks correct with two i2c_new_device()
>>> calls in it, one for each address, so I just need to figure out why
>>> lirc_zilog is getting an -EIO trying to get tx brought up.
>> 
>> So as we were discussing on irc today, the -EIO is within lirc_zilog's
>> send_boot_data() function. The firmware is loaded, and then we send the
>> z8 a command to activate the firmware, immediately follow by an attempt
>> to read the firmware version. The z8 is still busy when we do that, and
>> throwing in a simple mdelay() remedies the problem for both the hvr-1950
>> and the hdpvr -- tried 100 initially, and all the way down to 20 still
>> worked, didn't try any lower.
> 
> The Z8 on my HVR-1600 is using a 18.432 MHz crystal for its clock.
> 
> The Z8 CPU Fetch and Execution units are running with a pipeline depth
> of 1: 1 insn being executed while another 1 insn is being fetched.  Most
> Z8 fetch or execution cycle counts are in the range of 2-4 cycles.  So
> let's just assume an insn takes 4 cycles to execute.
> 
> 	18.432 MHz * 20 ms = 368,640 cycles 
> 	368,640 cycles / 4 cycles/insn = 92,160 insns
> 
> 20 ms is ~90k instructions, and seems like too long a delay to be just
> for Z8 latency.

Some further testing today with a try-check success-delay-retry loop
shows one i2c_master_send() failure plus a udelay(100), and the second
i2c_master_send() typically always works (I haven't seen it *not* work
on the HVR-1950 in cursory testing).

A second similar loop has proven necessary for IR TX attempts to actually
claim to succeed -- not 100% certain they really worked, as I don't have
the IR emitter with me at the moment, its at home hooked up to my hdpvr,
but I suspect its working fine now.

> I find it interesting that for the HVR-1600 the delay isn't needed at
> all.
> 
> I'm wondering if there might also be some Linux/USB latency in getting
> commands shoved over to the HVR-1950's controller (or maybe latency in
> the HVR-1950's controller too), for which this delay is really
> accounting.  I suppose in kernel tracing can be used to find the latency
> on shoving things across the USB and watching for any Ack from the
> HVR-1950 controller.  An experiment for some other day, I guess.

Yeah, definitely seems to be specific to usb devices. On the plus side,
the delay loops should only add insignificant delay overhead for pci
devices, since if they work on the first call, there won't be any delay
added.

Oh, I've also got IR RX with ir-kbd-i2c attached to the HVR-1950 working
much better now, with the polling interval reverted to the standard
100ms, but simply introducing the same i2c_master_send() poll that
lirc_zilog uses into get_key_haup_xvr().

I want to test today's changes with the hdpvr tonight (and verify that
tx is working on the HVR-1950) before I send along my current stack of
patches, and I have suspicions that most of the hdpvr-specific crud in
lirc_zilog is bogus now -- it *should* behave exactly like the HVR-1950,
which obviously doesn't follow any of those hdpvr-specific code paths,
so I'm hoping we can rip out some additional complexity from lirc_zilog.

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Andy Walls]

On Thu, 2011-01-20 at 16:49 -0500, Jarod Wilson wrote:
> On Jan 20, 2011, at 8:22 AM, Andy Walls wrote:
> 
> > On Wed, 2011-01-19 at 23:45 -0500, Jarod Wilson wrote:
> >> On Jan 19, 2011, at 3:08 PM, Jarod Wilson wrote:
> > 
> >>> I'm working on
> >>> fixing up hdpvr-i2c further right now, and will do some more prodding
> >>> with pvrusb2, the code for which looks correct with two i2c_new_device()
> >>> calls in it, one for each address, so I just need to figure out why
> >>> lirc_zilog is getting an -EIO trying to get tx brought up.
> >> 
> >> So as we were discussing on irc today, the -EIO is within lirc_zilog's
> >> send_boot_data() function. The firmware is loaded, and then we send the
> >> z8 a command to activate the firmware, immediately follow by an attempt
> >> to read the firmware version. The z8 is still busy when we do that, and
> >> throwing in a simple mdelay() remedies the problem for both the hvr-1950
> >> and the hdpvr -- tried 100 initially, and all the way down to 20 still
> >> worked, didn't try any lower.
> > 
> > The Z8 on my HVR-1600 is using a 18.432 MHz crystal for its clock.
> > 
> > The Z8 CPU Fetch and Execution units are running with a pipeline depth
> > of 1: 1 insn being executed while another 1 insn is being fetched.  Most
> > Z8 fetch or execution cycle counts are in the range of 2-4 cycles.  So
> > let's just assume an insn takes 4 cycles to execute.
> > 
> > 	18.432 MHz * 20 ms = 368,640 cycles 
> > 	368,640 cycles / 4 cycles/insn = 92,160 insns
> > 
> > 20 ms is ~90k instructions, and seems like too long a delay to be just
> > for Z8 latency.
> 
> Some further testing today with a try-check success-delay-retry loop
> shows one i2c_master_send() failure plus a udelay(100), and the second
> i2c_master_send() typically always works (I haven't seen it *not* work
> on the HVR-1950 in cursory testing).

Cool.

> A second similar loop has proven necessary for IR TX attempts to actually
> claim to succeed -- not 100% certain they really worked, as I don't have
> the IR emitter with me at the moment, its at home hooked up to my hdpvr,
> but I suspect its working fine now.

I'm not sure I follow here.  By "claim" I assume you mean no
i2c_master_send() error.

> > I find it interesting that for the HVR-1600 the delay isn't needed at
> > all.
> > 
> > I'm wondering if there might also be some Linux/USB latency in getting
> > commands shoved over to the HVR-1950's controller (or maybe latency in
> > the HVR-1950's controller too), for which this delay is really
> > accounting.  I suppose in kernel tracing can be used to find the latency
> > on shoving things across the USB and watching for any Ack from the
> > HVR-1950 controller.  An experiment for some other day, I guess.
> 
> Yeah, definitely seems to be specific to usb devices. On the plus side,
> the delay loops should only add insignificant delay overhead for pci
> devices, since if they work on the first call, there won't be any delay
> added.

Cool, nice implementation.

> Oh, I've also got IR RX with ir-kbd-i2c attached to the HVR-1950 working
> much better now, with the polling interval reverted to the standard
> 100ms, but simply introducing the same i2c_master_send() poll that
> lirc_zilog uses into get_key_haup_xvr().

You might want to check what video cards in the source tree request of,
or are defaulted by, ir-kbd-i2c to use get_key_haup_xvr().  If it's only
the chips at address 0x71, you're probably OK.


> I want to test today's changes with the hdpvr tonight (and verify that
> tx is working on the HVR-1950) before I send along my current stack of
> patches, and I have suspicions that most of the hdpvr-specific crud in
> lirc_zilog is bogus now -- it *should* behave exactly like the HVR-1950,
> which obviously doesn't follow any of those hdpvr-specific code paths,
> so I'm hoping we can rip out some additional complexity from lirc_zilog.

Good riddance to old kludges. :)

You could then rename the i2c client strings back to
"ir_[tr]x_z8f0811_haup".  We're going to modify struct IR_i2c_init_data
with all the bridge specific parameters that need to be sent anyway, so
no need to encode that information implicitly in the client's name
anymore.

Regards,
Andy

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jarod Wilson]

On Thu, Jan 20, 2011 at 8:10 PM, Andy Walls <awalls@md.metrocast.net> wrote:
> On Thu, 2011-01-20 at 16:49 -0500, Jarod Wilson wrote:
>> On Jan 20, 2011, at 8:22 AM, Andy Walls wrote:
...
>> Some further testing today with a try-check success-delay-retry loop
>> shows one i2c_master_send() failure plus a udelay(100), and the second
>> i2c_master_send() typically always works (I haven't seen it *not* work
>> on the HVR-1950 in cursory testing).
>
> Cool.
>
>> A second similar loop has proven necessary for IR TX attempts to actually
>> claim to succeed -- not 100% certain they really worked, as I don't have
>> the IR emitter with me at the moment, its at home hooked up to my hdpvr,
>> but I suspect its working fine now.
>
> I'm not sure I follow here.  By "claim" I assume you mean no
> i2c_master_send() error.

Yeah, no errors in the code, and irsend says transmit was successful.
Just tried it out with the emitter hooked up, and it is indeed working
perfectly, sending commands to my cable box.

In short, IR on the HVR-1950 is behaving perfectly right now, including RX
with ir-kbd-i2c and both RX and TX with lirc_zilog.

...
>> Oh, I've also got IR RX with ir-kbd-i2c attached to the HVR-1950 working
>> much better now, with the polling interval reverted to the standard
>> 100ms, but simply introducing the same i2c_master_send() poll that
>> lirc_zilog uses into get_key_haup_xvr().
>
> You might want to check what video cards in the source tree request of,
> or are defaulted by, ir-kbd-i2c to use get_key_haup_xvr().  If it's only
> the chips at address 0x71, you're probably OK.

>From what I can tell, its only chips at 0x71.

>> I want to test today's changes with the hdpvr tonight (and verify that
>> tx is working on the HVR-1950) before I send along my current stack of
>> patches, and I have suspicions that most of the hdpvr-specific crud in
>> lirc_zilog is bogus now -- it *should* behave exactly like the HVR-1950,
>> which obviously doesn't follow any of those hdpvr-specific code paths,
>> so I'm hoping we can rip out some additional complexity from lirc_zilog.
>
> Good riddance to old kludges. :)
>
> You could then rename the i2c client strings back to
> "ir_[tr]x_z8f0811_haup".  We're going to modify struct IR_i2c_init_data
> with all the bridge specific parameters that need to be sent anyway, so
> no need to encode that information implicitly in the client's name
> anymore.

Unfortunately, my suspicions were wrong. The hdpvr-specific tweaks for
both RX and TX are necessary.

On RX, instead of getting two distinct keypresses (each with index 0), you
get two streams of keypresses, 00 to 08. On TX, we get a -EIO from the
i2c_master_recv() after where we normally bail out on the hdpvr.

Ah well. At least it means I'm not making more changes to lirc_zilog
tonight, so I'll send along the patches I've got queued up shortly.

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Mike Isely]

On Wed, 19 Jan 2011, Andy Walls wrote:

   [...]

> 
> So the HVR-1950 only has Z8's capable of both Tx and Rx?  No HVR-1950
> has an Rx only Z8 unit?

As far as I know, that is indeed the case - Tx and Rx always.

It's the older 24xxx devices where there could be a difference, and 
that's why the probe only takes place there.  (And in the receive-only 
24xxx configuration it's not a Z8 but something wierd that is only 
accessible through FX2 commands not via I2C, which is why the bridge 
driver emulates the older I2C chip, making IR reception behave like the 
original 29xxx device.)

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jarod Wilson]

On Jan 19, 2011, at 8:20 AM, Mike Isely wrote:

> This probing behavior does not happen for HVR-1950 (or HVR-1900) since 
> there's only one possible IR configuration there.

Just to be 100% clear, the device I'm poking it is definitely an
HVR-1950, using ir_scheme PVR2_IR_SCHEME_ZILOG, so the probe bits
shouldn't coming into play with anything I'm doing. Only just now
started looking at the pvrusb2 code. Wow, there's a LOT of it. ;)

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Mike Isely]

On Wed, 19 Jan 2011, Jarod Wilson wrote:

> On Jan 19, 2011, at 8:20 AM, Mike Isely wrote:
> 
> > This probing behavior does not happen for HVR-1950 (or HVR-1900) since 
> > there's only one possible IR configuration there.
> 
> Just to be 100% clear, the device I'm poking it is definitely an
> HVR-1950, using ir_scheme PVR2_IR_SCHEME_ZILOG, so the probe bits
> shouldn't coming into play with anything I'm doing. Only just now
> started looking at the pvrusb2 code. Wow, there's a LOT of it. ;)

Yes, and yes :-)

The standalone driver version (which is loaded with ifdef's that allow 
compilation back to 2.6.11) makes the in-kernel driver look small by 
comparison.

There is a fair degree of compartmentalization between the modules.  
The roadmap to what it does for just HVR-1950 you can find by first 
looking at the declarations in pvrusb2-devattr.h and then the 
device-specific configurations in pvrusb2-devattr.c.  From there you can 
usually grep your way around to see how those configuration bits affect 
the rest of the driver.  Most of the really fun stuff is in 
pvrusb2-hdw.c.  Pretty much everything else supports or uses that 
central component.

The actual stuff which deals with I2C is not that large.  Beyond making 
the access possible at all, the driver largely just tries to stay out of 
the way of external logic that needs to reach the bus.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jarod Wilson]

On Jan 19, 2011, at 11:57 AM, Mike Isely wrote:

> On Wed, 19 Jan 2011, Jarod Wilson wrote:
> 
>> On Jan 19, 2011, at 8:20 AM, Mike Isely wrote:
>> 
>>> This probing behavior does not happen for HVR-1950 (or HVR-1900) since 
>>> there's only one possible IR configuration there.
>> 
>> Just to be 100% clear, the device I'm poking it is definitely an
>> HVR-1950, using ir_scheme PVR2_IR_SCHEME_ZILOG, so the probe bits
>> shouldn't coming into play with anything I'm doing. Only just now
>> started looking at the pvrusb2 code. Wow, there's a LOT of it. ;)
> 
> Yes, and yes :-)
> 
> The standalone driver version (which is loaded with ifdef's that allow 
> compilation back to 2.6.11) makes the in-kernel driver look small by 
> comparison.
> 
> There is a fair degree of compartmentalization between the modules.  
> The roadmap to what it does for just HVR-1950 you can find by first 
> looking at the declarations in pvrusb2-devattr.h and then the 
> device-specific configurations in pvrusb2-devattr.c.  From there you can 
> usually grep your way around to see how those configuration bits affect 
> the rest of the driver.  Most of the really fun stuff is in 
> pvrusb2-hdw.c.  Pretty much everything else supports or uses that 
> central component.
> 
> The actual stuff which deals with I2C is not that large.  Beyond making 
> the access possible at all, the driver largely just tries to stay out of 
> the way of external logic that needs to reach the bus.

Cool, thanks much for the pointers, that does help. Based on just
looking at pvrusb2-i2c-core.c's pvr2_i2c_register_ir() versus the
hdpvr's register function, I think I already see how to make the
IR part co-operate with lirc_zilog, and have hacked up a quick patch
to test that theory out...

Basically, rather than calling i2c_new_device() independently for
each address (0x70 and 0x71), call it a single time with an
i2c_board_info struct that looks similar to what's in the hdpvr
driver now. The -EIO I was seeing from lirc_zilog, from what I can
recall, is identical to what was happening with the hdpvr prior to
commit 37634d7308c5c1bdde03ab703a3cac3f0fb12453 (in media_tree.git).

Should be able to test this after lunch.

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jarod Wilson]

On Jan 19, 2011, at 7:21 AM, Andy Walls wrote:

> On Wed, 2011-01-19 at 00:20 -0500, Jarod Wilson wrote:
>> On Jan 16, 2011, at 10:29 PM, Andy Walls wrote:
>> 
>>> On Sun, 2011-01-16 at 14:20 -0500, Andy Walls wrote:
>>>> Mauro,
>>>> 
>>>> Please pull the one ir-kbd-i2c change and multiple lirc_zilog changes
>>>> for 2.6.38.
>>>> 
>>>> The one ir-kbd-i2c change is to put back a case to have ir-kbd-i2c set
>>>> defaults for I2C client address 0x71.  I know I was the one who
>>>> recommend that ir-kbd-i2c not do this, but I discovered pvrusb2 and bttv
>>>> rely on it for the moment - Mea culpa.
>>>> 
>>>> The lirc_zilog changes are tested to work with both Tx and Rx with an
>>>> HVR-1600.  I don't want to continue much further on lirc_zilog changes,
>>>> unitl a few things happen:
>>>> 
>>>> 1. I have developed, and have had tested, a patch for the pvrusb2 driver
>>>> to allow the in kernel lirc_zilog to bind to a Z8 on a pvrusb2 supported
>>>> device.
>>> 
>>> Mauro,
>>> 
>>> I have developed a patch for pvrusb2 and Mike Isely provided his Ack.  I
>>> have added it to my "z8" branch and this pull request.
>> 
>> I've finally got around to trying it out with the HVR-1950 I've got here,
>> and it does do the trick for ir-kbd-i2c (albeit I never see proper key
>> repeats, only alternating press/release key events).
> 
> Yay, a small victory.
> 
> I explicitly set the polling interval to 260 ms in pvrusb2, based on
> your hdpvr findings and lirc_zilog code.  I guess you can try tweaking
> that back to 100 ms.

Ah, okay, hadn't yet looked at the code. That explains why it was acting
just like the hdpvr when its interval is 260 then. :)

I'll confirm whether or not the behavior of the 1950 is identical with
an interval of 100.


>> Not working with
>> lirc_zilog yet, it fails to load, due to an -EIO ret to one of the
>> i2c_master_send() calls in lirc_zilog during probe of the TX side. Haven't
>> looked into it any more than that yet.
> 
> Well technically lirc_zilog doesn't "probe" anymore.  It relies on the
> bridge driver telling it the truth.
> 
> But yes, lirc_zilog is overly sensitive to anything not being right
> during lirc_zilog.c:ir_probe().
> 
> BTW, does your HVR-1950 have a blaster?

Yes, it does, looks like a jack identical to the one on the hdpvr, which
is good, since I don't have the 1950's blaster cable.


> The simple code I added to
> pvrusb2 doesn't try to detect a unit on address 0x70.  It always assumes
> the Z8 is Tx capable.
> 
> With the cx18 and ivtv cards, the Hauppauge EEPROM indicates whether a
> blaster is present.  For those bridge drivers, I can communicate that
> information to the IR modules.
> 
> For the hdpvr and pvrusb2, my short term plan was to always assume a Z8
> could Tx, and make lirc_zilog not barf when it couldn't talk to a Tx
> unit.

Sounds sane to me.


> I notice that Mike had written some short, simple IR unit
> detection code here for other reasons:
> 
> http://git.linuxtv.org/media_tree.git?a=blob;f=drivers/media/video/pvrusb2/pvrusb2-i2c-core.c;h=ccc884948f34b385563ccbf548c5f80b33cd4f08;hb=refs/heads/staging/for_2.6.38-rc1#l661
> http://git.linuxtv.org/media_tree.git?a=blob;f=drivers/media/video/pvrusb2/pvrusb2-i2c-core.c;h=ccc884948f34b385563ccbf548c5f80b33cd4f08;hb=refs/heads/staging/for_2.6.38-rc1#l542
> 
> For debugging, you might want to hack in a probe of address 0x70 for
> your HVR-1950, to ensure the Tx side responds in the bridge driver.

Yeah, will definitely have to take a closer look at the pvrusb2 code.

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Devin Heitmueller]

On Wed, Jan 19, 2011 at 11:08 AM, Jarod Wilson <jarod@wilsonet.com> wrote:
>> BTW, does your HVR-1950 have a blaster?
>
> Yes, it does, looks like a jack identical to the one on the hdpvr, which
> is good, since I don't have the 1950's blaster cable.

Correct - it uses the same cable as the HD-PVR.  The IR receiver on
that device is built into the front of the unit though, unlike the PCI
cards where it's on the cable.

Devin

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

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jean Delvare]

Hi Andy,

On Sun, 16 Jan 2011 14:20:49 -0500, Andy Walls wrote:
> 3. I hear from Jean, or whomever really cares about ir-kbd-i2c, if
> adding some new fields for struct IR_i2c_init_data is acceptable.
> Specifically, I'd like to add a transceiver_lock mutex, a transceiver
> reset callback, and a data pointer for that reset callback.
> (Only lirc_zilog would use the reset callback and data pointer.)

Adding fields to these structures is perfectly fine, if you need to do
that, just go on.

But I'm a little confused about the names you chose,
"ir_transceiver_lock" and "transceiver_lock". These seem too
TX-oriented for a mutex that is supposed to synchronize TX and RX
access. It's particularly surprising for the ir-kbd-i2c driver, which
as far as I know only supports RX. The name "xcvr_lock" you used for
lirc_zilog seems more appropriate.

-- 
Jean Delvare

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Mike Isely]

On Wed, 19 Jan 2011, Jean Delvare wrote:

> Hi Andy,
> 
> On Sun, 16 Jan 2011 14:20:49 -0500, Andy Walls wrote:
> > 3. I hear from Jean, or whomever really cares about ir-kbd-i2c, if
> > adding some new fields for struct IR_i2c_init_data is acceptable.
> > Specifically, I'd like to add a transceiver_lock mutex, a transceiver
> > reset callback, and a data pointer for that reset callback.
> > (Only lirc_zilog would use the reset callback and data pointer.)
> 
> Adding fields to these structures is perfectly fine, if you need to do
> that, just go on.
> 
> But I'm a little confused about the names you chose,
> "ir_transceiver_lock" and "transceiver_lock". These seem too
> TX-oriented for a mutex that is supposed to synchronize TX and RX
> access. It's particularly surprising for the ir-kbd-i2c driver, which
> as far as I know only supports RX. The name "xcvr_lock" you used for
> lirc_zilog seems more appropriate.

Actually the term "transceiver" is normally understood to mean both 
directions.  Otherwise it would be "receiver" or "transmitter".  
Another screwy as aspect of english, and I say this as a native english 
speaker.  The term "xcvr" is usually just considered to be shorthand for 
"transceiver".

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Jean Delvare]

On Wed, 19 Jan 2011 09:09:47 -0600 (CST), Mike Isely wrote:
> On Wed, 19 Jan 2011, Jean Delvare wrote:
> 
> > Hi Andy,
> > 
> > On Sun, 16 Jan 2011 14:20:49 -0500, Andy Walls wrote:
> > > 3. I hear from Jean, or whomever really cares about ir-kbd-i2c, if
> > > adding some new fields for struct IR_i2c_init_data is acceptable.
> > > Specifically, I'd like to add a transceiver_lock mutex, a transceiver
> > > reset callback, and a data pointer for that reset callback.
> > > (Only lirc_zilog would use the reset callback and data pointer.)
> > 
> > Adding fields to these structures is perfectly fine, if you need to do
> > that, just go on.
> > 
> > But I'm a little confused about the names you chose,
> > "ir_transceiver_lock" and "transceiver_lock". These seem too
> > TX-oriented for a mutex that is supposed to synchronize TX and RX
> > access. It's particularly surprising for the ir-kbd-i2c driver, which
> > as far as I know only supports RX. The name "xcvr_lock" you used for
> > lirc_zilog seems more appropriate.
> 
> Actually the term "transceiver" is normally understood to mean both 
> directions.  Otherwise it would be "receiver" or "transmitter".  
> Another screwy as aspect of english, and I say this as a native english 
> speaker.  The term "xcvr" is usually just considered to be shorthand for 
> "transceiver".

Oh. I stand corrected, thanks.

-- 
Jean Delvare

Re: [GIT PATCHES for 2.6.38] Zilog Z8 IR unit fixes

[Andy Walls]

Jean,

As Mike noted, "transceiver" is a contraction of "transmitter-receiver".  But I wouldn't blame English speakers in general for that, just English speaking engineers. ;)

English speaking engineers (likely) also orignated use of the "x" as shorthand for "trans-" and "crys-".

"Transceiver_lock" was intended to mean a lock protecting access to both portions of a chip: tx and rx.

I'm still mulling it all over though.  Some change will be made to IR_i2c_init_data. I found I need some more time to design exactly what I need.

Regards,
Andy

Mike Isely <isely@isely.net> wrote:

>On Wed, 19 Jan 2011, Jean Delvare wrote:
>
>> Hi Andy,
>> 
>> On Sun, 16 Jan 2011 14:20:49 -0500, Andy Walls wrote:
>> > 3. I hear from Jean, or whomever really cares about ir-kbd-i2c, if
>> > adding some new fields for struct IR_i2c_init_data is acceptable.
>> > Specifically, I'd like to add a transceiver_lock mutex, a transceiver
>> > reset callback, and a data pointer for that reset callback.
>> > (Only lirc_zilog would use the reset callback and data pointer.)
>> 
>> Adding fields to these structures is perfectly fine, if you need to do
>> that, just go on.
>> 
>> But I'm a little confused about the names you chose,
>> "ir_transceiver_lock" and "transceiver_lock". These seem too
>> TX-oriented for a mutex that is supposed to synchronize TX and RX
>> access. It's particularly surprising for the ir-kbd-i2c driver, which
>> as far as I know only supports RX. The name "xcvr_lock" you used for
>> lirc_zilog seems more appropriate.
>
>Actually the term "transceiver" is normally understood to mean both 
>directions.  Otherwise it would be "receiver" or "transmitter".  
>Another screwy as aspect of english, and I say this as a native english 
>speaker.  The term "xcvr" is usually just considered to be shorthand for 
>"transceiver".
>
>  -Mike
>
>
>-- 
>
>Mike Isely
>isely @ isely (dot) net
>PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
>--
>To unsubscribe from this list: send the line "unsubscribe linux-media" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html