All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 net] ionic: fix ptp support config breakage
@ 2021-05-06  4:18 Shannon Nelson
  2021-05-07  0:15 ` Jakub Kicinski
  0 siblings, 1 reply; 8+ messages in thread
From: Shannon Nelson @ 2021-05-06  4:18 UTC (permalink / raw)
  To: netdev, davem, kuba; +Cc: drivers, Shannon Nelson, Allen Hubbe

Driver link failed with undefined references in some
kernel config variations.

Fixes: 61db421da31b ("ionic: link in the new hw timestamp code")
Reported-by: kernel test robot <lkp@intel.com>
Cc: Allen Hubbe <allenbh@pensando.io>
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---

v3 - put version notes below ---, added Allen's Cc
v2 - added Fixes tag

 drivers/net/ethernet/pensando/ionic/Makefile    | 3 +--
 drivers/net/ethernet/pensando/ionic/ionic_phc.c | 3 +++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/Makefile b/drivers/net/ethernet/pensando/ionic/Makefile
index 4e7642a2d25f..61c40169cb1f 100644
--- a/drivers/net/ethernet/pensando/ionic/Makefile
+++ b/drivers/net/ethernet/pensando/ionic/Makefile
@@ -5,5 +5,4 @@ obj-$(CONFIG_IONIC) := ionic.o
 
 ionic-y := ionic_main.o ionic_bus_pci.o ionic_devlink.o ionic_dev.o \
 	   ionic_debugfs.o ionic_lif.o ionic_rx_filter.o ionic_ethtool.o \
-	   ionic_txrx.o ionic_stats.o ionic_fw.o
-ionic-$(CONFIG_PTP_1588_CLOCK) += ionic_phc.o
+	   ionic_txrx.o ionic_stats.o ionic_fw.o ionic_phc.o
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_phc.c b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
index a87c87e86aef..30c78808c45a 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_phc.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright(c) 2017 - 2021 Pensando Systems, Inc */
 
+#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
+
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 
@@ -613,3 +615,4 @@ void ionic_lif_free_phc(struct ionic_lif *lif)
 	devm_kfree(lif->ionic->dev, lif->phc);
 	lif->phc = NULL;
 }
+#endif /* IS_ENABLED(CONFIG_PTP_1588_CLOCK) */
-- 
2.17.1


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

* Re: [PATCH v3 net] ionic: fix ptp support config breakage
  2021-05-06  4:18 [PATCH v3 net] ionic: fix ptp support config breakage Shannon Nelson
@ 2021-05-07  0:15 ` Jakub Kicinski
  2021-05-07  0:21   ` Randy Dunlap
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2021-05-07  0:15 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev, davem, drivers, Allen Hubbe

On Wed,  5 May 2021 21:18:46 -0700 Shannon Nelson wrote:
> Driver link failed with undefined references in some
> kernel config variations.

This is really vague and the patch is not very obvious.

>  ionic-y := ionic_main.o ionic_bus_pci.o ionic_devlink.o ionic_dev.o \
>  	   ionic_debugfs.o ionic_lif.o ionic_rx_filter.o ionic_ethtool.o \
> -	   ionic_txrx.o ionic_stats.o ionic_fw.o
> -ionic-$(CONFIG_PTP_1588_CLOCK) += ionic_phc.o
> +	   ionic_txrx.o ionic_stats.o ionic_fw.o ionic_phc.o

So we'd replace a build dependency..

> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_phc.c b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
> index a87c87e86aef..30c78808c45a 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_phc.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
> @@ -1,6 +1,8 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /* Copyright(c) 2017 - 2021 Pensando Systems, Inc */
>  
> +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
> +
>  #include <linux/netdevice.h>
>  #include <linux/etherdevice.h>
>  
> @@ -613,3 +615,4 @@ void ionic_lif_free_phc(struct ionic_lif *lif)
>  	devm_kfree(lif->ionic->dev, lif->phc);
>  	lif->phc = NULL;
>  }
> +#endif /* IS_ENABLED(CONFIG_PTP_1588_CLOCK) */

.. with an ifdef around an entire file? Does not feel very clean.

The construct of using:

	drv-$(CONFIG_PTP_1588_CLOCK) += ptp.o

seems relatively common, why does it now work here?

Maybe the config in question has PTP as a module and ionic built in?
Then you should add depends on PTP_1588_CLOCK || !PTP_1588_CLOCK.

Maybe somehow the "ionic-y" confuses kbuild and it should be ionic-objs?

At the very least we need a better explanation in the commit message.

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

* Re: [PATCH v3 net] ionic: fix ptp support config breakage
  2021-05-07  0:15 ` Jakub Kicinski
@ 2021-05-07  0:21   ` Randy Dunlap
  2021-05-07  0:48     ` Shannon Nelson
  0 siblings, 1 reply; 8+ messages in thread
From: Randy Dunlap @ 2021-05-07  0:21 UTC (permalink / raw)
  To: Jakub Kicinski, Shannon Nelson; +Cc: netdev, davem, drivers, Allen Hubbe

On 5/6/21 5:15 PM, Jakub Kicinski wrote:
> On Wed,  5 May 2021 21:18:46 -0700 Shannon Nelson wrote:
>> Driver link failed with undefined references in some
>> kernel config variations.
> 
> This is really vague and the patch is not very obvious.
> 
>>  ionic-y := ionic_main.o ionic_bus_pci.o ionic_devlink.o ionic_dev.o \
>>  	   ionic_debugfs.o ionic_lif.o ionic_rx_filter.o ionic_ethtool.o \
>> -	   ionic_txrx.o ionic_stats.o ionic_fw.o
>> -ionic-$(CONFIG_PTP_1588_CLOCK) += ionic_phc.o
>> +	   ionic_txrx.o ionic_stats.o ionic_fw.o ionic_phc.o
> 
> So we'd replace a build dependency..
> 
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_phc.c b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>> index a87c87e86aef..30c78808c45a 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>> @@ -1,6 +1,8 @@
>>  // SPDX-License-Identifier: GPL-2.0
>>  /* Copyright(c) 2017 - 2021 Pensando Systems, Inc */
>>  
>> +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
>> +
>>  #include <linux/netdevice.h>
>>  #include <linux/etherdevice.h>
>>  
>> @@ -613,3 +615,4 @@ void ionic_lif_free_phc(struct ionic_lif *lif)
>>  	devm_kfree(lif->ionic->dev, lif->phc);
>>  	lif->phc = NULL;
>>  }
>> +#endif /* IS_ENABLED(CONFIG_PTP_1588_CLOCK) */
> 
> .. with an ifdef around an entire file? Does not feel very clean.
> 
> The construct of using:
> 
> 	drv-$(CONFIG_PTP_1588_CLOCK) += ptp.o
> 
> seems relatively common, why does it now work here?
> 
> Maybe the config in question has PTP as a module and ionic built in?
> Then you should add depends on PTP_1588_CLOCK || !PTP_1588_CLOCK.
> 
> Maybe somehow the "ionic-y" confuses kbuild and it should be ionic-objs?
> 
> At the very least we need a better explanation in the commit message.
> 

I'll take a look if someone can point me to the .config file.

-- 
~Randy


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

* Re: [PATCH v3 net] ionic: fix ptp support config breakage
  2021-05-07  0:21   ` Randy Dunlap
@ 2021-05-07  0:48     ` Shannon Nelson
  2021-05-07  1:12       ` Randy Dunlap
  0 siblings, 1 reply; 8+ messages in thread
From: Shannon Nelson @ 2021-05-07  0:48 UTC (permalink / raw)
  To: Randy Dunlap, Jakub Kicinski; +Cc: netdev, davem, drivers, Allen Hubbe

On 5/6/21 5:21 PM, Randy Dunlap wrote:
> On 5/6/21 5:15 PM, Jakub Kicinski wrote:
>> On Wed,  5 May 2021 21:18:46 -0700 Shannon Nelson wrote:
>>> Driver link failed with undefined references in some
>>> kernel config variations.
>> This is really vague and the patch is not very obvious.
>>
>>>   ionic-y := ionic_main.o ionic_bus_pci.o ionic_devlink.o ionic_dev.o \
>>>   	   ionic_debugfs.o ionic_lif.o ionic_rx_filter.o ionic_ethtool.o \
>>> -	   ionic_txrx.o ionic_stats.o ionic_fw.o
>>> -ionic-$(CONFIG_PTP_1588_CLOCK) += ionic_phc.o
>>> +	   ionic_txrx.o ionic_stats.o ionic_fw.o ionic_phc.o
>> So we'd replace a build dependency..
>>
>>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_phc.c b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>> index a87c87e86aef..30c78808c45a 100644
>>> --- a/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>> @@ -1,6 +1,8 @@
>>>   // SPDX-License-Identifier: GPL-2.0
>>>   /* Copyright(c) 2017 - 2021 Pensando Systems, Inc */
>>>   
>>> +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
>>> +
>>>   #include <linux/netdevice.h>
>>>   #include <linux/etherdevice.h>
>>>   
>>> @@ -613,3 +615,4 @@ void ionic_lif_free_phc(struct ionic_lif *lif)
>>>   	devm_kfree(lif->ionic->dev, lif->phc);
>>>   	lif->phc = NULL;
>>>   }
>>> +#endif /* IS_ENABLED(CONFIG_PTP_1588_CLOCK) */
>> .. with an ifdef around an entire file? Does not feel very clean.
>>
>> The construct of using:
>>
>> 	drv-$(CONFIG_PTP_1588_CLOCK) += ptp.o
>>
>> seems relatively common, why does it now work here?
>>
>> Maybe the config in question has PTP as a module and ionic built in?
>> Then you should add depends on PTP_1588_CLOCK || !PTP_1588_CLOCK.
>>
>> Maybe somehow the "ionic-y" confuses kbuild and it should be ionic-objs?
>>
>> At the very least we need a better explanation in the commit message.
>>
> I'll take a look if someone can point me to the .config file.
>

These are the notes I got from kernel test robot:
https://lore.kernel.org/lkml/202105041020.efEaBOYC-lkp@intel.com/
https://lore.kernel.org/lkml/202105041154.GrLZmjGh-lkp@intel.com/
https://lore.kernel.org/lkml/202105041634.paURyDp0-lkp@intel.com/
https://lore.kernel.org/lkml/202105050636.UXXDl7m2-lkp@intel.com/

sln


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

* Re: [PATCH v3 net] ionic: fix ptp support config breakage
  2021-05-07  0:48     ` Shannon Nelson
@ 2021-05-07  1:12       ` Randy Dunlap
  2021-05-07  1:16         ` Randy Dunlap
  0 siblings, 1 reply; 8+ messages in thread
From: Randy Dunlap @ 2021-05-07  1:12 UTC (permalink / raw)
  To: Shannon Nelson, Jakub Kicinski; +Cc: netdev, davem, drivers, Allen Hubbe

On 5/6/21 5:48 PM, Shannon Nelson wrote:
> On 5/6/21 5:21 PM, Randy Dunlap wrote:
>> On 5/6/21 5:15 PM, Jakub Kicinski wrote:
>>> On Wed,  5 May 2021 21:18:46 -0700 Shannon Nelson wrote:
>>>> Driver link failed with undefined references in some
>>>> kernel config variations.
>>> This is really vague and the patch is not very obvious.
>>>
>>>>   ionic-y := ionic_main.o ionic_bus_pci.o ionic_devlink.o ionic_dev.o \
>>>>          ionic_debugfs.o ionic_lif.o ionic_rx_filter.o ionic_ethtool.o \
>>>> -       ionic_txrx.o ionic_stats.o ionic_fw.o
>>>> -ionic-$(CONFIG_PTP_1588_CLOCK) += ionic_phc.o
>>>> +       ionic_txrx.o ionic_stats.o ionic_fw.o ionic_phc.o
>>> So we'd replace a build dependency..
>>>
>>>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_phc.c b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>> index a87c87e86aef..30c78808c45a 100644
>>>> --- a/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>> @@ -1,6 +1,8 @@
>>>>   // SPDX-License-Identifier: GPL-2.0
>>>>   /* Copyright(c) 2017 - 2021 Pensando Systems, Inc */
>>>>   +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
>>>> +
>>>>   #include <linux/netdevice.h>
>>>>   #include <linux/etherdevice.h>
>>>>   @@ -613,3 +615,4 @@ void ionic_lif_free_phc(struct ionic_lif *lif)
>>>>       devm_kfree(lif->ionic->dev, lif->phc);
>>>>       lif->phc = NULL;
>>>>   }
>>>> +#endif /* IS_ENABLED(CONFIG_PTP_1588_CLOCK) */
>>> .. with an ifdef around an entire file? Does not feel very clean.
>>>
>>> The construct of using:
>>>
>>>     drv-$(CONFIG_PTP_1588_CLOCK) += ptp.o
>>>
>>> seems relatively common, why does it now work here?
>>>
>>> Maybe the config in question has PTP as a module and ionic built in?
>>> Then you should add depends on PTP_1588_CLOCK || !PTP_1588_CLOCK.
>>>
>>> Maybe somehow the "ionic-y" confuses kbuild and it should be ionic-objs?
>>>
>>> At the very least we need a better explanation in the commit message.
>>>
>> I'll take a look if someone can point me to the .config file.
>>
> 
> These are the notes I got from kernel test robot:
> https://lore.kernel.org/lkml/202105041020.efEaBOYC-lkp@intel.com/
> https://lore.kernel.org/lkml/202105041154.GrLZmjGh-lkp@intel.com/
> https://lore.kernel.org/lkml/202105041634.paURyDp0-lkp@intel.com/
> https://lore.kernel.org/lkml/202105050636.UXXDl7m2-lkp@intel.com/

At first glance it looks like Jakub's suggestion of
>>> Maybe the config in question has PTP as a module and ionic built in?
>>> Then you should add depends on PTP_1588_CLOCK || !PTP_1588_CLOCK.

is what is needed, but I'm still doing build testing ATM.


-- 
~Randy


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

* Re: [PATCH v3 net] ionic: fix ptp support config breakage
  2021-05-07  1:12       ` Randy Dunlap
@ 2021-05-07  1:16         ` Randy Dunlap
  2021-05-07  1:30           ` Randy Dunlap
  0 siblings, 1 reply; 8+ messages in thread
From: Randy Dunlap @ 2021-05-07  1:16 UTC (permalink / raw)
  To: Shannon Nelson, Jakub Kicinski; +Cc: netdev, davem, drivers, Allen Hubbe

On 5/6/21 6:12 PM, Randy Dunlap wrote:
> On 5/6/21 5:48 PM, Shannon Nelson wrote:
>> On 5/6/21 5:21 PM, Randy Dunlap wrote:
>>> On 5/6/21 5:15 PM, Jakub Kicinski wrote:
>>>> On Wed,  5 May 2021 21:18:46 -0700 Shannon Nelson wrote:
>>>>> Driver link failed with undefined references in some
>>>>> kernel config variations.
>>>> This is really vague and the patch is not very obvious.
>>>>
>>>>>   ionic-y := ionic_main.o ionic_bus_pci.o ionic_devlink.o ionic_dev.o \
>>>>>          ionic_debugfs.o ionic_lif.o ionic_rx_filter.o ionic_ethtool.o \
>>>>> -       ionic_txrx.o ionic_stats.o ionic_fw.o
>>>>> -ionic-$(CONFIG_PTP_1588_CLOCK) += ionic_phc.o
>>>>> +       ionic_txrx.o ionic_stats.o ionic_fw.o ionic_phc.o
>>>> So we'd replace a build dependency..
>>>>
>>>>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_phc.c b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>>> index a87c87e86aef..30c78808c45a 100644
>>>>> --- a/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>>> @@ -1,6 +1,8 @@
>>>>>   // SPDX-License-Identifier: GPL-2.0
>>>>>   /* Copyright(c) 2017 - 2021 Pensando Systems, Inc */
>>>>>   +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
>>>>> +
>>>>>   #include <linux/netdevice.h>
>>>>>   #include <linux/etherdevice.h>
>>>>>   @@ -613,3 +615,4 @@ void ionic_lif_free_phc(struct ionic_lif *lif)
>>>>>       devm_kfree(lif->ionic->dev, lif->phc);
>>>>>       lif->phc = NULL;
>>>>>   }
>>>>> +#endif /* IS_ENABLED(CONFIG_PTP_1588_CLOCK) */
>>>> .. with an ifdef around an entire file? Does not feel very clean.
>>>>
>>>> The construct of using:
>>>>
>>>>     drv-$(CONFIG_PTP_1588_CLOCK) += ptp.o
>>>>
>>>> seems relatively common, why does it now work here?
>>>>
>>>> Maybe the config in question has PTP as a module and ionic built in?
>>>> Then you should add depends on PTP_1588_CLOCK || !PTP_1588_CLOCK.
>>>>
>>>> Maybe somehow the "ionic-y" confuses kbuild and it should be ionic-objs?
>>>>
>>>> At the very least we need a better explanation in the commit message.
>>>>
>>> I'll take a look if someone can point me to the .config file.
>>>
>>
>> These are the notes I got from kernel test robot:
>> https://lore.kernel.org/lkml/202105041020.efEaBOYC-lkp@intel.com/
>> https://lore.kernel.org/lkml/202105041154.GrLZmjGh-lkp@intel.com/
>> https://lore.kernel.org/lkml/202105041634.paURyDp0-lkp@intel.com/
>> https://lore.kernel.org/lkml/202105050636.UXXDl7m2-lkp@intel.com/
> 
> At first glance it looks like Jakub's suggestion of
>>>> Maybe the config in question has PTP as a module and ionic built in?
>>>> Then you should add depends on PTP_1588_CLOCK || !PTP_1588_CLOCK.
> 
> is what is needed, but I'm still doing build testing ATM.

Nope, eat my words. These build issues are not about PTP.
I'm still looking.

-- 
~Randy


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

* Re: [PATCH v3 net] ionic: fix ptp support config breakage
  2021-05-07  1:16         ` Randy Dunlap
@ 2021-05-07  1:30           ` Randy Dunlap
  2021-05-07  3:26             ` Shannon Nelson
  0 siblings, 1 reply; 8+ messages in thread
From: Randy Dunlap @ 2021-05-07  1:30 UTC (permalink / raw)
  To: Shannon Nelson, Jakub Kicinski; +Cc: netdev, davem, drivers, Allen Hubbe

On 5/6/21 6:16 PM, Randy Dunlap wrote:
> On 5/6/21 6:12 PM, Randy Dunlap wrote:
>> On 5/6/21 5:48 PM, Shannon Nelson wrote:
>>> On 5/6/21 5:21 PM, Randy Dunlap wrote:
>>>> On 5/6/21 5:15 PM, Jakub Kicinski wrote:
>>>>> On Wed,  5 May 2021 21:18:46 -0700 Shannon Nelson wrote:
>>>>>> Driver link failed with undefined references in some
>>>>>> kernel config variations.
>>>>> This is really vague and the patch is not very obvious.

When IONIC=y and PTP_1588_CLOCK=m...

>>>>>>   ionic-y := ionic_main.o ionic_bus_pci.o ionic_devlink.o ionic_dev.o \
>>>>>>          ionic_debugfs.o ionic_lif.o ionic_rx_filter.o ionic_ethtool.o \
>>>>>> -       ionic_txrx.o ionic_stats.o ionic_fw.o
>>>>>> -ionic-$(CONFIG_PTP_1588_CLOCK) += ionic_phc.o
>>>>>> +       ionic_txrx.o ionic_stats.o ionic_fw.o ionic_phc.o
>>>>> So we'd replace a build dependency..
>>>>>
>>>>>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_phc.c b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>>>> index a87c87e86aef..30c78808c45a 100644
>>>>>> --- a/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>>>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>>>> @@ -1,6 +1,8 @@
>>>>>>   // SPDX-License-Identifier: GPL-2.0
>>>>>>   /* Copyright(c) 2017 - 2021 Pensando Systems, Inc */
>>>>>>   +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
>>>>>> +
>>>>>>   #include <linux/netdevice.h>
>>>>>>   #include <linux/etherdevice.h>
>>>>>>   @@ -613,3 +615,4 @@ void ionic_lif_free_phc(struct ionic_lif *lif)
>>>>>>       devm_kfree(lif->ionic->dev, lif->phc);
>>>>>>       lif->phc = NULL;
>>>>>>   }
>>>>>> +#endif /* IS_ENABLED(CONFIG_PTP_1588_CLOCK) */
>>>>> .. with an ifdef around an entire file? Does not feel very clean.
>>>>>
>>>>> The construct of using:
>>>>>
>>>>>     drv-$(CONFIG_PTP_1588_CLOCK) += ptp.o
>>>>>
>>>>> seems relatively common, why does it now work here?
>>>>>
>>>>> Maybe the config in question has PTP as a module and ionic built in?
>>>>> Then you should add depends on PTP_1588_CLOCK || !PTP_1588_CLOCK.
>>>>>
>>>>> Maybe somehow the "ionic-y" confuses kbuild and it should be ionic-objs?
>>>>>
>>>>> At the very least we need a better explanation in the commit message.
>>>>>
>>>> I'll take a look if someone can point me to the .config file.
>>>>
>>>
>>> These are the notes I got from kernel test robot:
>>> https://lore.kernel.org/lkml/202105041020.efEaBOYC-lkp@intel.com/
>>> https://lore.kernel.org/lkml/202105041154.GrLZmjGh-lkp@intel.com/
>>> https://lore.kernel.org/lkml/202105041634.paURyDp0-lkp@intel.com/
>>> https://lore.kernel.org/lkml/202105050636.UXXDl7m2-lkp@intel.com/
>>
>> At first glance it looks like Jakub's suggestion of
>>>>> Maybe the config in question has PTP as a module and ionic built in?
>>>>> Then you should add depends on PTP_1588_CLOCK || !PTP_1588_CLOCK.
>>
>> is what is needed, but I'm still doing build testing ATM.
> 
> Nope, eat my words. These build issues are not about PTP.
> I'm still looking.

I have been trying to go to fast.. slow down, wait for the old computer.

Back to Jakub's suggestion -- that works for me. (copy-paste, whitespace damaged)


--- linux-next-20210506.orig/drivers/net/ethernet/pensando/Kconfig
+++ linux-next-20210506/drivers/net/ethernet/pensando/Kconfig
@@ -20,6 +20,7 @@ if NET_VENDOR_PENSANDO
 config IONIC
        tristate "Pensando Ethernet IONIC Support"
        depends on 64BIT && PCI
+       depends on PTP_1588_CLOCK || !PTP_1588_CLOCK
        select NET_DEVLINK
        select DIMLIB
        help

If PTP_1588_CLOCK=m, the depends limits IONIC to =m (or disabled).
If PTP_1588_CLOCK is disabled, IONIC can be any of y/m/n.


-- 
~Randy


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

* Re: [PATCH v3 net] ionic: fix ptp support config breakage
  2021-05-07  1:30           ` Randy Dunlap
@ 2021-05-07  3:26             ` Shannon Nelson
  0 siblings, 0 replies; 8+ messages in thread
From: Shannon Nelson @ 2021-05-07  3:26 UTC (permalink / raw)
  To: Randy Dunlap, Jakub Kicinski; +Cc: netdev, davem, drivers, Allen Hubbe

On 5/6/21 6:30 PM, Randy Dunlap wrote:
> On 5/6/21 6:16 PM, Randy Dunlap wrote:
>> On 5/6/21 6:12 PM, Randy Dunlap wrote:
>>> On 5/6/21 5:48 PM, Shannon Nelson wrote:
>>>> On 5/6/21 5:21 PM, Randy Dunlap wrote:
>>>>> On 5/6/21 5:15 PM, Jakub Kicinski wrote:
>>>>>> On Wed,  5 May 2021 21:18:46 -0700 Shannon Nelson wrote:
>>>>>>> Driver link failed with undefined references in some
>>>>>>> kernel config variations.
>>>>>> This is really vague and the patch is not very obvious.
> When IONIC=y and PTP_1588_CLOCK=m...
>
>>>>>>>    ionic-y := ionic_main.o ionic_bus_pci.o ionic_devlink.o ionic_dev.o \
>>>>>>>           ionic_debugfs.o ionic_lif.o ionic_rx_filter.o ionic_ethtool.o \
>>>>>>> -       ionic_txrx.o ionic_stats.o ionic_fw.o
>>>>>>> -ionic-$(CONFIG_PTP_1588_CLOCK) += ionic_phc.o
>>>>>>> +       ionic_txrx.o ionic_stats.o ionic_fw.o ionic_phc.o
>>>>>> So we'd replace a build dependency..
>>>>>>
>>>>>>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_phc.c b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>>>>> index a87c87e86aef..30c78808c45a 100644
>>>>>>> --- a/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>>>>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>>>>> @@ -1,6 +1,8 @@
>>>>>>>    // SPDX-License-Identifier: GPL-2.0
>>>>>>>    /* Copyright(c) 2017 - 2021 Pensando Systems, Inc */
>>>>>>>    +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
>>>>>>> +
>>>>>>>    #include <linux/netdevice.h>
>>>>>>>    #include <linux/etherdevice.h>
>>>>>>>    @@ -613,3 +615,4 @@ void ionic_lif_free_phc(struct ionic_lif *lif)
>>>>>>>        devm_kfree(lif->ionic->dev, lif->phc);
>>>>>>>        lif->phc = NULL;
>>>>>>>    }
>>>>>>> +#endif /* IS_ENABLED(CONFIG_PTP_1588_CLOCK) */
>>>>>> .. with an ifdef around an entire file? Does not feel very clean.
>>>>>>
>>>>>> The construct of using:
>>>>>>
>>>>>>      drv-$(CONFIG_PTP_1588_CLOCK) += ptp.o
>>>>>>
>>>>>> seems relatively common, why does it now work here?
>>>>>>
>>>>>> Maybe the config in question has PTP as a module and ionic built in?
>>>>>> Then you should add depends on PTP_1588_CLOCK || !PTP_1588_CLOCK.
>>>>>>
>>>>>> Maybe somehow the "ionic-y" confuses kbuild and it should be ionic-objs?
>>>>>>
>>>>>> At the very least we need a better explanation in the commit message.
>>>>>>
>>>>> I'll take a look if someone can point me to the .config file.
>>>>>
>>>> These are the notes I got from kernel test robot:
>>>> https://lore.kernel.org/lkml/202105041020.efEaBOYC-lkp@intel.com/
>>>> https://lore.kernel.org/lkml/202105041154.GrLZmjGh-lkp@intel.com/
>>>> https://lore.kernel.org/lkml/202105041634.paURyDp0-lkp@intel.com/
>>>> https://lore.kernel.org/lkml/202105050636.UXXDl7m2-lkp@intel.com/
>>> At first glance it looks like Jakub's suggestion of
>>>>>> Maybe the config in question has PTP as a module and ionic built in?
>>>>>> Then you should add depends on PTP_1588_CLOCK || !PTP_1588_CLOCK.
>>> is what is needed, but I'm still doing build testing ATM.
>> Nope, eat my words. These build issues are not about PTP.
>> I'm still looking.
> I have been trying to go to fast.. slow down, wait for the old computer.
>
> Back to Jakub's suggestion -- that works for me. (copy-paste, whitespace damaged)
>
>
> --- linux-next-20210506.orig/drivers/net/ethernet/pensando/Kconfig
> +++ linux-next-20210506/drivers/net/ethernet/pensando/Kconfig
> @@ -20,6 +20,7 @@ if NET_VENDOR_PENSANDO
>   config IONIC
>          tristate "Pensando Ethernet IONIC Support"
>          depends on 64BIT && PCI
> +       depends on PTP_1588_CLOCK || !PTP_1588_CLOCK
>          select NET_DEVLINK
>          select DIMLIB
>          help
>
> If PTP_1588_CLOCK=m, the depends limits IONIC to =m (or disabled).
> If PTP_1588_CLOCK is disabled, IONIC can be any of y/m/n.
>

Thanks, I'll follow up with this.
sln



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

end of thread, other threads:[~2021-05-07  3:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-06  4:18 [PATCH v3 net] ionic: fix ptp support config breakage Shannon Nelson
2021-05-07  0:15 ` Jakub Kicinski
2021-05-07  0:21   ` Randy Dunlap
2021-05-07  0:48     ` Shannon Nelson
2021-05-07  1:12       ` Randy Dunlap
2021-05-07  1:16         ` Randy Dunlap
2021-05-07  1:30           ` Randy Dunlap
2021-05-07  3:26             ` Shannon Nelson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.