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: read current level from the device #6662

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

patrickdelaunay
Copy link
Contributor

Always read current voltage level from the device instead of caching the level in struct regulator. This fixes issues for when the regulator level value depends on the parent regulator (supply). It is up the regulator driver to cache or not this value in their private data if applicable.

Copy link
Contributor

@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.

Makes sense to me.

Nitpicking; reading again the commit message, maybe words "current level" can be confusing; this is about the current voltage level, not the current current level. I would suggest to either replace current level with effective level or explicit current voltage level what do you think? Or maybe:

drivers: regulator: do not cache voltage level value

...

This this fixes the regulator framework, I think it's worth adding a Fixes: tag:
Fixes: 1a3d3273040b ("drivers: regulator framework").

@@ -286,10 +284,7 @@ static inline TEE_Result regulator_set_min_voltage(struct regulator *regulator)
* regulator_get_voltage() - Get regulator current level in microvolt
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should rephrase this to Get regulator effective voltage level in microvolt.
but that out of the scope of your P-R.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@patrickdelaunay
Copy link
Contributor Author

drivers: regulator: do not cache voltage level value

done

This this fixes the regulator framework, I think it's worth adding a Fixes: tag: Fixes: 1a3d3273040b ("drivers: regulator framework").

done

Copy link
Contributor

@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.

Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com> with minor fix in commit message:
"It is up the regulator driver to cache or not this value in its private data..."
or
"It is up the regulator drivers to cache or not this value in their private data..."

@patrickdelaunay
Copy link
Contributor Author

"It is up the regulator drivers to cache or not this value in their private data..."_

done
thanks for the review.

Always read current voltage level from the device instead of
caching the level in struct regulator. This fixes issues for
when the regulator level value depends on the parent regulator
(supply). It is up the regulator drivers to cache or not this
value in their private data if applicable.

Fixes: 1a3d327 ("drivers: regulator framework")
Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
@etienne-lms
Copy link
Contributor

@jforissier, is this change ok to be merged?

@jforissier
Copy link
Contributor

@jforissier, is this change ok to be merged?

It is, sorry for not noticing.

@jforissier jforissier merged commit b4d1c08 into OP-TEE:master Mar 11, 2024
7 checks passed
@etienne-lms
Copy link
Contributor

No worries. Thanks.

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