From patchwork Tue Jan 5 00:19:32 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stephan Olbrich X-Patchwork-Id: 7952141 Return-Path: X-Original-To: patchwork-linux-spi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 0DF46BEEE5 for ; Tue, 5 Jan 2016 00:20:09 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 03C9A20304 for ; Tue, 5 Jan 2016 00:20:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B6E31202D1 for ; Tue, 5 Jan 2016 00:20:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753770AbcAEAUG (ORCPT ); Mon, 4 Jan 2016 19:20:06 -0500 Received: from mout.gmx.net ([212.227.15.15]:59485 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753688AbcAEAUE convert rfc822-to-8bit (ORCPT ); Mon, 4 Jan 2016 19:20:04 -0500 Received: from chaos-desktop.localnet ([188.99.61.108]) by mail.gmx.com (mrgmx003) with ESMTPSA (Nemesis) id 0MEnjW-1aQk0a32Hi-00G12Z; Tue, 05 Jan 2016 01:19:34 +0100 From: Stephan Olbrich To: linux-rpi-kernel@lists.infradead.org Cc: Martin Sperl , linux-spi , Mark Brown Subject: Re: [PATCH v6 2/4] spi: bcm2835: add bcm2835 auxiliary spi device driver Date: Tue, 05 Jan 2016 01:19:32 +0100 Message-ID: <1994637.DU4i3KELDi@chaos-desktop> User-Agent: KMail/4.14.7 (Linux/4.2.1-040201-generic; KDE/4.14.8; x86_64; ; ) In-Reply-To: <36C84129-0322-41C5-B01A-AF6F002E001E@martin.sperl.org> References: <1441970527-2403-1-git-send-email-kernel@martin.sperl.org> <1866751.zFWfh0Kuks@chaos-desktop> <36C84129-0322-41C5-B01A-AF6F002E001E@martin.sperl.org> MIME-Version: 1.0 X-Provags-ID: V03:K0:c8VHNgKSInKDfJr+Skk8WaTa5DEY51D2+cRIv358tnNAepMXEnW la+a7r3pA5cfCMdPMr3D7VBcV9l6H4IoOR6lULOQ29nLLgd0e70brNzKJCu6+vGgZrZHY2/ y74011yZiK7kd1MwM3tSPRssXvq7i1ccwVjoqcQird3IlYmFcbrCjA0Qbtl0IwHv2+ZKpHc biUERIIH5arTYq0H4I9Og== X-UI-Out-Filterresults: notjunk:1; V01:K0:EX24u0aPYTo=:xyxKWy3xwZT3HXu4NYguMJ PAZLOEWwpprK0IcRnhKQepRaHaGHzH9iWq32c+DktrJn1K9vboVXgDoWbg2huZxyTHmWQ2dCP 3CS7zj3tqvQOTEyZbK5B02NSYgD5A0K3SWd9mfl/I0CKArTBX21PINiX8H91cFuSMfFWQf9w5 EQYNP2M9rTcsW4c9nK6L0+nH6iJwQ7AeMPJzuepY8iNtriPbkL4QoraLZmxGjc4F80wYQtHiC tROZwpwiDrZwWyHXzorPWQB2iugje0tQMsww3G0MWmSBu+nNs8jkv50aNhFL/PI3PV+EqJMz1 exRDrYCNojrIk2r9bKlq5PdllNz36eZZRIH9w8zRl7q6lMvxQqYSdFkRtIDZiRvOVSz+CnsI0 8vZhcijWfJWAfoUpIcUNwDaH2I+qr7fo2YTUaSCpGqZFepptxlhTrEdCwDnmQg35immpfRIOl QTDWfxaHPSU10/eaVGgq0NT4I0/YmqquU6Uau1L2diTA/Bxk/tVZsVVkZLJdRasz2gbmQgbkm 9bBJgrvd8TzO1W7npAnV2rZUM4NMlpSjqub8dWuhazZEUFdNR5aKyJ0NA8zZwHDfYT07Ssn7Q kcizXPxCqxbIKOyzc/BIrtEW2VKR36DxHS1xsm7kM+yecPnQ+rfu6LPl/paMCQ7obZP3PsDiM oVTXhrVv3A4BvGdc3HHasPCVOMo2jp01T7GAGtWuH2Hxv+BUZWyV8etI/TPv3F4ZYv8T02U9a fu/8oT56x+6mLl90rs/rTAVSuuXFsgjTMyLmotyXGnvkZbP+BzTTv9774oSiRXXnD0bgL3ALi +a6U7ws Sender: linux-spi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Am Montag, 4. Januar 2016, 15:23:52 schrieb Martin Sperl: > > On 04.01.2016, at 14:51, Stephan Olbrich wrote: > > > > According to the spi documentation [1]: "CPHA indicates the clock phase > > used to sample data; CPHA=0 says sample on the leading edge, CPHA=1 means > > the trailing edge." > > Which is from my understanding different from what the BMC2835 ARM > > Peripherals [2] says for BCM2835_AUX_SPI_CNTL0_CPHA_IN: > > "If 1 data is clocked in on the rising edge of the SPI clock > > If 0 data is clocked in on the falling edge of the SPI clock" > > > > I would expect the bits to be set dependant on the clock polarity (CPOL). > > I am only having “typical” devices with do Mode 0,0 or 1,1 for which it > works. Are you only writing to your devices or reading as well? From what I think how this works, reading should not have worked with Mode 0,0 > > The other issue I have, is that the chip select is set before the clock > > polarity and the polarity is reset and set again between each transfers of > > a message. If CPOL is set to 1 this leads to additional rising and > > falling edges of the clock while chip select is active, where no data is > > sampled. > This is something I have seen before with the main spi HW. > We may need to port the same thing there. > > The corresponding patch for spi-bcm2835.c is this: > acace73df2c1913a526c1b41e4741a4a6704c863 Thanks for the hint. I tried to implement a similar solution. Could you test if this works for you? It also contains a fix for the IRQ defines, a hopefully correct solution to the CPHA issue and reduces the number of unnecessary interrupts. --- drivers/spi/spi-bcm2835aux.c | 70 ++++++++++++++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 18 deletions(-) @@ -307,9 +313,6 @@ static int bcm2835aux_spi_transfer_one_poll(struct spi_master *master, } } - /* Transfer complete - reset SPI HW */ - bcm2835aux_spi_reset_hw(bs); - /* and return without waiting for completion */ return 0; } @@ -330,10 +333,6 @@ static int bcm2835aux_spi_transfer_one(struct spi_master *master, * resulting (potentially) in more interrupts when transferring * more than 12 bytes */ - bs->cntl[0] = BCM2835_AUX_SPI_CNTL0_ENABLE | - BCM2835_AUX_SPI_CNTL0_VAR_WIDTH | - BCM2835_AUX_SPI_CNTL0_MSBF_OUT; - bs->cntl[1] = BCM2835_AUX_SPI_CNTL1_MSBF_IN; /* set clock */ spi_hz = tfr->speed_hz; @@ -352,13 +351,6 @@ static int bcm2835aux_spi_transfer_one(struct spi_master *master, spi_used_hz = clk_hz / (2 * (speed + 1)); - /* handle all the modes */ - if (spi->mode & SPI_CPOL) - bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_CPOL; - if (spi->mode & SPI_CPHA) - bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_CPHA_OUT | - BCM2835_AUX_SPI_CNTL0_CPHA_IN; - /* set transmit buffers and length */ bs->tx_buf = tfr->tx_buf; bs->rx_buf = tfr->rx_buf; @@ -382,6 +374,46 @@ static int bcm2835aux_spi_transfer_one(struct spi_master *master, return bcm2835aux_spi_transfer_one_irq(master, spi, tfr); } +static int bcm2835aux_spi_prepare_message(struct spi_master *master, + struct spi_message *msg) +{ + struct spi_device *spi = msg->spi; + struct bcm2835aux_spi *bs = spi_master_get_devdata(master); + + bs->cntl[0] = BCM2835_AUX_SPI_CNTL0_ENABLE | + BCM2835_AUX_SPI_CNTL0_VAR_WIDTH | + BCM2835_AUX_SPI_CNTL0_MSBF_OUT; + bs->cntl[1] = BCM2835_AUX_SPI_CNTL1_MSBF_IN; + + /* handle all the modes */ + if (spi->mode & SPI_CPOL) { + bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_CPOL; + if (spi->mode & SPI_CPHA) + bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_CPHA_IN; + else + bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_CPHA_OUT; + } else { + if (spi->mode & SPI_CPHA) + bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_CPHA_OUT; + else + bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_CPHA_IN; + } + bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL1, bs->cntl[1]); + bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL0, bs->cntl[0]); + + return 0; +} + +static int bcm2835aux_spi_unprepare_message(struct spi_master *master, + struct spi_message *msg) +{ + struct bcm2835aux_spi *bs = spi_master_get_devdata(master); + + bcm2835aux_spi_reset_hw(bs); + + return 0; +} + static void bcm2835aux_spi_handle_err(struct spi_master *master, struct spi_message *msg) { @@ -410,6 +442,8 @@ static int bcm2835aux_spi_probe(struct platform_device *pdev) master->num_chipselect = -1; master->transfer_one = bcm2835aux_spi_transfer_one; master->handle_err = bcm2835aux_spi_handle_err; + master->prepare_message = bcm2835aux_spi_prepare_message; + master->unprepare_message = bcm2835aux_spi_unprepare_message; master->dev.of_node = pdev->dev.of_node; bs = spi_master_get_devdata(master); diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c index 7de6f84..d8be445 100644 --- a/drivers/spi/spi-bcm2835aux.c +++ b/drivers/spi/spi-bcm2835aux.c @@ -73,8 +73,8 @@ /* Bitfields in CNTL1 */ #define BCM2835_AUX_SPI_CNTL1_CSHIGH 0x00000700 -#define BCM2835_AUX_SPI_CNTL1_IDLE 0x00000080 -#define BCM2835_AUX_SPI_CNTL1_TXEMPTY 0x00000040 +#define BCM2835_AUX_SPI_CNTL1_TXEMPTY 0x00000080 +#define BCM2835_AUX_SPI_CNTL1_IDLE 0x00000040 #define BCM2835_AUX_SPI_CNTL1_MSBF_IN 0x00000002 #define BCM2835_AUX_SPI_CNTL1_KEEP_IN 0x00000001 @@ -212,9 +212,15 @@ static irqreturn_t bcm2835aux_spi_interrupt(int irq, void *dev_id) ret = IRQ_HANDLED; } - /* and if rx_len is 0 then wake up completion and disable spi */ + if (!bs->tx_len) { + /* disable tx fifo empty interrupt */ + bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL1, bs->cntl[1] | + BCM2835_AUX_SPI_CNTL1_IDLE); + } + + /* and if rx_len is 0 then disable interrupts and wake up completion */ if (!bs->rx_len) { - bcm2835aux_spi_reset_hw(bs); + bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL1, bs->cntl[1]); complete(&master->xfer_completion); }