[PATCH] bna: Implement ethtool flash_device entry point.

From: Krishna Gudipati <redacted>

Change details:
	- Implement ethtool flash_device() entry point to write the
	  firmware image to the flash firmware partition.

Signed-off-by: Krishna Gudipati <redacted>
---
 drivers/net/ethernet/brocade/bna/bfa_defs.h     |    1 +
 drivers/net/ethernet/brocade/bna/bnad_ethtool.c |   36 +++++++++++++++++++++++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/brocade/bna/bfa_defs.h b/drivers/net/ethernet/brocade/bna/bfa_defs.h
index 871c630..48f8773 100644
--- a/drivers/net/ethernet/brocade/bna/bfa_defs.h
+++ b/drivers/net/ethernet/brocade/bna/bfa_defs.h

@@ -297,6 +297,7 @@ enum bfa_mode {
 #define BFA_FLASH_PART_ENTRY_SIZE	32	/* partition entry size */
 #define BFA_FLASH_PART_MAX		32	/* maximal # of partitions */
 #define BFA_TOTAL_FLASH_SIZE		0x400000
+#define BFA_FLASH_PART_FWIMG		2
 #define BFA_FLASH_PART_MFG		7
 
 /*

diff --git a/drivers/net/ethernet/brocade/bna/bnad_ethtool.c b/drivers/net/ethernet/brocade/bna/bnad_ethtool.c
index 9b44ec8..3e18ee7 100644
--- a/drivers/net/ethernet/brocade/bna/bnad_ethtool.c
+++ b/drivers/net/ethernet/brocade/bna/bnad_ethtool.c

@@ -1072,6 +1072,41 @@ done:
 	return ret;
 }
 
+static int
+bnad_flash_device(struct net_device *netdev, struct ethtool_flash *eflash)
+{
+	struct bnad *bnad = netdev_priv(netdev);
+	struct bnad_iocmd_comp fcomp;
+	const struct firmware *fw;
+	unsigned long flags = 0;
+	int ret = 0;
+
+	if (request_firmware(&fw, eflash->data, &bnad->pcidev->dev)) {
+		pr_err("Can't locate firmware %s\n", eflash->data);
+		goto out;
+	}
+
+	fcomp.bnad = bnad;
+	fcomp.comp_status = 0;
+
+	init_completion(&fcomp.comp);
+	spin_lock_irqsave(&bnad->bna_lock, flags);
+	ret = bfa_nw_flash_update_part(&bnad->bna.flash, BFA_FLASH_PART_FWIMG,
+				bnad->id, (u8 *)fw->data, fw->size, 0,
+				bnad_cb_completion, &fcomp);
+	if (ret != BFA_STATUS_OK) {
+		spin_unlock_irqrestore(&bnad->bna_lock, flags);
+		goto out;
+	}
+
+	spin_unlock_irqrestore(&bnad->bna_lock, flags);
+	wait_for_completion(&fcomp.comp);
+	ret = fcomp.comp_status;
+out:
+	release_firmware(fw);
+	return ret;
+}
+
 static const struct ethtool_ops bnad_ethtool_ops = {
 	.get_settings = bnad_get_settings,
 	.set_settings = bnad_set_settings,

@@ -1090,6 +1125,7 @@ static const struct ethtool_ops bnad_ethtool_ops = {
 	.get_eeprom_len = bnad_get_eeprom_len,
 	.get_eeprom = bnad_get_eeprom,
 	.set_eeprom = bnad_set_eeprom,
+	.flash_device = bnad_flash_device,
 };
 
 void

-- 
1.7.3.rc1

On Tue, 2012-01-31 at 16:06 -0800, kgudipat@brocade.com wrote:

This results in returning 0.

+	}
+
+	fcomp.bnad = bnad;
+	fcomp.comp_status = 0;
+
+	init_completion(&fcomp.comp);
+	spin_lock_irqsave(&bnad->bna_lock, flags);

No need to save flags, this is not called in IRQ context!

+	ret = bfa_nw_flash_update_part(&bnad->bna.flash, BFA_FLASH_PART_FWIMG,
+				bnad->id, (u8 *)fw->data, fw->size, 0,
+				bnad_cb_completion, &fcomp);
+	if (ret != BFA_STATUS_OK) {
+		spin_unlock_irqrestore(&bnad->bna_lock, flags);
+		goto out;
+	}
+
+	spin_unlock_irqrestore(&bnad->bna_lock, flags);
+	wait_for_completion(&fcomp.comp);
+	ret = fcomp.comp_status;
+out:
+	release_firmware(fw);
+	return ret;

It looks like the value of ret will be an enumerator from enum
bfa_status and not a Linux error code.  In that case you need to convert
it before returning.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Thanks Ben for the review.

I am making the changes as per the review comments to return proper error code.

+	init_completion(&fcomp.comp);
+	spin_lock_irqsave(&bnad->bna_lock, flags);

No need to save flags, this is not called in IRQ context!

[Krishna]: bna_lock is used in Interrupt context as well, by using spin_lock_irq instead of
	     spin_lock_irqsave() - we will lose the state of the interrupts & will
	     unconditionally enable interrupts. We plan to stay with using spin_lock_irqsave()
	     as it's safe.

Thanks,
Krishna.

On Wed, 2012-02-01 at 14:10 -0800, Krishna Gudipati wrote:

Thanks Ben for the review.

I am making the changes as per the review comments to return proper error code.

quoted

+	init_completion(&fcomp.comp);
+	spin_lock_irqsave(&bnad->bna_lock, flags);

No need to save flags, this is not called in IRQ context!

[Krishna]: bna_lock is used in Interrupt context as well, by using spin_lock_irq instead of
	     spin_lock_irqsave() - we will lose the state of the interrupts & will
	     unconditionally enable interrupts. We plan to stay with using spin_lock_irqsave()
	     as it's safe.

bnad_flash_device() will be called in process context, therefore it is
correct to unconditionally enable interrupts.  It is safe but odd to use
spin_lock_irqsave()/spin_lock_irqrestore() in such a function.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.