Re: [PATCH 2/3] ARM64: add tegra-vi support in T210 device-tree

2015-09-22 Thread Bryan Wu

On 09/22/2015 05:17 AM, Thierry Reding wrote:

Hi Bryan,

This patchset really needs to be Cc'ed to linux-te...@vger.kernel.org,
it's becoming impossible to track it otherwise.


My bad, I copied and pasted old git send-email command line.


On Mon, Sep 21, 2015 at 11:55:54AM -0700, Bryan Wu wrote:
[...]

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi 
b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 1168bcd..3f6501f 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -109,9 +109,181 @@
  
  		vi@0,5408 {

compatible = "nvidia,tegra210-vi";
-   reg = <0x0 0x5408 0x0 0x0004>;
+   reg = <0x0 0x5408 0x0 0x800>;

This now no longer matches the address map in the TRM.


That's because we split out the CSI parts from VI.


interrupts = ;
status = "disabled";
+   clocks = <_car TEGRA210_CLK_VI>,
+<_car TEGRA210_CLK_CSI>,
+<_car TEGRA210_CLK_PLL_C>;
+   clock-names = "vi", "csi", "parent";
+   resets = <_car 20>;
+   reset-names = "vi";
+
+   power-domains = < TEGRA_POWERGATE_VENC>;

As an aside, this will currently prevent the driver from probing because
the generic power domain code will return -EPROBE_DEFER if this property
is encountered but the corresponding driver (for the PMC) hasn't
registered any power domains yet. So until the PMC driver changes have
been merged we can't add these power-domains properties.

That's not something for you to worry about, though. I'll make sure to
strip this out if it happens to get merged before the PMC driver changes
and add it back it afterwards.


OK, need I strip this out and provide a separated patch which waiting 
for the PMC changes merged?



+
+   iommus = < TEGRA_SWGROUP_VI>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+
+   vi_in0: endpoint {
+   remote-endpoint = <_out0>;
+   };
+   };
+   port@1 {
+   reg = <1>;
+
+   vi_in1: endpoint {
+   remote-endpoint = <_out1>;
+   };
+   };
+   port@2 {
+   reg = <2>;
+
+   vi_in2: endpoint {
+   remote-endpoint = <_out2>;
+   };
+   };
+   port@3 {
+   reg = <3>;
+
+   vi_in3: endpoint {
+   remote-endpoint = <_out3>;
+   };
+   };
+   port@4 {
+   reg = <4>;
+
+   vi_in4: endpoint {
+   remote-endpoint = <_out4>;
+   };
+   };
+   port@5 {
+   reg = <5>;
+
+   vi_in5: endpoint {
+   remote-endpoint = <_out5>;
+   };
+   };
+
+   };
+   };
+
+   csi@0,54080838 {

I think this and its siblings should be children of the vi node. They
are within the same memory aperture according to the TRM and the fact
that the VI needs to reference the "CSI" clock indicates that the
coupling is tighter than this current DT layout makes it out to be.


+   compatible = "nvidia,tegra210-csi";
+   reg = <0x0 0x54080838 0x0 0x700>;

Some of the internal register documentation indicates that the register
range actually starts at an offset of 0x800, it just so happens that we
don't use any of the registers from 0x800 to 0x837. So I think this
needs to be adapted.

Right, in the driver we don't use those registers from 0x800 to 0x837.


+   clocks = <_car TEGRA210_CLK_CILAB>;
+   clock-names = "cil";
+
+   ports {
+   #address-cells = <1>;
+

Re: [PATCH 2/3] ARM64: add tegra-vi support in T210 device-tree

2015-09-22 Thread Thierry Reding
Hi Bryan,

This patchset really needs to be Cc'ed to linux-te...@vger.kernel.org,
it's becoming impossible to track it otherwise.

On Mon, Sep 21, 2015 at 11:55:54AM -0700, Bryan Wu wrote:
[...]
> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi 
> b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> index 1168bcd..3f6501f 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> @@ -109,9 +109,181 @@
>  
>   vi@0,5408 {
>   compatible = "nvidia,tegra210-vi";
> - reg = <0x0 0x5408 0x0 0x0004>;
> + reg = <0x0 0x5408 0x0 0x800>;

This now no longer matches the address map in the TRM.

>   interrupts = ;
>   status = "disabled";
> + clocks = <_car TEGRA210_CLK_VI>,
> +  <_car TEGRA210_CLK_CSI>,
> +  <_car TEGRA210_CLK_PLL_C>;
> + clock-names = "vi", "csi", "parent";
> + resets = <_car 20>;
> + reset-names = "vi";
> +
> + power-domains = < TEGRA_POWERGATE_VENC>;

As an aside, this will currently prevent the driver from probing because
the generic power domain code will return -EPROBE_DEFER if this property
is encountered but the corresponding driver (for the PMC) hasn't
registered any power domains yet. So until the PMC driver changes have
been merged we can't add these power-domains properties.

That's not something for you to worry about, though. I'll make sure to
strip this out if it happens to get merged before the PMC driver changes
and add it back it afterwards.

> +
> + iommus = < TEGRA_SWGROUP_VI>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + vi_in0: endpoint {
> + remote-endpoint = <_out0>;
> + };
> + };
> + port@1 {
> + reg = <1>;
> +
> + vi_in1: endpoint {
> + remote-endpoint = <_out1>;
> + };
> + };
> + port@2 {
> + reg = <2>;
> +
> + vi_in2: endpoint {
> + remote-endpoint = <_out2>;
> + };
> + };
> + port@3 {
> + reg = <3>;
> +
> + vi_in3: endpoint {
> + remote-endpoint = <_out3>;
> + };
> + };
> + port@4 {
> + reg = <4>;
> +
> + vi_in4: endpoint {
> + remote-endpoint = <_out4>;
> + };
> + };
> + port@5 {
> + reg = <5>;
> +
> + vi_in5: endpoint {
> + remote-endpoint = <_out5>;
> + };
> + };
> +
> + };
> + };
> +
> + csi@0,54080838 {

I think this and its siblings should be children of the vi node. They
are within the same memory aperture according to the TRM and the fact
that the VI needs to reference the "CSI" clock indicates that the
coupling is tighter than this current DT layout makes it out to be.

> + compatible = "nvidia,tegra210-csi";
> + reg = <0x0 0x54080838 0x0 0x700>;

Some of the internal register documentation indicates that the register
range actually starts at an offset of 0x800, it just so happens that we
don't use any of the registers from 0x800 to 0x837. So I think this
needs to be adapted.

> + clocks = <_car TEGRA210_CLK_CILAB>;
> + clock-names = "cil";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + csi_in0: