Re: [RFC PATCH v3 3/6] dt-binding: tegra: Add VI and CSI bindings

[Sowjanya Komatineni]

On 2/20/20 11:45 AM, Rob Herring wrote:
> External email: Use caution opening links or attachments
>
>
> On Tue, Feb 18, 2020 at 9:28 PM Sowjanya Komatineni
> <skomatineni@xxxxxxxxxx> wrote:
> > On 2/18/20 3:15 PM, Rob Herring wrote:
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > On Fri, Feb 14, 2020 at 10:23:25AM -0800, Sowjanya Komatineni wrote:
> > > > Tegra contains VI controller which can support up to 6 MIPI CSI
> > > > camera sensors.
> > > >
> > > > Each Tegra CSI port from CSI unit can be one-to-one mapper to
> > > > VI channel and can capture from an external camera sensor or
> > > > from built-in test pattern generator.
> > > >
> > > > This patch adds dt-bindings for Tegra VI and CSI.
> > > >
> > > > Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
> > > > ---
> > > >    .../display/tegra/nvidia,tegra20-host1x.txt        | 55 ++++++++++++++++++----
> > > >    1 file changed, 47 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> > > > index 9999255ac5b6..3d0ed540a646 100644
> > > > --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> > > > +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> > > > @@ -40,14 +40,24 @@ of the following host1x client modules:
> > > >
> > > >      Required properties:
> > > >      - compatible: "nvidia,tegra<chip>-vi"
> > > > -  - reg: Physical base address and length of the controller's registers.
> > > > +  - reg: Physical base address and length of the controller registers.
> > > >      - interrupts: The interrupt outputs from the controller.
> > > > -  - clocks: Must contain one entry, for the module clock.
> > > > +  - clocks: Must contain an entry for the module clock "vi"
> > > >        See ../clocks/clock-bindings.txt for details.
> > > >      - resets: Must contain an entry for each entry in reset-names.
> > > >        See ../reset/reset.txt for details.
> > > > -  - reset-names: Must include the following entries:
> > > > -    - vi
> > > > +  - reset-names: Must include the entry "vi"
> > > > +
> > > > +  Tegra210 has CSI part of VI sharing same host interface and register
> > > > +  space. So, VI device node should have CSI child node.
> > > > +
> > > > +  - csi: mipi csi interface to vi
> > > > +
> > > > +    Required properties:
> > > > +    - compatible: "nvidia,tegra<chip>-csi"
> > > > +    - reg: Physical base address and length of the controller registers.
> > > > +    - clocks: Must contain entries csi, cilab, cilcd, cile clocks.
> > > > +      See ../clocks/clock-bindings.txt for details.
> > > >
> > > >    - epp: encoder pre-processor
> > > >
> > > > @@ -310,12 +320,41 @@ Example:
> > > >                 };
> > > >
> > > >                 vi {
> > > > -                     compatible = "nvidia,tegra20-vi";
> > > > -                     reg = <0x54080000 0x00040000>;
> > > > +                     compatible = "nvidia,tegra210-vi";
> > > > +                     reg = <0x0 0x54080000 0x0 0x700>;
> > > >                         interrupts = <0 69 0x04>;
> > > > -                     clocks = <&tegra_car TEGRA20_CLK_VI>;
> > > > -                     resets = <&tegra_car 100>;
> > > > +                     assigned-clocks = <&tegra_car TEGRA210_CLK_VI>;
> > > > +                     assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_C4_OUT0>;
> > > > +                     clocks = <&tegra_car TEGRA210_CLK_VI>;
> > > > +                     clock-names = "vi";
> > > > +                     resets = <&tegra_car 20>;
> > > >                         reset-names = "vi";
> > > > +
> > > > +                     #address-cells = <2>;
> > > > +                     #size-cells = <2>;
> > > > +
> > > > +                     ranges = <0x0 0x54080808 0x0 0x54080808 0x0 0x2000>;
> > > > +
> > > > +                     csi@0x54080838 {
> > > Drop '0x'
> > Will fix in v4
> > > > +                             compatible = "nvidia,tegra210-csi";
> > > > +                             reg = <0x0 0x54080838 0x0 0x2000>;
> > > Kind of odd that this address and ranges address are not the same. And
> > > also wrong that the size here exceeds the bounds of ranges.
> > >
> > > Also, best practice is to make the child address 0 or relative to the
> > > parent.
> > Actual CSI starts at offset 0x808 but we don't use couple of registers
> > at offset 0x808.
> >
> > Will update ranges in v4 to start from 0x838 offset and will make child
> > address relative to parent.
> Seems odd, but okay. And you will never, ever need to use those
> registers no matter what, and we can reject any DT change trying to
> change it later?
>
> Rob

Yes not required to access them by driver.

On T210, CSI registers under VI starts from location 0x54080808

SW don't need to access initial 3 registers at 0x54080808, 0x54080818, 
0x54080828

Actual CSI registers that are needed for SW starts from 0x54080838.