[RFC 1/2] [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver

[Dave Stevenson]

Hi Stefan.
Thanks for taking the time to review this.

On 15 June 2017 at 07:34, Stefan Wahren <stefan.wahren at i2se.com> wrote:
> Hi Dave,
>
> Am 14.06.2017 um 17:15 schrieb Dave Stevenson:
>> Document the DT bindings for the CSI2/CCP2 receiver peripheral
>> (known as Unicam) on BCM283x SoCs.
>>
>> Signed-off-by: Dave Stevenson <dave.stevenson at raspberrypi.org>
>
> please add the devicetree guys in CC for the binding.

Will do for V2.

>> ---
>>  .../devicetree/bindings/media/bcm2835-unicam.txt   | 76 ++++++++++++++++++++++
>>  1 file changed, 76 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/bcm2835-unicam.txt b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>> new file mode 100644
>> index 0000000..cc5a451
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>> @@ -0,0 +1,76 @@
>> +Broadcom BCM283x Camera Interface (Unicam)
>> +------------------------------------------
>> +
>> +The Unicam block on BCM283x SoCs is the receiver for either
>> +CSI-2 or CCP2 data from image sensors or similar devices.
>
> It would be nice to add some of your explanations to Hans in this
> document or into the driver.

Will do.

>> +
>> +Required properties:
>> +===================
>> +- compatible : must be "brcm,bcm2835-unicam".
>> +- reg                : physical base address and length of the register sets for the
>> +               device.
>> +- interrupts : should contain the IRQ line for this Unicam instance.
>> +- clocks     : list of clock specifiers, corresponding to entries in
>> +               clock-names property.
>> +- clock-names        : must contain an "lp_clock" entry, matching entries
>> +               in the clocks property.
>> +
>> +Optional properties
>> +===================
>> +- max-data-lanes: the hardware can support varying numbers of clock lanes.
>> +               This value is the maximum number supported by this instance.
>> +               Known values of 2 or 4. Default is 2.
>
> AFAIK, this isn't a common property yet. So possibly a vendor prefix
> must be added.

I think yoiu are correct that it isn't a common property. I was
wanting to cover the situation on the majority of the Pi boards where
Unicam1 has been used which can handle 4 lanes, but only 2 have been
broken out to the camera connector.
Happy to add a vendor prefix in V2.

>> +
>> +
>> +Unicam supports a single port node. It should contain one 'port' child node
>> +with child 'endpoint' node. Please refer to the bindings defined in
>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>> +
>> +Example:
>> +     csi1: csi at 7e801000 {
>> +             compatible = "brcm,bcm2835-unicam";
>> +             reg = <0x7e801000 0x800>,
>> +                   <0x7e802004 0x4>;
>> +             interrupts = <2 7>;
>> +             clocks = <&clocks BCM2835_CLOCK_CAM1>;
>> +             clock-names = "lp_clock";
>> +
>> +             port {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +
>> +                     endpoint {
>> +                             remote-endpoint = <&tc358743_0>;
>> +
>> +                     };
>> +             };
>> +     };
>> +
>> +     i2c0: i2c at 7e205000 {
>> +
>> +             tc358743: tc358743 at 0f {
>
> Usually the node name should describe the function of the node for example:
>
> tc358743: csi-hdmi-bridge at 0f

Will do.

> Best regards
> Stefan
>
>> +                     compatible = "toshiba,tc358743";
>> +                     reg = <0x0f>;
>> +                     status = "okay";
>> +
>> +                     clocks = <&tc358743_clk>;
>> +                     clock-names = "refclk";
>> +
>> +                     tc358743_clk: bridge-clk {
>> +                             compatible = "fixed-clock";
>> +                             #clock-cells = <0>;
>> +                             clock-frequency = <27000000>;
>> +                     };
>> +
>> +                     port {
>> +                             tc358743_0: endpoint {
>> +                                     remote-endpoint = <&csi1>;
>> +                                     clock-lanes = <0>;
>> +                                     data-lanes = <1 2 3 4>;
>> +                                     clock-noncontinuous;
>> +                                     link-frequencies =
>> +                                             /bits/ 64 <297000000>;
>> +                             };
>> +                     };
>> +             };
>> +     };

Thanks
  Dave