[PATCH] forcedeth: Addition of new device id (resend without pci_ids.h)

[PATCH] forcedeth: Addition of new device id (resend without pci_ids.h)

[Ayaz Abdulla]

[-- Attachment #1: Type: text/plain, Size: 110 bytes --]

Add support for new ethernet device in the MCP89 chipset.

Signed-off-by: Ayaz Abdulla <aabdulla@nvidia.com>


[-- Attachment #2: patch-forcedeth-mcp89-device --]
[-- Type: text/plain, Size: 1547 bytes --]

--- old/drivers/net/forcedeth.c	2009-06-02 14:53:45.000000000 -0400
+++ new/drivers/net/forcedeth.c	2009-06-03 11:59:53.000000000 -0400
@@ -70,6 +70,11 @@
 #define dprintk(x...)		do { } while (0)
 #endif
 
+/*
+ * Hardware device ids (no longer updating pci_ids.h)
+ */
+#define PCI_DEVICE_ID_NVIDIA_NVENET_40  0x0D7D
+
 #define TX_WORK_PER_LOOP  64
 #define RX_WORK_PER_LOOP  64
 
@@ -6149,7 +6154,8 @@
 	for (i = 0;i <= np->register_size/sizeof(u32); i++)
 		writel(np->saved_config_space[i], base+i*sizeof(u32));
 
-	pci_write_config_dword(pdev, NV_MSI_PRIV_OFFSET, NV_MSI_PRIV_VALUE);
+	if (np->device_id != PCI_DEVICE_ID_NVIDIA_NVENET_40)
+		pci_write_config_dword(pdev, NV_MSI_PRIV_OFFSET, NV_MSI_PRIV_VALUE);
 
 	/* restore phy state, including autoneg */
 	phy_init(dev);
@@ -6353,6 +6359,10 @@
 		PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_NVENET_39),
 		.driver_data = DEV_NEED_LINKTIMER|DEV_HAS_LARGEDESC|DEV_HAS_CHECKSUM|DEV_HAS_HIGH_DMA|DEV_HAS_MSI|DEV_HAS_POWER_CNTRL|DEV_HAS_PAUSEFRAME_TX_V3|DEV_HAS_STATISTICS_V3|DEV_HAS_TEST_EXTENDED|DEV_HAS_CORRECT_MACADDR|DEV_HAS_COLLISION_FIX|DEV_NEED_TX_LIMIT|DEV_HAS_GEAR_MODE,
 	},
+	{	/* MCP89 Ethernet Controller */
+		PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_NVENET_40),
+		.driver_data = DEV_NEED_LINKTIMER|DEV_HAS_LARGEDESC|DEV_HAS_CHECKSUM|DEV_HAS_HIGH_DMA|DEV_HAS_MSI|DEV_HAS_POWER_CNTRL|DEV_HAS_PAUSEFRAME_TX_V3|DEV_HAS_STATISTICS_V3|DEV_HAS_TEST_EXTENDED|DEV_HAS_CORRECT_MACADDR|DEV_HAS_COLLISION_FIX|DEV_NEED_TX_LIMIT|DEV_HAS_GEAR_MODE,
+	},
 	{0,},
 };
 

Re: [PATCH] forcedeth: Addition of new device id (resend without pci_ids.h)

[Ayaz Abdulla]

David Miller wrote:
> From: Ayaz Abdulla <aabdulla@nvidia.com>
> Date: Wed, 03 Jun 2009 12:06:04 -0400
> 
> 
>>Add support for new ethernet device in the MCP89 chipset.
>>
>>Signed-off-by: Ayaz Abdulla <aabdulla@nvidia.com>
> 
> 
> Just use the constant directly, please :-)

But there is place in the patch that refers to device id so it makes 
more sense to have a macro. i.e

+       if (np->device_id != PCI_DEVICE_ID_NVIDIA_NVENET_40)

other places in the code also refer to device ids for certain 
workarounds. i.e.

		if ((id->device == PCI_DEVICE_ID_NVIDIA_NVENET_12 ||
		     id->device == PCI_DEVICE_ID_NVIDIA_NVENET_13) &&



> 
> The new macro is completely superfluous.
> 
> Trust me, other drivers just use the PCI ID constant directly in their
> tables too, and no animals were harmed by the experiment :-)

Re: [PATCH] forcedeth: Addition of new device id (resend without pci_ids.h)

[Ayaz Abdulla]

David Miller wrote:
> From: Ayaz Abdulla <aabdulla@nvidia.com>
> Date: Wed, 03 Jun 2009 12:24:37 -0400
> 
> 
>>
>>David Miller wrote:
>>
>>>From: Ayaz Abdulla <aabdulla@nvidia.com>
>>>Date: Wed, 03 Jun 2009 12:06:04 -0400
>>>
>>>
>>>>Add support for new ethernet device in the MCP89 chipset.
>>>>
>>>>Signed-off-by: Ayaz Abdulla <aabdulla@nvidia.com>
>>>
>>>Just use the constant directly, please :-)
>>
>>But there is place in the patch that refers to device id so it makes
>>more sense to have a macro. i.e
>>
>>+       if (np->device_id != PCI_DEVICE_ID_NVIDIA_NVENET_40)
>>
>>other places in the code also refer to device ids for certain
>>workarounds. i.e.
>>
>>		if ((id->device == PCI_DEVICE_ID_NVIDIA_NVENET_12 ||
>>		     id->device == PCI_DEVICE_ID_NVIDIA_NVENET_13) &&
> 
> 
> And you can add the define when that is necessary.
> 
> Otherwise, it's being used in exactly one location, the ID table.
> And for that purpose it's superfluous.
> 
> You could also change the driver to use a bitmask of HW workaround
> cases, that gets set very early in the probe.  And the HW
> workarounds are keyed on those bit masks (one test) compared
> to the direct ID checks (potentially multiple compares).
> 
> It would be both cleaner and more efficient.  And you could still
> use the direct constants without any real loss of clarity.

Ok, I will do that in next patch...

Re: [PATCH] forcedeth: Addition of new device id (resend without pci_ids.h)

[David Miller]

From: Ayaz Abdulla <aabdulla@nvidia.com>
Date: Wed, 03 Jun 2009 12:06:04 -0400

> Add support for new ethernet device in the MCP89 chipset.
> 
> Signed-off-by: Ayaz Abdulla <aabdulla@nvidia.com>

Just use the constant directly, please :-)

The new macro is completely superfluous.

Trust me, other drivers just use the PCI ID constant directly in their
tables too, and no animals were harmed by the experiment :-)

Re: [PATCH] forcedeth: Addition of new device id (resend without pci_ids.h)

[David Miller]

From: Ayaz Abdulla <aabdulla@nvidia.com>
Date: Wed, 03 Jun 2009 12:24:37 -0400

> 
> 
> David Miller wrote:
>> From: Ayaz Abdulla <aabdulla@nvidia.com>
>> Date: Wed, 03 Jun 2009 12:06:04 -0400
>> 
>>>Add support for new ethernet device in the MCP89 chipset.
>>>
>>>Signed-off-by: Ayaz Abdulla <aabdulla@nvidia.com>
>> Just use the constant directly, please :-)
> 
> But there is place in the patch that refers to device id so it makes
> more sense to have a macro. i.e
> 
> +       if (np->device_id != PCI_DEVICE_ID_NVIDIA_NVENET_40)
> 
> other places in the code also refer to device ids for certain
> workarounds. i.e.
> 
> 		if ((id->device == PCI_DEVICE_ID_NVIDIA_NVENET_12 ||
> 		     id->device == PCI_DEVICE_ID_NVIDIA_NVENET_13) &&

And you can add the define when that is necessary.

Otherwise, it's being used in exactly one location, the ID table.
And for that purpose it's superfluous.

You could also change the driver to use a bitmask of HW workaround
cases, that gets set very early in the probe.  And the HW
workarounds are keyed on those bit masks (one test) compared
to the direct ID checks (potentially multiple compares).

It would be both cleaner and more efficient.  And you could still
use the direct constants without any real loss of clarity.