On Monday 07 December 2015 16:01:03 xuejiancheng wrote:

On 2015/12/4 18:56, Arnd Bergmann wrote:

quoted

On Friday 04 December 2015 11:21:28 xuejiancheng wrote:

quoted

Hi Arnd,

On 2015/12/3 17:44, Arnd Bergmann wrote:

quoted

On Thursday 03 December 2015 10:39:24 Jiancheng Xue wrote:

quoted

+#ifndef __DTS_HI3519_CLOCK_H
+#define __DTS_HI3519_CLOCK_H

Please try to avoid adding headers like this if you can at all.

I might ask you to merge the header file in one merge window
otherwise and submit the platform code one kernel later, as they
tendn to cause us needless dependencies otherwise.

Sorry. In v1, Rob suggested putting binding doc and header files in
a separate patch. The clock driver indeed depends on the header.

I will put the header and the clock driver in a patch, and keep the
binding doc in another patch.

Having split patches is better, I was really commenting on the fact
that ideally you would not have a header file at all. If we merge
the header through arm-soc, then you won't be able to merge the
clk driver easily, and if you merge the header through the clk
maintainer, I'm can't take your dts files.

Thank you for your comments. Because the clocks in the crg module have
different types and random layouts. If this header file is removed,
the clock driver and the dts files will get very complicated.

Could you help me acknowledge it if I put the header file and clock driver
in a patch?

Could you give me some suggestions If I want to keep this header file?

If this is another clock controller that has a random register layout,
then adding the header file is the least problematic solution indeed.

quoted

quoted

quoted

Where do those numbers come from? They are not consecutive, so it sounds
like they are directly from the data sheet and won't be needed in the driver.
If that's true, just use the numbers directly, as you do for everything
else.

The numbers are defined by myself, not directly from the data sheet. Some numbers
are reserved for device nodes which will be added later. So they are not consecutive now.

I don't understand. How do you decide which numbers to reserve? If the
numbers are completely arbitrary and you have no idea what other clocks
there are, you can simply have consecutive numbers here and do the driver
accordingly.

The clocks are divided into several groups according to their types. The clocks in
a group are expected to have consecutive numbers. So some numbers are reserved for
every group in this file, just like in some existing headers. Other clocks will be
added when other peripherals drivers are submitted. They will use the reserve numbers
and be inserted into existing groups.

Of course it is not required to reserve numbers for later added clocks.

Are you able to enumerate all the clocks then? If all clocks that are
supported by this clock controllers have names in the data sheet, I
would prefer to just list them all now rather than only enter the ones
we already need, to avoid having future merge conflicts.

Then we just need to add code to support those clocks when we need them,
but that can be done in parallel to adding the DT nodes and the driver,
rather than strongly serializing the patch flow on the header file patches.

	Arnd