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@xxxxxxxxxx> > > 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
Attachment:
signature.asc
Description: PGP signature
