Hi Geert,
Thanks for your comments.
On 2021-04-23 11:25:59 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
>
> On Tue, Apr 13, 2021 at 8:49 PM Niklas Söderlund
> <niklas.soderlund+renesas@xxxxxxxxxxxx> wrote:
> > The V3U have 32 VIN, 4 CSI-2 and 4 ISP nodes that interact with each
> > other for video capture. Add all nodes and record how they are
> > interconnected.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
>
> Thanks for your patch!
>
> The "standard" properties (reg/interrupts/clocks/resets/...) look good
> to me.
>
> > --- a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
> > @@ -1017,6 +1017,870 @@ msiof5: spi@e6c28000 {
> > status = "disabled";
> > };
> >
> > + vin00: video@e6ef0000 {
> > + compatible = "renesas,vin-r8a779a0";
> > + reg = <0 0xe6ef0000 0 0x1000>;
> > + interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&cpg CPG_MOD 730>;
> > + power-domains = <&sysc R8A779A0_PD_ALWAYS_ON>;
> > + resets = <&cpg 730>;
> > + renesas,id = <0>;
> > + status = "disabled";
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@2 {
> > + reg = <2>;
> > +
> > + vin00isp0: endpoint {
>
> Shouldn't this be endpoint@0 (+ appropriate {address,size}-cells above),
> as per the DT bindings?
> Same for vin 01-07.
It really should, but doing so for single endpoint if that endpoint is
@0 produced warnings when verifying the DTS. I think this is a bug in
the verification but after digging around in other DTS files it seemed
this was the "solution".
I will dig some more.
>
> > + remote-endpoint = <&isp0vin00>;
> > + };
> > + };
> > + };
> > + };
>
> > + isp0: isp@fed00000 {
> > + compatible = "renesas,isp-r8a779a0";
>
> renesas,r8a779a0-isp (for all ISP nodes).
Yes ;-)
>
> > + reg = <0 0xfed00000 0 0x10000>;
> > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&cpg CPG_MOD 612>;
> > + power-domains = <&sysc R8A779A0_PD_A3ISP01>;
> > + resets = <&cpg 612>;
> > + status = "disabled";
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > + isp0csi40: endpoint {
>
> Shouldn't this be endpoint@something (+ appropriate {address,size}-cells
> above), as the bindings say there can be multiple endpoints?
> Same for isp2csi42.
Same as above, will dig more.
>
> > + remote-endpoint = <&csi40isp0>;
> > + };
> > + };
>
> > + isp1: isp@fed20000 {
> > + compatible = "renesas,isp-r8a779a0";
> > + reg = <0 0xfed20000 0 0x10000>;
> > + interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&cpg CPG_MOD 613>;
> > + power-domains = <&sysc R8A779A0_PD_A3ISP01>;
> > + resets = <&cpg 613>;
> > + status = "disabled";
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + reg = <0>;
> > + isp1csi41: endpoint@1 {
> > + reg = <1>;
>
> BTW, where does the "1" come from?
> Same for isp3csi43.
Each ISP have one input port, modeled as port@0. This port can runtime
switched between two CSI-2 receivers, see bit 31 of the ISP ISPINPUTSEL0
register and the ISP block diagram.
For the final form of this node each ISP will have an endpoint@0 and
endpoint@1 to each of the two nodes it are connected to. Unfortunately
the datasheet I have access to do not document these relationships and
what is added in this patch is found by trail and error ;-P
>
> > + remote-endpoint = <&csi41isp1>;
> > + };
> > + };
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
--
Regards,
Niklas Söderlund
