Message ID | 1388758723-21653-1-git-send-email-a.seppala@gmail.com (mailing list archive) |
---|---|
State | Changes Requested, archived |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from <linux-media-owner@vger.kernel.org>) id 1Vz5eP-0006m3-A9; Fri, 03 Jan 2014 15:23:01 +0100 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.72/mailfrontend-7) with esmtp id 1Vz5eM-0004oo-0a; Fri, 03 Jan 2014 15:23:00 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751675AbaACOWo (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Fri, 3 Jan 2014 09:22:44 -0500 Received: from mail-la0-f49.google.com ([209.85.215.49]:47882 "EHLO mail-la0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751516AbaACOWn (ORCPT <rfc822; linux-media@vger.kernel.org>); Fri, 3 Jan 2014 09:22:43 -0500 Received: by mail-la0-f49.google.com with SMTP id er20so7971159lab.22 for <linux-media@vger.kernel.org>; Fri, 03 Jan 2014 06:22:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:mime-version:content-type :content-transfer-encoding; bh=K8lhsww2CWpsdK22E3SDrRaAJdE3LJuCyu7Q5W7pyuA=; b=pkZEi+HuXZvwefWhhN3mM69GgFp83c2I8BtoN2UJJAbxsChzGbPZELlzUawqiGa7Fd Ug2pv4yFRNcaETBiavNVbodxzvXckzNZtlv/IDWXSa5StauzbgojH7gtj1AKB4L45ORe O7/u9/rc++51Kq5ol0gN+xcHqH5wxabaZGg9DBtebbw6boKIf0wkc475Cz0NfhZGd7z2 SE5lOEGZ+2uLp17Hfgr4HScfiiB2KhMKgzGzSqCO2tmwAdDkeyAsJiLLgFymfBRaI8Wo ibt52OcV4PevhLSz4p45lwEyeT83LoX7WOJ1dz7HauxCpRNQ4652qQvVvq5GngPiA/WO sxVg== X-Received: by 10.152.3.70 with SMTP id a6mr290816laa.63.1388758960214; Fri, 03 Jan 2014 06:22:40 -0800 (PST) Received: from pixie.elisa-laajakaista.fi (a88-114-92-24.elisa-laajakaista.fi. [88.114.92.24]) by mx.google.com with ESMTPSA id qx1sm36591883lbb.15.2014.01.03.06.22.38 for <multiple recipients> (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 03 Jan 2014 06:22:39 -0800 (PST) From: =?UTF-8?q?Antti=20Sepp=C3=A4l=C3=A4?= <a.seppala@gmail.com> To: linux-media@vger.kernel.org Cc: Jarod Wilson <jarod@redhat.com>, =?UTF-8?q?Antti=20Sepp=C3=A4l=C3=A4?= <a.seppala@gmail.com> Subject: [PATCH] nuvoton-cir: Add support for user configurable wake-up codes Date: Fri, 3 Jan 2014 16:18:43 +0200 Message-Id: <1388758723-21653-1-git-send-email-a.seppala@gmail.com> X-Mailer: git-send-email 1.8.3.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2014.1.3.141215 X-PMX-Spam: Gauge=IIIIIIIII, Probability=9%, Report=' FORGED_FROM_GMAIL 0.1, MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODY_SIZE_5000_5999 0, BODY_SIZE_7000_LESS 0, CT_TEXT_PLAIN_UTF8_CAPS 0, DKIM_SIGNATURE 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __CT 0, __CTE 0, __CT_TEXT_PLAIN 0, __FRAUD_BODY_WEBMAIL 0, __FRAUD_WEBMAIL 0, __FRAUD_WEBMAIL_FROM 0, __FROM_GMAIL 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __HIGHBITS 0, __LINES_OF_YELLING 0, __MIME_TEXT_ONLY 0, __MIME_VERSION 0, __MULTIPLE_RCPTS_CC_X2 0, __PHISH_SPEAR_STRUCTURE_1 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS , __YOUTUBE_RCVD 0' |
Commit Message
Antti Seppälä
Jan. 3, 2014, 2:18 p.m. UTC
This patch introduces module parameters for setting wake-up codes to be
programmed into the hardware FIFO. This allows users to provide custom
IR sample sequences to trigger system wake-up from sleep states.
Usage:
modprobe nuvoton-cir wake_samples=0x90,0x11,0xa1... (up to 67 bytes)
Here is a summary of module parameters introduced by this patch:
* wake_samples: FIFO values to compare against received IR codes when
in sleep state.
* cmp_deep: Number of bytes the hardware will compare against. This is
currently autodetected by the driver.
* cmp_tolerance: The maximum allowed value difference between a single
wake-up byte and a sample read from IR to be still considered a match.
Default is 5.
Signed-off-by: Antti Seppälä <a.seppala@gmail.com>
---
drivers/media/rc/nuvoton-cir.c | 65 +++++++++++++++++++++++++++++++-----------
drivers/media/rc/nuvoton-cir.h | 6 ++--
2 files changed, 51 insertions(+), 20 deletions(-)
Comments
Em Fri, 3 Jan 2014 16:18:43 +0200 Antti Seppälä <a.seppala@gmail.com> escreveu: > This patch introduces module parameters for setting wake-up codes to be > programmed into the hardware FIFO. This allows users to provide custom > IR sample sequences to trigger system wake-up from sleep states. > > Usage: > modprobe nuvoton-cir wake_samples=0x90,0x11,0xa1... (up to 67 bytes) > > Here is a summary of module parameters introduced by this patch: > * wake_samples: FIFO values to compare against received IR codes when > in sleep state. > > * cmp_deep: Number of bytes the hardware will compare against. This is > currently autodetected by the driver. > > * cmp_tolerance: The maximum allowed value difference between a single > wake-up byte and a sample read from IR to be still considered a match. > Default is 5. Instead of adding it as a modprobe parameter, the better is to create an special sysfs node for devices that support wake up. Btw, there's another device driver also requiring a similar feature. Regards, Mauro > > Signed-off-by: Antti Seppälä <a.seppala@gmail.com> > --- > drivers/media/rc/nuvoton-cir.c | 65 +++++++++++++++++++++++++++++++----------- > drivers/media/rc/nuvoton-cir.h | 6 ++-- > 2 files changed, 51 insertions(+), 20 deletions(-) > > diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c > index 21ee0dc..b11ee43 100644 > --- a/drivers/media/rc/nuvoton-cir.c > +++ b/drivers/media/rc/nuvoton-cir.c > @@ -39,6 +39,15 @@ > > #include "nuvoton-cir.h" > > +/* debugging module parameter */ > +static int debug; > + > +/* Wake up configuration parameters */ > +static unsigned char wake_samples[WAKE_FIFO_LEN]; > +static unsigned int num_wake_samples; > +static unsigned char cmp_deep; > +static unsigned char cmp_tolerance = CIR_WAKE_CMP_TOLERANCE; > + > /* write val to config reg */ > static inline void nvt_cr_write(struct nvt_dev *nvt, u8 val, u8 reg) > { > @@ -418,13 +427,38 @@ static void nvt_cir_regs_init(struct nvt_dev *nvt) > > static void nvt_cir_wake_regs_init(struct nvt_dev *nvt) > { > + int i; > + u8 cmp_reg = CIR_WAKE_FIFO_CMP_BYTES; > + u8 ircon_reg = CIR_WAKE_IRCON_RXEN | CIR_WAKE_IRCON_R | > + CIR_WAKE_IRCON_RXINV | CIR_WAKE_IRCON_SAMPLE_PERIOD_SEL; > + /* > + * Enable TX and RX, specific carrier on = low, off = high, and set > + * sample period (currently 50us) > + */ > + nvt_cir_wake_reg_write(nvt, ircon_reg | CIR_WAKE_IRCON_MODE1, > + CIR_WAKE_IRCON); > + > + /* clear cir wake rx fifo */ > + nvt_clear_cir_wake_fifo(nvt); > + > + /* Write samples from module parameter to fifo */ > + for (i = 0; i < num_wake_samples; i++) > + nvt_cir_wake_reg_write(nvt, wake_samples[i], > + CIR_WAKE_WR_FIFO_DATA); > + > + /* Switch cir to wakeup mode and disable fifo writing */ > + nvt_cir_wake_reg_write(nvt, ircon_reg | CIR_WAKE_IRCON_MODE0, > + CIR_WAKE_IRCON); > + > /* set number of bytes needed for wake from s3 (default 65) */ > - nvt_cir_wake_reg_write(nvt, CIR_WAKE_FIFO_CMP_BYTES, > - CIR_WAKE_FIFO_CMP_DEEP); > + if (cmp_deep) > + cmp_reg = cmp_deep; > + else if (num_wake_samples) > + cmp_reg = num_wake_samples; > + nvt_cir_wake_reg_write(nvt, cmp_reg, CIR_WAKE_FIFO_CMP_DEEP); > > /* set tolerance/variance allowed per byte during wake compare */ > - nvt_cir_wake_reg_write(nvt, CIR_WAKE_CMP_TOLERANCE, > - CIR_WAKE_FIFO_CMP_TOL); > + nvt_cir_wake_reg_write(nvt, cmp_tolerance, CIR_WAKE_FIFO_CMP_TOL); > > /* set sample limit count (PE interrupt raised when reached) */ > nvt_cir_wake_reg_write(nvt, CIR_RX_LIMIT_COUNT >> 8, CIR_WAKE_SLCH); > @@ -434,18 +468,6 @@ static void nvt_cir_wake_regs_init(struct nvt_dev *nvt) > nvt_cir_wake_reg_write(nvt, CIR_WAKE_FIFOCON_RX_TRIGGER_LEV, > CIR_WAKE_FIFOCON); > > - /* > - * Enable TX and RX, specific carrier on = low, off = high, and set > - * sample period (currently 50us) > - */ > - nvt_cir_wake_reg_write(nvt, CIR_WAKE_IRCON_MODE0 | CIR_WAKE_IRCON_RXEN | > - CIR_WAKE_IRCON_R | CIR_WAKE_IRCON_RXINV | > - CIR_WAKE_IRCON_SAMPLE_PERIOD_SEL, > - CIR_WAKE_IRCON); > - > - /* clear cir wake rx fifo */ > - nvt_clear_cir_wake_fifo(nvt); > - > /* clear any and all stray interrupts */ > nvt_cir_wake_reg_write(nvt, 0xff, CIR_WAKE_IRSTS); > } > @@ -1232,6 +1254,17 @@ static void nvt_exit(void) > module_param(debug, int, S_IRUGO | S_IWUSR); > MODULE_PARM_DESC(debug, "Enable debugging output"); > > +module_param_array(wake_samples, byte, &num_wake_samples, S_IRUGO | S_IWUSR); > +MODULE_PARM_DESC(wake_samples, "FIFO sample bytes triggering wake"); > + > +module_param(cmp_deep, byte, S_IRUGO | S_IWUSR); > +MODULE_PARM_DESC(cmp_deep, "How many bytes need to compare\n" > + "\t\t(0 = auto (default))"); > + > +module_param(cmp_tolerance, byte, S_IRUGO | S_IWUSR); > +MODULE_PARM_DESC(cmp_tolerance, "Data tolerance to each wake sample byte\n" > + "\t\t(default = 5)"); > + > MODULE_DEVICE_TABLE(pnp, nvt_ids); > MODULE_DESCRIPTION("Nuvoton W83667HG-A & W83677HG-I CIR driver"); > > diff --git a/drivers/media/rc/nuvoton-cir.h b/drivers/media/rc/nuvoton-cir.h > index 07e8310..8209f84 100644 > --- a/drivers/media/rc/nuvoton-cir.h > +++ b/drivers/media/rc/nuvoton-cir.h > @@ -31,10 +31,6 @@ > /* platform driver name to register */ > #define NVT_DRIVER_NAME "nuvoton-cir" > > -/* debugging module parameter */ > -static int debug; > - > - > #define nvt_pr(level, text, ...) \ > printk(level KBUILD_MODNAME ": " text, ## __VA_ARGS__) > > @@ -64,6 +60,8 @@ static int debug; > #define TX_BUF_LEN 256 > #define RX_BUF_LEN 32 > > +#define WAKE_FIFO_LEN 67 > + > struct nvt_dev { > struct pnp_dev *pdev; > struct rc_dev *rdev;
This patch series introduces a simple sysfs file interface for reading and writing wakeup scancodes to rc drivers. This is an improved version of my previous patch for nuvoton-cir which did the same thing via module parameters. This is a more generic approach allowing other drivers to utilize the interface as well. I did not port winbond-cir to this method of wakeup scancode setting yet because I don't have the hardware to test it and I wanted first to get some comments about how the patch series looks. I did however write a simple support to read and write scancodes to rc-loopback module. Antti Seppälä (4): rc-core: Add defintions needed for sysfs callback rc-core: Add support for reading/writing wakeup scancodes via sysfs rc-loopback: Add support for reading/writing wakeup scancodes via sysfs nuvoton-cir: Add support for reading/writing wakeup scancodes via sysfs drivers/media/rc/nuvoton-cir.c | 81 ++++++++++++++++++++++++++ drivers/media/rc/nuvoton-cir.h | 2 + drivers/media/rc/rc-loopback.c | 31 ++++++++++ drivers/media/rc/rc-main.c | 129 +++++++++++++++++++++++++++++++++++++++++ include/media/rc-core.h | 13 +++++ 5 files changed, 256 insertions(+)
On Mon, Jan 20, 2014 at 09:39:43PM +0200, Antti Seppälä wrote: > This patch series introduces a simple sysfs file interface for reading > and writing wakeup scancodes to rc drivers. > > This is an improved version of my previous patch for nuvoton-cir which > did the same thing via module parameters. This is a more generic > approach allowing other drivers to utilize the interface as well. > > I did not port winbond-cir to this method of wakeup scancode setting yet > because I don't have the hardware to test it and I wanted first to get > some comments about how the patch series looks. I did however write a > simple support to read and write scancodes to rc-loopback module. Doesn't the nuvoton-cir driver need to know the IR protocol for wakeup? This is needed for winbond-cir; I guess this should be another sysfs file, something like "wakeup_protocol". Even if the nuvoton can only handle one IR protocol, maybe it should be exported (readonly) via sysfs? I'm happy to help with a winbond-cir implementation; I have the hardware. Sean -- 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
On 21 January 2014 14:28, Sean Young <sean@mess.org> wrote: > On Mon, Jan 20, 2014 at 09:39:43PM +0200, Antti Seppälä wrote: >> This patch series introduces a simple sysfs file interface for reading >> and writing wakeup scancodes to rc drivers. >> >> This is an improved version of my previous patch for nuvoton-cir which >> did the same thing via module parameters. This is a more generic >> approach allowing other drivers to utilize the interface as well. >> >> I did not port winbond-cir to this method of wakeup scancode setting yet >> because I don't have the hardware to test it and I wanted first to get >> some comments about how the patch series looks. I did however write a >> simple support to read and write scancodes to rc-loopback module. > > Doesn't the nuvoton-cir driver need to know the IR protocol for wakeup? > > This is needed for winbond-cir; I guess this should be another sysfs > file, something like "wakeup_protocol". Even if the nuvoton can only > handle one IR protocol, maybe it should be exported (readonly) via > sysfs? > > I'm happy to help with a winbond-cir implementation; I have the hardware. > > > Sean Nuvoton-cir doesn't care about the IR protocol because the hardware compares raw IR pulse lengths and wakes the system if received pulse is within certain tolerance of the one pre-programmed to the HW. This approach is agnostic to the used IR protocol. I glanced over the winbond-cir driver and porting the driver to use sysfs for wakeup scancodes looks doable. Also a new sysfs entry for setting the wakeup protocol would indeed be needed... I will take a closer look at this when I have some more time. -Antti -- 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
On Wed, Jan 22, 2014 at 05:46:28PM +0200, Antti Seppälä wrote: > On 21 January 2014 14:28, Sean Young <sean@mess.org> wrote: > > On Mon, Jan 20, 2014 at 09:39:43PM +0200, Antti Seppälä wrote: > >> This patch series introduces a simple sysfs file interface for reading > >> and writing wakeup scancodes to rc drivers. > >> > >> This is an improved version of my previous patch for nuvoton-cir which > >> did the same thing via module parameters. This is a more generic > >> approach allowing other drivers to utilize the interface as well. > >> > >> I did not port winbond-cir to this method of wakeup scancode setting yet > >> because I don't have the hardware to test it and I wanted first to get > >> some comments about how the patch series looks. I did however write a > >> simple support to read and write scancodes to rc-loopback module. > > > > Doesn't the nuvoton-cir driver need to know the IR protocol for wakeup? > > > > This is needed for winbond-cir; I guess this should be another sysfs > > file, something like "wakeup_protocol". Even if the nuvoton can only > > handle one IR protocol, maybe it should be exported (readonly) via > > sysfs? > > > > I'm happy to help with a winbond-cir implementation; I have the hardware. > > > > > > Sean > > Nuvoton-cir doesn't care about the IR protocol because the hardware > compares raw IR pulse lengths and wakes the system if received pulse > is within certain tolerance of the one pre-programmed to the HW. This > approach is agnostic to the used IR protocol. Your patch talks about scancodes which is something entirely different. This should be renamed to something better. So with the nuvoton you program a set of pulses and spaces; with the winbond you set the protocol and the scancode. I don't think there is any shared code here. Maybe it's better to implement the wakeup sysfs files in the drivers themselves rather than in rcdev, I guess that depends on whether there are other devices that implement similar functionality. Sean > I glanced over the winbond-cir driver and porting the driver to use > sysfs for wakeup scancodes looks doable. Also a new sysfs entry for > setting the wakeup protocol would indeed be needed... I will take a > closer look at this when I have some more time. > > -Antti > -- > 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 -- 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
On 22 January 2014 18:29, Sean Young <sean@mess.org> wrote: > On Wed, Jan 22, 2014 at 05:46:28PM +0200, Antti Seppälä wrote: >> On 21 January 2014 14:28, Sean Young <sean@mess.org> wrote: >> > On Mon, Jan 20, 2014 at 09:39:43PM +0200, Antti Seppälä wrote: >> >> This patch series introduces a simple sysfs file interface for reading >> >> and writing wakeup scancodes to rc drivers. >> >> >> >> This is an improved version of my previous patch for nuvoton-cir which >> >> did the same thing via module parameters. This is a more generic >> >> approach allowing other drivers to utilize the interface as well. >> >> >> >> I did not port winbond-cir to this method of wakeup scancode setting yet >> >> because I don't have the hardware to test it and I wanted first to get >> >> some comments about how the patch series looks. I did however write a >> >> simple support to read and write scancodes to rc-loopback module. >> > >> > Doesn't the nuvoton-cir driver need to know the IR protocol for wakeup? >> > >> > This is needed for winbond-cir; I guess this should be another sysfs >> > file, something like "wakeup_protocol". Even if the nuvoton can only >> > handle one IR protocol, maybe it should be exported (readonly) via >> > sysfs? >> > >> > I'm happy to help with a winbond-cir implementation; I have the hardware. >> > >> > >> > Sean >> >> Nuvoton-cir doesn't care about the IR protocol because the hardware >> compares raw IR pulse lengths and wakes the system if received pulse >> is within certain tolerance of the one pre-programmed to the HW. This >> approach is agnostic to the used IR protocol. > > Your patch talks about scancodes which is something entirely different. > This should be renamed to something better. > I agree that for the nuvoton my choice of wording (scancode) was a poor one. Perhaps wakeup_code would suit both drivers? > So with the nuvoton you program a set of pulses and spaces; with the > winbond you set the protocol and the scancode. I don't think there is > any shared code here. Maybe it's better to implement the wakeup > sysfs files in the drivers themselves rather than in rcdev, I guess that > depends on whether there are other devices that implement similar > functionality. > The code to be shared is the logic of creating, parsing and formatting the sysfs file. In the end the drivers are only interested in getting a bunch of values to write to the hardware. I was thinking about adding another file (wakeup_protocol sounds good) which would tell what semantics are used to interpret the contents of wakeup_code file (rc6, rc5, nec or raw). Would this be a decent solution? The other alternative is to push the sysfs handling to individual drivers. I'm ok with either way. Which one should I pursue? -Antti -- 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
On 22.01.2014 21:10, Antti Seppälä wrote: > On 22 January 2014 18:29, Sean Young <sean@mess.org> wrote: >> On Wed, Jan 22, 2014 at 05:46:28PM +0200, Antti Seppälä wrote: >>> On 21 January 2014 14:28, Sean Young <sean@mess.org> wrote: >>>> On Mon, Jan 20, 2014 at 09:39:43PM +0200, Antti Seppälä wrote: >>>>> This patch series introduces a simple sysfs file interface for reading >>>>> and writing wakeup scancodes to rc drivers. >>>>> >>>>> This is an improved version of my previous patch for nuvoton-cir which >>>>> did the same thing via module parameters. This is a more generic >>>>> approach allowing other drivers to utilize the interface as well. >>>>> >>>>> I did not port winbond-cir to this method of wakeup scancode setting yet >>>>> because I don't have the hardware to test it and I wanted first to get >>>>> some comments about how the patch series looks. I did however write a >>>>> simple support to read and write scancodes to rc-loopback module. >>>> >>>> Doesn't the nuvoton-cir driver need to know the IR protocol for wakeup? >>>> >>>> This is needed for winbond-cir; I guess this should be another sysfs >>>> file, something like "wakeup_protocol". Even if the nuvoton can only >>>> handle one IR protocol, maybe it should be exported (readonly) via >>>> sysfs? >>>> >>>> I'm happy to help with a winbond-cir implementation; I have the hardware. >>>> >>>> >>>> Sean >>> >>> Nuvoton-cir doesn't care about the IR protocol because the hardware >>> compares raw IR pulse lengths and wakes the system if received pulse >>> is within certain tolerance of the one pre-programmed to the HW. This >>> approach is agnostic to the used IR protocol. >> >> Your patch talks about scancodes which is something entirely different. >> This should be renamed to something better. >> > > I agree that for the nuvoton my choice of wording (scancode) was a > poor one. Perhaps wakeup_code would suit both drivers? > >> So with the nuvoton you program a set of pulses and spaces; with the >> winbond you set the protocol and the scancode. I don't think there is >> any shared code here. Maybe it's better to implement the wakeup >> sysfs files in the drivers themselves rather than in rcdev, I guess that >> depends on whether there are other devices that implement similar >> functionality. >> > > The code to be shared is the logic of creating, parsing and formatting > the sysfs file. In the end the drivers are only interested in getting > a bunch of values to write to the hardware. > > I was thinking about adding another file (wakeup_protocol sounds good) > which would tell what semantics are used to interpret the contents of > wakeup_code file (rc6, rc5, nec or raw). Would this be a decent > solution? > > The other alternative is to push the sysfs handling to individual > drivers. I'm ok with either way. Which one should I pursue? RTL2832U supports also that function. Could you study it too at least check that implementation will be suitable for it? regards Antti
On Wed, Jan 22, 2014 at 09:10:58PM +0200, Antti Seppälä wrote: > On 22 January 2014 18:29, Sean Young <sean@mess.org> wrote: > > On Wed, Jan 22, 2014 at 05:46:28PM +0200, Antti Seppälä wrote: > >> On 21 January 2014 14:28, Sean Young <sean@mess.org> wrote: > >> > On Mon, Jan 20, 2014 at 09:39:43PM +0200, Antti Seppälä wrote: > >> >> This patch series introduces a simple sysfs file interface for reading > >> >> and writing wakeup scancodes to rc drivers. > >> >> > >> >> This is an improved version of my previous patch for nuvoton-cir which > >> >> did the same thing via module parameters. This is a more generic > >> >> approach allowing other drivers to utilize the interface as well. > >> >> > >> >> I did not port winbond-cir to this method of wakeup scancode setting yet > >> >> because I don't have the hardware to test it and I wanted first to get > >> >> some comments about how the patch series looks. I did however write a > >> >> simple support to read and write scancodes to rc-loopback module. > >> > > >> > Doesn't the nuvoton-cir driver need to know the IR protocol for wakeup? > >> > > >> > This is needed for winbond-cir; I guess this should be another sysfs > >> > file, something like "wakeup_protocol". Even if the nuvoton can only > >> > handle one IR protocol, maybe it should be exported (readonly) via > >> > sysfs? > >> > > >> > I'm happy to help with a winbond-cir implementation; I have the hardware. > >> > > >> > > >> > Sean > >> > >> Nuvoton-cir doesn't care about the IR protocol because the hardware > >> compares raw IR pulse lengths and wakes the system if received pulse > >> is within certain tolerance of the one pre-programmed to the HW. This > >> approach is agnostic to the used IR protocol. > > > > Your patch talks about scancodes which is something entirely different. > > This should be renamed to something better. > > > > I agree that for the nuvoton my choice of wording (scancode) was a > poor one. Perhaps wakeup_code would suit both drivers? > > > So with the nuvoton you program a set of pulses and spaces; with the > > winbond you set the protocol and the scancode. I don't think there is > > any shared code here. Maybe it's better to implement the wakeup > > sysfs files in the drivers themselves rather than in rcdev, I guess that > > depends on whether there are other devices that implement similar > > functionality. > > > > The code to be shared is the logic of creating, parsing and formatting > the sysfs file. In the end the drivers are only interested in getting > a bunch of values to write to the hardware. > > I was thinking about adding another file (wakeup_protocol sounds good) > which would tell what semantics are used to interpret the contents of > wakeup_code file (rc6, rc5, nec or raw). Would this be a decent > solution? Good idea. I like it. Sean -- 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
Em Wed, 22 Jan 2014 21:00:24 +0000 Sean Young <sean@mess.org> escreveu: > On Wed, Jan 22, 2014 at 09:10:58PM +0200, Antti Seppälä wrote: > > On 22 January 2014 18:29, Sean Young <sean@mess.org> wrote: > > > On Wed, Jan 22, 2014 at 05:46:28PM +0200, Antti Seppälä wrote: > > >> On 21 January 2014 14:28, Sean Young <sean@mess.org> wrote: > > >> > On Mon, Jan 20, 2014 at 09:39:43PM +0200, Antti Seppälä wrote: > > >> >> This patch series introduces a simple sysfs file interface for reading > > >> >> and writing wakeup scancodes to rc drivers. > > >> >> > > >> >> This is an improved version of my previous patch for nuvoton-cir which > > >> >> did the same thing via module parameters. This is a more generic > > >> >> approach allowing other drivers to utilize the interface as well. > > >> >> > > >> >> I did not port winbond-cir to this method of wakeup scancode setting yet > > >> >> because I don't have the hardware to test it and I wanted first to get > > >> >> some comments about how the patch series looks. I did however write a > > >> >> simple support to read and write scancodes to rc-loopback module. > > >> > > > >> > Doesn't the nuvoton-cir driver need to know the IR protocol for wakeup? > > >> > > > >> > This is needed for winbond-cir; I guess this should be another sysfs > > >> > file, something like "wakeup_protocol". Even if the nuvoton can only > > >> > handle one IR protocol, maybe it should be exported (readonly) via > > >> > sysfs? > > >> > > > >> > I'm happy to help with a winbond-cir implementation; I have the hardware. > > >> > > > >> > > > >> > Sean > > >> > > >> Nuvoton-cir doesn't care about the IR protocol because the hardware > > >> compares raw IR pulse lengths and wakes the system if received pulse > > >> is within certain tolerance of the one pre-programmed to the HW. This > > >> approach is agnostic to the used IR protocol. > > > > > > Your patch talks about scancodes which is something entirely different. > > > This should be renamed to something better. > > > > > > > I agree that for the nuvoton my choice of wording (scancode) was a > > poor one. Perhaps wakeup_code would suit both drivers? > > > > > So with the nuvoton you program a set of pulses and spaces; with the > > > winbond you set the protocol and the scancode. I don't think there is > > > any shared code here. Maybe it's better to implement the wakeup > > > sysfs files in the drivers themselves rather than in rcdev, I guess that > > > depends on whether there are other devices that implement similar > > > functionality. > > > > > > > The code to be shared is the logic of creating, parsing and formatting > > the sysfs file. In the end the drivers are only interested in getting > > a bunch of values to write to the hardware. > > > > I was thinking about adding another file (wakeup_protocol sounds good) > > which would tell what semantics are used to interpret the contents of > > wakeup_code file (rc6, rc5, nec or raw). Would this be a decent > > solution? > > Good idea. I like it. Not sure if you saw it, but there's already another patchset proposing that, that got submitted before this changeset: https://patchwork.linuxtv.org/patch/21625/ > > Sean
On 23 January 2014 00:01, Mauro Carvalho Chehab <m.chehab@samsung.com> wrote: > Not sure if you saw it, but there's already another patchset proposing > that, that got submitted before this changeset: > https://patchwork.linuxtv.org/patch/21625/ I actually didn't notice that until now. Seems quite a similar kind of approach with even more advanced features than what I had in mind (namely the scancode filtering and masking). However it looks like that patchset has the same drawback about not knowing which protocol to use for the wakeup scancode as was pointed from my patch. I think I'll try to come up with a new patch addressing the comments I've seen so far. -Antti -- 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
Em Thu, 23 Jan 2014 21:11:09 +0200 Antti Seppälä <a.seppala@gmail.com> escreveu: > On 23 January 2014 00:01, Mauro Carvalho Chehab <m.chehab@samsung.com> wrote: > > Not sure if you saw it, but there's already another patchset proposing > > that, that got submitted before this changeset: > > https://patchwork.linuxtv.org/patch/21625/ > > I actually didn't notice that until now. Seems quite a similar kind of > approach with even more advanced features than what I had in mind > (namely the scancode filtering and masking). > > However it looks like that patchset has the same drawback about not > knowing which protocol to use for the wakeup scancode as was pointed > from my patch. Well, the protocol is important only if there are two or more active RCs that produce the same IR code on different protocols. Also, from the sysfs description made by James, it seems clear to me that the protocol to be used is the current protocol. I think is an unlikely border case to have some hardware that supports more than one IR protocols enabled for the wakeup to work, so James patch looks ok on my eyes. Also, nothing prevents to add latter a wakeup_filter_protocol sysfs node to allow to filter the wakeup scancode to a protocol set different than the one(s) currently enabled. > I think I'll try to come up with a new patch addressing the comments > I've seen so far. I guess I'll merge James approach, as there are a pile of other patches depending on it. If we need latter to distinguish between current_protocol and the wakeup one, as I said, a latter patch can add a "wakeup_filter_protocol" sysfs node to specify it. Regards, Mauro -- 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
On 4 February 2014 19:54, Mauro Carvalho Chehab <m.chehab@samsung.com> wrote: > Em Thu, 23 Jan 2014 21:11:09 +0200 > Antti Seppälä <a.seppala@gmail.com> escreveu: > >> On 23 January 2014 00:01, Mauro Carvalho Chehab <m.chehab@samsung.com> wrote: >> > Not sure if you saw it, but there's already another patchset proposing >> > that, that got submitted before this changeset: >> > https://patchwork.linuxtv.org/patch/21625/ >> >> I actually didn't notice that until now. Seems quite a similar kind of >> approach with even more advanced features than what I had in mind >> (namely the scancode filtering and masking). >> >> However it looks like that patchset has the same drawback about not >> knowing which protocol to use for the wakeup scancode as was pointed >> from my patch. > > Well, the protocol is important only if there are two or more active > RCs that produce the same IR code on different protocols. > > Also, from the sysfs description made by James, it seems clear to me > that the protocol to be used is the current protocol. > > I think is an unlikely border case to have some hardware that supports > more than one IR protocols enabled for the wakeup to work, so James > patch looks ok on my eyes. > > Also, nothing prevents to add latter a wakeup_filter_protocol sysfs node > to allow to filter the wakeup scancode to a protocol set different than > the one(s) currently enabled. > >> I think I'll try to come up with a new patch addressing the comments >> I've seen so far. > > I guess I'll merge James approach, as there are a pile of other patches > depending on it. If we need latter to distinguish between current_protocol > and the wakeup one, as I said, a latter patch can add a > "wakeup_filter_protocol" sysfs node to specify it. > > Regards, > Mauro Hi Mauro. How do you want to proceed with the nuvoton wakeup I initially set out to do? To wake up with nuvoton-cir we need to program several raw ir pulse/space lengths to the hardware and not a scancode. James's approach doesn't support this. To keep things simple maybe module parameters in my first patch (http://www.mail-archive.com/linux-media@vger.kernel.org/msg69782.html) are then the best thing to do for nuvoton? Other drivers can probably adapt to the suggested filter api. -Antti -- 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
Hi Antti, Em Wed, 05 Feb 2014 09:03:29 +0200 Antti Seppälä <a.seppala@gmail.com> escreveu: > On 4 February 2014 19:54, Mauro Carvalho Chehab <m.chehab@samsung.com> wrote: > > Em Thu, 23 Jan 2014 21:11:09 +0200 > > Antti Seppälä <a.seppala@gmail.com> escreveu: > > > >> On 23 January 2014 00:01, Mauro Carvalho Chehab <m.chehab@samsung.com> wrote: > >> > Not sure if you saw it, but there's already another patchset proposing > >> > that, that got submitted before this changeset: > >> > https://patchwork.linuxtv.org/patch/21625/ > >> > >> I actually didn't notice that until now. Seems quite a similar kind of > >> approach with even more advanced features than what I had in mind > >> (namely the scancode filtering and masking). > >> > >> However it looks like that patchset has the same drawback about not > >> knowing which protocol to use for the wakeup scancode as was pointed > >> from my patch. > > > > Well, the protocol is important only if there are two or more active > > RCs that produce the same IR code on different protocols. > > > > Also, from the sysfs description made by James, it seems clear to me > > that the protocol to be used is the current protocol. > > > > I think is an unlikely border case to have some hardware that supports > > more than one IR protocols enabled for the wakeup to work, so James > > patch looks ok on my eyes. > > > > Also, nothing prevents to add latter a wakeup_filter_protocol sysfs node > > to allow to filter the wakeup scancode to a protocol set different than > > the one(s) currently enabled. > > > >> I think I'll try to come up with a new patch addressing the comments > >> I've seen so far. > > > > I guess I'll merge James approach, as there are a pile of other patches > > depending on it. If we need latter to distinguish between current_protocol > > and the wakeup one, as I said, a latter patch can add a > > "wakeup_filter_protocol" sysfs node to specify it. > > > > Regards, > > Mauro > > Hi Mauro. > > How do you want to proceed with the nuvoton wakeup I initially set out to do? > > To wake up with nuvoton-cir we need to program several raw ir > pulse/space lengths to the hardware and not a scancode. James's > approach doesn't support this. > > To keep things simple maybe module parameters in my first patch > (http://www.mail-archive.com/linux-media@vger.kernel.org/msg69782.html) > are then the best thing to do for nuvoton? Other drivers can probably > adapt to the suggested filter api. Well, you're basically telling that you need an IR encoder to convert from protocol/scancode into pulse/space lengths. As we need/want a portable interface that will provide the same API to userspace, no matter what hardware, I can see two alternatives: 1) Use a lirc-like API for that, using the IR encoder code in userspace; 2) The same way as we mapped IR decoders, add IR encoders that can be used either for IR TX or to provide IR wakeup codes. IMO, (2) is a much more elegant option. Regards, Mauro -- 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
Hi Antti, On 05/02/14 07:03, Antti Seppälä wrote: > To wake up with nuvoton-cir we need to program several raw ir > pulse/space lengths to the hardware and not a scancode. James's > approach doesn't support this. Do the raw pulse/space lengths your hardware requires correspond to a single IR packet (mapping to a single scancode)? If so then my API is simply at a higher level of abstraction. I think this has the following advantages: * userspace sees a consistent interface at the same level of abstraction as it already has access to from input subsystem (i.e. scancodes). I.e. it doesn't need to care which IR device is in use, whether it does raw/hardware decode, or the details of the timings of the current protocol. * it supports hardware decoders which filter on the demodulated data rather than the raw pulse/space lengths. Of course to support this we'd need some per-protocol code to convert a scancode back to pulse/space lengths. I'd like to think that code could be generic, maybe as helper functions which multiple drivers could use, which could also handle corner cases of the API in a consistent way (e.g. user providing filter mask covering multiple scancodes, which presumably pulse/space). I see I've just crossed emails with Mauro who has just suggested something similar. I agree that his (2) is the more elegant option. Cheers James -- 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
On 05/02/14 09:39, James Hogan wrote: > Hi Antti, > > On 05/02/14 07:03, Antti Seppälä wrote: >> To wake up with nuvoton-cir we need to program several raw ir >> pulse/space lengths to the hardware and not a scancode. James's >> approach doesn't support this. > > Do the raw pulse/space lengths your hardware requires correspond to a > single IR packet (mapping to a single scancode)? > > If so then my API is simply at a higher level of abstraction. I think > this has the following advantages: > * userspace sees a consistent interface at the same level of abstraction > as it already has access to from input subsystem (i.e. scancodes). I.e. > it doesn't need to care which IR device is in use, whether it does > raw/hardware decode, or the details of the timings of the current protocol. > * it supports hardware decoders which filter on the demodulated data > rather than the raw pulse/space lengths. > > Of course to support this we'd need some per-protocol code to convert a > scancode back to pulse/space lengths. I'd like to think that code could > be generic, maybe as helper functions which multiple drivers could use, > which could also handle corner cases of the API in a consistent way > (e.g. user providing filter mask covering multiple scancodes, which > presumably pulse/space). hmm, I didn't complete that sentence :(. I meant: ..., which presumably pulse/space can't really represent very easily). Cheers James > > I see I've just crossed emails with Mauro who has just suggested > something similar. I agree that his (2) is the more elegant option. > > Cheers > James > -- 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
On 5 February 2014 11:42, James Hogan <james.hogan@imgtec.com> wrote: > On 05/02/14 09:39, James Hogan wrote: >> Hi Antti, >> >> On 05/02/14 07:03, Antti Seppälä wrote: >>> To wake up with nuvoton-cir we need to program several raw ir >>> pulse/space lengths to the hardware and not a scancode. James's >>> approach doesn't support this. >> >> Do the raw pulse/space lengths your hardware requires correspond to a >> single IR packet (mapping to a single scancode)? >> >> If so then my API is simply at a higher level of abstraction. I think >> this has the following advantages: >> * userspace sees a consistent interface at the same level of abstraction >> as it already has access to from input subsystem (i.e. scancodes). I.e. >> it doesn't need to care which IR device is in use, whether it does >> raw/hardware decode, or the details of the timings of the current protocol. >> * it supports hardware decoders which filter on the demodulated data >> rather than the raw pulse/space lengths. >> >> Of course to support this we'd need some per-protocol code to convert a >> scancode back to pulse/space lengths. I'd like to think that code could >> be generic, maybe as helper functions which multiple drivers could use, >> which could also handle corner cases of the API in a consistent way >> (e.g. user providing filter mask covering multiple scancodes, which >> presumably pulse/space). > > hmm, I didn't complete that sentence :(. > I meant: > ..., which presumably pulse/space can't really represent very easily). > > Cheers > James > >> >> I see I've just crossed emails with Mauro who has just suggested >> something similar. I agree that his (2) is the more elegant option. >> Yes, in nuvoton the ir pulses correspond to a scancode (or part of a scancode) After giving it some thought I agree that using scancodes is the most elegant way for specifying wakeup commands. Too bad that nuvoton does not work with scancodes. I pretty much agree with Mauro that the right solution would be to write an IR encoder and use it to convert the given scancode back to a format understood by nuvoton. Writing IR encoders for all the protocols and an encoder selector functionality is quite labourous and sadly I don't have time for that anytime soon. If anyone wants to step up I'd be more than happy to help though :) -Antti -- 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
Em Wed, 05 Feb 2014 20:16:04 +0200 Antti Seppälä <a.seppala@gmail.com> escreveu: > On 5 February 2014 11:42, James Hogan <james.hogan@imgtec.com> wrote: > > On 05/02/14 09:39, James Hogan wrote: > >> Hi Antti, > >> > >> On 05/02/14 07:03, Antti Seppälä wrote: > >>> To wake up with nuvoton-cir we need to program several raw ir > >>> pulse/space lengths to the hardware and not a scancode. James's > >>> approach doesn't support this. > >> > >> Do the raw pulse/space lengths your hardware requires correspond to a > >> single IR packet (mapping to a single scancode)? > >> > >> If so then my API is simply at a higher level of abstraction. I think > >> this has the following advantages: > >> * userspace sees a consistent interface at the same level of abstraction > >> as it already has access to from input subsystem (i.e. scancodes). I.e. > >> it doesn't need to care which IR device is in use, whether it does > >> raw/hardware decode, or the details of the timings of the current protocol. > >> * it supports hardware decoders which filter on the demodulated data > >> rather than the raw pulse/space lengths. > >> > >> Of course to support this we'd need some per-protocol code to convert a > >> scancode back to pulse/space lengths. I'd like to think that code could > >> be generic, maybe as helper functions which multiple drivers could use, > >> which could also handle corner cases of the API in a consistent way > >> (e.g. user providing filter mask covering multiple scancodes, which > >> presumably pulse/space). > > > > hmm, I didn't complete that sentence :(. > > I meant: > > ..., which presumably pulse/space can't really represent very easily). > > > > Cheers > > James > > > >> > >> I see I've just crossed emails with Mauro who has just suggested > >> something similar. I agree that his (2) is the more elegant option. > >> > > Yes, in nuvoton the ir pulses correspond to a scancode (or part of a scancode) > > After giving it some thought I agree that using scancodes is the most > elegant way for specifying wakeup commands. Too bad that nuvoton does > not work with scancodes. > I pretty much agree with Mauro that the right solution would be to > write an IR encoder and use it to convert the given scancode back to a > format understood by nuvoton. Ok, as we all agreed, I'll merge the remaining patches from James. > Writing IR encoders for all the protocols and an encoder selector > functionality is quite labourous and sadly I don't have time for that > anytime soon. If anyone wants to step up I'd be more than happy to > help though :) I suspect that writing one IR encoder should not be hard, as there are already some on LIRC userspace. I would love to have some time to write at least a few IR encoders, but, unfortunately, I would not have any time soon. Regards, Mauro -- 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
On 05/02/14 21:21, Mauro Carvalho Chehab wrote: > Em Wed, 05 Feb 2014 20:16:04 +0200 > Antti Seppälä <a.seppala@gmail.com> escreveu: > >> On 5 February 2014 11:42, James Hogan <james.hogan@imgtec.com> wrote: >>> On 05/02/14 09:39, James Hogan wrote: >>>> Hi Antti, >>>> >>>> On 05/02/14 07:03, Antti Seppälä wrote: >>>>> To wake up with nuvoton-cir we need to program several raw ir >>>>> pulse/space lengths to the hardware and not a scancode. James's >>>>> approach doesn't support this. >>>> >>>> Do the raw pulse/space lengths your hardware requires correspond to a >>>> single IR packet (mapping to a single scancode)? >>>> >>>> If so then my API is simply at a higher level of abstraction. I think >>>> this has the following advantages: >>>> * userspace sees a consistent interface at the same level of abstraction >>>> as it already has access to from input subsystem (i.e. scancodes). I.e. >>>> it doesn't need to care which IR device is in use, whether it does >>>> raw/hardware decode, or the details of the timings of the current protocol. >>>> * it supports hardware decoders which filter on the demodulated data >>>> rather than the raw pulse/space lengths. >>>> >>>> Of course to support this we'd need some per-protocol code to convert a >>>> scancode back to pulse/space lengths. I'd like to think that code could >>>> be generic, maybe as helper functions which multiple drivers could use, >>>> which could also handle corner cases of the API in a consistent way >>>> (e.g. user providing filter mask covering multiple scancodes, which >>>> presumably pulse/space). >>> >>> hmm, I didn't complete that sentence :(. >>> I meant: >>> ..., which presumably pulse/space can't really represent very easily). >>> >>> Cheers >>> James >>> >>>> >>>> I see I've just crossed emails with Mauro who has just suggested >>>> something similar. I agree that his (2) is the more elegant option. >>>> >> >> Yes, in nuvoton the ir pulses correspond to a scancode (or part of a scancode) >> >> After giving it some thought I agree that using scancodes is the most >> elegant way for specifying wakeup commands. Too bad that nuvoton does >> not work with scancodes. >> I pretty much agree with Mauro that the right solution would be to >> write an IR encoder and use it to convert the given scancode back to a >> format understood by nuvoton. > > Ok, as we all agreed, I'll merge the remaining patches from James. Thanks :) > >> Writing IR encoders for all the protocols and an encoder selector >> functionality is quite labourous and sadly I don't have time for that >> anytime soon. If anyone wants to step up I'd be more than happy to >> help though :) > > I suspect that writing one IR encoder should not be hard, as there > are already some on LIRC userspace. > > I would love to have some time to write at least a few IR encoders, > but, unfortunately, I would not have any time soon. For fun, I did some experimentation yesterday evening with adding basic generic IR encoding (just NEC implemented so far). Encoders can certainly be a lot simpler than decoders. I'll submit a few RFC patches later so Antti can see whether it would be suitable for nuvoton. Cheers James -- 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
Em Thu, 06 Feb 2014 10:46:04 +0000 James Hogan <james.hogan@imgtec.com> escreveu: > On 05/02/14 21:21, Mauro Carvalho Chehab wrote: > > Em Wed, 05 Feb 2014 20:16:04 +0200 > > Antti Seppälä <a.seppala@gmail.com> escreveu: > > > >> On 5 February 2014 11:42, James Hogan <james.hogan@imgtec.com> wrote: > >>> On 05/02/14 09:39, James Hogan wrote: > >>>> Hi Antti, > >>>> > >>>> On 05/02/14 07:03, Antti Seppälä wrote: > >>>>> To wake up with nuvoton-cir we need to program several raw ir > >>>>> pulse/space lengths to the hardware and not a scancode. James's > >>>>> approach doesn't support this. > >>>> > >>>> Do the raw pulse/space lengths your hardware requires correspond to a > >>>> single IR packet (mapping to a single scancode)? > >>>> > >>>> If so then my API is simply at a higher level of abstraction. I think > >>>> this has the following advantages: > >>>> * userspace sees a consistent interface at the same level of abstraction > >>>> as it already has access to from input subsystem (i.e. scancodes). I.e. > >>>> it doesn't need to care which IR device is in use, whether it does > >>>> raw/hardware decode, or the details of the timings of the current protocol. > >>>> * it supports hardware decoders which filter on the demodulated data > >>>> rather than the raw pulse/space lengths. > >>>> > >>>> Of course to support this we'd need some per-protocol code to convert a > >>>> scancode back to pulse/space lengths. I'd like to think that code could > >>>> be generic, maybe as helper functions which multiple drivers could use, > >>>> which could also handle corner cases of the API in a consistent way > >>>> (e.g. user providing filter mask covering multiple scancodes, which > >>>> presumably pulse/space). > >>> > >>> hmm, I didn't complete that sentence :(. > >>> I meant: > >>> ..., which presumably pulse/space can't really represent very easily). > >>> > >>> Cheers > >>> James > >>> > >>>> > >>>> I see I've just crossed emails with Mauro who has just suggested > >>>> something similar. I agree that his (2) is the more elegant option. > >>>> > >> > >> Yes, in nuvoton the ir pulses correspond to a scancode (or part of a scancode) > >> > >> After giving it some thought I agree that using scancodes is the most > >> elegant way for specifying wakeup commands. Too bad that nuvoton does > >> not work with scancodes. > >> I pretty much agree with Mauro that the right solution would be to > >> write an IR encoder and use it to convert the given scancode back to a > >> format understood by nuvoton. > > > > Ok, as we all agreed, I'll merge the remaining patches from James. > > Thanks :) Well, Rob asked some changes at the DT binding pathes. I'll tag the patches as changes requested. Please re-submit the remaining ones after fixing the pointed issues and getting his ack. Except for that, the series look OK on my eyes. > > > >> Writing IR encoders for all the protocols and an encoder selector > >> functionality is quite labourous and sadly I don't have time for that > >> anytime soon. If anyone wants to step up I'd be more than happy to > >> help though :) > > > > I suspect that writing one IR encoder should not be hard, as there > > are already some on LIRC userspace. > > > > I would love to have some time to write at least a few IR encoders, > > but, unfortunately, I would not have any time soon. > > For fun, I did some experimentation yesterday evening with adding basic > generic IR encoding (just NEC implemented so far). Encoders can > certainly be a lot simpler than decoders. Yes, you don't need to deal with time shifts there, nor with missing events ;) Well, using the encoders also for TX (as an another ioctl at LIRC) would require some extra bits to handle things like carrier frequency, duty cycle, etc, but lirc should be able to handle those already. So, a future patch expanding its usage to lirc would just need to add the proper binding. Btw, I just noticed that the LIRC UAPI interface is still at the wrong place. It is at: include/media/lirc.h but it should be at: include/uapi/.../lirc.h I'm wandering why nobody didn't notice it yet when compiling Lirc userspace... Perhaps userspace is just keeping a copy of that file without any process to sync it from the Kernel userspace headers. > I'll submit a few RFC patches later so Antti can see whether it would be > suitable for nuvoton. Thanks for that! Regards, Mauro -- 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
diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c index 21ee0dc..b11ee43 100644 --- a/drivers/media/rc/nuvoton-cir.c +++ b/drivers/media/rc/nuvoton-cir.c @@ -39,6 +39,15 @@ #include "nuvoton-cir.h" +/* debugging module parameter */ +static int debug; + +/* Wake up configuration parameters */ +static unsigned char wake_samples[WAKE_FIFO_LEN]; +static unsigned int num_wake_samples; +static unsigned char cmp_deep; +static unsigned char cmp_tolerance = CIR_WAKE_CMP_TOLERANCE; + /* write val to config reg */ static inline void nvt_cr_write(struct nvt_dev *nvt, u8 val, u8 reg) { @@ -418,13 +427,38 @@ static void nvt_cir_regs_init(struct nvt_dev *nvt) static void nvt_cir_wake_regs_init(struct nvt_dev *nvt) { + int i; + u8 cmp_reg = CIR_WAKE_FIFO_CMP_BYTES; + u8 ircon_reg = CIR_WAKE_IRCON_RXEN | CIR_WAKE_IRCON_R | + CIR_WAKE_IRCON_RXINV | CIR_WAKE_IRCON_SAMPLE_PERIOD_SEL; + /* + * Enable TX and RX, specific carrier on = low, off = high, and set + * sample period (currently 50us) + */ + nvt_cir_wake_reg_write(nvt, ircon_reg | CIR_WAKE_IRCON_MODE1, + CIR_WAKE_IRCON); + + /* clear cir wake rx fifo */ + nvt_clear_cir_wake_fifo(nvt); + + /* Write samples from module parameter to fifo */ + for (i = 0; i < num_wake_samples; i++) + nvt_cir_wake_reg_write(nvt, wake_samples[i], + CIR_WAKE_WR_FIFO_DATA); + + /* Switch cir to wakeup mode and disable fifo writing */ + nvt_cir_wake_reg_write(nvt, ircon_reg | CIR_WAKE_IRCON_MODE0, + CIR_WAKE_IRCON); + /* set number of bytes needed for wake from s3 (default 65) */ - nvt_cir_wake_reg_write(nvt, CIR_WAKE_FIFO_CMP_BYTES, - CIR_WAKE_FIFO_CMP_DEEP); + if (cmp_deep) + cmp_reg = cmp_deep; + else if (num_wake_samples) + cmp_reg = num_wake_samples; + nvt_cir_wake_reg_write(nvt, cmp_reg, CIR_WAKE_FIFO_CMP_DEEP); /* set tolerance/variance allowed per byte during wake compare */ - nvt_cir_wake_reg_write(nvt, CIR_WAKE_CMP_TOLERANCE, - CIR_WAKE_FIFO_CMP_TOL); + nvt_cir_wake_reg_write(nvt, cmp_tolerance, CIR_WAKE_FIFO_CMP_TOL); /* set sample limit count (PE interrupt raised when reached) */ nvt_cir_wake_reg_write(nvt, CIR_RX_LIMIT_COUNT >> 8, CIR_WAKE_SLCH); @@ -434,18 +468,6 @@ static void nvt_cir_wake_regs_init(struct nvt_dev *nvt) nvt_cir_wake_reg_write(nvt, CIR_WAKE_FIFOCON_RX_TRIGGER_LEV, CIR_WAKE_FIFOCON); - /* - * Enable TX and RX, specific carrier on = low, off = high, and set - * sample period (currently 50us) - */ - nvt_cir_wake_reg_write(nvt, CIR_WAKE_IRCON_MODE0 | CIR_WAKE_IRCON_RXEN | - CIR_WAKE_IRCON_R | CIR_WAKE_IRCON_RXINV | - CIR_WAKE_IRCON_SAMPLE_PERIOD_SEL, - CIR_WAKE_IRCON); - - /* clear cir wake rx fifo */ - nvt_clear_cir_wake_fifo(nvt); - /* clear any and all stray interrupts */ nvt_cir_wake_reg_write(nvt, 0xff, CIR_WAKE_IRSTS); } @@ -1232,6 +1254,17 @@ static void nvt_exit(void) module_param(debug, int, S_IRUGO | S_IWUSR); MODULE_PARM_DESC(debug, "Enable debugging output"); +module_param_array(wake_samples, byte, &num_wake_samples, S_IRUGO | S_IWUSR); +MODULE_PARM_DESC(wake_samples, "FIFO sample bytes triggering wake"); + +module_param(cmp_deep, byte, S_IRUGO | S_IWUSR); +MODULE_PARM_DESC(cmp_deep, "How many bytes need to compare\n" + "\t\t(0 = auto (default))"); + +module_param(cmp_tolerance, byte, S_IRUGO | S_IWUSR); +MODULE_PARM_DESC(cmp_tolerance, "Data tolerance to each wake sample byte\n" + "\t\t(default = 5)"); + MODULE_DEVICE_TABLE(pnp, nvt_ids); MODULE_DESCRIPTION("Nuvoton W83667HG-A & W83677HG-I CIR driver"); diff --git a/drivers/media/rc/nuvoton-cir.h b/drivers/media/rc/nuvoton-cir.h index 07e8310..8209f84 100644 --- a/drivers/media/rc/nuvoton-cir.h +++ b/drivers/media/rc/nuvoton-cir.h @@ -31,10 +31,6 @@ /* platform driver name to register */ #define NVT_DRIVER_NAME "nuvoton-cir" -/* debugging module parameter */ -static int debug; - - #define nvt_pr(level, text, ...) \ printk(level KBUILD_MODNAME ": " text, ## __VA_ARGS__) @@ -64,6 +60,8 @@ static int debug; #define TX_BUF_LEN 256 #define RX_BUF_LEN 32 +#define WAKE_FIFO_LEN 67 + struct nvt_dev { struct pnp_dev *pdev; struct rc_dev *rdev;