On 02/23/2016 12:57 AM, Rob Herring wrote: > On Mon, Feb 22, 2016 at 5:07 AM, Archit Taneja <architt at codeaurora.org> > wrote: >> >> >> 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 at vger.kernel.org >>>> Cc: Rob Herring <robh at kernel.org> >>>> >>>> Signed-off-by: Archit Taneja <architt at 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. > > That sounds like a problem with the common phy framework, not the DT > binding. Nothing says you have to use the common phy f/w if you use > the phy binding. I say that as the binding maintainer. As a kernel > developer, I would say fix the common phy framework to handle this > case. However, I don't think anything prevents the phy driver from > registering both a phy and a clock.
Okay. For some reason, I thought it would be wrong to use the same common phy bindings but use our own phy driver. I will convert the bindings to the generic phy binding. > >> 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. > > Doesn't the phy framework already support the former? It has power on > and init calls. Personally, I find the split there ill-defined. I always assumed that the init/exit ops were something that you would call just once (during probe/remove) and then forget about it. I only now noticed that init and power_on are often paired together (as are power_off and exit). I went through the common phy framework code in more detail, but I realized I would face the following issue: I was looking for the sequence: 1. enable PHY resources (enables clocks/regulators/pm_runtime_get_sync) 2. set_rate/enable the PLL clock provided by PHY 3. enable PHY (configure some PHY registers) I can achieve this using the common phy framework if I tie step 1) to phy_power_on(), and step 3) to phy_init(). The other way round won't work because phy_init() seems to disable runtime pm as it exits. If I use the phy framework in its current state, I'll end up stuffing the phy ops and use them in a weird way just to make sure my PHY works. Copying Kishon if he has any opinions. > >>>> 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. > > You can document it later if you want. I just asked 'cause I knew the > 8994 had one. Okay. Thanks, Archit -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
