From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grant Likely Subject: Re: [PATCH 1/2] spi: Add support for dma_min_bytes configuration. Date: Thu, 8 Apr 2010 09:08:47 -0600 Message-ID: References: <1270550389-30392-1-git-send-email-roman.tereshonkov@nokia.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, Samuel Ortiz To: roman.tereshonkov-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: spi-devel-general-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: linux-spi.vger.kernel.org On Thu, Apr 8, 2010 at 4:33 AM, wrote: > > Hi, > > >>-----Original Message----- >>From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On >>Behalf Of ext Grant Likely >>Sent: 08 April, 2010 09:27 >>To: Tereshonkov Roman (Nokia-D/Helsinki) >>Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org >>Subject: Re: [PATCH 1/2] spi: Add support for dma_min_bytes >>configuration. >> >>Hi Roman, >> >>On Tue, Apr 6, 2010 at 4:39 AM, Roman Tereshonkov >> wrote: >>> This parameters defines the minimum number of bytes when dma is used. >>> >>> Signed-off-by: Roman Tereshonkov >> >>The intent of this feature is... ? =A0Your patch needs a better >>description as to why it is needed; especially considering that it >>changes common code. >> >>But, inferring from the code that is written, I can guess what you're >>trying to do, if not why. =A0Why would spi_devices care what the minimum >>size of a DMA transfer is? =A0The SPI bus driver is in a far better >>position to make that determination. >> >>Also, since this essentially adds a new 'knob' for spi_devices to >>twiddle, what is the expected behaviour for SPI bus controllers that >>don't support it? =A0Is it a required feature for spi bus drivers to >>implement if they support DMA? > > The spi transactions can be handled in two ways: dma and pio. > For the best perfomance the minimun number of bytes when dma is used > can be found experimentaly and passed through the platform board config f= iles. > > Now I will talk about omap2/3 spi only. > If the mentioned parameter is not set then the static default one > is used (as it is nowdays). > This exludes the patch influence on other spi devices controlled > by the same omap2/3 spi master. > > I think you might be right. The better way would be to pass it > through the controller_data field of the spi_board_info. > Then I do not need to touch spi.h and spi.c. The question that must be asked, will this new optimization option actually be profiled for most boards? Should it really be a board-specific parameter? Or even an SoC specific parameter? Or to be even more specific, do *you* have two different boards that need a different value for the minimum dma bytes? If the answer is no, then I recommend profiling your platform and floating out a patch that changes the default value to what you find is best. If there is no opposition to the value you choose, then there is no need to make it a tunable until someone else comes along who needs it to be something different. The current value of DMA_MIN_BYTES hasn't changed since the driver was first submitted. I suspect that it has not been optimized. What does Samuel have to say about it? g. > > I will create a new patch. > But now I wonder about patches syncronization. > The first patch should go to linux-omap tree as it is for arch/arm/plat-o= map/include/plat/mcspi.h. > The second patch applied after the first one is for driver/spi/omap2_mcsp= i.h. > Or can you handle both patches? > > > Regards > Roman ---------------------------------------------------------------------------= --- Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev