Re: [PATCH 4/4] drm: exynos: hdmi: Add dt support for hdmiphy settings

[Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>]

On Tue, Oct 29, 2013 at 08:12:32AM +0000, Shirish S wrote:
> This patch adds dt support to hdmiphy config settings
> as it is board specific and depends on the signal pattern
> of board.
> 
> Signed-off-by: Shirish S <s.shirish-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  .../devicetree/bindings/video/exynos_hdmi.txt      |   34 +++++++++
>  drivers/gpu/drm/exynos/exynos_hdmi.c               |   79 +++++++++++++++++++-
>  2 files changed, 109 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> index 323983b..c685c90 100644
> --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> @@ -13,6 +13,32 @@ Required properties:
>  	b) pin number within the gpio controller.
>  	c) optional flags and pull up/down.
>  
> +- hdmiphy-configs: following information about the hdmiphy config settings.
> +        a) "nr-configs" specifies the number of pixel clocks supported.

I really don't see why this is necessary. It's redundant, and it's easy
for the driver to derive this from the number of config<N> nodes, which
it can count.

> +	b) "config<N>: config<N>" specifies the phy configuration settings,
> +		wher 'N' denotes the number of iteration.

The number of iteration?

> +		"pixel-clock" specifies the pixel clock
> +		"conifig-de-emphasis-level" specifies the 8 bit configuration
> +		of Data De-emphasis levels,below shown is example for
> +		data de-emphasis register at address 0x145D0040.
> +			hdmiphy@38[16] for bits[3:0] permitted values:
> +				0000 means 760 mVdiff && 1111 means 1400 mVdiff
> +				1LSB corresponds to 20mVdiff
> +			hdmiphy@38[16] for bits[7:4] permitted values:
> +				0000 	0dB
> +				0001	-0.25dB
> +				0010	-0.7dB
> +				0011	-1.15dB
> +				1111	-7.45dB
> +		"config-clock-level" specifies the 8 bit configuration for
> +		the corresponding clock level, for example if 0x145D005C
> +		is the address of clock level register.

I don't understand what this intended to mean.

> +			hdmiphy@38[23] for bits [1:0] permitted values:
> +				00 means 0 mVdiff && 11 means 60 mVdiff
> +			hdmiphy@38[23] for bits [7:3] permitted values:
> +				00000 is 790 mVdiff
> +				11111 is 1430 mVdiff
> +				1LSB corresponds to 20mVdiff

That last line was confusing. Why not state that this is a value between
790 and 1430 mV in 20mV increments?

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html