Re: [PATCH v4 2/5] dt-bindings: media: Document BCM283x CSI2/CCP2 receiver

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

 



Hi Sakari,

On Wed, Dec 02, 2020 at 11:20:31PM +0200, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Tue, Nov 10, 2020 at 06:40:33PM +0100, Jacopo Mondi wrote:
> > From: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> >
> > Document the DT bindings for the CSI2/CCP2 receiver peripheral (known as
> > Unicam) on BCM283x SoCs.
> >
> > Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
> > ---
> >  .../bindings/media/brcm,bcm2835-unicam.yaml   | 155 ++++++++++++++++++
> >  1 file changed, 155 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 0000000000000..6ffc900e8ae8f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
> > @@ -0,0 +1,155 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (C) 2020 Raspberry Pi (Trading) Ltd.
> > +%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@xxxxxxxxxxxxxxx>
> > +  - Raspberry Pi kernel list <kernel-list@xxxxxxxxxxxxxxx>
> > +
> > +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 whose name starts with "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
>
> The compatible string doesn't quite match with the title. Which SoCs are
> supported?
>

Afaict bcm2835 and bcm2711. There might be more I'm not aware of.
There should be no distinction between then, but Dave knows more.

Would a per-soc compatible work better or is a single brcm,unicam
good enough to start with and we can add per-soc compatibles if
there's any per-SoC difference that will need to be handled in future ?

> > +
> > +  reg:
> > +    items:
> > +      - description: Main registers block
> > +      - description: Clock registers block
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    minItems: 1
> > +    maxItems: 2
> > +
> > +  clock-names:
> > +    minItems: 1
> > +    items:
> > +      - const: lp
> > +      - const: core
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  brcm,num-data-lanes:
> > +    description:
> > +      The number of data lanes supported by this Unicam instance. It may be
> > +      larger than the number of data lanes routed on the board, as described by
> > +      the data-lanes property of the endpoint.
> > +    allOf:
> > +      - $ref: "/schemas/types.yaml#/definitions/uint32"
> > +      - enum: [1, 2, 4]
>
> Do you need this information besides how many lanes are connected?
>
> Does the number of lanes change even within the same model of SoC? Could
> you use the compatible string to differentiate between them?
>

I had a similar question:
https://patchwork.linuxtv.org/project/linux-media/patch/20200504092611.9798-5-laurent.pinchart@xxxxxxxxxxxxxxxx/#121079

I think the discussion ended with the fact that not all the available
lanes in the IP might be routed to the connector, and that depends on
the board.

> > +
> > +  port:
> > +    type: object
> > +    description:
> > +      Input port node, as described in video-interfaces.txt.
> > +
> > +    properties:
> > +      endpoint:
> > +        type: object
> > +
> > +        properties:
> > +          clock-lanes:
> > +            items:
> > +              - const: 0
>
> Please drop. There's nothing to tell software here.
>

Ack

> > +
> > +          data-lanes:
> > +            description:
> > +              Lane reordering is not supported, items shall be in order,
> > +              starting at 1.
> > +            allOf:
> > +              - $ref: "/schemas/types.yaml#/definitions/uint32-array"
> > +              - maxItems: 1
> > +                items:
> > +                  minItems: 1
> > +                  maxItems: 4
> > +                  items:
> > +                    - const: 1
> > +                    - const: 2
> > +                    - const: 3
> > +                    - const: 4
>
> No need to specify items.
>

Actually here we might need to support [1, 2] and [1, 2, 3, 4].
I'll rebase on the converted video-interfaces to see it that helps

> > +
> > +          lane-polarities:
> > +            description:
> > +              Lane inversion is not supported. If the property is specified, it
> > +              shall contain all 0's.
>
> Ditto.
>

Ack

> > +            allOf:
> > +              - $ref: "/schemas/types.yaml#/definitions/uint32-array"
> > +              - maxItems: 1
> > +                items:
> > +                  minItems: 2
> > +                  maxItems: 5
> > +                  items:
> > +                    - const: 0
> > +                    - const: 0
> > +                    - const: 0
> > +                    - const: 0
> > +                    - const: 0
> > +
> > +          remote-endpoint: true
> > +
> > +        required:
> > +          - data-lanes
> > +          - remote-endpoint
> > +
> > +        additionalProperties: false
> > +
> > +    required:
> > +      - endpoint
> > +
> > +    additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - power-domains
> > +  - brcm,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>
> > +
> > +    csi@7e801000 {
> > +        compatible = "brcm,bcm2835-unicam";
> > +        reg = <0x7e801000 0x800>,
> > +              <0x7e802004 0x4>;
> > +        interrupts = <2 7>;
> > +        clocks = <&clocks BCM2835_CLOCK_CAM1>;
> > +        clock-names = "lp";
> > +        power-domains = <&power RPI_POWER_DOMAIN_UNICAM1>;
> > +        brcm,num-data-lanes = <4>;
> > +
> > +        port {
> > +            csi1_ep: endpoint {
> > +                remote-endpoint = <&imx219_0>;
> > +                data-lanes = <1 2>;
> > +            };
> > +        };
> > +    };
> > +...
>
> --
> Sakari Ailus



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux