| Message ID | 20180717125729.23045-2-vkoul@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i2c: Add support for Qualcomm Camera Control Interface | expand |
On Tue, Jul 17, 2018 at 06:27:28PM +0530, Vinod Koul wrote: > From: Todor Tomov <todor.tomov@linaro.org> > > Add DT binding document for Qualcomm Camera Control Interface device > > Signed-off-by: Todor Tomov <todor.tomov@linaro.org> > Signed-off-by: Vinod Koul <vkoul@kernel.org> > --- > .../devicetree/bindings/i2c/i2c-qcom-cci.txt | 46 ++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > create mode 100644 Documentation/devicetree/bindings/i2c/i2c-qcom-cci.txt > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-qcom-cci.txt b/Documentation/devicetree/bindings/i2c/i2c-qcom-cci.txt > new file mode 100644 > index 000000000000..cb1fe158fba3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/i2c/i2c-qcom-cci.txt > @@ -0,0 +1,46 @@ > +Qualcomm Camera Control Interface controller > + > +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. > + - #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. > + - 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. > + > +Example: > + > + cci@a0c000 { > + compatible = "qcom,cci-v1.4.0"; > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0xa0c000 0x1000>; > + interrupts = <GIC_SPI 295 IRQ_TYPE_EDGE_RISING>; > + power-domains = <&mmcc CAMSS_GDSC>; > + clocks = <&mmcc MMSS_MMAGIC_AHB_CLK>, > + <&mmcc CAMSS_TOP_AHB_CLK>, > + <&mmcc CAMSS_CCI_AHB_CLK>, > + <&mmcc CAMSS_CCI_CLK>, > + <&mmcc CAMSS_AHB_CLK>; > + clock-names = "mmss_mmagic_ahb", > + "camss_top_ahb", > + "cci_ahb", > + "cci", > + "camss_ahb"; > + clock-frequency = <400000>; > + }; > -- > 2.14.4 >
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.
On Sun, Jul 22, 2018 at 9:54 PM Vinod <vkoul@kernel.org> wrote: > > 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 Maybe so, but I'm only looking at what is getting upstream. We already discussed this at length on the thermal binding[1]. As you've written it, we'll still have to update the doc to add other chips for an already documented version. Rob [1] https://lkml.org/lkml/2018/7/5/830
diff --git a/Documentation/devicetree/bindings/i2c/i2c-qcom-cci.txt b/Documentation/devicetree/bindings/i2c/i2c-qcom-cci.txt new file mode 100644 index 000000000000..cb1fe158fba3 --- /dev/null +++ b/Documentation/devicetree/bindings/i2c/i2c-qcom-cci.txt @@ -0,0 +1,46 @@ +Qualcomm Camera Control Interface controller + +Required properties: + - compatible: Should be one of: + - "qcom,cci-v1.0.8" for 8916; + - "qcom,cci-v1.4.0" for 8996. + - #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: + - power-domains: Power domain specifier. + +Optional: + - clock-frequency: Desired I2C bus clock frequency in Hz, defaults to 100 kHz + if omitted. + +Example: + + cci@a0c000 { + compatible = "qcom,cci-v1.4.0"; + #address-cells = <1>; + #size-cells = <0>; + reg = <0xa0c000 0x1000>; + interrupts = <GIC_SPI 295 IRQ_TYPE_EDGE_RISING>; + power-domains = <&mmcc CAMSS_GDSC>; + clocks = <&mmcc MMSS_MMAGIC_AHB_CLK>, + <&mmcc CAMSS_TOP_AHB_CLK>, + <&mmcc CAMSS_CCI_AHB_CLK>, + <&mmcc CAMSS_CCI_CLK>, + <&mmcc CAMSS_AHB_CLK>; + clock-names = "mmss_mmagic_ahb", + "camss_top_ahb", + "cci_ahb", + "cci", + "camss_ahb"; + clock-frequency = <400000>; + };