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

4 messages, 3 authors, 2023-10-05 · open the first message on its own page

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

From: Mirela Rabulea <mirela.rabulea@nxp.com>
Date: 2023-08-09 20:43:14

-----Original Message-----
From: Rob Herring <robh@kernel.org>
Sent: Wednesday, July 26, 2023 8:02 PM
To: Alexander Stein <redacted>
Cc: Conor Dooley <conor@kernel.org>; Mirela Rabulea
[off-list ref]; Ming Qian [off-list ref]; Shijie Qin
[off-list ref]; Eagle Zhou [off-list ref]; Mauro Carvalho
Chehab [off-list ref]; Krzysztof Kozlowski
[off-list ref]; Conor Dooley [off-list ref];
Shawn Guo [off-list ref]; Sascha Hauer [off-list ref];
Fabio Estevam [off-list ref]; Mark Brown [off-list ref];
Anson Huang [off-list ref]; dl-linux-imx [off-list ref];
Pengutronix Kernel Team [off-list ref]; 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:
quoted
Am Montag, 24. Juli 2023, 20:26:15 CEST schrieb Conor Dooley:
quoted
On Mon, Jul 24, 2023 at 02:21:00PM +0200, Alexander Stein wrote:
quoted
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 <redacted>
---
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.
quoted
But cross-checking the reference manual these clocks seems to be called
"jpeg"
quoted
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).
quoted
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
quoted
Best regards,
Alexander
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

From: Hans Verkuil <hidden>
Date: 2023-10-02 09:19:02

Rob, Conor,

On 09/08/2023 22:43, Mirela Rabulea wrote:
quoted
-----Original Message-----
From: Rob Herring <robh@kernel.org>
Sent: Wednesday, July 26, 2023 8:02 PM
To: Alexander Stein <redacted>
Cc: Conor Dooley <conor@kernel.org>; Mirela Rabulea
[off-list ref]; Ming Qian [off-list ref]; Shijie Qin
[off-list ref]; Eagle Zhou [off-list ref]; Mauro Carvalho
Chehab [off-list ref]; Krzysztof Kozlowski
[off-list ref]; Conor Dooley [off-list ref];
Shawn Guo [off-list ref]; Sascha Hauer [off-list ref];
Fabio Estevam [off-list ref]; Mark Brown [off-list ref];
Anson Huang [off-list ref]; dl-linux-imx [off-list ref];
Pengutronix Kernel Team [off-list ref]; 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:
quoted
Am Montag, 24. Juli 2023, 20:26:15 CEST schrieb Conor Dooley:
quoted
On Mon, Jul 24, 2023 at 02:21:00PM +0200, Alexander Stein wrote:
quoted
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 <redacted>
Is this patch OK or do you want changes?

It's a bit unclear.

Regards,

	Hans
quoted
quoted
quoted
quoted
---
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).
quoted
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.
quoted
quoted
But cross-checking the reference manual these clocks seems to be called
"jpeg"
quoted
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).
quoted
quoted
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
quoted
quoted
Best regards,
Alexander

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

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

From: Conor Dooley <conor@kernel.org>
Date: 2023-10-02 11:16:17

On Mon, Oct 02, 2023 at 11:18:56AM +0200, Hans Verkuil wrote:
Rob, Conor,

On 09/08/2023 22:43, Mirela Rabulea wrote:
quoted
quoted
-----Original Message-----
From: Rob Herring <robh@kernel.org>
Sent: Wednesday, July 26, 2023 8:02 PM
To: Alexander Stein <redacted>
Cc: Conor Dooley <conor@kernel.org>; Mirela Rabulea
[off-list ref]; Ming Qian [off-list ref]; Shijie Qin
[off-list ref]; Eagle Zhou [off-list ref]; Mauro Carvalho
Chehab [off-list ref]; Krzysztof Kozlowski
[off-list ref]; Conor Dooley [off-list ref];
Shawn Guo [off-list ref]; Sascha Hauer [off-list ref];
Fabio Estevam [off-list ref]; Mark Brown [off-list ref];
Anson Huang [off-list ref]; dl-linux-imx [off-list ref];
Pengutronix Kernel Team [off-list ref]; 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:
quoted
Am Montag, 24. Juli 2023, 20:26:15 CEST schrieb Conor Dooley:
quoted
On Mon, Jul 24, 2023 at 02:21:00PM +0200, Alexander Stein wrote:
quoted
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 <redacted>
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.

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

From: Hans Verkuil <hidden>
Date: 2023-10-05 08:19:08

On 02/10/2023 13:16, Conor Dooley wrote:
On Mon, Oct 02, 2023 at 11:18:56AM +0200, Hans Verkuil wrote:
quoted
Rob, Conor,

On 09/08/2023 22:43, Mirela Rabulea wrote:
quoted
quoted
-----Original Message-----
From: Rob Herring <robh@kernel.org>
Sent: Wednesday, July 26, 2023 8:02 PM
To: Alexander Stein <redacted>
Cc: Conor Dooley <conor@kernel.org>; Mirela Rabulea
[off-list ref]; Ming Qian [off-list ref]; Shijie Qin
[off-list ref]; Eagle Zhou [off-list ref]; Mauro Carvalho
Chehab [off-list ref]; Krzysztof Kozlowski
[off-list ref]; Conor Dooley [off-list ref];
Shawn Guo [off-list ref]; Sascha Hauer [off-list ref];
Fabio Estevam [off-list ref]; Mark Brown [off-list ref];
Anson Huang [off-list ref]; dl-linux-imx [off-list ref];
Pengutronix Kernel Team [off-list ref]; 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:
quoted
Am Montag, 24. Juli 2023, 20:26:15 CEST schrieb Conor Dooley:
quoted
On Mon, Jul 24, 2023 at 02:21:00PM +0200, Alexander Stein wrote:
quoted
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 <redacted>
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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help