[PATCH v2 02/23] at91: Make Ethernet device common

[linux@arm.linux.org.uk (Russell King - ARM Linux)]

On Thu, Apr 28, 2011 at 12:57:45PM -0500, H Hartley Sweeten wrote:
> On Thursday, April 28, 2011 2:15 AM, Russell King wrote:
> > On Thu, Apr 21, 2011 at 06:19:37PM -0500, H Hartley Sweeten wrote:
> >> +static struct platform_device at91_eth_device = {
> >> +	.name		= "macb",
> >> +	.id		= -1,
> >> +	.dev		= {
> >> +		.dma_mask		= &at91_eth_device.dev.coherent_dma_mask,
> >> +		.coherent_dma_mask	= DMA_BIT_MASK(32),
> >> 
> >> That will get rid of the static u64 variable used for every dma
> >> capable device.
> >
> > I've never really liked that, because coherent_dma_mask is never changed,
> > but the value pointed to be dma_mask can be by drivers.  See dma_set_mask().
> >
> > So, calling dma_set_mask() on any device which does the above will also
> > (unexpectedly?) change the coherent mask too.
> >
> > It's a minor point as we don't have many platform drivers using
> > dma_set_mask().
> 
> So, if I understand this correctly...
> 
> 1) dev.coherent_dma_mask is a constant that defines the default dma mask of
> the device.

dev.coherent_dma_mask is used for dma_alloc_coherent() allocations.

> 2) dev.dma_mask is a pointer to the dma mask that is being used by the device.

dev.dma_mask is a pointer to the DMA mask being used by the dma_map_* API.

> 3) The static variable pointed to by dev.dma_mask holds the dma mask being
> used by the device.  The value can be modified by the driver.
> 
> Why couldn't a new field be added to struct device to replace the static
> variable?  The code that sets up the device could do something like:
> 
> 	dev.used_dma_mask = dev.coherent_dma_mask;
> 	dev.dma_mask = &dev.used_dma_mask;

There was talk a while ago about getting rid of the pointer-ness of
dma_mask, but I think there's a bit of historic baggage in there to do
with the pre-device model PCI bus support code which gets in the way.
Not too sure about that though.