20/01/2021 12:05, Burakov, Anatoly:
> On 20-Jan-21 10:38 AM, Thomas Monjalon wrote:
> > 20/01/2021 11:32, Burakov, Anatoly:
> >> On 19-Jan-21 2:17 PM, Thomas Monjalon wrote:
> >>> 19/01/2021 12:23, Burakov, Anatoly:
> >>>> On 19-Jan-21 10:42 AM, Thomas Monjalon wrote:
> >>>>> 19/01/2021 11:29, Burakov, Anatoly:
> >>>>>> On 18-Jan-21 10:26 PM, Thomas Monjalon wrote:
> >>>>>>> 14/01/2021 15:46, Anatoly Burakov:
> >>>>>>>> +struct rte_power_monitor_cond {
> >>>>>>>> +    volatile void *addr;  /**< Address to monitor for changes */
> >>>>>>>> +    uint64_t val;         /**< Before attempting the monitoring, 
> >>>>>>>> the address
> >>>>>>>> +                           *   may be read and compared against 
> >>>>>>>> this value.
> >>>>>>>
> >>>>>>> "may" be read and compared?
> >>>>>>> Is there a case where there is no read and compare?
> >>>>>>
> >>>>>> Yes, if the mask is not set.
> >>>>>
> >>>>> If the mask is not set, the address is "read" anyway
> >>>>> or it is only "watched" for any change?
> >>>>>
> >>>>> Sorry the mechanism is really not clear to me.
> >>>>>
> >>>>
> >>>> The "value" is only used to avoid the sleep, i.e. to check if the write
> >>>> has already happened. We're waiting on *a write* rather than *a value*,
> >>>> so it's not equivalent to "wait until equal" call. It's more of a "sleep
> >>>> until something happens".
> >>>
> >>> Please make things explicit in doxygen.
> >>> The behaviour of each case should be explained crystal clear.
> >>> Thanks
> >>
> >> It is explained in the comments to `rte_power_monitor()` call. But OK,
> >> i'll add more clarification for the struct too.
> > 
> > Please avoid the word "may" in API description.
> > 
> > This is what is explained in rte_power_monitor:
> > "
> >   * Additionally, an `expected` 64-bit value and 64-bit mask are provided. 
> > If
> >   * mask is non-zero, the current value pointed to by the `p` pointer will 
> > be
> >   * checked against the expected value, and if they match, the entering of
> >   * optimized power state may be aborted.
> > "
> > 
> > Can we replace "may" by "will"?
> > 
> 
> Yep, we can. However, the "may" part was intended to leave some wiggle 
> room for a different implementation, should the need arise, and i find 
> "will" to be needlessly prescriptive. Frankly, i do not see the need for 
> such a detailed description of what the API does under the hood, as long 
> as it's clear what its effects are. The main purpose is waiting for a 
> write. The mask is only used to check whether the expected write has 
> already happened by the time we're calling the API. Whether the CPU then 
> does or does not go to sleep is not really relevant IMO.

I think it is relevant but I may be wrong.
Any other opinions?


Reply via email to