From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boojin Kim Subject: RE: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations Date: Wed, 27 Jul 2011 14:17:13 +0900 Message-ID: <000401cc4c1c$71d63240$558296c0$%kim@samsung.com> References: <1311557312-26107-1-git-send-email-boojin.kim@samsung.com> <1311557312-26107-7-git-send-email-boojin.kim@samsung.com> <003801cc4b77$51e9f3e0$f5bddba0$%kim@samsung.com> Content-Transfer-Encoding: 7BIT Return-path: Received: from mailout1.samsung.com ([203.254.224.24]:64074 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751684Ab1G0FRQ (ORCPT ); Wed, 27 Jul 2011 01:17:16 -0400 Received: from epcpsbgm1.samsung.com (mailout1.samsung.com [203.254.224.24]) by mailout1.samsung.com (Oracle Communications Messaging Exchange Server 7u4-19.01 64bit (built Sep 7 2010)) with ESMTP id <0LOZ00JQW80BRK00@mailout1.samsung.com> for linux-samsung-soc@vger.kernel.org; Wed, 27 Jul 2011 14:17:14 +0900 (KST) Received: from DOBOOJINKIM03 (12-23-120-72.csky.net [12.23.120.72]) by mmp1.samsung.com (iPlanet Messaging Server 5.2 Patch 2 (built Jul 14 2004)) with ESMTPA id <0LOZ005WT80QIN@mmp1.samsung.com> for linux-samsung-soc@vger.kernel.org; Wed, 27 Jul 2011 14:17:14 +0900 (KST) In-reply-to: Content-language: ko Sender: linux-samsung-soc-owner@vger.kernel.org List-Id: linux-samsung-soc@vger.kernel.org To: 'Jassi Brar' Cc: linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, 'Vinod Koul' , 'Dan Williams' , 'Kukjin Kim' , 'Grant Likely' , 'Mark Brown' 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 > 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 > > >> 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. From mboxrd@z Thu Jan 1 00:00:00 1970 From: boojin.kim@samsung.com (Boojin Kim) Date: Wed, 27 Jul 2011 14:17:13 +0900 Subject: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations In-Reply-To: References: <1311557312-26107-1-git-send-email-boojin.kim@samsung.com> <1311557312-26107-7-git-send-email-boojin.kim@samsung.com> <003801cc4b77$51e9f3e0$f5bddba0$%kim@samsung.com> Message-ID: <000401cc4c1c$71d63240$558296c0$%kim@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Jassi Brar wrote: > Sent: Wednesday, July 27, 2011 10:34 AM > To: Boojin Kim > Cc: linux-arm-kernel at lists.infradead.org; linux-samsung- > soc at 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 > wrote: > > Jassi Brar Wrote: > >> Sent: Monday, July 25, 2011 8:52 PM > >> To: Boojin Kim > >> Cc: linux-arm-kernel at lists.infradead.org; linux-samsung- > >> soc at 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 > > >> 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.