On Fri, Jun 10, 2016 at 04:49:47PM +0200, Luis R. Rodriguez wrote:
> On Fri, Jun 10, 2016 at 12:11:18PM +0200, Krzysztof Kozlowski wrote:
> > The dma-mapping core and the implementations do not change the
> > DMA attributes passed by pointer. Thus the pointer can point to const
> > data. However the attributes do not have to be a bitfield. Instead
> > unsigned long will do fine:
> >
> > 1. This is just simpler. Both in terms of reading the code and setting
> > attributes. Instead of initializing local attributes on the stack
> > and passing pointer to it to dma_set_attr(), just set the bits.
> >
> > 2. It brings safeness and checking for const correctness because the
> > attributes are passed by value.
>
> Do we not expect the number of argument to grow ? This "cleanup" would
> do away with such possibilities, and then require adding the API later,
> and this requiring a full set of collateral evolutions again when this
> is needed. What was the original motivation for using this instead of
> the approach you are suggesting ?
What do you mean by "possibilities of argument to grow"? Something like
adding new members to "struct dma_attrs" and changing its meaning?
I think such growth is still constrained - you cannot put there anything
without changing the meaning of the argument.
However you are right that "unsigned long" removes that possibility
completely.
The dma-attrs in current form were added around 2008 in 74bc7ceebfa1
("dma: add dma_*map*_attrs() interfaces"), I think. Since that time, for
example, the dma_map_*_attrs() did not change.
> If the concern is the const data, why not require const struct dma_attr
> for the APIs that we know can and should use const ?
The const is one concern. Complicated (more than expected) usage of dma
attributes by the caller is second.
Switching it to const would also reduce the possibilities of API
extension.
Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
