On 06/11/2024 14:57, Laurent Pinchart wrote: > Hi Krzysztof, > > On Wed, Nov 06, 2024 at 01:15:23PM +0100, Krzysztof Kozlowski wrote: >> On Wed, Nov 06, 2024 at 10:05:22AM +0000, Daniel Scally wrote: >>> Add the yaml binding for ARM's Mali-C55 Image Signal Processor. >>> >>> Acked-by: Nayden Kanchev <nayden.kanchev@xxxxxxx> >>> Co-developed-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx> >>> Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx> >>> Signed-off-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx> >>> --- >>> 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. >> >> >> ... >> >>> + 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 <krzysztof.kozlowski@xxxxxxxxxx> Best regards, Krzysztof
