Question on prism54 driver

Question on prism54 driver

[Jean-Baptiste Note]

Dear list,

Going through the prism54 code for reuse in the softmac driver, i
stumbled accross inconsistencies in the queue management functions for
the mgmt rx queue and the data rx queue that i cannot understand.

Is there any reason that we have the following call in
islpci_mgt_receive (file islpci_mgt.c):

		/* Ensure the results of device DMA are visible to the CPU. */
		pci_dma_sync_single(priv->pdev, buf->pci_addr,
				    frag_len, PCI_DMA_FROMDEVICE);

but have nothing of the sort in islpci_eth_receive (file islpci_eth.c) ?

In the same spirit, the control block is also written by DMA by the
device, I guess ; so how comes we don't have such a syncing call in the
interrupt handler before accessing the control block's values (file
islpci_dev.c) ?

Am i missing something very obvious or very subtle ? -- should shared
memory be always sync'd before reading, or should we avoid it in cases
where we know it's not necessary, as an optimization ?

JB

-- 
Jean-Baptiste Note
+33 (0)6 83 03 42 38
jean-baptiste.note@wanadoo.fr

Re: Question on prism54 driver

[Rainer Weikusat]

Jean-Baptiste Note <jean-baptiste.note@wanadoo.fr> writes:
> Going through the prism54 code for reuse in the softmac driver, i
> stumbled accross inconsistencies in the queue management functions for
> the mgmt rx queue and the data rx queue that i cannot understand.
>
> Is there any reason that we have the following call in
> islpci_mgt_receive (file islpci_mgt.c):
>
> 		/* Ensure the results of device DMA are visible to the CPU. */
> 		pci_dma_sync_single(priv->pdev, buf->pci_addr,
> 				    frag_len, PCI_DMA_FROMDEVICE);
>
> but have nothing of the sort in islpci_eth_receive (file islpci_eth.c)?
>
> In the same spirit, the control block is also written by DMA by the
> device, I guess ; so how comes we don't have such a syncing call in the
> interrupt handler before accessing the control block's values (file
> islpci_dev.c) ?
>
> Am i missing something very obvious or very subtle?

<sigh>
No, you are just hacking the code without even having tried to
understand

	a) what it is doing
        b) why it does this
[which, "by coincedence", happens to be the reason that this driver is
 so thouroughly broken]
</sigh>

The data transfers (all of them) use streaming DMA mappings, which
have to be synced explicitly between 'bus view' and 'CPU view',
because they may involve bounce buffers (in 2.4) and to ensure that
data the CPU has written to the cache is actually in memory and data
the DMA engine has written is not ignored because of (actually stale)
cache entries that predate it. The mgmt-queues use persistent
streaming mapping, which means they have to be synced everytime the
CPU intends to access them after they were accessed by the DMA engine.
The skbuffs used by the data transfer code are mapped before they are
passed to the device and unmapped (which syncs them) before the CPU
accesses them. This isn't needed for the control block, because it is
a consistent and not a streaming mapping.

BTW, I shouldn't be telling you how your code works, don't you think
so?

Re: Question on prism54 driver

[Jean-Baptiste Note]

Hello Rainer, dear list,

First of all, thanks for your quick reply, which is very enlightening.

> <sigh>
> No, you are just hacking the code without even having tried to
> understand
>
> 	a) what it is doing
>       b) why it does this
> [which, "by coincedence", happens to be the reason that this driver is
>  so thouroughly broken]
> </sigh>

Please don't let my dumb questions shed a bad light on the prism54
developers as a whole.

I have no doubt that such questions would be obvious to core prism54
developers; i only have reverse-engeneered the softmac usb protocol and
am trying to use the prism54 code to do a pci version of the softmac usb
driver. Rest assured that i will not touch the main prism54 code.

> which means they have to be synced everytime the CPU intends to access
> them after they were accessed by the DMA engine.  The skbuffs used by
> the data transfer code are mapped before they are passed to the device
> and unmapped (which syncs them) before the CPU accesses them. This
> isn't needed for the control block, because it is a consistent and not
> a streaming mapping.

Thank you for taking the time to look into this and answering so
thoroughly. I missed the call to pci_unmap_single, and didn't know about
the consistent mapping meaning.

> BTW, I shouldn't be telling you how your code works, don't you think
> so?

I'm sorry my listing as a prism54 developer may lead to some confusion,
and adversely affect all people in prism54 in addition to making a fool
of myself. This has raised my awareness level. Thanks also for this :)

JB

-- 
Jean-Baptiste Note
+33 (0)6 83 03 42 38
jean-baptiste.note@wanadoo.fr