On Fri, 2021-03-26 at 15:33 +0100, Benjamin Gaignard wrote:
>
> Le 26/03/2021 à 15:24, Philipp Zabel a écrit :
> > On Thu, Mar 18, 2021 at 09:20:46AM +0100, Benjamin Gaignard wrote:
> > > Split VPU node in two: one for G1 and one for G2 since they are
> > > different hardware blocks.
> > > Add syscon for hardware control block.
> > > Remove reg-names property that is useless.
> > > Each VPU node only need one interrupt.
> > >
> > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx>
> > > ---
> > > version 5:
> > > - use syscon instead of VPU reset
> > >
> > > arch/arm64/boot/dts/freescale/imx8mq.dtsi | 43 ++++++++++++++++++-----
> > > 1 file changed, 34 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > > index 17c449e12c2e..b537d153ebbd 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > > @@ -1329,15 +1329,16 @@ usb3_phy1: usb-phy@382f0040 {
> > > status = "disabled";
> > > };
> > >
> > > - vpu: video-codec@38300000 {
> > > + vpu_ctrl: syscon@38320000 {
> > > + compatible = "nxp,imx8mq-vpu-ctrl", "syscon";
> > > + reg = <0x38320000 0x10000>;
> > > + };
> > > +
> > > + vpu_g1: video-codec@38300000 {
> > > compatible = "nxp,imx8mq-vpu";
> > > - reg = <0x38300000 0x10000>,
> > > - <0x38310000 0x10000>,
> > > - <0x38320000 0x10000>;
> > > - reg-names = "g1", "g2", "ctrl";
> > > - interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
> > > - <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> > > - interrupt-names = "g1", "g2";
> > > + reg = <0x38300000 0x10000>;
> > > + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> > > + interrupt-names = "g1";
> > > clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
> > > <&clk IMX8MQ_CLK_VPU_G2_ROOT>,
> > > <&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
> > > @@ -1350,9 +1351,33 @@ vpu: video-codec@38300000 {
> > > <&clk IMX8MQ_VPU_PLL_OUT>,
> > > <&clk IMX8MQ_SYS1_PLL_800M>,
> > > <&clk IMX8MQ_VPU_PLL>;
> > > - assigned-clock-rates = <600000000>, <600000000>,
> > > + assigned-clock-rates = <600000000>, <300000000>,
> > I'd like to see this mentioned in the commit message.
>
> Yes I would do that.
> The value comes from the datasheet.
>
> >
> > > + <800000000>, <0>;
> > > + power-domains = <&pgc_vpu>;
> > > + nxp,imx8mq-vpu-ctrl = <&vpu_ctrl>;
> > > + };
> > > +
> > > + vpu_g2: video-codec@38310000 {
> > > + compatible = "nxp,imx8mq-vpu-g2";
> > > + reg = <0x38310000 0x10000>;
> > > + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> > > + interrupt-names = "g2";
> > > + clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
> > > + <&clk IMX8MQ_CLK_VPU_G2_ROOT>,
> > > + <&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
> > > + clock-names = "g1", "g2", "bus";
> > > + assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>,
> > Can the G1 clock configuration be dropped from the G2 device node and
> > the G2 clock configuration from the G1 device node? It looks weird that
> > these devices configure each other's clocks.
>
> No because if only one device node is enabled we need to configure the both
> clocks anyway.
>
Since this is akward, how about adding a comment here in the dtsi to clarify it?
Thanks,
Ezequiel
