spidernet: check if firmware was loaded correctly

We changed the name of the Kconfig symbols along with the move to arch/powerpc. This one hunk got lost during the conversion.

From: [email protected]

Index: linux-2.6.15-rc1/drivers/net/Kconfig
===================================================================
@@ -2120,7 +2120,7 @@ config BNX2
 
 config SPIDER_NET
 	tristate "Spider Gigabit Ethernet driver"
-	depends on PCI && PPC_BPA
+	depends on PCI && PPC_CELL
 	help
 	  This driver supports the Gigabit Ethernet chips present on the
 	  Cell Processor-Based Blades from IBM.

--

Uploading the device firmware may fail if wrong input data was provided by the user. This checks for the condition.

From: [email protected]

Index: linux-2.6.15-rc/drivers/net/spider_net.c
===================================================================
@@ -1836,7 +1836,7 @@ spider_net_setup_phy(struct spider_net_c
  * spider_net_download_firmware loads the firmware opened by
  * spider_net_init_firmware into the adapter.
  */
-static void
+static int 
 spider_net_download_firmware(struct spider_net_card *card,
 			     const struct firmware *firmware)
 {

@@ -1857,8 +1857,13 @@ spider_net_download_firmware(struct spid
 		}
 	}
 
+	if (spider_net_read_reg(card, SPIDER_NET_GSINIT))
+		return -EIO;
+	
 	spider_net_write_reg(card, SPIDER_NET_GSINIT,
 			     SPIDER_NET_RUN_SEQ_VALUE);
+
+	return 0;
 }
 
 /**

@@ -1909,9 +1914,8 @@ spider_net_init_firmware(struct spider_n
 		goto out;
 	}
 
-	spider_net_download_firmware(card, firmware);
-
-	err = 0;
+	if (!spider_net_download_firmware(card, firmware))
+		err = 0;
 out:
 	release_firmware(firmware);
 

Index: linux-2.6.15-rc/drivers/net/spider_net.h
===================================================================
@@ -155,7 +155,7 @@ extern char spider_net_driver_name[];
 /* set this first, then the FRAMENUM_VALUE */
 #define SPIDER_NET_GFXFRAMES_VALUE	0x00000000
 
-#define SPIDER_NET_STOP_SEQ_VALUE	0x00000000
+#define SPIDER_NET_STOP_SEQ_VALUE	0x007e0000
 #define SPIDER_NET_RUN_SEQ_VALUE	0x0000007e
 
 #define SPIDER_NET_PHY_CTRL_VALUE	0x00040040

--

This one should be sent to Jeff Garzik, along with patches 11, 13 and 14.

Paul.

Ok.

Jens, is it ok for you if you send the network driver stuff to [email protected], Cc: [email protected] yourself in the future?

	Arnd <><

Arnd Bergmann <[email protected]> wrote on 12/06/2005 11:23:39 AM:

Sure, I will do so for our next updates.

Jens

request_firmware() is sometimes problematic, especially in initramfs, reading the firmware from Open Firmware is much preferrable.

We still try to get the firmware from the file system first, in order to support old SLOF releases and to allow updates of the spidernet firmware without reflashing the system.

From: [email protected]

Index: linux-2.6.15-rc/drivers/net/spider_net.c
===================================================================
@@ -1895,16 +1895,27 @@ spider_net_download_firmware(struct spid
 static int
 spider_net_init_firmware(struct spider_net_card *card)
 {
-	const struct firmware *firmware;
+	struct firmware *firmware;
+	struct device_node *dn;
+	u8 *fw_prop;
 	int err = -EIO;
 
-	if (request_firmware(&firmware,
+	if (request_firmware((const struct firmware **)&firmware,
 			     SPIDER_NET_FIRMWARE_NAME, &card->pdev->dev) < 0) {
 		if (netif_msg_probe(card))
 			pr_err("Couldn't read in sequencer data file %s.\n",
 			       SPIDER_NET_FIRMWARE_NAME);
-		firmware = NULL;
-		goto out;
+
+		dn = pci_device_to_OF_node(card->pdev);
+		if (!dn)
+			goto out;
+
+		fw_prop = (u8 *)get_property(dn, "firmware", NULL);
+		if (!fw_prop)
+			goto out;
+
+		memcpy(firmware->data, fw_prop, 6 * SPIDER_NET_FIRMWARE_LEN * sizeof(u32));
+		firmware->size = 6 * SPIDER_NET_FIRMWARE_LEN * sizeof(u32);
 	}
 
 	if (firmware->size != 6 * SPIDER_NET_FIRMWARE_LEN * sizeof(u32)) {

--

The driver incorrectly used dma_addr_t to describe HW structures and consequently broke when that type was changed in 2.6.15-rc.

This changed spidernet to use u32 for 32 bit HW defined structure elements.

From: [email protected]

Index: linux-2.6.15-rc/drivers/net/spider_net.h
===================================================================
@@ -373,9 +373,9 @@ enum spider_net_descr_status {
 
 struct spider_net_descr {
 	/* as defined by the hardware */
-	dma_addr_t buf_addr;
+	u32 buf_addr;
 	u32 buf_size;
-	dma_addr_t next_descr_addr;
+	u32 next_descr_addr;
 	u32 dmac_cmd_status;
 	u32 result_size;
 	u32 valid_size;	/* all zeroes for tx */

Index: linux-2.6.15-rc/drivers/net/spider_net.c
===================================================================
@@ -480,6 +480,7 @@ static int
 spider_net_prepare_rx_descr(struct spider_net_card *card,
 			    struct spider_net_descr *descr)
 {
+	dma_addr_t buf;
 	int error = 0;
 	int offset;
 	int bufsize;

@@ -510,10 +511,11 @@ spider_net_prepare_rx_descr(struct spide
 	if (offset)
 		skb_reserve(descr->skb, SPIDER_NET_RXBUF_ALIGN - offset);
 	/* io-mmu-map the skb */
-	descr->buf_addr = pci_map_single(card->pdev, descr->skb->data,
+	buf = pci_map_single(card->pdev, descr->skb->data,
 					 SPIDER_NET_MAX_MTU,
 					 PCI_DMA_BIDIRECTIONAL);
-	if (descr->buf_addr == DMA_ERROR_CODE) {
+	descr->buf_addr = buf;
+	if (buf == DMA_ERROR_CODE) {
 		dev_kfree_skb_any(descr->skb);
 		if (netif_msg_rx_err(card))
 			pr_err("Could not iommu-map rx buffer\n");

@@ -914,15 +916,16 @@ spider_net_prepare_tx_descr(struct spide
 			    struct spider_net_descr *descr,
 			    struct sk_buff *skb)
 {
-	descr->buf_addr = pci_map_single(card->pdev, skb->data,
-					 skb->len, PCI_DMA_BIDIRECTIONAL);
-	if (descr->buf_addr == DMA_ERROR_CODE) {
+	dma_addr_t buf = pci_map_single(card->pdev, skb->data,
+					skb->len, PCI_DMA_BIDIRECTIONAL);
+	if (buf == DMA_ERROR_CODE) {
 		if (netif_msg_tx_err(card))
 			pr_err("could not iommu-map packet (%p, %i). "
 				  "Dropping packet\n", skb->data, skb->len);
 		return -ENOMEM;
 	}
 
+	descr->buf_addr = buf;
 	descr->buf_size = skb->len;
 	descr->skb = skb;
 	descr->data_status = 0;

--

Since the discussion came up again in irc, I looked up the existing file systems.

outside of fs/, we have the following file systems.

find -name \*.c | grep -v ^./fs | xargs grep struct.file_system_type.*= ./arch/ia64/kernel/perfmon.c:static struct file_system_type pfm_fs_type = { ./drivers/infiniband/core/uverbs_main.c:static struct file_system_type uverbs_event_fs = { ./drivers/isdn/capi/capifs.c:static struct file_system_type capifs_fs_type = { ./drivers/misc/ibmasm/ibmasmfs.c:static struct file_system_type ibmasmfs_type = { ./drivers/oprofile/oprofilefs.c:static struct file_system_type oprofilefs_type = { ./drivers/usb/core/inode.c:static struct file_system_type usb_fs_type = { ./drivers/usb/gadget/inode.c:static struct file_system_type gadgetfs_type = { ./ipc/mqueue.c:static struct file_system_type mqueue_fs_type = { ./kernel/cpuset.c:static struct file_system_type cpuset_fs_type = { ./kernel/futex.c:static struct file_system_type futex_fs_type = { ./mm/shmem.c:static struct file_system_type tmpfs_fs_type = { ./mm/tiny-shmem.c:static struct file_system_type tmpfs_fs_type = { ./net/socket.c:static struct file_system_type sock_fs_type = { ./net/sunrpc/rpc_pipe.c:static struct file_system_type rpc_pipe_fs_type = { ./security/inode.c:static struct file_system_type fs_type = { ./security/selinux/selinuxfs.c:static struct file_system_type sel_fs_type = {

In fs/, most code deals with actual files stored on a disk or similar, with the exception of:

./fs/binfmt_misc.c:static struct file_system_type bm_fs_type = { ./fs/block_dev.c:static struct file_system_type bd_type = { ./fs/debugfs/inode.c:static struct file_system_type debug_fs_type = { ./fs/devfs/base.c:static struct file_system_type devfs_fs_type = { ./fs/devpts/inode.c:static struct file_system_type devpts_fs_type = { ./fs/eventpoll.c:static struct file_system_type eventpoll_fs_type = { ./fs/hugetlbfs/inode.c:static struct file_system_type hugetlbfs_fs_type = { ./fs/inotify.c:static struct file_system_type inotify_fs_type = { ./fs/openpromfs/inode.c:static struct file_system_type openprom_fs_type = { ./fs/pipe.c:static struct file_system_type pipe_fs_type = { ./fs/proc/root.c:static struct file_system_type proc_fs_type = { ./fs/relayfs/inode.c:static struct file_system_type relayfs_fs_type = { ./fs/sysfs/mount.c:static struct file_system_type sysfs_fs_type = {

I guess there is no strict rule where these file systems go to, e.g. hugetlbs could just as well live near mm/shmem.c or any of those outside of fs/ could be moved in there.

I don't really care where I put spufs, but I would prefer to move the files only one more time at most.
Initially, they were in fs/spufs, and I moved them to arch/powerpc/platforms/cell/spufs at Pekkas suggestion.

	Arnd <><

Hi,

hugetlbs does not contain architecture specific code so I don't see it as a problem.

I would prefer them to stay in arch/powerpc/. As far as I understand, spufs will never have any use for platforms other than cell, so I really don't see any point in putting it in fs/.

Pekka

The point is that people making changes to the filesystem interfaces will be much more likely to notice and fix stuff that is under fs/ than code that is buried deep under arch/ somewhere. Filesystems should go under fs/ for the sake of long-term maintainability. The fact that it's only used on one architecture is irrelevant - you simply make sure (with the appropriate Kconfig bits) that it's only offered on that architecture.

Paul.

Hi Paul,

I think the fact that it is highly architecture specific is relevant. I have no way of testing spufs changes except on cell, no? And if I am developing on a cell, I probably will notice it in arch/ all the same. So I don't quite buy your the maintenace argument.

But as Arnd said, there are no clear rules on what kind of filesystems should go into fs/ so please do whatever you must.

Pekka

Think about someone changing the VFS layer interface and fixing up all the filesystems to accommodate that change. That person is doing some of your work for you, so you want to make it easy for him/her to find your filesystem. That's the sort of thing I was referring to as maintenance.

As for changes on the cell-specific side, the people doing those changes will know where to find it, so it isn't a problem having it in fs/.

Having it in fs/ also means that it is more likely that people familiar with VFS internals will look through your code and comment on it. I know that can be painful in the short term, but in the long term it will lead to better code.

Paul.

Am Dienstag 06 Dezember 2005 23:19 schrieb Paul Mackerras:

Yes, that is an excellent point. How should we proceed to get the code there?
Do you want to move the files around in your git tree or do you prefer me to send a full set of patches again and kill the existing copy? Obviously, I'd prefer the former, since it would mean less work for me with the same result.

	Arnd <><

FWIW, I think it's not a serious argument. Interface changes => grep time. And that means grep over the tree anyway.

That's solved by asking for review...

As far as I'm concerned, the only thing here that looks like a possible reason to move the entire thing is highly unusual semantics of final close and interesting use of VFS interfaces in spu_create(). I.e. it's not that we have a filesystem there.

OTOH, if you go looking for analogs as far as unusual interaction with VFS is concerned... net/unix is unlikely to get moved.

OK, well, where would you prefer the spufs code to go?

Could you review the spufs code (i.e. the patches posted by Arnd recently to [email protected]) please?

Thanks,
Paul.

Hi Paul,

Why not post them to LKML?

Pekka

Up to ppc folks, really - I don't see any serious objections to arch/powerpc/ variants; it could go there, it could go to fs/*. Objections along the lines of "it won't be found" are BS - any interface change is going to start with grep over the entire tree anyway.

If it's what you have in powerpc.git - see comments on IRC yesterday...

openpromfs seems to be a precedent here. It makes sense only on sparc and sparc64 but it lives in fs/.