Re: [PATCH] cpufreq: rockchip: add driver

From: Heiko Stübner
Date: Mon Mar 21 2016 - 11:14:00 EST


Hi,

Am Montag, 21. MÃrz 2016, 21:24:32 schrieb Feng Xiao:
> å 2016/3/21 17:58, Viresh Kumar åé:
> > On 21-03-16, 10:54, Heiko StÃbner wrote:
> >> I hadn't seen that yet ... nice that cpufreq-dt now also supports
> >> clusters :-)
> >>
> >> The other part still stands though, as we probably should register the
> >> platform-device somewhere else and not in some new special module.
> >>
> >> When everything is using cpufreq-dt now, I guess we could just add it to
> >> the core rockchip clk-code. Or was there some agreement where this
> >> should be done (obviously not the devicetree itself)?
>
> Of_clk_init is called early, and platform_device_register_simple should
> be called after devices_init, it will be failed to do it from clk-code.
> So we need add a new file or add module_init to each clock controller
> driver(like clk-rk3368.c, clk-rk3399.c) ?

as Viresh said, it should be ok to do it like your approach creating a module
in drivers/cpufreq. But the compatible check is necessary.

Doing it this way also makes it easier to have

> > Yeah, there was a discussion around creating a white or black list of
> > platforms that want to create a platform device for cpufreq-dt. That can
> > be done in cpufreq-dt.c or a new file, but I haven't worked out on that
> > yet.
> >
> > You can do it from clk-code or from the driver that was added in this
> > thread. Just that you need to match your platform's compatible string
> > before doing that.
> Rockchip-cpufreq.c depends on ARM_ROCKCHIP_CPUFREQ, it will not be
> compiled on non-Rockchip platforms.
> The driver can support all Rockchip SoCs up to now, add
> of_machine_is_compatible may be redundant ?

Please always keep multiplatform in mind. These days the kernel can be
compiled for multiple architectures at the same time, so you can have support
for Rockchip, Exynos, Qualcom and whatever in the same kernel image.

Therefore a compile-time check is not enough and you need to check the
actually running machine as well.


Heiko