On 20-07-18, 12:03, Rob Herring wrote: > On Tue, Jul 17, 2018 at 06:27:28PM +0530, Vinod Koul wrote: > > From: Todor Tomov <todor.tomov@xxxxxxxxxx> > > +Required properties: > > + - compatible: Should be one of: > > + - "qcom,cci-v1.0.8" for 8916; > > + - "qcom,cci-v1.4.0" for 8996. > > It's not very convincing to use version numbers instead of normal SoC > part# convention when there is a 1-1 mapping. I was under the impression that we don't have 1-1 mapping. An IP version is found in multiple SoCs > > + - #address-cells: Should be <1>. > > + - #size-cells: Should be <0>. > > + - reg: Base address of the controller and length of memory mapped region. > > + - interrupts: Specifier for CCI interrupt. > > + - clocks: List of clock specifiers, one for each entry in clock-names. > > + - clock-names: Should contain: > > + - "mmss_mmagic_ahb" - on 8996 only; > > + - "camss_top_ahb"; > > + - "cci_ahb"; > > + - "cci"; > > + - "camss_ahb". > > + > > +Required properties on 8996: > > Rather than '8996', use the compatible string. Makes sense > > + - power-domains: Power domain specifier. > > + > > +Optional: > > + - clock-frequency: Desired I2C bus clock frequency in Hz, defaults to 100 kHz > > + if omitted. > > This is the only place that says this block is or has a I2C controller. > Please make that more explicit (including what ever else this block > does). I'm guessing there's more given how many clocks and that it has a > power domain. Yes this also goes GPIO which will be added after this. -- ~Vinod