(4/25) sk98: change #define to typedef
Subsystems (2)
Quoted 3 lines
I don't think the driver is complex enough for all that os-common mess. If you look at the BSD sk driver it's about a third of the size of the Linux driver because it doesn't have all this mess.
Note that the kernel BSD sk driver was _not_ written by SysKonnect and only supports Genesis and Yukon1; linux supports Genesis (single and dual), Yukon1, Yukon Plus, Yukon EC, Yukon FE and Yukon2 (single and dual). The BSD sk driver supports substantially less functionality than the Linux driver. For example, there is no link failover capability in the sk driver; it has no power management; numerous chip changes and minor tweaks have not been added, since many problems have become apparent through our other supported platforms (some 15 operating systems) and have since been fixed in our common modules; sk has no wake-on-LAN and so on. The official SysKonnect BSD driver (yk) also uses the common modules and is not open source.
However, the real point is that the Linux driver (as indeed _all_ of our drivers) shares common code with all our supported platforms and this necessarily introduces some overhead. But it isn't excessive, and the benefits _far_ outweigh the drawbacks, namely, that we have a very wide range of systems sharing common code and thus the probability of finding and fixing bugs is very substantially higher than if we wrote a dedicated driver for each platform.
For instance the symbol SK_IOC mentioned in your mail:
Review
Quoted 2 lines
(quoted diff) +1
-1-#define SK_IOC char __iomem * +typedef void __iomem *SK_IOC;
is used in a large number of OS independent driver files (e.g. skvpd.c). The -for instance- file skvpd.c used by our Linux driver is used at the same time without any changes by all our other drivers running on other operating systems (e.g. Solaris, Windows).
This is the reason why we need to redefine this symbol to the Linux specific implementation (void __iomem).
Mirko
Quoted 34 lines · On Tue, 16 Nov 2004 15:56:15 +0000 Mirko Lindner <[email protected]> wrote:
Quoted 3 lines
I don't think the driver is complex enough for all that os-common mess. If you look at the BSD sk driver it's about a third of the size of the Linux driver because it doesn't have all this mess.
Note that the kernel BSD sk driver was _not_ written by SysKonnect and only supports Genesis and Yukon1; linux supports Genesis (single and dual), Yukon1, Yukon Plus, Yukon EC, Yukon FE and Yukon2 (single and dual). The BSD sk driver supports substantially less functionality than the Linux driver. For example, there is no link failover capability in the sk driver; it has no power management; numerous chip changes and minor tweaks have not been added, since many problems have become apparent through our other supported platforms (some 15 operating systems) and have since been fixed in our common modules; sk has no wake-on-LAN and so on. The official SysKonnect BSD driver (yk) also uses the common modules and is not open source.
However, the real point is that the Linux driver (as indeed _all_ of our drivers) shares common code with all our supported platforms and this necessarily introduces some overhead. But it isn't excessive, and the benefits _far_ outweigh the drawbacks, namely, that we have a very wide range of systems sharing common code and thus the probability of finding and fixing bugs is very substantially higher than if we wrote a dedicated driver for each platform.
For instance the symbol
SK_IOCmentioned in your mail:Review
Quoted 2 lines
(quoted diff) +1
-1-#define SK_IOC char __iomem * +typedef void __iomem *SK_IOC;is used in a large number of OS independent driver files (e.g.
skvpd.c). The -for instance- fileskvpd.cused by our Linux driver is used at the same time without any changes by all our other drivers running on other operating systems (e.g. Solaris, Windows).
Quoted 2 lines
This is the reason why we need to redefine this symbol to the Linux specific implementation (void
__iomem).
I understand the benefits of a common driver, but there are costs too. First, it means the community can't really support or fix the common code only SysKonnect can. Also, there is functionality in the common part that really isn't necessary:
* Vendor specific MIB support -- what tool could/does use that?
* Board specific bridging support
If the regular Linux infrastructure was used for these, then the
driver would get additional kernel bridge filtering, common management and security fixes.
My wish is that the common hardware layer could be used without dragging in all the MIB and bridging stuff.
Quoted 4 lines
Note that the kernel BSD sk driver was _not_ written by SysKonnect and only supports Genesis and Yukon1; linux supports Genesis (single and dual), Yukon1, Yukon Plus, Yukon EC, Yukon FE and Yukon2 (single and dual).
They have Yukon2 support for while - unlike Linux.
Quoted 3 lines
The BSD sk driver supports substantially less functionality than the Linux driver. For example, there is no link failover capability in the sk driver;
Which isn't something that belongs into a driver anyway. It also means you common code is obsfucated enough that no one noticed :)
Quoted 10 lines
For instance the symbol
SK_IOCmentioned in your mail:Review
Quoted 2 lines
(quoted diff) +1
-1-#define SK_IOC char __iomem * +typedef void __iomem *SK_IOC;is used in a large number of OS independent driver files (e.g.
skvpd.c). The -for instance- fileskvpd.cused by our Linux driver is used at the same time without any changes by all our other drivers running on other operating systems (e.g. Solaris, Windows).
Quoted 2 lines
This is the reason why we need to redefine this symbol to the Linux specific implementation (void
__iomem).
And you still haven't explained why your common code uses such a broken structure instead of having some semi-opaque private data passed around everywhere.
Also note that we don't new drivers using such horrible "common code" anymore, you can be happy you sneaked it in at all a few years ago.
Quoted 10 lines · On Wed, 17 Nov 2004 15:01:34 +0000 Christoph Hellwig <[email protected]> wrote:
Quoted 4 lines
Note that the kernel BSD sk driver was _not_ written by SysKonnect and only supports Genesis and Yukon1; linux supports Genesis (single and dual), Yukon1, Yukon Plus, Yukon EC, Yukon FE and Yukon2 (single and dual).
They have Yukon2 support for while - unlike Linux.
Quoted 3 lines
The BSD sk driver supports substantially less functionality than the Linux driver. For example, there is no link failover capability in the sk driver;
Is it even possible to do link failover with existing Open Source tools?
Also the whole Remote Link Mangement (RLMT) stuff has no security so if it is used over a hostile network look out.
I intrigued by the idea of of porting FreeBSD driver to Linux. But this card is already taking more time than I want to spend.