From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756309AbdJJNjF (ORCPT ); Tue, 10 Oct 2017 09:39:05 -0400 Received: from hqemgate16.nvidia.com ([216.228.121.65]:4952 "EHLO hqemgate16.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756278AbdJJNjC (ORCPT ); Tue, 10 Oct 2017 09:39:02 -0400 X-PGP-Universal: processed; by hqpgpgate101.nvidia.com on Tue, 10 Oct 2017 06:38:31 -0700 Subject: Re: [PATCH v3] platform/chrome: Use proper protocol transfer function To: Shawn N CC: Olof Johansson , Benson Leung , "Lee Jones" , , Doug Anderson , Brian Norris , "Brian Norris" , Gwendal Grignou , Enric Balletbo , Tomeu Vizoso , "linux-tegra@vger.kernel.org" References: <20170908205011.77986-1-briannorris@chromium.org> <02aa65e7-e967-055b-2af3-2e9b6ef77935@nvidia.com> <20170919171401.GA10968@google.com> <20170920061317.GB13616@google.com> From: Jon Hunter Message-ID: <6beb7b8c-f602-112b-73b2-22188dcea28e@nvidia.com> Date: Tue, 10 Oct 2017 14:35:55 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: X-Originating-IP: [10.21.132.144] X-ClientProxiedBy: UKMAIL101.nvidia.com (10.26.138.13) To UKMAIL101.nvidia.com (10.26.138.13) Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 26/09/17 00:15, Shawn N wrote: > On Wed, Sep 20, 2017 at 1:22 PM, Shawn N wrote: >> On Tue, Sep 19, 2017 at 11:13 PM, Brian Norris wrote: >>> Hi, >>> >>> On Tue, Sep 19, 2017 at 11:05:38PM -0700, Shawn N wrote: >>>> This is failing because our EC_CMD_GET_PROTOCOL_INFO host command is >>>> getting messed up, or the reply buffer is getting corrupted somehow. >>>> >>>> ec_dev->proto_version = >>>> min(EC_HOST_REQUEST_VERSION, >>>> fls(proto_info->protocol_versions) - 1); >>>> >> >> Checking this closer, the first host command we send after we boot the >> kernel (EC_CMD_GET_PROTOCOL_INFO) is failing due to protocol error >> (see 'SPI rx bad data' / 'SPI not ready' on the EC console). Since >> this doesn't seem to happen on the Chromium OS nyan_big release >> kernel, I suggest to hook up a logic analyzer and see if the SPI >> master is doing something bad. >> >> The error handling in cros_ec_cmd_xfer_spi() is completely wrong and >> we return -EAGAIN / EC_RES_IN_PROGRESS, which the caller interprets >> "the host command was received by the EC and is currently being >> handled, poll status until completion". So the caller polls status >> with EC_CMD_GET_COMMS_STATUS, sees no host command is in progress >> (which is interpreted to mean "the host command I sent previously has >> now successfully completed"), and returns success. The problem here is >> that the initial host command was never received at all, and no reply >> was ever received, so our reply data is all zero. >> >> Two things need to be fixed here: >> >> 1) Find out why the first host command after boot is failing. Probe >> SPI pins and see what's going on. >> 2) Fix error handling so we properly return an error (or properly >> retry the entire command) when a protocol error occurs (I made some >> attempt in https://chromium-review.googlesource.com/385080/, probably >> I should revisit that). > > The below patch will fix error handling and will make things mostly > work on nyan_big, because we'll fall back to V2 protocol after the > initial failure. But we should still investigate why we're getting > errors on the first host command. We aren't seeing these errors when > we send commands from firmware, so I suspect something is wrong in > kernel SPI HW initialization that causes the first command to fail. I have been looking into this a bit more and it appears to be timing related. I found that enabling some debug in the Tegra SPI driver the problem would go away and seems that adding a delay before sending the SPI message would also workaround the problem. Looking back at the Tegra Linux test history [0], it appears that after v4.10-rc5 [1] I start to see the following message which is the first indication of some SPI issues ... cros-ec-spi spi32766.0: packet too long (249 bytes, expected 4) I attempted to bisect this, but I was not successful because each time I would ended up somewhere different. I found that even with v4.9 I may see the issue 1 in 20 times and so I realised that I am not even sure when the problem really started or if has always been there. It seems that for older kernels it is harder to reproduce. I am wondering now if some timing has changed somewhere causing us to see the problem more frequently with newer kernels? I see the the cros-ec binding defines the following ... "google,cros-ec-spi-pre-delay: Some implementations of the EC need a little time to wake up from sleep before they can receive SPI transfers at a high clock rate. This property specifies the delay, in usecs, between the assertion of the CS to the start of the first clock pulse." I found that adding the following also worked around the problem ... diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi b/arch/arm/boot/dts/tegra124-nyan.dtsi index 5cf987b5401e..0baa6bfc0f36 100644 --- a/arch/arm/boot/dts/tegra124-nyan.dtsi +++ b/arch/arm/boot/dts/tegra124-nyan.dtsi @@ -317,6 +317,7 @@ interrupts = ; reg = <0>; + google,cros-ec-spi-pre-delay = <10>; google,cros-ec-spi-msg-delay = <2000>; i2c-tunnel { I have tried 50 boots with the above and I have seen no SPI failures on boot. I did look to see if it is possible to probe the SPI signals with a scope but from the schematics I am not sure if they are accessible or buried in the PCB. Is it possible that Tegra is sending the SPI message too soon for the EC? Cheers Jon [0] https://nvtb.github.io/linux/ [1] https://nvtb.github.io/linux/test_v4.10-rc5/20170123023102/boot/tegra124-nyan-big/tegra124-nyan-big/tegra_defconfig_log.txt -- nvpublic