From patchwork Mon Feb 1 15:08:46 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mauro Carvalho Chehab X-Patchwork-Id: 8180391 Return-Path: X-Original-To: patchwork-linux-media@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id D8E5EBEEED for ; Mon, 1 Feb 2016 15:09:02 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E2C3D2027D for ; Mon, 1 Feb 2016 15:09:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DCA2C2028D for ; Mon, 1 Feb 2016 15:09:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933266AbcBAPIy (ORCPT ); Mon, 1 Feb 2016 10:08:54 -0500 Received: from lists.s-osg.org ([54.187.51.154]:39700 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932962AbcBAPIw (ORCPT ); Mon, 1 Feb 2016 10:08:52 -0500 Received: from recife.lan (177.43.30.196.dynamic.adsl.gvt.net.br [177.43.30.196]) by lists.s-osg.org (Postfix) with ESMTPSA id 692FE462E7; Mon, 1 Feb 2016 07:08:50 -0800 (PST) Date: Mon, 1 Feb 2016 13:08:46 -0200 From: Mauro Carvalho Chehab To: Alec Leamas Cc: david@hardeman.nu, austin.lund@gmail.com, linux-media@vger.kernel.org Subject: Re: [PATCH] Revert "[media] media/rc: Send sync space information on lirc device" Message-ID: <20160201130846.36eb1cdc@recife.lan> In-Reply-To: <1453792266-1542-1-git-send-email-leamas.alec@gmail.com> References: <1453792266-1542-1-git-send-email-leamas.alec@gmail.com> Organization: Samsung X-Mailer: Claws Mail 3.13.1 (GTK+ 2.24.29; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Em Tue, 26 Jan 2016 08:11:06 +0100 Alec Leamas escreveu: > This reverts commit a8f29e89f2b54fbf2c52be341f149bc195b63a8b. This > commit handled drivers failing to issue a spac which causes sequences > of mark-mark-space instead of the expected space-mark-space-mark... > > The fix added an extra space for each and every timeout which fixes > the problem for the failing drivers. However, for existing working > drivers it the added space causes mark-space-space sequences in the > output which break userspace rightfully expecting > space-mark-space-mark... > > Thus, the fix is broken and reverted. The fix is discussed in > https://bugzilla.redhat.com/show_bug.cgi?id=1260862. In particular, > the original committer Austin Lund agrees. Reverting a patch applied on 3.18 seems very risky as other drivers may rely on this behavior. I guess the best thing here would be to detect if a space was already sent, before sending an extra space at ev.reset, e. g. something like the following (untested) patch. Could you please check if it solves the issue? Thanks, Mauro lirc: don't send two space events at reset The LIRC protocol doesn't expect to receive two space events without a pulse between them. This doesn't happen on the usual handling, but ev.reset could cause an extra long space event, with is wrong. Signed-off-by: Mauro Carvalho Chehab --- 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/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c index 5effc65d2947..e03ea0091dcd 100644 --- a/drivers/media/rc/ir-lirc-codec.c +++ b/drivers/media/rc/ir-lirc-codec.c @@ -39,13 +39,14 @@ static int ir_lirc_decode(struct rc_dev *dev, struct ir_raw_event ev) return -EINVAL; /* Packet start */ - if (ev.reset) { + if (ev.reset && lirc->last_ev_is_pulse) { /* Userspace expects a long space event before the start of * the signal to use as a sync. This may be done with repeat * packets and normal samples. But if a reset has been sent * then we assume that a long time has passed, so we send a * space with the maximum time value. */ sample = LIRC_SPACE(LIRC_VALUE_MASK); + lirc->last_ev_is_pulse = false; IR_dprintk(2, "delivering reset sync space to lirc_dev\n"); /* Carrier reports */ @@ -84,6 +85,7 @@ static int ir_lirc_decode(struct rc_dev *dev, struct ir_raw_event ev) (u64)LIRC_VALUE_MASK); gap_sample = LIRC_SPACE(lirc->gap_duration); + lirc->last_ev_is_pulse = false; lirc_buffer_write(dev->raw->lirc.drv->rbuf, (unsigned char *) &gap_sample); lirc->gap = false; @@ -91,6 +93,8 @@ static int ir_lirc_decode(struct rc_dev *dev, struct ir_raw_event ev) sample = ev.pulse ? LIRC_PULSE(ev.duration / 1000) : LIRC_SPACE(ev.duration / 1000); + lirc->last_ev_is_pulse = ev.pulse; + IR_dprintk(2, "delivering %uus %s to lirc_dev\n", TO_US(ev.duration), TO_STR(ev.pulse)); } diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h index 7359f3d03b64..6d9c92e0b7a7 100644 --- a/drivers/media/rc/rc-core-priv.h +++ b/drivers/media/rc/rc-core-priv.h @@ -108,7 +108,7 @@ struct ir_raw_event_ctrl { u64 gap_duration; bool gap; bool send_timeout_reports; - + bool last_ev_is_pulse; } lirc; struct xmp_dec { int state;