From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751689AbcGMWAL (ORCPT ); Wed, 13 Jul 2016 18:00:11 -0400 Received: from mail-vk0-f46.google.com ([209.85.213.46]:35348 "EHLO mail-vk0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751355AbcGMWAD (ORCPT ); Wed, 13 Jul 2016 18:00:03 -0400 MIME-Version: 1.0 In-Reply-To: <4302600.LGpaGycv0T@vostro.rjw.lan> References: <1463704347-22550-1-git-send-email-hotran@apm.com> <4302600.LGpaGycv0T@vostro.rjw.lan> From: Hoan Tran Date: Wed, 13 Jul 2016 15:00:01 -0700 Message-ID: Subject: Re: [PATCH v3] mailbox: pcc: Support HW-Reduced Communication Subspace type 2 To: "Rafael J. Wysocki" Cc: Jassi Brar , Ashwin Chaugule , Robert Moore , Alexey Klimov , lkml , linux acpi , Loc Ho , Duc Dang , "Prakash, Prashanth" 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 Rafael, On Mon, Jun 27, 2016 at 2:32 PM, Rafael J. Wysocki wrote: > On Monday, June 27, 2016 11:27:42 AM Hoan Tran wrote: >> Hi Jassi and Rafael, >> >> On Wed, Jun 15, 2016 at 9:19 AM, Prakash, Prashanth >> wrote: >> > >> > >> > On 6/9/2016 4:43 PM, Hoan Tran wrote: >> >> Hi Prashanth, >> >> >> >> On Thu, Jun 9, 2016 at 3:25 PM, Prakash, Prashanth >> >> wrote: >> >>> >> >>> On 6/9/2016 2:47 PM, Hoan Tran wrote: >> >>>> Hi Ashwin and Prashanth, >> >>>> >> >>>> On Wed, Jun 8, 2016 at 5:41 PM, Hoan Tran wrote: >> >>>>> 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. >> >>>> For platform notification, if the hardware doesn't support interlocked >> >>>> operations. I think we can use a workaround that, platform triggers >> >>>> interrupt to OSPM without touching status field. The OSPM PCC client >> >>>> will decide what to do with this interrupt. For example, OSPM sends a >> >>>> consumer command to check it. >> >>> How do we decide which platform can support this interlocked operation? >> >>> and how do we decide between a completion notification and platform >> >>> notification? >> >> Truly, we should follow the specification. But I don't know if there's >> >> any hardware support this interlocked operation. >> >> For the decide between a completion notification and platform notification >> >> - Completion notification: Bit "Command Complete" is set. >> >> - Platform notification: Bit "Command Complete" is not set. >> >> >> >>> I think the ACPI spec on platform notification is quite ambiguous and it is >> >>> best to get the necessary clarification and/or correction before implementing >> >>> anything related to platform notification. >> >> Agreed, a clarification inside ACPI Specification is needed >> > This patch look good to me, as it doesn't deal with platform notification. >> > We can try to get some clarification from spec side before handling the platform >> > notification pieces. >> > >> > Reviewed-by: Prashanth Prakash >> >> Do you have plan to apply this patch ? > > Yes. Thanks and hope it'll be in 4.8. Thanks Hoan > > Thanks, > Rafael >