From mboxrd@z Thu Jan 1 00:00:00 1970 From: Magnus Damm Subject: Re: [PATCH] spi: Allow using spi_bitbang_setup() with custom txrx_bufs() Date: Thu, 26 Nov 2009 16:50:46 +0900 Message-ID: References: <20091125082603.25618.57438.sendpatchset@rxone.opensource.se> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: spi-devel-general@lists.sourceforge.net, dbrownell@users.sourceforge.net, linux-kernel@vger.kernel.org, akpm@linux-foundation.org To: Grant Likely Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-spi.vger.kernel.org On Thu, Nov 26, 2009 at 4:21 PM, Grant Likely wrote: > On Wed, Nov 25, 2009 at 11:47 PM, Magnus Damm = wrote: >> On Thu, Nov 26, 2009 at 7:15 AM, Grant Likely wrote: >>> On Wed, Nov 25, 2009 at 1:26 AM, Magnus Damm wrote: >>>> =A0 =A0 =A0 =A0/* per-word shift register access, in hardware or b= itbanging */ >>>> - =A0 =A0 =A0 cs->txrx_word =3D bitbang->txrx_word[spi->mode & (SP= I_CPOL|SPI_CPHA)]; >>>> - =A0 =A0 =A0 if (!cs->txrx_word) >>>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL; >>>> + =A0 =A0 =A0 if (bitbang->txrx_bufs =3D=3D spi_bitbang_bufs) { >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 cs->txrx_word =3D bitbang->txrx_word= [spi->mode & mode_mask]; >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!cs->txrx_word) >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL; >>>> + =A0 =A0 =A0 } >>> >>> Hmmm... this smells like an ugly hack to me. =A0It seems to me that= if >>> some bitbang backend drivers don't want this code, then it should b= e >>> encoded into a callback so it can be overridden. =A0Thoughts. >> >> Yeah, it's far from clean. I want to make use of spi_bitbang_setup() >> in my MSIOF driver, but I want to avoid dummy txtx_word[] callbacks >> that will be unused since i'm using a driver specific >> bitbang->txrx_bufs function. >> >> I guess the attached patch is slightly cleaner? I like the idea of >> letting bitbang drivers use shared code for >> spi_bitbang_setup()/spi_bitbang_cleanup() with their private >> setup_transfer() function which in turn calls >> spi_bitbang_setup_transfer(). My impression is that there's quite a >> bit of duplicated setup()/cleanup() code. > > This is certainly less ugly. =A0But with the points brought up in the > other thread, I want to have a close look at spi-bitbang before I > start applying stuff. =A0It seems nasty. =A0Give me a few days. Sure, I plan on posting a V2 of the MSIOF driver. I plan to keep the dummy txrx_word() function for now - this to disconnect the cleanup from the driver integration, hope that sounds like a good plan. Cheers, / magnus