All of lore.kernel.org
 help / color / mirror / Atom feed
From: Abel Vesa <abelvesa@kernel.org>
To: Shawn Guo <shawnguo@kernel.org>
Cc: "Mirela Rabulea (OSS)" <mirela.rabulea@oss.nxp.com>,
	robh+dt@kernel.org, aisheng.dong@nxp.com, guoniu.zhou@nxp.com,
	linux-arm-kernel@lists.infradead.org, peng.fan@nxp.com,
	s.hauer@pengutronix.de, linux-imx@nxp.com,
	devicetree@vger.kernel.org, mchehab@kernel.org,
	hverkuil-cisco@xs4all.nl, linux-media@vger.kernel.org,
	linux-kernel@vger.kernel.org, paul.kocialkowski@bootlin.com,
	daniel.baluta@nxp.com, robert.chiras@nxp.com,
	laurentiu.palcu@nxp.com, p.zabel@pengutronix.de,
	ezequiel@collabora.com, Mirela Rabulea <mirela.rabulea@nxp.com>
Subject: Re: [PATCH v11] arm64: dts: imx8qxp: Add jpeg encoder/decoder nodes
Date: Fri, 14 May 2021 13:24:41 +0300	[thread overview]
Message-ID: <YJ5P6ZdAMIsYrTMX@ryzen.lan> (raw)
In-Reply-To: <20210513073832.GS3425@dragon>

On 21-05-13 15:38:33, Shawn Guo wrote:
> On Fri, Apr 23, 2021 at 01:14:14PM +0300, Mirela Rabulea (OSS) wrote:
> > From: Mirela Rabulea <mirela.rabulea@nxp.com>
> > 
> > Add dts for imaging subsytem, include jpeg nodes here.
> > Tested on imx8qxp only, should work on imx8qm, but it was not tested.
> > 
> > Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> 
> So the bindings and driver parts have been accepted already?
> 
> > ---
> > Changes in v11:
> >   Adress feedback from Aisheng Dong:
> >   - Rename img_jpeg_dec_clk/img_jpeg_enc_clk to jpeg_dec_lpcg/jpeg_enc_lpcg to make it visible it's lpcg not other type of clk
> >   - Drop the cameradev node, not needed for jpeg
> >   - Match assigned-clocks & assigned-clock-rates
> > 
> >  .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 82 +++++++++++++++++++
> >  arch/arm64/boot/dts/freescale/imx8qxp.dtsi    |  1 +
> >  2 files changed, 83 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > new file mode 100644
> > index 000000000000..c508e5d0c92b
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > @@ -0,0 +1,82 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019-2021 NXP
> > + * Zhou Guoniu <guoniu.zhou@nxp.com>
> > + */
> > +img_subsys: bus@58000000 {
> > +	compatible = "simple-bus";
> > +	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +	ranges = <0x58000000 0x0 0x58000000 0x1000000>;
> > +
> > +	img_ipg_clk: clock-img-ipg {
> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <200000000>;
> > +		clock-output-names = "img_ipg_clk";
> > +	};
> 
> Hmm, not sure a fixed-clock should be in the subsystem.
> 

Agreed. Assuming the img_ipg_clk is only used on 8QXP, you could move it
into imx8qxp-ss-img.dtsi. This way every other platform that uses this
ss file will not be impacted.

> > +
> > +	img_jpeg_dec_lpcg: clock-controller@585d0000 {
> > +		compatible = "fsl,imx8qxp-lpcg";
> > +		reg = <0x585d0000 0x10000>;
> > +		#clock-cells = <1>;
> > +		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> > +		clock-indices = <IMX_LPCG_CLK_0>,
> > +				<IMX_LPCG_CLK_4>;
> > +		clock-output-names = "img_jpeg_dec_lpcg_clk",
> > +				     "img_jpeg_dec_lpcg_ipg_clk";
> > +		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>;
> > +	};
> > +
> > +	img_jpeg_enc_lpcg: clock-controller@585f0000 {
> > +		compatible = "fsl,imx8qxp-lpcg";
> > +		reg = <0x585f0000 0x10000>;
> > +		#clock-cells = <1>;
> > +		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> > +		clock-indices = <IMX_LPCG_CLK_0>,
> > +				<IMX_LPCG_CLK_4>;
> > +		clock-output-names = "img_jpeg_enc_lpcg_clk",
> > +				     "img_jpeg_enc_lpcg_ipg_clk";
> > +		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>;
> > +	};
> > +
> > +	jpegdec: jpegdec@58400000 {
> 
> Keep nodes sorted in unit address.
> 
> Shawn
> 
> > +		compatible = "nxp,imx8qxp-jpgdec";
> > +		reg = <0x58400000 0x00050000 >;
> > +		interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&img_jpeg_dec_lpcg 0>,
> > +			 <&img_jpeg_dec_lpcg 1>;
> > +		clock-names = "per", "ipg";
> > +		assigned-clocks = <&img_jpeg_dec_lpcg 0>,
> > +				  <&img_jpeg_dec_lpcg 1>;
> > +		assigned-clock-rates = <200000000>, <200000000>;
> > +		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S0>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S1>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S2>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S3>;
> > +	};
> > +
> > +	jpegenc: jpegenc@58450000 {
> > +		compatible = "nxp,imx8qxp-jpgenc";
> > +		reg = <0x58450000 0x00050000 >;
> > +		interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&img_jpeg_enc_lpcg 0>,
> > +			 <&img_jpeg_enc_lpcg 1>;
> > +		clock-names = "per", "ipg";
> > +		assigned-clocks = <&img_jpeg_enc_lpcg 0>,
> > +				  <&img_jpeg_enc_lpcg 1>;
> > +		assigned-clock-rates = <200000000>, <200000000>;
> > +		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S0>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S1>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S2>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S3>;
> > +	};
> > +};
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > index 1e6b4995091e..2d9589309bd0 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > @@ -258,6 +258,7 @@
> >  	};
> >  
> >  	/* sorted in register address */
> > +	#include "imx8-ss-img.dtsi"
> >  	#include "imx8-ss-adma.dtsi"
> >  	#include "imx8-ss-conn.dtsi"
> >  	#include "imx8-ss-ddr.dtsi"
> > -- 
> > 2.17.1
> > 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Abel Vesa <abelvesa@kernel.org>
To: Shawn Guo <shawnguo@kernel.org>
Cc: "Mirela Rabulea (OSS)" <mirela.rabulea@oss.nxp.com>,
	robh+dt@kernel.org, aisheng.dong@nxp.com, guoniu.zhou@nxp.com,
	linux-arm-kernel@lists.infradead.org, peng.fan@nxp.com,
	s.hauer@pengutronix.de, linux-imx@nxp.com,
	devicetree@vger.kernel.org, mchehab@kernel.org,
	hverkuil-cisco@xs4all.nl, linux-media@vger.kernel.org,
	linux-kernel@vger.kernel.org, paul.kocialkowski@bootlin.com,
	daniel.baluta@nxp.com, robert.chiras@nxp.com,
	laurentiu.palcu@nxp.com, p.zabel@pengutronix.de,
	ezequiel@collabora.com, Mirela Rabulea <mirela.rabulea@nxp.com>
Subject: Re: [PATCH v11] arm64: dts: imx8qxp: Add jpeg encoder/decoder nodes
Date: Fri, 14 May 2021 13:24:41 +0300	[thread overview]
Message-ID: <YJ5P6ZdAMIsYrTMX@ryzen.lan> (raw)
In-Reply-To: <20210513073832.GS3425@dragon>

On 21-05-13 15:38:33, Shawn Guo wrote:
> On Fri, Apr 23, 2021 at 01:14:14PM +0300, Mirela Rabulea (OSS) wrote:
> > From: Mirela Rabulea <mirela.rabulea@nxp.com>
> > 
> > Add dts for imaging subsytem, include jpeg nodes here.
> > Tested on imx8qxp only, should work on imx8qm, but it was not tested.
> > 
> > Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> 
> So the bindings and driver parts have been accepted already?
> 
> > ---
> > Changes in v11:
> >   Adress feedback from Aisheng Dong:
> >   - Rename img_jpeg_dec_clk/img_jpeg_enc_clk to jpeg_dec_lpcg/jpeg_enc_lpcg to make it visible it's lpcg not other type of clk
> >   - Drop the cameradev node, not needed for jpeg
> >   - Match assigned-clocks & assigned-clock-rates
> > 
> >  .../arm64/boot/dts/freescale/imx8-ss-img.dtsi | 82 +++++++++++++++++++
> >  arch/arm64/boot/dts/freescale/imx8qxp.dtsi    |  1 +
> >  2 files changed, 83 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > new file mode 100644
> > index 000000000000..c508e5d0c92b
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> > @@ -0,0 +1,82 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019-2021 NXP
> > + * Zhou Guoniu <guoniu.zhou@nxp.com>
> > + */
> > +img_subsys: bus@58000000 {
> > +	compatible = "simple-bus";
> > +	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +	ranges = <0x58000000 0x0 0x58000000 0x1000000>;
> > +
> > +	img_ipg_clk: clock-img-ipg {
> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <200000000>;
> > +		clock-output-names = "img_ipg_clk";
> > +	};
> 
> Hmm, not sure a fixed-clock should be in the subsystem.
> 

Agreed. Assuming the img_ipg_clk is only used on 8QXP, you could move it
into imx8qxp-ss-img.dtsi. This way every other platform that uses this
ss file will not be impacted.

> > +
> > +	img_jpeg_dec_lpcg: clock-controller@585d0000 {
> > +		compatible = "fsl,imx8qxp-lpcg";
> > +		reg = <0x585d0000 0x10000>;
> > +		#clock-cells = <1>;
> > +		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> > +		clock-indices = <IMX_LPCG_CLK_0>,
> > +				<IMX_LPCG_CLK_4>;
> > +		clock-output-names = "img_jpeg_dec_lpcg_clk",
> > +				     "img_jpeg_dec_lpcg_ipg_clk";
> > +		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>;
> > +	};
> > +
> > +	img_jpeg_enc_lpcg: clock-controller@585f0000 {
> > +		compatible = "fsl,imx8qxp-lpcg";
> > +		reg = <0x585f0000 0x10000>;
> > +		#clock-cells = <1>;
> > +		clocks = <&img_ipg_clk>, <&img_ipg_clk>;
> > +		clock-indices = <IMX_LPCG_CLK_0>,
> > +				<IMX_LPCG_CLK_4>;
> > +		clock-output-names = "img_jpeg_enc_lpcg_clk",
> > +				     "img_jpeg_enc_lpcg_ipg_clk";
> > +		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>;
> > +	};
> > +
> > +	jpegdec: jpegdec@58400000 {
> 
> Keep nodes sorted in unit address.
> 
> Shawn
> 
> > +		compatible = "nxp,imx8qxp-jpgdec";
> > +		reg = <0x58400000 0x00050000 >;
> > +		interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&img_jpeg_dec_lpcg 0>,
> > +			 <&img_jpeg_dec_lpcg 1>;
> > +		clock-names = "per", "ipg";
> > +		assigned-clocks = <&img_jpeg_dec_lpcg 0>,
> > +				  <&img_jpeg_dec_lpcg 1>;
> > +		assigned-clock-rates = <200000000>, <200000000>;
> > +		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S0>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S1>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S2>,
> > +				<&pd IMX_SC_R_MJPEG_DEC_S3>;
> > +	};
> > +
> > +	jpegenc: jpegenc@58450000 {
> > +		compatible = "nxp,imx8qxp-jpgenc";
> > +		reg = <0x58450000 0x00050000 >;
> > +		interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&img_jpeg_enc_lpcg 0>,
> > +			 <&img_jpeg_enc_lpcg 1>;
> > +		clock-names = "per", "ipg";
> > +		assigned-clocks = <&img_jpeg_enc_lpcg 0>,
> > +				  <&img_jpeg_enc_lpcg 1>;
> > +		assigned-clock-rates = <200000000>, <200000000>;
> > +		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S0>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S1>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S2>,
> > +				<&pd IMX_SC_R_MJPEG_ENC_S3>;
> > +	};
> > +};
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > index 1e6b4995091e..2d9589309bd0 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > @@ -258,6 +258,7 @@
> >  	};
> >  
> >  	/* sorted in register address */
> > +	#include "imx8-ss-img.dtsi"
> >  	#include "imx8-ss-adma.dtsi"
> >  	#include "imx8-ss-conn.dtsi"
> >  	#include "imx8-ss-ddr.dtsi"
> > -- 
> > 2.17.1
> > 

  reply	other threads:[~2021-05-14 10:26 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-23 10:14 [PATCH v11] arm64: dts: imx8qxp: Add jpeg encoder/decoder nodes Mirela Rabulea (OSS)
2021-04-23 10:14 ` Mirela Rabulea (OSS)
2021-05-13  7:38 ` Shawn Guo
2021-05-13  7:38   ` Shawn Guo
2021-05-14 10:24   ` Abel Vesa [this message]
2021-05-14 10:24     ` Abel Vesa
2021-05-18  7:13     ` Aisheng Dong
2021-05-18  7:13       ` Aisheng Dong
2021-05-18  7:10   ` Aisheng Dong
2021-05-18  7:10     ` Aisheng Dong
2021-05-19  5:33   ` [EXT] " Mirela Rabulea
2021-05-19  5:33     ` Mirela Rabulea
2021-05-18  7:21 ` Aisheng Dong
2021-05-18  7:21   ` Aisheng Dong
  -- strict thread matches above, loose matches on Subject: below --
2021-05-19  5:33 Mirela Rabulea (OSS)
2021-05-19  5:33 ` Mirela Rabulea (OSS)
2021-05-19  6:57 ` Mirela Rabulea
2021-05-19  6:57   ` Mirela Rabulea

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YJ5P6ZdAMIsYrTMX@ryzen.lan \
    --to=abelvesa@kernel.org \
    --cc=aisheng.dong@nxp.com \
    --cc=daniel.baluta@nxp.com \
    --cc=devicetree@vger.kernel.org \
    --cc=ezequiel@collabora.com \
    --cc=guoniu.zhou@nxp.com \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=laurentiu.palcu@nxp.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=mirela.rabulea@nxp.com \
    --cc=mirela.rabulea@oss.nxp.com \
    --cc=p.zabel@pengutronix.de \
    --cc=paul.kocialkowski@bootlin.com \
    --cc=peng.fan@nxp.com \
    --cc=robert.chiras@nxp.com \
    --cc=robh+dt@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.