| Message ID | 20200514143612.2094-5-ricardo.canuelo@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt-bindings: display: ti,tfp410.txt: convert to yaml | expand |
Hi Ricardo, Thank you for the patch. On Thu, May 14, 2020 at 04:36:12PM +0200, Ricardo Cañuelo wrote: > 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) > and fixes the calculation of the setup and hold times. How about mentioning that the fix results from a change in the DT bindings ? Otherwise it sounds it was a bug in the driver. I would also mention that the patch fixes the min() calculation, which was clearly wrong. I think I would have split this in two patches. With this addressed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.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..21d99b4ea0c9 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 = 1200 - 350 * ((s32)deskew - 4); > + timings->hold_time_ps = max(0, 1300 + 350 * ((s32)deskew - 4)); > > return 0; > }
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c index e3eb6364c0f7..21d99b4ea0c9 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 = 1200 - 350 * ((s32)deskew - 4); + timings->hold_time_ps = max(0, 1300 + 350 * ((s32)deskew - 4)); return 0; }
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) and fixes the calculation of the setup and hold times. Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com> --- drivers/gpu/drm/bridge/ti-tfp410.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)