Prev: Lucky Winner( Contact The Claims Agent)
Next: 2.6.35-rc{12} regression: inactive console corrupted
From: Uwe Kleine-König on 22 Jun 2010 07:00
Hello,

IMHO it's strange that struct device.dma_mask is a pointer instead of a
plain u64. The reason this was done back then is described in
8ab1bc19e974fdebe76c065fe444979c84ba2f74[1]:

Attached is a patch which moves dma_mask into struct device and cleans up the
scsi mid-layer to use it (instead of using struct pci_dev). The advantage to
doing this is probably most apparent on non-pci bus architectures where
currently you have to construct a fake pci_dev just so you can get the bounce
buffers to work correctly.

The patch tries to perturb the minimum amount of code, so dma_mask in struct
device is simply a pointer to the one in pci_dev. However, it will make it
easy for me now to add generic device to MCA without having to go the fake pci
route.

As I work on such a non-pci bus architecture it's still ugly that this
is a pointer because I have to allocate extra memory for that.

Is there a reason not to get rid of struct pci_dev.dma_mask and use
struct pci_dev.dev.dma_mask instead? (Well apart from the needed
effort of course.)

If not, the following would be needed:

- remove struct pci.dma_mask
- make struct device.dma_mask an u64 (instead of u64*)
- substitue var.dma_mask by var.dev.dma_mask for all
struct pci_dev var
- substitue var.dma_mask by &(var.dma_mask) for all
struct device var

and note that there are statically initialized struct device (and maybe
struct pci_dev?) that need fixing, too. (e.g.
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=arch/arm/mach-mx2/devices.c;h=a0aeb8a4adc19ef419a0a045ad3b882131597106;hb=HEAD#l265
)

Additionally this could be done for struct device.dma_parms.

Julia: help!

Best regards
Uwe

[1] as this is pre-2.6.12 it's only available in tglx' histroy tree,
e.g. http://git.kernel.org/?p=linux/kernel/git/tglx/history.git;a=commitdiff;h=8ab1bc19e974fdebe76c065fe444979c84ba2f74

--
Pengutronix e.K. | Uwe Kleine-K�nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Konrad Rzeszutek Wilk on 30 Jun 2010 14:10
On Tue, Jun 22, 2010 at 12:52:33PM +0200, Uwe Kleine-K�nig wrote:
> Hello,
>
> IMHO it's strange that struct device.dma_mask is a pointer instead of a
> plain u64. The reason this was done back then is described in
> 8ab1bc19e974fdebe76c065fe444979c84ba2f74[1]:
>
> Attached is a patch which moves dma_mask into struct device and cleans up the
> scsi mid-layer to use it (instead of using struct pci_dev). The advantage to
> doing this is probably most apparent on non-pci bus architectures where
> currently you have to construct a fake pci_dev just so you can get the bounce
> buffers to work correctly.
>
> The patch tries to perturb the minimum amount of code, so dma_mask in struct
> device is simply a pointer to the one in pci_dev. However, it will make it
> easy for me now to add generic device to MCA without having to go the fake pci
> route.
>
> As I work on such a non-pci bus architecture it's still ugly that this
> is a pointer because I have to allocate extra memory for that.
>
> Is there a reason not to get rid of struct pci_dev.dma_mask and use
> struct pci_dev.dev.dma_mask instead? (Well apart from the needed
> effort of course.)

Lets CC Fujita. He has been redoing some of the DMA API, and making the
PCI DMA API be used in favour of the old DMA API.

But from the sounds of it for your architecture you need a DMA API, not
a PCI DMA and you want to merge the dma_mask in one? Preferably in the
struct device one?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: FUJITA Tomonori on 30 Jun 2010 21:40
> IMHO it's strange that struct device.dma_mask is a pointer instead of a
> plain u64. The reason this was done back then is described in
> 8ab1bc19e974fdebe76c065fe444979c84ba2f74[1]:
>
> Attached is a patch which moves dma_mask into struct device and cleans up the
> scsi mid-layer to use it (instead of using struct pci_dev). The advantage to
> doing this is probably most apparent on non-pci bus architectures where
> currently you have to construct a fake pci_dev just so you can get the bounce
> buffers to work correctly.
>
> The patch tries to perturb the minimum amount of code, so dma_mask in struct
> device is simply a pointer to the one in pci_dev. However, it will make it
> easy for me now to add generic device to MCA without having to go the fake pci
> route.

Yeah, that's a strange design. As the commit log said, it's due to the
historical reason. We invented the pci dma model first then moved to
the generic dma model.


> As I work on such a non-pci bus architecture it's still ugly that this
> is a pointer because I have to allocate extra memory for that.

The popular trick to avoid allocating the extra memory for that is:

device.dma_mask = &device.coherent_dma_mask;


> Is there a reason not to get rid of struct pci_dev.dma_mask and use
> struct pci_dev.dev.dma_mask instead? (Well apart from the needed
> effort of course.)
>
> If not, the following would be needed:
>
> - remove struct pci.dma_mask
> - make struct device.dma_mask an u64 (instead of u64*)
> - substitue var.dma_mask by var.dev.dma_mask for all
> struct pci_dev var
> - substitue var.dma_mask by &(var.dma_mask) for all
> struct device var
>
> and note that there are statically initialized struct device (and maybe
> struct pci_dev?) that need fixing, too. (e.g.
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=arch/arm/mach-mx2/devices.c;h=a0aeb8a4adc19ef419a0a045ad3b882131597106;hb=HEAD#l265
> )

That's exactly the perturbation that the commit log refers to.

We need to modify all the struct device at a time. We could, however,
I don't think that it's worth doing. Little gain.


> Additionally this could be done for struct device.dma_parms.

Yeah, we should have all the dma parameters in dma_parms.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: Lucky Winner( Contact The Claims Agent)
Next: 2.6.35-rc{12} regression: inactive console corrupted