[PATCH v2 1/3] dt-bindings: media: amphion: Fix subnode pattern

[PATCH v2 1/3] dt-bindings: media: amphion: Fix subnode pattern

[Alexander Stein]

DT nodes use dashes instead of underscore. Adjust pattern to also fix
warnings regarding nodes in arch/arm64/boot/dts/freescale/imx8-ss-vpu.dtsi

Fixes: 38ad8b32f3af ("dt-bindings: media: amphion: add amphion video codec bindings")
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v2:
* Fixed examples

 Documentation/devicetree/bindings/media/amphion,vpu.yaml | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/amphion,vpu.yaml b/Documentation/devicetree/bindings/media/amphion,vpu.yaml
index a9d80eaeeeb6..c0d83d755239 100644
--- a/Documentation/devicetree/bindings/media/amphion,vpu.yaml
+++ b/Documentation/devicetree/bindings/media/amphion,vpu.yaml
@@ -47,7 +47,7 @@ patternProperties:
     $ref: ../mailbox/fsl,mu.yaml#
 
 
-  "^vpu_core@[0-9a-f]+$":
+  "^vpu-core@[0-9a-f]+$":
     description:
       Each core correspond a decoder or encoder, need to configure them
       separately. NXP i.MX8QM SoC has one decoder and two encoder, i.MX8QXP SoC
@@ -143,7 +143,7 @@ examples:
         power-domains = <&pd IMX_SC_R_VPU_MU_2>;
       };
 
-      vpu_core0: vpu_core@2d080000 {
+      vpu_core0: vpu-core@2d080000 {
         compatible = "nxp,imx8q-vpu-decoder";
         reg = <0x2d080000 0x10000>;
         power-domains = <&pd IMX_SC_R_VPU_DEC_0>;
@@ -154,7 +154,7 @@ examples:
         memory-region = <&decoder_boot>, <&decoder_rpc>;
       };
 
-      vpu_core1: vpu_core@2d090000 {
+      vpu_core1: vpu-core@2d090000 {
         compatible = "nxp,imx8q-vpu-encoder";
         reg = <0x2d090000 0x10000>;
         power-domains = <&pd IMX_SC_R_VPU_ENC_0>;
@@ -165,7 +165,7 @@ examples:
         memory-region = <&encoder1_boot>, <&encoder1_rpc>;
       };
 
-      vpu_core2: vpu_core@2d0a0000 {
+      vpu_core2: vpu-core@2d0a0000 {
         reg = <0x2d0a0000 0x10000>;
         compatible = "nxp,imx8q-vpu-encoder";
         power-domains = <&pd IMX_SC_R_VPU_ENC_1>;
-- 
2.34.1

[PATCH v2 2/3] dt-bindings: spi: lpspi: Add power-domains

[Alexander Stein]

i.MX8(X) based SoC use a power domain. Allow supplying this domain in
bindings.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v2:
* None

 Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml b/Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml
index e91425012319..727c5346b8ce 100644
--- a/Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml
@@ -63,6 +63,9 @@ properties:
     maximum: 2
     default: 1
 
+  power-domains:
+    maxItems: 1
+
 required:
   - compatible
   - reg
-- 
2.34.1

[PATCH v2 3/3] dt-bindings: media: imx-jpeg: Add clocks property

[Alexander Stein]

i.MX8 and i.MX8X both use two clocks for accessing the periphery.
Add clocks and clock-names properties accordingly.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v2:
* None

 .../devicetree/bindings/media/nxp,imx8-jpeg.yaml          | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
index 3d9d1db37040..2533e16720f2 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
@@ -46,6 +46,14 @@ properties:
     minItems: 2               # Wrapper and 1 slot
     maxItems: 5               # Wrapper and 4 slots
 
+  clocks:
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: per
+      - const: ipg
+
 required:
   - compatible
   - reg
-- 
2.34.1

Re: [PATCH v2 1/3] dt-bindings: media: amphion: Fix subnode pattern

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 452 bytes --]

On Mon, Jul 24, 2023 at 02:20:58PM +0200, Alexander Stein wrote:
> DT nodes use dashes instead of underscore. Adjust pattern to also fix
> warnings regarding nodes in arch/arm64/boot/dts/freescale/imx8-ss-vpu.dtsi
> 
> Fixes: 38ad8b32f3af ("dt-bindings: media: amphion: add amphion video codec bindings")
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/3] dt-bindings: spi: lpspi: Add power-domains

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

On Mon, Jul 24, 2023 at 02:20:59PM +0200, Alexander Stein wrote:
> i.MX8(X) based SoC use a power domain. Allow supplying this domain in
> bindings.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

(btw, please send cover letters for multi-patch series)

> ---
> Changes in v2:
> * None
> 
>  Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml b/Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml
> index e91425012319..727c5346b8ce 100644
> --- a/Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml
> @@ -63,6 +63,9 @@ properties:
>      maximum: 2
>      default: 1
>  
> +  power-domains:
> +    maxItems: 1
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 3/3] dt-bindings: media: imx-jpeg: Add clocks property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]

On Mon, Jul 24, 2023 at 02:21:00PM +0200, Alexander Stein wrote:
> i.MX8 and i.MX8X both use two clocks for accessing the periphery.
> Add clocks and clock-names properties accordingly.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Changes in v2:
> * None
> 
>  .../devicetree/bindings/media/nxp,imx8-jpeg.yaml          | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> index 3d9d1db37040..2533e16720f2 100644
> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> @@ -46,6 +46,14 @@ properties:
>      minItems: 2               # Wrapper and 1 slot
>      maxItems: 5               # Wrapper and 4 slots
>  
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: per
> +      - const: ipg

What do "per" and "ipg" mean? I assume "per" is peripheral?

> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 3/3] dt-bindings: media: imx-jpeg: Add clocks property

[Alexander Stein]

Am Montag, 24. Juli 2023, 20:26:15 CEST schrieb Conor Dooley:
> On Mon, Jul 24, 2023 at 02:21:00PM +0200, Alexander Stein wrote:
> > i.MX8 and i.MX8X both use two clocks for accessing the periphery.
> > Add clocks and clock-names properties accordingly.
> > 
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > Changes in v2:
> > * None
> > 
> >  .../devicetree/bindings/media/nxp,imx8-jpeg.yaml          | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> > b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml index
> > 3d9d1db37040..2533e16720f2 100644
> > --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> > +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> > 
> > @@ -46,6 +46,14 @@ properties:
> >      minItems: 2               # Wrapper and 1 slot
> >      maxItems: 5               # Wrapper and 4 slots
> > 
> > +  clocks:
> > +    maxItems: 2
> > +
> > +  clock-names:
> > +    items:
> > +      - const: per
> > +      - const: ipg
> 
> What do "per" and "ipg" mean? I assume "per" is peripheral?

Actually I don't know what "ipg" stands for. It's a quite common name on i.MX 
platforms though. I opted for the names currently used in the DT. The driver 
doesn't care for the names currently.
But cross-checking the reference manual these clocks seems to be called "jpeg" 
and "ips", individually for both jpeg encoder and decoder.
Mirela (added to recipients): As the original author of the DT nodes, could 
you provide additional information regarding the clock names?

Best regards,
Alexander

> > +
> > 
> >  required:
> >    - compatible
> >    - reg


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH v2 3/3] dt-bindings: media: imx-jpeg: Add clocks property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2059 bytes --]

On Tue, Jul 25, 2023 at 07:31:55AM +0200, Alexander Stein wrote:
> Am Montag, 24. Juli 2023, 20:26:15 CEST schrieb Conor Dooley:
> > On Mon, Jul 24, 2023 at 02:21:00PM +0200, Alexander Stein wrote:
> > > i.MX8 and i.MX8X both use two clocks for accessing the periphery.
> > > Add clocks and clock-names properties accordingly.
> > > 
> > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > ---
> > > Changes in v2:
> > > * None
> > > 
> > >  .../devicetree/bindings/media/nxp,imx8-jpeg.yaml          | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> > > b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml index
> > > 3d9d1db37040..2533e16720f2 100644
> > > --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> > > +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> > > 
> > > @@ -46,6 +46,14 @@ properties:
> > >      minItems: 2               # Wrapper and 1 slot
> > >      maxItems: 5               # Wrapper and 4 slots
> > > 
> > > +  clocks:
> > > +    maxItems: 2
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: per
> > > +      - const: ipg
> > 
> > What do "per" and "ipg" mean? I assume "per" is peripheral?
> 
> Actually I don't know what "ipg" stands for. It's a quite common name on i.MX 
> platforms though. I opted for the names currently used in the DT. The driver 
> doesn't care for the names currently.

FWIW, my motivation was wondering how someone would know which clock to
put in which.

> But cross-checking the reference manual these clocks seems to be called "jpeg" 
> and "ips", individually for both jpeg encoder and decoder.

Hm, that seems confusing TBH. The reference manual is where I would be
going to try and figure out the numbers.

> Mirela (added to recipients): As the original author of the DT nodes, could 
> you provide additional information regarding the clock names?

That'd be great, thanks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 3/3] dt-bindings: media: imx-jpeg: Add clocks property

[Rob Herring]

On Tue, Jul 25, 2023 at 07:31:55AM +0200, Alexander Stein wrote:
> Am Montag, 24. Juli 2023, 20:26:15 CEST schrieb Conor Dooley:
> > On Mon, Jul 24, 2023 at 02:21:00PM +0200, Alexander Stein wrote:
> > > i.MX8 and i.MX8X both use two clocks for accessing the periphery.
> > > Add clocks and clock-names properties accordingly.
> > > 
> > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > ---
> > > Changes in v2:
> > > * None
> > > 
> > >  .../devicetree/bindings/media/nxp,imx8-jpeg.yaml          | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> > > b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml index
> > > 3d9d1db37040..2533e16720f2 100644
> > > --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> > > +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> > > 
> > > @@ -46,6 +46,14 @@ properties:
> > >      minItems: 2               # Wrapper and 1 slot
> > >      maxItems: 5               # Wrapper and 4 slots
> > > 
> > > +  clocks:
> > > +    maxItems: 2
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: per
> > > +      - const: ipg
> > 
> > What do "per" and "ipg" mean? I assume "per" is peripheral?
> 
> Actually I don't know what "ipg" stands for. It's a quite common name on i.MX 
> platforms though. I opted for the names currently used in the DT. The driver 
> doesn't care for the names currently.

Those names date back about 25 years to Motorola Mcore GSM SoCs. IPG 
came from IPG bus which IIRC stood for IP gasket. Essentially the bus 
was something like Arm APB being slave only. The IPG clock is 
essentially the bus and register access clock. 'per' is the functional 
clock in cases that need a defined clock rate such as UART baud clock. 

There is also a shared (between CPU and DSP) bus called SPBA from the 
same time which still lives on even though it isn't shared in i.MX 
chips.

> But cross-checking the reference manual these clocks seems to be called "jpeg" 
> and "ips", individually for both jpeg encoder and decoder.

Given this block is probably licensed IP, seems like it would use 
something different and be directly connected to AHB or AXI.

> Mirela (added to recipients): As the original author of the DT nodes, could 
> you provide additional information regarding the clock names?
> 
> Best regards,
> Alexander

RE: Re: [PATCH v2 3/3] dt-bindings: media: imx-jpeg: Add clocks property

[Mirela Rabulea]

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Wednesday, July 26, 2023 8:02 PM
> To: Alexander Stein <alexander.stein@ew.tq-group.com>
> Cc: Conor Dooley <conor@kernel.org>; Mirela Rabulea
> <mirela.rabulea@nxp.com>; Ming Qian <ming.qian@nxp.com>; Shijie Qin
> <shijie.qin@nxp.com>; Eagle Zhou <eagle.zhou@nxp.com>; Mauro Carvalho
> Chehab <mchehab@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>;
> Shawn Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Fabio Estevam <festevam@gmail.com>; Mark Brown <broonie@kernel.org>;
> Anson Huang <Anson.Huang@nxp.com>; dl-linux-imx <linux-imx@nxp.com>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; linux-
> media@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-spi@vger.kernel.org
> Subject: [EXT] Re: [PATCH v2 3/3] dt-bindings: media: imx-jpeg: Add clocks
> property
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
> 
> 
> On Tue, Jul 25, 2023 at 07:31:55AM +0200, Alexander Stein wrote:
> > Am Montag, 24. Juli 2023, 20:26:15 CEST schrieb Conor Dooley:
> > > On Mon, Jul 24, 2023 at 02:21:00PM +0200, Alexander Stein wrote:
> > > > i.MX8 and i.MX8X both use two clocks for accessing the periphery.
> > > > Add clocks and clock-names properties accordingly.
> > > >
> > > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > > ---
> > > > Changes in v2:
> > > > * None
> > > >
> > > >  .../devicetree/bindings/media/nxp,imx8-jpeg.yaml          | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> > > > b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml index
> > > > 3d9d1db37040..2533e16720f2 100644
> > > > --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> > > >
> > > > @@ -46,6 +46,14 @@ properties:
> > > >      minItems: 2               # Wrapper and 1 slot
> > > >      maxItems: 5               # Wrapper and 4 slots
> > > >
> > > > +  clocks:
> > > > +    maxItems: 2
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: per
> > > > +      - const: ipg
> > >
> > > What do "per" and "ipg" mean? I assume "per" is peripheral?
> >
> > Actually I don't know what "ipg" stands for. It's a quite common name
> > on i.MX platforms though. I opted for the names currently used in the
> > DT. The driver doesn't care for the names currently.

Hi,
Sorry for the late response.
Yes, the driver uses now the clk_bulk functions, so it does not care for the names anymore (in the past it used the per/ipg names to get the clocks).

> 
> Those names date back about 25 years to Motorola Mcore GSM SoCs. IPG came
> from IPG bus which IIRC stood for IP gasket. Essentially the bus was something
> like Arm APB being slave only. The IPG clock is essentially the bus and register
> access clock. 'per' is the functional clock in cases that need a defined clock rate
> such as UART baud clock.
> 
> There is also a shared (between CPU and DSP) bus called SPBA from the same
> time which still lives on even though it isn't shared in i.MX chips.

Unfortunately, I cannot provide an explanation for the IPG acronym, I asked around, will come back if I get an answer.

> 
> > But cross-checking the reference manual these clocks seems to be called
> "jpeg"
> > and "ips", individually for both jpeg encoder and decoder.
> 
> Given this block is probably licensed IP, seems like it would use something
> different and be directly connected to AHB or AXI.

Yes, the Cast JPEG Decoder/Encoder is a licensed core, and it there is also an NXP JPEG Decoder/Encoder Wrapper, which provides the interface for the Cast JPEG Decoder/Encoder. The wrapper also provides AXI DMA engines for fetching Jpeg bitstream from memory and feed it to the Cast Jpeg or for storing the decoded pixel data into system memory through AXI bus. The wrapper also provides APB interface for wrapper and Cast Jpeg register access.

From our hardware team, I got the information that: for jpeg wrapper, it has two clocks(axi and apb), for CAST IP it has one clock(axi, whose clock source is same with wrapper on chip).

> 
> > Mirela (added to recipients): As the original author of the DT nodes,
> > could you provide additional information regarding the clock names?

I understand that "ipg" usually is IP bus clk for register access, but I am not sure. Experimentally, I was not able to get register access unless both clocks were enabled. I'll get back if I get more details.

Regards,
Mirela

> >
> > Best regards,
> > Alexander

Re: [PATCH v2 3/3] dt-bindings: media: imx-jpeg: Add clocks property

[Hans Verkuil]

Rob, Conor,

On 09/08/2023 22:43, Mirela Rabulea wrote:
>> -----Original Message-----
>> From: Rob Herring <robh@kernel.org>
>> Sent: Wednesday, July 26, 2023 8:02 PM
>> To: Alexander Stein <alexander.stein@ew.tq-group.com>
>> Cc: Conor Dooley <conor@kernel.org>; Mirela Rabulea
>> <mirela.rabulea@nxp.com>; Ming Qian <ming.qian@nxp.com>; Shijie Qin
>> <shijie.qin@nxp.com>; Eagle Zhou <eagle.zhou@nxp.com>; Mauro Carvalho
>> Chehab <mchehab@kernel.org>; Krzysztof Kozlowski
>> <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>;
>> Shawn Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
>> Fabio Estevam <festevam@gmail.com>; Mark Brown <broonie@kernel.org>;
>> Anson Huang <Anson.Huang@nxp.com>; dl-linux-imx <linux-imx@nxp.com>;
>> Pengutronix Kernel Team <kernel@pengutronix.de>; linux-
>> media@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org; linux-spi@vger.kernel.org
>> Subject: [EXT] Re: [PATCH v2 3/3] dt-bindings: media: imx-jpeg: Add clocks
>> property
>>
>> Caution: This is an external email. Please take care when clicking links or
>> opening attachments. When in doubt, report the message using the 'Report this
>> email' button
>>
>>
>> On Tue, Jul 25, 2023 at 07:31:55AM +0200, Alexander Stein wrote:
>>> Am Montag, 24. Juli 2023, 20:26:15 CEST schrieb Conor Dooley:
>>>> On Mon, Jul 24, 2023 at 02:21:00PM +0200, Alexander Stein wrote:
>>>>> i.MX8 and i.MX8X both use two clocks for accessing the periphery.
>>>>> Add clocks and clock-names properties accordingly.
>>>>>
>>>>> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>

Is this patch OK or do you want changes?

It's a bit unclear.

Regards,

	Hans

>>>>> ---
>>>>> Changes in v2:
>>>>> * None
>>>>>
>>>>>  .../devicetree/bindings/media/nxp,imx8-jpeg.yaml          | 8 ++++++++
>>>>>  1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>>>> b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml index
>>>>> 3d9d1db37040..2533e16720f2 100644
>>>>> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>>>> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>>>>
>>>>> @@ -46,6 +46,14 @@ properties:
>>>>>      minItems: 2               # Wrapper and 1 slot
>>>>>      maxItems: 5               # Wrapper and 4 slots
>>>>>
>>>>> +  clocks:
>>>>> +    maxItems: 2
>>>>> +
>>>>> +  clock-names:
>>>>> +    items:
>>>>> +      - const: per
>>>>> +      - const: ipg
>>>>
>>>> What do "per" and "ipg" mean? I assume "per" is peripheral?
>>>
>>> Actually I don't know what "ipg" stands for. It's a quite common name
>>> on i.MX platforms though. I opted for the names currently used in the
>>> DT. The driver doesn't care for the names currently.
> 
> Hi,
> Sorry for the late response.
> Yes, the driver uses now the clk_bulk functions, so it does not care for the names anymore (in the past it used the per/ipg names to get the clocks).
> 
>>
>> Those names date back about 25 years to Motorola Mcore GSM SoCs. IPG came
>> from IPG bus which IIRC stood for IP gasket. Essentially the bus was something
>> like Arm APB being slave only. The IPG clock is essentially the bus and register
>> access clock. 'per' is the functional clock in cases that need a defined clock rate
>> such as UART baud clock.
>>
>> There is also a shared (between CPU and DSP) bus called SPBA from the same
>> time which still lives on even though it isn't shared in i.MX chips.
> 
> Unfortunately, I cannot provide an explanation for the IPG acronym, I asked around, will come back if I get an answer.
> 
>>
>>> But cross-checking the reference manual these clocks seems to be called
>> "jpeg"
>>> and "ips", individually for both jpeg encoder and decoder.
>>
>> Given this block is probably licensed IP, seems like it would use something
>> different and be directly connected to AHB or AXI.
> 
> Yes, the Cast JPEG Decoder/Encoder is a licensed core, and it there is also an NXP JPEG Decoder/Encoder Wrapper, which provides the interface for the Cast JPEG Decoder/Encoder. The wrapper also provides AXI DMA engines for fetching Jpeg bitstream from memory and feed it to the Cast Jpeg or for storing the decoded pixel data into system memory through AXI bus. The wrapper also provides APB interface for wrapper and Cast Jpeg register access.
> 
> From our hardware team, I got the information that: for jpeg wrapper, it has two clocks(axi and apb), for CAST IP it has one clock(axi, whose clock source is same with wrapper on chip).
> 
>>
>>> Mirela (added to recipients): As the original author of the DT nodes,
>>> could you provide additional information regarding the clock names?
> 
> I understand that "ipg" usually is IP bus clk for register access, but I am not sure. Experimentally, I was not able to get register access unless both clocks were enabled. I'll get back if I get more details.
> 
> Regards,
> Mirela
> 
>>>
>>> Best regards,
>>> Alexander

Re: [PATCH v2 3/3] dt-bindings: media: imx-jpeg: Add clocks property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2105 bytes --]

On Mon, Oct 02, 2023 at 11:18:56AM +0200, Hans Verkuil wrote:
> Rob, Conor,
> 
> On 09/08/2023 22:43, Mirela Rabulea wrote:
> >> -----Original Message-----
> >> From: Rob Herring <robh@kernel.org>
> >> Sent: Wednesday, July 26, 2023 8:02 PM
> >> To: Alexander Stein <alexander.stein@ew.tq-group.com>
> >> Cc: Conor Dooley <conor@kernel.org>; Mirela Rabulea
> >> <mirela.rabulea@nxp.com>; Ming Qian <ming.qian@nxp.com>; Shijie Qin
> >> <shijie.qin@nxp.com>; Eagle Zhou <eagle.zhou@nxp.com>; Mauro Carvalho
> >> Chehab <mchehab@kernel.org>; Krzysztof Kozlowski
> >> <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>;
> >> Shawn Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> >> Fabio Estevam <festevam@gmail.com>; Mark Brown <broonie@kernel.org>;
> >> Anson Huang <Anson.Huang@nxp.com>; dl-linux-imx <linux-imx@nxp.com>;
> >> Pengutronix Kernel Team <kernel@pengutronix.de>; linux-
> >> media@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> >> kernel@lists.infradead.org; linux-spi@vger.kernel.org
> >> Subject: [EXT] Re: [PATCH v2 3/3] dt-bindings: media: imx-jpeg: Add clocks
> >> property
> >>
> >> Caution: This is an external email. Please take care when clicking links or
> >> opening attachments. When in doubt, report the message using the 'Report this
> >> email' button
> >>
> >>
> >> On Tue, Jul 25, 2023 at 07:31:55AM +0200, Alexander Stein wrote:
> >>> Am Montag, 24. Juli 2023, 20:26:15 CEST schrieb Conor Dooley:
> >>>> On Mon, Jul 24, 2023 at 02:21:00PM +0200, Alexander Stein wrote:
> >>>>> i.MX8 and i.MX8X both use two clocks for accessing the periphery.
> >>>>> Add clocks and clock-names properties accordingly.
> >>>>>
> >>>>> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> 
> Is this patch OK or do you want changes?
> 
> It's a bit unclear.

It's unclear to me too at this point, but I wonder if the names should
just be removed and replaced by descriptions in the clocks property?

One of the responses here mentioned that the names aren't even needed by
hardware.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 3/3] dt-bindings: media: imx-jpeg: Add clocks property

[Hans Verkuil]

On 02/10/2023 13:16, Conor Dooley wrote:
> On Mon, Oct 02, 2023 at 11:18:56AM +0200, Hans Verkuil wrote:
>> Rob, Conor,
>>
>> On 09/08/2023 22:43, Mirela Rabulea wrote:
>>>> -----Original Message-----
>>>> From: Rob Herring <robh@kernel.org>
>>>> Sent: Wednesday, July 26, 2023 8:02 PM
>>>> To: Alexander Stein <alexander.stein@ew.tq-group.com>
>>>> Cc: Conor Dooley <conor@kernel.org>; Mirela Rabulea
>>>> <mirela.rabulea@nxp.com>; Ming Qian <ming.qian@nxp.com>; Shijie Qin
>>>> <shijie.qin@nxp.com>; Eagle Zhou <eagle.zhou@nxp.com>; Mauro Carvalho
>>>> Chehab <mchehab@kernel.org>; Krzysztof Kozlowski
>>>> <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>;
>>>> Shawn Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
>>>> Fabio Estevam <festevam@gmail.com>; Mark Brown <broonie@kernel.org>;
>>>> Anson Huang <Anson.Huang@nxp.com>; dl-linux-imx <linux-imx@nxp.com>;
>>>> Pengutronix Kernel Team <kernel@pengutronix.de>; linux-
>>>> media@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
>>>> kernel@lists.infradead.org; linux-spi@vger.kernel.org
>>>> Subject: [EXT] Re: [PATCH v2 3/3] dt-bindings: media: imx-jpeg: Add clocks
>>>> property
>>>>
>>>> Caution: This is an external email. Please take care when clicking links or
>>>> opening attachments. When in doubt, report the message using the 'Report this
>>>> email' button
>>>>
>>>>
>>>> On Tue, Jul 25, 2023 at 07:31:55AM +0200, Alexander Stein wrote:
>>>>> Am Montag, 24. Juli 2023, 20:26:15 CEST schrieb Conor Dooley:
>>>>>> On Mon, Jul 24, 2023 at 02:21:00PM +0200, Alexander Stein wrote:
>>>>>>> i.MX8 and i.MX8X both use two clocks for accessing the periphery.
>>>>>>> Add clocks and clock-names properties accordingly.
>>>>>>>
>>>>>>> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
>>
>> Is this patch OK or do you want changes?
>>
>> It's a bit unclear.
> 
> It's unclear to me too at this point, but I wonder if the names should
> just be removed and replaced by descriptions in the clocks property?
> 
> One of the responses here mentioned that the names aren't even needed by
> hardware.

Right, I'm marking this as "Obsoleted" based on Mirela's reply. Alexander,
if you believe this is still needed, then please post a v2.

Regards,

	Hans