linux-kernel - Re: [PATCH v3 05/14] media: dt-bindings: marvell,mmp2-ccic: Add Marvell MMP2 camera

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181122200341.GA8279@w540>
Date:   Thu, 22 Nov 2018 21:08:08 +0100
From:   jacopo mondi <jacopo@...ndi.org>
To:     Lubomir Rintel <lkundrak@...sk>
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Jonathan Corbet <corbet@....net>, linux-media@...r.kernel.org,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        James Cameron <quozl@...top.org>, Pavel Machek <pavel@....cz>,
        Libin Yang <lbyang@...vell.com>,
        Albert Wang <twang13@...vell.com>
Subject: Re: [PATCH v3 05/14] media: dt-bindings: marvell,mmp2-ccic: Add
 Marvell MMP2 camera

Hi Lubomir,

On Tue, Nov 20, 2018 at 11:03:10AM +0100, Lubomir Rintel wrote:
> Add Marvell MMP2 camera host interface.
>
> Signed-off-by: Lubomir Rintel <lkundrak@...sk>
>
> ---
> Changes since v2:
> - Added #clock-cells, clock-names, port
>
>  .../bindings/media/marvell,mmp2-ccic.txt      | 37 +++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
>
> diff --git a/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
> new file mode 100644
> index 000000000000..e5e8ca90e7f7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
> @@ -0,0 +1,37 @@
> +Marvell MMP2 camera host interface
> +
> +Required properties:
> + - compatible: Should be "marvell,mmp2-ccic"
> + - reg: register base and size
> + - interrupts: the interrupt number
> + - #clock-cells: must be 0
> + - any required generic properties defined in video-interfaces.txt

I don't think video-interfaces applies here. It described bindings to
be used for endpoint and port nodes.

> +
> +Optional properties:
> + - clocks: input clock (see clock-bindings.txt)

What do you think of:
"reference to the input clock as specified by clock-bindings.txt"

> + - clock-names: names of the clocks used, may include "axi", "func" and
> +                "phy"

"may include" is abit vague. Which clock should the interface be
powered from, and in which case?

> + - clock-output-names: should contain the name of the clock driving the
> +                       sensor master clock MCLK

This is a property for the clock provider part, and I will just list
the only clock this interfaces provides here:

    - clock-output-names: Optional clock source for sensors. Shall be "mclk".

See a comment on patch 14 on the use of the clock provider part.

> +
> +Required subnodes:
> + - port: the parallel bus interface port with a single endpoint linked to
> +         the sensor's endpoint as described in video-interfaces.txt
> +
> +Example:
> +
> +	camera0: camera@...0a000 {
> +		compatible = "marvell,mmp2-ccic";
> +		reg = <0xd420a000 0x800>;
> +		interrupts = <42>;
> +		clocks = <&soc_clocks MMP2_CLK_CCIC0>;
> +		clock-names = "axi";
> +		#clock-cells = <0>;
> +		clock-output-names = "mclk";
> +
> +		port {
> +			camera0_0: endpoint {
> +				remote-endpoint = <&ov7670_0>;

I'm debated, your sensor does not support configuring the parallel bus,
that's fine, but as "bindings describe hardware" shouldn't you list
here the bus configurations the HW interface supports and list their default
values? Or there are none for real in this platform?

Thanks
  j


> +			};
> +		};
> +	};
> --
> 2.19.1
>

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)