[PATCH v12 03/15] dt-bindings: media: Add bindings for ARM mali-c55
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Sun Nov 9 01:01:51 PST 2025
Hi Dan
On Sat, Nov 08, 2025 at 03:16:05PM +0000, Dan Scally wrote:
> Hi Jacopo, Prabhakar
>
> On 08/11/2025 13:07, Jacopo Mondi wrote:
> > Hi
> >
> > On Wed, Nov 05, 2025 at 01:35:59PM +0000, Lad, Prabhakar wrote:
> > > Hi Dan,
> > >
> > > On Mon, Nov 3, 2025 at 4:17 PM Dan Scally <dan.scally at ideasonboard.com> 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/
> > > >
> > > Thanks for the info.
> >
> > I won't question the Mali clock assignment as I don't have the
> > documentation you mentioned.
> >
> > But looking at the patch you shared
> >
> > IVC:
> > + clocks = <&cpg CPG_MOD 0xe3>,
> > + <&cpg CPG_MOD 0xe4>,
> > + <&cpg CPG_MOD 0xe5>;
> > + clock-names = "reg", "axi", "isp";
> >
> > Mali:
> > + clocks = <&cpg CPG_MOD 0xe2>,
> > + <&cpg CPG_MOD 0xe4>,
> > + <&cpg CPG_MOD 0xe5>;
> > + clock-names = "vclk", "aclk", "hclk";
> >
> > It seems the IVC-only clock is
> >
> > <cpg CPG_MOD 0xe3> "reg"
> >
> > trying to match the clocks here with the V2H documentation and the
> > above names
> >
> > clk IVC Mali RZ V2H Doc
> > 0xe2 vclk vin_aclk Video input data AXI bus clock
> > 0xe3 reg pclk Input Video Control block register access APB clock
> > 0xe4 axi aclk reg_aclk AXI to AHB bus bridge AXI slave cloc
> > 0xe5 isp hclk isp_sclk ISP system clock (pixel clock)
>
> No I think it's:
>
> clk IVC ISP V2H doc Desc
> 0xe2 vclk reg_aclk AXI to AHB bus bridge AXI slave clock
> 0xe3 reg pclk Input Video Control Block register access APB clock
> 0xe4 axi aclk vin_aclk Video input data AXI bus clock
> 0xe5 isp hclk isp_sclk ISP system clock (pixel clock)
I failed to find a proper clock description in the RZ V2H doc (the ISP
clocks are blank in table 4.4-2 List of Clock Signals (11/16))
Is there another version ? Not that I doubt your version here, just
for my education..
>
> So the IVC needs 227, 228, 229 and the bindings name them reg, axi and isp.
Makes sense according to the table above
> The ISP needs 226, 228 and 229 and the bindings name them vclk, aclk, hclk.
> If we want to rename the ISP's binding names then 228 and 229 become "axi"
> and "isp", and we need a sensible name for the "reg_aclk" from the Renesas
> documentation..."ahb"? or "reg"?
>
>
> >
> > I would only question if the IVC shouldn't actually only get its reg
> > clock as the other 2 are mandatory for the ISP even when integrated
> > inline without an IVC.
>
> In my mind, given that we're treating them as separate devices, they ought
> to be fully (if worthlessly) functional independently. If the IVC gets only
> its reg clock then it will not stream until you power on the ISP separately,
> which does not seem right to me.>
> > I guess the question is if there are other IVC instances not paired
> > with a Mali ISP in other SoCs ?
>
> Good question, I too would be interested to know.
>
As we don't know, one more reason to describe the IVC as a stand-alone
fully-functional device ?
>
> > >
> > > > > 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".
> >
> > Let alone I find 'reg' better, but if the documentation uses 'pclk'
> > why has 'reg' been used ?
>
> I used the documentation names for the ISP, but Krzysztof later convinced me
> that following the documentation names isn't the best approach given it
> doesn't really tell us anything. The "reg" was because the docs describe
> that one as the register access clock.
Ok, I guessed it was about using generic names. Makes sense..
With the last issues from Prabhakar addressed
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
Thanks
j
>
> Thanks
> Dan
>
> >
> > Thanks
> > j
> >
> > > >
> > > Got you.
> > >
> > > Cheers,
> > > Prabhakar
> > >
>
More information about the linux-arm-kernel
mailing list