Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1462995 > unrolled thread

Re: [PATCH v3] mailbox: pcc: Support HW-Reduced Communication Subspace type 2

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3] mailbox: pcc: Support HW-Reduced Communication  Subspace type 2 Hoan Tran <hotran@apm.com> - 2016-08-15 19:00 +0200
    Re: [PATCH v3] mailbox: pcc: Support HW-Reduced Communication Subspace type 2 "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-08-16 01:20 +0200
      Re: [PATCH v3] mailbox: pcc: Support HW-Reduced Communication  Subspace type 2 Hoan Tran <hotran@apm.com> - 2016-08-16 01:50 +0200
        Re: [PATCH v3] mailbox: pcc: Support HW-Reduced Communication Subspace type 2 "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-08-16 14:00 +0200
          Re: [PATCH v3] mailbox: pcc: Support HW-Reduced Communication  Subspace type 2 Hoan Tran <hotran@apm.com> - 2016-08-16 18:30 +0200

#1462995 — Re: [PATCH v3] mailbox: pcc: Support HW-Reduced Communication Subspace type 2

On Mon, Jun 27, 2016 at 2:32 PM, Rafael J. Wysocki <rjw@rjwysocki.net> 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
> > <pprakash@codeaurora.org> wrote:
> > >
> > >
> > > On 6/9/2016 4:43 PM, Hoan Tran wrote:
> > >> Hi Prashanth,
> > >>
> > >> On Thu, Jun 9, 2016 at 3:25 PM, Prakash, Prashanth
> > >> <pprakash@codeaurora.org> 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 <hotran@apm.com> wrote:
> > >>>>> Hi Prashanth,
> > >>>>>
> > >>>>>
> > >>>>> On Wed, Jun 8, 2016 at 5:32 PM, Prakash, Prashanth
> > >>>>> <pprakash@codeaurora.org> wrote:
> > >>>>>> On 6/8/2016 10:24 AM, Hoan Tran wrote:
> > >>>>>>> Hi Ashwin,
> > >>>>>>>
> > >>>>>>> On Wed, Jun 8, 2016 at 5:18 AM, Ashwin Chaugule
> > >>>>>>> <ashwin.chaugule@linaro.org> wrote:
> > >>>>>>>> + Prashanth (Can you please have a look as well?)
> > >>>>>>>>
> > >>>>>>>> On 31 May 2016 at 15:35, Hoan Tran <hotran@apm.com> 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 <pprakash@codeaurora.org>
> >
> > 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?

Thanks
Hoan

[toc] | [next] | [standalone]

#1463262 — Re: [PATCH v3] mailbox: pcc: Support HW-Reduced Communication Subspace type 2

On Monday, August 15, 2016 09:45:24 AM Hoan Tran wrote:
> On Mon, Jun 27, 2016 at 2:32 PM, Rafael J. Wysocki <rjw@rjwysocki.net> 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
> > > <pprakash@codeaurora.org> wrote:
> > > >
> > > >
> > > > On 6/9/2016 4:43 PM, Hoan Tran wrote:
> > > >> Hi Prashanth,
> > > >>
> > > >> On Thu, Jun 9, 2016 at 3:25 PM, Prakash, Prashanth
> > > >> <pprakash@codeaurora.org> 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 <hotran@apm.com> wrote:
> > > >>>>> Hi Prashanth,
> > > >>>>>
> > > >>>>>
> > > >>>>> On Wed, Jun 8, 2016 at 5:32 PM, Prakash, Prashanth
> > > >>>>> <pprakash@codeaurora.org> wrote:
> > > >>>>>> On 6/8/2016 10:24 AM, Hoan Tran wrote:
> > > >>>>>>> Hi Ashwin,
> > > >>>>>>>
> > > >>>>>>> On Wed, Jun 8, 2016 at 5:18 AM, Ashwin Chaugule
> > > >>>>>>> <ashwin.chaugule@linaro.org> wrote:
> > > >>>>>>>> + Prashanth (Can you please have a look as well?)
> > > >>>>>>>>
> > > >>>>>>>> On 31 May 2016 at 15:35, Hoan Tran <hotran@apm.com> 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 <pprakash@codeaurora.org>
> > >
> > > 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?

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1463283

Hi Rafael,

On Mon, Aug 15, 2016 at 4:18 PM, Rafael J. Wysocki <rjw@rjwysocki.net> 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 <rjw@rjwysocki.net> 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
>> > > <pprakash@codeaurora.org> wrote:
>> > > >
>> > > >
>> > > > On 6/9/2016 4:43 PM, Hoan Tran wrote:
>> > > >> Hi Prashanth,
>> > > >>
>> > > >> On Thu, Jun 9, 2016 at 3:25 PM, Prakash, Prashanth
>> > > >> <pprakash@codeaurora.org> 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 <hotran@apm.com> wrote:
>> > > >>>>> Hi Prashanth,
>> > > >>>>>
>> > > >>>>>
>> > > >>>>> On Wed, Jun 8, 2016 at 5:32 PM, Prakash, Prashanth
>> > > >>>>> <pprakash@codeaurora.org> wrote:
>> > > >>>>>> On 6/8/2016 10:24 AM, Hoan Tran wrote:
>> > > >>>>>>> Hi Ashwin,
>> > > >>>>>>>
>> > > >>>>>>> On Wed, Jun 8, 2016 at 5:18 AM, Ashwin Chaugule
>> > > >>>>>>> <ashwin.chaugule@linaro.org> wrote:
>> > > >>>>>>>> + Prashanth (Can you please have a look as well?)
>> > > >>>>>>>>
>> > > >>>>>>>> On 31 May 2016 at 15:35, Hoan Tran <hotran@apm.com> 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 <pprakash@codeaurora.org>
>> > >
>> > > 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
<pprakash@codeaurora.org>". Is it OK?

Thanks
Hoan

>
> Thanks,
> Rafael
>

[toc] | [prev] | [next] | [standalone]

#1463735 — Re: [PATCH v3] mailbox: pcc: Support HW-Reduced Communication Subspace type 2

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 <rjw@rjwysocki.net> 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 <rjw@rjwysocki.net> 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
> >> > > <pprakash@codeaurora.org> wrote:
> >> > > >
> >> > > >
> >> > > > On 6/9/2016 4:43 PM, Hoan Tran wrote:
> >> > > >> Hi Prashanth,
> >> > > >>
> >> > > >> On Thu, Jun 9, 2016 at 3:25 PM, Prakash, Prashanth
> >> > > >> <pprakash@codeaurora.org> 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 <hotran@apm.com> wrote:
> >> > > >>>>> Hi Prashanth,
> >> > > >>>>>
> >> > > >>>>>
> >> > > >>>>> On Wed, Jun 8, 2016 at 5:32 PM, Prakash, Prashanth
> >> > > >>>>> <pprakash@codeaurora.org> wrote:
> >> > > >>>>>> On 6/8/2016 10:24 AM, Hoan Tran wrote:
> >> > > >>>>>>> Hi Ashwin,
> >> > > >>>>>>>
> >> > > >>>>>>> On Wed, Jun 8, 2016 at 5:18 AM, Ashwin Chaugule
> >> > > >>>>>>> <ashwin.chaugule@linaro.org> wrote:
> >> > > >>>>>>>> + Prashanth (Can you please have a look as well?)
> >> > > >>>>>>>>
> >> > > >>>>>>>> On 31 May 2016 at 15:35, Hoan Tran <hotran@apm.com> 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 <pprakash@codeaurora.org>
> >> > >
> >> > > 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
> <pprakash@codeaurora.org>". Is it OK?

Yes, it is.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1463930

On Tue, Aug 16, 2016 at 4:56 AM, Rafael J. Wysocki <rjw@rjwysocki.net> 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 <rjw@rjwysocki.net> 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 <rjw@rjwysocki.net> 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
>> >> > > <pprakash@codeaurora.org> wrote:
>> >> > > >
>> >> > > >
>> >> > > > On 6/9/2016 4:43 PM, Hoan Tran wrote:
>> >> > > >> Hi Prashanth,
>> >> > > >>
>> >> > > >> On Thu, Jun 9, 2016 at 3:25 PM, Prakash, Prashanth
>> >> > > >> <pprakash@codeaurora.org> 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 <hotran@apm.com> wrote:
>> >> > > >>>>> Hi Prashanth,
>> >> > > >>>>>
>> >> > > >>>>>
>> >> > > >>>>> On Wed, Jun 8, 2016 at 5:32 PM, Prakash, Prashanth
>> >> > > >>>>> <pprakash@codeaurora.org> wrote:
>> >> > > >>>>>> On 6/8/2016 10:24 AM, Hoan Tran wrote:
>> >> > > >>>>>>> Hi Ashwin,
>> >> > > >>>>>>>
>> >> > > >>>>>>> On Wed, Jun 8, 2016 at 5:18 AM, Ashwin Chaugule
>> >> > > >>>>>>> <ashwin.chaugule@linaro.org> wrote:
>> >> > > >>>>>>>> + Prashanth (Can you please have a look as well?)
>> >> > > >>>>>>>>
>> >> > > >>>>>>>> On 31 May 2016 at 15:35, Hoan Tran <hotran@apm.com> 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 <pprakash@codeaurora.org>
>> >> > >
>> >> > > 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
>> <pprakash@codeaurora.org>". Is it OK?
>
> Yes, it is.

Already resend.

Thanks
Hoan

>
> Thanks,
> Rafael
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web