Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1419706

Re: [PATCH v4 01/44] dma-mapping: Use unsigned long for dma_attrs

From Krzysztof Kozlowski <k.kozlowski@samsung.com>
Newsgroups linux.kernel
Subject Re: [PATCH v4 01/44] dma-mapping: Use unsigned long for dma_attrs
Date 2016-06-10 22:20 +0200
Message-ID <rIB4m-1c8-25@gated-at.bofh.it> (permalink)
References <rIrHH-3il-3@gated-at.bofh.it> <rIrRo-3lN-31@gated-at.bofh.it> <rIvUZ-5O9-17@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v4 01/44] dma-mapping: Use unsigned long for dma_attrs Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-06-10 12:30 +0200
  Re: [PATCH v4 01/44] dma-mapping: Use unsigned long for dma_attrs "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-10 16:50 +0200
    Re: [PATCH v4 01/44] dma-mapping: Use unsigned long for dma_attrs Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-06-10 22:20 +0200
      Re: [PATCH v4 01/44] dma-mapping: Use unsigned long for dma_attrs "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-10 22:30 +0200
        Re: [PATCH v4 01/44] dma-mapping: Use unsigned long for dma_attrs Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-06-10 22:50 +0200
          Re: [PATCH v4 01/44] dma-mapping: Use unsigned long for dma_attrs "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-10 23:10 +0200
    Re: [PATCH v4 01/44] dma-mapping: Use unsigned long for dma_attrs Christoph Hellwig <hch@infradead.org> - 2016-06-13 18:10 +0200
      Re: [PATCH v4 01/44] dma-mapping: Use unsigned long for dma_attrs "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-13 19:50 +0200

csiph-web