| Message ID | 20230524091722.522118-6-jiawenwu@trustnetic.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | TXGBE PHYLINK support | expand |
Hi Jiawen,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Jiawen-Wu/net-txgbe-Add-software-nodes-to-support-phylink/20230524-173221
base: net-next/main
patch link: https://lore.kernel.org/r/20230524091722.522118-6-jiawenwu%40trustnetic.com
patch subject: [PATCH net-next v9 5/9] net: txgbe: Add SFP module identify
config: csky-randconfig-r003-20230525 (https://download.01.org/0day-ci/archive/20230526/202305261959.mnGUW17n-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/c382745a6443e8ff9b3fab9b10c90b216b2ca59b
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jiawen-Wu/net-txgbe-Add-software-nodes-to-support-phylink/20230524-173221
git checkout c382745a6443e8ff9b3fab9b10c90b216b2ca59b
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 ~/bin/make.cross W=1 O=build_dir ARCH=csky olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 ~/bin/make.cross W=1 O=build_dir ARCH=csky SHELL=/bin/bash drivers/net/phy/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305261959.mnGUW17n-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/net/phy/sfp.c: In function 'sfp_i2c_read':
>> drivers/net/phy/sfp.c:609:23: error: implicit declaration of function 'i2c_transfer' [-Werror=implicit-function-declaration]
609 | ret = i2c_transfer(sfp->i2c, msgs, ARRAY_SIZE(msgs));
| ^~~~~~~~~~~~
drivers/net/phy/sfp.c: In function 'sfp_i2c_configure':
>> drivers/net/phy/sfp.c:653:14: error: implicit declaration of function 'i2c_check_functionality' [-Werror=implicit-function-declaration]
653 | if (!i2c_check_functionality(i2c, I2C_FUNC_I2C))
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/net/phy/sfp.c: In function 'sfp_cleanup':
>> drivers/net/phy/sfp.c:2919:17: error: implicit declaration of function 'i2c_put_adapter' [-Werror=implicit-function-declaration]
2919 | i2c_put_adapter(sfp->i2c);
| ^~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM
Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI)
Selected by [y]:
- TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]
WARNING: unmet direct dependencies detected for SFP
Depends on [n]: NETDEVICES [=y] && PHYLIB [=y] && I2C [=n] && PHYLINK [=y] && (HWMON [=n] || HWMON [=n]=n)
Selected by [y]:
- TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]
vim +/i2c_transfer +609 drivers/net/phy/sfp.c
73970055450eeb Russell King 2017-07-25 583
3bb35261c74e39 Jon Nettleton 2018-02-27 584 static int sfp_i2c_read(struct sfp *sfp, bool a2, u8 dev_addr, void *buf,
3bb35261c74e39 Jon Nettleton 2018-02-27 585 size_t len)
73970055450eeb Russell King 2017-07-25 586 {
73970055450eeb Russell King 2017-07-25 587 struct i2c_msg msgs[2];
426c6cbc409cbd Pali Rohár 2021-01-25 588 u8 bus_addr = a2 ? 0x51 : 0x50;
426c6cbc409cbd Pali Rohár 2021-01-25 589 size_t block_size = sfp->i2c_block_size;
28e74a7cfd6403 Russell King 2019-06-02 590 size_t this_len;
73970055450eeb Russell King 2017-07-25 591 int ret;
73970055450eeb Russell King 2017-07-25 592
73970055450eeb Russell King 2017-07-25 593 msgs[0].addr = bus_addr;
73970055450eeb Russell King 2017-07-25 594 msgs[0].flags = 0;
73970055450eeb Russell King 2017-07-25 595 msgs[0].len = 1;
73970055450eeb Russell King 2017-07-25 596 msgs[0].buf = &dev_addr;
73970055450eeb Russell King 2017-07-25 597 msgs[1].addr = bus_addr;
73970055450eeb Russell King 2017-07-25 598 msgs[1].flags = I2C_M_RD;
73970055450eeb Russell King 2017-07-25 599 msgs[1].len = len;
73970055450eeb Russell King 2017-07-25 600 msgs[1].buf = buf;
73970055450eeb Russell King 2017-07-25 601
28e74a7cfd6403 Russell King 2019-06-02 602 while (len) {
28e74a7cfd6403 Russell King 2019-06-02 603 this_len = len;
0d035bed2a4a6c Russell King 2020-12-09 604 if (this_len > block_size)
0d035bed2a4a6c Russell King 2020-12-09 605 this_len = block_size;
28e74a7cfd6403 Russell King 2019-06-02 606
28e74a7cfd6403 Russell King 2019-06-02 607 msgs[1].len = this_len;
28e74a7cfd6403 Russell King 2019-06-02 608
3bb35261c74e39 Jon Nettleton 2018-02-27 @609 ret = i2c_transfer(sfp->i2c, msgs, ARRAY_SIZE(msgs));
73970055450eeb Russell King 2017-07-25 610 if (ret < 0)
73970055450eeb Russell King 2017-07-25 611 return ret;
73970055450eeb Russell King 2017-07-25 612
28e74a7cfd6403 Russell King 2019-06-02 613 if (ret != ARRAY_SIZE(msgs))
28e74a7cfd6403 Russell King 2019-06-02 614 break;
28e74a7cfd6403 Russell King 2019-06-02 615
28e74a7cfd6403 Russell King 2019-06-02 616 msgs[1].buf += this_len;
28e74a7cfd6403 Russell King 2019-06-02 617 dev_addr += this_len;
28e74a7cfd6403 Russell King 2019-06-02 618 len -= this_len;
28e74a7cfd6403 Russell King 2019-06-02 619 }
28e74a7cfd6403 Russell King 2019-06-02 620
28e74a7cfd6403 Russell King 2019-06-02 621 return msgs[1].buf - (u8 *)buf;
73970055450eeb Russell King 2017-07-25 622 }
73970055450eeb Russell King 2017-07-25 623
3bb35261c74e39 Jon Nettleton 2018-02-27 624 static int sfp_i2c_write(struct sfp *sfp, bool a2, u8 dev_addr, void *buf,
73970055450eeb Russell King 2017-07-25 625 size_t len)
73970055450eeb Russell King 2017-07-25 626 {
3bb35261c74e39 Jon Nettleton 2018-02-27 627 struct i2c_msg msgs[1];
3bb35261c74e39 Jon Nettleton 2018-02-27 628 u8 bus_addr = a2 ? 0x51 : 0x50;
3bb35261c74e39 Jon Nettleton 2018-02-27 629 int ret;
3bb35261c74e39 Jon Nettleton 2018-02-27 630
3bb35261c74e39 Jon Nettleton 2018-02-27 631 msgs[0].addr = bus_addr;
3bb35261c74e39 Jon Nettleton 2018-02-27 632 msgs[0].flags = 0;
3bb35261c74e39 Jon Nettleton 2018-02-27 633 msgs[0].len = 1 + len;
3bb35261c74e39 Jon Nettleton 2018-02-27 634 msgs[0].buf = kmalloc(1 + len, GFP_KERNEL);
3bb35261c74e39 Jon Nettleton 2018-02-27 635 if (!msgs[0].buf)
3bb35261c74e39 Jon Nettleton 2018-02-27 636 return -ENOMEM;
3bb35261c74e39 Jon Nettleton 2018-02-27 637
3bb35261c74e39 Jon Nettleton 2018-02-27 638 msgs[0].buf[0] = dev_addr;
3bb35261c74e39 Jon Nettleton 2018-02-27 639 memcpy(&msgs[0].buf[1], buf, len);
3bb35261c74e39 Jon Nettleton 2018-02-27 640
3bb35261c74e39 Jon Nettleton 2018-02-27 641 ret = i2c_transfer(sfp->i2c, msgs, ARRAY_SIZE(msgs));
3bb35261c74e39 Jon Nettleton 2018-02-27 642
3bb35261c74e39 Jon Nettleton 2018-02-27 643 kfree(msgs[0].buf);
3bb35261c74e39 Jon Nettleton 2018-02-27 644
3bb35261c74e39 Jon Nettleton 2018-02-27 645 if (ret < 0)
3bb35261c74e39 Jon Nettleton 2018-02-27 646 return ret;
3bb35261c74e39 Jon Nettleton 2018-02-27 647
3bb35261c74e39 Jon Nettleton 2018-02-27 648 return ret == ARRAY_SIZE(msgs) ? len : 0;
73970055450eeb Russell King 2017-07-25 649 }
73970055450eeb Russell King 2017-07-25 650
73970055450eeb Russell King 2017-07-25 651 static int sfp_i2c_configure(struct sfp *sfp, struct i2c_adapter *i2c)
73970055450eeb Russell King 2017-07-25 652 {
73970055450eeb Russell King 2017-07-25 @653 if (!i2c_check_functionality(i2c, I2C_FUNC_I2C))
73970055450eeb Russell King 2017-07-25 654 return -EINVAL;
73970055450eeb Russell King 2017-07-25 655
73970055450eeb Russell King 2017-07-25 656 sfp->i2c = i2c;
73970055450eeb Russell King 2017-07-25 657 sfp->read = sfp_i2c_read;
3bb35261c74e39 Jon Nettleton 2018-02-27 658 sfp->write = sfp_i2c_write;
73970055450eeb Russell King 2017-07-25 659
e85b1347ace677 Marek Behún 2022-09-30 660 return 0;
e85b1347ace677 Marek Behún 2022-09-30 661 }
e85b1347ace677 Marek Behún 2022-09-30 662
On Fri, May 26, 2023 at 07:30:45PM +0800, kernel test robot wrote: > Kconfig warnings: (for reference only) > WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM > Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI) > Selected by [y]: > - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y] > WARNING: unmet direct dependencies detected for SFP > Depends on [n]: NETDEVICES [=y] && PHYLIB [=y] && I2C [=n] && PHYLINK [=y] && (HWMON [=n] || HWMON [=n]=n) > Selected by [y]: > - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y] ... and is basically caused by "select SFP". No. Do not do this unless you look at the dependencies for SFP and ensure that those are also satisfied - because if you don't you create messes like the above build errors.
On Friday, May 26, 2023 7:37 PM, Russell King (Oracle) wrote: > On Fri, May 26, 2023 at 07:30:45PM +0800, kernel test robot wrote: > > Kconfig warnings: (for reference only) > > WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM > > Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI) > > Selected by [y]: > > - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y] > > WARNING: unmet direct dependencies detected for SFP > > Depends on [n]: NETDEVICES [=y] && PHYLIB [=y] && I2C [=n] && PHYLINK [=y] && (HWMON [=n] || HWMON [=n]=n) > > Selected by [y]: > > - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y] > > ... and is basically caused by "select SFP". No. Do not do this unless > you look at the dependencies for SFP and ensure that those are also > satisfied - because if you don't you create messes like the above > build errors. So how do I make sure that the module I need compiles and loads correctly, rely on the user to manually select it?
On Monday, May 29, 2023 10:06 AM, Jiawen Wu wrote: > On Friday, May 26, 2023 7:37 PM, Russell King (Oracle) wrote: > > On Fri, May 26, 2023 at 07:30:45PM +0800, kernel test robot wrote: > > > Kconfig warnings: (for reference only) > > > WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM > > > Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI) > > > Selected by [y]: > > > - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y] > > > WARNING: unmet direct dependencies detected for SFP > > > Depends on [n]: NETDEVICES [=y] && PHYLIB [=y] && I2C [=n] && PHYLINK [=y] && (HWMON [=n] || HWMON [=n]=n) > > > Selected by [y]: > > > - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y] > > > > ... and is basically caused by "select SFP". No. Do not do this unless > > you look at the dependencies for SFP and ensure that those are also > > satisfied - because if you don't you create messes like the above > > build errors. > > So how do I make sure that the module I need compiles and loads correctly, > rely on the user to manually select it? When I changed the TXGBE config to: ... depends on SFP select PCS_XPCS ... the compilation gave an error: drivers/net/phy/Kconfig:16:error: recursive dependency detected! drivers/net/phy/Kconfig:16: symbol PHYLIB is selected by PHYLINK drivers/net/phy/Kconfig:6: symbol PHYLINK is selected by PCS_XPCS drivers/net/pcs/Kconfig:8: symbol PCS_XPCS is selected by TXGBE drivers/net/ethernet/wangxun/Kconfig:40: symbol TXGBE depends on SFP drivers/net/phy/Kconfig:63: symbol SFP depends on PHYLIB For a resolution refer to Documentation/kbuild/kconfig-language.rst subsection "Kconfig recursive dependency limitations" Seems deleting "depends on SFP" is the correct way. But is this normal? How do we ensure the dependency between TXGBE and SFP?
On Tuesday, May 30, 2023 4:41 PM, Jiawen Wu wrote: > On Monday, May 29, 2023 10:06 AM, Jiawen Wu wrote: > > On Friday, May 26, 2023 7:37 PM, Russell King (Oracle) wrote: > > > On Fri, May 26, 2023 at 07:30:45PM +0800, kernel test robot wrote: > > > > Kconfig warnings: (for reference only) > > > > WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM > > > > Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI) > > > > Selected by [y]: > > > > - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y] > > > > WARNING: unmet direct dependencies detected for SFP > > > > Depends on [n]: NETDEVICES [=y] && PHYLIB [=y] && I2C [=n] && PHYLINK [=y] && (HWMON [=n] || HWMON [=n]=n) > > > > Selected by [y]: > > > > - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y] > > > > > > ... and is basically caused by "select SFP". No. Do not do this unless > > > you look at the dependencies for SFP and ensure that those are also > > > satisfied - because if you don't you create messes like the above > > > build errors. > > > > So how do I make sure that the module I need compiles and loads correctly, > > rely on the user to manually select it? > > When I changed the TXGBE config to: > ... > depends on SFP > select PCS_XPCS > ... > the compilation gave an error: > > drivers/net/phy/Kconfig:16:error: recursive dependency detected! > drivers/net/phy/Kconfig:16: symbol PHYLIB is selected by PHYLINK > drivers/net/phy/Kconfig:6: symbol PHYLINK is selected by PCS_XPCS > drivers/net/pcs/Kconfig:8: symbol PCS_XPCS is selected by TXGBE > drivers/net/ethernet/wangxun/Kconfig:40: symbol TXGBE depends on SFP > drivers/net/phy/Kconfig:63: symbol SFP depends on PHYLIB > For a resolution refer to Documentation/kbuild/kconfig-language.rst > subsection "Kconfig recursive dependency limitations" > > Seems deleting "depends on SFP" is the correct way. But is this normal? > How do we ensure the dependency between TXGBE and SFP? Hi Russell, Could you please give me some suggestions? I checked "kconfig-language" doc, the practical solution is that swap all "select FOO" to "depends on FOO" or swap all "depends on FOO" to "select FOO". Config PCS_XPCS has to be selected in order to load modules properly, so how should I fix the warning?
On Tue, May 30, 2023 at 04:40:36PM +0800, Jiawen Wu wrote: > On Monday, May 29, 2023 10:06 AM, Jiawen Wu wrote: > > On Friday, May 26, 2023 7:37 PM, Russell King (Oracle) wrote: > > > On Fri, May 26, 2023 at 07:30:45PM +0800, kernel test robot wrote: > > > > Kconfig warnings: (for reference only) > > > > WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM > > > > Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI) > > > > Selected by [y]: > > > > - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y] > > > > WARNING: unmet direct dependencies detected for SFP > > > > Depends on [n]: NETDEVICES [=y] && PHYLIB [=y] && I2C [=n] && PHYLINK [=y] && (HWMON [=n] || HWMON [=n]=n) > > > > Selected by [y]: > > > > - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y] > > > > > > ... and is basically caused by "select SFP". No. Do not do this unless > > > you look at the dependencies for SFP and ensure that those are also > > > satisfied - because if you don't you create messes like the above > > > build errors. > > > > So how do I make sure that the module I need compiles and loads correctly, > > rely on the user to manually select it? > > When I changed the TXGBE config to: > ... > depends on SFP > select PCS_XPCS > ... > the compilation gave an error: > > drivers/net/phy/Kconfig:16:error: recursive dependency detected! > drivers/net/phy/Kconfig:16: symbol PHYLIB is selected by PHYLINK > drivers/net/phy/Kconfig:6: symbol PHYLINK is selected by PCS_XPCS > drivers/net/pcs/Kconfig:8: symbol PCS_XPCS is selected by TXGBE > drivers/net/ethernet/wangxun/Kconfig:40: symbol TXGBE depends on SFP > drivers/net/phy/Kconfig:63: symbol SFP depends on PHYLIB > For a resolution refer to Documentation/kbuild/kconfig-language.rst > subsection "Kconfig recursive dependency limitations" > > Seems deleting "depends on SFP" is the correct way. But is this normal? > How do we ensure the dependency between TXGBE and SFP? First, I would do this: select PHYLINK select PCS_XPCS but then I'm principled, and I don't agree that PCS_XPCS should be selecting PHYLINK. The second thing I don't particularly like is selecting user visible symbols, but as I understand it, with TXGBE, the SFP slot is not an optional feature, so there's little option. So, because SFP requires I2C: select I2C select SFP That is basically what I meant by "you look at the dependencies for SFP and ensure that those are also satisfied". Adding that "select I2C" also solves the unmet dependencies for I2C_DESIGNWARE_PLATFORM. However, even with that, we're not done with the evilness of select, because there's one more permitted configuration combination that will break. If you build TXGBE into the kernel, that will force SFP=y, I2C=y, PHYLINK=y, PHYLIB=y. So far so good. However, if HWMON=m, then things will again break. So I would also suggest: select HWMON if TXGBE=y even though you don't require it, it solves the build fallout from where HWMON=m but you force SFP=y. Maybe someone else has better ideas how to do this, but the above is the best I can come up with. IMHO, select is nothing but pure evil, and should be used with utmost care and a full understanding of its ramifications, and a realisation that it *totally* and *utterly* blows away any "depends on" on the target of the select statement. An option that states that it depends on something else generally does because... oddly enough, it _depends_ on that other option. So, if select forces an option on without its dependencies, then it's not surprising that stuff fails to build. Whenever a select statement is added, one must _always_ look at the target symbol and consider any "depends on" there, and how to ensure that those dependencies are guaranteed to always be satisfied.
On Wednesday, May 31, 2023 5:48 PM, Russell King (Oracle) wrote: > On Tue, May 30, 2023 at 04:40:36PM +0800, Jiawen Wu wrote: > > On Monday, May 29, 2023 10:06 AM, Jiawen Wu wrote: > > > On Friday, May 26, 2023 7:37 PM, Russell King (Oracle) wrote: > > > > On Fri, May 26, 2023 at 07:30:45PM +0800, kernel test robot wrote: > > > > > Kconfig warnings: (for reference only) > > > > > WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM > > > > > Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI) > > > > > Selected by [y]: > > > > > - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y] > > > > > WARNING: unmet direct dependencies detected for SFP > > > > > Depends on [n]: NETDEVICES [=y] && PHYLIB [=y] && I2C [=n] && PHYLINK [=y] && (HWMON [=n] || HWMON [=n]=n) > > > > > Selected by [y]: > > > > > - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y] > > > > > > > > ... and is basically caused by "select SFP". No. Do not do this unless > > > > you look at the dependencies for SFP and ensure that those are also > > > > satisfied - because if you don't you create messes like the above > > > > build errors. > > > > > > So how do I make sure that the module I need compiles and loads correctly, > > > rely on the user to manually select it? > > > > When I changed the TXGBE config to: > > ... > > depends on SFP > > select PCS_XPCS > > ... > > the compilation gave an error: > > > > drivers/net/phy/Kconfig:16:error: recursive dependency detected! > > drivers/net/phy/Kconfig:16: symbol PHYLIB is selected by PHYLINK > > drivers/net/phy/Kconfig:6: symbol PHYLINK is selected by PCS_XPCS > > drivers/net/pcs/Kconfig:8: symbol PCS_XPCS is selected by TXGBE > > drivers/net/ethernet/wangxun/Kconfig:40: symbol TXGBE depends on SFP > > drivers/net/phy/Kconfig:63: symbol SFP depends on PHYLIB > > For a resolution refer to Documentation/kbuild/kconfig-language.rst > > subsection "Kconfig recursive dependency limitations" > > > > Seems deleting "depends on SFP" is the correct way. But is this normal? > > How do we ensure the dependency between TXGBE and SFP? > > First, I would do this: > > select PHYLINK > select PCS_XPCS > > but then I'm principled, and I don't agree that PCS_XPCS should be > selecting PHYLINK. > > The second thing I don't particularly like is selecting user visible > symbols, but as I understand it, with TXGBE, the SFP slot is not an > optional feature, so there's little option. > > So, because SFP requires I2C: > > select I2C > select SFP > > That is basically what I meant by "you look at the dependencies for > SFP and ensure that those are also satisfied". > > Adding that "select I2C" also solves the unmet dependencies for > I2C_DESIGNWARE_PLATFORM. > > However, even with that, we're not done with the evilness of select, > because there's one more permitted configuration combination that > will break. > > If you build TXGBE into the kernel, that will force SFP=y, I2C=y, > PHYLINK=y, PHYLIB=y. So far so good. However, if HWMON=m, then things > will again break. So I would also suggest: > > select HWMON if TXGBE=y > > even though you don't require it, it solves the build fallout from > where HWMON=m but you force SFP=y. > > Maybe someone else has better ideas how to do this, but the above is > the best I can come up with. > > > IMHO, select is nothing but pure evil, and should be used with utmost > care and a full understanding of its ramifications, and a realisation > that it *totally* and *utterly* blows away any "depends on" on the > target of the select statement. > > An option that states that it depends on something else generally does > because... oddly enough, it _depends_ on that other option. So, if > select forces an option on without its dependencies, then it's not > surprising that stuff fails to build. > > Whenever a select statement is added, one must _always_ look at the > target symbol and consider any "depends on" there, and how to ensure > that those dependencies are guaranteed to always be satisfied. Thanks for the detailed explanation. I'll check each of the required options, and use "depends on" whenever possible.
diff --git a/drivers/net/ethernet/wangxun/Kconfig b/drivers/net/ethernet/wangxun/Kconfig index ec058a72afb6..90812d76181d 100644 --- a/drivers/net/ethernet/wangxun/Kconfig +++ b/drivers/net/ethernet/wangxun/Kconfig @@ -44,6 +44,7 @@ config TXGBE select REGMAP select COMMON_CLK select LIBWX + select SFP help This driver supports Wangxun(R) 10GbE PCI Express family of adapters. diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c index 6ea33e753df4..3da5f5538f34 100644 --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c @@ -158,6 +158,25 @@ static int txgbe_i2c_register(struct txgbe *txgbe) return 0; } +static int txgbe_sfp_register(struct txgbe *txgbe) +{ + struct pci_dev *pdev = txgbe->wx->pdev; + struct platform_device_info info = {}; + struct platform_device *sfp_dev; + + info.parent = &pdev->dev; + info.fwnode = software_node_fwnode(txgbe->nodes.group[SWNODE_SFP]); + info.name = "sfp"; + info.id = (pdev->bus->number << 8) | pdev->devfn; + sfp_dev = platform_device_register_full(&info); + if (IS_ERR(sfp_dev)) + return PTR_ERR(sfp_dev); + + txgbe->sfp_dev = sfp_dev; + + return 0; +} + int txgbe_init_phy(struct txgbe *txgbe) { int ret; @@ -180,8 +199,16 @@ int txgbe_init_phy(struct txgbe *txgbe) goto err_unregister_clk; } + ret = txgbe_sfp_register(txgbe); + if (ret) { + wx_err(txgbe->wx, "failed to register sfp\n"); + goto err_unregister_i2c; + } + return 0; +err_unregister_i2c: + platform_device_unregister(txgbe->i2c_dev); err_unregister_clk: clkdev_drop(txgbe->clock); clk_unregister(txgbe->clk); @@ -193,6 +220,7 @@ int txgbe_init_phy(struct txgbe *txgbe) void txgbe_remove_phy(struct txgbe *txgbe) { + platform_device_unregister(txgbe->sfp_dev); platform_device_unregister(txgbe->i2c_dev); clkdev_drop(txgbe->clock); clk_unregister(txgbe->clk); diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h index 55979abf01f2..fc91e0fc37a6 100644 --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h @@ -149,6 +149,7 @@ struct txgbe_nodes { struct txgbe { struct wx *wx; struct txgbe_nodes nodes; + struct platform_device *sfp_dev; struct platform_device *i2c_dev; struct clk_lookup *clock; struct clk *clk;