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: regulator: fix error case in regulator_supported_voltages() + split asserts #6784

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

etienne-lms
Copy link
Contributor

2 changes:

  • Fix case when .supported_voltages() handler is supported to assert voltage description when the handler succeeds and to use pre-filled fallback description when the handler returns TEE_ERROR_NOT_SUPPORTED instead of asserting @*desc content while that pointer was likely not initialized with a valid address.
    Fixes: af5b988 ("drivers: regulator: supported voltage consider levels bounds")
  • Split assertions in regulator_supported_voltages() to ease debugging, providing a better indication of which condition is not fulfilled.

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.

@gatien or @patrickdelaunay, may I ask you to have a look at these 2 changes?

@GseoC
Copy link
Contributor

GseoC commented Apr 16, 2024

The commit description of drivers: regulator: fix error case in regulator_supported_voltages() commit is a bit unclear to me, would you mind rephrasing it? maybe something like:

Fix case when .supported_voltages() handler exists but reports TEE_ERROR_NOT_SUPPORTED.
In this case, the regulator should use the fallback values.

I let you mix my suggestion with whatever you esteem right.

With this changed:

Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>

@patrickdelaunay
Copy link
Contributor

For the series:

Acked-by: Patrick Delaunay patrick.delaunay@foss.st.com

@etienne-lms
Copy link
Contributor Author

The commit description of drivers: regulator: fix error case in regulator_supported_voltages() commit is a bit unclear to me, would you mind rephrasing it? maybe something like:

Thanks Gatien, indeed the sentence is far to long and not readable.

Fix case when .supported_voltages() handler is supported. If successful
it shall assert voltage description data. If it returns
TEE_ERROR_NOT_SUPPORTED, it shall use the pre-filled fallback description.

Fixes: af5b988 ("drivers: regulator: supported voltage consider levels bounds")
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Acked-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
Split assertions in regulator_supported_voltages() to ease debugging,
providing a better indication of which condition is not fulfilled.

Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Acked-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
@etienne-lms
Copy link
Contributor Author

I've rephrased in 1st commit message and applied the review patches to both.

Copy link

This pull request has been marked as a stale pull request because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this pull request will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants