Re: [PATCH v2 1/2] dt-bindings: i2c: Add binding document for Qualcomm CCI

[Vinod <vkoul@kernel.org>]

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@linaro.org>

> > +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