* [PATCH v2 1/1] net: fec: ptp: avoid register access when ipg clock is disabled
@ 2021-02-25 21:15 Heiko Thiery
2021-02-26 7:22 ` Heiko Thiery
2021-02-26 15:23 ` Richard Cochran
0 siblings, 2 replies; 6+ messages in thread
From: Heiko Thiery @ 2021-02-25 21:15 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Fugang Duan, David S . Miller, Jakub Kicinski, Richard Cochran,
Heiko Thiery
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 <heiko.thiery@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] net: fec: ptp: avoid register access when ipg clock is disabled
2021-02-25 21:15 [PATCH v2 1/1] net: fec: ptp: avoid register access when ipg clock is disabled Heiko Thiery
@ 2021-02-26 7:22 ` Heiko Thiery
2021-02-26 15:23 ` Richard Cochran
1 sibling, 0 replies; 6+ messages in thread
From: Heiko Thiery @ 2021-02-26 7:22 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Fugang Duan, David S . Miller, Jakub Kicinski, Richard Cochran
Hi all,
Am Do., 25. Feb. 2021 um 22:15 Uhr schrieb Heiko Thiery
<heiko.thiery@gmail.com>:
>
> 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 <heiko.thiery@gmail.com>
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] net: fec: ptp: avoid register access when ipg clock is disabled
2021-02-25 21:15 [PATCH v2 1/1] net: fec: ptp: avoid register access when ipg clock is disabled Heiko Thiery
2021-02-26 7:22 ` Heiko Thiery
@ 2021-02-26 15:23 ` Richard Cochran
2021-02-26 23:44 ` Jakub Kicinski
1 sibling, 1 reply; 6+ messages in thread
From: Richard Cochran @ 2021-02-26 15:23 UTC (permalink / raw)
To: Heiko Thiery
Cc: netdev, linux-kernel, Fugang Duan, David S . Miller, Jakub Kicinski
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 <heiko.thiery@gmail.com>
> ---
> 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 <richardcochran@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] net: fec: ptp: avoid register access when ipg clock is disabled
2021-02-26 15:23 ` Richard Cochran
@ 2021-02-26 23:44 ` Jakub Kicinski
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2021-02-26 23:44 UTC (permalink / raw)
To: Richard Cochran
Cc: Heiko Thiery, netdev, linux-kernel, Fugang Duan, David S . Miller
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 <heiko.thiery@gmail.com>
> > ---
> > 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 <richardcochran@gmail.com>
Applied, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] net: fec: ptp: avoid register access when ipg clock is disabled
2021-02-25 20:55 Heiko Thiery
@ 2021-02-25 22:34 ` kernel test robot
0 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2021-02-25 22:34 UTC (permalink / raw)
To: Heiko Thiery, netdev, linux-kernel
Cc: kbuild-all, clang-built-linux, Fugang Duan, David S . Miller,
Jakub Kicinski, Richard Cochran, Heiko Thiery
[-- Attachment #1: Type: text/plain, Size: 3810 bytes --]
Hi Heiko,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net/master]
[also build test ERROR on net-next/master ipvs/master linus/master v5.11 next-20210225]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Heiko-Thiery/net-fec-ptp-avoid-register-access-when-ipg-clock-is-disabled/20210226-050218
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 6cf739131a15e4177e58a1b4f2bede9d5da78552
config: mips-randconfig-r013-20210225 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project a921aaf789912d981cbb2036bdc91ad7289e1523)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://github.com/0day-ci/linux/commit/a7258bb15d947bb808b6209012a56ae993ec6001
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Heiko-Thiery/net-fec-ptp-avoid-register-access-when-ipg-clock-is-disabled/20210226-050218
git checkout a7258bb15d947bb808b6209012a56ae993ec6001
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
>> drivers/net/ethernet/freescale/fec_ptp.c:380:14: error: use of undeclared identifier 'fep'
mutex_lock(&fep->ptp_clk_mutex);
^
drivers/net/ethernet/freescale/fec_ptp.c:383:17: error: use of undeclared identifier 'fep'
mutex_unlock(&fep->ptp_clk_mutex);
^
>> drivers/net/ethernet/freescale/fec_ptp.c:384:3: warning: misleading indentation; statement is not part of the previous 'if' [-Wmisleading-indentation]
return -EINVAL;
^
drivers/net/ethernet/freescale/fec_ptp.c:382:2: note: previous statement is here
if (!adapter->ptp_clk_on)
^
drivers/net/ethernet/freescale/fec_ptp.c:388:16: error: use of undeclared identifier 'fep'
mutex_unlock(&fep->ptp_clk_mutex);
^
1 warning and 3 errors generated.
vim +/fep +380 drivers/net/ethernet/freescale/fec_ptp.c
364
365 /**
366 * fec_ptp_gettime
367 * @ptp: the ptp clock structure
368 * @ts: timespec structure to hold the current time value
369 *
370 * read the timecounter and return the correct value on ns,
371 * after converting it into a struct timespec.
372 */
373 static int fec_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
374 {
375 struct fec_enet_private *adapter =
376 container_of(ptp, struct fec_enet_private, ptp_caps);
377 u64 ns;
378 unsigned long flags;
379
> 380 mutex_lock(&fep->ptp_clk_mutex);
381 /* Check the ptp clock */
382 if (!adapter->ptp_clk_on)
383 mutex_unlock(&fep->ptp_clk_mutex);
> 384 return -EINVAL;
385 spin_lock_irqsave(&adapter->tmreg_lock, flags);
386 ns = timecounter_read(&adapter->tc);
387 spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
388 mutex_unlock(&fep->ptp_clk_mutex);
389
390 *ts = ns_to_timespec64(ns);
391
392 return 0;
393 }
394
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30869 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/1] net: fec: ptp: avoid register access when ipg clock is disabled
@ 2021-02-25 20:55 Heiko Thiery
2021-02-25 22:34 ` kernel test robot
0 siblings, 1 reply; 6+ messages in thread
From: Heiko Thiery @ 2021-02-25 20:55 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Fugang Duan, David S . Miller, Jakub Kicinski, Richard Cochran,
Heiko Thiery
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 <heiko.thiery@gmail.com>
---
v2:
- add mutex (thanks to Richard)
drivers/net/ethernet/freescale/fec_ptp.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 2e344aada4c6..22f5e800c2d7 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -377,9 +377,15 @@ static int fec_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
u64 ns;
unsigned long flags;
+ mutex_lock(&fep->ptp_clk_mutex);
+ /* Check the ptp clock */
+ if (!adapter->ptp_clk_on)
+ mutex_unlock(&fep->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(&fep->ptp_clk_mutex);
*ts = ns_to_timespec64(ns);
--
2.30.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-26 23:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-25 21:15 [PATCH v2 1/1] net: fec: ptp: avoid register access when ipg clock is disabled Heiko Thiery
2021-02-26 7:22 ` Heiko Thiery
2021-02-26 15:23 ` Richard Cochran
2021-02-26 23:44 ` Jakub Kicinski
-- strict thread matches above, loose matches on Subject: below --
2021-02-25 20:55 Heiko Thiery
2021-02-25 22:34 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).