lkml.org 
[lkml]   [2016]   [Oct]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [v2 2/2] fpga: Add support for Lattice iCE40 FPGAs
From
Date
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
> <joel@airwebreathe.org.uk> 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
>
>

\
 
 \ /
  Last update: 2016-10-29 23:39    [W:0.091 / U:0.596 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site