From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boojin Kim Subject: RE: [PATCH V4 12/14] spi/s3c64xx: Add support DMA engine API Date: Wed, 27 Jul 2011 14:05:39 +0900 Message-ID: <000301cc4c1a$d494bd40$7dbe37c0$%kim@samsung.com> References: <1311557312-26107-1-git-send-email-boojin.kim@samsung.com> <1311557312-26107-13-git-send-email-boojin.kim@samsung.com> <1311592630.29703.13.camel@vkoul-mobl4> <003701cc4b76$c52963f0$4f7c2bd0$%kim@samsung.com> <1311675280.24316.3.camel@vkoul-mobl4> Content-Transfer-Encoding: 7BIT Return-path: Received: from mailout1.samsung.com ([203.254.224.24]:58708 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751972Ab1G0FFn (ORCPT ); Wed, 27 Jul 2011 01:05:43 -0400 Received: from epcpsbgm2.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 <0LOZ00JB77HFRK00@mailout1.samsung.com> for linux-samsung-soc@vger.kernel.org; Wed, 27 Jul 2011 14:05:40 +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 <0LOZ00ACF7HGP8@mmp1.samsung.com> for linux-samsung-soc@vger.kernel.org; Wed, 27 Jul 2011 14:05:41 +0900 (KST) In-reply-to: <1311675280.24316.3.camel@vkoul-mobl4> Content-language: ko Sender: linux-samsung-soc-owner@vger.kernel.org List-Id: linux-samsung-soc@vger.kernel.org To: 'Vinod Koul' Cc: vinod.koul@intel.com, 'Kukjin Kim' , 'Jassi Brar' , 'Grant Likely' , linux-samsung-soc@vger.kernel.org, 'Mark Brown' , 'Dan Williams' , linux-arm-kernel@lists.infradead.org Vinod Koul Wrote: > Sent: Tuesday, July 26, 2011 7:15 PM > To: Boojin Kim > Cc: vinod.koul@intel.com; 'Kukjin Kim'; 'Jassi Brar'; 'Grant Likely'; > linux-samsung-soc@vger.kernel.org; 'Mark Brown'; 'Dan Williams'; > linux-arm-kernel@lists.infradead.org > Subject: RE: [PATCH V4 12/14] spi/s3c64xx: Add support DMA engine API > > On Tue, 2011-07-26 at 18:31 +0900, Boojin Kim wrote: > > Vinod Koul Wrote: > > > Sent: Monday, July 25, 2011 8:17 PM > > > To: Boojin Kim > > > Cc: vinod.koul@intel.com; linux-arm-kernel@lists.infradead.org; > linux- > > > samsung-soc@vger.kernel.org; Kukjin Kim; Jassi Brar; Grant Likely; > > > Mark Brown; Dan Williams > > > Subject: Re: [PATCH V4 12/14] spi/s3c64xx: Add support DMA engine > API > > > > > > On Mon, 2011-07-25 at 10:28 +0900, Boojin Kim wrote: > > > > This patch adds to support DMA generic API to transfer raw > > > > SPI data. Basiclly the spi driver uses DMA generic API if > > > > architecture supports it. Otherwise, uses Samsung specific > > > > S3C-PL330 APIs. > > > > > > > > Signed-off-by: Boojin Kim > > > > Cc: Grant Likely > > > > Signed-off-by: Kukjin Kim > > > > --- > > > > drivers/spi/spi_s3c64xx.c | 141 ++++++++++++++++++++++--------- > ---- > > > ---------- > > > > 1 files changed, 69 insertions(+), 72 deletions(-) > > > > > > > > diff --git a/drivers/spi/spi_s3c64xx.c > b/drivers/spi/spi_s3c64xx.c > > > > index 8945e20..a4cf76a 100644 > > > > --- a/drivers/spi/spi_s3c64xx.c > > > > +++ b/drivers/spi/spi_s3c64xx.c > > > > @@ -172,6 +172,9 @@ struct s3c64xx_spi_driver_data { > > > > unsigned state; > > > > unsigned cur_mode, cur_bpw; > > > > unsigned cur_speed; > > > > + unsigned rx_ch; > > > > + unsigned tx_ch; > > > > + struct samsung_dma_ops *ops; > > > > }; > > > > > > > > static struct s3c2410_dma_client s3c64xx_spi_dma_client = { > > > > @@ -227,6 +230,38 @@ static void flush_fifo(struct > > > s3c64xx_spi_driver_data *sdd) > > > > writel(val, regs + S3C64XX_SPI_CH_CFG); > > > > } > > > > > > > > +static void s3c64xx_spi_dma_rxcb(void *data) > > > > +{ > > > > + struct s3c64xx_spi_driver_data *sdd > > > > + = (struct s3c64xx_spi_driver_data *)data; > > > > + unsigned long flags; > > > > + > > > > + spin_lock_irqsave(&sdd->lock, flags); > > > > + > > > > + sdd->state &= ~RXBUSY; > > > > + /* If the other done */ > > > > + if (!(sdd->state & TXBUSY)) > > > > + complete(&sdd->xfer_completion); > > > > + > > > > + spin_unlock_irqrestore(&sdd->lock, flags); > > > > +} > > > > + > > > > +static void s3c64xx_spi_dma_txcb(void *data) > > > > +{ > > > > + struct s3c64xx_spi_driver_data *sdd > > > > + = (struct s3c64xx_spi_driver_data *)data; > > > > + unsigned long flags; > > > > + > > > > + spin_lock_irqsave(&sdd->lock, flags); > > > > + > > > > + sdd->state &= ~TXBUSY; > > > > + /* If the other done */ > > > > + if (!(sdd->state & RXBUSY)) > > > > + complete(&sdd->xfer_completion); > > > > + > > > > + spin_unlock_irqrestore(&sdd->lock, flags); > > > > +} > > > I don't see much diff in these two functions and you should be > able to > > > use a generic one which takes care of both TX and RX, does your > > > callback > > > data know if the channel is for TX or RX? If not then a helper to > do > > > above should take care well and making code simpler > > I'm very agree with you. > > But, I think it isn't deeply related to this patch series. So, I > wish to make > > and submit it after this patch series is finished. > > > Since you are reworking this driver would make sense to do now, it > doesn't sound to be a big change. Ok. I will follow your request. I will make new patch for it and send it with V5 patchset. > > -- > ~Vinod Koul > Intel Corp. From mboxrd@z Thu Jan 1 00:00:00 1970 From: boojin.kim@samsung.com (Boojin Kim) Date: Wed, 27 Jul 2011 14:05:39 +0900 Subject: [PATCH V4 12/14] spi/s3c64xx: Add support DMA engine API In-Reply-To: <1311675280.24316.3.camel@vkoul-mobl4> References: <1311557312-26107-1-git-send-email-boojin.kim@samsung.com> <1311557312-26107-13-git-send-email-boojin.kim@samsung.com> <1311592630.29703.13.camel@vkoul-mobl4> <003701cc4b76$c52963f0$4f7c2bd0$%kim@samsung.com> <1311675280.24316.3.camel@vkoul-mobl4> Message-ID: <000301cc4c1a$d494bd40$7dbe37c0$%kim@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Vinod Koul Wrote: > Sent: Tuesday, July 26, 2011 7:15 PM > To: Boojin Kim > Cc: vinod.koul at intel.com; 'Kukjin Kim'; 'Jassi Brar'; 'Grant Likely'; > linux-samsung-soc at vger.kernel.org; 'Mark Brown'; 'Dan Williams'; > linux-arm-kernel at lists.infradead.org > Subject: RE: [PATCH V4 12/14] spi/s3c64xx: Add support DMA engine API > > On Tue, 2011-07-26 at 18:31 +0900, Boojin Kim wrote: > > Vinod Koul Wrote: > > > Sent: Monday, July 25, 2011 8:17 PM > > > To: Boojin Kim > > > Cc: vinod.koul at intel.com; linux-arm-kernel at lists.infradead.org; > linux- > > > samsung-soc at vger.kernel.org; Kukjin Kim; Jassi Brar; Grant Likely; > > > Mark Brown; Dan Williams > > > Subject: Re: [PATCH V4 12/14] spi/s3c64xx: Add support DMA engine > API > > > > > > On Mon, 2011-07-25 at 10:28 +0900, Boojin Kim wrote: > > > > This patch adds to support DMA generic API to transfer raw > > > > SPI data. Basiclly the spi driver uses DMA generic API if > > > > architecture supports it. Otherwise, uses Samsung specific > > > > S3C-PL330 APIs. > > > > > > > > Signed-off-by: Boojin Kim > > > > Cc: Grant Likely > > > > Signed-off-by: Kukjin Kim > > > > --- > > > > drivers/spi/spi_s3c64xx.c | 141 ++++++++++++++++++++++--------- > ---- > > > ---------- > > > > 1 files changed, 69 insertions(+), 72 deletions(-) > > > > > > > > diff --git a/drivers/spi/spi_s3c64xx.c > b/drivers/spi/spi_s3c64xx.c > > > > index 8945e20..a4cf76a 100644 > > > > --- a/drivers/spi/spi_s3c64xx.c > > > > +++ b/drivers/spi/spi_s3c64xx.c > > > > @@ -172,6 +172,9 @@ struct s3c64xx_spi_driver_data { > > > > unsigned state; > > > > unsigned cur_mode, cur_bpw; > > > > unsigned cur_speed; > > > > + unsigned rx_ch; > > > > + unsigned tx_ch; > > > > + struct samsung_dma_ops *ops; > > > > }; > > > > > > > > static struct s3c2410_dma_client s3c64xx_spi_dma_client = { > > > > @@ -227,6 +230,38 @@ static void flush_fifo(struct > > > s3c64xx_spi_driver_data *sdd) > > > > writel(val, regs + S3C64XX_SPI_CH_CFG); > > > > } > > > > > > > > +static void s3c64xx_spi_dma_rxcb(void *data) > > > > +{ > > > > + struct s3c64xx_spi_driver_data *sdd > > > > + = (struct s3c64xx_spi_driver_data *)data; > > > > + unsigned long flags; > > > > + > > > > + spin_lock_irqsave(&sdd->lock, flags); > > > > + > > > > + sdd->state &= ~RXBUSY; > > > > + /* If the other done */ > > > > + if (!(sdd->state & TXBUSY)) > > > > + complete(&sdd->xfer_completion); > > > > + > > > > + spin_unlock_irqrestore(&sdd->lock, flags); > > > > +} > > > > + > > > > +static void s3c64xx_spi_dma_txcb(void *data) > > > > +{ > > > > + struct s3c64xx_spi_driver_data *sdd > > > > + = (struct s3c64xx_spi_driver_data *)data; > > > > + unsigned long flags; > > > > + > > > > + spin_lock_irqsave(&sdd->lock, flags); > > > > + > > > > + sdd->state &= ~TXBUSY; > > > > + /* If the other done */ > > > > + if (!(sdd->state & RXBUSY)) > > > > + complete(&sdd->xfer_completion); > > > > + > > > > + spin_unlock_irqrestore(&sdd->lock, flags); > > > > +} > > > I don't see much diff in these two functions and you should be > able to > > > use a generic one which takes care of both TX and RX, does your > > > callback > > > data know if the channel is for TX or RX? If not then a helper to > do > > > above should take care well and making code simpler > > I'm very agree with you. > > But, I think it isn't deeply related to this patch series. So, I > wish to make > > and submit it after this patch series is finished. > > > Since you are reworking this driver would make sense to do now, it > doesn't sound to be a big change. Ok. I will follow your request. I will make new patch for it and send it with V5 patchset. > > -- > ~Vinod Koul > Intel Corp.