Zilog Z8 IR unit fixes

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.

Quoted 14 lines · 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:

Quoted 3 lines

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.

Quoted 2 lines

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

Quoted 7 lines

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.

Quoted 19 lines · Andy Walls

Quoted 17 lines · 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.

Quoted 4 lines

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.

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.

Quoted 2 lines · Andy Walls

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.

Quoted 7 lines · Jean Delvare

Quoted 2 lines · Andy Walls

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

Quoted 4 lines · Jarod Wilson

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.

Quoted 10 lines · Andy Walls

Quoted 4 lines · Jarod Wilson

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.

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.

Hi Andy,

Quoted 14 lines · Andy Walls

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.

Quoted 4 lines

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.)

Quoted 17 lines · Jean Delvare

Hi Andy,

Quoted 14 lines · Andy Walls

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.

Quoted 8 lines

Quoted 4 lines

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.

Quoted 17 lines · Jean Delvare

Hi Andy,

Quoted 14 lines · Andy Walls

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.

Quoted 8 lines

Quoted 4 lines

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.

Quoted 28 lines · Jarod Wilson

Quoted 17 lines · Jean Delvare

Hi Andy,

Quoted 14 lines · Andy Walls

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.

Quoted 8 lines

Quoted 4 lines

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.)

Quoted 6 lines

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.

Quoted 17 lines · Jean Delvare

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.

Quoted 3 lines

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.

Quoted 5 lines · Jarod Wilson

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.

Quoted 8 lines · Jarod Wilson

Quoted 5 lines · Jarod Wilson

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.

Quoted 7 lines

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.

Quoted 27 lines · Andy Walls

Quoted 8 lines · Jarod Wilson

Quoted 5 lines · Jarod Wilson

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.

Quoted 7 lines

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

Quoted 2 lines

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.

Quoted 9 lines

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.

Quoted 1 line · Jarod Wilson

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

Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 6 lines

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.

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

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

Quoted 2 lines · Mike Isely

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. ;)

Quoted 9 lines · Jarod Wilson

Quoted 2 lines · Mike Isely

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.

Quoted 21 lines · Jarod Wilson

Quoted 19 lines · Andy Walls

Quoted 17 lines · 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.

Quoted 8 lines

Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 1 line

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.

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.)

Hi Andy,

Quoted 5 lines · Andy Walls

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.

Quoted 12 lines · Jean Delvare

Hi Andy,

Quoted 5 lines · Andy Walls

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.

Quoted 6 lines

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".