| Message ID | 1311557312-26107-7-git-send-email-boojin.kim@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 25, 2011 at 10:28:24AM +0900, Boojin Kim wrote: > +static unsigned samsung_dmadev_request(enum dma_ch dma_ch, > + struct samsung_dma_info *info) > +{ > + struct dma_chan *chan; > + dma_cap_mask_t mask; > + struct dma_slave_config slave_config; > + > + dma_cap_zero(mask); > + dma_cap_set(info->cap, mask); > + > + chan = dma_request_channel(mask, pl330_filter, (void *)dma_ch); > + > + if (info->direction == DMA_FROM_DEVICE) { > + memset(&slave_config, 0, sizeof(struct dma_slave_config)); > + slave_config.direction = info->direction; > + slave_config.src_addr = info->fifo; > + slave_config.src_addr_width = info->width; This should really set slave_config.src_maxburst to something sensible too, even if that's just '1'. > + dmaengine_slave_config(chan, &slave_config); > + } else if (info->direction == DMA_TO_DEVICE) { > + memset(&slave_config, 0, sizeof(struct dma_slave_config)); > + slave_config.direction = info->direction; > + slave_config.dst_addr = info->fifo; > + slave_config.dst_addr_width = info->width; Ditto for dst_maxburst. > + dmaengine_slave_config(chan, &slave_config); > + } > + > + return (unsigned)chan; I hope these interfaces yet cleaned away soon so that these casts can be killed off. > +struct samsung_dma_prep_info { > + enum dma_transaction_type cap; > + enum dma_data_direction direction; > + unsigned buf; dma_addr_t ? > + unsigned long period; > + unsigned long len; > + void (*fp)(void *data); > + void *fp_param; > +}; > + > +struct samsung_dma_info { > + enum dma_transaction_type cap; > + enum dma_data_direction direction; > + enum dma_slave_buswidth width; > + unsigned fifo; dma_addr_t ? > + struct s3c2410_dma_client *client; > +}; > + > +struct samsung_dma_ops { > + bool init; Unused?
Russell King - ARM Linux Wrote: > Sent: Monday, July 25, 2011 6:36 PM > To: Boojin Kim > Cc: linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; > Kukjin Kim; Vinod Koul; Jassi Brar; Grant Likely; Mark Brown; Dan Williams > Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations > > On Mon, Jul 25, 2011 at 10:28:24AM +0900, Boojin Kim wrote: > > +static unsigned samsung_dmadev_request(enum dma_ch dma_ch, > > + struct samsung_dma_info *info) > > +{ > > + struct dma_chan *chan; > > + dma_cap_mask_t mask; > > + struct dma_slave_config slave_config; > > + > > + dma_cap_zero(mask); > > + dma_cap_set(info->cap, mask); > > + > > + chan = dma_request_channel(mask, pl330_filter, (void *)dma_ch); > > + > > + if (info->direction == DMA_FROM_DEVICE) { > > + memset(&slave_config, 0, sizeof(struct dma_slave_config)); > > + slave_config.direction = info->direction; > > + slave_config.src_addr = info->fifo; > > + slave_config.src_addr_width = info->width; > > This should really set slave_config.src_maxburst to something sensible too, > even if that's just '1'. I will address your comment. > > > + dmaengine_slave_config(chan, &slave_config); > > + } else if (info->direction == DMA_TO_DEVICE) { > > + memset(&slave_config, 0, sizeof(struct dma_slave_config)); > > + slave_config.direction = info->direction; > > + slave_config.dst_addr = info->fifo; > > + slave_config.dst_addr_width = info->width; > > Ditto for dst_maxburst. > > > + dmaengine_slave_config(chan, &slave_config); > > + } > > + > > + return (unsigned)chan; > > I hope these interfaces yet cleaned away soon so that these casts can be > killed off. > > > +struct samsung_dma_prep_info { > > + enum dma_transaction_type cap; > > + enum dma_data_direction direction; > > + unsigned buf; > > dma_addr_t ? > > > + unsigned long period; > > + unsigned long len; > > + void (*fp)(void *data); > > + void *fp_param; > > +}; > > + > > +struct samsung_dma_info { > > + enum dma_transaction_type cap; > > + enum dma_data_direction direction; > > + enum dma_slave_buswidth width; > > + unsigned fifo; > > dma_addr_t ? > > > + struct s3c2410_dma_client *client; > > +}; > > + > > +struct samsung_dma_ops { > > + bool init; > > Unused? Yes, It's my mistake. I will address your comment.
On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim <boojin.kim@samsung.com> wrote: > + > +static bool pl330_filter(struct dma_chan *chan, void *param) > +{ > + struct dma_pl330_peri *peri = (struct dma_pl330_peri *)chan->private; > + unsigned dma_ch = (unsigned)param; > + > + if (peri->peri_id != dma_ch) > + return false; > + > + return true; > +} This is what I meant... if we keep chan_id for paltform assigned IDs, these filter functions could simply become static bool pl330_filter(struct dma_chan *chan, void *param) { return chan->chan_id == param } And ideally in the long run, we could just drop the filter callback and add expected channel ID to the request_channel call. > + > +static inline int s3c_dma_trigger(unsigned ch) > +{ > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START); > +} > + > +static inline int s3c_dma_started(unsigned ch) > +{ > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED); > +} > + > +static inline int s3c_dma_flush(unsigned ch) > +{ > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH); > +} > + > +static inline int s3c_dma_stop(unsigned ch) > +{ > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP); > +} > + > +static struct samsung_dma_ops s3c_dma_ops = { > + .request = s3c_dma_request, > + .release = s3c_dma_release, > + .prepare = s3c_dma_prepare, > + .trigger = s3c_dma_trigger, > + .started = s3c_dma_started, > + .flush = s3c_dma_flush, > + .stop = s3c_dma_stop, These last 4 should be gnereallized into one callback with OP argument.
On Mon, Jul 25, 2011 at 05:21:44PM +0530, Jassi Brar wrote: > On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim <boojin.kim@samsung.com> wrote: > > > + > > +static bool pl330_filter(struct dma_chan *chan, void *param) > > +{ > > + struct dma_pl330_peri *peri = (struct dma_pl330_peri *)chan->private; > > + unsigned dma_ch = (unsigned)param; > > + > > + if (peri->peri_id != dma_ch) > > + return false; > > + > > + return true; > > +} > This is what I meant... if we keep chan_id for paltform assigned IDs, > these filter functions could simply become > > static bool pl330_filter(struct dma_chan *chan, void *param) > { > return chan->chan_id == param > } > > And ideally in the long run, we could just drop the filter callback > and add expected channel ID to the request_channel call. So what if you have a PL080 and PL330 drivers registered with the DMA engine code, and you need to match a specific PL330 channel?
Jassi Brar Wrote: > Sent: Monday, July 25, 2011 8:52 PM > To: Boojin Kim > Cc: linux-arm-kernel@lists.infradead.org; linux-samsung- > soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant > Likely; Mark Brown > Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations > > On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim <boojin.kim@samsung.com> > wrote: > > > + > > +static bool pl330_filter(struct dma_chan *chan, void *param) > > +{ > > + struct dma_pl330_peri *peri = (struct dma_pl330_peri *)chan- > >private; > > + unsigned dma_ch = (unsigned)param; > > + > > + if (peri->peri_id != dma_ch) > > + return false; > > + > > + return true; > > +} > This is what I meant... if we keep chan_id for paltform assigned IDs, > these filter functions could simply become > > static bool pl330_filter(struct dma_chan *chan, void *param) > { > return chan->chan_id == param > } > > And ideally in the long run, we could just drop the filter callback > and add expected channel ID to the request_channel call. The chan_id is set by dmaengine. So, We don't use it to hold the user specific Id. > > > > > + > > +static inline int s3c_dma_trigger(unsigned ch) > > +{ > > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START); > > +} > > + > > +static inline int s3c_dma_started(unsigned ch) > > +{ > > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED); > > +} > > + > > +static inline int s3c_dma_flush(unsigned ch) > > +{ > > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH); > > +} > > + > > +static inline int s3c_dma_stop(unsigned ch) > > +{ > > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP); > > +} > > + > > +static struct samsung_dma_ops s3c_dma_ops = { > > + .request = s3c_dma_request, > > + .release = s3c_dma_release, > > + .prepare = s3c_dma_prepare, > > + .trigger = s3c_dma_trigger, > > + .started = s3c_dma_started, > > + .flush = s3c_dma_flush, > > + .stop = s3c_dma_stop, > > These last 4 should be gnereallized into one callback with OP argument. I don't have any idea about it. Can you explain it in more detail?
On Tue, Jul 26, 2011 at 1:32 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Mon, Jul 25, 2011 at 05:21:44PM +0530, Jassi Brar wrote: >> On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim <boojin.kim@samsung.com> wrote: >> >> > + >> > +static bool pl330_filter(struct dma_chan *chan, void *param) >> > +{ >> > + struct dma_pl330_peri *peri = (struct dma_pl330_peri *)chan->private; >> > + unsigned dma_ch = (unsigned)param; >> > + >> > + if (peri->peri_id != dma_ch) >> > + return false; >> > + >> > + return true; >> > +} >> This is what I meant... if we keep chan_id for paltform assigned IDs, >> these filter functions could simply become >> >> static bool pl330_filter(struct dma_chan *chan, void *param) >> { >> return chan->chan_id == param >> } >> >> And ideally in the long run, we could just drop the filter callback >> and add expected channel ID to the request_channel call. > > So what if you have a PL080 and PL330 drivers registered with the DMA > engine code, and you need to match a specific PL330 channel? > Before we start, let me be clear that I don't say it's all possible today as such. We will need changes to the API and modifications to the damc/client drivers (which are already misusing the API as it turns out). I hope we agree, clients (esp 'generic' ones of the future) should not ask for channels from a particular dmac. They should simply tell DMA API, what capabilities they expect from the allocated channel - of whatever index on whichever dmac. And the platform should have specified capabilities of a channel while introducing it to the DMA API. Yes, imho, DMA API should only work with a pool of channels and not dmacs - it shouldn't matter from which dmac a channel comes. So when a client asks for a channel, the dmaengine would return the first free channel that has just as much the requested capability. That is, the generic filter function would look equivalent of :- return is_free(chan) && (cap_requested == cap_of(chan) & cap_requested) Now it all boils down to defining the set of _practically_ possible capabilities and a method to exchange them between the API, Client and DMAC drivers. The above is the overall approach. The following is my idea of implementing it as of now, I am open to suggestions. Assuming :- *********** There are no more than 256 types of DMA'able devices (MMC, USB, SPI etc) -- [8bits] A type of device never has more than 16 instances in a platform (MMC-0, MMC-1, MMC-2 etc) -- [4bits] Mem->Mem, Mem->Dev, Dev->Mem, Dev->Dev capability in [4bits] Max_Burst_Len for any channel is less than 64KB, in power of two [4bits] Support programmable Max_Burst_Len(tunable in geometric steps of 2) [1bit] Max_RW_width/fifo_width is less than 128, in power of two -- [3bits] Support programmable Max_RW_Width(tunable in geometric steps of 2) [1bit] Finally, No platform has more than 128 channels with identicial capabilities [7bits] For a channel, when specific values of these fields are appended together, we get a unqiue 32-bits value called 'chan_id' Though we can use 64-bits to store the capabilities if we think number or size of these fields need to be increased. Thanks, Jassi
On Tue, Jul 26, 2011 at 11:13:35PM +0530, Jassi Brar wrote: > Before we start, let me be clear that I don't say it's all possible > today as such. It _is_ possible today. 1. Arrange for individual DMA engine drivers to provide a filter function - eg, pl08x_filter_id() for pl08x channels. 2. Have the filter function check chan->device->dev->driver == its own struct driver as the very first thing, and reject on failure. 3. Have its own matching algorithm using whatever data is appropriate for the DMA engine driver. 4. Platforms (or SoC code) provide the match function plus match data via platform data to each driver. That is essentially (except for (2)) how we deal with differing DMA engine drivers with AMBA peripherals, and it seems to work remarkably well. > I hope we agree, clients (esp 'generic' ones of the future) > should not ask for channels from a particular dmac. > They should simply tell DMA API, what capabilities they expect > from the allocated channel - of whatever index on whichever dmac. > And the platform should have specified capabilities of a channel > while introducing it to the DMA API. Yes, imho, DMA API should > only work with a pool of channels and not dmacs - it shouldn't > matter from which dmac a channel comes. You're quite clearly confused. "DMA API" has not very much to do with the subject we're discussing. The "DMA API" is the API for mapping and unmapping buffers for DMA. It has not much to do with the DMA engine API. So I'm not going to bother trying to decode what you said above until you start using the right terminology. I'm a stickler for that, get used to it. No point talking about how round bananas are when you're actually discussing orange fruit. > So when a client asks for a channel, the dmaengine would return > the first free channel that has just as much the requested capability. No. For slave DMA, peripheral drivers normally need a specific channel on the DMA controller. Peripheral drivers normally do not know which channel they need, or even which DMA controller they require. That information is specific to the platform and SoC. So, returning the "first free channel that matches the capability" is senseless - insane - for slave DMA. > That is, the generic filter function would look equivalent of :- > > return is_free(chan) && (cap_requested == cap_of(chan) & cap_requested) > > > Now it all boils down to defining the set of _practically_ possible capabilities > and a method to exchange them between the API, Client and DMAC drivers. What are these capabilities? > The above is the overall approach. > > The following is my idea of implementing it as of now, I am open to suggestions. > > Assuming :- > *********** > There are no more than 256 types of DMA'able devices > (MMC, USB, SPI etc) -- [8bits] > > A type of device never has more than 16 instances in a platform > (MMC-0, MMC-1, MMC-2 etc) -- [4bits] > > Mem->Mem, Mem->Dev, Dev->Mem, Dev->Dev capability in [4bits] No. You're being far too specific to your own DMA controller. Many DMA controllers (eg, PL08x) have a number of physical channels, all of which can be programmed for M2M, M2P, P2M and P2P. There is nothing in the design which restricts what a channel can do. So to put that as the basis for a generic match algorithm is absolutely senseless. > Max_Burst_Len for any channel is less than 64KB, in power of two [4bits] > Support programmable Max_Burst_Len(tunable in geometric steps of 2) [1bit] > > Max_RW_width/fifo_width is less than 128, in power of two -- [3bits] > Support programmable Max_RW_Width(tunable in geometric steps of 2) [1bit] > > Finally, No platform has more than 128 channels with identicial > capabilities [7bits] There is absolutely no way that this will work in reality - are you on a different planet? As has already been described to you, MMCI can use one DMA channel, and programs it according to the DMA direction, and has in the past adjusted the burst size according to what's going on with the driver. With your model, it would have to claim N channels from the DMA engine for all possible different configurations. That's utterly unworkable and unrealistic. So no, I don't think your idea of 'capabilities' is any solution. It just creates yet more problems - more than it solves.
On Tue, Jul 26, 2011 at 11:44 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > 1. Arrange for individual DMA engine drivers to provide a filter > function - eg, pl08x_filter_id() for pl08x channels. > > 2. Have the filter function check chan->device->dev->driver == its > own struct driver as the very first thing, and reject on failure. > > 3. Have its own matching algorithm using whatever data is appropriate > for the DMA engine driver. IMHO it should be possible to have client drivers not use platform specific details while choosing a channel. Otherwise, they are not really 'generic' 'platform-independent' client drivers. >> I hope we agree, clients (esp 'generic' ones of the future) >> should not ask for channels from a particular dmac. >> They should simply tell DMA API, what capabilities they expect >> from the allocated channel - of whatever index on whichever dmac. >> And the platform should have specified capabilities of a channel >> while introducing it to the DMA API. Yes, imho, DMA API should >> only work with a pool of channels and not dmacs - it shouldn't >> matter from which dmac a channel comes. > > You're quite clearly confused. "DMA API" has not very much to do with > the subject we're discussing. The "DMA API" is the API for mapping and > unmapping buffers for DMA. It has not much to do with the DMA engine > API. So I'm not going to bother trying to decode what you said above > until you start using the right terminology. I'm a stickler for that, > get used to it. > > No point talking about how round bananas are when you're actually > discussing orange fruit. Sorry, my carelessness. I should have written DMAENGINE API, instead of DMA API. If you want I can repost with s/DMA API/DMAENGINE API/ > >> So when a client asks for a channel, the dmaengine would return >> the first free channel that has just as much the requested capability. > > No. For slave DMA, peripheral drivers normally need a specific > channel on the DMA controller. Of course they do. And It is the responsibility of platform to encode such information as a 'capability' of the channel. Capability to reach a particular peripheral, say. > Peripheral drivers normally do not > know which channel they need, or even which DMA controller they > require. That information is specific to the platform and SoC. For that very reason I proposed, say, if MMC client driver is probed for 2nd instance of MMC controller, it should ask for the following capabilities (MMC << TYPE_SHFT) | (2 << INST_SHFT) //I don't show other fields for simplicity It is the job of _platform_ to set TYPE and INSTANCE fields to 'MMC' and 2 resp _only_ for the channel(s) that can reach its MMC_2 controller. That way, when the client 'generically' asks for DMA_MMC_2, it will be given only one of those channels that can talk to second instance of MMC controller. >> That is, the generic filter function would look equivalent of :- >> >> return is_free(chan) && (cap_requested == cap_of(chan) & cap_requested) >> >> >> Now it all boils down to defining the set of _practically_ possible capabilities >> and a method to exchange them between the API, Client and DMAC drivers. > > What are these capabilities? I had mentioned the list below under 'Assuming'. We may add more. > >> The above is the overall approach. >> >> The following is my idea of implementing it as of now, I am open to suggestions. >> >> Assuming :- >> *********** >> There are no more than 256 types of DMA'able devices >> (MMC, USB, SPI etc) -- [8bits] >> >> A type of device never has more than 16 instances in a platform >> (MMC-0, MMC-1, MMC-2 etc) -- [4bits] >> >> Mem->Mem, Mem->Dev, Dev->Mem, Dev->Dev capability in [4bits] > > No. You're being far too specific to your own DMA controller. Many > DMA controllers (eg, PL08x) have a number of physical channels, all of > which can be programmed for M2M, M2P, P2M and P2P. There is nothing > in the design which restricts what a channel can do. I am not being specific, at least not on this point. Btw, I have worked on pl080 as well and I have tried to be not specific to not only pl330 but also pl080. If you notice I have assigned 4bits to the field. Each of the 4 bits can be set independently. So, a capable pl08x channel can set all 4 bits to indicate that it can do all 4 types - M2M, M2P, P2M and P2P. >> Max_Burst_Len for any channel is less than 64KB, in power of two [4bits] >> Support programmable Max_Burst_Len(tunable in geometric steps of 2) [1bit] >> >> Max_RW_width/fifo_width is less than 128, in power of two -- [3bits] >> Support programmable Max_RW_Width(tunable in geometric steps of 2) [1bit] >> >> Finally, No platform has more than 128 channels with identicial >> capabilities [7bits] > > There is absolutely no way that this will work in reality - are you on > a different planet? As has already been described to you, MMCI can > use one DMA channel, and programs it according to the DMA direction, > and has in the past adjusted the burst size according to what's going > on with the driver. > > With your model, it would have to claim N channels from the DMA > engine for all possible different configurations. I don't think so. What I proposed is only a list of capabilities and method to _select_ a suitable channel in a platform independent way. For example, if MMCI driver wants to be able to change directions, it should specify Mem->Dev and Dev->Mem capability while requesting a channel. And use slave_config call to switch directions. (and yes we have to modify slave_config call and maybe some other parts too) For being able to change burst size, it could set some 'ignore' bit + default value to the 'fifo_width' field so that DMAENGINE ignores checking the fifo_width capability and upon slave_config either set requested burst_size if supported or return error if the requested burst size is not supported. That is implementation detail. Sincerely, I would to know other capabilities that are more flexible than I have assumed here or missed altogether. I have not yet written any code. I just wanted to share my belief that it is possible to have client drivers truly independent of the platform they run on. And by not using platform_data of DMA channels. Probably you don't, but if you do want, I can send rudimentary implementation of what I propose, in a few days. Thanks
On Tue, Jul 26, 2011 at 3:05 PM, Boojin Kim <boojin.kim@samsung.com> wrote: > Jassi Brar Wrote: >> Sent: Monday, July 25, 2011 8:52 PM >> To: Boojin Kim >> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung- >> soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant >> Likely; Mark Brown >> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations >> >> On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim <boojin.kim@samsung.com> >> wrote: >> >> > + >> > +static bool pl330_filter(struct dma_chan *chan, void *param) >> > +{ >> > + struct dma_pl330_peri *peri = (struct dma_pl330_peri *)chan- >> >private; >> > + unsigned dma_ch = (unsigned)param; >> > + >> > + if (peri->peri_id != dma_ch) >> > + return false; >> > + >> > + return true; >> > +} >> This is what I meant... if we keep chan_id for paltform assigned IDs, >> these filter functions could simply become >> >> static bool pl330_filter(struct dma_chan *chan, void *param) >> { >> return chan->chan_id == param >> } >> >> And ideally in the long run, we could just drop the filter callback >> and add expected channel ID to the request_channel call. > The chan_id is set by dmaengine. So, We don't use it to hold the user specific > Id. Not for long. See https://lkml.org/lkml/2011/7/26/245 >> > + >> > +static inline int s3c_dma_trigger(unsigned ch) >> > +{ >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START); >> > +} >> > + >> > +static inline int s3c_dma_started(unsigned ch) >> > +{ >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED); >> > +} >> > + >> > +static inline int s3c_dma_flush(unsigned ch) >> > +{ >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH); >> > +} >> > + >> > +static inline int s3c_dma_stop(unsigned ch) >> > +{ >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP); >> > +} >> > + >> > +static struct samsung_dma_ops s3c_dma_ops = { >> > + .request = s3c_dma_request, >> > + .release = s3c_dma_release, >> > + .prepare = s3c_dma_prepare, >> > + .trigger = s3c_dma_trigger, >> > + .started = s3c_dma_started, >> > + .flush = s3c_dma_flush, >> > + .stop = s3c_dma_stop, >> >> These last 4 should be gnereallized into one callback with OP argument. > I don't have any idea about it. Can you explain it in more detail? static int s3c_dma_control(unsigned ch, enum s3c2410_chan_op/*or similar*/ op) { return s3c2410_dma_ctrl(ch, op); } static struct samsung_dma_ops s3c_dma_ops = { .request = s3c_dma_request, .release = s3c_dma_release, .prepare = s3c_dma_prepare, .control = s3c_dma_control,
Jassi Brar wrote: > Sent: Wednesday, July 27, 2011 10:34 AM > To: Boojin Kim > Cc: linux-arm-kernel@lists.infradead.org; linux-samsung- > soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant > Likely; Mark Brown > Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations > > On Tue, Jul 26, 2011 at 3:05 PM, Boojin Kim <boojin.kim@samsung.com> > wrote: > > Jassi Brar Wrote: > >> Sent: Monday, July 25, 2011 8:52 PM > >> To: Boojin Kim > >> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung- > >> soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant > >> Likely; Mark Brown > >> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA > operations > >> > >> On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim > <boojin.kim@samsung.com> > >> wrote: > >> > >> > + > >> > +static bool pl330_filter(struct dma_chan *chan, void *param) > >> > +{ > >> > + struct dma_pl330_peri *peri = (struct dma_pl330_peri > *)chan- > >> >private; > >> > + unsigned dma_ch = (unsigned)param; > >> > + > >> > + if (peri->peri_id != dma_ch) > >> > + return false; > >> > + > >> > + return true; > >> > +} > >> This is what I meant... if we keep chan_id for paltform assigned > IDs, > >> these filter functions could simply become > >> > >> static bool pl330_filter(struct dma_chan *chan, void *param) > >> { > >> return chan->chan_id == param > >> } > >> > >> And ideally in the long run, we could just drop the filter callback > >> and add expected channel ID to the request_channel call. > > The chan_id is set by dmaengine. So, We don't use it to hold the > user specific > > Id. > Not for long. > See https://lkml.org/lkml/2011/7/26/245 > > > >> > + > >> > +static inline int s3c_dma_trigger(unsigned ch) > >> > +{ > >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START); > >> > +} > >> > + > >> > +static inline int s3c_dma_started(unsigned ch) > >> > +{ > >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED); > >> > +} > >> > + > >> > +static inline int s3c_dma_flush(unsigned ch) > >> > +{ > >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH); > >> > +} > >> > + > >> > +static inline int s3c_dma_stop(unsigned ch) > >> > +{ > >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP); > >> > +} > >> > + > >> > +static struct samsung_dma_ops s3c_dma_ops = { > >> > + .request = s3c_dma_request, > >> > + .release = s3c_dma_release, > >> > + .prepare = s3c_dma_prepare, > >> > + .trigger = s3c_dma_trigger, > >> > + .started = s3c_dma_started, > >> > + .flush = s3c_dma_flush, > >> > + .stop = s3c_dma_stop, > >> > >> These last 4 should be gnereallized into one callback with OP > argument. > > I don't have any idea about it. Can you explain it in more detail? > > static int s3c_dma_control(unsigned ch, enum s3c2410_chan_op/*or > similar*/ op) > { > return s3c2410_dma_ctrl(ch, op); > } > > static struct samsung_dma_ops s3c_dma_ops = { > .request = s3c_dma_request, > .release = s3c_dma_release, > .prepare = s3c_dma_prepare, > .control = s3c_dma_control, 'Struct samsung_dma_ops' is used for both 'S3C-DMA-OPS' for 's3c dma' and 'DMA-OPS' for 'dmaengine'. If I modify "Struct samsung_dma_ops" as your guide, It gives un-expectable effect to DMA-OPS. Actually, We don't want the client with 'DMA-OPS' uses 'enum s3c2410_chan_op' operation anymore.
On Wed, Jul 27, 2011 at 10:47 AM, Boojin Kim <boojin.kim@samsung.com> wrote: > Jassi Brar wrote: >> Sent: Wednesday, July 27, 2011 10:34 AM >> To: Boojin Kim >> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung- >> soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant >> Likely; Mark Brown >> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations >> >> On Tue, Jul 26, 2011 at 3:05 PM, Boojin Kim <boojin.kim@samsung.com> >> wrote: >> > Jassi Brar Wrote: >> >> Sent: Monday, July 25, 2011 8:52 PM >> >> To: Boojin Kim >> >> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung- >> >> soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant >> >> Likely; Mark Brown >> >> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA >> operations >> >> >> >> On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim >> <boojin.kim@samsung.com> >> >> wrote: >> >> >> >> > + >> >> > +static bool pl330_filter(struct dma_chan *chan, void *param) >> >> > +{ >> >> > + struct dma_pl330_peri *peri = (struct dma_pl330_peri >> *)chan- >> >> >private; >> >> > + unsigned dma_ch = (unsigned)param; >> >> > + >> >> > + if (peri->peri_id != dma_ch) >> >> > + return false; >> >> > + >> >> > + return true; >> >> > +} >> >> This is what I meant... if we keep chan_id for paltform assigned >> IDs, >> >> these filter functions could simply become >> >> >> >> static bool pl330_filter(struct dma_chan *chan, void *param) >> >> { >> >> return chan->chan_id == param >> >> } >> >> >> >> And ideally in the long run, we could just drop the filter callback >> >> and add expected channel ID to the request_channel call. >> > The chan_id is set by dmaengine. So, We don't use it to hold the >> user specific >> > Id. >> Not for long. >> See https://lkml.org/lkml/2011/7/26/245 >> >> >> >> > + >> >> > +static inline int s3c_dma_trigger(unsigned ch) >> >> > +{ >> >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START); >> >> > +} >> >> > + >> >> > +static inline int s3c_dma_started(unsigned ch) >> >> > +{ >> >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED); >> >> > +} >> >> > + >> >> > +static inline int s3c_dma_flush(unsigned ch) >> >> > +{ >> >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH); >> >> > +} >> >> > + >> >> > +static inline int s3c_dma_stop(unsigned ch) >> >> > +{ >> >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP); >> >> > +} >> >> > + >> >> > +static struct samsung_dma_ops s3c_dma_ops = { >> >> > + .request = s3c_dma_request, >> >> > + .release = s3c_dma_release, >> >> > + .prepare = s3c_dma_prepare, >> >> > + .trigger = s3c_dma_trigger, >> >> > + .started = s3c_dma_started, >> >> > + .flush = s3c_dma_flush, >> >> > + .stop = s3c_dma_stop, >> >> >> >> These last 4 should be gnereallized into one callback with OP >> argument. >> > I don't have any idea about it. Can you explain it in more detail? >> >> static int s3c_dma_control(unsigned ch, enum s3c2410_chan_op/*or >> similar*/ op) >> { >> return s3c2410_dma_ctrl(ch, op); >> } >> >> static struct samsung_dma_ops s3c_dma_ops = { >> .request = s3c_dma_request, >> .release = s3c_dma_release, >> .prepare = s3c_dma_prepare, >> .control = s3c_dma_control, > 'Struct samsung_dma_ops' is used for both 'S3C-DMA-OPS' for 's3c dma' and > 'DMA-OPS' for 'dmaengine'. > If I modify "Struct samsung_dma_ops" as your guide, It gives un-expectable > effect to DMA-OPS. > Actually, We don't want the client with 'DMA-OPS' uses 'enum s3c2410_chan_op' > operation anymore. " enum s3c2410_chan_op /* or similar */ " <<<< note 'or similar' Defining a callback for each value of the argument doesn't make sense.
Jassi Brar wrote: > Sent: Wednesday, July 27, 2011 4:58 PM > To: Boojin Kim > Cc: Kukjin Kim; Vinod Koul; Mark Brown; Grant Likely; linux-samsung- > soc@vger.kernel.org; Dan Williams; linux-arm- > kernel@lists.infradead.org > Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations > > On Wed, Jul 27, 2011 at 10:47 AM, Boojin Kim <boojin.kim@samsung.com> > wrote: > > Jassi Brar wrote: > >> Sent: Wednesday, July 27, 2011 10:34 AM > >> To: Boojin Kim > >> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung- > >> soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant > >> Likely; Mark Brown > >> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA > operations > >> > >> On Tue, Jul 26, 2011 at 3:05 PM, Boojin Kim > <boojin.kim@samsung.com> > >> wrote: > >> > Jassi Brar Wrote: > >> >> Sent: Monday, July 25, 2011 8:52 PM > >> >> To: Boojin Kim > >> >> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung- > >> >> soc@vger.kernel.org; Vinod Koul; Dan Williams; Kukjin Kim; Grant > >> >> Likely; Mark Brown > >> >> Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA > >> operations > >> >> > >> >> On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim > >> <boojin.kim@samsung.com> > >> >> wrote: > >> >> > >> >> > + > >> >> > +static bool pl330_filter(struct dma_chan *chan, void *param) > >> >> > +{ > >> >> > + struct dma_pl330_peri *peri = (struct dma_pl330_peri > >> *)chan- > >> >> >private; > >> >> > + unsigned dma_ch = (unsigned)param; > >> >> > + > >> >> > + if (peri->peri_id != dma_ch) > >> >> > + return false; > >> >> > + > >> >> > + return true; > >> >> > +} > >> >> This is what I meant... if we keep chan_id for paltform assigned > >> IDs, > >> >> these filter functions could simply become > >> >> > >> >> static bool pl330_filter(struct dma_chan *chan, void *param) > >> >> { > >> >> return chan->chan_id == param > >> >> } > >> >> > >> >> And ideally in the long run, we could just drop the filter > callback > >> >> and add expected channel ID to the request_channel call. > >> > The chan_id is set by dmaengine. So, We don't use it to hold the > >> user specific > >> > Id. > >> Not for long. > >> See https://lkml.org/lkml/2011/7/26/245 > >> > >> > >> >> > + > >> >> > +static inline int s3c_dma_trigger(unsigned ch) > >> >> > +{ > >> >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START); > >> >> > +} > >> >> > + > >> >> > +static inline int s3c_dma_started(unsigned ch) > >> >> > +{ > >> >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED); > >> >> > +} > >> >> > + > >> >> > +static inline int s3c_dma_flush(unsigned ch) > >> >> > +{ > >> >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH); > >> >> > +} > >> >> > + > >> >> > +static inline int s3c_dma_stop(unsigned ch) > >> >> > +{ > >> >> > + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP); > >> >> > +} > >> >> > + > >> >> > +static struct samsung_dma_ops s3c_dma_ops = { > >> >> > + .request = s3c_dma_request, > >> >> > + .release = s3c_dma_release, > >> >> > + .prepare = s3c_dma_prepare, > >> >> > + .trigger = s3c_dma_trigger, > >> >> > + .started = s3c_dma_started, > >> >> > + .flush = s3c_dma_flush, > >> >> > + .stop = s3c_dma_stop, > >> >> > >> >> These last 4 should be gnereallized into one callback with OP > >> argument. > >> > I don't have any idea about it. Can you explain it in more detail? > >> > >> static int s3c_dma_control(unsigned ch, enum s3c2410_chan_op/*or > >> similar*/ op) > >> { > >> return s3c2410_dma_ctrl(ch, op); > >> } > >> > >> static struct samsung_dma_ops s3c_dma_ops = { > >> .request = s3c_dma_request, > >> .release = s3c_dma_release, > >> .prepare = s3c_dma_prepare, > >> .control = s3c_dma_control, > > 'Struct samsung_dma_ops' is used for both 'S3C-DMA-OPS' for 's3c > dma' and > > 'DMA-OPS' for 'dmaengine'. > > If I modify "Struct samsung_dma_ops" as your guide, It gives un- > expectable > > effect to DMA-OPS. > > Actually, We don't want the client with 'DMA-OPS' uses 'enum > s3c2410_chan_op' > > operation anymore. > > " enum s3c2410_chan_op /* or similar */ " <<<< note 'or similar' > > Defining a callback for each value of the argument doesn't make sense. Yes, The command may not be 'enum s3c2410_chan_op'. But it's very similar with 'enum s3c2410_chan_op'. So, It brings same result that I sad. Actually, I quietly agree with your comment. If the 'struct samsung_dma_ops' is only used for 's3c-dma-ops', I would address your comment. But, 'struct samsung_dma_ops' is used for all Samsung dma clients. I aim that 'struct samsung_dma_ops' provide the DMA clients with 'dmaengine' friendly DMA interface without any other command.
diff --git a/arch/arm/mach-s3c2410/include/mach/dma.h b/arch/arm/mach-s3c2410/include/mach/dma.h index b2b2a5b..e61a91f 100644 --- a/arch/arm/mach-s3c2410/include/mach/dma.h +++ b/arch/arm/mach-s3c2410/include/mach/dma.h @@ -13,7 +13,6 @@ #ifndef __ASM_ARCH_DMA_H #define __ASM_ARCH_DMA_H __FILE__ -#include <plat/dma.h> #include <linux/sysdev.h> #define MAX_DMA_TRANSFER_SIZE 0x100000 /* Data Unit is half word */ @@ -51,6 +50,13 @@ enum dma_ch { DMACH_MAX, /* the end entry */ }; +static inline bool samsung_dma_is_dmadev(void) +{ + return false; +} + +#include <plat/dma.h> + #define DMACH_LOW_LEVEL (1<<28) /* use this to specifiy hardware ch no */ /* we have 4 dma channels */ diff --git a/arch/arm/mach-s3c64xx/include/mach/dma.h b/arch/arm/mach-s3c64xx/include/mach/dma.h index 0a5d926..49c3a53 100644 --- a/arch/arm/mach-s3c64xx/include/mach/dma.h +++ b/arch/arm/mach-s3c64xx/include/mach/dma.h @@ -63,6 +63,10 @@ static __inline__ bool s3c_dma_has_circular(void) return true; } +static inline bool samsung_dma_is_dmadev(void) +{ + return false; +} #define S3C2410_DMAF_CIRCULAR (1 << 0) #include <plat/dma.h> diff --git a/arch/arm/plat-samsung/Makefile b/arch/arm/plat-samsung/Makefile index 53eb15b..9ecf2aa 100644 --- a/arch/arm/plat-samsung/Makefile +++ b/arch/arm/plat-samsung/Makefile @@ -62,9 +62,11 @@ obj-$(CONFIG_SAMSUNG_DEV_PWM) += dev-pwm.o # DMA support -obj-$(CONFIG_S3C_DMA) += dma.o +obj-$(CONFIG_S3C_DMA) += dma.o s3c-dma-ops.o -obj-$(CONFIG_S3C_PL330_DMA) += s3c-pl330.o +obj-$(CONFIG_DMADEV_PL330) += dma-ops.o + +obj-$(CONFIG_S3C_PL330_DMA) += s3c-pl330.o s3c-dma-ops.o # PM support diff --git a/arch/arm/plat-samsung/dma-ops.c b/arch/arm/plat-samsung/dma-ops.c new file mode 100644 index 0000000..9053433 --- /dev/null +++ b/arch/arm/plat-samsung/dma-ops.c @@ -0,0 +1,133 @@ +/* linux/arch/arm/plat-samsung/dma-ops.c + * + * Copyright (c) 2011 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Samsung DMA Operations + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/kernel.h> +#include <linux/errno.h> +#include <linux/amba/pl330.h> + +#include <mach/dma.h> + +static bool pl330_filter(struct dma_chan *chan, void *param) +{ + struct dma_pl330_peri *peri = (struct dma_pl330_peri *)chan->private; + unsigned dma_ch = (unsigned)param; + + if (peri->peri_id != dma_ch) + return false; + + return true; +} + +static unsigned samsung_dmadev_request(enum dma_ch dma_ch, + struct samsung_dma_info *info) +{ + struct dma_chan *chan; + dma_cap_mask_t mask; + struct dma_slave_config slave_config; + + dma_cap_zero(mask); + dma_cap_set(info->cap, mask); + + chan = dma_request_channel(mask, pl330_filter, (void *)dma_ch); + + if (info->direction == DMA_FROM_DEVICE) { + memset(&slave_config, 0, sizeof(struct dma_slave_config)); + slave_config.direction = info->direction; + slave_config.src_addr = info->fifo; + slave_config.src_addr_width = info->width; + dmaengine_slave_config(chan, &slave_config); + } else if (info->direction == DMA_TO_DEVICE) { + memset(&slave_config, 0, sizeof(struct dma_slave_config)); + slave_config.direction = info->direction; + slave_config.dst_addr = info->fifo; + slave_config.dst_addr_width = info->width; + dmaengine_slave_config(chan, &slave_config); + } + + return (unsigned)chan; +} + +static int samsung_dmadev_release(unsigned ch, + struct s3c2410_dma_client *client) +{ + dma_release_channel((struct dma_chan *)ch); + + return 0; +} + +static int samsung_dmadev_prepare(unsigned ch, + struct samsung_dma_prep_info *info) +{ + struct scatterlist sg; + struct dma_chan *chan = (struct dma_chan *)ch; + struct dma_async_tx_descriptor *desc; + + switch (info->cap) { + case DMA_SLAVE: + sg_init_table(&sg, 1); + sg_dma_len(&sg) = info->len; + sg_set_page(&sg, pfn_to_page(PFN_DOWN(info->buf)), + info->len, offset_in_page(info->buf)); + sg_dma_address(&sg) = info->buf; + + desc = chan->device->device_prep_slave_sg(chan, + &sg, 1, info->direction, DMA_PREP_INTERRUPT); + break; + case DMA_CYCLIC: + desc = chan->device->device_prep_dma_cyclic(chan, + info->buf, info->len, info->period, info->direction); + break; + default: + dev_err(&chan->dev->device, "unsupported format\n"); + return -EFAULT; + } + + if (!desc) { + dev_err(&chan->dev->device, "cannot prepare cyclic dma\n"); + return -EFAULT; + } + + desc->callback = info->fp; + desc->callback_param = info->fp_param; + + dmaengine_submit((struct dma_async_tx_descriptor *)desc); + + return 0; +} + +static inline int samsung_dmadev_trigger(unsigned ch) +{ + dma_async_issue_pending((struct dma_chan *)ch); + + return 0; +} + +static inline int samsung_dmadev_flush(unsigned ch) +{ + return dmaengine_terminate_all((struct dma_chan *)ch); +} + +struct samsung_dma_ops dmadev_ops = { + .request = samsung_dmadev_request, + .release = samsung_dmadev_release, + .prepare = samsung_dmadev_prepare, + .trigger = samsung_dmadev_trigger, + .started = NULL, + .flush = samsung_dmadev_flush, + .stop = samsung_dmadev_flush, +}; + +void *samsung_dmadev_get_ops(void) +{ + return &dmadev_ops; +} +EXPORT_SYMBOL(samsung_dmadev_get_ops); diff --git a/arch/arm/plat-samsung/include/plat/dma-ops.h b/arch/arm/plat-samsung/include/plat/dma-ops.h new file mode 100644 index 0000000..e5a68c0 --- /dev/null +++ b/arch/arm/plat-samsung/include/plat/dma-ops.h @@ -0,0 +1,64 @@ +/* arch/arm/plat-samsung/include/plat/dma-ops.h + * + * Copyright (c) 2011 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Samsung DMA support + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef __SAMSUNG_DMA_OPS_H_ +#define __SAMSUNG_DMA_OPS_H_ __FILE__ + +#include <linux/dmaengine.h> + +struct samsung_dma_prep_info { + enum dma_transaction_type cap; + enum dma_data_direction direction; + unsigned buf; + unsigned long period; + unsigned long len; + void (*fp)(void *data); + void *fp_param; +}; + +struct samsung_dma_info { + enum dma_transaction_type cap; + enum dma_data_direction direction; + enum dma_slave_buswidth width; + unsigned fifo; + struct s3c2410_dma_client *client; +}; + +struct samsung_dma_ops { + bool init; + unsigned (*request)(enum dma_ch ch, struct samsung_dma_info *info); + int (*release)(unsigned ch, struct s3c2410_dma_client *client); + int (*prepare)(unsigned ch, struct samsung_dma_prep_info *info); + int (*trigger)(unsigned ch); + int (*started)(unsigned ch); + int (*flush)(unsigned ch); + int (*stop)(unsigned ch); +}; + +extern void *samsung_dmadev_get_ops(void); +extern void *s3c_dma_get_ops(void); + +static inline void *__samsung_dma_get_ops(void) +{ + if (samsung_dma_is_dmadev()) + return samsung_dmadev_get_ops(); + else + return s3c_dma_get_ops(); +} + +/* + * samsung_dma_get_ops + * get the set of samsung dma operations + */ +#define samsung_dma_get_ops() __samsung_dma_get_ops() + +#endif /* __SAMSUNG_DMA_OPS_H_ */ diff --git a/arch/arm/plat-samsung/include/plat/dma-pl330.h b/arch/arm/plat-samsung/include/plat/dma-pl330.h index c402719..687489e 100644 --- a/arch/arm/plat-samsung/include/plat/dma-pl330.h +++ b/arch/arm/plat-samsung/include/plat/dma-pl330.h @@ -100,6 +100,10 @@ static inline bool s3c_dma_has_circular(void) return true; } +static inline bool samsung_dma_is_dmadev(void) +{ + return true; +} #include <plat/dma.h> #endif /* __DMA_PL330_H_ */ diff --git a/arch/arm/plat-samsung/include/plat/dma.h b/arch/arm/plat-samsung/include/plat/dma.h index 2e8f8c6..e87c6be 100644 --- a/arch/arm/plat-samsung/include/plat/dma.h +++ b/arch/arm/plat-samsung/include/plat/dma.h @@ -125,3 +125,4 @@ extern int s3c2410_dma_set_opfn(unsigned int, s3c2410_dma_opfn_t rtn); extern int s3c2410_dma_set_buffdone_fn(unsigned int, s3c2410_dma_cbfn_t rtn); +#include <plat/dma-ops.h> diff --git a/arch/arm/plat-samsung/s3c-dma-ops.c b/arch/arm/plat-samsung/s3c-dma-ops.c new file mode 100644 index 0000000..33ab324 --- /dev/null +++ b/arch/arm/plat-samsung/s3c-dma-ops.c @@ -0,0 +1,133 @@ +/* linux/arch/arm/plat-samsung/s3c-dma-ops.c + * + * Copyright (c) 2011 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Samsung S3C-DMA Operations + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/kernel.h> +#include <linux/errno.h> +#include <linux/slab.h> +#include <linux/types.h> + +#include <mach/dma.h> + +struct cb_data { + void (*fp) (void *); + void *fp_param; + unsigned ch; + struct list_head node; +}; + +static LIST_HEAD(dma_list); + +static void s3c_dma_cb(struct s3c2410_dma_chan *channel, void *param, + int size, enum s3c2410_dma_buffresult res) +{ + struct cb_data *data = param; + + data->fp(data->fp_param); +} + +static unsigned s3c_dma_request(enum dma_ch dma_ch, + struct samsung_dma_info *info) +{ + struct cb_data *data; + + if (s3c2410_dma_request(dma_ch, info->client, NULL) < 0) { + s3c2410_dma_free(dma_ch, info->client); + return 0; + } + + data = kzalloc(sizeof(struct cb_data), GFP_KERNEL); + data->ch = dma_ch; + list_add_tail(&data->node, &dma_list); + + if (info->direction == DMA_FROM_DEVICE) + s3c2410_dma_devconfig(dma_ch, S3C2410_DMASRC_HW, info->fifo); + else + s3c2410_dma_devconfig(dma_ch, S3C2410_DMASRC_MEM, info->fifo); + + if (info->cap == DMA_CYCLIC) + s3c2410_dma_setflags(dma_ch, S3C2410_DMAF_CIRCULAR); + + s3c2410_dma_config(dma_ch, info->width); + + return (unsigned)dma_ch; +} + +static int s3c_dma_release(unsigned ch, struct s3c2410_dma_client *client) +{ + struct cb_data *data; + + list_for_each_entry(data, &dma_list, node) + if (data->ch == ch) + break; + list_del(&data->node); + + s3c2410_dma_free(ch, client); + kfree(data); + + return 0; +} + +static int s3c_dma_prepare(unsigned ch, struct samsung_dma_prep_info *info) +{ + struct cb_data *data; + int len = (info->cap == DMA_CYCLIC) ? info->period : info->len; + + list_for_each_entry(data, &dma_list, node) + if (data->ch == ch) + break; + + if (!data->fp) { + s3c2410_dma_set_buffdone_fn(ch, s3c_dma_cb); + data->fp = info->fp; + data->fp_param = info->fp_param; + } + + s3c2410_dma_enqueue(ch, (void *)data, info->buf, len); + + return 0; +} + +static inline int s3c_dma_trigger(unsigned ch) +{ + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_START); +} + +static inline int s3c_dma_started(unsigned ch) +{ + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STARTED); +} + +static inline int s3c_dma_flush(unsigned ch) +{ + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_FLUSH); +} + +static inline int s3c_dma_stop(unsigned ch) +{ + return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_STOP); +} + +static struct samsung_dma_ops s3c_dma_ops = { + .request = s3c_dma_request, + .release = s3c_dma_release, + .prepare = s3c_dma_prepare, + .trigger = s3c_dma_trigger, + .started = s3c_dma_started, + .flush = s3c_dma_flush, + .stop = s3c_dma_stop, +}; + +void *s3c_dma_get_ops(void) +{ + return &s3c_dma_ops; +} +EXPORT_SYMBOL(s3c_dma_get_ops);
This patch adds common DMA operations which are used for Samsung DMA drivers. Currently there are two types of DMA driver for Samsung SoCs. The one is S3C-DMA for S3C SoCs and the other is PL330-DMA for S5P SoCs. This patch provides funcion pointers for common DMA operations to DMA client driver like SPI and Audio. It makes DMA client drivers support multi-platform. In addition, this common DMA operations implement the shared actions that are needed for DMA client driver. For example shared actions are filter() function for dma_request_channel() and parameter passing for device_prep_slave_sg(). Signed-off-by: Boojin Kim <boojin.kim@samsung.com> --- arch/arm/mach-s3c2410/include/mach/dma.h | 8 ++- arch/arm/mach-s3c64xx/include/mach/dma.h | 4 + arch/arm/plat-samsung/Makefile | 6 +- arch/arm/plat-samsung/dma-ops.c | 133 ++++++++++++++++++++++++ arch/arm/plat-samsung/include/plat/dma-ops.h | 64 +++++++++++ arch/arm/plat-samsung/include/plat/dma-pl330.h | 4 + arch/arm/plat-samsung/include/plat/dma.h | 1 + arch/arm/plat-samsung/s3c-dma-ops.c | 133 ++++++++++++++++++++++++ 8 files changed, 350 insertions(+), 3 deletions(-) create mode 100644 arch/arm/plat-samsung/dma-ops.c create mode 100644 arch/arm/plat-samsung/include/plat/dma-ops.h create mode 100644 arch/arm/plat-samsung/s3c-dma-ops.c