From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752220AbbGaJVc (ORCPT ); Fri, 31 Jul 2015 05:21:32 -0400 Received: from foss.arm.com ([217.140.101.70]:43808 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751855AbbGaJVa (ORCPT ); Fri, 31 Jul 2015 05:21:30 -0400 Message-ID: <55BB3E15.9030405@arm.com> Date: Fri, 31 Jul 2015 10:21:25 +0100 From: Sudeep Holla User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: Jassi Brar CC: Sudeep Holla , Linux Kernel Mailing List , Liviu Dudau , Punit Agrawal , "Jon Medhurst (Tixy)" , Lorenzo Pieralisi , Arnd Bergmann , Olof Johansson , Kevin Hilman Subject: Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol References: <1437649828-14540-1-git-send-email-sudeep.holla@arm.com> <1437649828-14540-3-git-send-email-sudeep.holla@arm.com> <55B89100.3020802@arm.com> <55B8CC23.5020306@arm.com> In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 30/07/15 18:56, Jassi Brar wrote: > On Wed, Jul 29, 2015 at 6:20 PM, Sudeep Holla wrote: >> On 29/07/15 12:19, Jassi Brar wrote: >> >>> Assuming the former, let me explain. When a client receives a >>> response, it can be sure that the request has already been read by the >>> remote. >> >> Waiting for the response would be too late for few expensive commands >> (e.g setting up external regulators). The remote firmware acknowledges >> Tx by setting status flags and will be ready to accept new commands. >> > No. Polling still happens. If anything, mbox_client_txdone() should > only speed up things. > Yes I understand and that's good. >>> If the protocol specifies every request has some response, the >> >> Not always true there can be few commands without response. The protocol >> specifies that we need check the status flag before sending the new >> command as it's bidirectional, hence polling is recommended (Section 2.2 >> Communication flow in the SCPI specification) >> > mbox_client_txdone() will only be called for commands that has some > response. Commands that don't have a response would be completed by > polling. > OK, got it >>> client should assert 'knows_txdone' and call mbox_client_txdone() upon >>> receiving a reply packet. >> >> Since this is not always true and not recommended in the specification, >> I am hesitant to use this option as the firmware can always change their >> internal mechanics without breaking the protocol. We need to ensure we are >> compliant to the spec. >> > I don't see how it could break compliance. > While I agree it shouldn't, the firmware guys won't support if we deviate from the spec. I won't get support for firmware bug fixes in that case. Having said that, I don't rule out the usage of TX_BY_ACK, I will need more time for testing(usually we stress test firmware using Linux for few of days continuously as we have hit issues after that :)) and getting things fixed if anything breaks. >>> So I said, cl->knows_txdone = false; is the root of problems you >> >> It could be and won't rule that out. I would prefer using knows_txdone >> and use mbox_client_txdone if feasible, but I can't as the without >> violating the specification. >> >> FYI, I had tried it and ended up with issues in the firmware. The >> argument from the firmware is that we aren't specification compliant, >> so I had to use polling. >> > I am sure you would have copy of that discarded code. Care to share? I > can't imagine how we handle completions locally could affect the > remote. The mbox_client_txdone() is untested so I don't rule out bugs, > otherwise it should only make things better. > I tested it with very old firmware almost 4-5 months back. I don't have the patch on top of this series handy, but will dig it out and give it a try with latest firmware. I will let you know the results. For now, I would keep this just polling and unblock others who are waiting on this series. Regards, Sudeep