Hi Archit, On Tuesday 23 February 2016 03:06 PM, Archit Taneja wrote: > > > On 02/23/2016 12:57 AM, Rob Herring wrote: >> On Mon, Feb 22, 2016 at 5:07 AM, Archit Taneja <architt@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx >>>>> Cc: Rob Herring <robh@xxxxxxxxxx> >>>>> >>>>> Signed-off-by: Archit Taneja <architt@xxxxxxxxxxxxxx> >>>>> --- >>>>> .../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 right, the phy ops were intended to be used that way. However because of different sequence requirements for different controllers, that's not followed always. > 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) get_sync is invoked by the phy core during phy_init and phy_power_on (it was done basically to access registers that have to be configured during init and power on). regulator is enabled during phy_power_on and clocks (opt func clocks) should be enable during runtime callbacks in the driver (main clock is enabled as part of get_sync). > > 2. set_rate/enable the PLL clock provided by PHY If the PHY is the source of clock, then the PHY should also be modeled as clock provider. (see rockchip-usb PHY) > > 3. enable PHY (configure some PHY registers) the general configuration of the PHY should go in phy_init (e.g any calibration settings) and registers to power_on the PHY should go in phy_power_on. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
