Patchwork be2net: Bug fix to support newer generation of BE ASIC

login
register
mail settings
Submitter Ajit Khaparde
Date Jan. 28, 2010, 7:56 a.m.
Message ID <20100128075633.GA4785@serverengines.com>
Download mbox | patch
Permalink /patch/43852/
State Accepted
Delegated to: David Miller
Headers show

Comments

Ajit Khaparde - Jan. 28, 2010, 7:56 a.m.
Bug fix in be2net for newer generation of BladeEngine ASIC.

Signed-off-by: Ajit Khaparde <ajitk@serverengines.com>
---
 drivers/net/benet/be.h      |    5 +++++
 drivers/net/benet/be_cmds.h |    3 ++-
 drivers/net/benet/be_main.c |   25 +++++++++++++++++++++++--
 3 files changed, 30 insertions(+), 3 deletions(-)
Ben Hutchings - Jan. 28, 2010, 10:40 p.m.
On Thu, 2010-01-28 at 13:26 +0530, Ajit Khaparde wrote:
> Bug fix in be2net for newer generation of BladeEngine ASIC.
[...]
> @@ -2162,6 +2168,7 @@ static int be_stats_init(struct be_adapter *adapter)
>  	cmd->va = pci_alloc_consistent(adapter->pdev, cmd->size, &cmd->dma);
>  	if (cmd->va == NULL)
>  		return -1;
> +	memset(cmd->va, cmd->size, 0);
>  	return 0;
>  }
>  
[...]

I don't think this does what you think it does.

Also, you should either send this patch to stable@kernel.org, or NAK the
addition of PCI ids for BladeEngine 3 in 2.6.32.7.

Ben.
Greg KH - Jan. 28, 2010, 10:51 p.m.
On Thu, Jan 28, 2010 at 10:40:29PM +0000, Ben Hutchings wrote:
> On Thu, 2010-01-28 at 13:26 +0530, Ajit Khaparde wrote:
> > Bug fix in be2net for newer generation of BladeEngine ASIC.
> [...]
> > @@ -2162,6 +2168,7 @@ static int be_stats_init(struct be_adapter *adapter)
> >  	cmd->va = pci_alloc_consistent(adapter->pdev, cmd->size, &cmd->dma);
> >  	if (cmd->va == NULL)
> >  		return -1;
> > +	memset(cmd->va, cmd->size, 0);

Heh, that's funny.  Yeah, that is not correct at all.  If that does
anything, something else is seriously wrong :)

> >  	return 0;
> >  }
> >  
> [...]
> 
> I don't think this does what you think it does.
> 
> Also, you should either send this patch to stable@kernel.org, or NAK the
> addition of PCI ids for BladeEngine 3 in 2.6.32.7.

Why?  How does the pci id patch for 2.6.32.7 matter with this change?

confused,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings - Jan. 28, 2010, 11:20 p.m.
On Thu, 2010-01-28 at 14:51 -0800, Greg KH wrote:
> On Thu, Jan 28, 2010 at 10:40:29PM +0000, Ben Hutchings wrote:
[...]
> > Also, you should either send this patch to stable@kernel.org, or NAK the
> > addition of PCI ids for BladeEngine 3 in 2.6.32.7.
> 
> Why?  How does the pci id patch for 2.6.32.7 matter with this change?

Because it looks like the devices being added won't work without this.

Ben.
Greg KH - Jan. 28, 2010, 11:28 p.m.
On Thu, Jan 28, 2010 at 11:20:28PM +0000, Ben Hutchings wrote:
> On Thu, 2010-01-28 at 14:51 -0800, Greg KH wrote:
> > On Thu, Jan 28, 2010 at 10:40:29PM +0000, Ben Hutchings wrote:
> [...]
> > > Also, you should either send this patch to stable@kernel.org, or NAK the
> > > addition of PCI ids for BladeEngine 3 in 2.6.32.7.
> > 
> > Why?  How does the pci id patch for 2.6.32.7 matter with this change?
> 
> Because it looks like the devices being added won't work without this.

Hm, what is "this"?  I seem to have been lost on this thread.

Care to explain it better?  2.6.32.7 is now out with the pci ids, so if
I need to add something else for the next .32-stable kernel, please let
me know.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings - Jan. 28, 2010, 11:55 p.m.
On Thu, 2010-01-28 at 15:28 -0800, Greg KH wrote:
> On Thu, Jan 28, 2010 at 11:20:28PM +0000, Ben Hutchings wrote:
> > On Thu, 2010-01-28 at 14:51 -0800, Greg KH wrote:
> > > On Thu, Jan 28, 2010 at 10:40:29PM +0000, Ben Hutchings wrote:
> > [...]
> > > > Also, you should either send this patch to stable@kernel.org, or NAK the
> > > > addition of PCI ids for BladeEngine 3 in 2.6.32.7.
> > > 
> > > Why?  How does the pci id patch for 2.6.32.7 matter with this change?
> > 
> > Because it looks like the devices being added won't work without this.
> 
> Hm, what is "this"?  I seem to have been lost on this thread.

The patch I was replying to
<http://article.gmane.org/gmane.linux.network/150441> changes I/O setup
for be2net to use PCI BAR 0 on the new device whereas it uses BAR 1 on
older devices.

> Care to explain it better?  2.6.32.7 is now out with the pci ids, so if
> I need to add something else for the next .32-stable kernel, please let
> me know.

Well that will be for Ajit to sort out.

Ben.
Ajit Khaparde - Jan. 29, 2010, 5:51 a.m.
On 28/01/10 23:55 +0000, Ben Hutchings wrote:
> On Thu, 2010-01-28 at 15:28 -0800, Greg KH wrote:
> > On Thu, Jan 28, 2010 at 11:20:28PM +0000, Ben Hutchings wrote:
> > > On Thu, 2010-01-28 at 14:51 -0800, Greg KH wrote:
> > > > On Thu, Jan 28, 2010 at 10:40:29PM +0000, Ben Hutchings wrote:
> > > [...]
> > > > > Also, you should either send this patch to stable@kernel.org, or NAK the
> > > > > addition of PCI ids for BladeEngine 3 in 2.6.32.7.
> > > > 
> > > > Why?  How does the pci id patch for 2.6.32.7 matter with this change?
> > > 
> > > Because it looks like the devices being added won't work without this.
> > 
> > Hm, what is "this"?  I seem to have been lost on this thread.
> 
> The patch I was replying to
> <http://article.gmane.org/gmane.linux.network/150441> changes I/O setup
> for be2net to use PCI BAR 0 on the new device whereas it uses BAR 1 on
> older devices.
> 
> > Care to explain it better?  2.6.32.7 is now out with the pci ids, so if
> > I need to add something else for the next .32-stable kernel, please let
> > me know.
> 
> Well that will be for Ajit to sort out.

My apologies for the confusion.
I was out of action for almost a month. As I got back few days back,
I noticed that the patch to include the new PCI IDs was pushed to
the stable kernel. So, I gathered the changes necessary for supporting the
new generation of ASIC, that were made in my absence and spun this patch.

When I mailed the patch, I was not sure when to CC stable@kernel.org
We would like to have the PCI Ids for BladeEngine 3 in 2.6.32 kernel.

Dave,
I plan rectify the error pointed by Ben and Greg and spin the patch again and
resend it.  Please let me know if you think otherwise.

Thanks
Ajit
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Jan. 29, 2010, 5:56 a.m.
From: Ajit Khaparde <ajitkhaparde@gmail.com>
Date: Fri, 29 Jan 2010 11:21:59 +0530

> I plan rectify the error pointed by Ben and Greg and spin the patch again and
> resend it.  Please let me know if you think otherwise.

I already made the obvious memset() arg fix in my tree
as I said I would in my reply to Ben's posting.

It never makes sense to 'respin' patches I've already
applied to net-2.6, once I push it out to kernel.org
it's irreversable so you must send relative fixes.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ajit Khaparde - Jan. 29, 2010, 6:47 a.m.
On 28/01/10 21:56 -0800, David Miller wrote:
> From: Ajit Khaparde <ajitkhaparde@gmail.com>
> Date: Fri, 29 Jan 2010 11:21:59 +0530
> 
> > I plan rectify the error pointed by Ben and Greg and spin the patch again and
> > resend it.  Please let me know if you think otherwise.
> 
> I already made the obvious memset() arg fix in my tree
> as I said I would in my reply to Ben's posting.
I never saw this mail. Probably its lost for good.
It did not even show up on
http://patchwork.ozlabs.org/patch/43852

Thanks
> 
> It never makes sense to 'respin' patches I've already
> applied to net-2.6, once I push it out to kernel.org
> it's irreversable so you must send relative fixes.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Jan. 29, 2010, 7:03 a.m.
From: Ben Hutchings <ben@decadent.org.uk>
Date: Thu, 28 Jan 2010 22:40:29 +0000

> On Thu, 2010-01-28 at 13:26 +0530, Ajit Khaparde wrote:
>> Bug fix in be2net for newer generation of BladeEngine ASIC.
> [...]
>> @@ -2162,6 +2168,7 @@ static int be_stats_init(struct be_adapter *adapter)
>>  	cmd->va = pci_alloc_consistent(adapter->pdev, cmd->size, &cmd->dma);
>>  	if (cmd->va == NULL)
>>  		return -1;
>> +	memset(cmd->va, cmd->size, 0);
>>  	return 0;
>>  }
>>  
> [...]
> 
> I don't think this does what you think it does.

I'll fix this.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Jan. 29, 2010, 7:05 a.m.
From: Ajit Khaparde <ajitk@serverengines.com>
Date: Fri, 29 Jan 2010 12:17:49 +0530

> On 28/01/10 21:56 -0800, David Miller wrote:
>> From: Ajit Khaparde <ajitkhaparde@gmail.com>
>> Date: Fri, 29 Jan 2010 11:21:59 +0530
>> 
>> > I plan rectify the error pointed by Ben and Greg and spin the patch again and
>> > resend it.  Please let me know if you think otherwise.
>> 
>> I already made the obvious memset() arg fix in my tree
>> as I said I would in my reply to Ben's posting.
> I never saw this mail. Probably its lost for good.
> It did not even show up on
> http://patchwork.ozlabs.org/patch/43852

Ugh, it was stuck in my email client waiting for me to hit
'send', which I just did :-)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/benet/be.h b/drivers/net/benet/be.h
index 9fd8e5e..5bc7459 100644
--- a/drivers/net/benet/be.h
+++ b/drivers/net/benet/be.h
@@ -276,8 +276,13 @@  struct be_adapter {
 	int link_speed;
 	u8 port_type;
 	u8 transceiver;
+	u8 generation;		/* BladeEngine ASIC generation */
 };
 
+/* BladeEngine Generation numbers */
+#define BE_GEN2 2
+#define BE_GEN3 3
+
 extern const struct ethtool_ops be_ethtool_ops;
 
 #define drvr_stats(adapter)		(&adapter->stats.drvr_stats)
diff --git a/drivers/net/benet/be_cmds.h b/drivers/net/benet/be_cmds.h
index c002b83..13b33c8 100644
--- a/drivers/net/benet/be_cmds.h
+++ b/drivers/net/benet/be_cmds.h
@@ -164,7 +164,8 @@  struct be_cmd_req_hdr {
 	u8 domain;		/* dword 0 */
 	u32 timeout;		/* dword 1 */
 	u32 request_length;	/* dword 2 */
-	u32 rsvd;		/* dword 3 */
+	u8 version;		/* dword 3 */
+	u8 rsvd[3];		/* dword 3 */
 };
 
 #define RESP_HDR_INFO_OPCODE_SHIFT	0	/* bits 0 - 7 */
diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index 33ab8c7..90c5661 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -2051,6 +2051,7 @@  static void be_unmap_pci_bars(struct be_adapter *adapter)
 static int be_map_pci_bars(struct be_adapter *adapter)
 {
 	u8 __iomem *addr;
+	int pcicfg_reg;
 
 	addr = ioremap_nocache(pci_resource_start(adapter->pdev, 2),
 			pci_resource_len(adapter->pdev, 2));
@@ -2064,8 +2065,13 @@  static int be_map_pci_bars(struct be_adapter *adapter)
 		goto pci_map_err;
 	adapter->db = addr;
 
-	addr = ioremap_nocache(pci_resource_start(adapter->pdev, 1),
-			pci_resource_len(adapter->pdev, 1));
+	if (adapter->generation == BE_GEN2)
+		pcicfg_reg = 1;
+	else
+		pcicfg_reg = 0;
+
+	addr = ioremap_nocache(pci_resource_start(adapter->pdev, pcicfg_reg),
+			pci_resource_len(adapter->pdev, pcicfg_reg));
 	if (addr == NULL)
 		goto pci_map_err;
 	adapter->pcicfg = addr;
@@ -2162,6 +2168,7 @@  static int be_stats_init(struct be_adapter *adapter)
 	cmd->va = pci_alloc_consistent(adapter->pdev, cmd->size, &cmd->dma);
 	if (cmd->va == NULL)
 		return -1;
+	memset(cmd->va, cmd->size, 0);
 	return 0;
 }
 
@@ -2240,6 +2247,20 @@  static int __devinit be_probe(struct pci_dev *pdev,
 		goto rel_reg;
 	}
 	adapter = netdev_priv(netdev);
+
+	switch (pdev->device) {
+	case BE_DEVICE_ID1:
+	case OC_DEVICE_ID1:
+		adapter->generation = BE_GEN2;
+		break;
+	case BE_DEVICE_ID2:
+	case OC_DEVICE_ID2:
+		adapter->generation = BE_GEN3;
+		break;
+	default:
+		adapter->generation = 0;
+	}
+
 	adapter->pdev = pdev;
 	pci_set_drvdata(pdev, adapter);
 	adapter->netdev = netdev;