RE: [PATCH v4 08/13] dt-bindings: display: bridge: Repurpose lvds-encoder

2019-11-19 Thread Fabrizio Castro
Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart 
> Sent: 19 November 2019 00:08
> Subject: Re: [PATCH v4 08/13] dt-bindings: display: bridge: Repurpose 
> lvds-encoder
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Wed, Nov 13, 2019 at 03:51:27PM +, Fabrizio Castro wrote:
> > In an effort to repurpose lvds-encoder.c to also serve the
> > function of LVDS decoders, we ended up defining a new "generic"
> > compatible string ("lvds-decoder"), therefore adapt the dt schema
> > to allow for the new compatible string.
> >
> > Signed-off-by: Fabrizio Castro 
> >
> > ---
> > v3->v4:
> > * Improved title and description according to Laurent's comments
> > * Reworked definition of the compatible property
> > v2->v3:
> > * Extracted conversion to lvds-codec as per Rob's comment
> > v1->v2:
> > * Converted to dt-schema as per Neil's comment
> > ---
> >  .../{lvds-transmitter.yaml => lvds-codec.yaml} | 54 
> > +-
> >  1 file changed, 42 insertions(+), 12 deletions(-)
> >  rename 
> > Documentation/devicetree/bindings/display/bridge/{lvds-transmitter.yaml => 
> > lvds-codec.yaml} (61%)
> >
> > diff --git 
> > a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.yaml
> b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > similarity index 61%
> > rename from 
> > Documentation/devicetree/bindings/display/bridge/lvds-transmitter.yaml
> > rename to Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > index 27de616..0ecc8a4 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > @@ -1,17 +1,17 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  %YAML 1.2
> >  ---
> > -$id: http://devicetree.org/schemas/display/bridge/lvds-transmitter.yaml#
> > +$id: http://devicetree.org/schemas/display/bridge/lvds-codec.yaml#
> >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> >
> > -title: Parallel to LVDS Encoder
> > +title: Transparent LVDS encoders and decoders
> >
> >  maintainers:
> >- Laurent Pinchart 
> >
> >  description: |
> > -  This binding supports the parallel to LVDS encoders that don't require 
> > any
> > -  configuration.
> > +  This binding supports transparent LVDS encoders and decoders that don't
> > +  require any configuration.
> >
> >LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. 
> > Multiple
> >incompatible data link layers have been used over time to transmit image 
> > data
> > @@ -33,12 +33,14 @@ properties:
> >  description: |
> >Must list the device specific compatible string first, followed by 
> > the
> >generic compatible string.
> > -items:
> > -  - enum:
> > -- ti,ds90c185   # For the TI DS90C185 FPD-Link Serializer
> > -- ti,ds90c187   # For the TI DS90C187 FPD-Link Serializer
> > -- ti,sn75lvds83 # For the TI SN75LVDS83 FlatLink transmitter
> > -  - const: lvds-encoder # Generic LVDS encoder compatible fallback
> > +oneOf:
> > +  - items:
> > +- enum:
> > +  - ti,ds90c185   # For the TI DS90C185 FPD-Link Serializer
> > +  - ti,ds90c187   # For the TI DS90C187 FPD-Link Serializer
> > +  - ti,sn75lvds83 # For the TI SN75LVDS83 FlatLink transmitter
> > +- const: lvds-encoder # Generic LVDS encoder compatible fallback
> > +  - const: lvds-decoder   # Generic LVDS decoders compatible fallback
> >
> >ports:
> >  type: object
> > @@ -49,12 +51,14 @@ properties:
> >port@0:
> >  type: object
> >  description: |
> > -  Port 0 is for parallel input
> > +  With LVDS encoders port 0 is for parallel input
> > +  With LVDS decoders port 0 is for LVDS input
> 
> How about
> 
>   For LVDS encoders, port 0 is the parallel input
>   For LVDS decoders, port 0 is the LVDS input

It's ok with me

> 
> >
> >port@1:
> >  type: object
> >  description: |
> > -  Port 1 is for LVDS output
> > +  With LVDS encoders port 1 is for LVDS output
> > +  With LVDS decoders port 1 is for parallel output
> 
> And similarly here ?

Here too

> 
> If you're fine 

Re: [PATCH v4 08/13] dt-bindings: display: bridge: Repurpose lvds-encoder

2019-11-18 Thread Laurent Pinchart
Hi Fabrizio,

Thank you for the patch.

On Wed, Nov 13, 2019 at 03:51:27PM +, Fabrizio Castro wrote:
> In an effort to repurpose lvds-encoder.c to also serve the
> function of LVDS decoders, we ended up defining a new "generic"
> compatible string ("lvds-decoder"), therefore adapt the dt schema
> to allow for the new compatible string.
> 
> Signed-off-by: Fabrizio Castro 
> 
> ---
> v3->v4:
> * Improved title and description according to Laurent's comments
> * Reworked definition of the compatible property
> v2->v3:
> * Extracted conversion to lvds-codec as per Rob's comment
> v1->v2:
> * Converted to dt-schema as per Neil's comment
> ---
>  .../{lvds-transmitter.yaml => lvds-codec.yaml} | 54 
> +-
>  1 file changed, 42 insertions(+), 12 deletions(-)
>  rename 
> Documentation/devicetree/bindings/display/bridge/{lvds-transmitter.yaml => 
> lvds-codec.yaml} (61%)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.yaml 
> b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> similarity index 61%
> rename from 
> Documentation/devicetree/bindings/display/bridge/lvds-transmitter.yaml
> rename to Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> index 27de616..0ecc8a4 100644
> --- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> @@ -1,17 +1,17 @@
>  # SPDX-License-Identifier: GPL-2.0
>  %YAML 1.2
>  ---
> -$id: http://devicetree.org/schemas/display/bridge/lvds-transmitter.yaml#
> +$id: http://devicetree.org/schemas/display/bridge/lvds-codec.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Parallel to LVDS Encoder
> +title: Transparent LVDS encoders and decoders
>  
>  maintainers:
>- Laurent Pinchart 
>  
>  description: |
> -  This binding supports the parallel to LVDS encoders that don't require any
> -  configuration.
> +  This binding supports transparent LVDS encoders and decoders that don't
> +  require any configuration.
>  
>LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. 
> Multiple
>incompatible data link layers have been used over time to transmit image 
> data
> @@ -33,12 +33,14 @@ properties:
>  description: |
>Must list the device specific compatible string first, followed by the
>generic compatible string.
> -items:
> -  - enum:
> -- ti,ds90c185   # For the TI DS90C185 FPD-Link Serializer
> -- ti,ds90c187   # For the TI DS90C187 FPD-Link Serializer
> -- ti,sn75lvds83 # For the TI SN75LVDS83 FlatLink transmitter
> -  - const: lvds-encoder # Generic LVDS encoder compatible fallback
> +oneOf:
> +  - items:
> +- enum:
> +  - ti,ds90c185   # For the TI DS90C185 FPD-Link Serializer
> +  - ti,ds90c187   # For the TI DS90C187 FPD-Link Serializer
> +  - ti,sn75lvds83 # For the TI SN75LVDS83 FlatLink transmitter
> +- const: lvds-encoder # Generic LVDS encoder compatible fallback
> +  - const: lvds-decoder   # Generic LVDS decoders compatible fallback
>  
>ports:
>  type: object
> @@ -49,12 +51,14 @@ properties:
>port@0:
>  type: object
>  description: |
> -  Port 0 is for parallel input
> +  With LVDS encoders port 0 is for parallel input
> +  With LVDS decoders port 0 is for LVDS input

How about

  For LVDS encoders, port 0 is the parallel input
  For LVDS decoders, port 0 is the LVDS input

>  
>port@1:
>  type: object
>  description: |
> -  Port 1 is for LVDS output
> +  With LVDS encoders port 1 is for LVDS output
> +  With LVDS decoders port 1 is for parallel output

And similarly here ?

If you're fine with this change there's no need to resubmit, I'll change
this when applying, and

Reviewed-by: Laurent Pinchart 

>  
>  required:
>- port@0
> @@ -96,4 +100,30 @@ examples:
>};
>  };
>  
> +  - |
> +lvds-decoder {
> +  compatible = "lvds-decoder";
> +
> +  ports {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +port@0 {
> +  reg = <0>;
> +
> +  lvds_dec_in: endpoint {
> +remote-endpoint = <_out_lvds>;
> +  };
> +};
> +
> +port@1 {
> +  reg = <1>;
> +
> +  lvds_dec_out: endpoint {
> +remote-endpoint = <_panel_in>;
> +  };
> +};
> +  };
> +};
> +
>  ...

-- 
Regards,

Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 08/13] dt-bindings: display: bridge: Repurpose lvds-encoder

2019-11-18 Thread Rob Herring
On Wed, 13 Nov 2019 15:51:27 +, Fabrizio Castro wrote:
> In an effort to repurpose lvds-encoder.c to also serve the
> function of LVDS decoders, we ended up defining a new "generic"
> compatible string ("lvds-decoder"), therefore adapt the dt schema
> to allow for the new compatible string.
> 
> Signed-off-by: Fabrizio Castro 
> 
> ---
> v3->v4:
> * Improved title and description according to Laurent's comments
> * Reworked definition of the compatible property
> v2->v3:
> * Extracted conversion to lvds-codec as per Rob's comment
> v1->v2:
> * Converted to dt-schema as per Neil's comment
> ---
>  .../{lvds-transmitter.yaml => lvds-codec.yaml} | 54 
> +-
>  1 file changed, 42 insertions(+), 12 deletions(-)
>  rename 
> Documentation/devicetree/bindings/display/bridge/{lvds-transmitter.yaml => 
> lvds-codec.yaml} (61%)
> 

Reviewed-by: Rob Herring 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel