[PATCH v12 03/15] dt-bindings: media: Add bindings for ARM mali-c55

[Dan Scally]

Hi Prabhakar

On 28/10/2025 18:23, Lad, Prabhakar wrote:
> Hi Daniel,
> 
> Thank you for the patch.
> 
> On Thu, Oct 2, 2025 at 11:19 AM Daniel Scally
> <dan.scally at ideasonboard.com> wrote:
>>
>> Add the yaml binding for ARM's Mali-C55 Image Signal Processor.
>>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski at linaro.org>
>> Acked-by: Nayden Kanchev <nayden.kanchev at arm.com>
>> Co-developed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
>> ---
>> Changes in v12:
>>
>>          - _Actually_ dropped the arm,inline property mode, having forgotten to
>>            do so in v11.
>>
>> Changes in v11:
>>
>>          - Dropped in arm,inline_mode property. This is now identical to the
>>            reviewed version 8, so I have kept the tags on there.
>>
>> Changes in v10:
>>
>>          - None
>>
>> Changes in v9:
>>
>>          - Added the arm,inline_mode property to differentiate between inline and
>>            memory input configurations
>>
>> 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
>>
>> Changes in v7:
>>
>>          - None
>>
>> Changes in v6:
>>
>>          - None
>>
>> Changes in v5:
>>
>>          - None
>>
>> Changes in v4:
>>
>>          - Switched to port instead of ports
>>
>> Changes in v3:
>>
>>          - Dropped the video clock as suggested by Laurent. I didn't retain it
>>          for the purposes of the refcount since this driver will call .s_stream()
>>          for the sensor driver which will refcount the clock anyway.
>>          - Clarified that the port is a parallel input port rather (Sakari)
>>
>> Changes in v2:
>>
>>          - Added clocks information
>>          - Fixed the warnings raised by Rob
>> ---
>>   .../devicetree/bindings/media/arm,mali-c55.yaml    | 82 ++++++++++++++++++++++
>>   1 file changed, 82 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/arm,mali-c55.yaml b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..efc88fd2c447e98dd82a1fc1bae234147eb967a8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
>> @@ -0,0 +1,82 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/arm,mali-c55.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: ARM Mali-C55 Image Signal Processor
>> +
>> +maintainers:
>> +  - Daniel Scally <dan.scally at ideasonboard.com>
>> +  - Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: arm,mali-c55
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    items:
>> +      - description: ISP Video Clock
>> +      - description: ISP AXI clock
>> +      - description: ISP AHB-lite clock
> As per RZ/V2H HW manual we have reg clock looking at the driver code
> it does have readl. IVC has reg clock if IVC driver fails are you
> still able to read/write regs from ISP driver?
 >
 > I think we do need to pass reg clock too.

Yes - but I should clarify that the names are from the arm documentation that we had when we 
originally developed the ISP driver. The RZ/V2H documentation treats the ISP and IVC as one block 
that shares 4 clocks and resets, but when we originally developed the ISP driver the platform we 
used had the ISP implemented as an inline configuration (taking data directly from a csi-2 receiver 
without an IVC equivalent), and the documentation detailed just the three clocks and resets. The 
dtsi changes for the RZ/V2H(P) [1] assign clocks 226, 228 and 229 to the ISP which are named 
reg_aclk, vin_aclk and isp_sclk in the renesas documentation.

The IVC gets pclk, vin_aclk and isp_sclk.

[1] https://lore.kernel.org/linux-renesas-soc/20251010-kakip_dts-v1-1-64f798ad43c9@ideasonboard.com/

> Also for IVC we do have a main clock (which is a system clock).  Can
> you please educate me on what is the purpose of it. Just curious as we
> pass to IVC and not ISP.

The IVC uniquely gets the one called "pclk" in renesas documentation, with the description "Input 
Video Control block register access APB clock".

Thanks
Dan

> 
>> +
>> +  clock-names:
>> +    items:
>> +      - const: vclk
>> +      - const: aclk
>> +      - const: hclk
> Not sure if we want to have the same names as IVC or vice versa.
> 
>> +
>> +  resets:
>> +    items:
>> +      - description: vclk domain reset
>> +      - description: aclk domain reset
>> +      - description: hclk domain reset
> Same query here, wrt register reset.
> 
>> +
>> +  reset-names:
>> +    items:
>> +      - const: vresetn
>> +      - const: aresetn
>> +      - const: hresetn
> ditto naming.
> 
>> +
>> +  port:
>> +    $ref: /schemas/graph.yaml#/properties/port
>> +    description: Input parallel video bus
>> +
>> +    properties:
>> +      endpoint:
>> +        $ref: /schemas/graph.yaml#/properties/endpoint
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - clocks
>> +  - clock-names
>> +  - port
> maybe also resets and rest-names should be part of required properties?
> 
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    mali_c55: isp at 400000 {
> we could drop `mali_c55`
> 
>> +      compatible = "arm,mali-c55";
>> +      reg = <0x400000 0x200000>;
>> +      clocks = <&clk 0>, <&clk 1>, <&clk 2>;
>> +      clock-names = "vclk", "aclk", "hclk";
>> +      resets = <&resets 0>, <&resets 1>, <&resets 2>;
>> +      reset-names = "vresetn", "aresetn", "hresetn";
>> +      interrupts = <0>;
> I would have a non-zero val here.
> 
> Cheers,
> Prabhakar
> 
>> +
>> +      port {
>> +        isp_in: endpoint {
>> +            remote-endpoint = <&csi2_rx_out>;
>> +        };
>> +      };
>> +    };
>> +...
>>
>> --
>> 2.43.0
>>
>>