Hi Roger, On Wednesday 14 May 2014 06:46 PM, Roger Quadros wrote: > Hi Kishon, > > On 05/06/2014 04:33 PM, Kishon Vijay Abraham I wrote: >> APLL used by PCIE phy can either use external clock as input or the clock >> from DPLL. Added support for the APLL to use external clock as input here. >> >> Cc: Rajendra Nayak <rna...@ti.com> >> Cc: Tero Kristo <t-kri...@ti.com> >> Cc: Paul Walmsley <p...@pwsan.com> >> Signed-off-by: Kishon Vijay Abraham I <kis...@ti.com> >> --- >> Documentation/devicetree/bindings/phy/ti-phy.txt | 4 ++ >> drivers/phy/phy-ti-pipe3.c | 75 >> ++++++++++++++-------- >> 2 files changed, 52 insertions(+), 27 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt >> b/Documentation/devicetree/bindings/phy/ti-phy.txt >> index bc9afb5..d50f8ee 100644 >> --- a/Documentation/devicetree/bindings/phy/ti-phy.txt >> +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt >> @@ -76,6 +76,10 @@ Required properties: >> * "dpll_ref_m2" - external dpll ref clk >> * "phy-div" - divider for apll >> * "div-clk" - apll clock >> + * "apll_mux" - mux for pcie apll >> + * "refclk_ext" - external reference clock for pcie apll >> + - ti,ext-clk: To specifiy if PCIE apll should use external clock. >> Applicable >> + only to PCIE PHY. > > Instead of specifying both clock sources "dpll_ref_clock", "refclk_ext" and > then specifying a 3rd control option "ti,ext-clk" to select one of the 2 > sources, why can't the DT just supply one clock source, i.e. the one that is > being used in the board instance? The driver should then just configure the > clock rate that is needed at that node. Shouldn't the clock framework > automatically take care of muxing and parent rates?
Want the dt to have all the clocks used by the controller. "ti,ext-clk" should go in the board dt file (suggested by Nishanth). The point is at some point later if some one wants to change the clock source, it should be a simple enabling "ti,ext-clk" flag instead of finding the clock phandle etc.. > >> >> Optional properties: >> - ctrl-module : phandle of the control module used by PHY driver to power >> on >> diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c >> index d43019d..5513aa0 100644 >> --- a/drivers/phy/phy-ti-pipe3.c >> +++ b/drivers/phy/phy-ti-pipe3.c >> @@ -293,7 +293,7 @@ static int ti_pipe3_probe(struct platform_device *pdev) >> struct device_node *control_node; >> struct platform_device *control_pdev; >> const struct of_device_id *match; >> - struct clk *clk; >> + struct clk *clk, *pclk; >> >> phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL); >> if (!phy) { >> @@ -302,6 +302,20 @@ static int ti_pipe3_probe(struct platform_device *pdev) >> } >> phy->dev = &pdev->dev; >> >> + control_node = of_parse_phandle(node, "ctrl-module", 0); >> + if (!control_node) { >> + dev_err(&pdev->dev, "Failed to get control device phandle\n"); >> + return -EINVAL; >> + } >> + >> + control_pdev = of_find_device_by_node(control_node); >> + if (!control_pdev) { >> + dev_err(&pdev->dev, "Failed to get control device\n"); >> + return -EINVAL; >> + } >> + >> + phy->control_dev = &control_pdev->dev; >> + > > Why this code was moved move is not part of patch description/subject. external clock support needs 'control_dev', so had to move the get control device part before configuring the clocks. > >> if (!of_device_is_compatible(node, "ti,phy-pipe3-pcie")) { >> match = of_match_device(of_match_ptr(ti_pipe3_id_table), >> &pdev->dev); >> @@ -345,19 +359,40 @@ static int ti_pipe3_probe(struct platform_device *pdev) >> } >> >> if (of_device_is_compatible(node, "ti,phy-pipe3-pcie")) { >> - clk = devm_clk_get(phy->dev, "dpll_ref"); >> - if (IS_ERR(clk)) { >> - dev_err(&pdev->dev, "unable to get dpll ref clk\n"); >> - return PTR_ERR(clk); >> + if (!of_property_read_bool(node, "ti,ext-clk")) { >> + clk = devm_clk_get(phy->dev, "dpll_ref"); >> + if (IS_ERR(clk)) { >> + dev_err(&pdev->dev, >> + "unable to get dpll ref clk\n"); >> + return PTR_ERR(clk); >> + } >> + clk_set_rate(clk, 1500000000); >> + >> + clk = devm_clk_get(phy->dev, "dpll_ref_m2"); >> + if (IS_ERR(clk)) { >> + dev_err(&pdev->dev, >> + "unable to get dpll ref m2 clk\n"); >> + return PTR_ERR(clk); >> + } >> + clk_set_rate(clk, 100000000); >> + } else { >> + omap_control_pcie_tx_rx_control(phy->control_dev, >> + OMAP_CTRL_PCIE_PHY_RX_ACSPCIE); ^^here Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html