Re: [RFC PATCH v3 03/11] media: dt-bindings: media: Add bindings for bcm2835-unicam

[Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>]

Hi Stefan,

On 02/02/2022 19:33, Stefan Wahren wrote:
> Hi Jean-Michel,
> 
> please drop the first "media:" before dt-bindings.
> 
> Am 02.02.22 um 18:56 schrieb Jean-Michel Hautbois:
>> Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam
>> camera interface. Also add a MAINTAINERS entry for it.
>>
>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
>> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
>> ---
>> Dave: I assumed you were the maintainer for this file, as I based it on the
>> bcm2835-unicam.txt file. Are  you happy to be added directly as the
>> maintainer, or should this be specified as "Raspberry Pi Kernel
>> Maintenance <kernel-list@raspberrypi.com>"
>> ---
>>   .../bindings/media/brcm,bcm2835-unicam.yaml   | 107 ++++++++++++++++++
>>   MAINTAINERS                                   |   7 ++
>>   2 files changed, 114 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
>> new file mode 100644
>> index 000000000000..5bf41a8834fa
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
>> @@ -0,0 +1,107 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Broadcom BCM283x Camera Interface (Unicam)
>> +
>> +maintainers:
>> +  - Dave Stevenson <dave.stevenson@raspberrypi.com>
>> +
>> +description: |-
>> +  The Unicam block on BCM283x SoCs is the receiver for either
>> +  CSI-2 or CCP2 data from image sensors or similar devices.
>> +
>> +  The main platform using this SoC is the Raspberry Pi family of boards.
>> +  On the Pi the VideoCore firmware can also control this hardware block,
>> +  and driving it from two different processors will cause issues.
>> +  To avoid this, the firmware checks the device tree configuration
>> +  during boot. If it finds device tree nodes starting by csi then
>> +  it will stop the firmware accessing the block, and it can then
>> +  safely be used via the device tree binding.
>> +
>> +properties:
>> +  compatible:
>> +    const: brcm,bcm2835-unicam
>> +
>> +  reg:
>> +    maxItems: 2
> I would be nice to have reg-names here similar to the clocks.

Sure, I just don't know what the names are ;-).

>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    items:
>> +      - description: Clock for the camera.
>> +      - description: Clock for the vpu.
>> +
>> +  clock-names:
>> +    items:
>> +      - const: lp
>> +      - const: vpu
>> +
>> +  power-domains:
>> +    items:
>> +      - description: Unicam power domain
>> +
>> +  num-data-lanes:
>> +    items:
>> +      - enum: [ 2, 4 ]
>> +
>> +  port:
>> +    additionalProperties: false
>> +    $ref: /schemas/graph.yaml#/$defs/port-base
>> +
>> +    properties:
>> +      endpoint:
>> +        $ref: /schemas/media/video-interfaces.yaml#
>> +        unevaluatedProperties: false
>> +
>> +        properties:
>> +          data-lanes: true
>> +          link-frequencies: true
>> +
>> +        required:
>> +          - data-lanes
>> +          - link-frequencies
>> +
>> +    required:
>> +      - endpoint
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - clocks
>> +  - clock-names
>> +  - power-domains
>> +  - num-data-lanes
>> +  - port
>> +
>> +additionalProperties: False
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/bcm2835.h>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/power/raspberrypi-power.h>
>> +    csi1: csi@7e801000 {
>> +        compatible = "brcm,bcm2835-unicam";
>> +        reg = <0x7e801000 0x800>,
>> +              <0x7e802004 0x4>;
>> +        interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
>> +        clocks = <&clocks BCM2835_CLOCK_CAM1>,
>> +                 <&firmware_clocks 4>;
>> +        clock-names = "lp", "vpu";
>> +        power-domains = <&power RPI_POWER_DOMAIN_UNICAM1>;
>> +        num-data-lanes = <2>;
>> +        port {
>> +                csi1_ep: endpoint {
>> +                        remote-endpoint = <&imx219_0>;
>> +                        data-lanes = <1 2>;
>> +                        link-frequencies = /bits/ 64 <456000000>;
>> +                };
>> +        };
>> +    };
>> +...
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a0770a861ca4..29344ea86847 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -3670,6 +3670,13 @@ N:	bcm113*
>>   N:	bcm216*
>>   N:	kona
>>   
>> +BROADCOM BCM2835 CAMERA DRIVER
>> +M:	Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
>> +L:	linux-media@vger.kernel.org
>> +S:	Maintained
>> +F:	Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
>> +F:	arch/arm/boot/dts/bcm283x*
>> +
> 
> I suggest to make the MAINTAINERS changes a single separate patch
> instead of small incremental changes.

I can make it a separate patch, indeed.

> 
> Best regards
> 
>>   BROADCOM BCM47XX MIPS ARCHITECTURE
>>   M:	Hauke Mehrtens <hauke@hauke-m.de>
>>   M:	Rafał Miłecki <zajec5@gmail.com>
>