linux-kernel - Re: [v2 2/2] fpga: Add support for Lattice iCE40 FPGAs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <2779d292-779d-b57a-1b0a-ed67644f6c2e@airwebreathe.org.uk>
Date:   Sat, 29 Oct 2016 15:38:08 -0600
From:   Joel Holdsworth <joel@...webreathe.org.uk>
To:     Moritz Fischer <moritz.fischer@...us.com>
Cc:     atull <atull@...nsource.altera.com>,
        Ian Campbell <ijc+devicetree@...lion.org.uk>,
        Kumar Gala <galak@...eaurora.org>,
        Mark Rutland <mark.rutland@....com>,
        "pawel.moll@....com" <pawel.moll@....com>,
        Rob Herring <robh+dt@...nel.org>,
        Devicetree List <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [v2 2/2] fpga: Add support for Lattice iCE40 FPGAs

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@...webreathe.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
>
>