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

arm: pl310: fix cache sync #2035

Merged
merged 1 commit into from
Jan 13, 2018
Merged

arm: pl310: fix cache sync #2035

merged 1 commit into from
Jan 13, 2018

Conversation

MrVan
Copy link
Contributor

@MrVan MrVan commented Dec 29, 2017

According to PL310 TRM:
Atomic operations:
The following are atomic operations:
Clean Line by PA or by Set/Way
Invalidate Line by PA
Clean and Invalidate Line by PA or by Set/Way
Cache Sync.
These operations stall the slave ports until they are complete.
When these registers are read, bit [0], the C flag, indicates that
a background operation is in progress. When written, bit 0 must be
zero.

So write 1 to sync register is not correct.

Signed-off-by: Peng Fan peng.fan@nxp.com

Copy link
Contributor

@jenswi-linaro jenswi-linaro left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
I think we should wait for feedback from @etienne-lms too.

@@ -96,7 +96,7 @@ loop_cli_sync:
cmp r1, #0
bne loop_cli_sync
Copy link
Contributor

Choose a reason for hiding this comment

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

Not strictly related to the change. Is this loop needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If only PL310 is supported, it loop is not needed. L220 needs that loop :) Keep that loop for PL310 seems no harm.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation. A comment in the code would be welcome. :-)

@MrVan
Copy link
Contributor Author

MrVan commented Jan 12, 2018

@jenswi-linaro Updated with your tags applied and comments added. @etienne-lms do you have comments?

@etienne-lms
Copy link
Contributor

No comment but looks good, even better than before since PL310 cache_sync seems to expect being written to 0 not 1.
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>

According to PL310 TRM:
Atomic operations:
The following are atomic operations:
    Clean Line by PA or by Set/Way
    Invalidate Line by PA
    Clean and Invalidate Line by PA or by Set/Way
    Cache Sync.
These operations stall the slave ports until they are complete.
When these registers are read, bit [0], the C flag, indicates that
a background operation is in progress. When written, bit 0 must be
zero.

So write 1 to sync register is not correct.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
@MrVan
Copy link
Contributor Author

MrVan commented Jan 13, 2018

Updated with Tags applied. Thanks.

@jforissier jforissier merged commit a235648 into OP-TEE:master Jan 13, 2018
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.

4 participants