Hi Laurent On Fri, Mar 01, 2024 at 11:58:57PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Fri, Feb 23, 2024 at 05:30:09PM +0100, Jacopo Mondi wrote: > > Add bindings for the Raspberry Pi PiSP Back End memory-to-memory image > > signal processor. > > > > Datasheet: > > https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx> > > --- > > .../bindings/media/raspberrypi,pispbe.yaml | 63 +++++++++++++++++++ > > 1 file changed, 63 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/raspberrypi,pispbe.yaml > > > > diff --git a/Documentation/devicetree/bindings/media/raspberrypi,pispbe.yaml b/Documentation/devicetree/bindings/media/raspberrypi,pispbe.yaml > > new file mode 100644 > > index 000000000000..d7839f32eabf > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/raspberrypi,pispbe.yaml > > @@ -0,0 +1,63 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/media/raspberrypi,pispbe.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Raspberry Pi PiSP Image Signal Processor (ISP) Back End > > + > > +maintainers: > > + - Raspberry Pi Kernel Maintenance <kernel-list@xxxxxxxxxxxxxxx> > > + - Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx> > > + > > +description: | > > + The Raspberry Pi PiSP Image Signal Processor (ISP) Back End is an image > > + processor that fetches images in Bayer or Grayscale format from DRAM memory > > + in tiles and produces images consumable by application. > > s/application/applications/ > > > + > > + The full ISP documentation is available at: > > s/:$// > > > + https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf > > + > > +properties: > > + compatible: > > + items: > > + - enum: > > + - brcm,bcm2712-pispbe > > + - const: raspberrypi,pispbe > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + clocks: > > + maxItems: 1 > > As this is a SoC IP with only memory and register interfaces, I would > expect two clocks to be present, one for the register interface (AHB ? > AXI4-Lite ?) and one for the memory interfaces (AXI4 ?). While the > register interface clock is likely always enabled (in all cases that > matter in practice) in the BCM2712, I'm not sure this can be guaranteed > for future integration in different SoCs. Should we plan for this, and > either define two clocks already (with one of them being optional), or > name the single clock ? > > I know v1 named this clock "isp_be", and the name was dropped upon > Krzysztof's request, but I think naming the single clock "axi" or "aclk" > (assuming that one of them would be the right name) would be fine for > the reason explained above. > The PiSP datasheet does not offer many information on the IP integration, only a small graph with the memory interfacing, but no clocks. However your reasoning makes sense, and unless someone from RPi suggests the contrary, I'll do so > > + > > + iommus: > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - clocks > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + > > + soc { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + isp@880000 { > > + compatible = "brcm,bcm2712-pispbe", "raspberrypi,pispbe"; > > + reg = <0x10 0x00880000 0x0 0x4000>; > > Double space, I don't know if that's on purpose. > Ofc it was not. Thanks j > > + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&firmware_clocks 7>; > > + iommus = <&iommu2>; > > + }; > > + }; > > -- > Regards, > > Laurent Pinchart
