This series converts the DT binding for the TI TFP410 DPI-to-DVI encoder to json-schema.
This also fixes a minor bug in the ti-tfp410 driver that would cause a wrong calculation of the setup and hold times when the de-skew feature is enabled. The retrieval of the de-skew value from the DT has also been updated to reflect the binding changes.
Changes in v3:
- ti,tfp410.yaml - Original translation to yaml separated into its own commit with no additional changes (Laurent Pinchart). - Redefinition of ti,deskew property in its own commit (Laurent). - Removal of unnecessary text (Laurent). - New conditional constraint for ti,deskew (Laurent). - "ports" node set as optional (Rob Herring).
- dove-sbc-a510.dts: removed. This comes from Rob's suggestion that the "ports" node should be optional instead of defining empty port nodes in DTs files.
- ti-tfp410.c - Original changes split into two commits. The first one changes the datatype and retrieval of the deskew property. The second one fixes the calculation of hold and setup times (Laurent).
The bindings have been tested with:
make dt_binding_check ARCH=<arch> DT_SCHEMA_FILES=<...ti,tfp410.yaml> make dtbs_check ARCH=<arch> DT_SCHEMA_FILES=<...ti,tfp410.yaml>
for <arch> = arm and arm64. This uncovered a number of dts files that use the TFP410 but not through I2C and don't define the ti,deskew property. These should be fixed at some point.
Ricardo Cañuelo (5): dt-bindings: display: ti,tfp410.txt: convert to yaml dt-bindings: display: ti,tfp410.yaml: Redefine ti,deskew property drm/bridge: tfp410: fix de-skew value retrieval from DT drm/bridge: tfp410: Fix setup and hold time calculation dt-bindings: display: ti,tfp410.yaml: make the ports node optional
.../bindings/display/bridge/ti,tfp410.txt | 66 --------- .../bindings/display/bridge/ti,tfp410.yaml | 130 ++++++++++++++++++ drivers/gpu/drm/bridge/ti-tfp410.c | 10 +- 3 files changed, 135 insertions(+), 71 deletions(-) delete mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
Convert the DT binding documentation for the TI TFP410 DPI-to-DVI encoder to json-schema.
Signed-off-by: Ricardo Cañuelo ricardo.canuelo@collabora.com Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- .../bindings/display/bridge/ti,tfp410.txt | 66 --------- .../bindings/display/bridge/ti,tfp410.yaml | 130 ++++++++++++++++++ 2 files changed, 130 insertions(+), 66 deletions(-) delete mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt deleted file mode 100644 index 5ff4f64ef8e8..000000000000 --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt +++ /dev/null @@ -1,66 +0,0 @@ -TFP410 DPI to DVI encoder -========================= - -Required properties: -- compatible: "ti,tfp410" - -Optional properties: -- powerdown-gpios: power-down gpio -- reg: I2C address. If and only if present the device node should be placed - into the I2C controller node where the TFP410 I2C is connected to. -- ti,deskew: data de-skew in 350ps increments, from -4 to +3, as configured - through th DK[3:1] pins. This property shall be present only if the TFP410 - is not connected through I2C. - -Required nodes: - -This device has two video ports. Their connections are modeled using the OF -graph bindings specified in [1]. Each port node shall have a single endpoint. - -- Port 0 is the DPI input port. Its endpoint subnode shall contain a - pclk-sample and bus-width property and a remote-endpoint property as specified - in [1]. - - If pclk-sample is not defined, pclk-sample = 0 should be assumed for - backward compatibility. - - If bus-width is not defined then bus-width = 24 should be assumed for - backward compatibility. - bus-width = 24: 24 data lines are connected and single-edge mode - bus-width = 12: 12 data lines are connected and dual-edge mode - -- Port 1 is the DVI output port. Its endpoint subnode shall contain a - remote-endpoint property is specified in [1]. - -[1] Documentation/devicetree/bindings/media/video-interfaces.txt - - -Example -------- - -tfp410: encoder@0 { - compatible = "ti,tfp410"; - powerdown-gpios = <&twl_gpio 2 GPIO_ACTIVE_LOW>; - ti,deskew = <4>; - - ports { - #address-cells = <1>; - #size-cells = <0>; - - port@0 { - reg = <0>; - - tfp410_in: endpoint@0 { - pclk-sample = <1>; - bus-width = <24>; - remote-endpoint = <&dpi_out>; - }; - }; - - port@1 { - reg = <1>; - - tfp410_out: endpoint@0 { - remote-endpoint = <&dvi_connector_in>; - }; - }; - }; -}; diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml new file mode 100644 index 000000000000..5569a4907765 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml @@ -0,0 +1,130 @@ +# SPDX-License-Identifier: GPL-2.0-only +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/bridge/ti,tfp410.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: TFP410 DPI to DVI encoder + +maintainers: + - Tomi Valkeinen tomi.valkeinen@ti.com + - Jyri Sarha jsarha@ti.com + +properties: + compatible: + const: ti,tfp410 + + reg: + description: I2C address of the device. + maxItems: 1 + + powerdown-gpios: + maxItems: 1 + + ti,deskew: + description: + Data de-skew in 350ps increments, from -4 to +3, as configured + through th DK[3:1] pins. This property shall be present only if the TFP410 + is not connected through I2C. + maxItems: 1 + $ref: /schemas/types.yaml#/definitions/uint32 + + ports: + description: + A node containing input and output port nodes with endpoint + definitions as documented in + Documentation/devicetree/bindings/media/video-interfaces.txt + type: object + + properties: + port@0: + description: DPI input port. + type: object + + properties: + reg: + const: 0 + + endpoint: + type: object + + properties: + pclk-sample: + description: + Endpoint sampling edge. + enum: + - 0 # Falling edge + - 1 # Rising edge + default: 0 + + bus-width: + description: + Endpoint bus width. + enum: + - 12 # 12 data lines connected and dual-edge mode + - 24 # 24 data lines connected and single-edge mode + default: 24 + + port@1: + description: DVI output port. + type: object + + properties: + reg: + const: 1 + + endpoint: + type: object + + required: + - port@0 + - port@1 + +required: + - compatible + - ports + +if: + required: + - reg +then: + properties: + ti,deskew: false +else: + required: + - ti,deskew + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + tfp410: encoder { + compatible = "ti,tfp410"; + powerdown-gpios = <&twl_gpio 2 GPIO_ACTIVE_LOW>; + ti,deskew = <3>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + tfp410_in: endpoint { + pclk-sample = <1>; + bus-width = <24>; + remote-endpoint = <&dpi_out>; + }; + }; + + port@1 { + reg = <1>; + tfp410_out: endpoint { + remote-endpoint = <&dvi_connector_in>; + }; + }; + }; + }; + +...
This changes how the 'ti,deskew' property is defined. It's now an unsigned value from 0 to 7 instead of a signed value from -4 to 3. Until the dtc carries the integer sign through to the yaml output it's easier to define signed types as unsigned.
Signed-off-by: Ricardo Cañuelo ricardo.canuelo@collabora.com Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- .../devicetree/bindings/display/bridge/ti,tfp410.yaml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml index 5569a4907765..605831c1e836 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml @@ -23,11 +23,12 @@ properties:
ti,deskew: description: - Data de-skew in 350ps increments, from -4 to +3, as configured - through th DK[3:1] pins. This property shall be present only if the TFP410 - is not connected through I2C. - maxItems: 1 + Data de-skew value in 350ps increments, from 0 to 7, as configured + through the DK[3:1] pins. The de-skew multiplier is computed as + (DK[3:1] - 4), so it ranges from -4 to 3. $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 7
ports: description:
The tfp410 has a data de-skew feature that allows the user to compensate the skew between IDCK and the pixel data and control signals.
In the driver, the setup and hold times are calculated from the de-skew value. This retrieves the deskew value from the DT using the proper datatype and range check as described by the binding (u32 from 0 to 7).
This fix results from a change in the ti,tfp410 DT binding.
Signed-off-by: Ricardo Cañuelo ricardo.canuelo@collabora.com Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/bridge/ti-tfp410.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c index e3eb6364c0f7..dfde811f3411 100644 --- a/drivers/gpu/drm/bridge/ti-tfp410.c +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -220,7 +220,7 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c) struct device_node *ep; u32 pclk_sample = 0; u32 bus_width = 24; - s32 deskew = 0; + u32 deskew = 0;
/* Start with defaults. */ *timings = tfp410_default_timings; @@ -274,12 +274,12 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c) }
/* Get the setup and hold time from vendor-specific properties. */ - of_property_read_u32(dvi->dev->of_node, "ti,deskew", (u32 *)&deskew); - if (deskew < -4 || deskew > 3) + of_property_read_u32(dvi->dev->of_node, "ti,deskew", &deskew); + if (deskew > 7) return -EINVAL;
- timings->setup_time_ps = min(0, 1200 - 350 * deskew); - timings->hold_time_ps = min(0, 1300 + 350 * deskew); + timings->setup_time_ps = min(0, 1200 - 350 * ((s32)deskew - 4)); + timings->hold_time_ps = min(0, 1300 + 350 * ((s32)deskew - 4));
return 0; }
This fixes a bug in the calculation of the setup and hold times based on the deskew configuration.
Signed-off-by: Ricardo Cañuelo ricardo.canuelo@collabora.com Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/bridge/ti-tfp410.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c index dfde811f3411..21d99b4ea0c9 100644 --- a/drivers/gpu/drm/bridge/ti-tfp410.c +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -278,8 +278,8 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c) if (deskew > 7) return -EINVAL;
- timings->setup_time_ps = min(0, 1200 - 350 * ((s32)deskew - 4)); - timings->hold_time_ps = min(0, 1300 + 350 * ((s32)deskew - 4)); + timings->setup_time_ps = 1200 - 350 * ((s32)deskew - 4); + timings->hold_time_ps = max(0, 1300 + 350 * ((s32)deskew - 4));
return 0; }
Make the ports node optional, since there are some DTs that don't define any ports for ti,tfp410.
Signed-off-by: Ricardo Cañuelo ricardo.canuelo@collabora.com --- Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml | 1 - 1 file changed, 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml index 605831c1e836..1c9421eb80fa 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml @@ -83,7 +83,6 @@ properties:
required: - compatible - - ports
if: required:
Hi Ricardo,
Thank you for the patch.
On Thu, Jun 11, 2020 at 12:23:56PM +0200, Ricardo Cañuelo wrote:
Make the ports node optional, since there are some DTs that don't define any ports for ti,tfp410.
Signed-off-by: Ricardo Cañuelo ricardo.canuelo@collabora.com
Shouldn't we fix those DTs instead ? What's the point of a TFP410 without ports in DT ?
Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml | 1 - 1 file changed, 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml index 605831c1e836..1c9421eb80fa 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml @@ -83,7 +83,6 @@ properties:
required:
- compatible
- ports
if: required:
Hi Laurent,
Thanks for reviewing the patch
On Thu, 2020-06-11 at 19:08 +0300, Laurent Pinchart wrote:
Hi Ricardo,
Thank you for the patch.
On Thu, Jun 11, 2020 at 12:23:56PM +0200, Ricardo Cañuelo wrote:
Make the ports node optional, since there are some DTs that don't define any ports for ti,tfp410.
Signed-off-by: Ricardo Cañuelo ricardo.canuelo@collabora.com
Shouldn't we fix those DTs instead ? What's the point of a TFP410 without ports in DT ?
This comes from the discussion in the previous version of this series.
In the DTs that don't define any ports (it's dove-sbc-a510.dts only, actually) it's not clear how to define the ports (I'm not familiar with this board). Initially I defined a set of empty ports just to comply with the binding requirements, but Rob suggested that we might as well declare them as optional, since having an empty port definition with no remote endpoints is no better than having no ports at all.
I understand both opinions but I just don't know which is the best option at this point.
Cheers, Ricardo
Hi Ricardo,
On Mon, Jun 15, 2020 at 11:38:07AM +0200, Ricardo Cañuelo wrote:
On Thu, 2020-06-11 at 19:08 +0300, Laurent Pinchart wrote:
On Thu, Jun 11, 2020 at 12:23:56PM +0200, Ricardo Cañuelo wrote:
Make the ports node optional, since there are some DTs that don't define any ports for ti,tfp410.
Signed-off-by: Ricardo Cañuelo ricardo.canuelo@collabora.com
Shouldn't we fix those DTs instead ? What's the point of a TFP410 without ports in DT ?
This comes from the discussion in the previous version of this series.
In the DTs that don't define any ports (it's dove-sbc-a510.dts only, actually) it's not clear how to define the ports (I'm not familiar with this board). Initially I defined a set of empty ports just to comply with the binding requirements, but Rob suggested that we might as well declare them as optional, since having an empty port definition with no remote endpoints is no better than having no ports at all.
I understand both opinions but I just don't know which is the best option at this point.
How about keeping the ports mandatory, and leaving dove-sbc-a510.dts to be fixed later ?
Hi Laurent,
On Tue, 2020-06-16 at 04:51 +0300, Laurent Pinchart wrote:
How about keeping the ports mandatory, and leaving dove-sbc-a510.dts to be fixed later ?
That's fine by me, I'll prepare a new version of the series. Thanks for your input!
Cheers, Ricardo
On Mon, Jun 15, 2020 at 11:38:07AM +0200, Ricardo Cañuelo wrote:
Hi Laurent,
Thanks for reviewing the patch
On Thu, 2020-06-11 at 19:08 +0300, Laurent Pinchart wrote:
Hi Ricardo,
Thank you for the patch.
On Thu, Jun 11, 2020 at 12:23:56PM +0200, Ricardo Cañuelo wrote:
Make the ports node optional, since there are some DTs that don't define any ports for ti,tfp410.
Signed-off-by: Ricardo Cañuelo ricardo.canuelo@collabora.com
Shouldn't we fix those DTs instead ? What's the point of a TFP410 without ports in DT ?
This comes from the discussion in the previous version of this series.
In the DTs that don't define any ports (it's dove-sbc-a510.dts only, actually) it's not clear how to define the ports (I'm not familiar with this board). Initially I defined a set of empty ports just to comply with the binding requirements, but Rob suggested that we might as well declare them as optional, since having an empty port definition with no remote endpoints is no better than having no ports at all.
I did? Must have missed some context.
I understand both opinions but I just don't know which is the best option at this point.
Just leave the warning to be fixed.
Rob
On Thu, Jun 11, 2020 at 12:23:56PM +0200, Ricardo Cañuelo wrote:
Make the ports node optional, since there are some DTs that don't define any ports for ti,tfp410.
Only arch/arm/boot/dts/dove-sbc-a510.dts AFAICT... It should be updated IMO.
Signed-off-by: Ricardo Cañuelo ricardo.canuelo@collabora.com
Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml | 1 - 1 file changed, 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml index 605831c1e836..1c9421eb80fa 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml @@ -83,7 +83,6 @@ properties:
required:
- compatible
- ports
if: required: -- 2.18.0
Hi Rob,
On Wed, Jun 17, 2020 at 04:34:55PM -0600, Rob Herring wrote:
On Thu, Jun 11, 2020 at 12:23:56PM +0200, Ricardo Cañuelo wrote:
Make the ports node optional, since there are some DTs that don't define any ports for ti,tfp410.
Only arch/arm/boot/dts/dove-sbc-a510.dts AFAICT... It should be updated IMO.
Agreed, but Ricardo wasn't sure how to update it. It would be nice if someone with knowledge of the hardware could have a look.
By the way, this patch is dropped from v4 of the series.
Signed-off-by: Ricardo Cañuelo ricardo.canuelo@collabora.com
Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml | 1 - 1 file changed, 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml index 605831c1e836..1c9421eb80fa 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml @@ -83,7 +83,6 @@ properties:
required:
- compatible
- ports
if: required:
dri-devel@lists.freedesktop.org
-
Laurent Pinchart -
Ricardo Cañuelo -
Rob Herring