Re: [PATCH v9 2/2] dt-bindings: Add Truly NT35597 panel driver bindings

[Abhinav Kumar <abhinavk@codeaurora.org>]

Hi Rob

Had one question on below comment. Can you please help to clarify?

Thanks

Abhinav
On 2018-10-04 12:42, Abhinav Kumar wrote:
> Hi Rob
> 
> Thanks for the review. Will copy the DT list in the next patchset.
> 
> Some comments inline.
> 
> Thanks
> 
> Abhinav
> 
> On 2018-10-04 12:01, Rob Herring wrote:
>> If you want DT bindings reviewed, you have to cc the DT list. (Or wait
>> for Sean Paul to ping me on IRC)
>> 
>> On Fri, Sep 28, 2018 at 7:39 PM Abhinav Kumar 
>> <abhinavk@codeaurora.org> wrote:
>>> 
>>> Add the device tree bindings for Truly NT35597 panel driver. This 
>>> panel
>>> driver supports both single DSI and dual DSI.
>> 
>> By driver, you means drives panel timing signals or a Linux driver?
>> The former is okay, the latter is not.
> Yes, I mean it driver panel timing signals.
>> 
>>> 
>>> However, this patch series supports only dual DSI.
>>> 
>>> Changes in v9:
>>>   - None
>>> 
>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>> Reviewed-by: Sean Paul <seanpaul@chromium.org>
>>> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
>>> ---
>>>  .../devicetree/bindings/display/truly,nt35597.txt  | 60 
>>> ++++++++++++++++++++++
>>>  1 file changed, 60 insertions(+)
>>>  create mode 100644 
>>> Documentation/devicetree/bindings/display/truly,nt35597.txt
>>> 
>>> diff --git 
>>> a/Documentation/devicetree/bindings/display/truly,nt35597.txt 
>>> b/Documentation/devicetree/bindings/display/truly,nt35597.txt
>>> new file mode 100644
>>> index 0000000..46b29eb
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/display/truly,nt35597.txt
>>> @@ -0,0 +1,60 @@
>>> +Truly model NT35597 DSI display driver
>>> +
>>> +The Truly NT35597 is a generic display driver, currently only 
>>> configured
>>> +for use in the 2K display on the Qualcomm SDM845.
>> 
>> So this is for *all* SDM845 based boards?
> This is for SDM845 MTP board. Let me mention that as well.
>> 
>>> +
>>> +Required properties:
>>> +- compatible: should be "truly,nt35597-2K-display"
>>> +- vdda-supply: phandle of the regulator that provides the supply 
>>> voltage
>>> +  Power IC supply
>>> +- vdispp-supply: phandle of the regulator that provides the supply 
>>> voltage
>>> +  for positive LCD bias
>>> +- vdispn-supply: phandle of the regulator that provides the supply 
>>> voltage
>>> +  for negative LCD bias
>>> +- reset-gpios: phandle of gpio for reset line
>>> +  This should be 8mA, gpio can be configured using mux, pinctrl, 
>>> pinctrl-names
>>> +  (active low)
>>> +- mode-gpios: phandle of the gpio for choosing the mode of the 
>>> display
>>> +  for single DSI or Dual DSI
>>> +  (active high)
>> 
>> active high doesn't really make sense for this.
> Alright, will remove this.
Here you would prefer us to make this GPIO active low right?
OR you were just mentioning that I can remove the active high 
documentation?
>> 
>>> +  This should be low for dual DSI and high for single DSI mode
>>> +- ports: This device has two video ports driven by two DSIs. Their 
>>> connections
>>> +  are modelled using the OF graph bindings specified in
>> 
>> modeled
>> 
>>> +  Documentation/devicetree/bindings/graph.txt.
>>> +  - port@0: DSI input port driven by master DSI
>>> +  - port@1: DSI input port driven by secondary DSI
>>> +
>>> +Example:
>>> +
>>> +       dsi@ae94000 {
>>> +               panel@0 {
>>> +                       compatible = "truly,nt35597-2K-display";
>>> +                       reg = <0>;
>>> +                       vdda-supply = <&pm8998_l14>;
>>> +                       vdispp-supply = <&lab_regulator>;
>>> +                       vdispn-supply = <&ibb_regulator>;
>>> +                       pinctrl-names = "default", "suspend";
>>> +                       pinctrl-0 = <&dpu_dsi_active>;
>>> +                       pinctrl-1 = <&dpu_dsi_suspend>;
>>> +
>>> +                       reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
>>> +                       mode-gpios = <&tlmm 52 GPIO_ACTIVE_HIGH>;
>>> +                       ports {
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <0>;
>>> +                               port@0 {
>>> +                                       reg = <0>;
>>> +                                       panel0_in: endpoint {
>>> +                                               remote-endpoint = 
>>> <&dsi0_out>;
>>> +                                       };
>>> +                               };
>>> +
>>> +                               port@1 {
>>> +                                       reg = <1>;
>>> +                                       panel1_in: endpoint {
>>> +                                               remote-endpoint = 
>>> <&dsi1_out>;
>>> +                                       };
>>> +                               };
>>> +                       };
>>> +               };
>>> +       };
>>> --
>>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>>> Forum,
>>> a Linux Foundation Collaborative Project
>>> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel