[PATCH 0/2] net: phy: call phy_disable_interrupts() in phy_init

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0/2] net: phy: call phy_disable_interrupts() in phy_init_hw()
@ 2020-06-22 10:28 Jisheng Zhang
  2020-06-22 10:29 ` [PATCH 1/2] net: phy: export phy_disable_interrupts() Jisheng Zhang
  2020-06-22 10:29 ` [PATCH 2/2] net: phy: call phy_disable_interrupts() in phy_init_hw() Jisheng Zhang
  0 siblings, 2 replies; 5+ messages in thread
From: Jisheng Zhang @ 2020-06-22 10:28 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Heiner Kallweit, Russell King,
	David S. Miller, Jakub Kicinski
  Cc: netdev, linux-kernel

We face an issue with rtl8211f, a pin is shared between INTB and PMEB,
and the PHY Register Accessible Interrupt is enabled by default, so
the INTB/PMEB pin is always active in polling mode case.

As Heiner pointed out "I was thinking about calling
phy_disable_interrupts() in phy_init_hw(), to have a defined init
state as we don't know in which state the PHY is if the PHY driver is
loaded. We shouldn't assume that it's the chip power-on defaults, BIOS
or boot loader could have changed this. Or in case of dual-boot
systems the other OS could leave the PHY in whatever state."

patch1 exports phy_disable_interrupts() so that it could be used in
phy_init_hw() to have a defined init state.

patch2 calls phy_disable_interrupts() in phy_init_hw() to have a
defined init state.

Jisheng Zhang (2):
  net: phy: export phy_disable_interrupts()
  net: phy: call phy_disable_interrupts() in phy_init_hw()

 drivers/net/phy/phy.c        | 3 ++-
 drivers/net/phy/phy_device.c | 7 +++++--
 include/linux/phy.h          | 1 +
 3 files changed, 8 insertions(+), 3 deletions(-)

-- 
2.27.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] net: phy: export phy_disable_interrupts()
  2020-06-22 10:28 [PATCH 0/2] net: phy: call phy_disable_interrupts() in phy_init_hw() Jisheng Zhang
@ 2020-06-22 10:29 ` Jisheng Zhang
  2020-06-22 12:50   ` kernel test robot
  2020-06-22 16:30   ` kernel test robot
  2020-06-22 10:29 ` [PATCH 2/2] net: phy: call phy_disable_interrupts() in phy_init_hw() Jisheng Zhang
  1 sibling, 2 replies; 5+ messages in thread
From: Jisheng Zhang @ 2020-06-22 10:29 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Heiner Kallweit, Russell King,
	David S. Miller, Jakub Kicinski
  Cc: netdev, linux-kernel

We face an issue with rtl8211f, a pin is shared between INTB and PMEB,
and the PHY Register Accessible Interrupt is enabled by default, so
the INTB/PMEB pin is always active in polling mode case.

As Heiner pointed out "I was thinking about calling
phy_disable_interrupts() in phy_init_hw(), to have a defined init
state as we don't know in which state the PHY is if the PHY driver is
loaded. We shouldn't assume that it's the chip power-on defaults, BIOS
or boot loader could have changed this. Or in case of dual-boot
systems the other OS could leave the PHY in whatever state."

Export phy_disable_interrupts() so that it could be used in
phy_init_hw() to have a defined init state.

Suggested-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 drivers/net/phy/phy.c | 3 ++-
 include/linux/phy.h   | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 1de3938628f4..a3d92a15da71 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -840,7 +840,7 @@ static void phy_error(struct phy_device *phydev)
  * phy_disable_interrupts - Disable the PHY interrupts from the PHY side
  * @phydev: target phy_device struct
  */
-static int phy_disable_interrupts(struct phy_device *phydev)
+int phy_disable_interrupts(struct phy_device *phydev)
 {
 	int err;
 
@@ -852,6 +852,7 @@ static int phy_disable_interrupts(struct phy_device *phydev)
 	/* Clear the interrupt */
 	return phy_clear_interrupt(phydev);
 }
+EXPORT_SYMBOL_GPL(phy_request_interrupt);
 
 /**
  * phy_interrupt - PHY interrupt handler
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 8c05d0fb5c00..b693b609b2f5 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1416,6 +1416,7 @@ int phy_ethtool_ksettings_set(struct phy_device *phydev,
 int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd);
 int phy_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd);
 int phy_do_ioctl_running(struct net_device *dev, struct ifreq *ifr, int cmd);
+int phy_disable_interrupts(struct phy_device *phydev);
 void phy_request_interrupt(struct phy_device *phydev);
 void phy_free_interrupt(struct phy_device *phydev);
 void phy_print_status(struct phy_device *phydev);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] net: phy: export phy_disable_interrupts()
  2020-06-22 10:29 ` [PATCH 1/2] net: phy: export phy_disable_interrupts() Jisheng Zhang
@ 2020-06-22 12:50   ` kernel test robot
  2020-06-22 16:30   ` kernel test robot
  1 sibling, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-06-22 12:50 UTC (permalink / raw)
  To: Jisheng Zhang, Andrew Lunn, Florian Fainelli, Heiner Kallweit,
	Russell King, David S. Miller, Jakub Kicinski
  Cc: kbuild-all, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 8472 bytes --]

Hi Jisheng,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.8-rc2 next-20200622]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jisheng-Zhang/net-phy-call-phy_disable_interrupts-in-phy_init_hw/20200622-183400
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 625d3449788f85569096780592549d0340e9c0c7
config: um-allmodconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=um 

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 >>):

   cc1: warning: arch/um/include/uapi: No such file or directory [-Wmissing-include-dirs]
   In file included from include/linux/uaccess.h:11,
                    from include/linux/crypto.h:21,
                    from include/crypto/hash.h:11,
                    from include/linux/uio.h:10,
                    from include/linux/socket.h:8,
                    from include/linux/compat.h:15,
                    from include/linux/ethtool.h:17,
                    from include/linux/netdevice.h:37,
                    from drivers/net/phy/phy.c:17:
   arch/um/include/asm/uaccess.h: In function '__access_ok':
   arch/um/include/asm/uaccess.h:17:29: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
      17 |    (((unsigned long) (addr) >= FIXADDR_USER_START) && \
         |                             ^~
   arch/um/include/asm/uaccess.h:45:3: note: in expansion of macro '__access_ok_vsyscall'
      45 |   __access_ok_vsyscall(addr, size) ||
         |   ^~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/kernel.h:11,
                    from drivers/net/phy/phy.c:11:
   include/asm-generic/fixmap.h: In function 'fix_to_virt':
   include/asm-generic/fixmap.h:32:19: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
      32 |  BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
         |                   ^~
   include/linux/compiler.h:372:9: note: in definition of macro '__compiletime_assert'
     372 |   if (!(condition))     \
         |         ^~~~~~~~~
   include/linux/compiler.h:392:2: note: in expansion of macro '_compiletime_assert'
     392 |  _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |  ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:50:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
      50 |  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
         |  ^~~~~~~~~~~~~~~~
   include/asm-generic/fixmap.h:32:2: note: in expansion of macro 'BUILD_BUG_ON'
      32 |  BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
         |  ^~~~~~~~~~~~
   In file included from include/linux/linkage.h:7,
                    from include/linux/kernel.h:8,
                    from drivers/net/phy/phy.c:11:
   drivers/net/phy/phy.c: At top level:
>> include/linux/export.h:67:36: error: redefinition of '__ksymtab_phy_request_interrupt'
      67 |  static const struct kernel_symbol __ksymtab_##sym  \
         |                                    ^~~~~~~~~~
   include/linux/export.h:108:2: note: in expansion of macro '__KSYMTAB_ENTRY'
     108 |  __KSYMTAB_ENTRY(sym, sec)
         |  ^~~~~~~~~~~~~~~
   include/linux/export.h:147:39: note: in expansion of macro '___EXPORT_SYMBOL'
     147 | #define __EXPORT_SYMBOL(sym, sec, ns) ___EXPORT_SYMBOL(sym, sec, ns)
         |                                       ^~~~~~~~~~~~~~~~
   include/linux/export.h:155:34: note: in expansion of macro '__EXPORT_SYMBOL'
     155 | #define _EXPORT_SYMBOL(sym, sec) __EXPORT_SYMBOL(sym, sec, "")
         |                                  ^~~~~~~~~~~~~~~
   include/linux/export.h:158:29: note: in expansion of macro '_EXPORT_SYMBOL'
     158 | #define EXPORT_SYMBOL(sym)  _EXPORT_SYMBOL(sym, "")
         |                             ^~~~~~~~~~~~~~
>> drivers/net/phy/phy.c:928:1: note: in expansion of macro 'EXPORT_SYMBOL'
     928 | EXPORT_SYMBOL(phy_request_interrupt);
         | ^~~~~~~~~~~~~
   include/linux/export.h:67:36: note: previous definition of '__ksymtab_phy_request_interrupt' was here
      67 |  static const struct kernel_symbol __ksymtab_##sym  \
         |                                    ^~~~~~~~~~
   include/linux/export.h:108:2: note: in expansion of macro '__KSYMTAB_ENTRY'
     108 |  __KSYMTAB_ENTRY(sym, sec)
         |  ^~~~~~~~~~~~~~~
   include/linux/export.h:147:39: note: in expansion of macro '___EXPORT_SYMBOL'
     147 | #define __EXPORT_SYMBOL(sym, sec, ns) ___EXPORT_SYMBOL(sym, sec, ns)
         |                                       ^~~~~~~~~~~~~~~~
   include/linux/export.h:155:34: note: in expansion of macro '__EXPORT_SYMBOL'
     155 | #define _EXPORT_SYMBOL(sym, sec) __EXPORT_SYMBOL(sym, sec, "")
         |                                  ^~~~~~~~~~~~~~~
   include/linux/export.h:159:33: note: in expansion of macro '_EXPORT_SYMBOL'
     159 | #define EXPORT_SYMBOL_GPL(sym)  _EXPORT_SYMBOL(sym, "_gpl")
         |                                 ^~~~~~~~~~~~~~
>> drivers/net/phy/phy.c:855:1: note: in expansion of macro 'EXPORT_SYMBOL_GPL'
     855 | EXPORT_SYMBOL_GPL(phy_request_interrupt);
         | ^~~~~~~~~~~~~~~~~

vim +/__ksymtab_phy_request_interrupt +67 include/linux/export.h

f50169324df4ad Paul Gortmaker    2011-05-23  41  
7290d58095712a Ard Biesheuvel    2018-08-21  42  #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
7290d58095712a Ard Biesheuvel    2018-08-21  43  #include <linux/compiler.h>
7290d58095712a Ard Biesheuvel    2018-08-21  44  /*
7290d58095712a Ard Biesheuvel    2018-08-21  45   * Emit the ksymtab entry as a pair of relative references: this reduces
7290d58095712a Ard Biesheuvel    2018-08-21  46   * the size by half on 64-bit architectures, and eliminates the need for
7290d58095712a Ard Biesheuvel    2018-08-21  47   * absolute relocations that require runtime processing on relocatable
7290d58095712a Ard Biesheuvel    2018-08-21  48   * kernels.
7290d58095712a Ard Biesheuvel    2018-08-21  49   */
7290d58095712a Ard Biesheuvel    2018-08-21  50  #define __KSYMTAB_ENTRY(sym, sec)					\
7290d58095712a Ard Biesheuvel    2018-08-21  51  	__ADDRESSABLE(sym)						\
7290d58095712a Ard Biesheuvel    2018-08-21  52  	asm("	.section \"___ksymtab" sec "+" #sym "\", \"a\"	\n"	\
ed13fc33f76303 Matthias Maennich 2019-09-06  53  	    "	.balign	4					\n"	\
7290d58095712a Ard Biesheuvel    2018-08-21  54  	    "__ksymtab_" #sym ":				\n"	\
7290d58095712a Ard Biesheuvel    2018-08-21  55  	    "	.long	" #sym "- .				\n"	\
7290d58095712a Ard Biesheuvel    2018-08-21  56  	    "	.long	__kstrtab_" #sym "- .			\n"	\
c3a6cf19e695c8 Masahiro Yamada   2019-10-18  57  	    "	.long	__kstrtabns_" #sym "- .			\n"	\
7290d58095712a Ard Biesheuvel    2018-08-21  58  	    "	.previous					\n")
7290d58095712a Ard Biesheuvel    2018-08-21  59  
7290d58095712a Ard Biesheuvel    2018-08-21  60  struct kernel_symbol {
7290d58095712a Ard Biesheuvel    2018-08-21  61  	int value_offset;
7290d58095712a Ard Biesheuvel    2018-08-21  62  	int name_offset;
8651ec01daedad Matthias Maennich 2019-09-06  63  	int namespace_offset;
7290d58095712a Ard Biesheuvel    2018-08-21  64  };
7290d58095712a Ard Biesheuvel    2018-08-21  65  #else
7290d58095712a Ard Biesheuvel    2018-08-21  66  #define __KSYMTAB_ENTRY(sym, sec)					\
7290d58095712a Ard Biesheuvel    2018-08-21 @67  	static const struct kernel_symbol __ksymtab_##sym		\
7290d58095712a Ard Biesheuvel    2018-08-21  68  	__attribute__((section("___ksymtab" sec "+" #sym), used))	\
ed13fc33f76303 Matthias Maennich 2019-09-06  69  	__aligned(sizeof(void *))					\
c3a6cf19e695c8 Masahiro Yamada   2019-10-18  70  	= { (unsigned long)&sym, __kstrtab_##sym, __kstrtabns_##sym }
7290d58095712a Ard Biesheuvel    2018-08-21  71  

---
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: 22959 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] net: phy: export phy_disable_interrupts()
  2020-06-22 10:29 ` [PATCH 1/2] net: phy: export phy_disable_interrupts() Jisheng Zhang
  2020-06-22 12:50   ` kernel test robot
@ 2020-06-22 16:30   ` kernel test robot
  1 sibling, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-06-22 16:30 UTC (permalink / raw)
  To: Jisheng Zhang, Andrew Lunn, Florian Fainelli, Heiner Kallweit,
	Russell King, David S. Miller, Jakub Kicinski
  Cc: kbuild-all, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1353 bytes --]

Hi Jisheng,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.8-rc2 next-20200622]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jisheng-Zhang/net-phy-call-phy_disable_interrupts-in-phy_init_hw/20200622-183400
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 625d3449788f85569096780592549d0340e9c0c7
config: x86_64-lkp (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   /tmp/ccwmxln4.s: Assembler messages:
>> /tmp/ccwmxln4.s:247: Error: symbol `__kstrtab_phy_request_interrupt' is already defined
>> /tmp/ccwmxln4.s:249: Error: symbol `__kstrtabns_phy_request_interrupt' is already defined
   /tmp/ccwmxln4.s:255: Error: symbol `__ksymtab_phy_request_interrupt' is already defined

---
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: 28579 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] net: phy: call phy_disable_interrupts() in phy_init_hw()
  2020-06-22 10:28 [PATCH 0/2] net: phy: call phy_disable_interrupts() in phy_init_hw() Jisheng Zhang
  2020-06-22 10:29 ` [PATCH 1/2] net: phy: export phy_disable_interrupts() Jisheng Zhang
@ 2020-06-22 10:29 ` Jisheng Zhang
  1 sibling, 0 replies; 5+ messages in thread
From: Jisheng Zhang @ 2020-06-22 10:29 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Heiner Kallweit, Russell King,
	David S. Miller, Jakub Kicinski
  Cc: netdev, linux-kernel

Call phy_disable_interrupts() in phy_init_hw() to "have a defined init
state as we don't know in which state the PHY is if the PHY driver is
loaded. We shouldn't assume that it's the chip power-on defaults, BIOS
or boot loader could have changed this. Or in case of dual-boot
systems the other OS could leave the PHY in whatever state." as Pointed
out by Heiner.

Suggested-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 drivers/net/phy/phy_device.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 04946de74fa0..f17d397ba689 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1090,10 +1090,13 @@ int phy_init_hw(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	if (phydev->drv->config_init)
+	if (phydev->drv->config_init) {
 		ret = phydev->drv->config_init(phydev);
+		if (ret < 0)
+			return ret;
+	}
 
-	return ret;
+	return phy_disable_interrupts(phydev);
 }
 EXPORT_SYMBOL(phy_init_hw);
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-06-22 16:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-22 10:28 [PATCH 0/2] net: phy: call phy_disable_interrupts() in phy_init_hw() Jisheng Zhang
2020-06-22 10:29 ` [PATCH 1/2] net: phy: export phy_disable_interrupts() Jisheng Zhang
2020-06-22 12:50   ` kernel test robot
2020-06-22 16:30   ` kernel test robot
2020-06-22 10:29 ` [PATCH 2/2] net: phy: call phy_disable_interrupts() in phy_init_hw() Jisheng Zhang

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).