Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755264AbcJ2ViY (ORCPT ); Sat, 29 Oct 2016 17:38:24 -0400 Received: from auth.a.painless.aa.net.uk ([90.155.4.51]:58028 "EHLO auth.a.painless.aa.net.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752675AbcJ2ViX (ORCPT ); Sat, 29 Oct 2016 17:38:23 -0400 Subject: Re: [v2 2/2] fpga: Add support for Lattice iCE40 FPGAs To: Moritz Fischer References: <1477283989-21947-1-git-send-email-joel@airwebreathe.org.uk> <1477283989-21947-2-git-send-email-joel@airwebreathe.org.uk> <20161024222805.GA5754@live.com> <331cd8da-f69a-b51a-ddbd-f18ee44cac8a@airwebreathe.org.uk> Cc: atull , Ian Campbell , Kumar Gala , Mark Rutland , "pawel.moll@arm.com" , Rob Herring , Devicetree List , Linux Kernel Mailing List From: Joel Holdsworth Message-ID: <2779d292-779d-b57a-1b0a-ed67644f6c2e@airwebreathe.org.uk> Date: Sat, 29 Oct 2016 15:38:08 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Painless-Spam-Score: -1.5 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1695 Lines: 45 I just submitted a version of the driver without copy-pasted chip-select code. The PATCH v4 version uses a pair of zero-byte SPI transfers to control the CS line. I didn't get a response from the linux-spi mailing list, but in my opinion they're probably not going to want spi_set_cs to become a public-internal API, and zero-byte transfers have the desired effect. Thanks Joel On 10/25/2016 10:48 AM, Moritz Fischer wrote: > Hi Joel, > > On Mon, Oct 24, 2016 at 9:51 PM, Joel Holdsworth > wrote: > >> I think my set_cs() function is ok-ish. It's copied from spi_set_cs() in >> drivers/spi/spi.c . This function is a static internal helper, so I >> copy/pasted the function into the ice40 driver. Given that it's only 4-lines >> of code, it didn't seem too bad - though I'm not exactly sure why >> spi_set_cs() isn't a public API. It seems like quite a common-place thing to >> need to do with certain devices. >> >> However, perhaps the function is internal because the authors of the SPI >> framework foresaw how easy it would be to screw up a shared bus with that >> function. I had to take care to make sure the SPI bus was locked throughout. >> >> Do you agree that it's the right thing to copy the function in? Or do you >> think it would be better to ask for spi_set_cs to be exposed publicly? > > I'd poke the SPI maintainers about what their reasoning was to make it > non-public, > and how they'd go about doing what you're trying to do. > I can imagine there might be some SPI controllers where the above > doesn't work well, > because the controller automatically handles the CS line and you don't > get control over it. > > Cheers, > > Moritz > >