| Message ID | 1324431172.2844.147.camel@deadeye |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
I accidentally left signing enabled for this one, which I had intended to disable since you've said that patchwork doesn't deal with signatures. Except this looks OK on patchwork: <http://patchwork.ozlabs.org/patch/132548/>. If that also looks OK to you, is there any reason to avoid signing networking patches? Ben.
From: Ben Hutchings <ben@decadent.org.uk> Date: Wed, 21 Dec 2011 01:32:52 +0000 > - dbg(DBG_RX, "In fst_rx_dma %lx %lx %d\n", > - (unsigned long) skb, (unsigned long) mem, len); > + dbg(DBG_RX, "In fst_rx_dma %x %x %d\n", (u32)skb, mem, len); This is more appropriately fixed by using "%p" instead of casting to a 32-bit int. -- 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
From: Ben Hutchings <ben@decadent.org.uk> Date: Wed, 21 Dec 2011 01:42:26 +0000 > I accidentally left signing enabled for this one, which I had intended > to disable since you've said that patchwork doesn't deal with > signatures. Except this looks OK on patchwork: > <http://patchwork.ozlabs.org/patch/132548/>. > > If that also looks OK to you, is there any reason to avoid signing > networking patches? What do you mean exactly by "signing". Do you mean simply the "Signed-off-by: " tag, which you should always add to your own patches, or something else? -- 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
On Tue, 2011-12-20 at 23:46 -0500, David Miller wrote: > From: Ben Hutchings <ben@decadent.org.uk> > Date: Wed, 21 Dec 2011 01:32:52 +0000 > > > - dbg(DBG_RX, "In fst_rx_dma %lx %lx %d\n", > > - (unsigned long) skb, (unsigned long) mem, len); > > + dbg(DBG_RX, "In fst_rx_dma %x %x %d\n", (u32)skb, mem, len); > > This is more appropriately fixed by using "%p" instead of casting > to a 32-bit int. 'skb' is the physical address of the data in the skb. Whereas 'mem' is the offset of the corresponding buffer in shared memory. All completely clear, right? ;-) Ben.
On Tue, 2011-12-20 at 23:48 -0500, David Miller wrote: > From: Ben Hutchings <ben@decadent.org.uk> > Date: Wed, 21 Dec 2011 01:42:26 +0000 > > > I accidentally left signing enabled for this one, which I had intended > > to disable since you've said that patchwork doesn't deal with > > signatures. Except this looks OK on patchwork: > > <http://patchwork.ozlabs.org/patch/132548/>. > > > > If that also looks OK to you, is there any reason to avoid signing > > networking patches? > > What do you mean exactly by "signing". > > Do you mean simply the "Signed-off-by: " tag, which you should always > add to your own patches, or something else? GPG signing. Ben.
Hi,
Thanks for the patches. I will apply them to the master copy of the source code here.
Regards
Kevin Curtis
Linux Development
FarSite Communications Ltd http://www.farsite.com
Winner of The Queen's Award for Enterprise 2009
tel: +44 1256 330461
fax: +44 1256 854931
-----Original Message-----
From: Ben Hutchings [mailto:ben@decadent.org.uk]
Sent: 21 December 2011 05:41
To: David Miller
Cc: Kevin Curtis; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
On Tue, 2011-12-20 at 23:46 -0500, David Miller wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Wed, 21 Dec 2011 01:32:52 +0000
>
> > - dbg(DBG_RX, "In fst_rx_dma %lx %lx %d\n",
> > - (unsigned long) skb, (unsigned long) mem, len);
> > + dbg(DBG_RX, "In fst_rx_dma %x %x %d\n", (u32)skb, mem, len);
>
> This is more appropriately fixed by using "%p" instead of casting to a
> 32-bit int.
'skb' is the physical address of the data in the skb. Whereas 'mem' is the offset of the corresponding buffer in shared memory.
All completely clear, right? ;-)
Ben.
--
Ben Hutchings
Humans are not rational beings; they are rationalising beings.
diff --git a/drivers/net/wan/farsync.c b/drivers/net/wan/farsync.c index ebb9f24..98c04c6 100644 --- a/drivers/net/wan/farsync.c +++ b/drivers/net/wan/farsync.c @@ -886,15 +886,13 @@ fst_rx_dma_complete(struct fst_card_info *card, struct fst_port_info *port, * Receive a frame through the DMA */ static inline void -fst_rx_dma(struct fst_card_info *card, dma_addr_t skb, - dma_addr_t mem, int len) +fst_rx_dma(struct fst_card_info *card, dma_addr_t skb, u32 mem, int len) { /* * This routine will setup the DMA and start it */ - dbg(DBG_RX, "In fst_rx_dma %lx %lx %d\n", - (unsigned long) skb, (unsigned long) mem, len); + dbg(DBG_RX, "In fst_rx_dma %x %x %d\n", (u32)skb, mem, len); if (card->dmarx_in_progress) { dbg(DBG_ASS, "In fst_rx_dma while dma in progress\n"); } @@ -915,20 +913,19 @@ fst_rx_dma(struct fst_card_info *card, dma_addr_t skb, * Send a frame through the DMA */ static inline void -fst_tx_dma(struct fst_card_info *card, unsigned char *skb, - unsigned char *mem, int len) +fst_tx_dma(struct fst_card_info *card, dma_addr_t skb, u32 mem, int len) { /* * This routine will setup the DMA and start it. */ - dbg(DBG_TX, "In fst_tx_dma %p %p %d\n", skb, mem, len); + dbg(DBG_TX, "In fst_tx_dma %x %x %d\n", (u32)skb, mem, len); if (card->dmatx_in_progress) { dbg(DBG_ASS, "In fst_tx_dma while dma in progress\n"); } - outl((unsigned long) skb, card->pci_conf + DMAPADR1); /* Copy from here */ - outl((unsigned long) mem, card->pci_conf + DMALADR1); /* to here */ + outl(skb, card->pci_conf + DMAPADR1); /* Copy from here */ + outl(mem, card->pci_conf + DMALADR1); /* to here */ outl(len, card->pci_conf + DMASIZ1); /* for this length */ outl(0x000000004, card->pci_conf + DMADPR1); /* In this direction */ @@ -1405,9 +1402,7 @@ do_bottom_half_tx(struct fst_card_info *card) card->dma_len_tx = skb->len; card->dma_txpos = port->txpos; fst_tx_dma(card, - (char *) card-> - tx_dma_handle_card, - (char *) + card->tx_dma_handle_card, BUF_OFFSET(txBuffer[pi] [port->txpos][0]), skb->len);
Consistently use dma_addr_t for DMA addresses, except in fst_{rx,tx}_dma where we truncate them to 32 bits. (We know this is OK because that's the default DMA mask for PCI devices.) Consistently use u32 for buffer offsets within shared memory. Compile-tested only. Signed-off-by: Ben Hutchings <ben@decadent.org.uk> --- drivers/net/wan/farsync.c | 19 +++++++------------ 1 files changed, 7 insertions(+), 12 deletions(-)