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

Jacopo Mondi jacopo.mondi at ideasonboard.com
Sat Nov 8 05:14:32 PST 2025


Hi Dan

On Mon, Nov 03, 2025 at 04:17:24PM +0000, Dan Scally wrote:
> 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?
> >

Have you missed this question from Prabhakar ? What do you think ?

> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    mali_c55: isp at 400000 {
> > we could drop `mali_c55`
> >

I think we should

> > > +      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.

With the right include you could even use the actual value from the
dts:
                interrupts = <GIC_SPI 859 IRQ_TYPE_LEVEL_HIGH>;

Please make sure to run dt_bindings_check if you do so

With a few questions clarified, if you resend and drop the label here
(you can even resend this single patch as v12.1 if it's more
convenient) I think this driver should be collected for v6.19.

(Do you plan a v6 of the IVC with the small comments from Prabhakar
addressed ? Ideally we want it to get in at the same time)

Thanks
  j

> >
> > Cheers,
> > Prabhakar
> >
> > > +
> > > +      port {
> > > +        isp_in: endpoint {
> > > +            remote-endpoint = <&csi2_rx_out>;
> > > +        };
> > > +      };
> > > +    };
> > > +...
> > >
> > > --
> > > 2.43.0
> > >
> > >
>
>



More information about the linux-arm-kernel mailing list