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

[hkallweit1@gmail.com (Heiner Kallweit)]

Am 03.10.2017 um 12:57 schrieb Sudeep Holla:
> 
> 
> On 02/10/17 23:07, Heiner Kallweit wrote:
>> Am 02.10.2017 um 13:17 schrieb Sudeep Holla:
>>>
>>>
>>> On 29/09/17 22:44, Heiner Kallweit wrote:
>>>> 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>
> 
> [...]
> 
>>> 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)
> 
>> 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 ?
> 
>>
>> 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.