Re: [PATCH] Jazzsonic driver updates

Re: [PATCH] Jazzsonic driver updates

[Geert Uytterhoeven]

On Fri, 28 Jan 2005, Linux Kernel Mailing List wrote:
> ChangeSet 1.1986, 2005/01/28 00:12:28-05:00, ralf@linux-mips.org
> 
> 	[PATCH] Jazzsonic driver updates
> 	
> 	 o Resurrect the Jazz SONIC driver after years of it not having been tested
> 	 o Convert from Space.c initialization to module_init / platform device.
> 	
> 	Signed-off-by: Jeff Garzik <jgarzik@pobox.com>

> --- a/drivers/net/sonic.c	2005-03-06 18:10:39 -08:00
> +++ b/drivers/net/sonic.c	2005-03-06 18:10:39 -08:00
> @@ -116,7 +116,7 @@
>  	/*
>  	 * Map the packet data into the logical DMA address space
>  	 */
> -	if ((laddr = vdma_alloc(PHYSADDR(skb->data), skb->len)) == ~0UL) {
> +	if ((laddr = vdma_alloc(CPHYSADDR(skb->data), skb->len)) == ~0UL) {
                                ^^^^^^^^^
This part broke compilation for Mac/m68k.

>  		printk("%s: no VDMA entry for transmit available.\n",
>  		       dev->name);
>  		dev_kfree_skb(skb);

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH] Re: [PATCH] Jazzsonic driver updates

[Finn Thain]

On Tue, 22 Mar 2005, Geert Uytterhoeven wrote:

> On Fri, 28 Jan 2005, Linux Kernel Mailing List wrote:
> > ChangeSet 1.1986, 2005/01/28 00:12:28-05:00, ralf@linux-mips.org
> > 
> > 	[PATCH] Jazzsonic driver updates
> > 	
> > 	 o Resurrect the Jazz SONIC driver after years of it not having been tested
> > 	 o Convert from Space.c initialization to module_init / platform device.
> > 	
> > 	Signed-off-by: Jeff Garzik <jgarzik@pobox.com>
> 
> > --- a/drivers/net/sonic.c	2005-03-06 18:10:39 -08:00
> > +++ b/drivers/net/sonic.c	2005-03-06 18:10:39 -08:00
> > @@ -116,7 +116,7 @@
> >  	/*
> >  	 * Map the packet data into the logical DMA address space
> >  	 */
> > -	if ((laddr = vdma_alloc(PHYSADDR(skb->data), skb->len)) == ~0UL) {
> > +	if ((laddr = vdma_alloc(CPHYSADDR(skb->data), skb->len)) == ~0UL) {
>                                 ^^^^^^^^^
> This part broke compilation for Mac/m68k.
> 

Compilation is easily fixed, a patch is below.

BTW, this will still leave the macsonic driver in a non-working state. I 
have a patch to make it work again, which also re-introduces the support 
for 16-bit cards that went missing way back in 2.4.something (jazzsonic 
cards are all 32-bit, as are some mac cards). My patch basically ports the 
current macsonic driver from mac68k 2.2 branch, and has been widely tested 
on macs. Problem is, it hasn't yet been tested on MIPS, and it makes a lot 
of changes to sonic.c, which is shared by jazzsonic and macsonic.

Can anyone with a Jazz machine help out with this?

One of the difficulties is that MIPS and m68k architectures each have 
their own repo, and last time I looked, the SONIC code in the MIPS repo 
didn't agree with the SONIC code in the m68k repo. But, it shouldn't be 
difficult to make macsonic and jazzsonic work with the same core driver 
code again. Besides, we already have two SONIC drivers in the tree, 
seperating jazzsonic and macsonic would add a third.

-f

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>


--- a/drivers/net/macsonic.c	2005-03-25 04:01:51.000000000 +1100
+++ b/drivers/net/macsonic.c	2005-03-25 04:48:39.000000000 +1100
@@ -638,6 +638,7 @@
 #define vdma_free(baz)
 #define sonic_chiptomem(bat) (bat)
 #define PHYSADDR(quux) (quux)
+#define CPHYSADDR(quux) (quux)
 
 #define sonic_request_irq       request_irq
 #define sonic_free_irq          free_irq

Re: [PATCH] Jazzsonic driver updates

[Ralf Baechle]

On Tue, Mar 22, 2005 at 06:13:17PM +0100, Geert Uytterhoeven wrote:

> On Fri, 28 Jan 2005, Linux Kernel Mailing List wrote:
> > ChangeSet 1.1986, 2005/01/28 00:12:28-05:00, ralf@linux-mips.org
> > 
> > 	[PATCH] Jazzsonic driver updates
> > 	
> > 	 o Resurrect the Jazz SONIC driver after years of it not having been tested
> > 	 o Convert from Space.c initialization to module_init / platform device.
> > 	
> > 	Signed-off-by: Jeff Garzik <jgarzik@pobox.com>
> 
> > --- a/drivers/net/sonic.c	2005-03-06 18:10:39 -08:00
> > +++ b/drivers/net/sonic.c	2005-03-06 18:10:39 -08:00
> > @@ -116,7 +116,7 @@
> >  	/*
> >  	 * Map the packet data into the logical DMA address space
> >  	 */
> > -	if ((laddr = vdma_alloc(PHYSADDR(skb->data), skb->len)) == ~0UL) {
> > +	if ((laddr = vdma_alloc(CPHYSADDR(skb->data), skb->len)) == ~0UL) {
>                                 ^^^^^^^^^
> This part broke compilation for Mac/m68k.
> 
> >  		printk("%s: no VDMA entry for transmit available.\n",
> >  		       dev->name);
> >  		dev_kfree_skb(skb);

Oh funny.  vdma_alloc() was created 10 years ago as an internal API for
the Jazz machines.  Didn't realize m68k had cloned it :-)  If anything
it seems this should be converted to the modern DMA API.

  Ralf

Re: [PATCH] Jazzsonic driver updates

[Finn Thain]

On Wed, 23 Mar 2005, Ralf Baechle wrote:

> On Tue, Mar 22, 2005 at 06:13:17PM +0100, Geert Uytterhoeven wrote:
> 
> > On Fri, 28 Jan 2005, Linux Kernel Mailing List wrote:
> > > ChangeSet 1.1986, 2005/01/28 00:12:28-05:00, ralf@linux-mips.org
> > > 
> > > 	[PATCH] Jazzsonic driver updates
> > > 	
> > > 	 o Resurrect the Jazz SONIC driver after years of it not having been tested
> > > 	 o Convert from Space.c initialization to module_init / platform device.
> > > 	
> > > 	Signed-off-by: Jeff Garzik <jgarzik@pobox.com>
> > 
> > > --- a/drivers/net/sonic.c	2005-03-06 18:10:39 -08:00
> > > +++ b/drivers/net/sonic.c	2005-03-06 18:10:39 -08:00
> > > @@ -116,7 +116,7 @@
> > >  	/*
> > >  	 * Map the packet data into the logical DMA address space
> > >  	 */
> > > -	if ((laddr = vdma_alloc(PHYSADDR(skb->data), skb->len)) == ~0UL) {
> > > +	if ((laddr = vdma_alloc(CPHYSADDR(skb->data), skb->len)) == ~0UL) {
> >                                 ^^^^^^^^^
> > This part broke compilation for Mac/m68k.
> > 
> > >  		printk("%s: no VDMA entry for transmit available.\n",
> > >  		       dev->name);
> > >  		dev_kfree_skb(skb);
> 
> Oh funny.  vdma_alloc() was created 10 years ago as an internal API for
> the Jazz machines.  Didn't realize m68k had cloned it :-)

m68k doesn't clone it. This is from macsonic.c,

#define vdma_alloc(foo, bar) ((u32)foo)

> If anything it seems this should be converted to the modern DMA API.

Sure, but until that happens, it would be nice if the upstream kernel had 
the MIPS repo versions of sonic.c, sonic.h and jazzsonic.c. That way it 
might be possible for me to put together a patch for macsonic that both 
archs are happy with.

It doesn't require a new DMA API to fix all the built-in ethernet ports on 
68k Macs.

-f

>   Ralf

Re: [PATCH] Jazzsonic driver updates

[Finn Thain]

On Wed, 23 Mar 2005, Ralf Baechle wrote:

> On Tue, Mar 22, 2005 at 06:13:17PM +0100, Geert Uytterhoeven wrote:
> 
> > On Fri, 28 Jan 2005, Linux Kernel Mailing List wrote:
> > > ChangeSet 1.1986, 2005/01/28 00:12:28-05:00, ralf@linux-mips.org
> > > 
> > > 	[PATCH] Jazzsonic driver updates
> > > 	
> > > 	 o Resurrect the Jazz SONIC driver after years of it not having been tested
> > > 	 o Convert from Space.c initialization to module_init / platform device.
> > > 	
> > > 	Signed-off-by: Jeff Garzik <jgarzik@pobox.com>
> > 
> > > --- a/drivers/net/sonic.c	2005-03-06 18:10:39 -08:00
> > > +++ b/drivers/net/sonic.c	2005-03-06 18:10:39 -08:00
> > > @@ -116,7 +116,7 @@
> > >  	/*
> > >  	 * Map the packet data into the logical DMA address space
> > >  	 */
> > > -	if ((laddr = vdma_alloc(PHYSADDR(skb->data), skb->len)) == ~0UL) {
> > > +	if ((laddr = vdma_alloc(CPHYSADDR(skb->data), skb->len)) == ~0UL) {
> >                                 ^^^^^^^^^
> > This part broke compilation for Mac/m68k.
> > 
> > >  		printk("%s: no VDMA entry for transmit available.\n",
> > >  		       dev->name);
> > >  		dev_kfree_skb(skb);
> 
> Oh funny.  vdma_alloc() was created 10 years ago as an internal API for
> the Jazz machines.  Didn't realize m68k had cloned it :-)  If anything
> it seems this should be converted to the modern DMA API.

I've just started merging my Mac sonic work into 2.6.12-rc6. m68k doesn't 
yet implement the modern DMA API, but it is easy to fake it for a while 
for macsonic.c. So I don't mind converting macsonic, jazzsonic and the 
shared sonic driver core to the new API.

But, I knowing nothing about the Jazz DMA controller. I need some help 
from the MIPS people:

Would I be right to say that vdma_{alloc,free}() can be changed to 
dma_{,un}map_single? The other Jazz specific routine that sonic uses is 
vdma_log2phys, and I don't know if that has a better alternative.

-f

>   Ralf
> -
> To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] Jazzsonic driver updates

[Ralf Baechle]

On Sun, Jun 12, 2005 at 04:05:30PM +1000, Finn Thain wrote:

> > Oh funny.  vdma_alloc() was created 10 years ago as an internal API for
> > the Jazz machines.  Didn't realize m68k had cloned it :-)  If anything
> > it seems this should be converted to the modern DMA API.
> 
> I've just started merging my Mac sonic work into 2.6.12-rc6. m68k doesn't 
> yet implement the modern DMA API, but it is easy to fake it for a while 
> for macsonic.c. So I don't mind converting macsonic, jazzsonic and the 
> shared sonic driver core to the new API.
> 
> But, I knowing nothing about the Jazz DMA controller. I need some help 
> from the MIPS people:

It's a while since I last touched that machine, so remember this is from
10 year old memory ...

The Jazz DMA hardware is an MMU that translates virtual DMA to physical
addresses.  It's virtual DMA address space is 16MB in size, it's page
size is 4kB.  That's a set of capabilities that nicely translates into
the DMA API.

> Would I be right to say that vdma_{alloc,free}() can be changed to 
> dma_{,un}map_single? The other Jazz specific routine that sonic uses is 
> vdma_log2phys, and I don't know if that has a better alternative.

The use of that call should simply be eleminated entirely.  DMA API
functions such as dma_alloc_coherent or dma_map_single will return a
dma_handle which along with the virtual address returned is everything
ever needed to program a DMA engine.

  Ralf

Re: [PATCH] Jazzsonic driver updates

[Finn Thain]

On Wed, 15 Jun 2005, Ralf Baechle wrote:

> On Sun, Jun 12, 2005 at 04:05:30PM +1000, Finn Thain wrote:
> 
> > > Oh funny.  vdma_alloc() was created 10 years ago as an internal API 
> > > for the Jazz machines.  Didn't realize m68k had cloned it :-)  If 
> > > anything it seems this should be converted to the modern DMA API.
> > 
> > I've just started merging my Mac sonic work into 2.6.12-rc6. m68k 
> > doesn't yet implement the modern DMA API, but it is easy to fake it 
> > for a while for macsonic.c.

Roman Zippel has since posted an implementation of that API for m68k, BTW. 
So I'll use that for macsonic, and possibly the driver core as well if I 
can figure out how to do this with jazzsonic.

> > So I don't mind converting macsonic, jazzsonic and the shared sonic 
> > driver core to the new API.
> > 
> > But, I knowing nothing about the Jazz DMA controller. I need some help 
> > from the MIPS people:
> 
> It's a while since I last touched that machine, so remember this is from 
> 10 year old memory ...
> 
> The Jazz DMA hardware is an MMU that translates virtual DMA to physical 
> addresses.  It's virtual DMA address space is 16MB in size, it's page 
> size is 4kB.  That's a set of capabilities that nicely translates into 
> the DMA API.

I gather that someone has already put this hardware under the control of 
the generic DMA API?

> > Would I be right to say that vdma_{alloc,free}() can be changed to 
> > dma_{,un}map_single? The other Jazz specific routine that sonic uses 
> > is vdma_log2phys, and I don't know if that has a better alternative.
> 
> The use of that call should simply be eleminated entirely.  DMA API
> functions such as dma_alloc_coherent or dma_map_single will return a
> dma_handle which along with the virtual address returned is everything
> ever needed to program a DMA engine.

The sonic chip stores packets inside a "receive resource area" at a 
physical address that depends on the packets it previously stored there. 

So the chip determines that address and the driver has to convert it from 
a physical to a virtual address with KSEG1ADDR(vdma_log2phys(x)), in order 
to memcpy the received packet.

>From what code I've looked at, and from what you've told me, I'm guessing 
that bus_to_virt() won't cut it here (?)

-f


> 
>   Ralf
> 

Re: [PATCH] Jazzsonic driver updates

[Ralf Baechle]

On Thu, Jun 16, 2005 at 12:02:33AM +1000, Finn Thain wrote:

> > The Jazz DMA hardware is an MMU that translates virtual DMA to physical 
> > addresses.  It's virtual DMA address space is 16MB in size, it's page 
> > size is 4kB.  That's a set of capabilities that nicely translates into 
> > the DMA API.
> 
> I gather that someone has already put this hardware under the control of 
> the generic DMA API?

That's being worked on.

> > > Would I be right to say that vdma_{alloc,free}() can be changed to 
> > > dma_{,un}map_single? The other Jazz specific routine that sonic uses 
> > > is vdma_log2phys, and I don't know if that has a better alternative.
> > 
> > The use of that call should simply be eleminated entirely.  DMA API
> > functions such as dma_alloc_coherent or dma_map_single will return a
> > dma_handle which along with the virtual address returned is everything
> > ever needed to program a DMA engine.
> 
> The sonic chip stores packets inside a "receive resource area" at a 
> physical address that depends on the packets it previously stored there. 
> 
> So the chip determines that address and the driver has to convert it from 
> a physical to a virtual address with KSEG1ADDR(vdma_log2phys(x)), in order 
> to memcpy the received packet.

Wrong.  The Sonic only does DMA to DMA addresses which will be translated
to physical addresses by the IOMMU.  That is it never ever deals with
physical addresses directly.

When transmitting or receiving a buffer it has to be mapped into the
DMA controller's address space first, for example through dma_map_single.

As the result you will obtain a DMA address which you will feed to the
Sonic or put into a rx or tx ring, whatever.  And you need to remember it
in the driver private data, just like the virtual address of the buffer
or the struct sk_buff pointer.  So all you need is to look at your private
data to find that virtual address you need for memcpy.

tg3 is a reasonable example of a driver - albeit not a simple one.

> >From what code I've looked at, and from what you've told me, I'm guessing 
> that bus_to_virt() won't cut it here (?)

bus_to_virt and virt_to_bus are shooting offences these days, no less.

  Ralf

Re: [PATCH] Jazzsonic driver updates

[Finn Thain]

On Wed, 15 Jun 2005, Ralf Baechle wrote:

> On Thu, Jun 16, 2005 at 12:02:33AM +1000, Finn Thain wrote:
> 
> > > The Jazz DMA hardware is an MMU that translates virtual DMA to 
> > > physical addresses.  It's virtual DMA address space is 16MB in size, 
> > > it's page size is 4kB.  That's a set of capabilities that nicely 
> > > translates into the DMA API.
> > 
> > I gather that someone has already put this hardware under the control 
> > of the generic DMA API?
> 
> That's being worked on.

OK, I guess I should code for it then. It is difficult to avoid converting 
the sonic core to the DMA API if I'm going to adopt it for macsonic. It 
just means that my changes to jazzsonic can't be tested yet, and so my 
work will remain out-of-tree.

> > > > Would I be right to say that vdma_{alloc,free}() can be changed to 
> > > > dma_{,un}map_single? The other Jazz specific routine that sonic 
> > > > uses is vdma_log2phys, and I don't know if that has a better 
> > > > alternative.
> > > 
> > > The use of that call should simply be eleminated entirely.  DMA API 
> > > functions such as dma_alloc_coherent or dma_map_single will return a 
> > > dma_handle which along with the virtual address returned is 
> > > everything ever needed to program a DMA engine.
> > 
> > The sonic chip stores packets inside a "receive resource area" at a 
> > physical address that depends on the packets it previously stored 
> > there.
> > 
> > So the chip determines that address and the driver has to convert it 
> > from a physical to a virtual address with KSEG1ADDR(vdma_log2phys(x)), 
> > in order to memcpy the received packet.
> 
> Wrong.  The Sonic only does DMA to DMA addresses which will be 
> translated to physical addresses by the IOMMU.  That is it never ever 
> deals with physical addresses directly.

OK.

> When transmitting or receiving a buffer it has to be mapped into the DMA 
> controller's address space first, for example through dma_map_single.
> 
> As the result you will obtain a DMA address which you will feed to the 
> Sonic or put into a rx or tx ring, whatever.  And you need to remember 
> it in the driver private data, just like the virtual address of the 
> buffer or the struct sk_buff pointer.  So all you need is to look at 
> your private data to find that virtual address you need for memcpy.

Not quite. The virtual address you are talking about is the beginning of 
the receive resource area, which is mapped as you describe. But, the chip 
then decides where in that area the recieved packet gets buffered, and the 
chip reports the address of that packet as a pointer. And this is why 
vdma_log2phys is used to convert the location of the packet (in IOMMU 
virtual address space) into CPU virtual address where the packet can be 
found by memcpy. And that is why I was asking for an alternative to 
vdma_log2phys. But I think calculating the offset of the packet with some 
IOMMU virtual pointer arithmetic will suffice in this case, since the 
initial mapping is probably going to be linear.

-f

> tg3 is a reasonable example of a driver - albeit not a simple one.
> 
> > >From what code I've looked at, and from what you've told me, I'm guessing 
> > that bus_to_virt() won't cut it here (?)
> 
> bus_to_virt and virt_to_bus are shooting offences these days, no less.
> 
>   Ralf
> -
> To unsubscribe from this list: send the line "unsubscribe linux-mips" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] Jazzsonic driver updates

[Finn Thain]

On Thu, 16 Jun 2005, I wrote:

> ... the chip then decides where in that area the recieved packet gets 
> buffered ... and that is why I was asking for an alternative to 
> vdma_log2phys...

But I forgot that it is possible to have the sonic chip store no more than 
one packet in each buffer before moving to the next one in the ring 
(though this isn't the case at present). So, as you say, vdma_log2phys 
isn't really required.

-f

Re: [PATCH] Jazzsonic driver updates

[Ralf Baechle]

On Thu, Jun 16, 2005 at 03:45:54AM +1000, Finn Thain wrote:

> > ... the chip then decides where in that area the recieved packet gets 
> > buffered ... and that is why I was asking for an alternative to 
> > vdma_log2phys...
> 
> But I forgot that it is possible to have the sonic chip store no more than 
> one packet in each buffer before moving to the next one in the ring 
> (though this isn't the case at present). So, as you say, vdma_log2phys 
> isn't really required.

Using that option doesn't really seem a good idea anyway.

  Ralf

Re: [PATCH] Jazzsonic driver updates

[Finn Thain]

On Thu, 16 Jun 2005, Ralf Baechle wrote:

> On Thu, Jun 16, 2005 at 03:45:54AM +1000, Finn Thain wrote:
> 
> > > ... the chip then decides where in that area the recieved packet gets 
> > > buffered ... and that is why I was asking for an alternative to 
> > > vdma_log2phys...
> > 
> > But I forgot that it is possible to have the sonic chip store no more than 
> > one packet in each buffer before moving to the next one in the ring 
> > (though this isn't the case at present). So, as you say, vdma_log2phys 
> > isn't really required.
> 
> Using that option doesn't really seem a good idea anyway.

Sorry, I mislead you (it has been 4 months since I looked at this). The 
existing code does only permit one packet per buffer. I did the same in my 
zero-copy version. So vdma_log2phys really isn't needed, and my patch 
actually removed it along with the memcpy. 

-f

> 
>   Ralf
> -
> To unsubscribe from this list: send the line "unsubscribe linux-mips" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>