On Tue, Aug 16, 2016 at 4:56 AM, Rafael J. Wysocki <[email protected]> wrote: > On Monday, August 15, 2016 04:41:22 PM Hoan Tran wrote: >> Hi Rafael, >> >> On Mon, Aug 15, 2016 at 4:18 PM, Rafael J. Wysocki <[email protected]> >> wrote: >> > On Monday, August 15, 2016 09:45:24 AM Hoan Tran wrote: >> >> On Mon, Jun 27, 2016 at 2:32 PM, Rafael J. Wysocki <[email protected]> >> >> 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 >> >> > > <[email protected]> wrote: >> >> > > > >> >> > > > >> >> > > > On 6/9/2016 4:43 PM, Hoan Tran wrote: >> >> > > >> Hi Prashanth, >> >> > > >> >> >> > > >> On Thu, Jun 9, 2016 at 3:25 PM, Prakash, Prashanth >> >> > > >> <[email protected]> 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 <[email protected]> wrote: >> >> > > >>>>> Hi Prashanth, >> >> > > >>>>> >> >> > > >>>>> >> >> > > >>>>> On Wed, Jun 8, 2016 at 5:32 PM, Prakash, Prashanth >> >> > > >>>>> <[email protected]> wrote: >> >> > > >>>>>> On 6/8/2016 10:24 AM, Hoan Tran wrote: >> >> > > >>>>>>> Hi Ashwin, >> >> > > >>>>>>> >> >> > > >>>>>>> On Wed, Jun 8, 2016 at 5:18 AM, Ashwin Chaugule >> >> > > >>>>>>> <[email protected]> wrote: >> >> > > >>>>>>>> + Prashanth (Can you please have a look as well?) >> >> > > >>>>>>>> >> >> > > >>>>>>>> On 31 May 2016 at 15:35, Hoan Tran <[email protected]> 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 <[email protected]> >> >> > > >> >> > > Do you have plan to apply this patch ? >> >> > >> >> > Yes. >> >> > >> >> > Thanks, >> >> > Rafael >> >> > >> >> >> >> Hi Rafael, >> >> >> >> This patch had an ACK from Prashanth. Can you consider to merge >> >> this patch please? >> > >> > Can you please resend it with the ACK? >> >> Correcting, it got a "Reviewed-by: Prashanth Prakash >> <[email protected]>". Is it OK? > > Yes, it is.
Already resend. Thanks Hoan > > Thanks, > Rafael >
