From patchwork Thu Feb 11 18:44:55 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Martin Sperl X-Patchwork-Id: 8284111 Return-Path: X-Original-To: patchwork-linux-spi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 6003D9F3CD for ; Thu, 11 Feb 2016 18:44:59 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 83F29202FE for ; Thu, 11 Feb 2016 18:44:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 82273202EB for ; Thu, 11 Feb 2016 18:44:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752323AbcBKSox (ORCPT ); Thu, 11 Feb 2016 13:44:53 -0500 Received: from 212-186-180-163.dynamic.surfer.at ([212.186.180.163]:43541 "EHLO cgate.sperl.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752307AbcBKSow convert rfc822-to-8bit (ORCPT ); Thu, 11 Feb 2016 13:44:52 -0500 Received: from msmac.intern.sperl.org (account martin@sperl.org [10.10.10.11] verified) by sperl.org (CommuniGate Pro SMTP 6.1.2) with ESMTPSA id 6393373; Thu, 11 Feb 2016 18:44:50 +0000 Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2104\)) Subject: Re: [PATCH 4/4] spi: bcm2835aux: fix CPOL/CPHA setting From: Martin Sperl In-Reply-To: <50D0ECA7-8116-4248-AC85-6E1C8002A185@sperl.org> Date: Thu, 11 Feb 2016 19:44:55 +0100 Cc: linux-rpi-kernel@lists.infradead.org, Eric Anholt , linux-spi@vger.kernel.org, Mark Brown , linux-arm-kernel@lists.infradead.org Message-Id: <26830EBF-A5F2-4BE3-88A2-C9E0A97A4881@sperl.org> References: <1455041435-8015-1-git-send-email-stephanolbrich@gmx.de> <87a8n94e3x.fsf@eliezer.anholt.net> <16A848EA-6E08-48C4-B3DD-DF27333B7458@sperl.org> <10547867.CcbLVQlgyH@chaos-desktop> <50D0ECA7-8116-4248-AC85-6E1C8002A185@sperl.org> To: Stephan Olbrich X-Mailer: Apple Mail (2.2104) Sender: linux-spi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable 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 > On 11.02.2016, at 17:19, Martin Sperl wrote: >> At the end with the last rising edge of SCLK MOSI goes to low and stays there >> while SCLK goes to low and then CS to inactive. >> >> So the slave has no chance to get the first bit but gets an additional 0 at the >> end. Actually SPI says: sample AT the clock change - not shortly after. So the behavior in principle is correct - even if it is not perfect... >> It may not be easy visible with 0x00,0x82,0x00 as data, so could you repeat >> your test with 0xAA,0xFF if you see the same as I do or not? > > I did retest also with 0x81,0x00,0x81 and saw my expected behavior - but maybe > me understanding of those modes is wrong. > > I will send you the images for the pattern I have generated with > 0x81, 0x00, 0x81 as a personal email. There is a means to add an additional HOLD time of 1, 4 or 7 system-clock cycles (undivided) that extend the period while MOSI is still driven with the old bit value after the clock change. This can get fixed like this (copy paste, so tabs are spaces): It still means that CPHA=1 may fail to work with devices that run a software based SPI implementation that has a delay between clock change and sampling. SPI slaves that latch the value on clock change should be fine. Hopefully the policies for the delays were chosen wisely… But for all practical purposes the DOUTHOLD is 7 systen clock cycles (undivided or .000000027s) for anything below 18.3MHz (=256MHz/(2*(1+6)). If this is an unacceptable limitation, then we can disable CPHA support by removing SPI_CPHA from BCM2835_AUX_SPI_MODE_BITS. Maybe by making it a configuration option in the device-tree? Martin-- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c index bd490c0..6052628 100644 --- a/drivers/spi/spi-bcm2835aux.c +++ b/drivers/spi/spi-bcm2835aux.c @@ -63,6 +63,10 @@ #define BCM2835_AUX_SPI_CNTL0_VAR_CS 0x00008000 #define BCM2835_AUX_SPI_CNTL0_VAR_WIDTH 0x00004000 #define BCM2835_AUX_SPI_CNTL0_DOUTHOLD 0x00003000 +#define BCM2835_AUX_SPI_CNTL0_DOUTHOLD_0 0x00000000 +#define BCM2835_AUX_SPI_CNTL0_DOUTHOLD_1 0x00001000 +#define BCM2835_AUX_SPI_CNTL0_DOUTHOLD_4 0x00002000 +#define BCM2835_AUX_SPI_CNTL0_DOUTHOLD_7 0x00003000 #define BCM2835_AUX_SPI_CNTL0_ENABLE 0x00000800 #define BCM2835_AUX_SPI_CNTL0_CPHA_IN 0x00000400 #define BCM2835_AUX_SPI_CNTL0_CLEARFIFO 0x00000200 @@ -341,10 +345,35 @@ static int bcm2835aux_spi_transfer_one(struct spi_master * } else { /* the slowest we can go */ speed = BCM2835_AUX_SPI_CNTL0_SPEED_MAX; } + /* mask out old speed from previous spi_transfer */ + bs->cntl[0] &= ~BCM2835_AUX_SPI_CNTL0_SPEED; + /* and set speed for this round */ bs->cntl[0] |= speed << BCM2835_AUX_SPI_CNTL0_SPEED_SHIFT; spi_used_hz = clk_hz / (2 * (speed + 1)); + /* and configure hold time - needs to take speed into account */ + /* first clear it */ + bs->cntl[0] &= ~BCM2835_AUX_SPI_CNTL0_DOUTHOLD; + switch (speed) { + case 0: /* divider 2 */ + case 1: /* divider 4 */ + /* data hold time of 1 system clock cycle */ + bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_DOUTHOLD_1; + break; + case 2: /* divider 6 */ + case 3: /* divider 8 */ + case 4: /* divider 10 */ + case 5: /* divider 12 */ + /* data hold time of 4 system clock cycle */ + bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_DOUTHOLD_4; + break; + default: /* divider 14 and above */ + /* data hold time of 7 system clock cycle */ + bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_DOUTHOLD_7; + break; + } + /* set transmit buffers and length */ bs->tx_buf = tfr->tx_buf; bs->rx_buf = tfr->rx_buf;