Update 2 #25
Merged
Update 2 #25
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
lftan
commented
Apr 23, 2024
lftan
commented
- Update description and fix grammar in some service group chapters.
- Refer to SBI spec for System Reset Types and System Suspend Types.
pathakraul
reviewed
Apr 24, 2024
pathakraul
reviewed
Apr 24, 2024
src/srvgrp-system-suspend.adoc
Outdated
| Platform can optionally support notifications of events which might occur in the platform. PuC can send these notification messages to AP if they are implemented | ||
| and AP has subscribed to these. Events supported are described above in System | ||
| Suspend Notifications. | ||
| This service allows the AP to subscribe to System Suspend service group |
There was a problem hiding this comment.
Can we use single convention for names? example: System Suspend or system suspend. This applies to such service-groups/services names in the whole spec. Either is fine for me
There was a problem hiding this comment.
Will change to "System Suspend", since it is a special term in the spec.
There was a problem hiding this comment.
Maybe we should use SYSTEM_SUSPEND instead of "System Suspend" when used for the service group name.
Same for other service groups.
There was a problem hiding this comment.
But at places in text we are referring the name as functionality in general and not the service group name
There was a problem hiding this comment.
Lets go with anything but single convention, we can deal with this later
pathakraul
reviewed
Apr 24, 2024
src/srvgrp-system-suspend.adoc
Outdated
| executes WFI instruction. | ||
| type. It requires that all harts except the calling hart to be in the STOPPED | ||
| state as defined by the Hart State Management service group. The system | ||
| transitions into the low power state only after the requesting HART executes |
There was a problem hiding this comment.
Should we write HART in smallcase everywhere?
added 6 commits
April 24, 2024 12:05
- Fix grammar - Fine tune description - 80 character per line Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
Refer to Suspend Types definition in SBI spec for SUSPEND_TYPE. Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
…service The returned Attributes flags (bit-30) already indicates the SUSPEND_TYPE is support or not. Remove RPMI_ERROR_NOT_SUPPORTED from return code. Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
- Fix grammar - Fine tune description - 80 characters per line Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
Refer to Reset Types definition in SBI spec for RESET_TYPE. Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
So that RPMI_ERROR_NOT_SUPPORTED can display in one line. Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
pathakraul
reviewed
Apr 24, 2024
src/srvgrp-base.adoc
Outdated
|
|
||
| * Initial handshaking between the Application processor and Platform | ||
| micro-controller. | ||
| * Initial handshaking between the Application processor and the Platform |
There was a problem hiding this comment.
Application Processor and Platform Microcontroller everywhere ?
pathakraul
reviewed
Apr 24, 2024
src/srvgrp-base.adoc
Outdated
| @@ -26,23 +29,25 @@ Following table lists the service group | |||
| | 0x80000 - 0xFFFFF | _Implementation Specific Service Groups_ | |||
| |=== | |||
| NOTE: The services listed in each service group do not follow any sequence and | |||
| are not defined in any specific order. It's possible that any particular service | |||
| in any service group inherently requires to be called first before other defined services before that. | |||
| are not defined in any specific order. It's possible that a particular service | |||
There was a problem hiding this comment.
I think we should reframe this to mention that services defined in the sequence in spec does not represent the calling order in the implementation
pathakraul
reviewed
Apr 24, 2024
added 7 commits
April 25, 2024 14:28
- Fix grammar - Fine tune description - 80 characters per line Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
Change "Micro-controller" to "Microcontroller". Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
We use "AP" in the document now, change it. Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
Uses same term in whole document. Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
Since prefix already has BASE, rename it. Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
Other service groups don't have this, remove this. Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.