From owner-freebsd-embedded@freebsd.org Fri May 27 15:50:18 2016 Return-Path: Delivered-To: freebsd-embedded@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 935D0B4C823 for ; Fri, 27 May 2016 15:50:18 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-io0-x236.google.com (mail-io0-x236.google.com [IPv6:2607:f8b0:4001:c06::236]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4E3C01139; Fri, 27 May 2016 15:50:18 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: by mail-io0-x236.google.com with SMTP id f8so74431844ioe.3; Fri, 27 May 2016 08:50:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc; bh=SUTC3c1nCGqC5ei15wCSlRMTZz4V2xX9kLkzwPdP36o=; b=mUUFyQoJutb1gMzQyPiTAwI9QMRU3mHyvCKamJk8tSz9PBOflqcRlC39milSAxNJ60 MIqfVxpBXGTXycpjSIbwEg5FGqwkdV6SMerAdjqGIOp8ZEsuNjJV6+kHd2GzIFcK6Wpk uDxh4EupM6pTkqm9QNC+qRh9A41kfOqy+oMUYE4cN4LoXOZY0pUlUYXUBKV/1bmO/iKS CrzIGQ90+JrpK1SeDjEPicsC0v7rKobhhC1aF2UGMKgmRY6mJfT+HiiVi4hbT15M99n4 v6RMn4ROJD9ax/yuzwqReprMH2M4QtW6bhofhv4ovbG8WOrG3dAYByu786JAZ2XOwdrA XY8w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:date :message-id:subject:from:to:cc; bh=SUTC3c1nCGqC5ei15wCSlRMTZz4V2xX9kLkzwPdP36o=; b=WTUpJICQpIiPtxGiMNiKUJk8zpat8oniSqVbxJJWBybd0QvY1zeU1f4cvMLe4a/0lX +AUuSdBc4ItEbQ6cGsPCxdOyK4/vsTItZCmfs6UfItXUUy9F4TqjrAM2RRLSlFGx5aDr l9LFwaX1aYlViuAODVAhtWEdwBpps6+BENbH2fWtwF+FJIM5FT7LNk/p6UfB8wmf8um/ DO7HE3DH4/L60tdFJppJ+WPKQdEupL1pz0sR357eYcXAiKOoHBTWC+zQTMprGg15iDyp A6mCoOW7iHfOeHhS5rMNRqGlQifR7noWFjbxr/yrxEOvI4tzo71Wgg4P89jX1mDiq8/c H3rQ== X-Gm-Message-State: ALyK8tK8VflwpFDx7HyzIpxiBUOYCDmsypbTivpJCfzFBynpI8U0f1bFFB6jmWzBDf9FKwVhXJNDxKNVaF764Q== MIME-Version: 1.0 X-Received: by 10.107.144.67 with SMTP id s64mr11064394iod.165.1464364217490; Fri, 27 May 2016 08:50:17 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.36.113.3 with HTTP; Fri, 27 May 2016 08:50:17 -0700 (PDT) In-Reply-To: References: Date: Fri, 27 May 2016 08:50:17 -0700 X-Google-Sender-Auth: NRdm_GQ7V8Un-3Nwwt82ijsU8T4 Message-ID: Subject: Re: spibus: migrate to bus acquire/release semantics From: Adrian Chadd To: Patrick Kelsey Cc: "freebsd-embedded@freebsd.org" Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-embedded@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Dedicated and Embedded Systems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 May 2016 15:50:18 -0000 On 26 May 2016 at 22:26, Patrick Kelsey wrote: > > > On Thu, May 26, 2016 at 10:00 PM, Adrian Chadd wrote: >> >> hi, >> >> Here's the first part of the work needed to bring mmcspi into the >> tree. It also fixes concurrent SPI controller accesses for a variety >> of controllers which currently don't consistently implement it - >> AR71xx spi doesn't, but brcm2835 spi does. >> >> https://reviews.freebsd.org/D6588 >> >> I'd appreciate some feedback and testing. >> >> Now, this doesn't implement it for all of the controllers, only the >> simple ones. The broadcom and other arm ones aren't migrated, because >> CS is asserted in a variety of different ways there. Now, it's not a >> /huge/ deal, it just means that CS isn't asserted for the entirety of >> a transaction set. For mmcspi (which I think assumes this is the >> case), we may have to add a controller ivar that indicates if it >> supports it or not, so mmcspi and other devices can implement it >> appropriately. >> >> Another good example of this would be an LCD controller. Ideally you'd do >> this: >> >> * acquire SPI bus >> * assert command line >> * do SPI transaction, with CS asserted >> * deassert command line, assert data line >> * do SPI transaction, with CS asserted >> * release SPI bus >> >> Now, CS doesn't have to be asserted between transactions, but the >> command/data commands have to happen together like that. >> >> Thanks! >> > > Thanks for continuing to work on this. > > One thing mmcspi requires is the ability to control chip select state on a > per-SPI-transfer basis, because correct initialization requires providing at > least 74 clocks to the device with both CS and MOSI set to high. That means > initialization requires: > > acquire bus > clock out enough 0xff with CS high to meet the requirement > release bus > > The mmcspi driver is written so that CS will remain asserted across however > many individual SPI-transfers it takes to execute a given MMC/SD command. > It is not entirely clear whether this is required. I only had access to the > simplified spec, which omits SPI timing diagrams found in the full spec, and > possibly further related discussion. Based on the range of goofy behaviors > I observed in the set of cards I tested with, I was motivated to be > conservative with the treatment of CS during MMC/SD command execution. > > If it is the case that holding CS asserted during the entire MMC/SD command > execution is truly required in some or all cases, then if the underlying SPI > controller doesn't support that, I don't think there is anything the mmcspi > driver can do other than print a warning. > > Aside from whether holding CS asserted across all of the individual > transfers that the mmcspi driver issues during execution of a single MMC/SD > command is truly required, I think this is a useful level of control to have > in the interface. There are certainly SPI devices out there that do > specific things on low-to-high transitions of CS and thus can require such > care. One example is the LTC2440 analog-to-digital converter, which will > exit serial-data-out mode and start a new conversion on a low-to-high CS > transition. It also so happens that with this device, you detect > end-of-conversion by checking the first bit clocked out of it. This > encourages a driver implementation that does an 8-bit read to check end of > conversion (for bus efficiency), then reads the remaining 24 bits if the end > of conversion flag is set in the initial 8 bits retrieved. If CS can't be > held high across the final 8-bit poll read and subsequent 24-bit > rest-of-result read, the device will change modes and the conversion data > will be lost. Without CS control across transactions, the fallback would be > for completion polling to take 4 times as long as it needs to as 32-bit > reads would always have to be used. Hi, Right. I think warner mentioned something about devices that wanted and devices that didn't want this kind of behaviour. Ian has suggested that we implement it as a per-acquire flag, so the SPI user can determine how they'd like to do things. I'd like to do that after doing the initial conversion, and then likely after I implement some ivars that expose spibus capabilities. So, my (vague) plan is to get this patchset into -HEAD, as it's one enormous API churn that should be a no-op with the current users, and then add ivars for the bus capabilities so things like mmcspi can decide whether they're going to get what they need out of it, and then start porting more of the mmcspi patchset. What hardware were you testing on? I have some mmcspi hardware here, but I'd rather first verify that this works on the original hardware for this project before trying to adapt it to work on the $10 AR9331 part I have.. Thanks! -adrian