service group: Combine return format as array #34
Conversation
src/srvgrp-clock.adoc
Outdated
| 3+| *Linear Format* | ||
| | *Word* | *Name* | *Description* | ||
| | 0 | min_Hz | lowest physical rate | ||
| | 1 | max_Hz | highest physical rate |
There was a problem hiding this comment.
Change to "Highest clock rate in Hz"
There was a problem hiding this comment.
Yes, we should remove metric from the field name and mention it either in description of the field or in the service group description, applicable for all service groups which deals with physical quantity
src/srvgrp-clock.adoc
Outdated
| | 1 | FLAGS | uint32 | _Reserved_ and must be `0`. | ||
| | 2 | REMAINING | uint32 | Remaining number of clock rates. (number of arrays) | ||
| | 3 | RETURNED | uint32 | Number of clock rates returned. (number of arrays) | ||
| | 4 | CLOCK_RATE[N] | struct | Clock rate. (Refer to <<table_clock_rate>>) |
There was a problem hiding this comment.
Introducing struct is going to create more confusion in representing words index. We should only use the primitive datatypes. Effectively its an array, though code can use struct.
There was a problem hiding this comment.
Everyplace where the struct will be will require that struct definition also with fields having primitive types.
There was a problem hiding this comment.
@pathakraul Joshua suggestion to use unint32[] is okay?
There was a problem hiding this comment.
uint32[2] also minor modification are required in Clock Rate Format section in this service group renaming tuple to array.
There was a problem hiding this comment.
how about the other service group that I had used 'struct' but it could be with different size?
There was a problem hiding this comment.
Lets go through them case by case. Which other cases are there apart from Voltage levels. I can see in spec that PERFORMANCE group also follows what we are thinking for Clock.
What other cases are there?
There was a problem hiding this comment.
performance have the same size for array, i guess its only voltage having different size
There was a problem hiding this comment.
Description in VOLT_GET_SUPPORTED_LEVELS is missing the case when the format is Multi Linear. Since in this format its possible that there are multiple ranges and any range may not fit in the DATA and overflow. What will happen in this case. Message must contain complete range and if a range is supposed to hit the DATA boundary then that range must be left for another call of this service.
| Each item in the triplet is a clock rate value. | ||
| CLOCK_RATE[0] = min_Hz (lowest physical rate that the clock can synthesize) | ||
| CLOCK_RATE[1] = max_Hz (highest physical rate that the clock can synthesize) | ||
| CLOCK_RATE[2] = step_Hz (Step between two successive rates) | ||
| ---- | ||
| ! [29:0] ! _Reserved_ |
There was a problem hiding this comment.
Check the point 4. in this #37. Please take care of these points highlighted in this issue in this PR and wherever possible.
|
@yeongjoshua Updates? |
Sorry, been working on some urgent task. Still working on it. |
Combine return format as single array. Signed-off-by: Joshua Yeong <[email protected]>
Update power state matching ACPI's device power management object definition. Signed-off-by: Joshua Yeong <[email protected]>
|
@pathakraul i remove the part where we mentioned that AP/Application Processor and PuC/Platform microcontroller can be use interchangeably. |
I also remove this in my PR #42: |
|
I dropped the last 2 patches as covered in #42 |
Combine return format as single array.