On 2/4/21 6:15 PM, Doug Anderson wrote:
Hi,
[...]
+properties:
+ compatible:
+ const: ti,sn65dsi83
+
+ reg:
+ const: 0x2d
+
+ enable-gpios:
+ maxItems: 1
+ description: GPIO specifier for bridge_en pin (active high).
I see two regulators: vcc and vcore. Shouldn't those be listed?
Those are not implemented and not tested, so if someone needs them later
on, they can be added then.
I also see an interrupt pin on the datasheet. Probably should be
listed too even if the chip can be made to work fine without hooking
it up. It can just be optional, right?
It is optional and again completely untested, so it can be added later
if needed.
It wouldn't hurt to list the refclk here too even if the code doesn't
use it. From sn65dsi86 it was handy that the bindings already had all
this type of stuff so that when we added the feature we didn't have to
go back to the bindings.
In my case, the refclock are derived from the DSI link.
+ ports:
+ type: object
+ additionalProperties: false
+
+ properties:
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ port@0:
+ type: object
+ additionalProperties: false
+
+ description:
+ Video port for MIPI DSI input
+
+ properties:
+ reg:
+ const: 0
+
+ endpoint:
+ type: object
+ additionalProperties: false
+ properties:
+ remote-endpoint: true
+ data-lanes:
+ description: array of physical DSI data lane indexes.
The chip doesn't allow for arbitrary remapping here, right? So you're
just using this as the official way to specify the number of lanes? I
guess the only valid values are:
<0>
<0 1>
<0 1 2>
<0 1 2 3>
Shouldn't that be <1 2 3 4> ?
In sn65dsi86 we attempted to enforce that a valid option was selected
for the output lanes. Could you do something similar? If nothing
else adding a description of the valid options would be good.
I saw the binding, but I was under the impressions the DSI86 can do lane
reordering, isn't that the case ? Maybe I misunderstood it.
But yes, if you have a suggestion how to make a non-cryptic list of
those four lane mapping options, please do share this info.
+ required:
+ - reg
+
+ port@1:
+ type: object
+ additionalProperties: false
+
+ description:
+ Video port for LVDS output (panel or bridge).
+
+ properties:
+ reg:
+ const: 1
+
+ endpoint:
+ type: object
+ additionalProperties: false
+ properties:
+ remote-endpoint: true
Worth adding the data-lanes here too? I guess this part allows you
two different orders for the LVDS outputs?
I don't really want to add any properties which I cannot test and then
end up with DT bindings which would become poor ABI in the future.
+
+ required:
+ - reg
+
+ required:
+ - "#address-cells"
+ - "#size-cells"
+ - port@0
+ - port@1
+
+required:
+ - compatible
+ - reg
+ - enable-gpios
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ bridge@2d {
+ compatible = "ti,sn65dsi83";
+ reg = <0x2d>;
+
+ enable-gpios = <&gpio2 1 GPIO_ACTIVE_HIGH>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ endpoint {
+ remote-endpoint = <&dsi0_out>;
+ data-lanes = <1 2 3 4>;
Should the above be <0 1 2 3>?
Well, git grep data-lanes seems to indicate some count from 1, some from 0 .
