Re: [PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
On Tue, May 27, 2014 at 8:54 PM, Mike Turquette mturque...@linaro.org wrote:
Quoting Nishanth Menon (2014-05-15 05:33:13)
On 05/15/2014 07:18 AM, Kishon Vijay Abraham I wrote:
Hi,
On Thursday 15 May 2014 05:42 PM, Nishanth Menon wrote:
On Thu, May 15, 2014 at 6:59 AM, Kishon Vijay Abraham I kis...@ti.com
wrote:
Hi Nishant,
On Thursday 15 May 2014 05:16 PM, Nishanth Menon wrote:
On Thu, May 15, 2014 at 4:25 AM, Roger Quadros rog...@ti.com wrote:
On 05/15/2014 12:15 PM, Kishon Vijay Abraham I wrote:
Hi Nishanth,
On Wednesday 14 May 2014 09:04 PM, Nishanth Menon wrote:
On Wed, May 14, 2014 at 10:19 AM, Kishon Vijay Abraham I
kis...@ti.com wrote:
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..
Wonder if that is implicit by the presence of refclk_ext in the
clocks provided?
IMO the presence of refclk_ext is useless unless the board
indicates it
provides the clock source.
refclk_ext holds phandle for *fixed-clock*, so irrespective of
whether the
board provides a clock or not, it can have that handle for
configuring in PRCM.
However if the board does not provide the clock source, configuring
refclk_ext
in PRCM is useless.
I think what Nishant meant is that if refclk_ext is provided it
means that the driver
should use that over dpll_ref_clock so no need of a separate
ti,ext-clk flag.
yes, thank you for clarifying - it does indeed redundant to have
ti,ext-clk. and apologies on being a little obscure in the comment.
Irrespective of whether external reference clock is used or not, all DRA7
(apll) has an input for external reference clock (and also a PRCM
register for
programming it) and it has to be specified in dt no?
Why is that a binding for ti-phy? that is a problem for the APLL clock
driver (selecting it's own source). PHY properties should describe
itself - let the bindings of the APLL describe itself. please dont
mix the two up.
The apll clock node is like this
apll_pcie_in_clk_mux: apll_pcie_in_clk_mux@4ae06118 {
compatible = mux-clock;
clocks = dpll_pcie_ref_m2ldo_ck, pciesref_acs_clk_ck;
#clock-cells = 0;
reg = 0x4a00821c 0x4;
bit-mask = 0x80;
};
The external reference clock is denoted by *pciesref_acs_clk_ck*.
refclk_ext holds the phandle to *pciesref_acs_clk_ck* and is used in
clk_set_parent to set the parent of apll mux.
So, How about this: if refclk_ext is not defined, dont do setparent,
if it is defined, do setparent.
in short:
Optional Properties:
* refclk_ext - external reference clock for pcie apll - if defined,
used as the parent to apll_mux
That said, my problem in general with this approach(which many folks
have taken of describing parent of clock X in hardware block binding
for Y) is the following:
The binding now has dependency on clock tree hierarchy. What if
towmorrow, we have a tree where refclk_ext parent of muxZ parent of
apll_mux? the
Re: [PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
Hi,
On Thursday 15 May 2014 06:03 PM, Nishanth Menon wrote:
On 05/15/2014 07:18 AM, Kishon Vijay Abraham I wrote:
Hi,
On Thursday 15 May 2014 05:42 PM, Nishanth Menon wrote:
On Thu, May 15, 2014 at 6:59 AM, Kishon Vijay Abraham I kis...@ti.com
wrote:
Hi Nishant,
On Thursday 15 May 2014 05:16 PM, Nishanth Menon wrote:
On Thu, May 15, 2014 at 4:25 AM, Roger Quadros rog...@ti.com wrote:
On 05/15/2014 12:15 PM, Kishon Vijay Abraham I wrote:
Hi Nishanth,
On Wednesday 14 May 2014 09:04 PM, Nishanth Menon wrote:
On Wed, May 14, 2014 at 10:19 AM, Kishon Vijay Abraham I
kis...@ti.com wrote:
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..
Wonder if that is implicit by the presence of refclk_ext in the
clocks provided?
IMO the presence of refclk_ext is useless unless the board indicates
it
provides the clock source.
refclk_ext holds phandle for *fixed-clock*, so irrespective of whether
the
board provides a clock or not, it can have that handle for configuring
in PRCM.
However if the board does not provide the clock source, configuring
refclk_ext
in PRCM is useless.
I think what Nishant meant is that if refclk_ext is provided it means
that the driver
should use that over dpll_ref_clock so no need of a separate
ti,ext-clk flag.
yes, thank you for clarifying - it does indeed redundant to have
ti,ext-clk. and apologies on being a little obscure in the comment.
Irrespective of whether external reference clock is used or not, all DRA7
(apll) has an input for external reference clock (and also a PRCM register
for
programming it) and it has to be specified in dt no?
Why is that a binding for ti-phy? that is a problem for the APLL clock
driver (selecting it's own source). PHY properties should describe
itself - let the bindings of the APLL describe itself. please dont
mix the two up.
The apll clock node is like this
apll_pcie_in_clk_mux: apll_pcie_in_clk_mux@4ae06118 {
compatible = mux-clock;
clocks = dpll_pcie_ref_m2ldo_ck, pciesref_acs_clk_ck;
#clock-cells = 0;
reg = 0x4a00821c 0x4;
bit-mask = 0x80;
};
The external reference clock is denoted by *pciesref_acs_clk_ck*.
refclk_ext holds the phandle to *pciesref_acs_clk_ck* and is used in
clk_set_parent to set the parent of apll mux.
So, How about this: if refclk_ext is not defined, dont do setparent,
if it is defined, do setparent.
in short:
Optional Properties:
* refclk_ext - external reference clock for pcie apll - if defined,
used as the parent to apll_mux
just realized, refclk_ext is not a property by itself but just one of the
phandle for clocks/clock-names. Since we want to indicate if we are using
external clock from *board* dts file, I'm not sure if there is a way to
*append* a property.
Thanks
Kishon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
Quoting Nishanth Menon (2014-05-15 05:33:13)
On 05/15/2014 07:18 AM, Kishon Vijay Abraham I wrote:
Hi,
On Thursday 15 May 2014 05:42 PM, Nishanth Menon wrote:
On Thu, May 15, 2014 at 6:59 AM, Kishon Vijay Abraham I kis...@ti.com
wrote:
Hi Nishant,
On Thursday 15 May 2014 05:16 PM, Nishanth Menon wrote:
On Thu, May 15, 2014 at 4:25 AM, Roger Quadros rog...@ti.com wrote:
On 05/15/2014 12:15 PM, Kishon Vijay Abraham I wrote:
Hi Nishanth,
On Wednesday 14 May 2014 09:04 PM, Nishanth Menon wrote:
On Wed, May 14, 2014 at 10:19 AM, Kishon Vijay Abraham I
kis...@ti.com wrote:
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..
Wonder if that is implicit by the presence of refclk_ext in the
clocks provided?
IMO the presence of refclk_ext is useless unless the board indicates
it
provides the clock source.
refclk_ext holds phandle for *fixed-clock*, so irrespective of whether
the
board provides a clock or not, it can have that handle for configuring
in PRCM.
However if the board does not provide the clock source, configuring
refclk_ext
in PRCM is useless.
I think what Nishant meant is that if refclk_ext is provided it means
that the driver
should use that over dpll_ref_clock so no need of a separate
ti,ext-clk flag.
yes, thank you for clarifying - it does indeed redundant to have
ti,ext-clk. and apologies on being a little obscure in the comment.
Irrespective of whether external reference clock is used or not, all DRA7
(apll) has an input for external reference clock (and also a PRCM
register for
programming it) and it has to be specified in dt no?
Why is that a binding for ti-phy? that is a problem for the APLL clock
driver (selecting it's own source). PHY properties should describe
itself - let the bindings of the APLL describe itself. please dont
mix the two up.
The apll clock node is like this
apll_pcie_in_clk_mux: apll_pcie_in_clk_mux@4ae06118 {
compatible = mux-clock;
clocks = dpll_pcie_ref_m2ldo_ck, pciesref_acs_clk_ck;
#clock-cells = 0;
reg = 0x4a00821c 0x4;
bit-mask = 0x80;
};
The external reference clock is denoted by *pciesref_acs_clk_ck*.
refclk_ext holds the phandle to *pciesref_acs_clk_ck* and is used in
clk_set_parent to set the parent of apll mux.
So, How about this: if refclk_ext is not defined, dont do setparent,
if it is defined, do setparent.
in short:
Optional Properties:
* refclk_ext - external reference clock for pcie apll - if defined,
used as the parent to apll_mux
That said, my problem in general with this approach(which many folks
have taken of describing parent of clock X in hardware block binding
for Y) is the following:
The binding now has dependency on clock tree hierarchy. What if
towmorrow, we have a tree where refclk_ext parent of muxZ parent of
apll_mux? the binding breaks down. Lets not forget that we consider DT
as an ABI - we
Re: [PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
Hi Nishanth, On Wednesday 14 May 2014 09:04 PM, Nishanth Menon wrote: On Wed, May 14, 2014 at 10:19 AM, Kishon Vijay Abraham I kis...@ti.com wrote: 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.. Wonder if that is implicit by the presence of refclk_ext in the clocks provided? IMO the presence of refclk_ext is useless unless the board indicates it provides the clock source. refclk_ext holds phandle for *fixed-clock*, so irrespective of whether the board provides a clock or not, it can have that handle for configuring in PRCM. However if the board does not provide the clock source, configuring refclk_ext in PRCM is useless. Thanks Kishon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
On 05/15/2014 12:15 PM, Kishon Vijay Abraham I wrote: Hi Nishanth, On Wednesday 14 May 2014 09:04 PM, Nishanth Menon wrote: On Wed, May 14, 2014 at 10:19 AM, Kishon Vijay Abraham I kis...@ti.com wrote: 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.. Wonder if that is implicit by the presence of refclk_ext in the clocks provided? IMO the presence of refclk_ext is useless unless the board indicates it provides the clock source. refclk_ext holds phandle for *fixed-clock*, so irrespective of whether the board provides a clock or not, it can have that handle for configuring in PRCM. However if the board does not provide the clock source, configuring refclk_ext in PRCM is useless. I think what Nishant meant is that if refclk_ext is provided it means that the driver should use that over dpll_ref_clock so no need of a separate ti,ext-clk flag. cheers, -roger -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
On Thu, May 15, 2014 at 4:25 AM, Roger Quadros rog...@ti.com wrote: On 05/15/2014 12:15 PM, Kishon Vijay Abraham I wrote: Hi Nishanth, On Wednesday 14 May 2014 09:04 PM, Nishanth Menon wrote: On Wed, May 14, 2014 at 10:19 AM, Kishon Vijay Abraham I kis...@ti.com wrote: 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.. Wonder if that is implicit by the presence of refclk_ext in the clocks provided? IMO the presence of refclk_ext is useless unless the board indicates it provides the clock source. refclk_ext holds phandle for *fixed-clock*, so irrespective of whether the board provides a clock or not, it can have that handle for configuring in PRCM. However if the board does not provide the clock source, configuring refclk_ext in PRCM is useless. I think what Nishant meant is that if refclk_ext is provided it means that the driver should use that over dpll_ref_clock so no need of a separate ti,ext-clk flag. yes, thank you for clarifying - it does indeed redundant to have ti,ext-clk. and apologies on being a little obscure in the comment. Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
Hi Nishant, On Thursday 15 May 2014 05:16 PM, Nishanth Menon wrote: On Thu, May 15, 2014 at 4:25 AM, Roger Quadros rog...@ti.com wrote: On 05/15/2014 12:15 PM, Kishon Vijay Abraham I wrote: Hi Nishanth, On Wednesday 14 May 2014 09:04 PM, Nishanth Menon wrote: On Wed, May 14, 2014 at 10:19 AM, Kishon Vijay Abraham I kis...@ti.com wrote: 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.. Wonder if that is implicit by the presence of refclk_ext in the clocks provided? IMO the presence of refclk_ext is useless unless the board indicates it provides the clock source. refclk_ext holds phandle for *fixed-clock*, so irrespective of whether the board provides a clock or not, it can have that handle for configuring in PRCM. However if the board does not provide the clock source, configuring refclk_ext in PRCM is useless. I think what Nishant meant is that if refclk_ext is provided it means that the driver should use that over dpll_ref_clock so no need of a separate ti,ext-clk flag. yes, thank you for clarifying - it does indeed redundant to have ti,ext-clk. and apologies on being a little obscure in the comment. Irrespective of whether external reference clock is used or not, all DRA7 (apll) has an input for external reference clock (and also a PRCM register for programming it) and it has to be specified in dt no? Thanks Kishon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
On Thu, May 15, 2014 at 6:59 AM, Kishon Vijay Abraham I kis...@ti.com wrote: Hi Nishant, On Thursday 15 May 2014 05:16 PM, Nishanth Menon wrote: On Thu, May 15, 2014 at 4:25 AM, Roger Quadros rog...@ti.com wrote: On 05/15/2014 12:15 PM, Kishon Vijay Abraham I wrote: Hi Nishanth, On Wednesday 14 May 2014 09:04 PM, Nishanth Menon wrote: On Wed, May 14, 2014 at 10:19 AM, Kishon Vijay Abraham I kis...@ti.com wrote: 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.. Wonder if that is implicit by the presence of refclk_ext in the clocks provided? IMO the presence of refclk_ext is useless unless the board indicates it provides the clock source. refclk_ext holds phandle for *fixed-clock*, so irrespective of whether the board provides a clock or not, it can have that handle for configuring in PRCM. However if the board does not provide the clock source, configuring refclk_ext in PRCM is useless. I think what Nishant meant is that if refclk_ext is provided it means that the driver should use that over dpll_ref_clock so no need of a separate ti,ext-clk flag. yes, thank you for clarifying - it does indeed redundant to have ti,ext-clk. and apologies on being a little obscure in the comment. Irrespective of whether external reference clock is used or not, all DRA7 (apll) has an input for external reference clock (and also a PRCM register for programming it) and it has to be specified in dt no? Why is that a binding for ti-phy? that is a problem for the APLL clock driver (selecting it's own source). PHY properties should describe itself - let the bindings of the APLL describe itself. please dont mix the two up. Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
Hi,
On Thursday 15 May 2014 05:42 PM, Nishanth Menon wrote:
On Thu, May 15, 2014 at 6:59 AM, Kishon Vijay Abraham I kis...@ti.com wrote:
Hi Nishant,
On Thursday 15 May 2014 05:16 PM, Nishanth Menon wrote:
On Thu, May 15, 2014 at 4:25 AM, Roger Quadros rog...@ti.com wrote:
On 05/15/2014 12:15 PM, Kishon Vijay Abraham I wrote:
Hi Nishanth,
On Wednesday 14 May 2014 09:04 PM, Nishanth Menon wrote:
On Wed, May 14, 2014 at 10:19 AM, Kishon Vijay Abraham I kis...@ti.com
wrote:
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..
Wonder if that is implicit by the presence of refclk_ext in the
clocks provided?
IMO the presence of refclk_ext is useless unless the board indicates it
provides the clock source.
refclk_ext holds phandle for *fixed-clock*, so irrespective of whether the
board provides a clock or not, it can have that handle for configuring in
PRCM.
However if the board does not provide the clock source, configuring
refclk_ext
in PRCM is useless.
I think what Nishant meant is that if refclk_ext is provided it means
that the driver
should use that over dpll_ref_clock so no need of a separate
ti,ext-clk flag.
yes, thank you for clarifying - it does indeed redundant to have
ti,ext-clk. and apologies on being a little obscure in the comment.
Irrespective of whether external reference clock is used or not, all DRA7
(apll) has an input for external reference clock (and also a PRCM register
for
programming it) and it has to be specified in dt no?
Why is that a binding for ti-phy? that is a problem for the APLL clock
driver (selecting it's own source). PHY properties should describe
itself - let the bindings of the APLL describe itself. please dont
mix the two up.
The apll clock node is like this
apll_pcie_in_clk_mux: apll_pcie_in_clk_mux@4ae06118 {
compatible = mux-clock;
clocks = dpll_pcie_ref_m2ldo_ck, pciesref_acs_clk_ck;
#clock-cells = 0;
reg = 0x4a00821c 0x4;
bit-mask = 0x80;
};
The external reference clock is denoted by *pciesref_acs_clk_ck*.
refclk_ext holds the phandle to *pciesref_acs_clk_ck* and is used in
clk_set_parent to set the parent of apll mux.
Thanks
Kishon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
On 05/15/2014 07:18 AM, Kishon Vijay Abraham I wrote:
Hi,
On Thursday 15 May 2014 05:42 PM, Nishanth Menon wrote:
On Thu, May 15, 2014 at 6:59 AM, Kishon Vijay Abraham I kis...@ti.com
wrote:
Hi Nishant,
On Thursday 15 May 2014 05:16 PM, Nishanth Menon wrote:
On Thu, May 15, 2014 at 4:25 AM, Roger Quadros rog...@ti.com wrote:
On 05/15/2014 12:15 PM, Kishon Vijay Abraham I wrote:
Hi Nishanth,
On Wednesday 14 May 2014 09:04 PM, Nishanth Menon wrote:
On Wed, May 14, 2014 at 10:19 AM, Kishon Vijay Abraham I
kis...@ti.com wrote:
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..
Wonder if that is implicit by the presence of refclk_ext in the
clocks provided?
IMO the presence of refclk_ext is useless unless the board indicates it
provides the clock source.
refclk_ext holds phandle for *fixed-clock*, so irrespective of whether
the
board provides a clock or not, it can have that handle for configuring
in PRCM.
However if the board does not provide the clock source, configuring
refclk_ext
in PRCM is useless.
I think what Nishant meant is that if refclk_ext is provided it means
that the driver
should use that over dpll_ref_clock so no need of a separate
ti,ext-clk flag.
yes, thank you for clarifying - it does indeed redundant to have
ti,ext-clk. and apologies on being a little obscure in the comment.
Irrespective of whether external reference clock is used or not, all DRA7
(apll) has an input for external reference clock (and also a PRCM register
for
programming it) and it has to be specified in dt no?
Why is that a binding for ti-phy? that is a problem for the APLL clock
driver (selecting it's own source). PHY properties should describe
itself - let the bindings of the APLL describe itself. please dont
mix the two up.
The apll clock node is like this
apll_pcie_in_clk_mux: apll_pcie_in_clk_mux@4ae06118 {
compatible = mux-clock;
clocks = dpll_pcie_ref_m2ldo_ck, pciesref_acs_clk_ck;
#clock-cells = 0;
reg = 0x4a00821c 0x4;
bit-mask = 0x80;
};
The external reference clock is denoted by *pciesref_acs_clk_ck*.
refclk_ext holds the phandle to *pciesref_acs_clk_ck* and is used in
clk_set_parent to set the parent of apll mux.
So, How about this: if refclk_ext is not defined, dont do setparent,
if it is defined, do setparent.
in short:
Optional Properties:
* refclk_ext - external reference clock for pcie apll - if defined,
used as the parent to apll_mux
That said, my problem in general with this approach(which many folks
have taken of describing parent of clock X in hardware block binding
for Y) is the following:
The binding now has dependency on clock tree hierarchy. What if
towmorrow, we have a tree where refclk_ext parent of muxZ parent of
apll_mux? the binding breaks down. Lets not forget that we consider DT
as an ABI - we dont want to change the binding in the future.
I had always preferred describing parent-child relationship of clocks
by the approach Tero posted:
Re: [PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
Hi,
On Thursday 15 May 2014 06:03 PM, Nishanth Menon wrote:
On 05/15/2014 07:18 AM, Kishon Vijay Abraham I wrote:
Hi,
On Thursday 15 May 2014 05:42 PM, Nishanth Menon wrote:
On Thu, May 15, 2014 at 6:59 AM, Kishon Vijay Abraham I kis...@ti.com
wrote:
Hi Nishant,
On Thursday 15 May 2014 05:16 PM, Nishanth Menon wrote:
On Thu, May 15, 2014 at 4:25 AM, Roger Quadros rog...@ti.com wrote:
On 05/15/2014 12:15 PM, Kishon Vijay Abraham I wrote:
Hi Nishanth,
On Wednesday 14 May 2014 09:04 PM, Nishanth Menon wrote:
On Wed, May 14, 2014 at 10:19 AM, Kishon Vijay Abraham I
kis...@ti.com wrote:
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..
Wonder if that is implicit by the presence of refclk_ext in the
clocks provided?
IMO the presence of refclk_ext is useless unless the board indicates
it
provides the clock source.
refclk_ext holds phandle for *fixed-clock*, so irrespective of whether
the
board provides a clock or not, it can have that handle for configuring
in PRCM.
However if the board does not provide the clock source, configuring
refclk_ext
in PRCM is useless.
I think what Nishant meant is that if refclk_ext is provided it means
that the driver
should use that over dpll_ref_clock so no need of a separate
ti,ext-clk flag.
yes, thank you for clarifying - it does indeed redundant to have
ti,ext-clk. and apologies on being a little obscure in the comment.
Irrespective of whether external reference clock is used or not, all DRA7
(apll) has an input for external reference clock (and also a PRCM register
for
programming it) and it has to be specified in dt no?
Why is that a binding for ti-phy? that is a problem for the APLL clock
driver (selecting it's own source). PHY properties should describe
itself - let the bindings of the APLL describe itself. please dont
mix the two up.
The apll clock node is like this
apll_pcie_in_clk_mux: apll_pcie_in_clk_mux@4ae06118 {
compatible = mux-clock;
clocks = dpll_pcie_ref_m2ldo_ck, pciesref_acs_clk_ck;
#clock-cells = 0;
reg = 0x4a00821c 0x4;
bit-mask = 0x80;
};
The external reference clock is denoted by *pciesref_acs_clk_ck*.
refclk_ext holds the phandle to *pciesref_acs_clk_ck* and is used in
clk_set_parent to set the parent of apll mux.
So, How about this: if refclk_ext is not defined, dont do setparent,
if it is defined, do setparent.
in short:
Optional Properties:
* refclk_ext - external reference clock for pcie apll - if defined,
used as the parent to apll_mux
Ok, will remove ti,ext-clk.
Thanks
Kishon
That said, my problem in general with this approach(which many folks
have taken of describing parent of clock X in hardware block binding
for Y) is the following:
The binding now has dependency on clock tree hierarchy. What if
towmorrow, we have a tree where refclk_ext parent of muxZ parent of
apll_mux? the binding breaks down. Lets not forget that we consider DT
as an ABI - we dont want to change the binding in the future.
I had
Re: [PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
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?
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.
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, 15);
+
+ 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, 1);
+ } else {
+ omap_control_pcie_tx_rx_control(phy-control_dev,
+ OMAP_CTRL_PCIE_PHY_RX_ACSPCIE);
+
+ clk = clk_get(phy-dev, apll_mux);
+ if (IS_ERR(clk)) {
+ dev_err(pdev-dev, unable to get apll mux
clk\n);
+ return PTR_ERR(clk);
+ }
+
+ pclk = clk_get(phy-dev, refclk_ext);
+ if (IS_ERR(pclk)) {
+ dev_err(pdev-dev, unable to get ext ref clk
for apll\n);
+ return PTR_ERR(pclk);
+ }
+
+ clk_set_parent(clk, pclk);
}
-
Re: [PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
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, 15);
+
+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, 1);
+} 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 linux-omap in
the body of a message to
Re: [PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
On Wed, May 14, 2014 at 10:19 AM, Kishon Vijay Abraham I kis...@ti.com wrote: 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.. Wonder if that is implicit by the presence of refclk_ext in the clocks provided? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/17] phy: ti-pipe3: add external clock support for PCIe PHY
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.
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;
+
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, 15);
+
+ 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, 1);
+ } else {
+ omap_control_pcie_tx_rx_control(phy-control_dev,
+ OMAP_CTRL_PCIE_PHY_RX_ACSPCIE);
+
+ clk = clk_get(phy-dev, apll_mux);
+ if (IS_ERR(clk)) {
+ dev_err(pdev-dev, unable to get apll mux
clk\n);
+ return PTR_ERR(clk);
+ }
+
+ pclk = clk_get(phy-dev, refclk_ext);
+ if (IS_ERR(pclk)) {
+ dev_err(pdev-dev, unable to get ext ref clk
for apll\n);
+ return PTR_ERR(pclk);
+ }
+
+ clk_set_parent(clk, pclk);
}
- clk_set_rate(clk, 15);
-
- 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, 1);
clk = devm_clk_get(phy-dev, phy-div);
if (IS_ERR(clk)) {
@@ -375,20 +410,6 @@ static int ti_pipe3_probe(struct platform_device *pdev)
phy-div_clk = ERR_PTR(-ENODEV);
}
-
