| Message ID | 1409033072-10508-1-git-send-email-peppe.cavallaro@st.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Giuseppe Cavallaro <peppe.cavallaro@st.com> Date: Tue, 26 Aug 2014 08:04:32 +0200 > @@ -638,6 +638,15 @@ static int stmmac_init_ptp(struct stmmac_priv *priv) > if (!(priv->dma_cap.time_stamp || priv->dma_cap.atime_stamp)) > return -EOPNOTSUPP; > > + /* Fall-back to main clock in case of no PTP ref is passed */ > + priv->clk_ptp_ref = devm_clk_get(priv->device, "clk_ptp_ref"); > + if (IS_ERR(priv->clk_ptp_ref)) { > + priv->clk_ptp_rate = clk_get_rate(priv->stmmac_clk); This code path will leave an IS_ERR() value in clk_ptp_ref. ... > static void stmmac_release_ptp(struct stmmac_priv *priv) > { > + if (priv->clk_ptp_ref) > + clk_disable_unprepare(priv->clk_ptp_ref); > stmmac_ptp_unregister(priv); > } Which this code will erroneously pass into clk_disable_unprepare(). Please NULL the pointer out properl when devm_clk_get() errors. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi David On 8/28/2014 1:26 AM, David Miller wrote: > From: Giuseppe Cavallaro <peppe.cavallaro@st.com> > Date: Tue, 26 Aug 2014 08:04:32 +0200 > >> @@ -638,6 +638,15 @@ static int stmmac_init_ptp(struct stmmac_priv *priv) >> if (!(priv->dma_cap.time_stamp || priv->dma_cap.atime_stamp)) >> return -EOPNOTSUPP; >> >> + /* Fall-back to main clock in case of no PTP ref is passed */ >> + priv->clk_ptp_ref = devm_clk_get(priv->device, "clk_ptp_ref"); >> + if (IS_ERR(priv->clk_ptp_ref)) { >> + priv->clk_ptp_rate = clk_get_rate(priv->stmmac_clk); > > This code path will leave an IS_ERR() value in clk_ptp_ref. > ... >> static void stmmac_release_ptp(struct stmmac_priv *priv) >> { >> + if (priv->clk_ptp_ref) >> + clk_disable_unprepare(priv->clk_ptp_ref); >> stmmac_ptp_unregister(priv); >> } > > Which this code will erroneously pass into clk_disable_unprepare(). > > Please NULL the pointer out properl when devm_clk_get() errors. I had sent the v2, let me know if it's ok for you. peppe > > Thanks. > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt index 9b03c57..eeed4e30 100644 --- a/Documentation/devicetree/bindings/net/stmmac.txt +++ b/Documentation/devicetree/bindings/net/stmmac.txt @@ -39,6 +39,10 @@ Optional properties: further clocks may be specified in derived bindings. - clock-names: One name for each entry in the clocks property, the first one should be "stmmaceth". +- clk_ptp_ref: this is the PTP reference clock; in case of the PTP is + available this clock is used for programming the Timestamp Addend Register. + If not passed then the system clock will be used and this is fine on some + platforms. Examples: diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h index ca01035..128a0b7 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h @@ -105,6 +105,8 @@ struct stmmac_priv { struct ptp_clock *ptp_clock; struct ptp_clock_info ptp_clock_ops; unsigned int default_addend; + struct clk *clk_ptp_ref; + unsigned int clk_ptp_rate; u32 adv_ts; int use_riwt; int irq_wake; diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 51a89d4..8dbd9c0 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -603,16 +603,16 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr) /* calculate default added value: * formula is : * addend = (2^32)/freq_div_ratio; - * where, freq_div_ratio = STMMAC_SYSCLOCK/50MHz - * hence, addend = ((2^32) * 50MHz)/STMMAC_SYSCLOCK; - * NOTE: STMMAC_SYSCLOCK should be >= 50MHz to + * where, freq_div_ratio = clk_ptp_ref_i/50MHz + * hence, addend = ((2^32) * 50MHz)/clk_ptp_ref_i; + * NOTE: clk_ptp_ref_i should be >= 50MHz to * achive 20ns accuracy. * * 2^x * y == (y << x), hence * 2^32 * 50000000 ==> (50000000 << 32) */ temp = (u64) (50000000ULL << 32); - priv->default_addend = div_u64(temp, STMMAC_SYSCLOCK); + priv->default_addend = div_u64(temp, priv->clk_ptp_rate); priv->hw->ptp->config_addend(priv->ioaddr, priv->default_addend); @@ -638,6 +638,15 @@ static int stmmac_init_ptp(struct stmmac_priv *priv) if (!(priv->dma_cap.time_stamp || priv->dma_cap.atime_stamp)) return -EOPNOTSUPP; + /* Fall-back to main clock in case of no PTP ref is passed */ + priv->clk_ptp_ref = devm_clk_get(priv->device, "clk_ptp_ref"); + if (IS_ERR(priv->clk_ptp_ref)) { + priv->clk_ptp_rate = clk_get_rate(priv->stmmac_clk); + } else { + clk_prepare_enable(priv->clk_ptp_ref); + priv->clk_ptp_rate = clk_get_rate(priv->clk_ptp_ref); + } + priv->adv_ts = 0; if (priv->dma_cap.atime_stamp && priv->extend_desc) priv->adv_ts = 1; @@ -657,6 +666,8 @@ static int stmmac_init_ptp(struct stmmac_priv *priv) static void stmmac_release_ptp(struct stmmac_priv *priv) { + if (priv->clk_ptp_ref) + clk_disable_unprepare(priv->clk_ptp_ref); stmmac_ptp_unregister(priv); } diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h index 3dbc047..4535df3 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h @@ -25,8 +25,6 @@ #ifndef __STMMAC_PTP_H__ #define __STMMAC_PTP_H__ -#define STMMAC_SYSCLOCK 62500000 - /* IEEE 1588 PTP register offsets */ #define PTP_TCR 0x0700 /* Timestamp Control Reg */ #define PTP_SSIR 0x0704 /* Sub-Second Increment Reg */
The PTP reference clock, used for setting the addend in the Timestamp Addend Register, was erroneously hard-coded (as reported in the databook just as example). The patch removes the macro named: STMMAC_SYSCLOCK and allows to use a reference clock (clk_ptp_ref_i) that can be passed from the platform. If not passed, the main driver clock will be used as default; note that this can be fine on some platforms. Note that, prior this patch, using the old STMMAC_SYSCLOCK on some platforms, as side effect, the ptp clock can move faster/slower than the system clock. Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com> --- Documentation/devicetree/bindings/net/stmmac.txt | 4 ++++ drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 ++ drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 19 +++++++++++++++---- drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h | 2 -- 4 files changed, 21 insertions(+), 6 deletions(-)