* [PATCH] cxgb{3,4}: streamline Kconfig options
@ 2011-02-17 13:29 Jan Beulich
2011-02-22 18:14 ` David Miller
0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2011-02-17 13:29 UTC (permalink / raw)
To: divy, dm; +Cc: linux-kbuild, netdev
The CHELSIO_T{3,4}_DEPENDS options are really awkward, and can be
easily dropped if the reverse dependencies of SCSI_CXGB{3,4}_ISCSI on
the former get converted to normal (forward) ones referring to
CHELSIO_T{3,4}.
Signed-off-by: Jan Beulich <jbeulich@novell.com>
---
drivers/net/Kconfig | 14 ++------------
drivers/scsi/cxgbi/cxgb3i/Kconfig | 3 +--
drivers/scsi/cxgbi/cxgb4i/Kconfig | 3 +--
3 files changed, 4 insertions(+), 16 deletions(-)
--- 2.6.38-rc5/drivers/net/Kconfig
+++ 2.6.38-rc5-kconfig-chelsio/drivers/net/Kconfig
@@ -2594,14 +2594,9 @@ config CHELSIO_T1_1G
Enables support for Chelsio's gigabit Ethernet PCI cards. If you
are using only 10G cards say 'N' here.
-config CHELSIO_T3_DEPENDS
- tristate
- depends on PCI && INET
- default y
-
config CHELSIO_T3
tristate "Chelsio Communications T3 10Gb Ethernet support"
- depends on CHELSIO_T3_DEPENDS
+ depends on PCI && INET
select FW_LOADER
select MDIO
help
@@ -2619,14 +2614,9 @@ config CHELSIO_T3
To compile this driver as a module, choose M here: the module
will be called cxgb3.
-config CHELSIO_T4_DEPENDS
- tristate
- depends on PCI && INET
- default y
-
config CHELSIO_T4
tristate "Chelsio Communications T4 Ethernet support"
- depends on CHELSIO_T4_DEPENDS
+ depends on PCI && INET
select FW_LOADER
select MDIO
help
--- 2.6.38-rc5/drivers/scsi/cxgbi/cxgb3i/Kconfig
+++ 2.6.38-rc5-kconfig-chelsio/drivers/scsi/cxgbi/cxgb3i/Kconfig
@@ -1,7 +1,6 @@
config SCSI_CXGB3_ISCSI
tristate "Chelsio T3 iSCSI support"
- depends on CHELSIO_T3_DEPENDS
- select CHELSIO_T3
+ depends on CHELSIO_T3
select SCSI_ISCSI_ATTRS
---help---
This driver supports iSCSI offload for the Chelsio T3 devices.
--- 2.6.38-rc5/drivers/scsi/cxgbi/cxgb4i/Kconfig
+++ 2.6.38-rc5-kconfig-chelsio/drivers/scsi/cxgbi/cxgb4i/Kconfig
@@ -1,7 +1,6 @@
config SCSI_CXGB4_ISCSI
tristate "Chelsio T4 iSCSI support"
- depends on CHELSIO_T4_DEPENDS
- select CHELSIO_T4
+ depends on CHELSIO_T4
select SCSI_ISCSI_ATTRS
---help---
This driver supports iSCSI offload for the Chelsio T4 devices.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] cxgb{3,4}: streamline Kconfig options
2011-02-17 13:29 [PATCH] cxgb{3,4}: streamline Kconfig options Jan Beulich
@ 2011-02-22 18:14 ` David Miller
2011-02-23 9:46 ` Jan Beulich
0 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2011-02-22 18:14 UTC (permalink / raw)
To: JBeulich; +Cc: divy, dm, linux-kbuild, netdev
From: "Jan Beulich" <JBeulich@novell.com>
Date: Thu, 17 Feb 2011 13:29:30 +0000
> The CHELSIO_T{3,4}_DEPENDS options are really awkward, and can be
> easily dropped if the reverse dependencies of SCSI_CXGB{3,4}_ISCSI on
> the former get converted to normal (forward) ones referring to
> CHELSIO_T{3,4}.
>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
I think the goal of these strange rules is not to be complicated
on purpose, but rather to cause the iSCSI drivers to appear without
the user having to know that he needs to enable the networking
driver in order for that to happen.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] cxgb{3,4}: streamline Kconfig options
2011-02-22 18:14 ` David Miller
@ 2011-02-23 9:46 ` Jan Beulich
2011-02-23 20:27 ` David Miller
0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2011-02-23 9:46 UTC (permalink / raw)
To: David Miller; +Cc: divy, dm, linux-kbuild, netdev
>>> On 22.02.11 at 19:14, David Miller <davem@davemloft.net> wrote:
> From: "Jan Beulich" <JBeulich@novell.com>
> Date: Thu, 17 Feb 2011 13:29:30 +0000
>
>> The CHELSIO_T{3,4}_DEPENDS options are really awkward, and can be
>> easily dropped if the reverse dependencies of SCSI_CXGB{3,4}_ISCSI on
>> the former get converted to normal (forward) ones referring to
>> CHELSIO_T{3,4}.
>>
>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
> I think the goal of these strange rules is not to be complicated
> on purpose, but rather to cause the iSCSI drivers to appear without
> the user having to know that he needs to enable the networking
> driver in order for that to happen.
While I realize that this might have been the reason, it's completely
contrary to how everyone else writes dependencies, and hence I
think these should be removed.
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] cxgb{3,4}: streamline Kconfig options
2011-02-23 9:46 ` Jan Beulich
@ 2011-02-23 20:27 ` David Miller
2011-02-24 8:00 ` Jan Beulich
0 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2011-02-23 20:27 UTC (permalink / raw)
To: JBeulich; +Cc: divy, dm, linux-kbuild, netdev
From: "Jan Beulich" <JBeulich@novell.com>
Date: Wed, 23 Feb 2011 09:46:10 +0000
>>>> On 22.02.11 at 19:14, David Miller <davem@davemloft.net> wrote:
>> From: "Jan Beulich" <JBeulich@novell.com>
>> Date: Thu, 17 Feb 2011 13:29:30 +0000
>>
>>> The CHELSIO_T{3,4}_DEPENDS options are really awkward, and can be
>>> easily dropped if the reverse dependencies of SCSI_CXGB{3,4}_ISCSI on
>>> the former get converted to normal (forward) ones referring to
>>> CHELSIO_T{3,4}.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>>
>> I think the goal of these strange rules is not to be complicated
>> on purpose, but rather to cause the iSCSI drivers to appear without
>> the user having to know that he needs to enable the networking
>> driver in order for that to happen.
>
> While I realize that this might have been the reason, it's completely
> contrary to how everyone else writes dependencies, and hence I
> think these should be removed.
If you knew you were changing the behavior of the config option in
this way, you sure didn't think it was worth mentioning in your commit
message.
I definitely would never expect to have to enable a scsi option to get
some network driver visible to enable in the config, and therefore I
could see the opposite being insanely frustrating too.
You can't ignore these issues and just say "that's not the normal way
so I'm going to change it anyways."
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] cxgb{3,4}: streamline Kconfig options
2011-02-23 20:27 ` David Miller
@ 2011-02-24 8:00 ` Jan Beulich
2011-02-24 19:41 ` Dimitris Michailidis
0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2011-02-24 8:00 UTC (permalink / raw)
To: David Miller; +Cc: divy, dm, linux-kbuild, netdev
>>> On 23.02.11 at 21:27, David Miller <davem@davemloft.net> wrote:
> From: "Jan Beulich" <JBeulich@novell.com>
> Date: Wed, 23 Feb 2011 09:46:10 +0000
>
>>>>> On 22.02.11 at 19:14, David Miller <davem@davemloft.net> wrote:
>>> From: "Jan Beulich" <JBeulich@novell.com>
>>> Date: Thu, 17 Feb 2011 13:29:30 +0000
>>>
>>>> The CHELSIO_T{3,4}_DEPENDS options are really awkward, and can be
>>>> easily dropped if the reverse dependencies of SCSI_CXGB{3,4}_ISCSI on
>>>> the former get converted to normal (forward) ones referring to
>>>> CHELSIO_T{3,4}.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>>>
>>> I think the goal of these strange rules is not to be complicated
>>> on purpose, but rather to cause the iSCSI drivers to appear without
>>> the user having to know that he needs to enable the networking
>>> driver in order for that to happen.
>>
>> While I realize that this might have been the reason, it's completely
>> contrary to how everyone else writes dependencies, and hence I
>> think these should be removed.
>
> If you knew you were changing the behavior of the config option in
> this way, you sure didn't think it was worth mentioning in your commit
> message.
I stated in the comment what I think this is - awkward.
> I definitely would never expect to have to enable a scsi option to get
> some network driver visible to enable in the config, and therefore I
> could see the opposite being insanely frustrating too.
The resulting dependency seems quite logical to me: Some higher
level networking functionality (iSCSI) depends on some lower level
networking functionality (an actual driver).
> You can't ignore these issues and just say "that's not the normal way
> so I'm going to change it anyways."
Admittedly I considered only my personal perspective.
Now, to get the whole discussion productive again - where do we
go from here? I don't think these drivers are so special that they
really need to behave backwards to how (almost?) everything else
is done... If changing it the way I did in the first try isn't deemed
acceptable, would it be at least acceptable to remove those
helper options (or, not as welcome from my perspective not the
least because of the odd dependency on INET instead of NET,
fold them into a single more generic one that others could also
benefit from)?
As to that INET vs NET dependency - is it possible that the
network drivers really just need NET, but the iSCSI ones need
INET? In which case the only common dependency would be
PCI - certainly not worth a custom helper option.
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] cxgb{3,4}: streamline Kconfig options
2011-02-24 8:00 ` Jan Beulich
@ 2011-02-24 19:41 ` Dimitris Michailidis
2011-02-25 19:51 ` Dimitris Michailidis
0 siblings, 1 reply; 9+ messages in thread
From: Dimitris Michailidis @ 2011-02-24 19:41 UTC (permalink / raw)
To: Jan Beulich; +Cc: David Miller, divy, linux-kbuild, netdev
Jan Beulich wrote:
> As to that INET vs NET dependency - is it possible that the
> network drivers really just need NET, but the iSCSI ones need
> INET? In which case the only common dependency would be
> PCI - certainly not worth a custom helper option.
I see about a dozen network drivers that depend on INET. These may be the
result of cut&paste from other drivers' Kconfig entries rather than actual
dependencies. Also some of these drivers select or selected in the past
INET_LRO and that may have something to do with their INET dependency, not sure.
Reading the commit message that introduced CHELSIO_T3_DEPENDS, it talks of
hidden dependencies that select does not see. I am not sure which exactly
but since it's been a few years since that commit I'll try to see what the
situation is today without the *_DEPENDS symbols and let you know.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] cxgb{3,4}: streamline Kconfig options
2011-02-24 19:41 ` Dimitris Michailidis
@ 2011-02-25 19:51 ` Dimitris Michailidis
2011-02-28 8:19 ` Jan Beulich
0 siblings, 1 reply; 9+ messages in thread
From: Dimitris Michailidis @ 2011-02-25 19:51 UTC (permalink / raw)
To: Jan Beulich; +Cc: David Miller, divy, linux-kbuild, netdev
Dimitris Michailidis wrote:
> Jan Beulich wrote:
>
>> As to that INET vs NET dependency - is it possible that the
>> network drivers really just need NET, but the iSCSI ones need
>> INET? In which case the only common dependency would be
>> PCI - certainly not worth a custom helper option.
>
> Reading the commit message that introduced CHELSIO_T3_DEPENDS, it talks
> of hidden dependencies that select does not see. I am not sure which
> exactly but since it's been a few years since that commit I'll try to
> see what the situation is today without the *_DEPENDS symbols and let
> you know.
I looked into this and found that with the current Kconfig the iSCSI driver
does not appear in the SCSI menu until one first enables NETDEVICES and
NETDEV_10000 in the network driver menu. It appears that the *_DEPENDS
symbols were added to capture dependencies on such symbols within the
network driver Kconfig, besides the dependencies the driver's entry listed
explicitly.
The patch below removes *T4*_DEPENDS and the network drivers' unnecessary
dependency on INET, and updates the iSCSI driver's entry so it is visible
without requiring any net driver options to be enabled first and has
adequate selects to be able to build the net driver (this part is adapted
from bnx2i's Kconfig entry). I still need to do the T3 part of this and
check that there isn't a conflict with the current scsi tree. Just for
review at this time.
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 0382332..0d314d5 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -2619,14 +2619,9 @@ config CHELSIO_T3
To compile this driver as a module, choose M here: the module
will be called cxgb3.
-config CHELSIO_T4_DEPENDS
- tristate
- depends on PCI && INET
- default y
-
config CHELSIO_T4
tristate "Chelsio Communications T4 Ethernet support"
- depends on CHELSIO_T4_DEPENDS
+ depends on PCI
select FW_LOADER
select MDIO
help
@@ -2644,14 +2639,9 @@ config CHELSIO_T4
To compile this driver as a module choose M here; the module
will be called cxgb4.
-config CHELSIO_T4VF_DEPENDS
- tristate
- depends on PCI && INET
- default y
-
config CHELSIO_T4VF
tristate "Chelsio Communications T4 Virtual Function Ethernet support"
- depends on CHELSIO_T4VF_DEPENDS
+ depends on PCI
help
This driver supports Chelsio T4-based gigabit and 10Gb Ethernet
adapters with PCI-E SR-IOV Virtual Functions.
diff --git a/drivers/scsi/cxgbi/cxgb4i/Kconfig
b/drivers/scsi/cxgbi/cxgb4i/Kconfig
index bb94b39..d5302c2 100644
--- a/drivers/scsi/cxgbi/cxgb4i/Kconfig
+++ b/drivers/scsi/cxgbi/cxgb4i/Kconfig
@@ -1,6 +1,8 @@
config SCSI_CXGB4_ISCSI
tristate "Chelsio T4 iSCSI support"
- depends on CHELSIO_T4_DEPENDS
+ depends on PCI && INET
+ select NETDEVICES
+ select NETDEV_10000
select CHELSIO_T4
select SCSI_ISCSI_ATTRS
---help---
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] cxgb{3,4}: streamline Kconfig options
2011-02-25 19:51 ` Dimitris Michailidis
@ 2011-02-28 8:19 ` Jan Beulich
2011-02-28 21:19 ` David Miller
0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2011-02-28 8:19 UTC (permalink / raw)
To: Dimitris Michailidis; +Cc: divy, David Miller, linux-kbuild, netdev
>>> On 25.02.11 at 20:51, Dimitris Michailidis <dm@chelsio.com> wrote:
> Dimitris Michailidis wrote:
>> Jan Beulich wrote:
>>
>>> As to that INET vs NET dependency - is it possible that the
>>> network drivers really just need NET, but the iSCSI ones need
>>> INET? In which case the only common dependency would be
>>> PCI - certainly not worth a custom helper option.
>>
>> Reading the commit message that introduced CHELSIO_T3_DEPENDS, it talks
>> of hidden dependencies that select does not see. I am not sure which
>> exactly but since it's been a few years since that commit I'll try to
>> see what the situation is today without the *_DEPENDS symbols and let
>> you know.
>
> I looked into this and found that with the current Kconfig the iSCSI driver
> does not appear in the SCSI menu until one first enables NETDEVICES and
> NETDEV_10000 in the network driver menu. It appears that the *_DEPENDS
> symbols were added to capture dependencies on such symbols within the
> network driver Kconfig, besides the dependencies the driver's entry listed
> explicitly.
>
> The patch below removes *T4*_DEPENDS and the network drivers' unnecessary
> dependency on INET, and updates the iSCSI driver's entry so it is visible
> without requiring any net driver options to be enabled first and has
> adequate selects to be able to build the net driver (this part is adapted
> from bnx2i's Kconfig entry). I still need to do the T3 part of this and
> check that there isn't a conflict with the current scsi tree. Just for
> review at this time.
Thanks, this looks good to me.
Jan
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 0382332..0d314d5 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -2619,14 +2619,9 @@ config CHELSIO_T3
> To compile this driver as a module, choose M here: the module
> will be called cxgb3.
>
> -config CHELSIO_T4_DEPENDS
> - tristate
> - depends on PCI && INET
> - default y
> -
> config CHELSIO_T4
> tristate "Chelsio Communications T4 Ethernet support"
> - depends on CHELSIO_T4_DEPENDS
> + depends on PCI
> select FW_LOADER
> select MDIO
> help
> @@ -2644,14 +2639,9 @@ config CHELSIO_T4
> To compile this driver as a module choose M here; the module
> will be called cxgb4.
>
> -config CHELSIO_T4VF_DEPENDS
> - tristate
> - depends on PCI && INET
> - default y
> -
> config CHELSIO_T4VF
> tristate "Chelsio Communications T4 Virtual Function Ethernet support"
> - depends on CHELSIO_T4VF_DEPENDS
> + depends on PCI
> help
> This driver supports Chelsio T4-based gigabit and 10Gb Ethernet
> adapters with PCI-E SR-IOV Virtual Functions.
> diff --git a/drivers/scsi/cxgbi/cxgb4i/Kconfig
> b/drivers/scsi/cxgbi/cxgb4i/Kconfig
> index bb94b39..d5302c2 100644
> --- a/drivers/scsi/cxgbi/cxgb4i/Kconfig
> +++ b/drivers/scsi/cxgbi/cxgb4i/Kconfig
> @@ -1,6 +1,8 @@
> config SCSI_CXGB4_ISCSI
> tristate "Chelsio T4 iSCSI support"
> - depends on CHELSIO_T4_DEPENDS
> + depends on PCI && INET
> + select NETDEVICES
> + select NETDEV_10000
> select CHELSIO_T4
> select SCSI_ISCSI_ATTRS
> ---help---
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] cxgb{3,4}: streamline Kconfig options
2011-02-28 8:19 ` Jan Beulich
@ 2011-02-28 21:19 ` David Miller
0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2011-02-28 21:19 UTC (permalink / raw)
To: JBeulich; +Cc: dm, divy, linux-kbuild, netdev
From: "Jan Beulich" <JBeulich@novell.com>
Date: Mon, 28 Feb 2011 08:19:58 +0000
>>>> On 25.02.11 at 20:51, Dimitris Michailidis <dm@chelsio.com> wrote:
>> Dimitris Michailidis wrote:
>>> Jan Beulich wrote:
>>>
>>>> As to that INET vs NET dependency - is it possible that the
>>>> network drivers really just need NET, but the iSCSI ones need
>>>> INET? In which case the only common dependency would be
>>>> PCI - certainly not worth a custom helper option.
>>>
>>> Reading the commit message that introduced CHELSIO_T3_DEPENDS, it talks
>>> of hidden dependencies that select does not see. I am not sure which
>>> exactly but since it's been a few years since that commit I'll try to
>>> see what the situation is today without the *_DEPENDS symbols and let
>>> you know.
>>
>> I looked into this and found that with the current Kconfig the iSCSI driver
>> does not appear in the SCSI menu until one first enables NETDEVICES and
>> NETDEV_10000 in the network driver menu. It appears that the *_DEPENDS
>> symbols were added to capture dependencies on such symbols within the
>> network driver Kconfig, besides the dependencies the driver's entry listed
>> explicitly.
>>
>> The patch below removes *T4*_DEPENDS and the network drivers' unnecessary
>> dependency on INET, and updates the iSCSI driver's entry so it is visible
>> without requiring any net driver options to be enabled first and has
>> adequate selects to be able to build the net driver (this part is adapted
>> from bnx2i's Kconfig entry). I still need to do the T3 part of this and
>> check that there isn't a conflict with the current scsi tree. Just for
>> review at this time.
>
> Thanks, this looks good to me.
Dimitris, please submit this anew with a proper commit log messag and
signoff. Thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-02-28 21:18 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-17 13:29 [PATCH] cxgb{3,4}: streamline Kconfig options Jan Beulich
2011-02-22 18:14 ` David Miller
2011-02-23 9:46 ` Jan Beulich
2011-02-23 20:27 ` David Miller
2011-02-24 8:00 ` Jan Beulich
2011-02-24 19:41 ` Dimitris Michailidis
2011-02-25 19:51 ` Dimitris Michailidis
2011-02-28 8:19 ` Jan Beulich
2011-02-28 21:19 ` David Miller
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).