Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: scmi-msg: correct voltage domain protocol version & clock rates enumeration #6951

Merged
merged 2 commits into from
Aug 22, 2024

Conversation

etienne-lms
Copy link
Contributor

@etienne-lms etienne-lms commented Jul 18, 2024

Fix the version ID of the implemented SCMI voltage domain protocol that is v2.0 (ID 0x20000), not v3.0 (ID 0x30000).
(Fixes: 006d89b ("drivers: scmi-msg: add SCMI Voltage Domain protocol"))

Fix value of remaining clocks to describe on SCMI clock protocol message CLOCK_DESCRIBE_RATES that does not take into account the number of returned clock in the response.
(Fixes: 90252e2 ("drivers: scmi-msg: clock adapts to output buffer size"))

@etienne-lms etienne-lms changed the title drivers: scmi-msg: correct voltage domain protocol version drivers: scmi-msg: correct voltage domain protocol version & clock rates enumeration Jul 18, 2024
Copy link
Contributor

@jforissier jforissier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For "drivers: scmi-msg: correct voltage domain protocol version":
Acked-by: Jerome Forissier <[email protected]>

@@ -321,7 +321,7 @@ static void scmi_clock_describe_rates(struct scmi_msg *msg)
}

out_count = rate_index - in_args->rate_index;
remaining = nb_rates - in_args->rate_index;
remaining = nb_rates - in_args->rate_index - out_count;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nb_rates - rate_index would be better, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have said nb_rates - out_count as out_count is the number of entries filled in the buffer. Maybe I'm missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nb_rates - rate_index would be better, no?

Indeed simpler.

I would have said nb_rates - out_count as out_count is the number of entries filled in the buffer. Maybe I'm missing something.

nb_rates gives the overall number of supported rates. out_count gives the number of rates filled in this very response message but does not count the rates skipped, from index 0 to index in_args->rate_index - 1.

Copy link
Contributor Author

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update the change.

@@ -321,7 +321,7 @@ static void scmi_clock_describe_rates(struct scmi_msg *msg)
}

out_count = rate_index - in_args->rate_index;
remaining = nb_rates - in_args->rate_index;
remaining = nb_rates - in_args->rate_index - out_count;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nb_rates - rate_index would be better, no?

Indeed simpler.

I would have said nb_rates - out_count as out_count is the number of entries filled in the buffer. Maybe I'm missing something.

nb_rates gives the overall number of supported rates. out_count gives the number of rates filled in this very response message but does not count the rates skipped, from index 0 to index in_args->rate_index - 1.

@jforissier
Copy link
Contributor

Acked-by: Jerome Forissier <[email protected]>

@etienne-lms
Copy link
Contributor Author

Fixup commit squashed and review tag applied.
@GseoC, do you want to provide a review tag?

pkcs11_1007 test sporadic failure faced during Ci test for this P-R but it's unrelated to this change (I still don't understand exactly what happens there, still tracking through #6952).

@GseoC
Copy link
Contributor

GseoC commented Aug 21, 2024

Sure, Acked-by: Gatien Chevallier <[email protected]>

Fix the version ID of the implemented SCMI voltage domain protocol that
is v2.0 (ID 0x20000), not v3.0 (ID 0x30000).

Fixes: 006d89b ("drivers: scmi-msg: add SCMI Voltage Domain protocol")
Signed-off-by: Etienne Carriere <[email protected]>
Acked-by: Jerome Forissier <[email protected]>
Acked-by: Gatien Chevallier <[email protected]>
Fix value of remaining clocks to describe on SCMI clock protocol
message CLOCK_DESCRIBE_RATES that does not take into account the
number of returned clock in the response.

Fixes: 90252e2 ("drivers: scmi-msg: clock adapts to output buffer size")
Signed-off-by: Etienne Carriere <[email protected]>
Acked-by: Jerome Forissier <[email protected]>
Acked-by: Gatien Chevallier <[email protected]>
@etienne-lms
Copy link
Contributor Author

Review tags applied. Thanks.

@jforissier jforissier merged commit 229670c into OP-TEE:master Aug 22, 2024
8 checks passed
@etienne-lms etienne-lms deleted the scmi branch August 28, 2024 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants