Hi Ezequiel,
On Montag, 9. Mai 2022 16:17:03 CEST Ezequiel Garcia wrote:
> Hi Nicolas,
>
> On Sun, May 8, 2022 at 5:26 PM Nicolas Frattaroli
> <frattaroli.nicolas@xxxxxxxxx> wrote:
> >
> > The RK3566 and RK3568 come with a dedicated Hantro instance solely for
> > encoding. This patch adds a node for this to the device tree, along with
> > a node for its MMU.
> >
> > Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@xxxxxxxxx>
> > ---
> > arch/arm64/boot/dts/rockchip/rk356x.dtsi | 21 +++++++++++++++++++++
> > 1 file changed, 21 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > index 7cdef800cb3c..2e3c9e1887e3 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > @@ -508,6 +508,27 @@ gpu: gpu@fde60000 {
> > status = "disabled";
> > };
> >
> > + vepu: video-codec@fdee0000 {
> > + compatible = "rockchip,rk3568-vepu";
> > + reg = <0x0 0xfdee0000 0x0 0x800>;
> > + interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupt-names = "vepu";
>
> It this block "encoder only" and if so, maybe we should remove the
> "interrupt-names" [1]?
>
> The driver is able to handle it. See:
>
> https://elixir.bootlin.com/linux/latest/source/drivers/staging/media/hantro/hantro_drv.c#L962
>
> You might have to adjust the dt-bindings for this.
>
> [1] https://lore.kernel.org/linux-media/20210324151715.GA3070006@xxxxxxxxxxxxxxxxxx/
What the Linux driver can handle should not matter to the device tree;
device trees are independent of drivers and kernels.
What does matter though is to be consistent in the bindings.
interrupt-names is a required property even if there's only a vdpu
interrupt. I modelled my vepu-only binding after this case.
If robh thinks there is no value to having the interrupt show up
as anything other than "default" in /proc/interrupts, then I respectfully
disagree with that opinion and point out that this should have been brought
up when the vdpu-only case in the bindings was made to require
interrupt-names also.
Changing the binding now that there theoretically could be drivers out
in the wild (though I doubt it) that do require interrupt-names, because
the binding told them that this is okay to do, seems unwise to me.
Regards,
Nicolas Frattaroli
>
> Thanks,
> Ezequiel
>
> > + clocks = <&cru ACLK_JENC>, <&cru HCLK_JENC>;
> > + clock-names = "aclk", "hclk";
> > + iommus = <&vepu_mmu>;
> > + power-domains = <&power RK3568_PD_RGA>;
> > + };
> > +
> > + vepu_mmu: iommu@fdee0800 {
> > + compatible = "rockchip,rk3568-iommu";
> > + reg = <0x0 0xfdee0800 0x0 0x40>;
> > + interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&cru ACLK_JENC>, <&cru HCLK_JENC>;
> > + clock-names = "aclk", "iface";
> > + power-domains = <&power RK3568_PD_RGA>;
> > + #iommu-cells = <0>;
> > + };
> > +
> > sdmmc2: mmc@fe000000 {
> > compatible = "rockchip,rk3568-dw-mshc", "rockchip,rk3288-dw-mshc";
> > reg = <0x0 0xfe000000 0x0 0x4000>;
> > --
> > 2.36.0
> >
> >
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@xxxxxxxxxxxxxxxxxxx
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
