From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935664AbcJUWEJ (ORCPT ); Fri, 21 Oct 2016 18:04:09 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:59362 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933808AbcJUWEH (ORCPT ); Fri, 21 Oct 2016 18:04:07 -0400 Date: Fri, 21 Oct 2016 15:03:24 -0700 From: Guenter Roeck To: Christopher Heiny Cc: Nick Dyer , Dmitry Torokhov , Chris Healy , Andrew Duggan , Mauro Carvalho Chehab , Hans Verkuil , linux-kernel@vger.kernel.org, linux-input@vger.kernel.org Subject: Re: [-next, 1/2] Input: synaptics-rmi4 - add support for F55 sensor tuning Message-ID: <20161021220324.GA3269@roeck-us.net> References: <1475292168-20961-1-git-send-email-linux@roeck-us.net> <20161017213008.GA20263@roeck-us.net> <20161020225107.GA13204@lava.h.shmanahar.org> <1477006119.5714.46.camel@synaptics.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1477006119.5714.46.camel@synaptics.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Authenticated_sender: guenter@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: guenter@roeck-us.net X-Authenticated-Sender: bh-25.webhostbox.net: guenter@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Oct 20, 2016 at 04:28:39PM -0700, Christopher Heiny wrote: > On Thu, 2016-10-20 at 23:51 +0100, Nick Dyer wrote: > > On Mon, Oct 17, 2016 at 02:30:08PM -0700, Guenter Roeck wrote: > > > > > > On Fri, Sep 30, 2016 at 08:22:47PM -0700, Guenter Roeck wrote: > > > > > > > > Sensor tuning support is needed to determine the number of > > > > enabled > > > > tx and rx electrodes for use in F54 functions. > > > > > > > > The number of enabled electrodes is not identical to the total > > > > number > > > > of electrodes as reported with F55:Query0 and F55:Query1. It has > > > > to be > > > > calculated by analyzing F55:Ctrl1 (sensor receiver assignment) > > > > and > > > > F55:Ctrl2 (sensor transmitter assignment). > > > > > > > > Support for additional sensor tuning functions may be added > > > > later. > > > > > > > > Signed-off-by: Guenter Roeck > > > > > > Ping ... any comments on this patch and on > > > https://patchwork.kernel.org/patch/9359061/ ? > > > > > > Both patches now apply to mainline. > > > > > > Thanks, > > > Guenter > > > > Hi Guenter- > > > > I've reviewed and tested (on S7300 and S7813) both these patches now > > - you can add my sign-off. > > > > However, on the S7813 firmware, F55 is on PDT page 3, and nothing > > on page 2, so the default behaviour of the mainline driver means it > > is > > not initialised. > > > > So I think we need to revert this change in mainline: > > https://patchwork.kernel.org/patch/3796971/ > > > > See below the PDT scan with it reverted and some debug added. > > > > Christopher/Andrew: is there a better heuristic than scanning all 255 > > pages, given that some firmwares contain gaps? > > It's difficult to say.  It is against the RMI4 spec for there to be > gaps in the pages - you're supposed to be able to scan until you hit a > page with an empty PDT, and then stop. > > Since F55 is hardcoded to page 3 for this firmware, it may be a > customer specific deviation.  This may have been done to accommodate a > customer-written driver that did not scan the PDT, but instead always > looked for F55 on page 3.  This idea is supported by the existence of > the F51 custom function on page 4, since F51 almost always requires > customer driver code to handle it. > > In my opinion, the Non-standard bit should have been set in the PDT to > indicate that special handling was required, but that wasn't done in > this case. > > Anyway, given that this sort of thing has escaped into the wild, I'm > unsure what to advise.  Just off the top of my head, one possibility is > to have a "keep-going" option to scan the first 128 pages (0x00 through > 0x7F), regardless of whether an empty page is encountered.  This could > be triggered either by a product ID on the "known goofy list", or by a > boot/load time flag.  I'm sure there are other possibilities, though. > Maybe introduce quirks if the problematic device and/or problematic firmware version can be identified ? Not sure if scanning 128 pages would be necessary, though - requiring two empty pages to stop scanning might be sufficient. Guenter