Messages in this thread | | | Subject | Re: [PATCH v2 1/9] clk: hi3519: add dt-binding document and header file | From | xuejiancheng <> | Date | Tue, 8 Dec 2015 17:45:25 +0800 |
| |
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.
Is it OK if I put the header file and the clock driver in a patch?
If it's not OK, could you tell me how should I separate the patches?
Thank you.
> >>>>> 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 > > . >
| |