Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Jay Cliburn wrote: Jeff Garzik wrote: As a driver maintainer, you need to patch sets, and submit them in a timely fashion to me. Note I said patch set, not patch, in following with Rule #3 from Documentation/SubmittingPatches. Also make sure to review http://linux.yyz.us/patch-format.html Understood. Both references reviewed. Thanks. Sorry, but one last question... These two patches generated overnight by Andrew: Message-Id: <[EMAIL PROTECTED]> Subject: + git-netdev-all-atl1-pm-fix.patch added to -mm tree and Message-Id: <[EMAIL PROTECTED]> Subject: + git-netdev-all-atl1-build-fix.patch added to -mm tree Do I include these in my patch set that I submit to you, or do you apply them to netdev directly? There is no hard and fast rule. If the patch is obvious and submitted in correct form (Andrew's notifications are not in such a form), then I might go ahead and apply it. But I usually reply to the patch with "applied" if so. In general, you can answer this question yourself. Look at netdev-2.6.git#atl1 and see what's in there. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Jeff Garzik wrote: As a driver maintainer, you need to patch sets, and submit them in a timely fashion to me. Note I said patch set, not patch, in following with Rule #3 from Documentation/SubmittingPatches. Also make sure to review http://linux.yyz.us/patch-format.html Understood. Both references reviewed. Thanks. Sorry, but one last question... These two patches generated overnight by Andrew: Message-Id: <[EMAIL PROTECTED]> Subject: + git-netdev-all-atl1-pm-fix.patch added to -mm tree and Message-Id: <[EMAIL PROTECTED]> Subject: + git-netdev-all-atl1-build-fix.patch added to -mm tree Do I include these in my patch set that I submit to you, or do you apply them to netdev directly? Jay - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Jay Cliburn wrote: Jeff, shall I add this to the larger patch I'm working on for submittal later this weekend, or do you just add it directly to netdev? (I prefer to do the former if it's okay with you.) As a driver maintainer, you need to patch sets, and submit them in a timely fashion to me. Note I said patch set, not patch, in following with Rule #3 from Documentation/SubmittingPatches. Also make sure to review http://linux.yyz.us/patch-format.html Jeff - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Luca Tettamanti wrote:
[snip]
Anyway...
Unconditionally enable MSI in atl1 driver. Also remove some useless
#ifdef since pci_{en,dis}able_msi() are no-op when MSI support is not
configured in.
Signed-off-by: Luca Tettamanti <[EMAIL PROTECTED]>
Acked-by: Jay Cliburn <[EMAIL PROTECTED]>
I tested this patch today. Works fine on my Asus M2V mainboard (Via
K8T890).
Here are the eth0 entries in /proc/interrupts before and after the patch.
BEFORE:36: 93 322091 IO-APIC-fasteoi eth0
AFTER: 2298: 85 7289 PCI-MSI-edge eth0
Jeff, shall I add this to the larger patch I'm working on for submittal
later this weekend, or do you just add it directly to netdev? (I prefer
to do the former if it's okay with you.)
Jay
---
Patch against current netdev tree.
drivers/net/atl1/atl1.h |1 -
drivers/net/atl1/atl1_main.c | 22 +++---
drivers/net/atl1/atl1_param.c | 13 -
3 files changed, 7 insertions(+), 29 deletions(-)
diff --git a/drivers/net/atl1/atl1.h b/drivers/net/atl1/atl1.h
index 1d13e8f..0b30e1c 100644
--- a/drivers/net/atl1/atl1.h
+++ b/drivers/net/atl1/atl1.h
@@ -281,7 +281,6 @@ struct atl1_adapter {
struct atl1_smb smb;
struct atl1_cmb cmb;
- int enable_msi;
u32 pci_state[16];
};
diff --git a/drivers/net/atl1/atl1_main.c b/drivers/net/atl1/atl1_main.c
index c0b1555..68e6cd1 100644
--- a/drivers/net/atl1/atl1_main.c
+++ b/drivers/net/atl1/atl1_main.c
@@ -1793,18 +1793,12 @@ s32 atl1_up(struct atl1_adapter *adapter)
goto err_up;
}
-#ifdef CONFIG_PCI_MSI
- if (adapter->enable_msi) {
- err = pci_enable_msi(adapter->pdev);
- if (err) {
- dev_info(>pdev->dev, "Unable to enable MSI:
%d\n", err);
- adapter->enable_msi = 0;
- }
- }
-#endif
- if (!adapter->enable_msi)
+ err = pci_enable_msi(adapter->pdev);
+ if (err) {
+ dev_info(>pdev->dev, "Unable to enable MSI: %d\n",
err);
irq_flags |= IRQF_SHARED;
-
+ }
+
err = request_irq(adapter->pdev->irq, _intr, irq_flags,
netdev->name, netdev);
if (unlikely(err))
@@ -1821,6 +1815,7 @@ s32 atl1_up(struct atl1_adapter *adapter)
free_irq(adapter->pdev->irq, netdev);
err_up:
+ pci_disable_msi(adapter->pdev);
/* free rx_buffers */
atl1_clean_rx_ring(adapter);
return err;
@@ -1836,10 +1831,7 @@ void atl1_down(struct atl1_adapter *adapter)
atl1_irq_disable(adapter);
free_irq(adapter->pdev->irq, netdev);
-#ifdef CONFIG_PCI_MSI
- if (adapter->enable_msi)
- pci_disable_msi(adapter->pdev);
-#endif
+ pci_disable_msi(adapter->pdev);
atl1_reset_hw(>hw);
adapter->cmb.cmb->int_stats = 0;
diff --git a/drivers/net/atl1/atl1_param.c b/drivers/net/atl1/atl1_param.c
index 4732f43..caa2d83 100644
--- a/drivers/net/atl1/atl1_param.c
+++ b/drivers/net/atl1/atl1_param.c
@@ -68,10 +68,6 @@ static int num_flash_vendor = 0;
module_param_array_named(flash_vendor, flash_vendor, int, _flash_vendor,
0);
MODULE_PARM_DESC(flash_vendor, "SPI flash vendor");
-int enable_msi;
-module_param(enable_msi, int, 0444);
-MODULE_PARM_DESC(enable_msi, "Enable PCI MSI");
-
#define DEFAULT_INT_MOD_CNT100 /* 200us */
#define MAX_INT_MOD_CNT65000
#define MIN_INT_MOD_CNT50
@@ -211,13 +207,4 @@ void __devinit atl1_check_options(struct atl1_adapter
*adapter)
adapter->hw.flash_vendor = (u8) (opt.def);
}
}
-
- { /* PCI MSI usage */
-
-#ifdef CONFIG_PCI_MSI
- adapter->enable_msi = enable_msi;
-#else
- adapter->enable_msi = 0;
-#endif
- }
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Luca Tettamanti wrote:
[snip]
Anyway...
Unconditionally enable MSI in atl1 driver. Also remove some useless
#ifdef since pci_{en,dis}able_msi() are no-op when MSI support is not
configured in.
Signed-off-by: Luca Tettamanti [EMAIL PROTECTED]
Acked-by: Jay Cliburn [EMAIL PROTECTED]
I tested this patch today. Works fine on my Asus M2V mainboard (Via
K8T890).
Here are the eth0 entries in /proc/interrupts before and after the patch.
BEFORE:36: 93 322091 IO-APIC-fasteoi eth0
AFTER: 2298: 85 7289 PCI-MSI-edge eth0
Jeff, shall I add this to the larger patch I'm working on for submittal
later this weekend, or do you just add it directly to netdev? (I prefer
to do the former if it's okay with you.)
Jay
---
Patch against current netdev tree.
drivers/net/atl1/atl1.h |1 -
drivers/net/atl1/atl1_main.c | 22 +++---
drivers/net/atl1/atl1_param.c | 13 -
3 files changed, 7 insertions(+), 29 deletions(-)
diff --git a/drivers/net/atl1/atl1.h b/drivers/net/atl1/atl1.h
index 1d13e8f..0b30e1c 100644
--- a/drivers/net/atl1/atl1.h
+++ b/drivers/net/atl1/atl1.h
@@ -281,7 +281,6 @@ struct atl1_adapter {
struct atl1_smb smb;
struct atl1_cmb cmb;
- int enable_msi;
u32 pci_state[16];
};
diff --git a/drivers/net/atl1/atl1_main.c b/drivers/net/atl1/atl1_main.c
index c0b1555..68e6cd1 100644
--- a/drivers/net/atl1/atl1_main.c
+++ b/drivers/net/atl1/atl1_main.c
@@ -1793,18 +1793,12 @@ s32 atl1_up(struct atl1_adapter *adapter)
goto err_up;
}
-#ifdef CONFIG_PCI_MSI
- if (adapter-enable_msi) {
- err = pci_enable_msi(adapter-pdev);
- if (err) {
- dev_info(adapter-pdev-dev, Unable to enable MSI:
%d\n, err);
- adapter-enable_msi = 0;
- }
- }
-#endif
- if (!adapter-enable_msi)
+ err = pci_enable_msi(adapter-pdev);
+ if (err) {
+ dev_info(adapter-pdev-dev, Unable to enable MSI: %d\n,
err);
irq_flags |= IRQF_SHARED;
-
+ }
+
err = request_irq(adapter-pdev-irq, atl1_intr, irq_flags,
netdev-name, netdev);
if (unlikely(err))
@@ -1821,6 +1815,7 @@ s32 atl1_up(struct atl1_adapter *adapter)
free_irq(adapter-pdev-irq, netdev);
err_up:
+ pci_disable_msi(adapter-pdev);
/* free rx_buffers */
atl1_clean_rx_ring(adapter);
return err;
@@ -1836,10 +1831,7 @@ void atl1_down(struct atl1_adapter *adapter)
atl1_irq_disable(adapter);
free_irq(adapter-pdev-irq, netdev);
-#ifdef CONFIG_PCI_MSI
- if (adapter-enable_msi)
- pci_disable_msi(adapter-pdev);
-#endif
+ pci_disable_msi(adapter-pdev);
atl1_reset_hw(adapter-hw);
adapter-cmb.cmb-int_stats = 0;
diff --git a/drivers/net/atl1/atl1_param.c b/drivers/net/atl1/atl1_param.c
index 4732f43..caa2d83 100644
--- a/drivers/net/atl1/atl1_param.c
+++ b/drivers/net/atl1/atl1_param.c
@@ -68,10 +68,6 @@ static int num_flash_vendor = 0;
module_param_array_named(flash_vendor, flash_vendor, int, num_flash_vendor,
0);
MODULE_PARM_DESC(flash_vendor, SPI flash vendor);
-int enable_msi;
-module_param(enable_msi, int, 0444);
-MODULE_PARM_DESC(enable_msi, Enable PCI MSI);
-
#define DEFAULT_INT_MOD_CNT100 /* 200us */
#define MAX_INT_MOD_CNT65000
#define MIN_INT_MOD_CNT50
@@ -211,13 +207,4 @@ void __devinit atl1_check_options(struct atl1_adapter
*adapter)
adapter-hw.flash_vendor = (u8) (opt.def);
}
}
-
- { /* PCI MSI usage */
-
-#ifdef CONFIG_PCI_MSI
- adapter-enable_msi = enable_msi;
-#else
- adapter-enable_msi = 0;
-#endif
- }
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Jay Cliburn wrote: Jeff, shall I add this to the larger patch I'm working on for submittal later this weekend, or do you just add it directly to netdev? (I prefer to do the former if it's okay with you.) As a driver maintainer, you need to patch sets, and submit them in a timely fashion to me. Note I said patch set, not patch, in following with Rule #3 from Documentation/SubmittingPatches. Also make sure to review http://linux.yyz.us/patch-format.html Jeff - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Jeff Garzik wrote: As a driver maintainer, you need to patch sets, and submit them in a timely fashion to me. Note I said patch set, not patch, in following with Rule #3 from Documentation/SubmittingPatches. Also make sure to review http://linux.yyz.us/patch-format.html Understood. Both references reviewed. Thanks. Sorry, but one last question... These two patches generated overnight by Andrew: Message-Id: [EMAIL PROTECTED] Subject: + git-netdev-all-atl1-pm-fix.patch added to -mm tree and Message-Id: [EMAIL PROTECTED] Subject: + git-netdev-all-atl1-build-fix.patch added to -mm tree Do I include these in my patch set that I submit to you, or do you apply them to netdev directly? Jay - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Jay Cliburn wrote: Jeff Garzik wrote: As a driver maintainer, you need to patch sets, and submit them in a timely fashion to me. Note I said patch set, not patch, in following with Rule #3 from Documentation/SubmittingPatches. Also make sure to review http://linux.yyz.us/patch-format.html Understood. Both references reviewed. Thanks. Sorry, but one last question... These two patches generated overnight by Andrew: Message-Id: [EMAIL PROTECTED] Subject: + git-netdev-all-atl1-pm-fix.patch added to -mm tree and Message-Id: [EMAIL PROTECTED] Subject: + git-netdev-all-atl1-build-fix.patch added to -mm tree Do I include these in my patch set that I submit to you, or do you apply them to netdev directly? There is no hard and fast rule. If the patch is obvious and submitted in correct form (Andrew's notifications are not in such a form), then I might go ahead and apply it. But I usually reply to the patch with applied if so. In general, you can answer this question yourself. Look at netdev-2.6.git#atl1 and see what's in there. Jeff - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Randy Dunlap wrote: Stephen Hemminger wrote: On Tue, 23 Jan 2007 16:33:29 -0500 Jeff Garzik <[EMAIL PROTECTED]> wrote: Stephen Hemminger wrote: IMHO the MSI disabling should be removed from drivers and be done in the PCI core. That is the consensus opinion. Currently drivers implement the MSI tests because the core PCI code hasn't been up to snuff. I (and others) have been discouraging that, but when a user faces a choice between working and non-working network, the pragmatic solution wins. Linus's remark (IIRC) was to not enable CONFIG_PCI_MSI then. Most distros, especially cutting edge ones like Fedora, will enable CONFIG_PCI_MSI. All efforts to get us to the point where we can remove the MSI tests from drivers are strongly supported... Jeff So far, either MSI works for all devices or is broken, so it makes sense to have a "msi=off" boot option (if there isn't already) There is one, but it's spelled "pci=nomsi". Yep. Thanks for repeating this, and refreshing the collective memory. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Stephen Hemminger wrote: On Tue, 23 Jan 2007 16:33:29 -0500 Jeff Garzik <[EMAIL PROTECTED]> wrote: Stephen Hemminger wrote: IMHO the MSI disabling should be removed from drivers and be done in the PCI core. That is the consensus opinion. Currently drivers implement the MSI tests because the core PCI code hasn't been up to snuff. I (and others) have been discouraging that, but when a user faces a choice between working and non-working network, the pragmatic solution wins. Linus's remark (IIRC) was to not enable CONFIG_PCI_MSI then. All efforts to get us to the point where we can remove the MSI tests from drivers are strongly supported... Jeff So far, either MSI works for all devices or is broken, so it makes sense to have a "msi=off" boot option (if there isn't already) There is one, but it's spelled "pci=nomsi". -- ~Randy - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
On Tue, 23 Jan 2007 16:33:29 -0500 Jeff Garzik <[EMAIL PROTECTED]> wrote: > Stephen Hemminger wrote: > > IMHO the MSI disabling should be removed from drivers and be done > > in the PCI core. > > That is the consensus opinion. > > Currently drivers implement the MSI tests because the core PCI code > hasn't been up to snuff. I (and others) have been discouraging that, > but when a user faces a choice between working and non-working network, > the pragmatic solution wins. > > All efforts to get us to the point where we can remove the MSI tests > from drivers are strongly supported... > > Jeff So far, either MSI works for all devices or is broken, so it makes sense to have a "msi=off" boot option (if there isn't already) -- Stephen Hemminger <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Stephen Hemminger wrote: IMHO the MSI disabling should be removed from drivers and be done in the PCI core. That is the consensus opinion. Currently drivers implement the MSI tests because the core PCI code hasn't been up to snuff. I (and others) have been discouraging that, but when a user faces a choice between working and non-working network, the pragmatic solution wins. All efforts to get us to the point where we can remove the MSI tests from drivers are strongly supported... Jeff - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Il Tue, Jan 23, 2007 at 11:25:22AM -0800, Stephen Hemminger ha scritto:
> On Mon, 22 Jan 2007 21:00:04 +0100
> Luca Tettamanti <[EMAIL PROTECTED]> wrote:
>
> > Il Sun, Jan 21, 2007 at 09:33:39PM -0600, Jay Cliburn ha scritto:
> > > Randy Dunlap wrote:
> > > >On Sun, 21 Jan 2007 15:07:37 -0600 Jay Cliburn wrote:
> > > [snip]
> > > >>+
> > > >>+int enable_msi;
> > > >>+module_param(enable_msi, int, 0444);
> > > >>+MODULE_PARM_DESC(enable_msi, "Enable PCI MSI");
> > > >
> > > >Hm, I thought that we didn't want individual drivers having MSI config
> > > >options...
> > >
> > > Luca? This one was yours IIRC. Care to chime in?
> >
> > I remember that discussion, but since there's no sistem-wide MSI
> > blacklist (or whitelist) I don't think it's safe to enable it
> > unconditionally. Judging from bug reports on lkml it's not safe to
> > assume that MSI support is sane on non-Intel chipsets.
> >
> > Luca
>
> There is MSI blacklisting see drivers/pci/quirks.c code.
> But the blacklist isn't complete enough yet.
>
> IMHO the MSI disabling should be removed from drivers and be done
> in the PCI core. But it doesn't seem to have gotten widespread
> support.
Does the INTx madness (like this one:
http://marc.theaimsgroup.com/?l=linux-kernel=116668921431574=2
) affect also PCI-E INTx emulation?
Anyway...
Unconditionally enable MSI in atl1 driver. Also remove some useless
#ifdef since pci_{en,dis}able_msi() are no-op when MSI support is not
configured in.
Signed-off-by: Luca Tettamanti <[EMAIL PROTECTED]>
---
Patch against current netdev tree.
drivers/net/atl1/atl1.h |1 -
drivers/net/atl1/atl1_main.c | 22 +++---
drivers/net/atl1/atl1_param.c | 13 -
3 files changed, 7 insertions(+), 29 deletions(-)
diff --git a/drivers/net/atl1/atl1.h b/drivers/net/atl1/atl1.h
index 1d13e8f..0b30e1c 100644
--- a/drivers/net/atl1/atl1.h
+++ b/drivers/net/atl1/atl1.h
@@ -281,7 +281,6 @@ struct atl1_adapter {
struct atl1_smb smb;
struct atl1_cmb cmb;
- int enable_msi;
u32 pci_state[16];
};
diff --git a/drivers/net/atl1/atl1_main.c b/drivers/net/atl1/atl1_main.c
index c0b1555..68e6cd1 100644
--- a/drivers/net/atl1/atl1_main.c
+++ b/drivers/net/atl1/atl1_main.c
@@ -1793,18 +1793,12 @@ s32 atl1_up(struct atl1_adapter *adapter)
goto err_up;
}
-#ifdef CONFIG_PCI_MSI
- if (adapter->enable_msi) {
- err = pci_enable_msi(adapter->pdev);
- if (err) {
- dev_info(>pdev->dev, "Unable to enable MSI:
%d\n", err);
- adapter->enable_msi = 0;
- }
- }
-#endif
- if (!adapter->enable_msi)
+ err = pci_enable_msi(adapter->pdev);
+ if (err) {
+ dev_info(>pdev->dev, "Unable to enable MSI: %d\n",
err);
irq_flags |= IRQF_SHARED;
-
+ }
+
err = request_irq(adapter->pdev->irq, _intr, irq_flags,
netdev->name, netdev);
if (unlikely(err))
@@ -1821,6 +1815,7 @@ s32 atl1_up(struct atl1_adapter *adapter)
free_irq(adapter->pdev->irq, netdev);
err_up:
+ pci_disable_msi(adapter->pdev);
/* free rx_buffers */
atl1_clean_rx_ring(adapter);
return err;
@@ -1836,10 +1831,7 @@ void atl1_down(struct atl1_adapter *adapter)
atl1_irq_disable(adapter);
free_irq(adapter->pdev->irq, netdev);
-#ifdef CONFIG_PCI_MSI
- if (adapter->enable_msi)
- pci_disable_msi(adapter->pdev);
-#endif
+ pci_disable_msi(adapter->pdev);
atl1_reset_hw(>hw);
adapter->cmb.cmb->int_stats = 0;
diff --git a/drivers/net/atl1/atl1_param.c b/drivers/net/atl1/atl1_param.c
index 4732f43..caa2d83 100644
--- a/drivers/net/atl1/atl1_param.c
+++ b/drivers/net/atl1/atl1_param.c
@@ -68,10 +68,6 @@ static int num_flash_vendor = 0;
module_param_array_named(flash_vendor, flash_vendor, int, _flash_vendor,
0);
MODULE_PARM_DESC(flash_vendor, "SPI flash vendor");
-int enable_msi;
-module_param(enable_msi, int, 0444);
-MODULE_PARM_DESC(enable_msi, "Enable PCI MSI");
-
#define DEFAULT_INT_MOD_CNT100 /* 200us */
#define MAX_INT_MOD_CNT65000
#define MIN_INT_MOD_CNT50
@@ -211,13 +207,4 @@ void __devinit atl1_check_options(struct atl1_adapter
*adapter)
adapter->hw.flash_vendor = (u8) (opt.def);
}
}
-
- { /* PCI MSI usage */
-
-#ifdef CONFIG_PCI_MSI
- adapter->enable_msi = enable_msi;
-#else
- adapter->enable_msi = 0;
-#endif
- }
}
Luca
--
Inquietudine sintetica
Solo, davanti all'ignoto
Tienimi stretto a te
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
On Mon, 22 Jan 2007 21:00:04 +0100 Luca Tettamanti <[EMAIL PROTECTED]> wrote: > Il Sun, Jan 21, 2007 at 09:33:39PM -0600, Jay Cliburn ha scritto: > > Randy Dunlap wrote: > > >On Sun, 21 Jan 2007 15:07:37 -0600 Jay Cliburn wrote: > > [snip] > > > > >>+ value = ioread16(hw->hw_addr + REG_PCIE_CAP_LIST); > > >>+ return ((value & 0xFF00) == 0x6C00) ? 0 : 1; > > > > > >Are there defines or enums for these? > > >Fewer magic numbers would be nice/helpful/readable. > > [snip] > > >>+ s32 ret; > > >>+ ret = atl1_write_phy_reg(hw, 29, 0x0029); > > > > > >Fewer magic numbers? > > > > Unfortunately, we don't have a spec. This is how the vendor coded it. > > > > [snip] > > >>+ > > >>+int enable_msi; > > >>+module_param(enable_msi, int, 0444); > > >>+MODULE_PARM_DESC(enable_msi, "Enable PCI MSI"); > > > > > >Hm, I thought that we didn't want individual drivers having MSI config > > >options... > > > > Luca? This one was yours IIRC. Care to chime in? > > I remember that discussion, but since there's no sistem-wide MSI > blacklist (or whitelist) I don't think it's safe to enable it > unconditionally. Judging from bug reports on lkml it's not safe to > assume that MSI support is sane on non-Intel chipsets. > > Luca There is MSI blacklisting see drivers/pci/quirks.c code. But the blacklist isn't complete enough yet. IMHO the MSI disabling should be removed from drivers and be done in the PCI core. But it doesn't seem to have gotten widespread support. -- Stephen Hemminger <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
On Mon, 22 Jan 2007 21:00:04 +0100 Luca Tettamanti [EMAIL PROTECTED] wrote: Il Sun, Jan 21, 2007 at 09:33:39PM -0600, Jay Cliburn ha scritto: Randy Dunlap wrote: On Sun, 21 Jan 2007 15:07:37 -0600 Jay Cliburn wrote: [snip] + value = ioread16(hw-hw_addr + REG_PCIE_CAP_LIST); + return ((value 0xFF00) == 0x6C00) ? 0 : 1; Are there defines or enums for these? Fewer magic numbers would be nice/helpful/readable. [snip] + s32 ret; + ret = atl1_write_phy_reg(hw, 29, 0x0029); Fewer magic numbers? Unfortunately, we don't have a spec. This is how the vendor coded it. [snip] + +int enable_msi; +module_param(enable_msi, int, 0444); +MODULE_PARM_DESC(enable_msi, Enable PCI MSI); Hm, I thought that we didn't want individual drivers having MSI config options... Luca? This one was yours IIRC. Care to chime in? I remember that discussion, but since there's no sistem-wide MSI blacklist (or whitelist) I don't think it's safe to enable it unconditionally. Judging from bug reports on lkml it's not safe to assume that MSI support is sane on non-Intel chipsets. Luca There is MSI blacklisting see drivers/pci/quirks.c code. But the blacklist isn't complete enough yet. IMHO the MSI disabling should be removed from drivers and be done in the PCI core. But it doesn't seem to have gotten widespread support. -- Stephen Hemminger [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Il Tue, Jan 23, 2007 at 11:25:22AM -0800, Stephen Hemminger ha scritto:
On Mon, 22 Jan 2007 21:00:04 +0100
Luca Tettamanti [EMAIL PROTECTED] wrote:
Il Sun, Jan 21, 2007 at 09:33:39PM -0600, Jay Cliburn ha scritto:
Randy Dunlap wrote:
On Sun, 21 Jan 2007 15:07:37 -0600 Jay Cliburn wrote:
[snip]
+
+int enable_msi;
+module_param(enable_msi, int, 0444);
+MODULE_PARM_DESC(enable_msi, Enable PCI MSI);
Hm, I thought that we didn't want individual drivers having MSI config
options...
Luca? This one was yours IIRC. Care to chime in?
I remember that discussion, but since there's no sistem-wide MSI
blacklist (or whitelist) I don't think it's safe to enable it
unconditionally. Judging from bug reports on lkml it's not safe to
assume that MSI support is sane on non-Intel chipsets.
Luca
There is MSI blacklisting see drivers/pci/quirks.c code.
But the blacklist isn't complete enough yet.
IMHO the MSI disabling should be removed from drivers and be done
in the PCI core. But it doesn't seem to have gotten widespread
support.
Does the INTx madness (like this one:
http://marc.theaimsgroup.com/?l=linux-kernelm=116668921431574w=2
) affect also PCI-E INTx emulation?
Anyway...
Unconditionally enable MSI in atl1 driver. Also remove some useless
#ifdef since pci_{en,dis}able_msi() are no-op when MSI support is not
configured in.
Signed-off-by: Luca Tettamanti [EMAIL PROTECTED]
---
Patch against current netdev tree.
drivers/net/atl1/atl1.h |1 -
drivers/net/atl1/atl1_main.c | 22 +++---
drivers/net/atl1/atl1_param.c | 13 -
3 files changed, 7 insertions(+), 29 deletions(-)
diff --git a/drivers/net/atl1/atl1.h b/drivers/net/atl1/atl1.h
index 1d13e8f..0b30e1c 100644
--- a/drivers/net/atl1/atl1.h
+++ b/drivers/net/atl1/atl1.h
@@ -281,7 +281,6 @@ struct atl1_adapter {
struct atl1_smb smb;
struct atl1_cmb cmb;
- int enable_msi;
u32 pci_state[16];
};
diff --git a/drivers/net/atl1/atl1_main.c b/drivers/net/atl1/atl1_main.c
index c0b1555..68e6cd1 100644
--- a/drivers/net/atl1/atl1_main.c
+++ b/drivers/net/atl1/atl1_main.c
@@ -1793,18 +1793,12 @@ s32 atl1_up(struct atl1_adapter *adapter)
goto err_up;
}
-#ifdef CONFIG_PCI_MSI
- if (adapter-enable_msi) {
- err = pci_enable_msi(adapter-pdev);
- if (err) {
- dev_info(adapter-pdev-dev, Unable to enable MSI:
%d\n, err);
- adapter-enable_msi = 0;
- }
- }
-#endif
- if (!adapter-enable_msi)
+ err = pci_enable_msi(adapter-pdev);
+ if (err) {
+ dev_info(adapter-pdev-dev, Unable to enable MSI: %d\n,
err);
irq_flags |= IRQF_SHARED;
-
+ }
+
err = request_irq(adapter-pdev-irq, atl1_intr, irq_flags,
netdev-name, netdev);
if (unlikely(err))
@@ -1821,6 +1815,7 @@ s32 atl1_up(struct atl1_adapter *adapter)
free_irq(adapter-pdev-irq, netdev);
err_up:
+ pci_disable_msi(adapter-pdev);
/* free rx_buffers */
atl1_clean_rx_ring(adapter);
return err;
@@ -1836,10 +1831,7 @@ void atl1_down(struct atl1_adapter *adapter)
atl1_irq_disable(adapter);
free_irq(adapter-pdev-irq, netdev);
-#ifdef CONFIG_PCI_MSI
- if (adapter-enable_msi)
- pci_disable_msi(adapter-pdev);
-#endif
+ pci_disable_msi(adapter-pdev);
atl1_reset_hw(adapter-hw);
adapter-cmb.cmb-int_stats = 0;
diff --git a/drivers/net/atl1/atl1_param.c b/drivers/net/atl1/atl1_param.c
index 4732f43..caa2d83 100644
--- a/drivers/net/atl1/atl1_param.c
+++ b/drivers/net/atl1/atl1_param.c
@@ -68,10 +68,6 @@ static int num_flash_vendor = 0;
module_param_array_named(flash_vendor, flash_vendor, int, num_flash_vendor,
0);
MODULE_PARM_DESC(flash_vendor, SPI flash vendor);
-int enable_msi;
-module_param(enable_msi, int, 0444);
-MODULE_PARM_DESC(enable_msi, Enable PCI MSI);
-
#define DEFAULT_INT_MOD_CNT100 /* 200us */
#define MAX_INT_MOD_CNT65000
#define MIN_INT_MOD_CNT50
@@ -211,13 +207,4 @@ void __devinit atl1_check_options(struct atl1_adapter
*adapter)
adapter-hw.flash_vendor = (u8) (opt.def);
}
}
-
- { /* PCI MSI usage */
-
-#ifdef CONFIG_PCI_MSI
- adapter-enable_msi = enable_msi;
-#else
- adapter-enable_msi = 0;
-#endif
- }
}
Luca
--
Inquietudine sintetica
Solo, davanti all'ignoto
Tienimi stretto a te
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Stephen Hemminger wrote: IMHO the MSI disabling should be removed from drivers and be done in the PCI core. That is the consensus opinion. Currently drivers implement the MSI tests because the core PCI code hasn't been up to snuff. I (and others) have been discouraging that, but when a user faces a choice between working and non-working network, the pragmatic solution wins. All efforts to get us to the point where we can remove the MSI tests from drivers are strongly supported... Jeff - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
On Tue, 23 Jan 2007 16:33:29 -0500 Jeff Garzik [EMAIL PROTECTED] wrote: Stephen Hemminger wrote: IMHO the MSI disabling should be removed from drivers and be done in the PCI core. That is the consensus opinion. Currently drivers implement the MSI tests because the core PCI code hasn't been up to snuff. I (and others) have been discouraging that, but when a user faces a choice between working and non-working network, the pragmatic solution wins. All efforts to get us to the point where we can remove the MSI tests from drivers are strongly supported... Jeff So far, either MSI works for all devices or is broken, so it makes sense to have a msi=off boot option (if there isn't already) -- Stephen Hemminger [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Stephen Hemminger wrote: On Tue, 23 Jan 2007 16:33:29 -0500 Jeff Garzik [EMAIL PROTECTED] wrote: Stephen Hemminger wrote: IMHO the MSI disabling should be removed from drivers and be done in the PCI core. That is the consensus opinion. Currently drivers implement the MSI tests because the core PCI code hasn't been up to snuff. I (and others) have been discouraging that, but when a user faces a choice between working and non-working network, the pragmatic solution wins. Linus's remark (IIRC) was to not enable CONFIG_PCI_MSI then. All efforts to get us to the point where we can remove the MSI tests from drivers are strongly supported... Jeff So far, either MSI works for all devices or is broken, so it makes sense to have a msi=off boot option (if there isn't already) There is one, but it's spelled pci=nomsi. -- ~Randy - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Randy Dunlap wrote: Stephen Hemminger wrote: On Tue, 23 Jan 2007 16:33:29 -0500 Jeff Garzik [EMAIL PROTECTED] wrote: Stephen Hemminger wrote: IMHO the MSI disabling should be removed from drivers and be done in the PCI core. That is the consensus opinion. Currently drivers implement the MSI tests because the core PCI code hasn't been up to snuff. I (and others) have been discouraging that, but when a user faces a choice between working and non-working network, the pragmatic solution wins. Linus's remark (IIRC) was to not enable CONFIG_PCI_MSI then. Most distros, especially cutting edge ones like Fedora, will enable CONFIG_PCI_MSI. All efforts to get us to the point where we can remove the MSI tests from drivers are strongly supported... Jeff So far, either MSI works for all devices or is broken, so it makes sense to have a msi=off boot option (if there isn't already) There is one, but it's spelled pci=nomsi. Yep. Thanks for repeating this, and refreshing the collective memory. Jeff - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Il Sun, Jan 21, 2007 at 09:33:39PM -0600, Jay Cliburn ha scritto: > Randy Dunlap wrote: > >On Sun, 21 Jan 2007 15:07:37 -0600 Jay Cliburn wrote: > [snip] > > >>+ value = ioread16(hw->hw_addr + REG_PCIE_CAP_LIST); > >>+ return ((value & 0xFF00) == 0x6C00) ? 0 : 1; > > > >Are there defines or enums for these? > >Fewer magic numbers would be nice/helpful/readable. > [snip] > >>+ s32 ret; > >>+ ret = atl1_write_phy_reg(hw, 29, 0x0029); > > > >Fewer magic numbers? > > Unfortunately, we don't have a spec. This is how the vendor coded it. > > [snip] > >>+ > >>+int enable_msi; > >>+module_param(enable_msi, int, 0444); > >>+MODULE_PARM_DESC(enable_msi, "Enable PCI MSI"); > > > >Hm, I thought that we didn't want individual drivers having MSI config > >options... > > Luca? This one was yours IIRC. Care to chime in? I remember that discussion, but since there's no sistem-wide MSI blacklist (or whitelist) I don't think it's safe to enable it unconditionally. Judging from bug reports on lkml it's not safe to assume that MSI support is sane on non-Intel chipsets. Luca -- Il coraggio non mi manca. E` la paura che mi frega... - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Il Sun, Jan 21, 2007 at 09:33:39PM -0600, Jay Cliburn ha scritto: Randy Dunlap wrote: On Sun, 21 Jan 2007 15:07:37 -0600 Jay Cliburn wrote: [snip] + value = ioread16(hw-hw_addr + REG_PCIE_CAP_LIST); + return ((value 0xFF00) == 0x6C00) ? 0 : 1; Are there defines or enums for these? Fewer magic numbers would be nice/helpful/readable. [snip] + s32 ret; + ret = atl1_write_phy_reg(hw, 29, 0x0029); Fewer magic numbers? Unfortunately, we don't have a spec. This is how the vendor coded it. [snip] + +int enable_msi; +module_param(enable_msi, int, 0444); +MODULE_PARM_DESC(enable_msi, Enable PCI MSI); Hm, I thought that we didn't want individual drivers having MSI config options... Luca? This one was yours IIRC. Care to chime in? I remember that discussion, but since there's no sistem-wide MSI blacklist (or whitelist) I don't think it's safe to enable it unconditionally. Judging from bug reports on lkml it's not safe to assume that MSI support is sane on non-Intel chipsets. Luca -- Il coraggio non mi manca. E` la paura che mi frega... - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Randy Dunlap wrote: On Sun, 21 Jan 2007 15:07:37 -0600 Jay Cliburn wrote: [snip] + value = ioread16(hw->hw_addr + REG_PCIE_CAP_LIST); + return ((value & 0xFF00) == 0x6C00) ? 0 : 1; Are there defines or enums for these? Fewer magic numbers would be nice/helpful/readable. [snip] + s32 ret; + ret = atl1_write_phy_reg(hw, 29, 0x0029); Fewer magic numbers? Unfortunately, we don't have a spec. This is how the vendor coded it. [snip] + +int enable_msi; +module_param(enable_msi, int, 0444); +MODULE_PARM_DESC(enable_msi, "Enable PCI MSI"); Hm, I thought that we didn't want individual drivers having MSI config options... Luca? This one was yours IIRC. Care to chime in? Randy, thank you for the review. Jay - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
On Sun, 21 Jan 2007 15:07:37 -0600 Jay Cliburn wrote:
> This patch contains auxiliary C files for the Attansic L1 gigabit ethernet
> adapter driver.
>
> Signed-off-by: Jay Cliburn <[EMAIL PROTECTED]>
> Signed-off-by: Chris Snook <[EMAIL PROTECTED]>
> ---
>
> atl1_ethtool.c | 436 ++
> atl1_hw.c | 728
> +
> atl1_param.c | 223 +
> 3 files changed, 1387 insertions(+)
>
> diff --git a/drivers/net/atl1/atl1_ethtool.c b/drivers/net/atl1/atl1_ethtool.c
> new file mode 100644
> index 000..4c6e505
> --- /dev/null
> +++ b/drivers/net/atl1/atl1_ethtool.c
> @@ -0,0 +1,436 @@
> +/**
Please use "/**" _only_ to begin kernel-doc comments
(and this is not a kernel-doc comment).
(occurs at multiple other places in this and the other patches)
> + * Copyright(c) 2005 - 2006 Attansic Corporation. All rights reserved.
> + * Copyright(c) 2006 Chris Snook <[EMAIL PROTECTED]>
> + * Copyright(c) 2006 Jay Cliburn <[EMAIL PROTECTED]>
> + *
> + * Derived from Intel e1000 driver
> + * Copyright(c) 1999 - 2005 Intel Corporation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + * This program is distributed in the hope that 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, write to the Free Software Foundation, Inc., 59
> + * Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> + **/
> +
> +
> +static int atl1_get_settings(struct net_device *netdev, struct ethtool_cmd
> *ecmd)
> +{
> + struct atl1_adapter *adapter = netdev_priv(netdev);
> + struct atl1_hw *hw = >hw;
> +
> + ecmd->supported = (SUPPORTED_10baseT_Half |
> +SUPPORTED_10baseT_Full |
> +SUPPORTED_100baseT_Half |
> +SUPPORTED_100baseT_Full |
> +SUPPORTED_1000baseT_Full |
> +SUPPORTED_Autoneg | SUPPORTED_TP);
> + ecmd->advertising = ADVERTISED_TP;
> + if (hw->media_type == MEDIA_TYPE_AUTO_SENSOR ||
> + hw->media_type == MEDIA_TYPE_1000M_FULL) {
> + ecmd->advertising |= ADVERTISED_Autoneg;
> + if (hw->media_type == MEDIA_TYPE_AUTO_SENSOR) {
> + ecmd->advertising |= ADVERTISED_Autoneg;
> + ecmd->advertising |=
> + (ADVERTISED_10baseT_Half |
> + ADVERTISED_10baseT_Full |
> + ADVERTISED_100baseT_Half |
> + ADVERTISED_100baseT_Full |
> + ADVERTISED_1000baseT_Full);
> + } else {
> + ecmd->advertising |= (ADVERTISED_1000baseT_Full);
Kernel coding style is not to use braces around one-statement "blocks."
(occurs in multiple other places)
> + }
> + }
> + ecmd->port = PORT_TP;
> + ecmd->phy_address = 0;
> + ecmd->transceiver = XCVR_INTERNAL;
> +
> + if (netif_carrier_ok(adapter->netdev)) {
> + u16 link_speed, link_duplex;
> + atl1_get_speed_and_duplex(hw, _speed, _duplex);
> + ecmd->speed = link_speed;
> + if (link_duplex == FULL_DUPLEX)
> + ecmd->duplex = DUPLEX_FULL;
> + else
> + ecmd->duplex = DUPLEX_HALF;
> + } else {
> + ecmd->speed = -1;
> + ecmd->duplex = -1;
> + }
> + if (hw->media_type == MEDIA_TYPE_AUTO_SENSOR ||
> + hw->media_type == MEDIA_TYPE_1000M_FULL) {
> + ecmd->autoneg = AUTONEG_ENABLE;
> + } else {
> + ecmd->autoneg = AUTONEG_DISABLE;
> + }
> +
> + return 0;
> +}
> +
> +static int atl1_set_settings(struct net_device *netdev, struct ethtool_cmd
> *ecmd)
> +{
> + struct atl1_adapter *adapter = netdev_priv(netdev);
> + struct atl1_hw *hw = >hw;
> + u16 phy_data;
> + int ret_val = 0;
> + u16 old_media_type = hw->media_type;
> +
> + if (netif_running(adapter->netdev)) {
> + printk(KERN_DEBUG "%s: ethtool shutting down link adapter\n",
What's a "link adapter"?
> + atl1_driver_name);
> + atl1_down(adapter);
> + }
> +
> + if (ecmd->autoneg == AUTONEG_ENABLE) {
> + hw->media_type = MEDIA_TYPE_AUTO_SENSOR;
> + } else {
> + if (ecmd->speed == SPEED_1000) {
> + if (ecmd->duplex != DUPLEX_FULL) {
> +
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
On Sun, 21 Jan 2007 15:07:37 -0600 Jay Cliburn wrote:
This patch contains auxiliary C files for the Attansic L1 gigabit ethernet
adapter driver.
Signed-off-by: Jay Cliburn [EMAIL PROTECTED]
Signed-off-by: Chris Snook [EMAIL PROTECTED]
---
atl1_ethtool.c | 436 ++
atl1_hw.c | 728
+
atl1_param.c | 223 +
3 files changed, 1387 insertions(+)
diff --git a/drivers/net/atl1/atl1_ethtool.c b/drivers/net/atl1/atl1_ethtool.c
new file mode 100644
index 000..4c6e505
--- /dev/null
+++ b/drivers/net/atl1/atl1_ethtool.c
@@ -0,0 +1,436 @@
+/**
Please use /** _only_ to begin kernel-doc comments
(and this is not a kernel-doc comment).
(occurs at multiple other places in this and the other patches)
+ * Copyright(c) 2005 - 2006 Attansic Corporation. All rights reserved.
+ * Copyright(c) 2006 Chris Snook [EMAIL PROTECTED]
+ * Copyright(c) 2006 Jay Cliburn [EMAIL PROTECTED]
+ *
+ * Derived from Intel e1000 driver
+ * Copyright(c) 1999 - 2005 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that 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, write to the Free Software Foundation, Inc., 59
+ * Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ **/
+
+
+static int atl1_get_settings(struct net_device *netdev, struct ethtool_cmd
*ecmd)
+{
+ struct atl1_adapter *adapter = netdev_priv(netdev);
+ struct atl1_hw *hw = adapter-hw;
+
+ ecmd-supported = (SUPPORTED_10baseT_Half |
+SUPPORTED_10baseT_Full |
+SUPPORTED_100baseT_Half |
+SUPPORTED_100baseT_Full |
+SUPPORTED_1000baseT_Full |
+SUPPORTED_Autoneg | SUPPORTED_TP);
+ ecmd-advertising = ADVERTISED_TP;
+ if (hw-media_type == MEDIA_TYPE_AUTO_SENSOR ||
+ hw-media_type == MEDIA_TYPE_1000M_FULL) {
+ ecmd-advertising |= ADVERTISED_Autoneg;
+ if (hw-media_type == MEDIA_TYPE_AUTO_SENSOR) {
+ ecmd-advertising |= ADVERTISED_Autoneg;
+ ecmd-advertising |=
+ (ADVERTISED_10baseT_Half |
+ ADVERTISED_10baseT_Full |
+ ADVERTISED_100baseT_Half |
+ ADVERTISED_100baseT_Full |
+ ADVERTISED_1000baseT_Full);
+ } else {
+ ecmd-advertising |= (ADVERTISED_1000baseT_Full);
Kernel coding style is not to use braces around one-statement blocks.
(occurs in multiple other places)
+ }
+ }
+ ecmd-port = PORT_TP;
+ ecmd-phy_address = 0;
+ ecmd-transceiver = XCVR_INTERNAL;
+
+ if (netif_carrier_ok(adapter-netdev)) {
+ u16 link_speed, link_duplex;
+ atl1_get_speed_and_duplex(hw, link_speed, link_duplex);
+ ecmd-speed = link_speed;
+ if (link_duplex == FULL_DUPLEX)
+ ecmd-duplex = DUPLEX_FULL;
+ else
+ ecmd-duplex = DUPLEX_HALF;
+ } else {
+ ecmd-speed = -1;
+ ecmd-duplex = -1;
+ }
+ if (hw-media_type == MEDIA_TYPE_AUTO_SENSOR ||
+ hw-media_type == MEDIA_TYPE_1000M_FULL) {
+ ecmd-autoneg = AUTONEG_ENABLE;
+ } else {
+ ecmd-autoneg = AUTONEG_DISABLE;
+ }
+
+ return 0;
+}
+
+static int atl1_set_settings(struct net_device *netdev, struct ethtool_cmd
*ecmd)
+{
+ struct atl1_adapter *adapter = netdev_priv(netdev);
+ struct atl1_hw *hw = adapter-hw;
+ u16 phy_data;
+ int ret_val = 0;
+ u16 old_media_type = hw-media_type;
+
+ if (netif_running(adapter-netdev)) {
+ printk(KERN_DEBUG %s: ethtool shutting down link adapter\n,
What's a link adapter?
+ atl1_driver_name);
+ atl1_down(adapter);
+ }
+
+ if (ecmd-autoneg == AUTONEG_ENABLE) {
+ hw-media_type = MEDIA_TYPE_AUTO_SENSOR;
+ } else {
+ if (ecmd-speed == SPEED_1000) {
+ if (ecmd-duplex != DUPLEX_FULL) {
+ printk(KERN_WARNING
+%s: can't force to 1000M half duplex\n,
+
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Randy Dunlap wrote: On Sun, 21 Jan 2007 15:07:37 -0600 Jay Cliburn wrote: [snip] + value = ioread16(hw-hw_addr + REG_PCIE_CAP_LIST); + return ((value 0xFF00) == 0x6C00) ? 0 : 1; Are there defines or enums for these? Fewer magic numbers would be nice/helpful/readable. [snip] + s32 ret; + ret = atl1_write_phy_reg(hw, 29, 0x0029); Fewer magic numbers? Unfortunately, we don't have a spec. This is how the vendor coded it. [snip] + +int enable_msi; +module_param(enable_msi, int, 0444); +MODULE_PARM_DESC(enable_msi, Enable PCI MSI); Hm, I thought that we didn't want individual drivers having MSI config options... Luca? This one was yours IIRC. Care to chime in? Randy, thank you for the review. Jay - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Jay Cliburn wrote:
+static u32 atl1_get_tx_csum(struct net_device *netdev)
+{
+ return (netdev->features & NETIF_F_HW_CSUM) != 0;
+}
+
+static int atl1_set_tx_csum(struct net_device *netdev, u32 data)
+{
+ if (data)
+ netdev->features |= NETIF_F_HW_CSUM;
+ else
+ netdev->features &= ~NETIF_F_HW_CSUM;
+
+ return 0;
+}
+
+static int atl1_set_tso(struct net_device *netdev, u32 data)
+{
+ if (data)
+ netdev->features |= NETIF_F_TSO;
+ else
+ netdev->features &= ~NETIF_F_TSO;
+ return 0;
+}
There should be generic functions covering this.
+static u32 ether_crc_le(int length, unsigned char *data)
+{
+ u32 crc = ~0; /* Initial value. */
+ while (--length >= 0) {
+ unsigned char current_octet = *data++;
+ int bit;
+ for (bit = 8; --bit >= 0; current_octet >>= 1) {
+ if ((crc ^ current_octet) & 1) {
+ crc >>= 1;
+ crc ^= 0xedb88320;
+ } else
+ crc >>= 1;
+ }
+ }
+ return ~crc;
+}
this duplicates a library function
+/**
+ * Reset the transmit and receive units; mask and clear all interrupts.
+ * hw - Struct containing variables accessed by shared code
+ * return : ATL1_SUCCESS or idle status (if error)
+ **/
+s32 atl1_reset_hw(struct atl1_hw * hw)
+{
+ u32 icr;
+ u16 pci_cfg_cmd_word;
+ int i;
+
+ /* Workaround for PCI problem when BIOS sets MMRBC incorrectly. */
+ atl1_read_pci_cfg(hw, PCI_REG_COMMAND, _cfg_cmd_word);
+ if ((pci_cfg_cmd_word &
+(CMD_IO_SPACE | CMD_MEMORY_SPACE | CMD_BUS_MASTER))
+ != (CMD_IO_SPACE | CMD_MEMORY_SPACE | CMD_BUS_MASTER)) {
+ pci_cfg_cmd_word |=
+ (CMD_IO_SPACE | CMD_MEMORY_SPACE | CMD_BUS_MASTER);
+ atl1_write_pci_cfg(hw, PCI_REG_COMMAND, _cfg_cmd_word);
+ }
This duplicates bits set by pci_enable_device() and pci_set_master()
+ /**
+ * Clear Interrupt mask to stop board from generating
+ * interrupts & Clear any pending interrupt events
+ **/
+ /**
+* atl1_write32(hw, REG_IMR, 0);
+* atl1_write32(hw, REG_ISR, 0x);
+**/
+
+ /**
+* Issue Soft Reset to the MAC. This will reset the chip's
+* transmit, receive, DMA. It will not effect
+* the current PCI configuration. The global reset bit is self-
+* clearing, and should clear within a microsecond.
+**/
+ /*atl1_write32(hw, REG_MASTER_CTRL, MASTER_CTRL_SOFT_RST);*/
+ atl1_write32(hw, REG_MASTER_CTRL, MASTER_CTRL_SOFT_RST);
+ wmb();
PCI posting (need to read a register before delaying to guarantee flush;
probably makes wmb superfluous)
+ atl1_write16(hw, REG_GPHY_ENABLE, 1);
+
+ msec_delay(1); /* delay about 1ms */
ditto
+ /* Wait at least 10ms for All module to be Idle */
+ for (i = 0; i < 10; i++) {
+ icr = atl1_read32(hw, REG_IDLE_STATUS);
+ if (!icr)
+ break;
+ msec_delay(1); /* delay 1 ms */
+ cpu_relax();/* FIXME: is this still the right way to do
this? */
+ }
+
+ if (icr)
+ return icr;
+
+ return ATL1_SUCCESS;
+}
+
+static inline bool atl1_eth_address_valid(u8 * p_addr)
+{
+ /* Invalid PermanentAddress ? */
+ if (((p_addr[0] == 0) &&
+(p_addr[1] == 0) &&
+(p_addr[2] == 0) &&
+(p_addr[3] == 0) && (p_addr[4] == 0) && (p_addr[5] == 0)
+ ) || (p_addr[0] & 1))
+ /* Multicast address or Broadcast Address */
+ return false;
+
+ return true;
look at is_valid_ether_addr() lib function
+ if (atl1_get_permanent_address(hw)) {
+ hw->perm_mac_addr[0] = 0x00;
+ hw->perm_mac_addr[1] = 0x13;
+ hw->perm_mac_addr[2] = 0x74;
+ hw->perm_mac_addr[3] = 0x00;
+ hw->perm_mac_addr[4] = 0x5c;
+ hw->perm_mac_addr[5] = 0x38;
+ }
standard technique is to use random bytes, not a fixed address like
this, when MAC address is otherwise unavailable. consider what happens
when two MACs are present.
+ for (i = 0; i < NODE_ADDRESS_SIZE; i++)
+ hw->mac_addr[i] = hw->perm_mac_addr[i];
+ return ATL1_SUCCESS;
+}
+
+/**
+ * Hashes an address to determine its location in the multicast table
+ * hw - Struct containing variables accessed by shared code
+ * mc_addr - the multicast address to hash
+ *
+ * atl1_hash_mc_addr
+ * purpose
+ * set hash value for a multicast address
+ * hash calcu processing :
+ * 1. calcu 32bit CRC for multicast address
+ * 2. reverse crc with MSB to LSB
+ **/
+u32 atl1_hash_mc_addr(struct atl1_hw * hw, u8
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
Jay Cliburn wrote:
+static u32 atl1_get_tx_csum(struct net_device *netdev)
+{
+ return (netdev-features NETIF_F_HW_CSUM) != 0;
+}
+
+static int atl1_set_tx_csum(struct net_device *netdev, u32 data)
+{
+ if (data)
+ netdev-features |= NETIF_F_HW_CSUM;
+ else
+ netdev-features = ~NETIF_F_HW_CSUM;
+
+ return 0;
+}
+
+static int atl1_set_tso(struct net_device *netdev, u32 data)
+{
+ if (data)
+ netdev-features |= NETIF_F_TSO;
+ else
+ netdev-features = ~NETIF_F_TSO;
+ return 0;
+}
There should be generic functions covering this.
+static u32 ether_crc_le(int length, unsigned char *data)
+{
+ u32 crc = ~0; /* Initial value. */
+ while (--length = 0) {
+ unsigned char current_octet = *data++;
+ int bit;
+ for (bit = 8; --bit = 0; current_octet = 1) {
+ if ((crc ^ current_octet) 1) {
+ crc = 1;
+ crc ^= 0xedb88320;
+ } else
+ crc = 1;
+ }
+ }
+ return ~crc;
+}
this duplicates a library function
+/**
+ * Reset the transmit and receive units; mask and clear all interrupts.
+ * hw - Struct containing variables accessed by shared code
+ * return : ATL1_SUCCESS or idle status (if error)
+ **/
+s32 atl1_reset_hw(struct atl1_hw * hw)
+{
+ u32 icr;
+ u16 pci_cfg_cmd_word;
+ int i;
+
+ /* Workaround for PCI problem when BIOS sets MMRBC incorrectly. */
+ atl1_read_pci_cfg(hw, PCI_REG_COMMAND, pci_cfg_cmd_word);
+ if ((pci_cfg_cmd_word
+(CMD_IO_SPACE | CMD_MEMORY_SPACE | CMD_BUS_MASTER))
+ != (CMD_IO_SPACE | CMD_MEMORY_SPACE | CMD_BUS_MASTER)) {
+ pci_cfg_cmd_word |=
+ (CMD_IO_SPACE | CMD_MEMORY_SPACE | CMD_BUS_MASTER);
+ atl1_write_pci_cfg(hw, PCI_REG_COMMAND, pci_cfg_cmd_word);
+ }
This duplicates bits set by pci_enable_device() and pci_set_master()
+ /**
+ * Clear Interrupt mask to stop board from generating
+ * interrupts Clear any pending interrupt events
+ **/
+ /**
+* atl1_write32(hw, REG_IMR, 0);
+* atl1_write32(hw, REG_ISR, 0x);
+**/
+
+ /**
+* Issue Soft Reset to the MAC. This will reset the chip's
+* transmit, receive, DMA. It will not effect
+* the current PCI configuration. The global reset bit is self-
+* clearing, and should clear within a microsecond.
+**/
+ /*atl1_write32(hw, REG_MASTER_CTRL, MASTER_CTRL_SOFT_RST);*/
+ atl1_write32(hw, REG_MASTER_CTRL, MASTER_CTRL_SOFT_RST);
+ wmb();
PCI posting (need to read a register before delaying to guarantee flush;
probably makes wmb superfluous)
+ atl1_write16(hw, REG_GPHY_ENABLE, 1);
+
+ msec_delay(1); /* delay about 1ms */
ditto
+ /* Wait at least 10ms for All module to be Idle */
+ for (i = 0; i 10; i++) {
+ icr = atl1_read32(hw, REG_IDLE_STATUS);
+ if (!icr)
+ break;
+ msec_delay(1); /* delay 1 ms */
+ cpu_relax();/* FIXME: is this still the right way to do
this? */
+ }
+
+ if (icr)
+ return icr;
+
+ return ATL1_SUCCESS;
+}
+
+static inline bool atl1_eth_address_valid(u8 * p_addr)
+{
+ /* Invalid PermanentAddress ? */
+ if (((p_addr[0] == 0)
+(p_addr[1] == 0)
+(p_addr[2] == 0)
+(p_addr[3] == 0) (p_addr[4] == 0) (p_addr[5] == 0)
+ ) || (p_addr[0] 1))
+ /* Multicast address or Broadcast Address */
+ return false;
+
+ return true;
look at is_valid_ether_addr() lib function
+ if (atl1_get_permanent_address(hw)) {
+ hw-perm_mac_addr[0] = 0x00;
+ hw-perm_mac_addr[1] = 0x13;
+ hw-perm_mac_addr[2] = 0x74;
+ hw-perm_mac_addr[3] = 0x00;
+ hw-perm_mac_addr[4] = 0x5c;
+ hw-perm_mac_addr[5] = 0x38;
+ }
standard technique is to use random bytes, not a fixed address like
this, when MAC address is otherwise unavailable. consider what happens
when two MACs are present.
+ for (i = 0; i NODE_ADDRESS_SIZE; i++)
+ hw-mac_addr[i] = hw-perm_mac_addr[i];
+ return ATL1_SUCCESS;
+}
+
+/**
+ * Hashes an address to determine its location in the multicast table
+ * hw - Struct containing variables accessed by shared code
+ * mc_addr - the multicast address to hash
+ *
+ * atl1_hash_mc_addr
+ * purpose
+ * set hash value for a multicast address
+ * hash calcu processing :
+ * 1. calcu 32bit CRC for multicast address
+ * 2. reverse crc with MSB to LSB
+ **/
+u32 atl1_hash_mc_addr(struct atl1_hw * hw, u8 * mc_addr)
+{
+ u32 crc32,
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
> +#define ATL1_STATS_LEN sizeof(atl1_gstrings_stats) / sizeof(struct
> atl1_stats)
Just use an opencoded ARRAY_SIZE().
> +void atl1_read_pci_cfg(struct atl1_hw *hw, u32 reg, u16 * value)
> +{
> +struct atl1_adapter *adapter = hw->back;
> +pci_read_config_word(adapter->pdev, reg, value);
> +}
> +
> +void atl1_write_pci_cfg(struct atl1_hw *hw, u32 reg, u16 * value)
> +{
> +struct atl1_adapter *adapter = hw->back;
> +pci_write_config_word(adapter->pdev, reg, *value);
> +}
Please just kill these types of wrappers and use pci_read_config_word/
pci_write_config_word directly.
> +static inline bool atl1_eth_address_valid(u8 * p_addr)
> +{
> + /* Invalid PermanentAddress ? */
> + if (((p_addr[0] == 0) &&
> + (p_addr[1] == 0) &&
> + (p_addr[2] == 0) &&
> + (p_addr[3] == 0) && (p_addr[4] == 0) && (p_addr[5] == 0)
> + ) || (p_addr[0] & 1))
> + /* Multicast address or Broadcast Address */
> + return false;
> +
> + return true;
> +}
Don't we have a generic helper for this kind of thing?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] atl1: Ancillary C files for Attansic L1 driver
+#define ATL1_STATS_LEN sizeof(atl1_gstrings_stats) / sizeof(struct
atl1_stats)
Just use an opencoded ARRAY_SIZE().
+void atl1_read_pci_cfg(struct atl1_hw *hw, u32 reg, u16 * value)
+{
+struct atl1_adapter *adapter = hw-back;
+pci_read_config_word(adapter-pdev, reg, value);
+}
+
+void atl1_write_pci_cfg(struct atl1_hw *hw, u32 reg, u16 * value)
+{
+struct atl1_adapter *adapter = hw-back;
+pci_write_config_word(adapter-pdev, reg, *value);
+}
Please just kill these types of wrappers and use pci_read_config_word/
pci_write_config_word directly.
+static inline bool atl1_eth_address_valid(u8 * p_addr)
+{
+ /* Invalid PermanentAddress ? */
+ if (((p_addr[0] == 0)
+ (p_addr[1] == 0)
+ (p_addr[2] == 0)
+ (p_addr[3] == 0) (p_addr[4] == 0) (p_addr[5] == 0)
+ ) || (p_addr[0] 1))
+ /* Multicast address or Broadcast Address */
+ return false;
+
+ return true;
+}
Don't we have a generic helper for this kind of thing?
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
