mbox series

[0/6] media: ite-cir driver cleanups

Message ID cover.1613989288.git.sean@mess.org
Headers show
Series media: ite-cir driver cleanups | expand

Message

Sean Young Feb. 22, 2021, 10:43 a.m. UTC
This should not be any functional changes, just various cleanups.

Sean Young (6):
  media: ite-cir: remove unused fields
  media: ite-cir: set parent device
  media: ite-cir: use standard logging and reduce noise
  media: ite-cir: carrier and duty cycle can be set via ioctl
  media: ite-cir: move runtime information into driver data
  media: ite-cir: check for receive overflow

 drivers/media/rc/ite-cir.c | 341 +++++++++----------------------------
 drivers/media/rc/ite-cir.h |  49 ++----
 2 files changed, 91 insertions(+), 299 deletions(-)

Comments

Matthias Reichl Feb. 23, 2021, 4:13 p.m. UTC | #1
Hi Sean,

On Mon, Feb 22, 2021 at 10:43:23AM +0000, Sean Young wrote:
> This should not be any functional changes, just various cleanups.

> 

> Sean Young (6):

>   media: ite-cir: remove unused fields

>   media: ite-cir: set parent device

>   media: ite-cir: use standard logging and reduce noise

>   media: ite-cir: carrier and duty cycle can be set via ioctl

>   media: ite-cir: move runtime information into driver data

>   media: ite-cir: check for receive overflow

> 

>  drivers/media/rc/ite-cir.c | 341 +++++++++----------------------------

>  drivers/media/rc/ite-cir.h |  49 ++----

>  2 files changed, 91 insertions(+), 299 deletions(-)


I did a few tests with this series on top of 5.10.17 and so far
everything looks fine with ITE8713 CIR.

I could only test receiving (don't have a transmitter on that PC)
and also couldn't test the carrier options (I'm using a demodulating
TSOP receiver here), but your changes look sane to me.

so long,

Hias
Sean Young Feb. 23, 2021, 5:50 p.m. UTC | #2
Hello Hias,

On Tue, Feb 23, 2021 at 05:13:07PM +0100, Matthias Reichl wrote:
> Hi Sean,

> 

> On Mon, Feb 22, 2021 at 10:43:23AM +0000, Sean Young wrote:

> > This should not be any functional changes, just various cleanups.

> > 

> > Sean Young (6):

> >   media: ite-cir: remove unused fields

> >   media: ite-cir: set parent device

> >   media: ite-cir: use standard logging and reduce noise

> >   media: ite-cir: carrier and duty cycle can be set via ioctl

> >   media: ite-cir: move runtime information into driver data

> >   media: ite-cir: check for receive overflow

> > 

> >  drivers/media/rc/ite-cir.c | 341 +++++++++----------------------------

> >  drivers/media/rc/ite-cir.h |  49 ++----

> >  2 files changed, 91 insertions(+), 299 deletions(-)

> 

> I did a few tests with this series on top of 5.10.17 and so far

> everything looks fine with ITE8713 CIR.


Great, thank you very much!

> I could only test receiving (don't have a transmitter on that PC)

> and also couldn't test the carrier options (I'm using a demodulating

> TSOP receiver here), but your changes look sane to me.


I only have got an eeepc box, with the same limitations. Do you happen to
know what hardware there is for this driver? It would be nice to test this
properly.

This is the only IR driver that can change the RX carrier range, so it's
kinda interesting.

Thanks!

Sean
Matthias Reichl Feb. 23, 2021, 7:16 p.m. UTC | #3
Hi Sean,

On Tue, Feb 23, 2021 at 05:50:01PM +0000, Sean Young wrote:
> On Tue, Feb 23, 2021 at 05:13:07PM +0100, Matthias Reichl wrote:

> > I could only test receiving (don't have a transmitter on that PC)

> > and also couldn't test the carrier options (I'm using a demodulating

> > TSOP receiver here), but your changes look sane to me.

> 

> I only have got an eeepc box, with the same limitations. Do you happen to

> know what hardware there is for this driver? It would be nice to test this

> properly.

> 

> This is the only IR driver that can change the RX carrier range, so it's

> kinda interesting.


In the most recent bug report I got (about a month ago) the user was
using an intel NUC (NUC10i3FNH/NUC10i3FNB according to DMI):
https://forum.libreelec.tv/thread/23211-intel-nightly-build-problem-with-mce-remote/?postID=148823#post148823

I've also seen NUCs with Nuvoton CIR though and I suspect they'll
all probably have a demodulating receiver soldered directly to the
board, so probably not easy to tinker with (haven't checked though).

I have only an ancient Pentium 4 mainboard (Gigabyte 8IPE775) with
an IT8712 here. IR RX/TX and CIR RX/TX are all exposed on a header,
but the BIOS is somewhat odd and doesn't contain CIR on/off settings
or announce the CIR via ACPI. So I had to manually patch ACPI tables
to get that beast working.... ITE8713 pnpid was close enough (couldn't
find a datasheet for 8713, only 8712 - that does mention demodulating
though).

I don't think I have a simple IR diode or transistor here to easily
test that, only demodulating TSOPs (and some clones) - hooking that
up to the mainboard wouldn't be too hard.

so long,

Hias
Sean Young Feb. 24, 2021, 9:57 a.m. UTC | #4
Hi Hias,

On Tue, Feb 23, 2021 at 08:16:15PM +0100, Matthias Reichl wrote:
> Hi Sean,

> 

> On Tue, Feb 23, 2021 at 05:50:01PM +0000, Sean Young wrote:

> > On Tue, Feb 23, 2021 at 05:13:07PM +0100, Matthias Reichl wrote:

> > > I could only test receiving (don't have a transmitter on that PC)

> > > and also couldn't test the carrier options (I'm using a demodulating

> > > TSOP receiver here), but your changes look sane to me.

> > 

> > I only have got an eeepc box, with the same limitations. Do you happen to

> > know what hardware there is for this driver? It would be nice to test this

> > properly.

> > 

> > This is the only IR driver that can change the RX carrier range, so it's

> > kinda interesting.

> 

> In the most recent bug report I got (about a month ago) the user was

> using an intel NUC (NUC10i3FNH/NUC10i3FNB according to DMI):

> https://forum.libreelec.tv/thread/23211-intel-nightly-build-problem-with-mce-remote/?postID=148823#post148823

> 

> I've also seen NUCs with Nuvoton CIR though and I suspect they'll

> all probably have a demodulating receiver soldered directly to the

> board, so probably not easy to tinker with (haven't checked though).

> 

> I have only an ancient Pentium 4 mainboard (Gigabyte 8IPE775) with

> an IT8712 here. IR RX/TX and CIR RX/TX are all exposed on a header,

> but the BIOS is somewhat odd and doesn't contain CIR on/off settings

> or announce the CIR via ACPI. So I had to manually patch ACPI tables

> to get that beast working.... ITE8713 pnpid was close enough (couldn't

> find a datasheet for 8713, only 8712 - that does mention demodulating

> though).

> 

> I don't think I have a simple IR diode or transistor here to easily

> test that, only demodulating TSOPs (and some clones) - hooking that

> up to the mainboard wouldn't be too hard.


That's a good point, that's probably the way to go. I'll have a look at
modifying my eeepc box.

Having said that, if all the hardware out there uses TSOP-type IR
receivers, I'm not sure how much use there is having this hooked up in
the driver; same for transmit.


Sean
Matthias Reichl Feb. 26, 2021, 1:57 p.m. UTC | #5
Hi Sean,

On Wed, Feb 24, 2021 at 09:57:47AM +0000, Sean Young wrote:
> Hi Hias,

> 

> On Tue, Feb 23, 2021 at 08:16:15PM +0100, Matthias Reichl wrote:

> > Hi Sean,

> > 

> > On Tue, Feb 23, 2021 at 05:50:01PM +0000, Sean Young wrote:

> > > On Tue, Feb 23, 2021 at 05:13:07PM +0100, Matthias Reichl wrote:

> > > > I could only test receiving (don't have a transmitter on that PC)

> > > > and also couldn't test the carrier options (I'm using a demodulating

> > > > TSOP receiver here), but your changes look sane to me.

> > > 

> > > I only have got an eeepc box, with the same limitations. Do you happen to

> > > know what hardware there is for this driver? It would be nice to test this

> > > properly.

> > > 

> > > This is the only IR driver that can change the RX carrier range, so it's

> > > kinda interesting.

> > 

> > In the most recent bug report I got (about a month ago) the user was

> > using an intel NUC (NUC10i3FNH/NUC10i3FNB according to DMI):

> > https://forum.libreelec.tv/thread/23211-intel-nightly-build-problem-with-mce-remote/?postID=148823#post148823

> > 

> > I've also seen NUCs with Nuvoton CIR though and I suspect they'll

> > all probably have a demodulating receiver soldered directly to the

> > board, so probably not easy to tinker with (haven't checked though).

> > 

> > I have only an ancient Pentium 4 mainboard (Gigabyte 8IPE775) with

> > an IT8712 here. IR RX/TX and CIR RX/TX are all exposed on a header,

> > but the BIOS is somewhat odd and doesn't contain CIR on/off settings

> > or announce the CIR via ACPI. So I had to manually patch ACPI tables

> > to get that beast working.... ITE8713 pnpid was close enough (couldn't

> > find a datasheet for 8713, only 8712 - that does mention demodulating

> > though).

> > 

> > I don't think I have a simple IR diode or transistor here to easily

> > test that, only demodulating TSOPs (and some clones) - hooking that

> > up to the mainboard wouldn't be too hard.

> 

> That's a good point, that's probably the way to go. I'll have a look at

> modifying my eeepc box.


On a second thought it's probably a bit more involved than just
connecting an IR photo diode/transistor. CIRRX is a digital input
and to get acceptable performance we'd also need an AGC circuit
and a comparator / schmitt-trigger.

It might be easier to just feed a modulated digital signal into CIRRX,
either from CIRTX or from a RPi running pwm-ir-tx.

> Having said that, if all the hardware out there uses TSOP-type IR

> receivers, I'm not sure how much use there is having this hooked up in

> the driver; same for transmit.


If it's not too much hassle I'd vote for keeping the RX demodulation
feature in the driver - it's an interesting feature and might come in
handy at times.

For TX we need to keep carrier function as CIRTX always transmits
modulated singals according to the IT8712 datasheet (unmodulated TX
output would be quite odd, haven't seen that yet).

so long,

Hias
Sean Young March 1, 2021, 10:42 a.m. UTC | #6
Hi Hias,

On Fri, Feb 26, 2021 at 02:57:13PM +0100, Matthias Reichl wrote:
> On Wed, Feb 24, 2021 at 09:57:47AM +0000, Sean Young wrote:

> > That's a good point, that's probably the way to go. I'll have a look at

> > modifying my eeepc box.

> 

> On a second thought it's probably a bit more involved than just

> connecting an IR photo diode/transistor. CIRRX is a digital input

> and to get acceptable performance we'd also need an AGC circuit

> and a comparator / schmitt-trigger.


Yes, I had been thinking about building something like this.

> It might be easier to just feed a modulated digital signal into CIRRX,

> either from CIRTX or from a RPi running pwm-ir-tx.


That's a great idea, I hadn't thought of that. That's a lot easier!

> > Having said that, if all the hardware out there uses TSOP-type IR

> > receivers, I'm not sure how much use there is having this hooked up in

> > the driver; same for transmit.

> 

> If it's not too much hassle I'd vote for keeping the RX demodulation

> feature in the driver - it's an interesting feature and might come in

> handy at times.


It's kind of interesting, I agree. 

> For TX we need to keep carrier function as CIRTX always transmits

> modulated singals according to the IT8712 datasheet (unmodulated TX

> output would be quite odd, haven't seen that yet).


There are IR signals which are unmodulated. In IRP this would start
with {0k, and there are several of such protocols: Jerrold, Revox for
example:

http://www.hifi-remote.com/wiki/index.php/Revox

I never seen such hardware and I have no idea how well this works in
practise.


Sean