On 10/08/2013 11:33 AM, Nishanth Menon wrote: > On 10/08/2013 12:08 PM, Stephen Warren wrote: >> On 10/08/2013 10:14 AM, Nishanth Menon wrote: >>> On 10/08/2013 09:39 AM, Laxman Dewangan wrote: >>>> Thanks Nishanth for review. >>>> >>>> On Tuesday 08 October 2013 06:59 PM, Nishanth Menon wrote: >>>>> On 10/08/2013 08:21 AM, Laxman Dewangan wrote: >>>>>> Palmas devices has two clock output CLK32K_KG and CLK32K_KG_AUDIO >>>>> not all palmas devices have 2 clocks - example: tps659038 >>>> >>>> This is for generic palmas and I have seen it for TPS65913, TPS65914, >>>> TPS80036. If the generic one is not compatible then it need to add >>>> device specific and at that time, it is require to update the binding >>>> document accordingly. >>> >>> ?? you do have two clocks inside the device they should be represented >>> as two compatible entities - that simplifies everyone's life. >> >> I think the terminology you're using here is quite confusing. >> >> Are you talking about having two different compatible values for two >> different HW designs, where those different designs implement different >> sets of clocks (which makes sense), or two different DT nodes for two >> different clocks (which IMHO doesn't always, unless those different >> clocks *truly* are separate IP blocks with completely independent >> register regions, and where those IP blocks are likely to be re-used >> as-is in other chips). > > clk32k and clk32k_audio are two different resources and since these > are two different resource instances - a "compatible" matching an > actual device is my suggestion. The fact that two clocks are two different resources isn't at all relevant to DT structure. HW module design is what's relevant. > clk32k and clk32k_audio are two different resources because they have > their specific set of controls registers and may even be independently > present in a Palmas variant. That's a better argument, assuming that: The registers for those two clocks aren't randomly interleaved with other registers within the HW module. That would imply that the clock registers aren't independant HW blocks. > To highlight this: The example of tps659038 where clk32k is not > present, but clk32k_audio is present (and happens to be disabled by > default - thanks to an OTP on the chip - on platform like DRA7-evm, it > is used to for 32k clk for wlan -currently hacked in u-boot using > plain i2c writes[1] - yes it is yucky). That can easily be handled by having separate compatible values for a monolithic overall Palmas or Palmas-clock node/HW-block. The fact that different chips are different doesn't, in and of itself, need to influence whether the different clocks are represented as different DT nodes. > Obviously, there are many ways to implement this. based on the current > implementation, it indicates that if i create a node with > "ti,palmas-clk" -i'd create two clocks - that is wrong for tps659038. > > Now (with the current approach), if I have to create a one clock for > tps659038, i have to fix the for adding clock providers, add up > "ti,tps659038-clk" etc.. it is doable - but IMHO, I dont need to do it > with only the relevant nodes in dts. > > Further, it has no way to indicate that device X uses clock Y using > clocks =<&xyz> either. Sorry, I just don't understand that. If a clock provider provides two clocks, it could number then e.g. 0 and 1. Clock consumers would reference those IDs. If a different chip that uses the same binding only supports one of those two clocks, just have the driver return an error if the DT attempts to use/reference the invalid clock ID; nothign could be simpler. -- 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