2.6.16, sk98lin out of date

2.6.16, sk98lin out of date

[Alistair John Strachan]

Hi,

As I'm sure the drivers/net maintainers are well aware, SysKonnect constantly 
update their sk98lin/sky2 driver without bothering to sync their changes with 
the official linux kernel.

I quickly downloaded driver version 8.31 from http://www.skd.de/ today and 
used the install script to generate a diff against 2.6.16-rc3. Even after 
fixing up some DOS EOLs in the package, the diff was still well over 1.5MBs. 
This is an enormous number of changes.

The reason I'm posting is that my DFI LanParty-UT SLI-D works with this 
version of the driver, but not the version 2.6.16-rc3.

[alistair] 10:55 [~] dmesg | grep sk98lin
sk98lin: Could not read VPD data.
sk98lin: probe of 0000:01:0a.0 failed with error -5

I wonder if it's worth just identifying the "quick fix" (I've seen workarounds 
for corrupt VPDs like mine) or whether a general merge up would be 
beneficial..

If nobody's maintaining this driver I wouldn't mind helping out.

-- 
Cheers,
Alistair.

'No sense being pessimistic, it probably wouldn't work anyway.'
Third year Computer Science undergraduate.
1F2 55 South Clerk Street, Edinburgh, UK.

Re: 2.6.16, sk98lin out of date

[Matti Aarnio]

On Mon, Feb 13, 2006 at 10:58:03AM +0000, Alistair John Strachan wrote:
> From:	Alistair John Strachan <s0348365@sms.ed.ac.uk>
> To:	netdev@vger.kernel.org
> Subject: 2.6.16, sk98lin out of date
> Date:	Mon, 13 Feb 2006 10:58:03 +0000
> Cc:	linux-kernel@vger.kernel.org
> 
> Hi,
> 
> As I'm sure the drivers/net maintainers are well aware, SysKonnect constantly 
> update their sk98lin/sky2 driver without bothering to sync their changes with 
> the official linux kernel.

My understanding is, that  skge  driver has superceded the  sk98lin  in
most uses.

There could, of course, be a change to insert vendor driver _as_is_ into
baseline kernel with its own name.


> I quickly downloaded driver version 8.31 from http://www.skd.de/ today and 
> used the install script to generate a diff against 2.6.16-rc3. Even after 
> fixing up some DOS EOLs in the package, the diff was still well over 1.5MBs. 
> This is an enormous number of changes.
> 
> The reason I'm posting is that my DFI LanParty-UT SLI-D works with this 
> version of the driver, but not the version 2.6.16-rc3.
> 
> [alistair] 10:55 [~] dmesg | grep sk98lin
> sk98lin: Could not read VPD data.
> sk98lin: probe of 0000:01:0a.0 failed with error -5
> 
> I wonder if it's worth just identifying the "quick fix" (I've seen
> workarounds for corrupt VPDs like mine) or whether a general merge
> up would be beneficial..

See if  'skge'  driver would work instead of  'sk98lin'  ?

> If nobody's maintaining this driver I wouldn't mind helping out.
> 
> -- 
> Cheers,
> Alistair.

  /Matti Aarnio

Re: 2.6.16, sk98lin out of date

[Mws]

[-- Attachment #1.1: Type: text/plain, Size: 1631 bytes --]

On Monday 13 February 2006 11:58, you wrote:
> Hi,
> 
> As I'm sure the drivers/net maintainers are well aware, SysKonnect constantly 
> update their sk98lin/sky2 driver without bothering to sync their changes with 
> the official linux kernel.
> 
> I quickly downloaded driver version 8.31 from http://www.skd.de/ today and 
> used the install script to generate a diff against 2.6.16-rc3. Even after 
> fixing up some DOS EOLs in the package, the diff was still well over 1.5MBs. 
> This is an enormous number of changes.
> 
> The reason I'm posting is that my DFI LanParty-UT SLI-D works with this 
> version of the driver, but not the version 2.6.16-rc3.
> 
> [alistair] 10:55 [~] dmesg | grep sk98lin
> sk98lin: Could not read VPD data.
> sk98lin: probe of 0000:01:0a.0 failed with error -5
> 
> I wonder if it's worth just identifying the "quick fix" (I've seen workarounds 
> for corrupt VPDs like mine) or whether a general merge up would be 
> beneficial..
> 
> If nobody's maintaining this driver I wouldn't mind helping out.
> 
hi,
as i do have the same problem i may help you out.

at first, syskonnect did send their kernel diffs/patches but they we're rejected caused
by coding style, indention and some people thinking that things can be done better.

i personally do apply these drivers with the generate patch function of the install.sh.

there were small modifications within kernel 2.6.16 with i also sended as a diff to syskonnect, 
but got not reply.

i attached the diff, maybe it helps ya out. it works with my 2 onboard marvell yukon gbit adapters.

regards
marcel


[-- Attachment #1.2: sk98lin_v8.28.1.3_adjustments_for_2.6.16.patch --]
[-- Type: text/x-diff, Size: 3376 bytes --]

diff -ruN linux/drivers/net/sk98lin/h/skdrv2nd.h linux-new/drivers/net/sk98lin/h/skdrv2nd.h
--- linux/drivers/net/sk98lin/h/skdrv2nd.h	2006-01-18 10:11:04.000000000 +0100
+++ linux-new/drivers/net/sk98lin/h/skdrv2nd.h	2005-09-29 10:26:12.000000000 +0200
@@ -61,7 +61,7 @@
 #ifdef HAVE_POLL_CONTROLLER
 #define SK_POLL_CONTROLLER
 #define CONFIG_SK98LIN_NAPI
+#elif defined CONFIG_NET_POLL_CONTROLLER
-#elif CONFIG_NET_POLL_CONTROLLER
 #define SK_POLL_CONTROLLER
 #define CONFIG_SK98LIN_NAPI
 #endif
diff -ruN linux/drivers/net/sk98lin/skge.c linux-new/drivers/net/sk98lin/skge.c
--- linux/drivers/net/sk98lin/skge.c	2006-01-18 10:22:44.000000000 +0100
+++ linux-new/drivers/net/sk98lin/skge.c	2005-09-29 10:26:12.000000000 +0200
@@ -107,7 +107,7 @@
 static int 	__devinit sk98lin_init_device(struct pci_dev *pdev, const struct pci_device_id *ent);
 static void 	sk98lin_remove_device(struct pci_dev *pdev);
 #ifdef CONFIG_PM
+static int	sk98lin_suspend(struct pci_dev *pdev, pm_message_t state);
-static int	sk98lin_suspend(struct pci_dev *pdev, u32 state);
 static int	sk98lin_resume(struct pci_dev *pdev);
 static void	SkEnableWOMagicPacket(SK_AC *pAC, SK_IOC IoC, SK_MAC_ADDR MacAddr);
 #endif
@@ -971,7 +971,7 @@
  */
 static int sk98lin_suspend(
 struct pci_dev	*pdev,   /* pointer to the device that is to suspend */
+pm_message_t	state)  /* what power state is desired by Linux?    */
-u32		state)  /* what power state is desired by Linux?    */
 {
 	struct net_device   *dev  = pci_get_drvdata(pdev);
 	DEV_NET		    *pNet = (DEV_NET*) dev->priv;
@@ -1024,7 +1024,7 @@
 #else
 	pci_save_state(pdev, pAC->PciState);
 #endif
+	pci_set_power_state(pdev, pci_choose_state(pdev, state)); /* set the state */
-	pci_set_power_state(pdev, state); /* set the state */
 
 	return 0;
 }
diff -ruN linux/drivers/net/sk98lin/sky2.c linux-new/drivers/net/sk98lin/sky2.c
--- linux/drivers/net/sk98lin/sky2.c	2006-01-18 10:51:36.000000000 +0100
+++ linux-new/drivers/net/sk98lin/sky2.c	2005-09-29 10:26:12.000000000 +0200
@@ -33,7 +33,7 @@
 #include "h/skdrv1st.h"
 #include "h/skdrv2nd.h"
 #include <linux/tcp.h>
+#include <linux/ip.h>
-
 /******************************************************************************
  *
  * Local Function Prototypes
@@ -337,7 +337,7 @@
 			PUSH_PKT_AS_LAST_IN_QUEUE(&pAC->TxPort[Port][0].TxQ_free, pSkPacket);
 			POP_FIRST_PKT_FROM_QUEUE(&pAC->TxPort[Port][0].TxAQ_working, pSkPacket);
 		}
+#ifdef USE_SYNC_TX_QUEUE
-#if USE_SYNC_TX_QUEUE
 		POP_FIRST_PKT_FROM_QUEUE(&pAC->TxPort[Port][0].TxSQ_working, pSkPacket);
 		while (pSkPacket != NULL) {
 			if ((pSkFrag = pSkPacket->pFrag) != NULL) {
@@ -902,7 +902,7 @@
 	pAC->TxPort[Port][TX_PRIO_LOW].TxAQ_waiting.pTail = NULL;
 	spin_lock_init(&pAC->TxPort[Port][TX_PRIO_LOW].TxAQ_waiting.QueueLock);
 	
+#ifdef USE_SYNC_TX_QUEUE
-#if USE_SYNC_TX_QUEUE
 	pAC->TxPort[Port][TX_PRIO_LOW].TxSQ_working.pHead = NULL;
 	pAC->TxPort[Port][TX_PRIO_LOW].TxSQ_working.pTail = NULL;
 	spin_lock_init(&pAC->TxPort[Port][TX_PRIO_LOW].TxSQ_working.QueueLock);
@@ -2311,7 +2311,7 @@
 					SK_DBG_MSG(pAC, SK_DBGMOD_DRV, SK_DBGCAT_DRV_INT_SRC,
 						("No changes for TxA%d\n", Port + 1));
 				}
+#ifdef USE_SYNC_TX_QUEUE
-#if USE_SYNC_TX_QUEUE
 				if (HW_SYNC_TX_SUPPORTED(pAC)) {
 					/* 
 					** Do we have a new Done idx ? 

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: 2.6.16, sk98lin out of date

[Alistair John Strachan]

On Monday 13 February 2006 11:05, Matti Aarnio wrote:
> On Mon, Feb 13, 2006 at 10:58:03AM +0000, Alistair John Strachan wrote:
> > Hi,
> >
> > As I'm sure the drivers/net maintainers are well aware, SysKonnect
> > constantly update their sk98lin/sky2 driver without bothering to sync
> > their changes with the official linux kernel.
>
> My understanding is, that  skge  driver has superceded the  sk98lin  in
> most uses.
>
> There could, of course, be a change to insert vendor driver _as_is_ into
> baseline kernel with its own name.

Thanks Matti, I wasn't even aware of this driver. Might I suggest the "old" 
driver be marked as such in Linux 2.6.16. I guess I must've skipped over it 
because it begins with "New", and does not contain the word "Marvell", which 
is indicated exclusively by lspci.

-- 
Cheers,
Alistair.

'No sense being pessimistic, it probably wouldn't work anyway.'
Third year Computer Science undergraduate.
1F2 55 South Clerk Street, Edinburgh, UK.

Re: 2.6.16, sk98lin out of date

[Lee Revell]

On Mon, 2006-02-13 at 12:06 +0100, Mws wrote:
> hi,
> as i do have the same problem i may help you out.
> 
> at first, syskonnect did send their kernel diffs/patches but they
> we're rejected caused
> by coding style, indention and some people thinking that things can be
> done better. 

Haha, they didn't like the LKML code review so they just stopped sending
patches?  Classic.  Remind me not to buy their gear.

Lee

Re: 2.6.16, sk98lin out of date

[Daniel Drake]

Alistair John Strachan wrote:
> Thanks Matti, I wasn't even aware of this driver. Might I suggest the
> "old" driver be marked as such in Linux 2.6.16. I guess I must've
> skipped over it because it begins with "New", and does not contain
> the word "Marvell", which is indicated exclusively by lspci.

I changed the help text of all 3 drivers (sk98lin/skge/sky2) to
point out which ones are/aren't interchangable in 2.6.16. The situation 
is a little confusing.

The reason that the sk98lin diff is so huge is because SysKonnect
effectively added support for a substantially different range of cards
(Yukon-2) into the existing driver. This is far from the driver quality
required for the kernel today, so Stephen Hemminger (skge author) wrote 
a new driver (sky2) for the Yukon-2 range.

The long term plan is to obsolete and remove sk98lin, but we aren't 
ready yet: skge issues pop up every month or two, and sky2 is young. 
Stephen's own words:

> I applaud the initiative, but this it is too premature to obsolete
> the existing driver. There may be lots of chip versions and other
> variables that make the existing driver a better choice.

Daniel

Re: 2.6.16, sk98lin out of date

[Willy Tarreau]

On Mon, Feb 13, 2006 at 02:03:14PM -0500, Lee Revell wrote:
> On Mon, 2006-02-13 at 12:06 +0100, Mws wrote:
> > hi,
> > as i do have the same problem i may help you out.
> > 
> > at first, syskonnect did send their kernel diffs/patches but they
> > we're rejected caused
> > by coding style, indention and some people thinking that things can be
> > done better. 
> 
> Haha, they didn't like the LKML code review so they just stopped sending
> patches?  Classic.  Remind me not to buy their gear.

Lee, it's not always that simple. When you submit one driver, sometimes
reviewers tell you that for whatever reason your driver's structure is
wrong and it has to be changed a lot (and sometimes they're right of
course). But when you don't have enough ressource to do the job twice,
the best you can do is to maintain it out of tree, which is already a
pain. I'm not saying that it is what happened with their driver, I don't
know the history. However, I found your reaction somewhat hasty. I
personally would prefer to offer time and help before deciding that
I don't want anyone's products on this basis. It's not as if they
did not release their driver's source !

Cheers,
Willy

Re: 2.6.16, sk98lin out of date

[Lee Revell]

On Mon, 2006-02-13 at 21:34 +0100, Willy Tarreau wrote:
> On Mon, Feb 13, 2006 at 02:03:14PM -0500, Lee Revell wrote:
> > On Mon, 2006-02-13 at 12:06 +0100, Mws wrote:
> > > hi,
> > > as i do have the same problem i may help you out.
> > > 
> > > at first, syskonnect did send their kernel diffs/patches but they
> > > we're rejected caused
> > > by coding style, indention and some people thinking that things can be
> > > done better. 
> > 
> > Haha, they didn't like the LKML code review so they just stopped sending
> > patches?  Classic.  Remind me not to buy their gear.
> 
> Lee, it's not always that simple. When you submit one driver, sometimes
> reviewers tell you that for whatever reason your driver's structure is
> wrong and it has to be changed a lot (and sometimes they're right of
> course). But when you don't have enough ressource to do the job twice,
> the best you can do is to maintain it out of tree, which is already a
> pain. I'm not saying that it is what happened with their driver, I don't
> know the history. However, I found your reaction somewhat hasty. I
> personally would prefer to offer time and help before deciding that
> I don't want anyone's products on this basis. It's not as if they
> did not release their driver's source!

True, that was a little harsh.  I just find it a little sad that all
these vendors insist on throwing away months of work rather than simply
researching what the linux kernel coding standards are ahead of time.

Lee

Re: 2.6.16, sk98lin out of date

[Alistair John Strachan]

On Monday 13 February 2006 19:48, Daniel Drake wrote:
> Alistair John Strachan wrote:
> > Thanks Matti, I wasn't even aware of this driver. Might I suggest the
> > "old" driver be marked as such in Linux 2.6.16. I guess I must've
> > skipped over it because it begins with "New", and does not contain
> > the word "Marvell", which is indicated exclusively by lspci.
>
> I changed the help text of all 3 drivers (sk98lin/skge/sky2) to
> point out which ones are/aren't interchangable in 2.6.16. The situation
> is a little confusing.

Thanks, this would have prevented my mistake.

> The reason that the sk98lin diff is so huge is because SysKonnect
> effectively added support for a substantially different range of cards
> (Yukon-2) into the existing driver. This is far from the driver quality
> required for the kernel today, so Stephen Hemminger (skge author) wrote
> a new driver (sky2) for the Yukon-2 range.
>
> The long term plan is to obsolete and remove sk98lin, but we aren't
> ready yet: skge issues pop up every month or two, and sky2 is young.
>
> Stephen's own words:
> > I applaud the initiative, but this it is too premature to obsolete
> > the existing driver. There may be lots of chip versions and other
> > variables that make the existing driver a better choice.

Well, Stephen's driver works great for me and sk98lin frankly didn't.

-- 
Cheers,
Alistair.

'No sense being pessimistic, it probably wouldn't work anyway.'
Third year Computer Science undergraduate.
1F2 55 South Clerk Street, Edinburgh, UK.