From patchwork Sun Apr 17 00:07:44 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Heiko_St=C3=BCbner?= X-Patchwork-Id: 8862291 Return-Path: X-Original-To: patchwork-linux-rockchip@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 C6518BF29F for ; Sun, 17 Apr 2016 00:08:21 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id AA319201CE for ; Sun, 17 Apr 2016 00:08:20 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6E43D201B9 for ; Sun, 17 Apr 2016 00:08:19 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1araG9-0005qG-C5; Sun, 17 Apr 2016 00:08:17 +0000 Received: from gloria.sntech.de ([95.129.55.99]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1araG6-0005a8-2i for linux-rockchip@lists.infradead.org; Sun, 17 Apr 2016 00:08:15 +0000 Received: from ip9234b7c8.dynamic.kabel-deutschland.de ([146.52.183.200] helo=diego.localnet) by gloria.sntech.de with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.80) (envelope-from ) id 1araFd-000556-00; Sun, 17 Apr 2016 02:07:45 +0200 From: Heiko =?ISO-8859-1?Q?St=FCbner?= To: Javier Martinez Canillas , linux-rockchip@lists.infradead.org Subject: Re: Issues with cros_ec and "spi: rockchip: check return value of dmaengine_prep_slave_sg" Date: Sun, 17 Apr 2016 02:07:44 +0200 Message-ID: <2001833.m4ljKS3hpv@diego> User-Agent: KMail/4.14.10 (Linux/4.4.0-1-amd64; KDE/4.14.14; x86_64; ; ) In-Reply-To: References: <5681888.LvkE41Ux3K@phil> <32438337.LoARtRoxf0@phil> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160416_170814_295133_C6DFBAE3 X-CRM114-Status: GOOD ( 28.69 ) X-Spam-Score: -2.9 (--) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Enric Balletbo Serra , Shawn Lin , Doug Anderson , Tomeu Vizoso , linux-spi@vger.kernel.org Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, 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 Am Montag, 11. April 2016, 13:54:51 schrieb Javier Martinez Canillas: > Hello Heiko, > > Sorry for the late response, I got this email while I was sick and > forgot to answer when I recovered. > > On Sat, Apr 2, 2016 at 9:37 AM, Heiko Stuebner wrote: > > Hi Shawn, > > > > Am Samstag, 2. April 2016, 17:56:17 schrieb Shawn Lin: > >> ? 2016/4/2 7:52, Heiko Stuebner ??: > >> > it looks like commit ea9849113343 ("spi: rockchip: check return value > >> > of > >> > dmaengine_prep_slave_sg") negatively affects the cros_ec spi backend. > >> > > >> > During boot on the most recent mainline kernel I get: > >> > > >> > [ 1.025480] cros-ec-spi spi0.0: Chrome EC device registered > >> > [ 1.641636] input: cros_ec as > >> > /devices/platform/ff110000.spi/spi_master/spi0/spi0.0/ff110000.spi:ec@0 > >> > > >> > :keyboard-controller/input/input0 [ 2.340214] cros-ec-spi spi0.0: EC > >> > > >> > failed to respond in time > >> > [ 2.357735] cros-ec-spi spi0.0: packet too long (249 bytes, expected > >> > 4) [ 2.470353] cros-ec-spi spi0.0: EC failed to respond in time > >> > [ 2.508495] cros-ec-spi spi0.0: packet too long (249 bytes, expected > >> > 4) [ 2.620176] cros-ec-spi spi0.0: EC failed to respond in time > >> > [ 2.637345] cros-ec-spi spi0.0: packet too long (249 bytes, expected > >> > 4) [ 2.750245] cros-ec-spi spi0.0: EC failed to respond in time > >> > [ 2.767519] cros-ec-spi spi0.0: packet too long (249 bytes, expected > >> > 4) > >> > > >> > The cros-ec works ok after boot without further errors [aka keyboard > >> > and everything working correctly] and I haven't been able to figure out > >> > what goes wrong, but was able to bisect the issue down to the commit > >> > mentioned above. > >> > >> Which Soc I can reproduce it? > > > > I can see that on both a veyron-pinky as well as a veyron-jerry, so the > > rk3288-based devices. > > > >> I'm not able to find out how this commit to break the cros-ec. So I > >> prone to think this commit just expose some issues rather than > >> introducing negatively affects. I guess, before this commit, cros-ec > >> always get successful transfer, but the reality is that the tranfer > >> does failed in the early booting stage but spi-rockchip doesn't log out > >> anything. If that is the case, that means spi-rockchip fails to prepare > >> sg for some unknown reasons? > > > > That is a possibility. > > I also agree with this theory. AFAICT the mentioned commit is only > adding some checks so it seems the problem is with on the spi-rockchip > driver. > > > I haven't had much experience with both spi and cros-ec and it seems I've > > forgotten to also include Javier in my Cc-list who die the cros-ec > > mainlining. I've corrected that now and maybe he has some additional idea > > what may go wrong there. > > Unfortunately I'm neither familiar with Rockchip platforms nor have > access to Rockchip based Chromebooks to reproduce this issue. But I > don't see this error on my Exynos based Chromebooks so that's another > sign that the issue may be in the spi-rockchip driver. > > I've added to the cc list to Enric and Tomeu who were working on the > cros_ec drivers lately and AFAIK have access to some Rockchip > Chromebooks. I dug a bit further now, and it really seems to be the rockchip-spi (or more importantly the dma-part) at fault. If I just set the use_dma always to 0 and thus force pio mode, everything works fine. With dma transfers enabled it seems to bunch commands together in some way. For packets that are reported as the controller not responding, it seems like they were already transfered with the previous to long packet in cros_ec_cmd_xfer_spi [0]: [ 4.928480] cros-ec-spi spi0.0: packet too long (249 bytes, expected 4) [ 4.935104] cros-ec-spi spi0.0: bytes: [ 4.949560] 3 f9 0 0 4 0 0 0 4 0 0 0 0 0 0 0 ed ed ed ed ed ed ed ed ed ed ed ed ed ed ed ed 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 [ 5.010978] cros-ec-spi spi0.0: packet too long (249 bytes, expected 4) [ 5.017599] cros-ec-spi spi0.0: bytes: [ 5.022044] 3 f9 0 0 4 0 0 0 fa fa fa fa fa fa fa fa ec 3 f9 0 0 4 0 0 0 0 0 0 0 ed ed ed 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 [ 5.071057] cros_ec_spi_receive_packet: needing 124 bytes [ 5.312045] cros-ec-spi spi0.0: EC failed to respond in time [ 5.329564] cros-ec-spi spi0.0: packet too long (249 bytes, expected 4) [ 5.336197] cros-ec-spi spi0.0: bytes: [ 5.340651] 3 f9 0 0 4 0 0 0 fa fa fa fa fa fa ec 3 f9 0 0 4 0 0 0 0 0 0 0 ed ed ed ed ed 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 [ 5.389937] cros_ec_spi_receive_packet: needing 124 bytes [ 5.597082] cros-ec-spi spi0.0: EC failed to respond in time [ 5.603102] cros_ec_spi_receive_packet: needing 124 bytes [ 5.812077] cros-ec-spi spi0.0: EC failed to respond in time It also only affects the big packages, all the 10-20 byte long packages are transfered nicely it seems. I'll dig further, but if anybody has an idea where the dma-code makes a wrong turn I would be happy about pointers :-) [0] diff is: diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c index ebe9b94..24da42a 100644 --- a/drivers/mfd/cros_ec_spi.c +++ b/drivers/mfd/cros_ec_spi.c @@ -584,6 +589,10 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev, if (len > ec_msg->insize) { dev_err(ec_dev->dev, "packet too long (%d bytes, expected %d)", len, ec_msg->insize); +dev_err(ec_dev->dev, "bytes: "); +for (i = 0; i < len; i++) + printk("%x ", ptr[i]); +printk("\n"); ret = -ENOSPC; goto exit; }