From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jassi Brar Subject: Re: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations Date: Wed, 27 Jul 2011 07:03:36 +0530 Message-ID: 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> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mail-ew0-f46.google.com ([209.85.215.46]:60287 "EHLO mail-ew0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752014Ab1G0Bdh convert rfc822-to-8bit (ORCPT ); Tue, 26 Jul 2011 21:33:37 -0400 Received: by ewy4 with SMTP id 4so936268ewy.19 for ; Tue, 26 Jul 2011 18:33:36 -0700 (PDT) In-Reply-To: <003801cc4b77$51e9f3e0$f5bddba0$%kim@samsung.com> Sender: linux-samsung-soc-owner@vger.kernel.org List-Id: linux-samsung-soc@vger.kernel.org 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 On Tue, Jul 26, 2011 at 3:05 PM, Boojin Kim wr= ote: > 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 operation= s >> >> On Mon, Jul 25, 2011 at 6:58 AM, Boojin Kim >> wrote: >> >> > + >> > +static bool pl330_filter(struct dma_chan *chan, void *param) >> > +{ >> > + =C2=A0 =C2=A0 =C2=A0 struct dma_pl330_peri *peri =3D (struct dma= _pl330_peri *)chan- >> >private; >> > + =C2=A0 =C2=A0 =C2=A0 unsigned dma_ch =3D (unsigned)param; >> > + >> > + =C2=A0 =C2=A0 =C2=A0 if (peri->peri_id !=3D dma_ch) >> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return false; >> > + >> > + =C2=A0 =C2=A0 =C2=A0 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) >> { >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 return chan->chan_id =3D=3D 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) >> > +{ >> > + =C2=A0 =C2=A0 =C2=A0 return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_S= TART); >> > +} >> > + >> > +static inline int s3c_dma_started(unsigned ch) >> > +{ >> > + =C2=A0 =C2=A0 =C2=A0 return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_S= TARTED); >> > +} >> > + >> > +static inline int s3c_dma_flush(unsigned ch) >> > +{ >> > + =C2=A0 =C2=A0 =C2=A0 return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_F= LUSH); >> > +} >> > + >> > +static inline int s3c_dma_stop(unsigned ch) >> > +{ >> > + =C2=A0 =C2=A0 =C2=A0 return s3c2410_dma_ctrl(ch, S3C2410_DMAOP_S= TOP); >> > +} >> > + >> > +static struct samsung_dma_ops s3c_dma_ops =3D { >> > + =C2=A0 =C2=A0 =C2=A0 .request =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D s3c= _dma_request, >> > + =C2=A0 =C2=A0 =C2=A0 .release =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D s3c= _dma_release, >> > + =C2=A0 =C2=A0 =C2=A0 .prepare =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D s3c= _dma_prepare, >> > + =C2=A0 =C2=A0 =C2=A0 .trigger =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D s3c= _dma_trigger, >> > + =C2=A0 =C2=A0 =C2=A0 .started =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D s3c= _dma_started, >> > + =C2=A0 =C2=A0 =C2=A0 .flush =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D= s3c_dma_flush, >> > + =C2=A0 =C2=A0 =C2=A0 .stop =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D= s3c_dma_stop, >> >> These last 4 should be gnereallized into one callback with OP argume= nt. > 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 simila= r*/ op) { return s3c2410_dma_ctrl(ch, op); } static struct samsung_dma_ops s3c_dma_ops =3D { .request =3D s3c_dma_request, .release =3D s3c_dma_release, .prepare =3D s3c_dma_prepare, .control =3D s3c_dma_control, From mboxrd@z Thu Jan 1 00:00:00 1970 From: jassisinghbrar@gmail.com (Jassi Brar) Date: Wed, 27 Jul 2011 07:03:36 +0530 Subject: [PATCH V4 06/14] ARM: SAMSUNG: Add common DMA operations In-Reply-To: <003801cc4b77$51e9f3e0$f5bddba0$%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> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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,