From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933269AbcFIAlZ (ORCPT ); Wed, 8 Jun 2016 20:41:25 -0400 Received: from mail-vk0-f42.google.com ([209.85.213.42]:35891 "EHLO mail-vk0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932960AbcFIAlW (ORCPT ); Wed, 8 Jun 2016 20:41:22 -0400 MIME-Version: 1.0 In-Reply-To: <0f07c6dc-76b7-ab28-541b-090f6a2432a6@codeaurora.org> References: <1463704347-22550-1-git-send-email-hotran@apm.com> <0f07c6dc-76b7-ab28-541b-090f6a2432a6@codeaurora.org> Date: Wed, 8 Jun 2016 17:41:20 -0700 Message-ID: Subject: Re: [PATCH v3] mailbox: pcc: Support HW-Reduced Communication Subspace type 2 From: Hoan Tran To: "Prakash, Prashanth" Cc: Ashwin Chaugule , Jassi Brar , "Rafael J. Wysocki" , Robert Moore , Alexey Klimov , lkml , linux acpi , Loc Ho , Duc Dang Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Prashanth, On Wed, Jun 8, 2016 at 5:32 PM, Prakash, Prashanth wrote: > > > On 6/8/2016 10:24 AM, Hoan Tran wrote: >> Hi Ashwin, >> >> On Wed, Jun 8, 2016 at 5:18 AM, Ashwin Chaugule >> wrote: >>> + Prashanth (Can you please have a look as well?) >>> >>> On 31 May 2016 at 15:35, Hoan Tran wrote: >>>> Hi Ashwin, >>> Hi, >>> >>> Sorry about the delay. I'm in the middle of switching jobs and >>> locations, so its been a bit crazy lately. >> It's ok and hope you're doing well. >> >>> I dont have any major >>> concerns with this code, although there could be subtle issues with >>> this IRQ thing. In this patchset, your intent is to add support for >>> PCC subspace type 2. But you're also adding support for tx command >>> completion which is not specific to Type 2. We could support that even >>> in Type 1. Hence I wanted to separate the two, not just for review, >>> but also the async IRQ completion has subtle issues esp. in the case >>> of async platform notification, where you could have a PCC client in >>> the OS writing to the cmd bit and the platform sending an async >>> notification by writing to some bits in the same 8byte address as the >>> cmd bit. So we need some mutual exclusivity there, otherwise the OS >>> and platform could step on each other. Perhaps Prashanth has better >>> insight into this. >> I think, this mutual exclusivity could be in another patch. > Ashwin, > Sorry, I am not sure how we can prevent platform and OSPM from stepping on > each other. There is a line is spec that says "all operations on status field > must be made using interlocked operations", but not sure what these > interlocked operation translates to. Yes, I had the same question about how to prevent it. > > Hoan, > Even if we are not using platform notification, we still need to clear the doorbell > interrupt bit in the PCC interrupt handler (Section14.2.2 and 14.4). I didn't see > clearing the doorbell interrupt bit in this patch (and platform is supposed to set > it again when it is sending the interrupt again). Did I miss it? or is it intentionally > left out to avoid the race that Ashwin mentioned above? > The PCC client driver is supposed to do that. Which mean, the mbox_chan_received_data() function should clear it. Thanks Hoan > > Thanks, > Prashanth > >