linux-kernel - Re: [PATCH 1/3] ARM: dts: Bindings for Altera Quadspi Controller Version 2

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 27 Jun 2017 07:32:21 -0700 (PDT)
From:   matthew.gerlach@...ux.intel.com
To:     Marek Vasut <marek.vasut@...il.com>
cc:     vndao@...era.com, dwmw2@...radead.org, computersforpeace@...il.com,
        boris.brezillon@...e-electrons.com, richard@....at,
        cyrille.pitchen@...ev4u.fr, robh+dt@...nel.org,
        mark.rutland@....com, linux-mtd@...ts.infradead.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        gregkh@...uxfoundation.org, davem@...emloft.net, mchehab@...nel.org
Subject: Re: [PATCH 1/3] ARM: dts: Bindings for Altera Quadspi Controller
 Version 2



On Tue, 27 Jun 2017, Marek Vasut wrote:

Hi Marek,

Thanks for the feedback.  See my comments below.

Matthew Gerlach

> On 06/26/2017 06:13 PM, matthew.gerlach@...ux.intel.com wrote:
>> From: Matthew Gerlach <matthew.gerlach@...ux.intel.com>
>>
>> Device Tree bindings for Version 2 of the Altera Quadspi Controller
>> that can be optionally paired with a windowed bridge.
>>
>> Signed-off-by: Matthew Gerlach <matthew.gerlach@...ux.intel.com>
>> ---
>>  .../devicetree/bindings/mtd/altera-quadspi-v2.txt  | 37 ++++++++++++++++++++++
>>  1 file changed, 37 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt b/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt
>> new file mode 100644
>> index 0000000..8ba63d7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt
>> @@ -0,0 +1,37 @@
>> +* Altera Quad SPI Controller Version 2
>> +
>> +Required properties:
>> +- compatible : Should be "altr,quadspi-v2".
>> +- reg : Contains at least two entries, and possibly three entries, each of
>> +	which is a tuple consisting of a physical address and length.
>> +- reg-names : Should contain the names "avl_csr" and "avl_mem" corresponding
>> +	      to the control and status registers and qspi memory, respectively.
>> +
>> +
>> +The Altera Quad SPI Controller Version 2 can be paired with a windowed bridge
>> +in order to reduce the footprint of the memory interface.  When a windowed
>> +bridge is used, reads and writes of data must be 32 bits wide.
>> +
>> +Optional properties:
>> +- reg-names : Should contain the name "avl_window", if the windowed bridge
>> +	      is used.  This name corresponds to the register space that
>> +	      controls the window.
>> +- window-size : The size of the window which must be an even power of 2.
>> +- read-bit-reverse : A boolean indicating the data read from the flash should
>> +		     be bit reversed on a byte by byte basis before being
>> +		     delivered to the MTD layer.
>> +- write-bit-reverse : A boolean indicating the data written to the flash should
>> +		      be bit reversed on a byte by byte basis.
>
> Is there ever a usecase where you need to set just one of these props ?
> Also, they're altera specific, so altr, prefix should be added.

In general, I think if bit reversal is required, it would be required in
both directions.  However, anything is possible when using FPGAs.  So
I thought separate booleans would be future proofing the bindings.
Thinking about this binding more, I wonder if the binding name(s)
should be (read|write)-bit8-reverse to indicate reversings the bits
in a byte as opposed to reversing the bits in a 32 bit word?

I don't think bit reversal is specific to Altera/Intel components. I see
a nand driver performing bit reversal, and I think I've recently seen 
other FPGA based drivers requiring bit reversal.

>
>> +Example:
>> +
>> +qspi: spi@...01000 {
>> +	compatible = "altr,quadspi-v2";
>> +	reg = <0xa0001000 0x40>, <0xb0000000 0x4000000>;
>> +	reg-names = "avl_csr", "avl_mem";
>> +
>> +	flash@0 {
>> +		reg = <0>;
>> +		label = "FPGA Image";
>> +	};
>> +};
>>
>
>
> -- 
> Best regards,
> Marek Vasut
>