On Wed, Mar 06, 2024 at 11:42:51AM +0000, Naushir Patuck wrote: > On Tue, 5 Mar 2024 at 15:25, Jacopo Mondi wrote: > > On Fri, Mar 01, 2024 at 11:58:57PM +0200, Laurent Pinchart wrote: > > > 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 > > There is only a single clock that clocks the whole BE block, so does > the clock need to be explicitly named? If it does, perhaps we can > just use "clk" as this is not explicitly an AXI or APB clock? If there's really a single clock then I don't think it needs to be named. I was expecting there would be a clock for the register interface, separate from the processing clock. > > > > + > > > > + 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. > > > > > > + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&firmware_clocks 7>; > > > > + iommus = <&iommu2>; > > > > + }; > > > > + }; -- Regards, Laurent Pinchart
