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 <[email protected]>
>> Cc: Tero Kristo <[email protected]>
>> Cc: Paul Walmsley <[email protected]>
>> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
>> ---
>> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
