Thread (19 messages) 19 messages, 2 authors, 2017-10-04
STALE3182d

[PATCH 3/5] firmware: arm_scpi: pre-populate dvfs info in scpi_probe

From: hkallweit1@gmail.com (Heiner Kallweit)
Date: 2017-10-03 16:00:55

Am 03.10.2017 um 12:57 schrieb Sudeep Holla:

On 02/10/17 23:07, Heiner Kallweit wrote:
quoted
Am 02.10.2017 um 13:17 schrieb Sudeep Holla:
quoted

On 29/09/17 22:44, Heiner Kallweit wrote:
quoted
Pre-populating the dvfs info data in scpi_probe allows to make 
all memory allocations device-managed. This helps to simplify 
scpi_remove and eventually to get rid of scpi_remove completely.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
[...]
quoted
quoted
Does it make sense to continue to complete all MAX_DVFS_DOMAINS ? 
Just wondering if there will be any firmware that returns errors 
for few domains(e.g. not supported in initial versions of 
firmware). I don't want to stop probing due to that. Let me know 
what you think.
The (legacy) SCPI firmware on my system seems to ignore the domain
in CMD_GET_DVFS_INFO. It returns the same dvfs info independent of
the domain parameter. So indeed prepopulating may not be the best
idea.
I can't follow that. Does the behavior change probe time vs runtime ?
I am fine with probe time populate, just that we can't stop or propagate
any error as it fails to probe other dependent drivers which may work
fine without DVFS(e.g. clocks, sensors and power domains)
quoted
Still we need to do something in the remove callback to deal with the
scenario you describe (error for few domains).
devm_* APIs will take care of freeing DVFS domain info, so what else
needs to be done ? I just noticed devm_kfree(NULL) can produce WARN_ON
splat, is that what you are referring ?
quoted
Remove does for (i = 0; i < MAX_DVFS_DOMAINS && info->dvfs[i]; i++) {
and therefore stops at the first unpopulated domain and doesn't free
the memory for further populated domains. I'll provide a patch for
it.
Does that mean you are re-introducing scpi_remove ? I kind of liked
removing it.
Sorry for the confusion. Then I'll go with the original approach and
just make sure that errors whilst populating dvfs info are ignored.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help