| Message ID | 1252772444-25749-1-git-send-email-s-paulraj@ti.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Sandeep, On Sat, Sep 12, 2009 at 21:50:44, Paulraj, Sandeep wrote: > From: Sandeep Paulraj <s-paulraj@ti.com> > > This API is very similar to the edma_free_slot and > edma_free_channel APIs. It is actually a consolidated > version of these 2 APIs. > A resource can be a channel or a slot. > Using this API, the EDMA driver code makes the > determination to free up a channel or a slot. > The user does not have to decide the correct "free" API > from among edma_free_channel or edma_free_slot. > > This API will be used by TI codecs and the DVSDK. Hmm, wonder how it improves convenience considering that channels and slots are allocated separately. Presumably, user will need to maintain separate cookies for channels and slots. IMHO, in the absence of an alloc_resource() API, this sounds incongruous. [...] > +/** > + * edma_free_resource - deallocate DMA parameter RAM channel/resource > + * @resource: Either a channel or a slot to be freed > + * > + * This deallocates the DMA channel and associated parameter RAM slot > + * if the resource happens to be a channel, i.e if the resource number is > + * less than 64 for DaVinci SOCs and less than 32 for Primus. ^^^^^^^ I think you mean to say DA8XX/OMAP-L1 Thanks, Sekhar
Since I was the one to ask Sandeep for this API, I will offer up my reasoning... Without this API, in order to call either edma_free_slot() or edma_free_channel() the LinuxUtils EDMAK device driver will have to carry a "slot-vs-channel" "flag" or "cookie" around with the EDMA allocation record. Then we need to consult this flag and call one or the other "free". Other drivers will be doing this as well. Having this API will simplify code that allocates both "slots" and "channels". It seems prudent to consolidate the decision making to one place (edma_free_resource()) instead of having it be sprinkled throughout the calling code, and also removes the possibility of the driver calling the wrong one. Regards, - Rob > -----Original Message----- > From: > davinci-linux-open-source-bounces+rtivy=ti.com@linux.davincids p.com [mailto:davinci-linux-open-source-> bounces+rtivy=ti.com@linux.davincidsp.com] On Behalf Of Nori, Sekhar > Sent: Saturday, September 12, 2009 9:27 PM > To: Paulraj, Sandeep > Cc: davinci-linux-open-source@linux.davincidsp.com > Subject: RE: [PATCH] DaVinci: EDMA: New API edma_free_resource > > Hi Sandeep, > > On Sat, Sep 12, 2009 at 21:50:44, Paulraj, Sandeep wrote: > > From: Sandeep Paulraj <s-paulraj@ti.com> > > > > This API is very similar to the edma_free_slot and > edma_free_channel > > APIs. It is actually a consolidated version of these 2 APIs. > > A resource can be a channel or a slot. > > Using this API, the EDMA driver code makes the > determination to free > > up a channel or a slot. > > The user does not have to decide the correct "free" API from among > > edma_free_channel or edma_free_slot. > > > > This API will be used by TI codecs and the DVSDK. > > Hmm, wonder how it improves convenience considering that > channels and slots are allocated separately. > Presumably, user will need to maintain separate cookies for > channels and slots. > > IMHO, in the absence of an alloc_resource() API, this sounds > incongruous. > > [...] > > +/** > > + * edma_free_resource - deallocate DMA parameter RAM > channel/resource > > + * @resource: Either a channel or a slot to be freed > > + * > > + * This deallocates the DMA channel and associated > parameter RAM slot > > + * if the resource happens to be a channel, i.e if the resource > > +number is > > + * less than 64 for DaVinci SOCs and less than 32 for Primus. > > ^^^^^^^ I think you mean to say DA8XX/OMAP-L1 > > Thanks, > Sekhar > > _______________________________________________ > Davinci-linux-open-source mailing list > Davinci-linux-open-source@linux.davincidsp.com > http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source > _______________________________________________ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
On Mon, Sep 14, 2009 at 22:12:09, Tivy, Robert wrote: > Since I was the one to ask Sandeep for this API, I will offer up my reasoning... > > Without this API, in order to call either edma_free_slot() or edma_free_channel() the LinuxUtils EDMAK device driver will have to carry a "slot-vs-channel" "flag" or "cookie" around with the EDMA allocation record. Then we need to consult this flag and call one or the other "free". Other drivers will be doing this as well. Having this API will simplify code that allocates both "slots" and "channels". It seems prudent to consolidate the decision making to one place (edma_free_resource()) instead of having it be sprinkled throughout the calling code, and also removes the possibility of the driver calling the wrong one. > Yes, the drivers need to maintain separate record of slots and channels. In the current EDMA API, both are treated as different entities. So, you need different variables to store information about them. This should also eliminate the need for another flag and the possibility of calling one free API for the other. Thanks, Sekhar > Regards, > > - Rob > > > -----Original Message----- > > From: > > davinci-linux-open-source-bounces+rtivy=ti.com@linux.davincids > p.com [mailto:davinci-linux-open-source-> bounces+rtivy=ti.com@linux.davincidsp.com] On Behalf Of Nori, Sekhar > > Sent: Saturday, September 12, 2009 9:27 PM > > To: Paulraj, Sandeep > > Cc: davinci-linux-open-source@linux.davincidsp.com > > Subject: RE: [PATCH] DaVinci: EDMA: New API edma_free_resource > > > > Hi Sandeep, > > > > On Sat, Sep 12, 2009 at 21:50:44, Paulraj, Sandeep wrote: > > > From: Sandeep Paulraj <s-paulraj@ti.com> > > > > > > This API is very similar to the edma_free_slot and > > edma_free_channel > > > APIs. It is actually a consolidated version of these 2 APIs. > > > A resource can be a channel or a slot. > > > Using this API, the EDMA driver code makes the > > determination to free > > > up a channel or a slot. > > > The user does not have to decide the correct "free" API from among > > > edma_free_channel or edma_free_slot. > > > > > > This API will be used by TI codecs and the DVSDK. > > > > Hmm, wonder how it improves convenience considering that > > channels and slots are allocated separately. > > Presumably, user will need to maintain separate cookies for > > channels and slots. > > > > IMHO, in the absence of an alloc_resource() API, this sounds > > incongruous. > > > > [...] > > > +/** > > > + * edma_free_resource - deallocate DMA parameter RAM > > channel/resource > > > + * @resource: Either a channel or a slot to be freed > > > + * > > > + * This deallocates the DMA channel and associated > > parameter RAM slot > > > + * if the resource happens to be a channel, i.e if the resource > > > +number is > > > + * less than 64 for DaVinci SOCs and less than 32 for Primus. > > > > ^^^^^^^ I think you mean to say DA8XX/OMAP-L1 > > > > Thanks, > > Sekhar > > > > _______________________________________________ > > Davinci-linux-open-source mailing list > > Davinci-linux-open-source@linux.davincidsp.com > > http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source > > >
"Tivy, Robert" <rtivy@ti.com> writes: > Since I was the one to ask Sandeep for this API, I will offer up my reasoning... > > Without this API, in order to call either edma_free_slot() or > edma_free_channel() the LinuxUtils EDMAK device driver will have to > carry a "slot-vs-channel" "flag" or "cookie" around with the EDMA > allocation record. Then we need to consult this flag and call one > or the other "free". Other drivers will be doing this as well. This sounds pretty normal to me, and expected. > Having this API will simplify code that allocates both "slots" and > "channels". It seems prudent to consolidate the decision making to > one place (edma_free_resource()) instead of having it be sprinkled > throughout the calling code, and also removes the possibility of the > driver calling the wrong one. I agree with Sekhar that a free_resource() without a corresponding alloc_resource() doesn't make sense and is more confusing. Kevin > > Regards, > > - Rob > >> -----Original Message----- >> From: >> davinci-linux-open-source-bounces+rtivy=ti.com@linux.davincids > p.com [mailto:davinci-linux-open-source-> bounces+rtivy=ti.com@linux.davincidsp.com] On Behalf Of Nori, Sekhar >> Sent: Saturday, September 12, 2009 9:27 PM >> To: Paulraj, Sandeep >> Cc: davinci-linux-open-source@linux.davincidsp.com >> Subject: RE: [PATCH] DaVinci: EDMA: New API edma_free_resource >> >> Hi Sandeep, >> >> On Sat, Sep 12, 2009 at 21:50:44, Paulraj, Sandeep wrote: >> > From: Sandeep Paulraj <s-paulraj@ti.com> >> > >> > This API is very similar to the edma_free_slot and >> edma_free_channel >> > APIs. It is actually a consolidated version of these 2 APIs. >> > A resource can be a channel or a slot. >> > Using this API, the EDMA driver code makes the >> determination to free >> > up a channel or a slot. >> > The user does not have to decide the correct "free" API from among >> > edma_free_channel or edma_free_slot. >> > >> > This API will be used by TI codecs and the DVSDK. >> >> Hmm, wonder how it improves convenience considering that >> channels and slots are allocated separately. >> Presumably, user will need to maintain separate cookies for >> channels and slots. >> >> IMHO, in the absence of an alloc_resource() API, this sounds >> incongruous. >> >> [...] >> > +/** >> > + * edma_free_resource - deallocate DMA parameter RAM >> channel/resource >> > + * @resource: Either a channel or a slot to be freed >> > + * >> > + * This deallocates the DMA channel and associated >> parameter RAM slot >> > + * if the resource happens to be a channel, i.e if the resource >> > +number is >> > + * less than 64 for DaVinci SOCs and less than 32 for Primus. >> >> ^^^^^^^ I think you mean to say DA8XX/OMAP-L1 >> >> Thanks, >> Sekhar >> >> _______________________________________________ >> Davinci-linux-open-source mailing list >> Davinci-linux-open-source@linux.davincidsp.com >> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source >> _______________________________________________ > Davinci-linux-open-source mailing list > Davinci-linux-open-source@linux.davincidsp.com > http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Thanks for considering this API. It's the "embedded programmer" in me that's always trying to consolidate code at as low a level as possible. Perhaps a better name would be "edma_free_channel_or_slot()", so it matches up with the allocation API naming, but it would just be a helper function that existed along with edma_free_channel() and edma_free_slot(). My request was certainly a selfish one, but since it would improve my EDMA driver code I thought it could do the same for others. I suppose the LinuxUtils EDMA driver is a unique client of the EDMA APIs, in that it is a resource manager and doesn't actually call any of the functional EDMA APIs (the ones that drive the EDMA HW), and from that perspective "slots" and "channels" are just different parts of the same resource space, so I'm not really representative of the typical EDMA API client. Regards, - Rob > -----Original Message----- > From: Kevin Hilman [mailto:khilman@deeprootsystems.com] > Sent: Wednesday, September 16, 2009 8:43 AM > To: Tivy, Robert > Cc: Nori, Sekhar; Paulraj, Sandeep; > davinci-linux-open-source@linux.davincidsp.com > Subject: Re: [PATCH] DaVinci: EDMA: New API edma_free_resource > > "Tivy, Robert" <rtivy@ti.com> writes: > > > Since I was the one to ask Sandeep for this API, I will > offer up my reasoning... > > > > Without this API, in order to call either edma_free_slot() or > > edma_free_channel() the LinuxUtils EDMAK device driver will have to > > carry a "slot-vs-channel" "flag" or "cookie" around with the EDMA > > allocation record. Then we need to consult this flag and > call one or > > the other "free". Other drivers will be doing this as well. > > This sounds pretty normal to me, and expected. > > > Having this API will simplify code that allocates both "slots" and > > "channels". It seems prudent to consolidate the decision making to > > one place (edma_free_resource()) instead of having it be sprinkled > > throughout the calling code, and also removes the > possibility of the > > driver calling the wrong one. > > I agree with Sekhar that a free_resource() without a corresponding > alloc_resource() doesn't make sense and is more confusing. > > Kevin > > > > > > Regards, > > > > - Rob > > > >> -----Original Message----- > >> From: > >> davinci-linux-open-source-bounces+rtivy=ti.com@linux.davincids > > p.com [mailto:davinci-linux-open-source-> > > bounces+rtivy=ti.com@linux.davincidsp.com] On Behalf Of Nori, Sekhar > >> Sent: Saturday, September 12, 2009 9:27 PM > >> To: Paulraj, Sandeep > >> Cc: davinci-linux-open-source@linux.davincidsp.com > >> Subject: RE: [PATCH] DaVinci: EDMA: New API edma_free_resource > >> > >> Hi Sandeep, > >> > >> On Sat, Sep 12, 2009 at 21:50:44, Paulraj, Sandeep wrote: > >> > From: Sandeep Paulraj <s-paulraj@ti.com> > >> > > >> > This API is very similar to the edma_free_slot and > >> edma_free_channel > >> > APIs. It is actually a consolidated version of these 2 APIs. > >> > A resource can be a channel or a slot. > >> > Using this API, the EDMA driver code makes the > >> determination to free > >> > up a channel or a slot. > >> > The user does not have to decide the correct "free" API > from among > >> > edma_free_channel or edma_free_slot. > >> > > >> > This API will be used by TI codecs and the DVSDK. > >> > >> Hmm, wonder how it improves convenience considering that > channels and > >> slots are allocated separately. > >> Presumably, user will need to maintain separate cookies > for channels > >> and slots. > >> > >> IMHO, in the absence of an alloc_resource() API, this sounds > >> incongruous. > >> > >> [...] > >> > +/** > >> > + * edma_free_resource - deallocate DMA parameter RAM > >> channel/resource > >> > + * @resource: Either a channel or a slot to be freed > >> > + * > >> > + * This deallocates the DMA channel and associated > >> parameter RAM slot > >> > + * if the resource happens to be a channel, i.e if the resource > >> > +number is > >> > + * less than 64 for DaVinci SOCs and less than 32 for Primus. > >> > >> ^^^^^^^ I think you mean to say DA8XX/OMAP-L1 > >> > >> Thanks, > >> Sekhar > >> > >> _______________________________________________ > >> Davinci-linux-open-source mailing list > >> Davinci-linux-open-source@linux.davincidsp.com > >> > http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-sourc > >> e _______________________________________________ > > Davinci-linux-open-source mailing list > > Davinci-linux-open-source@linux.davincidsp.com > > > http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source > > _______________________________________________ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
On Monday 14 September 2009, Tivy, Robert wrote: > Without this API, in order to call either edma_free_slot() or > edma_free_channel() the LinuxUtils EDMAK device driver will > have to carry a "slot-vs-channel" "flag" or "cookie" around > with the EDMA allocation record. It already needs one of those though doesn't it? You can't use a slot to trigger a DMA transfer (keyed on an event, manually, or by chaining). Unless it's been set up as a QDMA channel... which and thus needs even more special handling during deallocation. The reason there'is no edma_alloc_resource() is that there really are two very distinct resource types, which need distinct treatment almost everywhere. - Dave
Thanks Dave, understood. See my reply to Kevin that I just sent, it explains that the LinuxUtils EDMA driver is probably unlike other drivers, in that it doesn't actually use any of the functional EDMA APIs, hence can treat slots and channels equivalently once they're allocated. Regards, - Rob > -----Original Message----- > From: David Brownell [mailto:david-b@pacbell.net] > Sent: Wednesday, September 16, 2009 1:49 PM > To: davinci-linux-open-source@linux.davincidsp.com > Cc: Tivy, Robert; Nori, Sekhar; Paulraj, Sandeep > Subject: Re: [PATCH] DaVinci: EDMA: New API edma_free_resource > > On Monday 14 September 2009, Tivy, Robert wrote: > > Without this API, in order to call either edma_free_slot() or > > edma_free_channel() the LinuxUtils EDMAK device driver will have to > > carry a "slot-vs-channel" "flag" or "cookie" around with the EDMA > > allocation record. > > It already needs one of those though doesn't it? You can't > use a slot to trigger a DMA transfer (keyed on an event, > manually, or by chaining). Unless it's been set up as a QDMA > channel... which and thus needs even more special handling > during deallocation. > > The reason there'is no edma_alloc_resource() is that there > really are two very distinct resource types, which need > distinct treatment almost everywhere. > > - Dave > > > _______________________________________________ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/mach-davinci/dma.c index bfce4b5..97bfa6c 100644 --- a/arch/arm/mach-davinci/dma.c +++ b/arch/arm/mach-davinci/dma.c @@ -740,6 +740,38 @@ void edma_free_slot(unsigned slot) } EXPORT_SYMBOL(edma_free_slot); +/** + * edma_free_resource - deallocate DMA parameter RAM channel/resource + * @resource: Either a channel or a slot to be freed + * + * This deallocates the DMA channel and associated parameter RAM slot + * if the resource happens to be a channel, i.e if the resource number is + * less than 64 for DaVinci SOCs and less than 32 for Primus. + * Otherwise deallocate the parameter RAM slot. + * Callers are responsible for ensuring the slot is inactive, and will + * not be activated. + */ +void edma_free_resource(unsigned resource) +{ + unsigned ctlr; + + ctlr = EDMA_CTLR(resource); + resource = EDMA_CHAN_SLOT(resource); + + /* + * The resource cannot be greater than the number of slots + */ + if (resource >= edma_info[ctlr]->num_slots) + return; + + if (resource < edma_info[ctlr]->num_channels) + setup_dma_interrupt(resource, NULL, NULL); + + memcpy_toio(edmacc_regs_base[ctlr] + PARM_OFFSET(resource), + &dummy_paramset, PARM_SIZE); + clear_bit(resource, edma_info[ctlr]->edma_inuse); +} +EXPORT_SYMBOL(edma_free_resource); /** * edma_alloc_cont_slots- alloc contiguous parameter RAM slots diff --git a/arch/arm/mach-davinci/include/mach/edma.h b/arch/arm/mach-davinci/include/mach/edma.h index eb8bfd7..19d8e48 100644 --- a/arch/arm/mach-davinci/include/mach/edma.h +++ b/arch/arm/mach-davinci/include/mach/edma.h @@ -240,6 +240,9 @@ void edma_free_channel(unsigned channel); int edma_alloc_slot(unsigned ctlr, int slot); void edma_free_slot(unsigned slot); +/* free either a channel or slot */ +void edma_free_resource(unsigned resource); + /* alloc/free a set of contiguous parameter RAM slots */ int edma_alloc_cont_slots(unsigned ctlr, unsigned int id, int slot, int count); int edma_free_cont_slots(unsigned slot, int count);