Re: [PATCH 6/8] media: dt-bindings: Add bindings for Raspberry Pi PiSP Back End

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/02/2024 09:50, Jacopo Mondi wrote:

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

bcm2712 is manufactured by Broadcom, not Raspberry Pi, so it should be
rather Pi model?

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

I am fine with auto-detection, though.

...

>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +
>>> +    rpi1 {
>>
>> soc {
>>
> 
> Are you sure ? This will only ever live in the 'rp1' node.

What is "rp1" node? Does not look like a generic name.


> 
>>> +        #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 ?

I think you need to update your yamllint.

Best regards,
Krzysztof





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux