On 06/11/2024 14:57, Laurent Pinchart wrote:

Hi Krzysztof,

On Wed, Nov 06, 2024 at 01:15:23PM +0100, Krzysztof Kozlowski wrote:

quoted

On Wed, Nov 06, 2024 at 10:05:22AM +0000, Daniel Scally wrote:

quoted

Add the yaml binding for ARM's Mali-C55 Image Signal Processor.

Acked-by: Nayden Kanchev <redacted>
Co-developed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v8:

	- Added the video clock back in. Now that we have actual hardware it's
	  clear that it's necessary.
	- Added reset lines 
	- Dropped R-bs

These are trivial, so I wish you kept the review... but since you ask,
then comment further

I recommend using b4, so your cover letter changelog comes with nice
links to previous versions. I scrolled through entire cover letter for
this (for me that's almost the only point of cover letter) and could
not find them. Anyway, just a remark.


...

quoted

+  resets:
+    items:
+      - description: vclk domain reset
+      - description: aclk domain reset
+      - description: hclk domain reset
+
+  reset-names:
+    items:
+      - const: vresetn

drop "reset", it's redundant and rather come here with logical name. I
wonder what "n" means as well. It's not a GPIO to be "inverted"...

The aresetn and hresetn names come directly from a hardware manual
(vresetn seems to be called rstn in that document though). As far as I
understand, they are the names of the external signals of the IP core.
I tend to pick the hardware names for clock and reset names. That makes
it easier for integrators, and from a driver point of view it doesn't
change much as DT names are just a convention anyway.

That being said, if there's a good reason to do otherwise (such as
standardizing property names to make handling through common code
possible), that's fine too.

If these are from manual then it is fine, although sometimes the names
are really pointless in manual...

Reviewed-by: Krzysztof Kozlowski <redacted>

Best regards,
Krzysztof