Re: [PATCH v2 13/13] dt-bindings: msm/hdmi: Add HDMI PHY bindings

[Archit Taneja <architt@codeaurora.org>]

On 02/22/2016 08:24 AM, Rob Herring wrote:
> On Mon, Feb 15, 2016 at 12:23:26PM +0530, Archit Taneja wrote:
>> Add HDMI PHY bindings. Update the example to use HDMI PHY.
>>
>> Add a missing power-domains property in the HDMI core bindings.
>>
>> Cc: devicetree@vger.kernel.org
>> Cc: Rob Herring <robh@kernel.org>
>>
>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>> ---
>>   .../devicetree/bindings/display/msm/hdmi.txt       | 39 +++++++++++++++++++++-
>>   1 file changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/msm/hdmi.txt b/Documentation/devicetree/bindings/display/msm/hdmi.txt
>> index 379ee2e..4d71910 100644
>> --- a/Documentation/devicetree/bindings/display/msm/hdmi.txt
>> +++ b/Documentation/devicetree/bindings/display/msm/hdmi.txt
>> @@ -11,6 +11,7 @@ Required properties:
>>   - reg: Physical base address and length of the controller's registers
>>   - reg-names: "core_physical"
>>   - interrupts: The interrupt signal from the hdmi block.
>> +- power-domains: Should be <&mmcc MDSS_GDSC>.
>>   - clocks: device clocks
>>     See ../clocks/clock-bindings.txt for details.
>>   - qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin
>> @@ -18,6 +19,7 @@ Required properties:
>>   - qcom,hdmi-tx-hpd-gpio: hpd pin
>>   - core-vdda-supply: phandle to supply regulator
>>   - hdmi-mux-supply: phandle to mux regulator
>> +- qcom,hdmi-phy: phandle to HDMI PHY device node
>
> Why not use the generic phy binding?

You'd asked about this in the first version of this patch. You
probably missed reading my reply. Partially my fault since I
missed out putting the "In-Reply-to" when posting this set. I've
mentioned the reason again here:

The PHY in the HDMI and DSI blocks can't be implemented using the
common phy framework. The PHY blocks have a PLL sub-block within
them which acts as a pixel clock source for the display processor
block.

This dependency causes the need to split the phy power on sequence
into 2 parts (one to enable resources to enable the PLL, and the
other to enable the phy itself), which the phy framework can't
do. That's the main reason not to use it. There are some more
complex use cases for DSI PHY (drive two PHYs with the same
DSI PLL) which the phy framework can't support.

>
>>
>>   Optional properties:
>>   - qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin
>> @@ -27,6 +29,27 @@ Optional properties:
>>   - pinctrl-0: the default pinctrl state (active)
>>   - pinctrl-1: the "sleep" pinctrl state
>>
>> +HDMI PHY:
>> +Required properties:
>> +- compatible: Could be the following
>> +  * "qcom,hdmi-phy-8x60"
>> +  * "qcom,hdmi-phy-8960"
>> +  * "qcom,hdmi-phy-8x74"
>
> No wildcards please. Where's 8994?

I'll remove the wildcards. 8994 PHY isn't supported by the driver at
the moment. I could keep the 8994 compatible string, the driver will
bail out with an error. But that's something we already do for 8x74
since it doesn't have full PHY support either.

>
>> +  * "qcom,hdmi-phy-8996"
>> +- reg: Physical base address and length of the registers of the PHY sub blocks.
>> +- reg-names: The names of register regions. The following regions are required:
>> +  * "hdmi_pll"
>> +  * "hdmi_phy"
>> +  For HDMI PHY on msm8996, these additional register regions are required:
>> +    * "hdmi_tx_l0"
>> +    * "hdmi_tx_l1"
>> +    * "hdmi_tx_l3"
>> +    * "hdmi_tx_l4"
>> +- power-domains: Should be <&mmcc MDSS_GDSC>.
>> +- clocks: device clocks
>> +  See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
>> +- core-vdda-supply: phandle to vdda regulator device node
>> +
>>   Example:
>>
>>   / {
>> @@ -35,7 +58,7 @@ Example:
>>   	hdmi: qcom,hdmi-tx-8960@4a00000 {
>
> Node names should be generic, so just "hdmi".
>
>>   		compatible = "qcom,hdmi-tx-8960";
>>   		reg-names = "core_physical";
>> -		reg = <0x04a00000 0x1000>;
>> +		reg = <0x04a00000 0x2f0>;
>>   		interrupts = <GIC_SPI 79 0>;
>>   		power-domains = <&mmcc MDSS_GDSC>;
>>   		clock-names =
>> @@ -54,5 +77,19 @@ Example:
>>   		pinctrl-names = "default", "sleep";
>>   		pinctrl-0 = <&hpd_active  &ddc_active  &cec_active>;
>>   		pinctrl-1 = <&hpd_suspend &ddc_suspend &cec_suspend>;
>> +
>> +		qcom,hdmi-phy = <&hdmi_phy>;
>> +	};
>> +
>> +	hdmi_phy: qcom,hdmi-phy-8960@4a00400 {
>
> ditto. phy@...

I'll fix these.

Thanks,
Archit

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum, hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel