riscv-non-isa / riscv-rpmi

RISC-V Platform Management Interface Specification. OS-agnostic messaging interface for system management and control
https://jira.riscv.org/browse/RVG-151
Creative Commons Attribution 4.0 International
8 stars 8 forks source link

Synchronization in format across specification #37

Closed pathakraul closed 1 month ago

pathakraul commented 3 months ago

Creating this issue to list the items which should have same format across the specification. Lets keep updating this list here as we find them.

  1. Use of application processor instead of AP. Use platform microcontroller instead of PuC.

  2. Every service heading should have (service_id: 0xXY) appended. eg. Service: SYSRST_GET_ATTRIBUTES (service_id: 0x02).

  3. Array display format.

  4. Bit fields must occupy defined bits from LSB side and move towards MSB. Rest of the undefined Bits must be marked as RESERVED and must be 0.

~5. Use the *_NOT_FOUND error code for request parameter which directly represents resource for that service group and for rest of the parameters *_INVAL.~

  1. Use the *INVALID_PARAM for all parameters passed to a service if invalid or not supported. At some places if we have a specific defined error then we can use that, for example invalid address has *INVALID_ADDR

For example in Clock service group(get attributes service) - clock_id if not valid return RPMI_ERR_NOT_FOUND and for rest of the parameters like CLOCK_RATE_INDEX in this service should be RPMI_ERR_INVAL. similarly, in case of CPPC service group (probe register service), REG_ID must have RPMI_ERR_NOT_FOUND and for HART_ID it should be RPMI_ERR_INVAL

Since we have sufficient error codes and we can use for distinct error conditions

pathakraul commented 3 months ago

@yeongjoshua

lftan commented 3 months ago

For # 1, also use "application processor" instead of "Application Processor" (no capital letter in the first letter), same for "Platform Microcontroller".

I can take this change and # 2 change

pathakraul commented 3 months ago

For # 1, also use "application processor" instead of "Application Processor" (no capital letter in the first letter), same for "Platform Microcontroller".

I can take this change and # 2 change

Leave the Base, System Reset and Suspend, since i am preparing PRs locally and have made changes for these groups

lftan commented 3 months ago

For # 1, also use "application processor" instead of "Application Processor" (no capital letter in the first letter), same for "Platform Microcontroller". I can take this change and # 2 change

Leave the Base, System Reset and Suspend, since i am preparing PRs locally and have made changes for these groups

Okay. You want make change # 1 and # 2 for these 3 service groups as well? Or I can change them after your changes get in.

pathakraul commented 3 months ago

For # 1, also use "application processor" instead of "Application Processor" (no capital letter in the first letter), same for "Platform Microcontroller". I can take this change and # 2 change

Leave the Base, System Reset and Suspend, since i am preparing PRs locally and have made changes for these groups

Okay. You want make change # 1 and # 2 for these 3 service groups as well? Or I can change them after your changes get in.

Yeah, leave them completely. All points I have taken care already

pathakraul commented 3 months ago

@lftan @@yeongjoshua Updated another case for error code above in the description. Let know your thoughts

lftan commented 3 months ago

@lftan @@yeongjoshua Updated another case for error code above in the description. Let know your thoughts

Okay. Please update

lftan commented 2 months ago

Item # 1 and # 2 fix in https://github.com/riscv-non-isa/riscv-rpmi/pull/42

lftan commented 2 months ago

For "Return Status Code", do you think we want change to "Return status code". Remove unnecessary capital letters. @pathakraul

lftan commented 2 months ago

In the description of the table for each parameter, some with full stop ".", and some do not. Suggest adding full stop "." at the end of the sentence for proper grammar.

yeongjoshua commented 2 months ago

@pathakraul Does the picture need to use 'Application Processor' and 'Platform Microcontroller' instead?