On 2015/12/7 17:36, Arnd Bergmann wrote: > On Monday 07 December 2015 16:01:03 xuejiancheng wrote: >> On 2015/12/4 18:56, Arnd Bergmann wrote: >>> On Friday 04 December 2015 11:21:28 xuejiancheng wrote: >>>> Hi Arnd, >>>> >>>> On 2015/12/3 17:44, Arnd Bergmann wrote: >>>>> On Thursday 03 December 2015 10:39:24 Jiancheng Xue wrote: >>>>>> +#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. > >>>>> 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. OK. I'll enumerate all the clocks in the data sheet in next version patch. Thank you. > > 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
