On Fri, Feb 14, 2020 at 05:11:56PM +0100, Sam Ravnborg wrote:
Hi Maxime.
On Fri, Feb 14, 2020 at 01:24:39PM +0100, Maxime Ripard wrote:
Some LVDS encoders can support multiple polarities on the data and clock lanes, and similarly some panels require a given polarity on their inputs. Add a property on the panel to configure the encoder properly.
Signed-off-by: Maxime Ripard maxime@cerno.tech
Not a fan of this binding... In display-timing.txt we have a specification/description of the panel-timing node.
The panel-timing node already include information such as:
- hsync-active:
- vsync-active:
- de-active:
- pixelclk-active:
- syncclk-active:
But clock-active-low and data-active-low refer to the bus more than an individual timing. So maybe OK not to have it in a panel-timing node. But then it would IMO be better to include this in the display-timing node - so we make this available and standard for all users of the display-timing node.
I will dig up my patchset to make proper bindings for panel-timing and display-timing this weeked and resend them. Then we can discuss if this goes on top or this is specific for the lvds binding.
That makes sense, I'll wait for them to be merged then :)
Documentation/devicetree/bindings/display/panel/lvds.yaml | 10 ++++++++- 1 file changed, 10 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml b/Documentation/devicetree/bindings/display/panel/lvds.yaml index d0083301acbe..4a1111a1ab38 100644 --- a/Documentation/devicetree/bindings/display/panel/lvds.yaml +++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml @@ -90,6 +90,16 @@ properties: CTL2: Data Enable CTL3: 0
- clock-active-low:
- type: boolean
- description: >
Should this be "|" and not ">"? Did this pass dt_binding_check?
Yes. > means that this is a multi-line string that will drop the \n between each line, while | will keep it
For a string like this, I believe it makes more sense to let whatever is using to handle the wrapping, but I don't really have a strong opinion :)
If set, reverse the clock polarity on the clock lane.This text could be a bit more specific. If this is set then what? And it seems strange that a clock is active low. For a clock we often talk about raising or falling edge.
- data-active-low:
- type: boolean
- description: >
Same comment with ">"
If set, reverse the bit polarity on all data lanes.Same comment about a more explicit description.
I'll try to come up with something better. Thanks! Maxime