On 2016/1/20 14:38, Tomeu Vizoso wrote:

On 19 January 2016 at 19:20, Rob Herring [off-list ref] wrote:

quoted

On Fri, Jan 15, 2016 at 1:57 AM, xuejiancheng [off-list ref] wrote:

quoted

On 2016/1/14 21:16, xuejiancheng wrote:

quoted

Hi Mike,

On 2016/1/14 2:57, Michael Turquette wrote:

quoted

Quoting xuejiancheng (2016-01-12 19:03:01)

quoted

Hi Stephen,
   Thank you very much for your reply.

On 2016/1/13 6:12, Stephen Boyd wrote:

quoted

On 01/08, Jiancheng Xue wrote:

quoted

diff --git a/drivers/clk/hisilicon/Kconfig b/drivers/clk/hisilicon/Kconfig
index e434854..b6baebf 100644
--- a/drivers/clk/hisilicon/Kconfig
+++ b/drivers/clk/hisilicon/Kconfig

@@ -1,3 +1,10 @@
+config COMMON_CLK_HI3519
+    tristate "Clock Driver for Hi3519"

It looks like this has to be bool. Otherwise it needs to be a
platform driver and the hisilicon APIs need to be exported and
lose their __init markings.

Yes,it's a problem. I will fix it in next version. Thank you.

The best solution would be to make this clock driver a real platform
driver.

Now the work clock of the clocksource timer-sp804 is provided by this driver. So
it need to be registered early by CLK_OF_DECLARE. If the timer clock is treated
as a fixed-clock provider, this driver can be implemented as a platform driver.
Then the crg device must be registered before other clock consumer devices.Accordingly
the crg device node must be written above all other clock consumer devices node in dts files.
I think it is also a dependence.

Can you help me understand why it is better to make this driver a platform driver?
Thank you very much!

arch_initcall(customize_machine)
-->of_platform_populate
   -->of_platform_bus_create
     -->of_amba_device_create
        -->amba_device_add
           -->amba_get_enable_pclk
The call sequence above shows that the clock of the amba device must be registered before
amba_device_add. The clock of "arm,pl011" uart is registered in the probe function of the
platform driver "hi3519-crg". So the platform device "hi3519-crg" must be created before
the amba device "arm,pl011" uart.

It is a problem, but Tomeu had a fix to support deferred probes here.
That was part of the on-demand probing series, but maybe it needs to
be applied separately if we are moving clock drivers to platform
drivers.

Hi,

Marek Szyprowski has kindly taken those two patches as part of a series of him:

http://lkml.kernel.org/g/1450868368-5650-1-git-send-email-m.szyprowski at samsung.com

I think it would be great if you could test them and report.

Hi Tomeu,

I have applied the patch "https://lkml.org/lkml/2015/12/23/105" and tested on my hi3519-demb board.
It works even if the apb_pclk is registered later than the amba-pl011 device being registered.

But I think it is a problem if amba_read_periphid() returns -ENOMEM or -ENODEV when apb_pclk is available.
In this condition?amba_match() returns a non zero value which means the driver and device matches
and the amba_probe() will be called, but amba_device->periphid remains as 0. Then amba_lookup() called in
amba_probe() will return a null id pointer.The null pointer will be passed to amba_driver->probe() and
this may cause a segment fault.

Regards,

Jiancheng

Thanks,

Tomeu

.