linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 01/10] doc: DT: camss: Binding document for Qualcomm Camera subsystem driver
@ 2016-11-25 14:56 Todor Tomov
  2016-11-30 22:03 ` Rob Herring
  0 siblings, 1 reply; 4+ messages in thread
From: Todor Tomov @ 2016-11-25 14:56 UTC (permalink / raw)
  To: mchehab, laurent.pinchart+renesas, hans.verkuil, javier,
	s.nawrocki, linux-media, linux-kernel, robh+dt, mark.rutland,
	devicetree
  Cc: bjorn.andersson, srinivas.kandagatla, Todor Tomov

Add DT binding document for Qualcomm Camera subsystem driver.

Signed-off-by: Todor Tomov <todor.tomov@linaro.org>
---
 .../devicetree/bindings/media/qcom,camss.txt       | 196 +++++++++++++++++++++
 1 file changed, 196 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/qcom,camss.txt

diff --git a/Documentation/devicetree/bindings/media/qcom,camss.txt b/Documentation/devicetree/bindings/media/qcom,camss.txt
new file mode 100644
index 0000000..76ad89a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,camss.txt
@@ -0,0 +1,196 @@
+Qualcomm Camera Subsystem
+
+* Properties
+
+- compatible:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Should contain:
+		- "qcom,8x16-camss"
+- reg:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: Register ranges as listed in the reg-names property.
+- reg-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Should contain the following entries:
+		- "csiphy0"
+		- "csiphy0_clk_mux"
+		- "csiphy1"
+		- "csiphy1_clk_mux"
+		- "csid0"
+		- "csid1"
+		- "ispif"
+		- "csi_clk_mux"
+		- "vfe0"
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: Interrupts as listed in the interrupt-names property.
+- interrupt-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Should contain the following entries:
+		- "csiphy0"
+		- "csiphy1"
+		- "csid0"
+		- "csid1"
+		- "ispif"
+		- "vfe0"
+- power-domains:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: A phandle and power domain specifier pairs to the
+		    power domain which is responsible for collapsing
+		    and restoring power to the peripheral.
+- clocks:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: A list of phandle and clock specifier pairs as listed
+		    in clock-names property.
+- clock-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Should contain the following entries:
+		- "camss_top_ahb_clk"
+		- "ispif_ahb_clk"
+		- "csiphy0_timer_clk"
+		- "csiphy1_timer_clk"
+		- "csi0_ahb_clk"
+		- "csi0_clk"
+		- "csi0_phy_clk"
+		- "csi0_pix_clk"
+		- "csi0_rdi_clk"
+		- "csi1_ahb_clk"
+		- "csi1_clk"
+		- "csi1_phy_clk"
+		- "csi1_pix_clk"
+		- "csi1_rdi_clk"
+		- "camss_ahb_clk"
+		- "camss_vfe_vfe_clk"
+		- "camss_csi_vfe_clk"
+		- "iface_clk"
+		- "bus_clk"
+- vdda-supply:
+	Usage: required
+	Value type: <phandle>
+	Definition: A phandle to voltage supply for CSI2.
+- iommus:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: A list of phandle and IOMMU specifier pairs.
+
+* Nodes
+
+- ports:
+	Usage: required
+	Definition: As described in video-interfaces.txt in same directory.
+	Properties:
+		- reg:
+			Usage: required
+			Value type: <u32>
+			Definition: Selects CSI2 PHY interface - PHY0 or PHY1.
+	Endpoint node properties:
+		- clock-lanes:
+			Usage: required
+			Value type: <u32>
+			Definition: The clock lane.
+		- data-lanes:
+			Usage: required
+			Value type: <prop-encoded-array>
+			Definition: An array of data lanes.
+		- qcom,settle-cnt:
+			Usage: required
+			Value type: <u32>
+			Definition: The settle count parameter for CSI PHY.
+
+* An Example
+
+	camss: camss@1b00000 {
+		compatible = "qcom,8x16-camss";
+		reg = <0x1b0ac00 0x200>,
+			<0x1b00030 0x4>,
+			<0x1b0b000 0x200>,
+			<0x1b00038 0x4>,
+			<0x1b08000 0x100>,
+			<0x1b08400 0x100>,
+			<0x1b0a000 0x500>,
+			<0x1b00020 0x10>,
+			<0x1b10000 0x1000>;
+		reg-names = "csiphy0",
+			"csiphy0_clk_mux",
+			"csiphy1",
+			"csiphy1_clk_mux",
+			"csid0",
+			"csid1",
+			"ispif",
+			"csi_clk_mux",
+			"vfe0";
+		interrupts = <GIC_SPI 78 0>,
+			<GIC_SPI 79 0>,
+			<GIC_SPI 51 0>,
+			<GIC_SPI 52 0>,
+			<GIC_SPI 55 0>,
+			<GIC_SPI 57 0>;
+		interrupt-names = "csiphy0",
+			"csiphy1",
+			"csid0",
+			"csid1",
+			"ispif",
+			"vfe0";
+		power-domains = <&gcc VFE_GDSC>;
+		clocks = <&gcc GCC_CAMSS_TOP_AHB_CLK>,
+			<&gcc GCC_CAMSS_ISPIF_AHB_CLK>,
+			<&gcc GCC_CAMSS_CSI0PHYTIMER_CLK>,
+			<&gcc GCC_CAMSS_CSI1PHYTIMER_CLK>,
+			<&gcc GCC_CAMSS_CSI0_AHB_CLK>,
+			<&gcc GCC_CAMSS_CSI0_CLK>,
+			<&gcc GCC_CAMSS_CSI0PHY_CLK>,
+			<&gcc GCC_CAMSS_CSI0PIX_CLK>,
+			<&gcc GCC_CAMSS_CSI0RDI_CLK>,
+			<&gcc GCC_CAMSS_CSI1_AHB_CLK>,
+			<&gcc GCC_CAMSS_CSI1_CLK>,
+			<&gcc GCC_CAMSS_CSI1PHY_CLK>,
+			<&gcc GCC_CAMSS_CSI1PIX_CLK>,
+			<&gcc GCC_CAMSS_CSI1RDI_CLK>,
+			<&gcc GCC_CAMSS_AHB_CLK>,
+			<&gcc GCC_CAMSS_VFE0_CLK>,
+			<&gcc GCC_CAMSS_CSI_VFE0_CLK>,
+			<&gcc GCC_CAMSS_VFE_AHB_CLK>,
+			<&gcc GCC_CAMSS_VFE_AXI_CLK>;
+		clock-names = "camss_top_ahb_clk",
+			"ispif_ahb_clk",
+			"csiphy0_timer_clk",
+			"csiphy1_timer_clk",
+			"csi0_ahb_clk",
+			"csi0_clk",
+			"csi0_phy_clk",
+			"csi0_pix_clk",
+			"csi0_rdi_clk",
+			"csi1_ahb_clk",
+			"csi1_clk",
+			"csi1_phy_clk",
+			"csi1_pix_clk",
+			"csi1_rdi_clk",
+			"camss_ahb_clk",
+			"camss_vfe_vfe_clk",
+			"camss_csi_vfe_clk",
+			"iface_clk",
+			"bus_clk";
+		vdda-supply = <&pm8916_l2>;
+		iommus = <&apps_iommu 3>;
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			port@0 {
+				reg = <0>;
+				csiphy0_ep: endpoint {
+					clock-lanes = <1>;
+					data-lanes = <0 2>;
+					qcom,settle-cnt = <0xe>;
+					remote-endpoint = <&ov5645_ep>;
+				};
+			};
+		};
+	};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 01/10] doc: DT: camss: Binding document for Qualcomm Camera subsystem driver
  2016-11-25 14:56 [PATCH 01/10] doc: DT: camss: Binding document for Qualcomm Camera subsystem driver Todor Tomov
@ 2016-11-30 22:03 ` Rob Herring
  2017-01-09 14:33   ` Todor Tomov
  0 siblings, 1 reply; 4+ messages in thread
From: Rob Herring @ 2016-11-30 22:03 UTC (permalink / raw)
  To: Todor Tomov
  Cc: mchehab, laurent.pinchart+renesas, hans.verkuil, javier,
	s.nawrocki, linux-media, linux-kernel, mark.rutland, devicetree,
	bjorn.andersson, srinivas.kandagatla

On Fri, Nov 25, 2016 at 04:56:53PM +0200, Todor Tomov wrote:
> Add DT binding document for Qualcomm Camera subsystem driver.
> 
> Signed-off-by: Todor Tomov <todor.tomov@linaro.org>
> ---
>  .../devicetree/bindings/media/qcom,camss.txt       | 196 +++++++++++++++++++++
>  1 file changed, 196 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/qcom,camss.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,camss.txt b/Documentation/devicetree/bindings/media/qcom,camss.txt
> new file mode 100644
> index 0000000..76ad89a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,camss.txt
> @@ -0,0 +1,196 @@
> +Qualcomm Camera Subsystem
> +
> +* Properties
> +
> +- compatible:
> +	Usage: required
> +	Value type: <stringlist>
> +	Definition: Should contain:
> +		- "qcom,8x16-camss"

Don't use wildcards in compatible strings. One string per SoC.

> +- reg:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: Register ranges as listed in the reg-names property.
> +- reg-names:
> +	Usage: required
> +	Value type: <stringlist>
> +	Definition: Should contain the following entries:
> +		- "csiphy0"
> +		- "csiphy0_clk_mux"
> +		- "csiphy1"
> +		- "csiphy1_clk_mux"
> +		- "csid0"
> +		- "csid1"
> +		- "ispif"
> +		- "csi_clk_mux"
> +		- "vfe0"

Kind of looks like the phy's should be separate nodes since each phy has 
its own register range, irq, clocks, etc.

> +- interrupts:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: Interrupts as listed in the interrupt-names property.
> +- interrupt-names:
> +	Usage: required
> +	Value type: <stringlist>
> +	Definition: Should contain the following entries:
> +		- "csiphy0"
> +		- "csiphy1"
> +		- "csid0"
> +		- "csid1"
> +		- "ispif"
> +		- "vfe0"
> +- power-domains:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: A phandle and power domain specifier pairs to the
> +		    power domain which is responsible for collapsing
> +		    and restoring power to the peripheral.
> +- clocks:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: A list of phandle and clock specifier pairs as listed
> +		    in clock-names property.
> +- clock-names:
> +	Usage: required
> +	Value type: <stringlist>
> +	Definition: Should contain the following entries:
> +		- "camss_top_ahb_clk"
> +		- "ispif_ahb_clk"
> +		- "csiphy0_timer_clk"
> +		- "csiphy1_timer_clk"
> +		- "csi0_ahb_clk"
> +		- "csi0_clk"
> +		- "csi0_phy_clk"
> +		- "csi0_pix_clk"
> +		- "csi0_rdi_clk"
> +		- "csi1_ahb_clk"
> +		- "csi1_clk"
> +		- "csi1_phy_clk"
> +		- "csi1_pix_clk"
> +		- "csi1_rdi_clk"
> +		- "camss_ahb_clk"
> +		- "camss_vfe_vfe_clk"
> +		- "camss_csi_vfe_clk"
> +		- "iface_clk"
> +		- "bus_clk"
> +- vdda-supply:
> +	Usage: required
> +	Value type: <phandle>
> +	Definition: A phandle to voltage supply for CSI2.
> +- iommus:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: A list of phandle and IOMMU specifier pairs.
> +
> +* Nodes
> +
> +- ports:
> +	Usage: required
> +	Definition: As described in video-interfaces.txt in same directory.
> +	Properties:
> +		- reg:
> +			Usage: required
> +			Value type: <u32>
> +			Definition: Selects CSI2 PHY interface - PHY0 or PHY1.
> +	Endpoint node properties:
> +		- clock-lanes:
> +			Usage: required
> +			Value type: <u32>
> +			Definition: The clock lane.
> +		- data-lanes:
> +			Usage: required
> +			Value type: <prop-encoded-array>
> +			Definition: An array of data lanes.
> +		- qcom,settle-cnt:

This should go in phy node ideally.

> +			Usage: required
> +			Value type: <u32>
> +			Definition: The settle count parameter for CSI PHY.
> +
> +* An Example
> +
> +	camss: camss@1b00000 {
> +		compatible = "qcom,8x16-camss";
> +		reg = <0x1b0ac00 0x200>,
> +			<0x1b00030 0x4>,
> +			<0x1b0b000 0x200>,
> +			<0x1b00038 0x4>,
> +			<0x1b08000 0x100>,
> +			<0x1b08400 0x100>,
> +			<0x1b0a000 0x500>,
> +			<0x1b00020 0x10>,
> +			<0x1b10000 0x1000>;
> +		reg-names = "csiphy0",
> +			"csiphy0_clk_mux",
> +			"csiphy1",
> +			"csiphy1_clk_mux",
> +			"csid0",
> +			"csid1",
> +			"ispif",
> +			"csi_clk_mux",
> +			"vfe0";
> +		interrupts = <GIC_SPI 78 0>,
> +			<GIC_SPI 79 0>,
> +			<GIC_SPI 51 0>,
> +			<GIC_SPI 52 0>,
> +			<GIC_SPI 55 0>,
> +			<GIC_SPI 57 0>;
> +		interrupt-names = "csiphy0",
> +			"csiphy1",
> +			"csid0",
> +			"csid1",
> +			"ispif",
> +			"vfe0";
> +		power-domains = <&gcc VFE_GDSC>;
> +		clocks = <&gcc GCC_CAMSS_TOP_AHB_CLK>,
> +			<&gcc GCC_CAMSS_ISPIF_AHB_CLK>,
> +			<&gcc GCC_CAMSS_CSI0PHYTIMER_CLK>,
> +			<&gcc GCC_CAMSS_CSI1PHYTIMER_CLK>,
> +			<&gcc GCC_CAMSS_CSI0_AHB_CLK>,
> +			<&gcc GCC_CAMSS_CSI0_CLK>,
> +			<&gcc GCC_CAMSS_CSI0PHY_CLK>,
> +			<&gcc GCC_CAMSS_CSI0PIX_CLK>,
> +			<&gcc GCC_CAMSS_CSI0RDI_CLK>,
> +			<&gcc GCC_CAMSS_CSI1_AHB_CLK>,
> +			<&gcc GCC_CAMSS_CSI1_CLK>,
> +			<&gcc GCC_CAMSS_CSI1PHY_CLK>,
> +			<&gcc GCC_CAMSS_CSI1PIX_CLK>,
> +			<&gcc GCC_CAMSS_CSI1RDI_CLK>,
> +			<&gcc GCC_CAMSS_AHB_CLK>,
> +			<&gcc GCC_CAMSS_VFE0_CLK>,
> +			<&gcc GCC_CAMSS_CSI_VFE0_CLK>,
> +			<&gcc GCC_CAMSS_VFE_AHB_CLK>,
> +			<&gcc GCC_CAMSS_VFE_AXI_CLK>;
> +		clock-names = "camss_top_ahb_clk",
> +			"ispif_ahb_clk",
> +			"csiphy0_timer_clk",
> +			"csiphy1_timer_clk",
> +			"csi0_ahb_clk",
> +			"csi0_clk",
> +			"csi0_phy_clk",
> +			"csi0_pix_clk",
> +			"csi0_rdi_clk",
> +			"csi1_ahb_clk",
> +			"csi1_clk",
> +			"csi1_phy_clk",
> +			"csi1_pix_clk",
> +			"csi1_rdi_clk",
> +			"camss_ahb_clk",
> +			"camss_vfe_vfe_clk",
> +			"camss_csi_vfe_clk",
> +			"iface_clk",
> +			"bus_clk";
> +		vdda-supply = <&pm8916_l2>;
> +		iommus = <&apps_iommu 3>;
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			port@0 {
> +				reg = <0>;
> +				csiphy0_ep: endpoint {
> +					clock-lanes = <1>;
> +					data-lanes = <0 2>;
> +					qcom,settle-cnt = <0xe>;
> +					remote-endpoint = <&ov5645_ep>;
> +				};
> +			};
> +		};
> +	};
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 01/10] doc: DT: camss: Binding document for Qualcomm Camera subsystem driver
  2016-11-30 22:03 ` Rob Herring
@ 2017-01-09 14:33   ` Todor Tomov
  2017-06-19 11:56     ` Todor Tomov
  0 siblings, 1 reply; 4+ messages in thread
From: Todor Tomov @ 2017-01-09 14:33 UTC (permalink / raw)
  To: Rob Herring
  Cc: mchehab, laurent.pinchart+renesas, hans.verkuil, javier,
	s.nawrocki, linux-media, linux-kernel, mark.rutland, devicetree,
	bjorn.andersson, srinivas.kandagatla

Hi Rob,

Happy new year,
And thank you for the review.

On 12/01/2016 12:03 AM, Rob Herring wrote:
> On Fri, Nov 25, 2016 at 04:56:53PM +0200, Todor Tomov wrote:
>> Add DT binding document for Qualcomm Camera subsystem driver.
>>
>> Signed-off-by: Todor Tomov <todor.tomov@linaro.org>
>> ---
>>  .../devicetree/bindings/media/qcom,camss.txt       | 196 +++++++++++++++++++++
>>  1 file changed, 196 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/qcom,camss.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/qcom,camss.txt b/Documentation/devicetree/bindings/media/qcom,camss.txt
>> new file mode 100644
>> index 0000000..76ad89a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/qcom,camss.txt
>> @@ -0,0 +1,196 @@
>> +Qualcomm Camera Subsystem
>> +
>> +* Properties
>> +
>> +- compatible:
>> +	Usage: required
>> +	Value type: <stringlist>
>> +	Definition: Should contain:
>> +		- "qcom,8x16-camss"
> 
> Don't use wildcards in compatible strings. One string per SoC.

Ok, I'll fix this.

> 
>> +- reg:
>> +	Usage: required
>> +	Value type: <prop-encoded-array>
>> +	Definition: Register ranges as listed in the reg-names property.
>> +- reg-names:
>> +	Usage: required
>> +	Value type: <stringlist>
>> +	Definition: Should contain the following entries:
>> +		- "csiphy0"
>> +		- "csiphy0_clk_mux"
>> +		- "csiphy1"
>> +		- "csiphy1_clk_mux"
>> +		- "csid0"
>> +		- "csid1"
>> +		- "ispif"
>> +		- "csi_clk_mux"
>> +		- "vfe0"
> 
> Kind of looks like the phy's should be separate nodes since each phy has 
> its own register range, irq, clocks, etc.

Yes, there are a lot of hardware resources here.
I have decided to keep everything into a single platform device as this
represents it better from system point of view.

> 
>> +- interrupts:
>> +	Usage: required
>> +	Value type: <prop-encoded-array>
>> +	Definition: Interrupts as listed in the interrupt-names property.
>> +- interrupt-names:
>> +	Usage: required
>> +	Value type: <stringlist>
>> +	Definition: Should contain the following entries:
>> +		- "csiphy0"
>> +		- "csiphy1"
>> +		- "csid0"
>> +		- "csid1"
>> +		- "ispif"
>> +		- "vfe0"
>> +- power-domains:
>> +	Usage: required
>> +	Value type: <prop-encoded-array>
>> +	Definition: A phandle and power domain specifier pairs to the
>> +		    power domain which is responsible for collapsing
>> +		    and restoring power to the peripheral.
>> +- clocks:
>> +	Usage: required
>> +	Value type: <prop-encoded-array>
>> +	Definition: A list of phandle and clock specifier pairs as listed
>> +		    in clock-names property.
>> +- clock-names:
>> +	Usage: required
>> +	Value type: <stringlist>
>> +	Definition: Should contain the following entries:
>> +		- "camss_top_ahb_clk"
>> +		- "ispif_ahb_clk"
>> +		- "csiphy0_timer_clk"
>> +		- "csiphy1_timer_clk"
>> +		- "csi0_ahb_clk"
>> +		- "csi0_clk"
>> +		- "csi0_phy_clk"
>> +		- "csi0_pix_clk"
>> +		- "csi0_rdi_clk"
>> +		- "csi1_ahb_clk"
>> +		- "csi1_clk"
>> +		- "csi1_phy_clk"
>> +		- "csi1_pix_clk"
>> +		- "csi1_rdi_clk"
>> +		- "camss_ahb_clk"
>> +		- "camss_vfe_vfe_clk"
>> +		- "camss_csi_vfe_clk"
>> +		- "iface_clk"
>> +		- "bus_clk"
>> +- vdda-supply:
>> +	Usage: required
>> +	Value type: <phandle>
>> +	Definition: A phandle to voltage supply for CSI2.
>> +- iommus:
>> +	Usage: required
>> +	Value type: <prop-encoded-array>
>> +	Definition: A list of phandle and IOMMU specifier pairs.
>> +
>> +* Nodes
>> +
>> +- ports:
>> +	Usage: required
>> +	Definition: As described in video-interfaces.txt in same directory.
>> +	Properties:
>> +		- reg:
>> +			Usage: required
>> +			Value type: <u32>
>> +			Definition: Selects CSI2 PHY interface - PHY0 or PHY1.
>> +	Endpoint node properties:
>> +		- clock-lanes:
>> +			Usage: required
>> +			Value type: <u32>
>> +			Definition: The clock lane.
>> +		- data-lanes:
>> +			Usage: required
>> +			Value type: <prop-encoded-array>
>> +			Definition: An array of data lanes.
>> +		- qcom,settle-cnt:
> 
> This should go in phy node ideally.
> 
>> +			Usage: required
>> +			Value type: <u32>
>> +			Definition: The settle count parameter for CSI PHY.
>> +
>> +* An Example
>> +
>> +	camss: camss@1b00000 {
>> +		compatible = "qcom,8x16-camss";
>> +		reg = <0x1b0ac00 0x200>,
>> +			<0x1b00030 0x4>,
>> +			<0x1b0b000 0x200>,
>> +			<0x1b00038 0x4>,
>> +			<0x1b08000 0x100>,
>> +			<0x1b08400 0x100>,
>> +			<0x1b0a000 0x500>,
>> +			<0x1b00020 0x10>,
>> +			<0x1b10000 0x1000>;
>> +		reg-names = "csiphy0",
>> +			"csiphy0_clk_mux",
>> +			"csiphy1",
>> +			"csiphy1_clk_mux",
>> +			"csid0",
>> +			"csid1",
>> +			"ispif",
>> +			"csi_clk_mux",
>> +			"vfe0";
>> +		interrupts = <GIC_SPI 78 0>,
>> +			<GIC_SPI 79 0>,
>> +			<GIC_SPI 51 0>,
>> +			<GIC_SPI 52 0>,
>> +			<GIC_SPI 55 0>,
>> +			<GIC_SPI 57 0>;
>> +		interrupt-names = "csiphy0",
>> +			"csiphy1",
>> +			"csid0",
>> +			"csid1",
>> +			"ispif",
>> +			"vfe0";
>> +		power-domains = <&gcc VFE_GDSC>;
>> +		clocks = <&gcc GCC_CAMSS_TOP_AHB_CLK>,
>> +			<&gcc GCC_CAMSS_ISPIF_AHB_CLK>,
>> +			<&gcc GCC_CAMSS_CSI0PHYTIMER_CLK>,
>> +			<&gcc GCC_CAMSS_CSI1PHYTIMER_CLK>,
>> +			<&gcc GCC_CAMSS_CSI0_AHB_CLK>,
>> +			<&gcc GCC_CAMSS_CSI0_CLK>,
>> +			<&gcc GCC_CAMSS_CSI0PHY_CLK>,
>> +			<&gcc GCC_CAMSS_CSI0PIX_CLK>,
>> +			<&gcc GCC_CAMSS_CSI0RDI_CLK>,
>> +			<&gcc GCC_CAMSS_CSI1_AHB_CLK>,
>> +			<&gcc GCC_CAMSS_CSI1_CLK>,
>> +			<&gcc GCC_CAMSS_CSI1PHY_CLK>,
>> +			<&gcc GCC_CAMSS_CSI1PIX_CLK>,
>> +			<&gcc GCC_CAMSS_CSI1RDI_CLK>,
>> +			<&gcc GCC_CAMSS_AHB_CLK>,
>> +			<&gcc GCC_CAMSS_VFE0_CLK>,
>> +			<&gcc GCC_CAMSS_CSI_VFE0_CLK>,
>> +			<&gcc GCC_CAMSS_VFE_AHB_CLK>,
>> +			<&gcc GCC_CAMSS_VFE_AXI_CLK>;
>> +		clock-names = "camss_top_ahb_clk",
>> +			"ispif_ahb_clk",
>> +			"csiphy0_timer_clk",
>> +			"csiphy1_timer_clk",
>> +			"csi0_ahb_clk",
>> +			"csi0_clk",
>> +			"csi0_phy_clk",
>> +			"csi0_pix_clk",
>> +			"csi0_rdi_clk",
>> +			"csi1_ahb_clk",
>> +			"csi1_clk",
>> +			"csi1_phy_clk",
>> +			"csi1_pix_clk",
>> +			"csi1_rdi_clk",
>> +			"camss_ahb_clk",
>> +			"camss_vfe_vfe_clk",
>> +			"camss_csi_vfe_clk",
>> +			"iface_clk",
>> +			"bus_clk";
>> +		vdda-supply = <&pm8916_l2>;
>> +		iommus = <&apps_iommu 3>;
>> +		ports {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			port@0 {
>> +				reg = <0>;
>> +				csiphy0_ep: endpoint {
>> +					clock-lanes = <1>;
>> +					data-lanes = <0 2>;
>> +					qcom,settle-cnt = <0xe>;
>> +					remote-endpoint = <&ov5645_ep>;
>> +				};
>> +			};
>> +		};
>> +	};
>> -- 
>> 1.9.1
>>

-- 
Best regards,
Todor Tomov

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 01/10] doc: DT: camss: Binding document for Qualcomm Camera subsystem driver
  2017-01-09 14:33   ` Todor Tomov
@ 2017-06-19 11:56     ` Todor Tomov
  0 siblings, 0 replies; 4+ messages in thread
From: Todor Tomov @ 2017-06-19 11:56 UTC (permalink / raw)
  To: Rob Herring
  Cc: mchehab, laurent.pinchart+renesas, hans.verkuil, javier,
	s.nawrocki, linux-media, linux-kernel, mark.rutland, devicetree,
	bjorn.andersson, srinivas.kandagatla

Hi Rob,

On 01/09/2017 04:33 PM, Todor Tomov wrote:
> Hi Rob,
> 
> Happy new year,
> And thank you for the review.
> 
> On 12/01/2016 12:03 AM, Rob Herring wrote:
>> On Fri, Nov 25, 2016 at 04:56:53PM +0200, Todor Tomov wrote:
>>> Add DT binding document for Qualcomm Camera subsystem driver.
>>>
>>> Signed-off-by: Todor Tomov <todor.tomov@linaro.org>
>>> ---
>>>  .../devicetree/bindings/media/qcom,camss.txt       | 196 +++++++++++++++++++++
>>>  1 file changed, 196 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/media/qcom,camss.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/qcom,camss.txt b/Documentation/devicetree/bindings/media/qcom,camss.txt
>>> new file mode 100644
>>> index 0000000..76ad89a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/media/qcom,camss.txt
>>> @@ -0,0 +1,196 @@
>>> +Qualcomm Camera Subsystem
>>> +
>>> +* Properties
>>> +
>>> +- compatible:
>>> +	Usage: required
>>> +	Value type: <stringlist>
>>> +	Definition: Should contain:
>>> +		- "qcom,8x16-camss"
>>
>> Don't use wildcards in compatible strings. One string per SoC.
> 
> Ok, I'll fix this.
> 
>>
>>> +- reg:
>>> +	Usage: required
>>> +	Value type: <prop-encoded-array>
>>> +	Definition: Register ranges as listed in the reg-names property.
>>> +- reg-names:
>>> +	Usage: required
>>> +	Value type: <stringlist>
>>> +	Definition: Should contain the following entries:
>>> +		- "csiphy0"
>>> +		- "csiphy0_clk_mux"
>>> +		- "csiphy1"
>>> +		- "csiphy1_clk_mux"
>>> +		- "csid0"
>>> +		- "csid1"
>>> +		- "ispif"
>>> +		- "csi_clk_mux"
>>> +		- "vfe0"
>>
>> Kind of looks like the phy's should be separate nodes since each phy has 
>> its own register range, irq, clocks, etc.
> 
> Yes, there are a lot of hardware resources here.
> I have decided to keep everything into a single platform device as this
> represents it better from system point of view.
> 

Following this patch,
- following the short discussion which we had on Linaro Connect,
- following some evaluation of the possibilities,
I'd like to try to keep it as a single node. Some of the resources are separate
and so appear to be the clocks too but actually there are dependencies across the
hardware modules for clocks of their adjacent modules. In addition there
are configuration dependencies across the hardware modules - e.g. on a CSID module
a CSIPHY module's id needs to be set (and vice-versa), on a CSID module the
csi lane positions need to be set and so on. So I think a single driver to
handle this (and thus a single dt node) makes more sense. I'm preparing a
v2 of the patch set and will send it shortly.

-- 
Best regards,
Todor Tomov

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-06-19 11:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-25 14:56 [PATCH 01/10] doc: DT: camss: Binding document for Qualcomm Camera subsystem driver Todor Tomov
2016-11-30 22:03 ` Rob Herring
2017-01-09 14:33   ` Todor Tomov
2017-06-19 11:56     ` Todor Tomov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).