| Message ID | fe1664e502d28bcb6a5689787b8000e6c9288307.1484283610.git.vomlehn@texas.net |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, 2017-01-12 at 21:02 -0800, Alexander Loktionov wrote: > From: David VomLehn <vomlehn@texas.net> > > Patches to create the make and configuration files. This patch should _really_ be the last in the series not the first.
On 01/12/2017 09:06 PM, Joe Perches wrote: > On Thu, 2017-01-12 at 21:02 -0800, Alexander Loktionov wrote: >> From: David VomLehn <vomlehn@texas.net> >> >> Patches to create the make and configuration files. > This patch should _really_ be the last in the series > not the first. > Could you explain the basis for this? By convention, we put tables of content at the beginning of books and only indices at the back. Analogously, make and config files can be used to established the context for what follows, making it easier to understand. Once committed, of course, the order no longer matters except as bisection is concerned.
On Thu, 2017-01-12 at 21:24 -0800, David VomLehn wrote: > On 01/12/2017 09:06 PM, Joe Perches wrote: > > On Thu, 2017-01-12 at 21:02 -0800, Alexander Loktionov wrote: > > > From: David VomLehn <vomlehn@texas.net> > > > > > > Patches to create the make and configuration files. > > > > This patch should _really_ be the last in the series > > not the first. > > > > Could you explain the basis for this? By convention, we put tables of > content at the beginning of books and only indices at the back. > Analogously, make and config files can be used to established the > context for what follows, making it easier to understand. Once > committed, of course, the order no longer matters except as bisection is > concerned. As I wrote the first time: On Tue, 2016-12-27 at 08:15 -0800, Joe Perches wrote: > On Tue, 2016-12-27 at 05:17 -0800, David VomLehn wrote: > > Patches to create the make and configuration files. [] > Patch 1 will not build if CONFIG_AQTION is enabled. > Patch 1/12 should be reordered to be patch 12/12 and > all the other patches moved up appropriately. You don't create the files until later patches. If you applied just this first patch and tried to add CONFIG_AQTION=y to the .config, make fails. That's bad for git bisect. Every patch in this series should build properly. If you delay the adding of the Makefile and Kconfig until all the files are added, then it'd bisect fine.
On 01/12/2017 09:59 PM, Joe Perches wrote: > On Thu, 2017-01-12 at 21:24 -0800, David VomLehn wrote: >> On 01/12/2017 09:06 PM, Joe Perches wrote: >>> On Thu, 2017-01-12 at 21:02 -0800, Alexander Loktionov wrote: >>>> From: David VomLehn <vomlehn@texas.net> >>>> >>>> Patches to create the make and configuration files. >>> This patch should _really_ be the last in the series >>> not the first. >>> >> Could you explain the basis for this? By convention, we put tables of >> content at the beginning of books and only indices at the back. >> Analogously, make and config files can be used to established the >> context for what follows, making it easier to understand. Once >> committed, of course, the order no longer matters except as bisection is >> concerned. > As I wrote the first time: > > On Tue, 2016-12-27 at 08:15 -0800, Joe Perches wrote: >> On Tue, 2016-12-27 at 05:17 -0800, David VomLehn wrote: >>> Patches to create the make and configuration files. > [] >> Patch 1 will not build if CONFIG_AQTION is enabled. >> Patch 1/12 should be reordered to be patch 12/12 and >> all the other patches moved up appropriately. > You don't create the files until later patches. > > If you applied just this first patch and tried to > add CONFIG_AQTION=y to the .config, make fails. > > That's bad for git bisect. > Every patch in this series should build properly. > > If you delay the adding of the Makefile and Kconfig > until all the files are added, then it'd bisect fine. Please go back and re-read the latest patches; I think you will find your concern about CONFIG_AQTION addressed in the v5 patchset. The 01/13 patch no longer has the changes to Makefile and Kconfig in drivers/net/ethernet that will pull in the Makefile and Kconfig from drivers/net/ethernet/aquantia. Those changes are now in the 13/13 patch, which should make it bisectable. If I am missing something, please let me know.
On Thu, 2017-01-12 at 22:57 -0800, David VomLehn wrote: > On 01/12/2017 09:59 PM, Joe Perches wrote: > > On Thu, 2017-01-12 at 21:24 -0800, David VomLehn wrote: > > > On 01/12/2017 09:06 PM, Joe Perches wrote: > > > > On Thu, 2017-01-12 at 21:02 -0800, Alexander Loktionov wrote: > > > > > From: David VomLehn <vomlehn@texas.net> > > > > > > > > > > Patches to create the make and configuration files. > > > > > > > > This patch should _really_ be the last in the series > > > > not the first. > > > > > > > > > > Could you explain the basis for this? By convention, we put tables of > > > content at the beginning of books and only indices at the back. > > > Analogously, make and config files can be used to established the > > > context for what follows, making it easier to understand. Once > > > committed, of course, the order no longer matters except as bisection is > > > concerned. > > > > As I wrote the first time: > > > > On Tue, 2016-12-27 at 08:15 -0800, Joe Perches wrote: > > > On Tue, 2016-12-27 at 05:17 -0800, David VomLehn wrote: > > > > Patches to create the make and configuration files. > > > > [] > > > Patch 1 will not build if CONFIG_AQTION is enabled. > > > Patch 1/12 should be reordered to be patch 12/12 and > > > all the other patches moved up appropriately. > > > > You don't create the files until later patches. > > > > If you applied just this first patch and tried to > > add CONFIG_AQTION=y to the .config, make fails. > > > > That's bad for git bisect. > > Every patch in this series should build properly. > > > > If you delay the adding of the Makefile and Kconfig > > until all the files are added, then it'd bisect fine. > > Please go back and re-read the latest patches; I think you will find > your concern about CONFIG_AQTION addressed in the v5 patchset. The 01/13 > patch no longer has the changes to Makefile and Kconfig in > drivers/net/ethernet that will pull in the Makefile and Kconfig from > drivers/net/ethernet/aquantia. Those changes are now in the 13/13 patch, > which should make it bisectable. If I am missing something, please let > me know. Well fine then. I just looked subject titles and didn't notice that change. cheers, Joe
On 01/12/2017 09:02 PM, Alexander Loktionov wrote: > From: David VomLehn <vomlehn@texas.net> > > Patches to create the make and configuration files. > > Signed-off-by: Alexander Loktionov <Alexander.Loktionov@aquantia.com> > Signed-off-by: Dmitrii Tarakanov <Dmitrii.Tarakanov@aquantia.com> > Signed-off-by: Pavel Belous <Pavel.Belous@aquantia.com> > Signed-off-by: Dmitry Bezrukov <Dmitry.Bezrukov@aquantia.com> > Signed-off-by: David M. VomLehn <vomlehn@texas.net> > --- > drivers/net/ethernet/aquantia/Kconfig | 24 +++++++++++++++++++ > drivers/net/ethernet/aquantia/Makefile | 42 ++++++++++++++++++++++++++++++++++ > drivers/net/ethernet/aquantia/ver.h | 18 +++++++++++++++ > 3 files changed, 84 insertions(+) > create mode 100644 drivers/net/ethernet/aquantia/Kconfig > create mode 100644 drivers/net/ethernet/aquantia/Makefile > create mode 100644 drivers/net/ethernet/aquantia/ver.h > > diff --git a/drivers/net/ethernet/aquantia/Kconfig b/drivers/net/ethernet/aquantia/Kconfig > new file mode 100644 > index 0000000..a74a4c0 > --- /dev/null > +++ b/drivers/net/ethernet/aquantia/Kconfig > @@ -0,0 +1,24 @@ > +# > +# aQuantia device configuration > +# > + > +config NET_VENDOR_AQUANTIA > + bool "aQuantia devices" > + default y > + ---help--- > + Set this to y if you have an Ethernet network cards that uses the aQuantia > + chipset. Could be interesting specify which specific commercial products available. > + > + This option does not build any drivers; it casues the aQuantia > + drivers that can be built to appear in the list of Ethernet drivers. > + > + > +if NET_VENDOR_AQUANTIA > + > +config AQTION > + tristate "aQuantia AQtion Support" > + depends on PCI > + ---help--- > + This enables the support for the aQuantia AQtion Ethernet card. > + > +endif # NET_VENDOR_AQUANTIA > diff --git a/drivers/net/ethernet/aquantia/Makefile b/drivers/net/ethernet/aquantia/Makefile > new file mode 100644 > index 0000000..e4ae696 > --- /dev/null > +++ b/drivers/net/ethernet/aquantia/Makefile > @@ -0,0 +1,42 @@ > +################################################################################ > +# > +# aQuantia Ethernet Controller AQtion Linux Driver > +# Copyright(c) 2014-2017 aQuantia Corporation. > +# > +# This program is free software; you can redistribute it and/or modify it > +# under the terms and conditions of the GNU General Public License, > +# version 2, as published by the Free Software Foundation. > +# > +# This program is distributed in the hope it will be useful, but WITHOUT > +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > +# more details. > +# > +# You should have received a copy of the GNU General Public License along > +# with this program. If not, see <http://www.gnu.org/licenses/>. > +# > +# The full GNU General Public License is included in this distribution in > +# the file called "COPYING". > +# > +# Contact Information: <rdc-drv@aquantia.com> > +# aQuantia Corporation, 105 E. Tasman Dr. San Jose, CA 95134, USA > +# > +################################################################################ > + > +# > +# Makefile for the AQtion(tm) Ethernet driver > +# > + > +obj-$(CONFIG_AQTION) += atlantic.o > + > +atlantic-objs := aq_main.o \ > + aq_nic.o \ > + aq_pci_func.o \ > + aq_vec.o \ > + aq_ring.o \ > + aq_hw_utils.o \ > + aq_ethtool.o \ > + hw_atl/hw_atl_a0.o \ > + hw_atl/hw_atl_b0.o \ > + hw_atl/hw_atl_utils.o \ > + hw_atl/hw_atl_llh.o > diff --git a/drivers/net/ethernet/aquantia/ver.h b/drivers/net/ethernet/aquantia/ver.h > new file mode 100644 > index 0000000..636d646 > --- /dev/null > +++ b/drivers/net/ethernet/aquantia/ver.h > @@ -0,0 +1,18 @@ > +/* > + * aQuantia Corporation Network Driver > + * Copyright (C) 2014-2017 aQuantia Corporation. All rights reserved > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + */ > + > +#ifndef VER_H > +#define VER_H > + > +#define NIC_MAJOR_DRIVER_VERSION 1 > +#define NIC_MINOR_DRIVER_VERSION 5 > +#define NIC_BUILD_DRIVER_VERSION 339 > +#define NIC_REVISION_DRIVER_VERSION 0 That probably means nothing since it's the first time for the upstream kernel that such a driver exists. Defining versions of software is such an interesting subject anyway!
On 01/12/2017 09:02 PM, Alexander Loktionov wrote: > From: David VomLehn <vomlehn@texas.net> > > Patches to create the make and configuration files. > > Signed-off-by: Alexander Loktionov <Alexander.Loktionov@aquantia.com> > Signed-off-by: Dmitrii Tarakanov <Dmitrii.Tarakanov@aquantia.com> > Signed-off-by: Pavel Belous <Pavel.Belous@aquantia.com> > Signed-off-by: Dmitry Bezrukov <Dmitry.Bezrukov@aquantia.com> > Signed-off-by: David M. VomLehn <vomlehn@texas.net> > --- > drivers/net/ethernet/aquantia/Kconfig | 24 +++++++++++++++++++ > drivers/net/ethernet/aquantia/Makefile | 42 ++++++++++++++++++++++++++++++++++ > drivers/net/ethernet/aquantia/ver.h | 18 +++++++++++++++ > 3 files changed, 84 insertions(+) > create mode 100644 drivers/net/ethernet/aquantia/Kconfig > create mode 100644 drivers/net/ethernet/aquantia/Makefile > create mode 100644 drivers/net/ethernet/aquantia/ver.h > > diff --git a/drivers/net/ethernet/aquantia/Kconfig b/drivers/net/ethernet/aquantia/Kconfig > new file mode 100644 > index 0000000..a74a4c0 > --- /dev/null > +++ b/drivers/net/ethernet/aquantia/Kconfig > @@ -0,0 +1,24 @@ > +# > +# aQuantia device configuration > +# > + > +config NET_VENDOR_AQUANTIA > + bool "aQuantia devices" > + default y > + ---help--- > + Set this to y if you have an Ethernet network cards that uses the aQuantia > + chipset. > + > + This option does not build any drivers; it casues the aQuantia > + drivers that can be built to appear in the list of Ethernet drivers. > + > + > +if NET_VENDOR_AQUANTIA > + > +config AQTION > + tristate "aQuantia AQtion Support" > + depends on PCI > + ---help--- > + This enables the support for the aQuantia AQtion Ethernet card. > + > +endif # NET_VENDOR_AQUANTIA > diff --git a/drivers/net/ethernet/aquantia/Makefile b/drivers/net/ethernet/aquantia/Makefile > new file mode 100644 > index 0000000..e4ae696 > --- /dev/null > +++ b/drivers/net/ethernet/aquantia/Makefile > @@ -0,0 +1,42 @@ > +################################################################################ > +# > +# aQuantia Ethernet Controller AQtion Linux Driver > +# Copyright(c) 2014-2017 aQuantia Corporation. > +# > +# This program is free software; you can redistribute it and/or modify it > +# under the terms and conditions of the GNU General Public License, > +# version 2, as published by the Free Software Foundation. > +# > +# This program is distributed in the hope it will be useful, but WITHOUT > +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > +# more details. > +# > +# You should have received a copy of the GNU General Public License along > +# with this program. If not, see <http://www.gnu.org/licenses/>. > +# > +# The full GNU General Public License is included in this distribution in > +# the file called "COPYING". > +# > +# Contact Information: <rdc-drv@aquantia.com> > +# aQuantia Corporation, 105 E. Tasman Dr. San Jose, CA 95134, USA > +# > +################################################################################ > + > +# > +# Makefile for the AQtion(tm) Ethernet driver > +# > + > +obj-$(CONFIG_AQTION) += atlantic.o > + > +atlantic-objs := aq_main.o \ > + aq_nic.o \ > + aq_pci_func.o \ > + aq_vec.o \ > + aq_ring.o \ > + aq_hw_utils.o \ > + aq_ethtool.o \ > + hw_atl/hw_atl_a0.o \ > + hw_atl/hw_atl_b0.o \ > + hw_atl/hw_atl_utils.o \ > + hw_atl/hw_atl_llh.o You might want to create an aqtion or atlantic folder just in case you later on submit a driver for another aquantia NIC. That would keep the directory structure clean.
Yes, we did have it that way at one point. But...there is also the kernel philosophy of not putting in something for future expansion; you can always do it later... Honestly, I've vacillated on this particular one. On 01/14/2017 10:39 AM, Florian Fainelli wrote: > > On 01/12/2017 09:02 PM, Alexander Loktionov wrote: >> From: David VomLehn <vomlehn@texas.net> >> >> Patches to create the make and configuration files. >> >> Signed-off-by: Alexander Loktionov <Alexander.Loktionov@aquantia.com> >> Signed-off-by: Dmitrii Tarakanov <Dmitrii.Tarakanov@aquantia.com> >> Signed-off-by: Pavel Belous <Pavel.Belous@aquantia.com> >> Signed-off-by: Dmitry Bezrukov <Dmitry.Bezrukov@aquantia.com> >> Signed-off-by: David M. VomLehn <vomlehn@texas.net> >> --- >> drivers/net/ethernet/aquantia/Kconfig | 24 +++++++++++++++++++ >> drivers/net/ethernet/aquantia/Makefile | 42 ++++++++++++++++++++++++++++++++++ >> drivers/net/ethernet/aquantia/ver.h | 18 +++++++++++++++ >> 3 files changed, 84 insertions(+) >> create mode 100644 drivers/net/ethernet/aquantia/Kconfig >> create mode 100644 drivers/net/ethernet/aquantia/Makefile >> create mode 100644 drivers/net/ethernet/aquantia/ver.h >> >> diff --git a/drivers/net/ethernet/aquantia/Kconfig b/drivers/net/ethernet/aquantia/Kconfig >> new file mode 100644 >> index 0000000..a74a4c0 >> --- /dev/null >> +++ b/drivers/net/ethernet/aquantia/Kconfig >> @@ -0,0 +1,24 @@ >> +# >> +# aQuantia device configuration >> +# >> + >> +config NET_VENDOR_AQUANTIA >> + bool "aQuantia devices" >> + default y >> + ---help--- >> + Set this to y if you have an Ethernet network cards that uses the aQuantia >> + chipset. >> + >> + This option does not build any drivers; it casues the aQuantia >> + drivers that can be built to appear in the list of Ethernet drivers. >> + >> + >> +if NET_VENDOR_AQUANTIA >> + >> +config AQTION >> + tristate "aQuantia AQtion Support" >> + depends on PCI >> + ---help--- >> + This enables the support for the aQuantia AQtion Ethernet card. >> + >> +endif # NET_VENDOR_AQUANTIA >> diff --git a/drivers/net/ethernet/aquantia/Makefile b/drivers/net/ethernet/aquantia/Makefile >> new file mode 100644 >> index 0000000..e4ae696 >> --- /dev/null >> +++ b/drivers/net/ethernet/aquantia/Makefile >> @@ -0,0 +1,42 @@ >> +################################################################################ >> +# >> +# aQuantia Ethernet Controller AQtion Linux Driver >> +# Copyright(c) 2014-2017 aQuantia Corporation. >> +# >> +# This program is free software; you can redistribute it and/or modify it >> +# under the terms and conditions of the GNU General Public License, >> +# version 2, as published by the Free Software Foundation. >> +# >> +# This program is distributed in the hope it will be useful, but WITHOUT >> +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >> +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for >> +# more details. >> +# >> +# You should have received a copy of the GNU General Public License along >> +# with this program. If not, see <http://www.gnu.org/licenses/>. >> +# >> +# The full GNU General Public License is included in this distribution in >> +# the file called "COPYING". >> +# >> +# Contact Information: <rdc-drv@aquantia.com> >> +# aQuantia Corporation, 105 E. Tasman Dr. San Jose, CA 95134, USA >> +# >> +################################################################################ >> + >> +# >> +# Makefile for the AQtion(tm) Ethernet driver >> +# >> + >> +obj-$(CONFIG_AQTION) += atlantic.o >> + >> +atlantic-objs := aq_main.o \ >> + aq_nic.o \ >> + aq_pci_func.o \ >> + aq_vec.o \ >> + aq_ring.o \ >> + aq_hw_utils.o \ >> + aq_ethtool.o \ >> + hw_atl/hw_atl_a0.o \ >> + hw_atl/hw_atl_b0.o \ >> + hw_atl/hw_atl_utils.o \ >> + hw_atl/hw_atl_llh.o > You might want to create an aqtion or atlantic folder just in case you > later on submit a driver for another aquantia NIC. That would keep the > directory structure clean.
On 01/14/2017 10:42 AM, David VomLehn wrote: > Yes, we did have it that way at one point. But...there is also the > kernel philosophy of not putting in something for future expansion; you > can always do it later... Honestly, I've vacillated on this particular one. (please don't top post). There are several threads at the moment talking about renaming driver directory (synopsys/stmmac, mlx5), doing it later is certainly a possibility but is really frowned upon, since it will later on make the life of people backporting -stable changes a lot harder. Even if there is just one driver at the moment, I would go with a dedicated directory for it, there are enough object files that justify this choice IMHO.
On 01/14/2017 10:48 AM, Florian Fainelli wrote: > On 01/14/2017 10:42 AM, David VomLehn wrote: >> Yes, we did have it that way at one point. But...there is also the >> kernel philosophy of not putting in something for future expansion; you >> can always do it later... Honestly, I've vacillated on this particular one. > (please don't top post). There are several threads at the moment talking > about renaming driver directory (synopsys/stmmac, mlx5), doing it later > is certainly a possibility but is really frowned upon, since it will > later on make the life of people backporting -stable changes a lot harder. About that top-posting...I agree, but Microsoft has the entire planet doing it the other way. They find bottom posting confusing and often can't even figure out that you replied. One more reason I've hated Microsoft since 1976. So, I suspect I'm not the only one who occasionally slips up. My apologies. > > Even if there is just one driver at the moment, I would go with a > dedicated directory for it, there are enough object files that justify > this choice IMHO. Good argument.
diff --git a/drivers/net/ethernet/aquantia/Kconfig b/drivers/net/ethernet/aquantia/Kconfig new file mode 100644 index 0000000..a74a4c0 --- /dev/null +++ b/drivers/net/ethernet/aquantia/Kconfig @@ -0,0 +1,24 @@ +# +# aQuantia device configuration +# + +config NET_VENDOR_AQUANTIA + bool "aQuantia devices" + default y + ---help--- + Set this to y if you have an Ethernet network cards that uses the aQuantia + chipset. + + This option does not build any drivers; it casues the aQuantia + drivers that can be built to appear in the list of Ethernet drivers. + + +if NET_VENDOR_AQUANTIA + +config AQTION + tristate "aQuantia AQtion Support" + depends on PCI + ---help--- + This enables the support for the aQuantia AQtion Ethernet card. + +endif # NET_VENDOR_AQUANTIA diff --git a/drivers/net/ethernet/aquantia/Makefile b/drivers/net/ethernet/aquantia/Makefile new file mode 100644 index 0000000..e4ae696 --- /dev/null +++ b/drivers/net/ethernet/aquantia/Makefile @@ -0,0 +1,42 @@ +################################################################################ +# +# aQuantia Ethernet Controller AQtion Linux Driver +# Copyright(c) 2014-2017 aQuantia Corporation. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms and conditions of the GNU General Public License, +# version 2, as published by the Free Software Foundation. +# +# This program is distributed in the hope it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program. If not, see <http://www.gnu.org/licenses/>. +# +# The full GNU General Public License is included in this distribution in +# the file called "COPYING". +# +# Contact Information: <rdc-drv@aquantia.com> +# aQuantia Corporation, 105 E. Tasman Dr. San Jose, CA 95134, USA +# +################################################################################ + +# +# Makefile for the AQtion(tm) Ethernet driver +# + +obj-$(CONFIG_AQTION) += atlantic.o + +atlantic-objs := aq_main.o \ + aq_nic.o \ + aq_pci_func.o \ + aq_vec.o \ + aq_ring.o \ + aq_hw_utils.o \ + aq_ethtool.o \ + hw_atl/hw_atl_a0.o \ + hw_atl/hw_atl_b0.o \ + hw_atl/hw_atl_utils.o \ + hw_atl/hw_atl_llh.o diff --git a/drivers/net/ethernet/aquantia/ver.h b/drivers/net/ethernet/aquantia/ver.h new file mode 100644 index 0000000..636d646 --- /dev/null +++ b/drivers/net/ethernet/aquantia/ver.h @@ -0,0 +1,18 @@ +/* + * aQuantia Corporation Network Driver + * Copyright (C) 2014-2017 aQuantia Corporation. All rights reserved + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + */ + +#ifndef VER_H +#define VER_H + +#define NIC_MAJOR_DRIVER_VERSION 1 +#define NIC_MINOR_DRIVER_VERSION 5 +#define NIC_BUILD_DRIVER_VERSION 339 +#define NIC_REVISION_DRIVER_VERSION 0 + +#endif /* VER_H */