From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from avon.wwwdotorg.org ([70.85.31.133]:52643 "EHLO avon.wwwdotorg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760002Ab3HNQbq (ORCPT ); Wed, 14 Aug 2013 12:31:46 -0400 Message-ID: <520BB0EF.2090106@wwwdotorg.org> Date: Wed, 14 Aug 2013 10:31:43 -0600 From: Stephen Warren MIME-Version: 1.0 Subject: Re: Fwd: [PATCH] SPI: Set SPI bits per words in an OF DeviceTree SPI node References: <201308071544.r77Fij2Y006649@localhost.localdomain> <52026DBA.8000202@c-s.fr> <52027637.7000309@wwwdotorg.org> <520B845A.4070602@c-s.fr> <520BAE11.5040504@wwwdotorg.org> <520BB064.5070803@c-s.fr> In-Reply-To: <520BB064.5070803@c-s.fr> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: devicetree-owner@vger.kernel.org To: leroy christophe Cc: devicetree@vger.kernel.org, Ian Campbell , Mark Brown , Pawel Moll , Mark Rutland , Rob Herring , linux-spi@vger.kernel.org List-ID: On 08/14/2013 10:29 AM, leroy christophe wrote: > > Le 14/08/2013 18:19, Stephen Warren a �crit : >> On 08/14/2013 07:21 AM, leroy christophe wrote: >>>>> In the node, you then have to include the 'spi-bits' property. >>>>> >>>>> Exemple: >>>>> fpga-loader@7 { >>>>> compatible = "cs,fpga-loader"; >>>>> spi-max-frequency = <10000000>; >>>>> reg = <7>; >>>>> spi-cs-high; >>>>> spi-bits = <16>; >>>>> }; >>>> Shouldn't the driver for cs,fpga-loader be defining how many bit the >>>> SPI >>>> transactions should use? If the binding covers HW that can operate at >>>> various bits-per-word, it's reasonable for those individual bindings to >>>> define a property that sets that width, but I'm not convinced it's >>>> reasonable for the SPI core to allow any DT node to specify that, and >>>> presumably override the SPI device's driver's selection. >>> This is a tricky question. >>> >>> The issue in the CPM (communication co-processor) on the MPC 8xx CPU is >>> that, when you use the 16 bits mode, the CPM swaps the bytes, so the >>> driver must unswap them to use them properly. >>> It looks like their is the same issue with other CPUs, like the MPCs >>> having the Quick Engine. In the QE driver, it is done simple: the >>> transfert is forced at 8bits at all time regardless on what the driver >>> requests. This is known to work well, but I don't want to do that for >>> the CPM driver, because the 16 bits mode is a lot less CPU consuming >>> than the 8 bits mode, and my own drivers for my custom components do >>> work in 16 bits mode (byte swap is included in those drivers). >> If there's a byte-swap bug in the CPM HW/driver, it should be solved in >> the CPM driver, not in the drivers that happen to communicate over the >> CPM driver. As you say below, fixing it in the client drivers is the >> wrong thing to do, since then they won't work with other SPI controllers. >> >> Either way, this doesn't seem like something that should be represented >> in DT at all; it should be down to the CPM driver to indicate to the SPI >> sub-system that it doesn't support 16-bit transfers. >> >> An in general, your answer in no way addresses why this property should >> apply to all SPI devices. >> >>> That's the reason why I was proposing a way, through the Device Tree, to >>> select the speed instead of forcing it inside the driver. >>> Because the MAX7301 driver for instance, which used to force 16 bits >>> mode, does work perfectly well on the MPC8xx in 8 bits mode, but doesn't >>> work in 16 bits because it doesn't unswap the bytes. >>> If I modify that driver to swap the bytes, it will not work anymore on >>> platforms that don't require bytes to be swapped. >>> >>> So, really, I don't know what to do here. Do you have any suggestion ? >> Drop 16-bit support from the CPM driver if it doesn't work. >> > But as I said, I don't want to drop this support from the driver because > it does work as far as the driver is able to swap the bytes prior. > My own drivers are swapping the bytes and it works perfectly well. > I need to keep this capability because it allows a higher transfert > speed than the 8 bits mode on the CPM (about 6 times), and this is > important when I have to load a big FPGA on my board. > But on the other hand I also need drivers like the MAX7301 to work on my > board. > This driver works very well when I modify it to force 8 bits mode. But > in standard, the driver forces itself to 16 bits mode which makes it fail. > > So I need another solution than droping the 16 bits mode from the driver. Perhaps enhance the SPI subsystem with a new flag/quirk/... that indicates the host-controller supports byte-swapped 16-bit transactions.