Hi Krzysztof On Mon, Feb 12, 2024 at 09:12:11AM +0100, Krzysztof Kozlowski wrote: > On 09/02/2024 17:48, 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 > > > > > > +--- > > +$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: > > + - Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> > > + - David Plowman <david.plowman@xxxxxxxxxxxxxxx> > > + - Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx> > > + - Naushir Patuck <naush@xxxxxxxxxxxxxxx> > > + - Nick Hollinghurst <nick.hollinghurst@xxxxxxxxxxxxxxx> > > I assume all folks are fine being here? > I admint I haven't ask any single one of them, and I listed all of them thinking of "maintainers of PiSP", but when it comes to bindings only we can shorten the list if preferred ? > > + > > +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 applications. > > + > > + The full ISP documentation is available at: > > + https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf > > + > > +properties: > > + compatible: > > + const: raspberrypi,pispbe > > Nothing more specific? No model name, no version? It's quite generic > compatible which in general should not be allowed. I would assume that > at least version of Pi could denote some sort of a model... unless > version is detectable? > The driver matches on a version register and that should be enough to handle quirks which are specific to an IP revision in the driver itself. Considering how minimal the integration with the SoC is (one clock, one interrupt and one optional iommu reference) even if we'll get future revisions of the SoC I don't think there will be any need to match on a dedicated compatible for bindings-validation purposes. However I understand that to be future-proof it's good practice to allow a more flexible scheme, so we can have a generic fallback and a revision-specific entry. Would compatible: items: - enum: - raspberrypi,pipspbe-bcm2712 - const: raspberrypi,pispbe do in this case ? Also, let's see what RPi think as they are certainly more informed than me on what a good revision-specific match could be > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + clocks: > > + maxItems: 1 > > + > > + clock-names: > > + const: isp_be > > You can skip clock-names if they have only one entry and it matches > block name. Quite useless. > ack > > + > > + iommus: > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - clocks > > + - clock-names > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + > > + rpi1 { > > soc { > Are you sure ? This will only ever live in the 'rp1' node. > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + isp: pisp_be@880000 { > > Node names should be generic. See also an explanation and list of > examples (not exhaustive) in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > so: isp@ > > and drop unused label ok Thanks j PS: on v6.8-rc1 I'm seeing LINT Documentation/devicetree/bindings usage: yamllint [-h] [-] [-c CONFIG_FILE | -d CONFIG_DATA] [--list-files] [-f {parsable,standard,colored,github,auto}] [-s] [--no-warnings] [-v] [FILE_OR_DIR ...] when running dt_binding_check My setup should be reasonably up-to-date, is it me only seeing this ? > > > + compatible = "raspberrypi,pispbe"; > ds, > Krzysztof >
