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

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

 



Hi Linus

Thanks for your valuable comments.

Yes, we agree with your comments here and will address them.

Some details below on how we will take care of it.

Thanks

Abhinav

On 2018-08-03 04:20, Linus Walleij wrote:
On Fri, Aug 3, 2018 at 4:49 AM Abhinav Kumar <abhinavk@xxxxxxxxxxxxxx> wrote:

From: "abhinavk@xxxxxxxxxxxxxx" <abhinavk@xxxxxxxxxxxxxx>

Add the device tree bindings for Truly NT35597 panel.
This panel supports both single DSI and dual DSI.

I do not think this is a panel but a panel driver that can be used
with several physical panels. Can you confirm?
Yes, from the documentation I have I can see that this is a panel
driver and can support other panels/resolutions.

I suspect this since the command sequence in the driver seems
to contain a command for setting up the actual resolution and
a bunch of clocking properties for the physical panel.

+Required properties:
+- compatible: should be "truly,nt35597"

As with eg ili9322 I think this should have dual compatible strings
identifying the system it is used with since the resolution, clocking
etc is apparently
actually configurable.

compatible:
  "truly,nt35597", "qcom,reference-design1-name-display";
  "truly,nt35597", "qcom,reference-design2-name-display";

Then you send the command setting up resolution etc only for that
one system.

Yes, I agree we will can have dual compatible strings and we will pick
resolution/modes based on the compatible string similar to this:

https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/panel/panel-ilitek-ili9322.c#L659

+- 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 +- mode-gpios: phandle of the gpio for choosing the mode of the display
+  for single DSI or Dual DSI
+  This should be low for dual DSI and high for single DSI mode
+- display-timings: Node for the Panel timings

I don't think this belongs in the device tree at all.

Provide the timings from the driver based on the compatible string
instead, as you actually send commands to set up a certain timing in
the display controller.

(See ili9322 driver for an example of how I do this.)

Yes, will follow the example of ili9322 and do the same.


+- ports: This device has two video ports driven by two DSIs. Their connections
+  are modelled using the OF graph bindings specified in
+  Documentation/devicetree/bindings/graph.txt.
+  - port@0: DSI input port driven by master DSI
+  - port@1: DSI input port driven by secondary DSI

The rest seems fine.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux