Subject: Re: CVS commit: src/sys
To: Jachym Holecek <freza@dspfpga.com>
From: Yorick Hardy <yhardy@uj.ac.za>
List: tech-kern
Date: 02/26/2007 13:03:37
This is a multi-part message in MIME format.
--------------020908030203080702070005
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Jachym Holecek wrote:
> Hello,
>
> a couple of minor nits:
>
> # Yorick Hardy 2007-02-23:
>   
>> --- share/man/man9/bus_dma.9.orig	2007-02-23 16:21:58.000000000 +0200
>> +++ share/man/man9/bus_dma.9	2007-02-23 16:33:57.000000000 +0200
>> @@ -777,6 +784,53 @@
>>  .Pp
>>  Returns -1 to indicate failure.
>>  Otherwise, returns an opaque value to be interpreted by the device pager.
>> +.It Fn bus_dmatag_subregion "tag" "min_addr" "max_addr" "newtag" "flags"
>> +Creates a new bus_dma_tag_t in
>> +.Fa newtag
>> +from
>> +.Fa tag
>> +with the added constraint that data will only be transferred to
>> +memory addresses in the range (subregion)
>>     
>
> Aren't they bus addresses, really? On a system with IOMMU, the subregion
> constraint translates to address constraint on the _bus_ side of the
> translation, memory address isn't really restricted.
>
> Unless I've confused myself with terminology again :-).
>
>   
Thanks I changed this.
>> +.Fa min_addr
>> +to
>> +.Fa max_addr.
>> +The arguments are as follows:
>> +.Bl -tag -width nsegs -compact
>> +.It Fa tag
>> +This is the bus_dma_tag_t passed down from the parent driver via
>> +.Fa \*[Lt]bus\*[Gt]_attach_args .
>> +.It Fa min_addr
>> +The low address constraint for DMA transfers.
>> +This address is included.
>>     
>
> I should've asked earlier how much practical value the lower constraint
> has... No problem keeping it, just curious -- I can't imagine how would
> one need to FUBAR a device to make it unable to transfer below certain
> address.
>
>   
>> +.It Fa max_addr
>> +The high address constraint for DMA transfers.
>> +This address is not included.
>>     
>
> Maybe call it "limit" then (normally, I'd expect "maximum" is included
> in the range)?
>
>   
How about low and high, which is consistent with the use in
uvm_pglistalloc ?
>> [...]
>> +.It Fn bus_dmatag_destroy "tag"
>> +Frees any resources associated with
>> +.Fa tag
>> +when the tag is no longer needed.
>> +.Fn bus_dmatag_destroy
>> +should always be called when a 
>> +.Fa tag
>> +created by
>> +.Fn bus_dmatag_subregion
>> +is no longer needed.
>> +If given valid arguments,
>> +.Fn bus_dmatag_destroy
>> +always succeeds.
>>     
>
> It's void, so can't really fail nor succeed. Maybe just say it assumes
> valid argument (which means "behaviour undefined for invalid value")?
>
>   
Thanks, I changed this.
> Thanks for doing this, BTW.
>
>   
Thank you for your input.
> 	-- Jachym
>
>   
I hope the attached patch is an improvement.

-- 
Kind regards,

Yorick Hardy


--------------020908030203080702070005
Content-Type: text/plain;
 name="bus_dma2.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="bus_dma2.patch"

--- share/man/man9/bus_dma.9.orig	2007-02-23 16:21:58.000000000 +0200
+++ share/man/man9/bus_dma.9	2007-02-26 12:56:13.000000000 +0200
@@ -52,7 +52,9 @@
 .Nm bus_dmamem_free ,
 .Nm bus_dmamem_map ,
 .Nm bus_dmamem_unmap ,
-.Nm bus_dmamem_mmap
+.Nm bus_dmamem_mmap ,
+.Nm bus_dmatag_subregion ,
+.Nm bus_dmatag_destroy
 .Nd Bus and Machine Independent DMA Mapping Interface
 .Sh SYNOPSIS
 .In machine/bus.h
@@ -92,6 +94,11 @@
 .Ft paddr_t
 .Fn bus_dmamem_mmap "bus_dma_tag_t tag" "bus_dma_segment_t *segs" \
 "int nsegs" "off_t off" "int prot" "int flags"
+.Ft int
+.Fn bus_dmatag_subregion "bus_dma_tag_t tag" "bus_addr_t low" \
+"bus_addr_t high" "bus_dma_tag_t *newtag" "int flags"
+.Ft void
+.Fn bus_dmatag_destroy "bus_dma_tag_t tag" 
 .Sh DESCRIPTION
 Provide a bus- and machine-independent "DMA mapping interface."
 .Sh NOTES
@@ -777,6 +784,54 @@
 .Pp
 Returns -1 to indicate failure.
 Otherwise, returns an opaque value to be interpreted by the device pager.
+.It Fn bus_dmatag_subregion "tag" "low" "high" "newtag" "flags"
+Creates a new bus_dma_tag_t in
+.Fa newtag
+from
+.Fa tag
+with the added constraint that data will only be transferred to
+bus addresses in the range (subregion)
+.Fa low
+to
+.Fa high.
+The arguments are as follows:
+.Bl -tag -width nsegs -compact
+.It Fa tag
+This is the bus_dma_tag_t passed down from the parent driver via
+.Fa \*[Lt]bus\*[Gt]_attach_args .
+.It Fa low
+The low bus address constraint for DMA transfers.
+This address is included.
+.It Fa high
+The high bus address constraint for DMA transfers.
+This address is not included.
+.It Fa newtag
+A pointer to a bus_dma_tag_t which will store
+the tag limited to the subregion.
+.It Fa flags
+Flags are defined as follows:
+.Bl -tag -width BUS_DMA_COHERENT -compact
+.It Dv BUS_DMA_NOWAIT
+It is not safe to wait (sleep) for resources during this call.
+.El
+.El
+.Pp
+Returns 0 on success, or an error code to indicate mode of failure.
+Architectures which do not yet support this operation return EOPNOTSUPP.
+.It Fn bus_dmatag_destroy "tag"
+Frees any resources associated with
+.Fa tag
+when the tag is no longer needed.
+.Fn bus_dmatag_destroy
+should always be called when a 
+.Fa tag
+created by
+.Fn bus_dmatag_subregion
+is no longer needed.
+.Fn bus_dmatag_destroy
+assumes
+.Fa tag
+is a valid bus_dma_tag_t.
 .El
 .Sh SEE ALSO
 .Xr bus_space 9

--------------020908030203080702070005--