Patchwork [net,v2] be2net: Warn users of possible broken functionality on BE2 cards with very old FW versions with latest driver

login
register
mail settings
Submitter Somnath Kotur
Date Oct. 3, 2013, 10:04 a.m.
Message ID <cca19523-3f51-4f87-b0e1-2ecec6ba37d4@CMEXHTCAS1.ad.emulex.com>
Download mbox | patch
Permalink /patch/280250/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Somnath Kotur - Oct. 3, 2013, 10:04 a.m.
On very old FW versions < 4.0, the mailbox command to set interrupts
on the card succeeds even though it is not supported and should have
failed, leading to a scenario where interrupts do not work.
Hence warn users to upgrade to a suitable FW version to avoid seeing
broken functionality.

Signed-off-by: Somnath Kotur <somnath.kotur@emulex.com>
---
v2: Replaced all occurences of 'F/W' with 'FW' or 'Firmware' as suggested by
Sathya

 drivers/net/ethernet/emulex/benet/be_main.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
David Miller - Oct. 7, 2013, 4:31 p.m.
From: Somnath Kotur <somnath.kotur@emulex.com>
Date: Thu, 3 Oct 2013 15:34:29 +0530

> +	if (BE2_chip(adapter) && memcmp(adapter->fw_ver, "4.", 2) < 0) {
> +		dev_err(dev, "Firmware version is too old.IRQs may not work\n");

So many grammatical mistakes in one line.

First sentence got a period, second one did not.

Missing space between period and second sentence.
--
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
Joe Perches - Oct. 7, 2013, 4:44 p.m.
On Mon, 2013-10-07 at 12:31 -0400, David Miller wrote:
> From: Somnath Kotur <somnath.kotur@emulex.com>
> Date: Thu, 3 Oct 2013 15:34:29 +0530
> > +     if (BE2_chip(adapter) && memcmp(adapter->fw_ver, "4.", 2) < 0) {
> > +             dev_err(dev, "Firmware version is too old.IRQs may not work\n");
> 
> So many grammatical mistakes in one line.
> 
> First sentence got a period, second one did not.
> 
> Missing space between period and second sentence.

Periods are unnecessary here.
Just use a comma or a dash.

Also, it might be nicer to show the current firmware version too.

Maybe:

		dev_err(dev, "Firmware version is '%s' - Version 5 or higher is required for properly functional IRQs\n",
			adapter->fw_ver);

--
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
Ivan Vecera - Oct. 16, 2013, 2:59 p.m.
On 10/07/2013 06:31 PM, David Miller wrote:
> From: Somnath Kotur <somnath.kotur@emulex.com>
> Date: Thu, 3 Oct 2013 15:34:29 +0530
>
>> +	if (BE2_chip(adapter) && memcmp(adapter->fw_ver, "4.", 2) < 0) {
>> +		dev_err(dev, "Firmware version is too old.IRQs may not work\n");
>
> So many grammatical mistakes in one line.
>
> First sentence got a period, second one did not.
>
> Missing space between period and second sentence.
> --
> 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
>
Som, any plan to send v3?

Ivan
--
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
Somnath Kotur - Oct. 17, 2013, 5:41 a.m.
> -----Original Message-----
> From: Ivan Vecera [mailto:ivecera@redhat.com]
> Sent: Wednesday, October 16, 2013 8:30 PM
> To: Somnath Kotur
> Cc: David Miller; netdev@vger.kernel.org
> Subject: Re: [PATCH net v2] be2net: Warn users of possible broken
> functionality on BE2 cards with very old FW versions with latest driver
> 
> On 10/07/2013 06:31 PM, David Miller wrote:
> > From: Somnath Kotur <somnath.kotur@emulex.com>
> > Date: Thu, 3 Oct 2013 15:34:29 +0530
> >
> >> +	if (BE2_chip(adapter) && memcmp(adapter->fw_ver, "4.", 2) < 0) {
> >> +		dev_err(dev, "Firmware version is too old.IRQs may not
> work\n");
> >
> > So many grammatical mistakes in one line.
> >
> > First sentence got a period, second one did not.
> >
> > Missing space between period and second sentence.
> > --
> > 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
> >
> Som, any plan to send v3?
> 
> Ivan
HI Ivan,
   Yes , the problem was I was trying to stick within the 80 chars line limit , the missing space above would have pushed it over
needlessly warranting an extra line to accommodate a character.
Will rework the sentence using Joe Perches's suggestions as well as address Ben Hutching's concerns.
Stay tuned.

Thanks
Som
--
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/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 2c38cc4..9563ced 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -3247,6 +3247,11 @@  static int be_setup(struct be_adapter *adapter)
 
 	be_cmd_get_fw_ver(adapter, adapter->fw_ver, adapter->fw_on_flash);
 
+	if (BE2_chip(adapter) && memcmp(adapter->fw_ver, "4.", 2) < 0) {
+		dev_err(dev, "Firmware version is too old.IRQs may not work\n");
+		dev_err(dev, "Pls upgrade firmware to version >= 4.0\n");
+	}
+
 	if (adapter->vlans_added)
 		be_vid_config(adapter);