2021-02-25 21:17:23

by Heiko Thiery

Subject: [PATCH v2 1/1] net: fec: ptp: avoid register access when ipg clock is disabled

When accessing the timecounter register on an i.MX8MQ the kernel hangs.
This is only the case when the interface is down. This can be reproduced
by reading with 'phc_ctrl eth0 get'.

Like described in the change in 91c0d987a9788dcc5fe26baafd73bf9242b68900
the igp clock is disabled when the interface is down and leads to a
system hang.

So we check if the ptp clock status before reading the timecounter
register.

Signed-off-by: Heiko Thiery <[email protected]>
---
v2:
- add mutex (thanks to Richard)

v3:
I did a mistake and did not test properly
- add parenteses
- fix the used variable

drivers/net/ethernet/freescale/fec_ptp.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 2e344aada4c6..1753807cbf97 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -377,9 +377,16 @@ static int fec_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
u64 ns;
unsigned long flags;

+ mutex_lock(&adapter->ptp_clk_mutex);
+ /* Check the ptp clock */
+ if (!adapter->ptp_clk_on) {
+ mutex_unlock(&adapter->ptp_clk_mutex);
+ return -EINVAL;
+ }
spin_lock_irqsave(&adapter->tmreg_lock, flags);
ns = timecounter_read(&adapter->tc);
spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
+ mutex_unlock(&adapter->ptp_clk_mutex);

*ts = ns_to_timespec64(ns);

--
2.30.0

2021-02-26 07:25:26

by Heiko Thiery

Subject: Re: [PATCH v2 1/1] net: fec: ptp: avoid register access when ipg clock is disabled

Hi all,

Am Do., 25. Feb. 2021 um 22:15 Uhr schrieb Heiko Thiery
<[email protected]>:
>
> When accessing the timecounter register on an i.MX8MQ the kernel hangs.
> This is only the case when the interface is down. This can be reproduced
> by reading with 'phc_ctrl eth0 get'.
>
> Like described in the change in 91c0d987a9788dcc5fe26baafd73bf9242b68900
> the igp clock is disabled when the interface is down and leads to a
> system hang.
>
> So we check if the ptp clock status before reading the timecounter
> register.
>
> Signed-off-by: Heiko Thiery <[email protected]>


Sorry for the noise. But just realized that I sent a v3 version of the
patch but forgot to update the subject line (still v2). Should I
resend it with the correct subject?

> ---
> v2:
> - add mutex (thanks to Richard)
>
> v3:
> I did a mistake and did not test properly
> - add parenteses
> - fix the used variable
>
> drivers/net/ethernet/freescale/fec_ptp.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
> index 2e344aada4c6..1753807cbf97 100644
> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> @@ -377,9 +377,16 @@ static int fec_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
> u64 ns;
> unsigned long flags;
>
> + mutex_lock(&adapter->ptp_clk_mutex);
> + /* Check the ptp clock */
> + if (!adapter->ptp_clk_on) {
> + mutex_unlock(&adapter->ptp_clk_mutex);
> + return -EINVAL;
> + }
> spin_lock_irqsave(&adapter->tmreg_lock, flags);
> ns = timecounter_read(&adapter->tc);
> spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
> + mutex_unlock(&adapter->ptp_clk_mutex);
>
> *ts = ns_to_timespec64(ns);
>
> --
> 2.30.0
>

2021-02-26 15:26:06

by Richard Cochran

Subject: Re: [PATCH v2 1/1] net: fec: ptp: avoid register access when ipg clock is disabled

On Thu, Feb 25, 2021 at 10:15:16PM +0100, Heiko Thiery wrote:
> When accessing the timecounter register on an i.MX8MQ the kernel hangs.
> This is only the case when the interface is down. This can be reproduced
> by reading with 'phc_ctrl eth0 get'.
>
> Like described in the change in 91c0d987a9788dcc5fe26baafd73bf9242b68900
> the igp clock is disabled when the interface is down and leads to a
> system hang.
>
> So we check if the ptp clock status before reading the timecounter
> register.
>
> Signed-off-by: Heiko Thiery <[email protected]>
> ---
> v2:
> - add mutex (thanks to Richard)
>
> v3:
> I did a mistake and did not test properly
> - add parenteses
> - fix the used variable

Acked-by: Richard Cochran <[email protected]>

2021-02-26 23:47:50

by Jakub Kicinski

Subject: Re: [PATCH v2 1/1] net: fec: ptp: avoid register access when ipg clock is disabled

On Fri, 26 Feb 2021 07:23:31 -0800 Richard Cochran wrote:
> On Thu, Feb 25, 2021 at 10:15:16PM +0100, Heiko Thiery wrote:
> > When accessing the timecounter register on an i.MX8MQ the kernel hangs.
> > This is only the case when the interface is down. This can be reproduced
> > by reading with 'phc_ctrl eth0 get'.
> >
> > Like described in the change in 91c0d987a9788dcc5fe26baafd73bf9242b68900
> > the igp clock is disabled when the interface is down and leads to a
> > system hang.
> >
> > So we check if the ptp clock status before reading the timecounter
> > register.
> >
> > Signed-off-by: Heiko Thiery <[email protected]>
> > ---
> > v2:
> > - add mutex (thanks to Richard)
> >
> > v3:
> > I did a mistake and did not test properly
> > - add parenteses
> > - fix the used variable

On Fri, 26 Feb 2021 08:22:50 +0100 Heiko Thiery wrote:
> Sorry for the noise. But just realized that I sent a v3 version of the
> patch but forgot to update the subject line (still v2). Should I
> resend it with the correct subject?

No need, looks like patchwork caught the right version.

> Acked-by: Richard Cochran <[email protected]>

Applied, thanks!