linux-kernel - Re: [PATCH 2/8] dt-bindings: display: bridge: thc63lvd1024: Add lvds map property

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: <2645564.GDIDDgbfar@avalon>
Date:   Mon, 23 Apr 2018 15:02:59 +0300
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     Jacopo Mondi <jacopo+renesas@...ndi.org>
Cc:     architt@...eaurora.org, a.hajda@...sung.com, airlied@...ux.ie,
        daniel@...ll.ch, peda@...ntia.se,
        linux-renesas-soc@...r.kernel.org, linux-media@...r.kernel.org,
        devicetree@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/8] dt-bindings: display: bridge: thc63lvd1024: Add lvds map property

Hi Jacopo,

Thank you for the patch.

On Thursday, 19 April 2018 12:31:03 EEST Jacopo Mondi wrote:
> The THC63LVD1024 LVDS to RGB bridge supports two different input mapping
> modes, selectable by means of an external pin.
> 
> Describe the LVDS mode map through a newly defined mandatory property in
> device tree bindings.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@...ndi.org>
> ---
>  .../devicetree/bindings/display/bridge/thine,thc63lvd1024.txt          | 3
> +++ 1 file changed, 3 insertions(+)
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> index 37f0c04..0937595 100644
> ---
> a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> +++
> b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> @@ -12,6 +12,8 @@ Required properties:
>  - compatible: Shall be "thine,thc63lvd1024"
>  - vcc-supply: Power supply for TTL output, TTL CLOCKOUT signal, LVDS input,
> PPL and digital circuitry
> +- thine,map: LVDS mapping mode selection signal, pin name "MAP". Shall be
> <1>
> +  for mapping mode 1, <0> for mapping mode 2

That's sounds like an odd mapping. I suppose you have modeled it based on the 
state of the MAP pin instead of the mode number (MAP low means mode 2, MAP 
high means mode 1). To avoid confusing readers I would write it as

- thine,map: level of the MAP pin that selects the LVDS mapping mode. Shall be
  <0> for low level (mapping mode 2) or <1> for high level (mapping mode 1).

Apart from that this patch looks good to me.

Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>

>  Optional properties:
>  - powerdown-gpios: Power down GPIO signal, pin name "/PDWN". Active low
> @@ -36,6 +38,7 @@ Example:
> 
>  		vcc-supply = <&reg_lvds_vcc>;
>  		powerdown-gpios = <&gpio4 15 GPIO_ACTIVE_LOW>;
> +		thine,map = <1>;
> 
>  		ports {
>  			#address-cells = <1>;

-- 
Regards,

Laurent Pinchart