On Wed, Oct 4, 2017 at 5:01 PM, Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
>>> +
>>> +static struct platform_driver scmi_cpufreq_platdrv = {
>>> + .driver = {
>>> + .name = "scmi-cpufreq",
>>> + },
>>> + .probe = scmi_cpufreq_probe,
>>> + .remove = scmi_cpufreq_remove,
>>> +};
>>
>> You appear to have split this driver into the 'cpufreq' side and
>> the 'protocol' handler in a different file. I already commented that
>> the way the main scmi driver knows about all the protocols looks
>> bad, here we can see another aspect of the same problem.
>>
>> Rather than manually register a platform_device for the purpose
>> of connecting it to the cpufreq driver, there should be a way
>> to dynamically register the protocol from the cpufreq driver
>> and then have both in the same file.
>>
>
> I agree that should be possible. I took this approach for 2 reasons:
>
> 1. to avoid all sorts of probe ordering issues
> 2. we may have system with multiple instances of SCMI. E.g. a system
> may have multiple remote processors, each controlling dvfs/power mgmt
> of a subset of CPUs/devices controller in OS.
>
> I have to admit that I haven't thought too much in details yet. That's
> the main idea behind scmi_handle and restricting access to that only to
> sub-nodes in it. I am open to suggestions.
How about introducing a separate bus_type for protocols?
The platform_device you use here isn't really the best abstraction,
and with a new bus_type, you can handle multiple instances of
scmi as well as decoupling them from the protocol drivers.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
