Nouveau - Aug 2014 - [PATCH envytools] nvamemtiming: Handle target < initial case when iterating values

On 31/08/2014 16:19, Christian Costa wrote:
> Le 31/08/2014 16:11, Martin Peres a ?crit :
>> On 31/08/2014 16:01, Christian Costa wrote:
>>> Le 31/08/2014 15:12, Martin Peres a ?crit :
>>>> On 31/08/2014 15:00, Christian Costa wrote:
>>>>> Otherwise some values are not tested at all.
>>>> I would rather have a warning than the program doing stuff
behind
>>>> my back. This is a dev tool, dumb == good ;)
>>>>
>>> But why didn't you call it Dumb mode rather that Deep mode.
That
>>> way, I wouldn't have tried to use it and figure out what it
does. ;p
>>
>> Target is supposed to be higher than initial! Initial == minimum safe 
>> timing.
>>
>> You just reversed the order, try:
>> sudo nvamemtiming pramin 0x6a22 5 0 -d 3
>>
> Ok. It's intended then. Thanks!
I totally agree that this tool is a little confusing. If you have 
suggestions on how to improve the usage doc, feel free to propose me 
something better :)

fprintf(stderr, "\t-d timing_entry_high: For each indexes, iterate 
between the timing_entry and the timing_entry_high value (Deep mode)\n");

Speaking about this, this should be corrected to "For each index", no 
plural after each. I'll fix it right now.

Le 31/08/2014 16:23, Martin Peres a ?crit :
> On 31/08/2014 16:19, Christian Costa wrote:
>> Le 31/08/2014 16:11, Martin Peres a ?crit :
>>> On 31/08/2014 16:01, Christian Costa wrote:
>>>> Le 31/08/2014 15:12, Martin Peres a ?crit :
>>>>> On 31/08/2014 15:00, Christian Costa wrote:
>>>>>> Otherwise some values are not tested at all.
>>>>> I would rather have a warning than the program doing stuff
behind
>>>>> my back. This is a dev tool, dumb == good ;)
>>>>>
>>>> But why didn't you call it Dumb mode rather that Deep mode.
That
>>>> way, I wouldn't have tried to use it and figure out what it
does. ;p
>>>
>>> Target is supposed to be higher than initial! Initial == minimum 
>>> safe timing.
>>>
>>> You just reversed the order, try:
>>> sudo nvamemtiming pramin 0x6a22 5 0 -d 3
>>>
>> Ok. It's intended then. Thanks!
>
> I totally agree that this tool is a little confusing. If you have 
> suggestions on how to improve the usage doc, feel free to propose me 
> something better :)
>
> fprintf(stderr, "\t-d timing_entry_high: For each indexes, iterate 
> between the timing_entry and the timing_entry_high value (Deep
mode)\n");
>
> Speaking about this, this should be corrected to "For each
index", no
> plural after each. I'll fix it right now.
>
Ok. I will think about it.
Some comments in the source code can help also to know what it does and 
what it is used for. Same for README.

Le 31/08/2014 17:18, Christian Costa a ?crit :
> Le 31/08/2014 16:23, Martin Peres a ?crit :
>> On 31/08/2014 16:19, Christian Costa wrote:
>>> Le 31/08/2014 16:11, Martin Peres a ?crit :
>>>> On 31/08/2014 16:01, Christian Costa wrote:
>>>>> Le 31/08/2014 15:12, Martin Peres a ?crit :
>>>>>> On 31/08/2014 15:00, Christian Costa wrote:
>>>>>>> Otherwise some values are not tested at all.
>>>>>> I would rather have a warning than the program doing
stuff behind
>>>>>> my back. This is a dev tool, dumb == good ;)
>>>>>>
>>>>> But why didn't you call it Dumb mode rather that Deep
mode. That
>>>>> way, I wouldn't have tried to use it and figure out
what it does. ;p
>>>>
>>>> Target is supposed to be higher than initial! Initial ==
minimum
>>>> safe timing.
>>>>
>>>> You just reversed the order, try:
>>>> sudo nvamemtiming pramin 0x6a22 5 0 -d 3
>>>>
>>> Ok. It's intended then. Thanks!
>>
>> I totally agree that this tool is a little confusing. If you have 
>> suggestions on how to improve the usage doc, feel free to propose me 
>> something better :)
>>
>> fprintf(stderr, "\t-d timing_entry_high: For each indexes, iterate
>> between the timing_entry and the timing_entry_high value (Deep 
>> mode)\n");
>>
>> Speaking about this, this should be corrected to "For each
index", no
>> plural after each. I'll fix it right now.
>>
> Ok. I will think about it.
> Some comments in the source code can help also to know what it does 
> and what it is used for. Same for README.
Btw, the plural of index is indices.