On 06/27/2017 05:57 PM, matthew.gerlach@xxxxxxxxxxxxxxx wrote: > > > On Tue, 27 Jun 2017, Marek Vasut wrote: > >> On 06/27/2017 04:32 PM, matthew.gerlach@xxxxxxxxxxxxxxx wrote: >>> >>> >>> 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@xxxxxxxxxxxxxxx wrote: >>>>> From: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx> >>>>> >>>>> 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@xxxxxxxxxxxxxxx> >>>>> --- >>>>> .../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. >> >> Maybe we should drop this whole thing and add it when this is actually >> required. >> >> Are there any users of this in the wild currently ? >> >> What is the purpose of doing this per-byte bit reverse instead of >> storing th bits in the original order ? > > Hi Marek, > > Yes, there is hardware that has been in the wild for years that needs > this bit reversal. The specific use case is when a flash chip is > connected to > a FPGA, and the contents of the flash is used to configure the FPGA on > power up. In this use case, there is no processor involved with > configuring the FPGA. I am most familiar with this feature/bug with > Altera FPGAs, but I believe this issue exists with other programmable > devices. So the EPCQ/EPCS flash stores the bitstream in reverse or something ? What are you storing in that flash except for the bitstream, filesystem? Feel free to go into details, I believe it'd be useful to know exactly what the problem is you're trying to solve here. >>> 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. >> >> $ git grep bit.reverse Documentation/devicetree/ | wc -l >> 0 >> >> So we don't have such a generic binding . It's up to Rob (I guess) to >> decide whether this is SoC specific and should've altr, prefix or not. >> IMO it is. > > I agree there is no generic binding at this time, and I look forward > to any input from Rob and anyone else on this issue. I think it is > worth pointing out that this really isn't an issue of an SoC, but rather > it is an > issue of how data in the flash chip is accessed.I think what makes > this issue > "weird" is that we have different hardware accessing the data in the > flash with a different perspective. The FPGA looks at the data from one > perspective on power up, and a processor trying to update the flash has > a different perspective. Another thing I'd ask here is, is that bit-reverse a hardware property or is that some software configuration thing ? -- Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
