Re: [BUILD_FAILURE] 2.6.27-git2 - allyesconfig on powerpc selects CONFIG_INTEL_IOATDMA=y

Re: [BUILD_FAILURE] 2.6.27-git2 - allyesconfig on powerpc selects CONFIG_INTEL_IOATDMA=y

[Adrian Bunk]

On Mon, Oct 13, 2008 at 03:45:59PM +0530, Kamalesh Babulal wrote:
> Hi,
> 
>    2.6.27-git2 kernel build fails, while building the kernel with
> allyesconfig option. The allyesconfig selects CONFIG_INTEL_IOATDMA=y
> 
> CC   drivers/dma/ioat_dca.o
> drivers/dma/ioat_dca.c: In function ‘dca_enabled_in_bios’:
> drivers/dma/ioat_dca.c:81: error: implicit declaration of function ‘cpuid_eax’
> drivers/dma/ioat_dca.c: In function ‘system_has_dca_enabled’:
> drivers/dma/ioat_dca.c:91: error: implicit declaration of function ‘boot_cpu_has’
> drivers/dma/ioat_dca.c:91: error: ‘X86_FEATURE_DCA’ undeclared (first use in this function)
> drivers/dma/ioat_dca.c:91: error: (Each undeclared identifier is reported only once
> drivers/dma/ioat_dca.c:91: error: for each function it appears in.)
> drivers/dma/ioat_dca.c: In function ‘ioat_dca_get_tag’:
> drivers/dma/ioat_dca.c:190: error: implicit declaration of function ‘cpu_physical_id’
> make[2]: *** [drivers/dma/ioat_dca.o] Error 1
> make[1]: *** [drivers/dma] Error 2
> make: *** [drivers] Error 2
>...

Thanks for the report, the MYRI10GE and IXGBE commits that introduced 
the select's are really broken.

For fixing it I need to know the intended semantics.

Brian, Jesse, is it OK to limit the drivers to m with 
CONFIG_INTEL_IOATDMA=m ?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [BUILD_FAILURE] 2.6.27-git2 - allyesconfig on powerpc selects CONFIG_INTEL_IOATDMA=y

[Brice Goglin]

Adrian Bunk wrote:
> On Mon, Oct 13, 2008 at 03:45:59PM +0530, Kamalesh Babulal wrote:
>   
>> Hi,
>>
>>    2.6.27-git2 kernel build fails, while building the kernel with
>> allyesconfig option. The allyesconfig selects CONFIG_INTEL_IOATDMA=y
>>
>> CC   drivers/dma/ioat_dca.o
>> drivers/dma/ioat_dca.c: In function ‘dca_enabled_in_bios’:
>> drivers/dma/ioat_dca.c:81: error: implicit declaration of function ‘cpuid_eax’
>> drivers/dma/ioat_dca.c: In function ‘system_has_dca_enabled’:
>> drivers/dma/ioat_dca.c:91: error: implicit declaration of function ‘boot_cpu_has’
>> drivers/dma/ioat_dca.c:91: error: ‘X86_FEATURE_DCA’ undeclared (first use in this function)
>> drivers/dma/ioat_dca.c:91: error: (Each undeclared identifier is reported only once
>> drivers/dma/ioat_dca.c:91: error: for each function it appears in.)
>> drivers/dma/ioat_dca.c: In function ‘ioat_dca_get_tag’:
>> drivers/dma/ioat_dca.c:190: error: implicit declaration of function ‘cpu_physical_id’
>> make[2]: *** [drivers/dma/ioat_dca.o] Error 1
>> make[1]: *** [drivers/dma] Error 2
>> make: *** [drivers] Error 2
>> ...
>>     
>
> Thanks for the report, the MYRI10GE and IXGBE commits that introduced 
> the select's are really broken.
>
> For fixing it I need to know the intended semantics.
>
> Brian, Jesse, is it OK to limit the drivers to m with 
> CONFIG_INTEL_IOATDMA=m ?
>   

I think I would rather drop DCA from myri10ge if IOATDMA=m while
myri10ge=y. What's the simplest way to do so?

When Jesse told me to commit this in myri10ge, I thought it would be
nice to have DCA work the same than NETDMA/DMAengine does: you can have
NETDMA enabled without IOATDMA (either not built at all, or just not
loaded). You just don't get any DMA channel when you ask for one. Why
not do the same for DCA? There could be some generic DCA layer that can
be built all the time and returns DCA resources only if IOATDMA is
loaded/built ?

Brice

Re: [BUILD_FAILURE] 2.6.27-git2 - allyesconfig on powerpc selectsCONFIG_INTEL_IOATDMA=y

[Brandeburg, Jesse]

Adrian Bunk wrote:
> On Mon, Oct 13, 2008 at 03:45:59PM +0530, Kamalesh Babulal wrote:
>> Hi,
>> 
>>    2.6.27-git2 kernel build fails, while building the kernel with

from subject, on PPC.

>> allyesconfig option. The allyesconfig selects CONFIG_INTEL_IOATDMA=y
>> 
>> CC   drivers/dma/ioat_dca.o
>> drivers/dma/ioat_dca.c: In function ‘dca_enabled_in_bios’:
>> drivers/dma/ioat_dca.c:81: error: implicit declaration of function
>> ‘cpuid_eax’ drivers/dma/ioat_dca.c: In function
>> ‘system_has_dca_enabled’: 
>> drivers/dma/ioat_dca.c:91: error: implicit declaration of function
>> ‘boot_cpu_has’ drivers/dma/ioat_dca.c:91: error:
>> ‘X86_FEATURE_DCA’ undeclared (first use in this function)
>> drivers/dma/ioat_dca.c:91: error: (Each undeclared identifier is
>> reported only once drivers/dma/ioat_dca.c:91: error: for each
>> function it appears in.)  
>> drivers/dma/ioat_dca.c: In function ‘ioat_dca_get_tag’:
>> drivers/dma/ioat_dca.c:190: error: implicit declaration of function
>> ‘cpu_physical_id’ make[2]: *** [drivers/dma/ioat_dca.o] Error 1
>> make[1]: *** [drivers/dma] Error 2
>> make: *** [drivers] Error 2
>> ...
> 
> Thanks for the report, the MYRI10GE and IXGBE commits that introduced
> the select's are really broken.

it fixed an obvious bug with CONFIG_IXGBE=y and CONFIG_INTEL_IOATDMA=m, unfortunate that it is so difficult to use Kconfig for the uninitiated.
 
> For fixing it I need to know the intended semantics.
> 
> Brian, Jesse, is it OK to limit the drivers to m with
> CONFIG_INTEL_IOATDMA=m ?

why is the IOATDMA driver not depending on x86?  x86 hardware (32 and 64 bit) are the only machines that will have ioatdma support, since the ioatdma (and dca) device is only in Intel server chipsets since S5000 (aka S5000, S5400, S7400 and derivatives/followons)

What we want, is myri10ge and ixgbe drivers that can build whether or not CONFIG_INTEL_IOATDMA is enabled.  IF CONFIG_INTEL_IOATDMA *is* enabled (which it should not be on PPC) then there are several cases we want to work:
CONFIG_INTEL_IOATDMA=m  ---> CONFIG_IXGBE=[m|n]
CONFIG_INTEL_IOATDMA=y  ---> CONFIG_IXGBE=[m|y|n]
CONFIG_INTEL_IOATDMA=n  ---> CONFIG_IXGBE=[m|y|n]
CONFIG_INTEL_IOATDMA depends on X86

same for myri10ge I think.  I hope that cleared something up, we can seriously use some help from the experts to get Kconfig correct for this.

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [BUILD_FAILURE] 2.6.27-git2 - allyesconfig on powerpc selectsCONFIG_INTEL_IOATDMA=y

[Brice Goglin]

Brandeburg, Jesse wrote:
> What we want, is myri10ge and ixgbe drivers that can build whether or not CONFIG_INTEL_IOATDMA is enabled.  IF CONFIG_INTEL_IOATDMA *is* enabled (which it should not be on PPC) then there are several cases we want to work:
> CONFIG_INTEL_IOATDMA=m  ---> CONFIG_IXGBE=[m|n]
> CONFIG_INTEL_IOATDMA=y  ---> CONFIG_IXGBE=[m|y|n]
> CONFIG_INTEL_IOATDMA=n  ---> CONFIG_IXGBE=[m|y|n]
> CONFIG_INTEL_IOATDMA depends on X86
>   

I am not sure I want to prevent myri10ge=y just because ioatdma=m.

I would vote for adding some Kconfig stuff to define CONFIG_MYRI10GE_DCA
as boolean set to yes if (IOATDMA=y and MYRI10GE=y/m) or (IOATDMA=m and
MYRI10GE=m). And then use #ifdef CONFIG_MYRI10GE_DCA in the driver source.

Brice


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [BUILD_FAILURE] 2.6.27-git2 - allyesconfig on powerpc selectsCONFIG_INTEL_IOATDMA=y

[Adrian Bunk]

On Mon, Oct 13, 2008 at 08:21:45PM +0200, Brice Goglin wrote:
> Brandeburg, Jesse wrote:
> > What we want, is myri10ge and ixgbe drivers that can build whether or not CONFIG_INTEL_IOATDMA is enabled.  IF CONFIG_INTEL_IOATDMA *is* enabled (which it should not be on PPC) then there are several cases we want to work:
> > CONFIG_INTEL_IOATDMA=m  ---> CONFIG_IXGBE=[m|n]
> > CONFIG_INTEL_IOATDMA=y  ---> CONFIG_IXGBE=[m|y|n]
> > CONFIG_INTEL_IOATDMA=n  ---> CONFIG_IXGBE=[m|y|n]
> > CONFIG_INTEL_IOATDMA depends on X86
> 
> I am not sure I want to prevent myri10ge=y just because ioatdma=m.
>...

Technically both solutions are possible, which to choose is just a 
policy decision you have to agree on.

In practice it will anyway only rarely matter.

But considering that igb is in a similar situation it would be nice if 
all 3 drivers would handle it the same way.

> Brice

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [BUILD_FAILURE] 2.6.27-git2 - allyesconfig on powerpc selectsCONFIG_INTEL_IOATDMA=y

[Brice Goglin]

Adrian Bunk wrote:
> But considering that igb is in a similar situation it would be nice if 
> all 3 drivers would handle it the same way.
>   

Jesse,
What do you think of the below patch?
I am not very familiar with Kconfig, but it seems to solve the problem.
If a Kconfig guru could double-check...
Brice


myri10ge: Add MYRI10GE_DCA instead of selecting INTEL_IOATDMA

Add a bool MYRI10GE_DCA defined to y if MYRI10GE and DCA are enabled, but
MYRI10GE isn't y while DCA=m. And thus remove the need to select INTEL_IOATDMA
when MYRI10GE is enabled, so that non-x86 architectures can build the myri10ge.

Signed-off-by: Brice Goglin <brice@myri.com>

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index e9d5294..0162d55 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -2462,7 +2462,6 @@ config MYRI10GE
 	select FW_LOADER
 	select CRC32
 	select INET_LRO
-	select INTEL_IOATDMA
 	---help---
 	  This driver supports Myricom Myri-10G Dual Protocol interface in
 	  Ethernet mode. If the eeprom on your board is not recent enough,
@@ -2474,6 +2473,11 @@ config MYRI10GE
 	  To compile this driver as a module, choose M here. The module
 	  will be called myri10ge.
 
+config MYRI10GE_DCA
+	bool
+	default y
+	depends on MYRI10GE && DCA && !(MYRI10GE=y && DCA=m)
+
 config NETXEN_NIC
 	tristate "NetXen Multi port (1/10) Gigabit Ethernet NIC"
 	depends on PCI
diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index 6dce901..a9aebad 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -188,7 +188,7 @@ struct myri10ge_slice_state {
 	dma_addr_t fw_stats_bus;
 	int watchdog_tx_done;
 	int watchdog_tx_req;
-#if (defined CONFIG_DCA) || (defined CONFIG_DCA_MODULE)
+#ifdef CONFIG_MYRI10GE_DCA
 	int cached_dca_tag;
 	int cpu;
 	__be32 __iomem *dca_tag;
@@ -220,7 +220,7 @@ struct myri10ge_priv {
 	int msi_enabled;
 	int msix_enabled;
 	struct msix_entry *msix_vectors;
-#if (defined CONFIG_DCA) || (defined CONFIG_DCA_MODULE)
+#ifdef CONFIG_MYRI10GE_DCA
 	int dca_enabled;
 #endif
 	u32 link_state;
@@ -902,7 +902,7 @@ static int myri10ge_reset(struct myri10ge_priv *mgp)
 	struct myri10ge_slice_state *ss;
 	int i, status;
 	size_t bytes;
-#if (defined CONFIG_DCA) || (defined CONFIG_DCA_MODULE)
+#ifdef CONFIG_MYRI10GE_DCA
 	unsigned long dca_tag_off;
 #endif
 
@@ -1012,7 +1012,7 @@ static int myri10ge_reset(struct myri10ge_priv *mgp)
 	}
 	put_be32(htonl(mgp->intr_coal_delay), mgp->intr_coal_delay_ptr);
 
-#if (defined CONFIG_DCA) || (defined CONFIG_DCA_MODULE)
+#ifdef CONFIG_MYRI10GE_DCA
 	status = myri10ge_send_cmd(mgp, MXGEFW_CMD_GET_DCA_OFFSET, &cmd, 0);
 	dca_tag_off = cmd.data0;
 	for (i = 0; i < mgp->num_slices; i++) {
@@ -1051,7 +1051,7 @@ static int myri10ge_reset(struct myri10ge_priv *mgp)
 	return status;
 }
 
-#if (defined CONFIG_DCA) || (defined CONFIG_DCA_MODULE)
+#ifdef CONFIG_MYRI10GE_DCA
 static void
 myri10ge_write_dca(struct myri10ge_slice_state *ss, int cpu, int tag)
 {
@@ -1505,7 +1505,7 @@ static int myri10ge_poll(struct napi_struct *napi, int budget)
 	struct net_device *netdev = ss->mgp->dev;
 	int work_done;
 
-#if (defined CONFIG_DCA) || (defined CONFIG_DCA_MODULE)
+#ifdef CONFIG_MYRI10GE_DCA
 	if (ss->mgp->dca_enabled)
 		myri10ge_update_dca(ss);
 #endif
@@ -1736,7 +1736,7 @@ static const char myri10ge_gstrings_main_stats[][ETH_GSTRING_LEN] = {
 	"tx_boundary", "WC", "irq", "MSI", "MSIX",
 	"read_dma_bw_MBs", "write_dma_bw_MBs", "read_write_dma_bw_MBs",
 	"serial_number", "watchdog_resets",
-#if (defined CONFIG_DCA) || (defined CONFIG_DCA_MODULE)
+#ifdef CONFIG_MYRI10GE_DCA
 	"dca_capable_firmware", "dca_device_present",
 #endif
 	"link_changes", "link_up", "dropped_link_overflow",
@@ -1815,7 +1815,7 @@ myri10ge_get_ethtool_stats(struct net_device *netdev,
 	data[i++] = (unsigned int)mgp->read_write_dma;
 	data[i++] = (unsigned int)mgp->serial_number;
 	data[i++] = (unsigned int)mgp->watchdog_resets;
-#if (defined CONFIG_DCA) || (defined CONFIG_DCA_MODULE)
+#ifdef CONFIG_MYRI10GE_DCA
 	data[i++] = (unsigned int)(mgp->ss[0].dca_tag != NULL);
 	data[i++] = (unsigned int)(mgp->dca_enabled);
 #endif
@@ -3844,7 +3844,7 @@ static int myri10ge_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		dev_err(&pdev->dev, "failed reset\n");
 		goto abort_with_slices;
 	}
-#if (defined CONFIG_DCA) || (defined CONFIG_DCA_MODULE)
+#ifdef CONFIG_MYRI10GE_DCA
 	myri10ge_setup_dca(mgp);
 #endif
 	pci_set_drvdata(pdev, mgp);
@@ -3948,7 +3948,7 @@ static void myri10ge_remove(struct pci_dev *pdev)
 	netdev = mgp->dev;
 	unregister_netdev(netdev);
 
-#if (defined CONFIG_DCA) || (defined CONFIG_DCA_MODULE)
+#ifdef CONFIG_MYRI10GE_DCA
 	myri10ge_teardown_dca(mgp);
 #endif
 	myri10ge_dummy_rdma(mgp, 0);
@@ -3993,7 +3993,7 @@ static struct pci_driver myri10ge_driver = {
 #endif
 };
 
-#if (defined CONFIG_DCA) || (defined CONFIG_DCA_MODULE)
+#ifdef CONFIG_MYRI10GE_DCA
 static int
 myri10ge_notify_dca(struct notifier_block *nb, unsigned long event, void *p)
 {
@@ -4024,7 +4024,7 @@ static __init int myri10ge_init_module(void)
 		       myri10ge_driver.name, myri10ge_rss_hash);
 		myri10ge_rss_hash = MXGEFW_RSS_HASH_TYPE_SRC_PORT;
 	}
-#if (defined CONFIG_DCA) || (defined CONFIG_DCA_MODULE)
+#ifdef CONFIG_MYRI10GE_DCA
 	dca_register_notify(&myri10ge_dca_notifier);
 #endif
 	if (myri10ge_max_slices > MYRI10GE_MAX_SLICES)
@@ -4037,7 +4037,7 @@ module_init(myri10ge_init_module);
 
 static __exit void myri10ge_cleanup_module(void)
 {
-#if (defined CONFIG_DCA) || (defined CONFIG_DCA_MODULE)
+#ifdef CONFIG_MYRI10GE_DCA
 	dca_unregister_notify(&myri10ge_dca_notifier);
 #endif
 	pci_unregister_driver(&myri10ge_driver);



-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [BUILD_FAILURE] 2.6.27-git2 - allyesconfig on powerpc selectsCONFIG_INTEL_IOATDMA=y

[Adrian Bunk]

On Tue, Oct 14, 2008 at 11:12:30AM +0200, Brice Goglin wrote:
> Adrian Bunk wrote:
> > But considering that igb is in a similar situation it would be nice if 
> > all 3 drivers would handle it the same way.
> >   
> 
> Jesse,
> What do you think of the below patch?
> I am not very familiar with Kconfig, but it seems to solve the problem.
> If a Kconfig guru could double-check...

The patch looks good, but the main problem is to get an agreement 
on the policy.

> Brice
>...

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

RE: [BUILD_FAILURE] 2.6.27-git2 - allyesconfig on powerpc selectsCONFIG_INTEL_IOATDMA=y

[Brandeburg, Jesse]

Brice Goglin wrote:
> Adrian Bunk wrote:
>> But considering that igb is in a similar situation it would be nice
>> if all 3 drivers would handle it the same way.
>> 
> 
> Jesse,
> What do you think of the below patch?

Seems like a much better solution.  I can have Jeff Kirsher work on the
equivalent patches for igb, and ixgbe today.

> I am not very familiar with Kconfig, but it seems to solve the
> problem. 
> If a Kconfig guru could double-check...

Yeah, please Kconfig gurus on the list have a quick look.

Re: [BUILD_FAILURE] 2.6.27-git2 - allyesconfig on powerpc selectsCONFIG_INTEL_IOATDMA=y

[Adrian Bunk]

On Tue, Oct 14, 2008 at 09:10:33AM -0700, Brandeburg, Jesse wrote:
> Brice Goglin wrote:
> > Adrian Bunk wrote:
> >> But considering that igb is in a similar situation it would be nice
> >> if all 3 drivers would handle it the same way.
> >> 
> > 
> > Jesse,
> > What do you think of the below patch?
> 
> Seems like a much better solution.  I can have Jeff Kirsher work on the
> equivalent patches for igb, and ixgbe today.
> 
> > I am not very familiar with Kconfig, but it seems to solve the
> > problem. 
> > If a Kconfig guru could double-check...
> 
> Yeah, please Kconfig gurus on the list have a quick look.

Brice's patch looks fine.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [BUILD_FAILURE] 2.6.27-git2 - allyesconfig on powerpc selectsCONFIG_INTEL_IOATDMA=y

[Kamalesh Babulal]

* Adrian Bunk <bunk@kernel.org> [2008-10-14 20:58:22]:

> On Tue, Oct 14, 2008 at 09:10:33AM -0700, Brandeburg, Jesse wrote:
> > Brice Goglin wrote:
> > > Adrian Bunk wrote:
> > >> But considering that igb is in a similar situation it would be nice
> > >> if all 3 drivers would handle it the same way.
> > >> 
> > > 
> > > Jesse,
> > > What do you think of the below patch?
> > 
> > Seems like a much better solution.  I can have Jeff Kirsher work on the
> > equivalent patches for igb, and ixgbe today.
> > 
> > > I am not very familiar with Kconfig, but it seems to solve the
> > > problem. 
> > > If a Kconfig guru could double-check...
> > 
> > Yeah, please Kconfig gurus on the list have a quick look.
> 
> Brice's patch looks fine.
> 
> cu
> Adrian
> 

Thanks, the patch fixes the build failure with CONFIG_MYRI10GE=y, once
patch for CONFIG_IXGBE are send out by Jeff Kirsher/Jesse, I will retest
them.

-- 
Thanks & Regards,
Kamalesh Babulal,
Linux Technology Center,
IBM, ISTL.