spi: orion.c: Add direct access mode

This patch adds support for the direct access mode to the Orion SPI driver which is used on the Marvell Armada based SoCs. In this direct mode, all data written to (or read from) a specifically mapped MBus window (linked to one SPI chip-select on one of the SPI controllers) will be transferred directly to the SPI bus. Without the need to control the SPI registers in between. This can improve the SPI transfer rate in such cases.

Both, direct-read and -write mode are supported. But only the write mode has been tested. This mode especially benefits from the SPI direct mode, as the data bytes are written head-to-head to the SPI bus, without any additional addresses.

One use-case for this direct write mode is, programming a FPGA bitstream image into the FPGA connected to the SPI bus at maximum speed.

This mode is described in chapter "22.5.2 Direct Write to SPI" in the Marvell Armada XP Functional Spec Datasheet.

Signed-off-by Stefan Roese <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Gregory CLEMENT <[email protected]>

Cc Andrew Lunn <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Mark Brown <[email protected]>

---
v5:
- Added some documentation for the direct mode to the orion-spi DT bindings document, including an example and a reference to the mbus DT bindings documentation.

- cell-index : Which of multiple SPI controllers is this. Optional properties:
- interrupts : Is currently not used.

Documentation/devicetree/bindings/spi/spi-orion.txt +6 -0

@@ -23,3 +29,42 @@ Example:
 	       interrupts = <23>;
 	       status = "disabled";
        };
+
+Example with SPI direct mode support (optionally):
+	spi0: spi at 10600 {
+		compatible = "marvell,orion-spi";
+		#address-cells = <1>;
+		#size-cells = <0>;

Documentation/devicetree/bindings/spi/spi-orion.txt +9 -0

+To enable the direct mode, the board specific 'ranges' property in the
+'soc' node needs to add the entries for the desired SPI controllers
+and its chip-selects that are used in the direct mode instead of PIO
+mode. Here an example for this (SPI controller 0, device 1 and SPI
+controller 1, device 2 are used in direct mode. All other SPI device
+are used in the default indirect (PIO) mode):
+	soc {
+		/*
+		 * Enable the SPI direct access by configuring an entry

This would be easier if have a conclusive proof that 1MB per CS always enough. Is this something that is guaranteed in the SPI spec or the documentation for this controller?

Quoted 3 lines · Arnd Bergmann

This would be easier if have a conclusive proof that 1MB per CS always enough. Is this something that is guaranteed in the SPI spec or the documentation for this controller?

There's no spec for SPI but if there were it'd be hard to see it imposing a limit, one can transfer data as long as the bus is clocked (which some things like ADCs and DACs make use of).

Quoted 10 lines · On Monday 18 April 2016 12:04:15 Mark Brown wrote:

Quoted 3 lines · Arnd Bergmann

This would be easier if have a conclusive proof that 1MB per CS always enough. Is this something that is guaranteed in the SPI spec or the documentation for this controller?

There's no spec for SPI but if there were it'd be hard to see it imposing a limit, one can transfer data as long as the bus is clocked (which some things like ADCs and DACs make use of).

I just reread Stefan's patch and realized that I had fundamentally misunderstood how the transfer is done: I thought the offset inside of the window was used to address a NOR flash directly, but it seems it is just used to send arbitrarily long commands.

This means that the 1MB window is probably a reasonable size (provided that the (most importantly) the SPI-NOR driver can guarantee that it never exceeds this).

It also means that we are probably better off using the single-window mode of the SPI controller, where all CS lines share a single mbus window. The only real difference here is that we can map all endpoints using a single contiguous window into the CPU address space if we want, or we can still map them separately on a given board if that results in a nicer layout.

Quoted 12 lines · Arnd Bergmann

Quoted 10 lines · On Monday 18 April 2016 12:04:15 Mark Brown wrote:

Quoted 3 lines · Arnd Bergmann

This would be easier if have a conclusive proof that 1MB per CS always enough. Is this something that is guaranteed in the SPI spec or the documentation for this controller?

There's no spec for SPI but if there were it'd be hard to see it imposing a limit, one can transfer data as long as the bus is clocked (which some things like ADCs and DACs make use of).

Quoted 4 lines

I just reread Stefan's patch and realized that I had fundamentally misunderstood how the transfer is done: I thought the offset inside of the window was used to address a NOR flash directly, but it seems it is just used to send arbitrarily long commands.

Yes. SPI NOR flash support definitely needs some additional patches. To support the address and commands being inserted in the SPI messages via the corresponding registers. Again, this patch is only intended and tested for very "simple" TX messages without any addresses being inserted at all.

Quoted 3 lines

This means that the 1MB window is probably a reasonable size (provided that the (most importantly) the SPI-NOR driver can guarantee that it never exceeds this).

I have no problem switching from 1MiB to using 0xffffffff in the SPI controller 'reg' node to allow even bigger windows in v6.

Quoted 6 lines

It also means that we are probably better off using the single-window mode of the SPI controller, where all CS lines share a single mbus window. The only real difference here is that we can map all endpoints using a single contiguous window into the CPU address space if we want, or we can still map them separately on a given board if that results in a nicer layout.

Frankly, I've just read about this single-window mode for the first time. What I miss here in this mode though, is the configuration of the SPI device (chip-select 0...7) for this direct-mode in the SPI driver. With the currently implemented separate window method, your suggested method is easy: Either is window can be mapped (direct-mode) or not (indirect-mode). With this single-window mode I'm missing the per-device configuration for direct-mode. Or what is your idea on how to configure this access-mode in this case?

Quoted 18 lines · On Monday 18 April 2016 15:00:52 Stefan Roese wrote:

Quoted 12 lines · Arnd Bergmann

Quoted 10 lines · On Monday 18 April 2016 12:04:15 Mark Brown wrote:

Quoted 3 lines · Arnd Bergmann

This would be easier if have a conclusive proof that 1MB per CS always enough. Is this something that is guaranteed in the SPI spec or the documentation for this controller?

There's no spec for SPI but if there were it'd be hard to see it imposing a limit, one can transfer data as long as the bus is clocked (which some things like ADCs and DACs make use of).

Quoted 4 lines

I just reread Stefan's patch and realized that I had fundamentally misunderstood how the transfer is done: I thought the offset inside of the window was used to address a NOR flash directly, but it seems it is just used to send arbitrarily long commands.

Quoted 5 lines

Yes. SPI NOR flash support definitely needs some additional patches. To support the address and commands being inserted in the SPI messages via the corresponding registers. Again, this patch is only intended and tested for very "simple" TX messages without any addresses being inserted at all.

Ok, so I looked up the datasheet again and understood now that it actually does both ways: a) the direct read/write that I had always assumed it did with automatic cmd/addr/data generation, and b) the mode where you do each write transfer separately as implemented by your patch.

Those two modes seem to be wildly different in their needs for address space:

- For a) we definitely need the mapping to be the same size as the addressable storage of the SPI-NOR (or other device that fits into the cmd/addr/data pattern supported by the hardware)

- For b) the address is ignored and at most 32 bytes are transfered, so whatever the smallest MBUS mapping window size is would certainly be enough.

As a side-note, I see that you use a regular devm_ioremap_resource(), which I assume prevents the transfers from ever being more than a single 4-byte word, while burst mode with up to 32 bytes likely requires a write-combining mapping.

Quoted 6 lines

Quoted 3 lines

This means that the 1MB window is probably a reasonable size (provided that the (most importantly) the SPI-NOR driver can guarantee that it never exceeds this).

I have no problem switching from 1MiB to using 0xffffffff in the SPI controller 'reg' node to allow even bigger windows in v6.

How do you decide how much of the window to map though? Using 0xffffffff is nice because it better represents what the hardware looks like and doesn't limit you from having arbitrarily large mbus windows based on your needs, but you'd probably have to try mapping various sizes then to find the maximum supported one, or you'd have to do some ugly parsing of the parent node ranges.

Quoted 7 lines

Quoted 6 lines

It also means that we are probably better off using the single-window mode of the SPI controller, where all CS lines share a single mbus window. The only real difference here is that we can map all endpoints using a single contiguous window into the CPU address space if we want, or we can still map them separately on a given board if that results in a nicer layout.

Quoted 8 lines

Frankly, I've just read about this single-window mode for the first time. What I miss here in this mode though, is the configuration of the SPI device (chip-select 0...7) for this direct-mode in the SPI driver. With the currently implemented separate window method, your suggested method is easy: Either is window can be mapped (direct-mode) or not (indirect-mode). With this single-window mode I'm missing the per-device configuration for direct-mode. Or what is your idea on how to configure this access-mode in this case?

It would work the same way, the main difference is that for single-window mode all the devices get the same amount of address MBUS address space, e.g. 1MB per CS in a contiguous MBUS range, but you can still map subsections of it into the CPU, e.g.

&mbus {
	ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000>,   /* internal regs */
		 <MBUS_ID(0x01, 0x1e) 0x300000 0 0xf1100000 0x200000>, /* CS3+CS4 */
		 <MBUS_ID(0x01, 0x1d) 0x600000 0 0xf1300000 0x80000>, /* bootrom */
		 <MBUS_ID(0x01, 0x1e) 0x600000 0 0xf1380000 0x10000>; /* CS6 */

	spi at f0,01,10600 {
                 reg = <MBUS_ID(0xf0, 0x01) 0x10600 0x28>,       /* control */
                       <MBUS_ID(0x01, 0x1e) 0x000000 0x100000>, /* CS0 */
                       <MBUS_ID(0x01, 0x1e) 0x100000 0x100000>, /* CS1 */
                       <MBUS_ID(0x01, 0x1e) 0x200000 0x100000>, /* CS2 */
                       <MBUS_ID(0x01, 0x1e) 0x300000 0x100000>, /* CS3 */
                       <MBUS_ID(0x01, 0x1e) 0x400000 0x100000>, /* CS4 */
                       <MBUS_ID(0x01, 0x1e) 0x500000 0x100000>, /* CS5 */
                       <MBUS_ID(0x01, 0x1e) 0x600000 0x100000>, /* CS6 */
                       <MBUS_ID(0x01, 0x1e) 0x700000 0x100000>; /* CS7 */
	};
};

In this example, you have to configure the SPI controller to have 1MB per CS in single-window mode using the "SPI DecodeMode" and "SPI CS Adress Decode" registers. The mbus then contains two mappings, one 2MB window spanning CS3 and CS4 in a continuous CPU range, and one 64K window for a device at CS6, making it as small as possible to conserve CPU address space.

Quoted 14 lines · Arnd Bergmann

Quoted 1 line · On Monday 18 April 2016 15:00:52 Stefan Roese wrote:

On 18.04.2016 13:32, Arnd Bergmann wrote:

Ok, so I looked up the datasheet again and understood now that it actually does both ways: a) the direct read/write that I had always assumed it did with automatic cmd/addr/data generation, and b) the mode where you do each write transfer separately as implemented by your patch.

Those two modes seem to be wildly different in their needs for address space:

- For a) we definitely need the mapping to be the same size as the addressable storage of the SPI-NOR (or other device that fits into the cmd/addr/data pattern supported by the hardware)

Quoted 8 lines

- For b) the address is ignored and at most 32 bytes are transfered, so whatever the smallest MBUS mapping window size is would certainly be enough.

As a side-note, I see that you use a regular devm_ioremap_resource(), which I assume prevents the transfers from ever being more than a single 4-byte word, while burst mode with up to 32 bytes likely requires a write-combining mapping.

Thanks for the hint. I've tested again with devm_ioremap_wc() and the resulting SPI TX time is identical to the one using the driver with the uncached (non write-combined) mapping. This is most likely a result of the already fully saturated SPI bus speed being the bottle-neck here.

A bit more testing has shown, that using devm_ioremap_wc() leads to sporadic failures in the FPGA image downloading though. Now I'm unsure, if its really worth to add some cache flushing after the memcpy() call here (flush_cache_vmap ?) or better keep the uncached ioremap in place.

Quoted 7 lines

Quoted 6 lines

Quoted 3 lines

This means that the 1MB window is probably a reasonable size (provided that the (most importantly) the SPI-NOR driver can guarantee that it never exceeds this).

I have no problem switching from 1MiB to using 0xffffffff in the SPI controller 'reg' node to allow even bigger windows in v6.

Quoted 6 lines

How do you decide how much of the window to map though? Using 0xffffffff is nice because it better represents what the hardware looks like and doesn't limit you from having arbitrarily large mbus windows based on your needs, but you'd probably have to try mapping various sizes then to find the maximum supported one, or you'd have to do some ugly parsing of the parent node ranges.

This is for the single-window mode, right? My comment above was still referring to the original implementation. And yes, this gets a bit complicated and is most likely not really necessary. So I would prefer to keep the mapping fixed to a max. of 1MiB for now.

Quoted 5 lines

In this example, you have to configure the SPI controller to have 1MB per CS in single-window mode using the "SPI DecodeMode" and "SPI CS Adress Decode" registers. The mbus then contains two mappings, one 2MB window spanning CS3 and CS4 in a continuous CPU range, and one 64K window for a device at CS6, making it as small as possible to conserve CPU address space.

Thanks. I've prepared a new version implementing this single- window mode. Please let me know how you prefer the cache issue above to be handled (uncached mapping, flush or ?) and I'll update the patch accordingly.

Quoted 24 lines · On Tuesday 19 April 2016 11:42:25 Stefan Roese wrote:

Quoted 14 lines · Arnd Bergmann

Quoted 1 line · On Monday 18 April 2016 15:00:52 Stefan Roese wrote:

On 18.04.2016 13:32, Arnd Bergmann wrote:

Ok, so I looked up the datasheet again and understood now that it actually does both ways: a) the direct read/write that I had always assumed it did with automatic cmd/addr/data generation, and b) the mode where you do each write transfer separately as implemented by your patch.

Those two modes seem to be wildly different in their needs for address space:

- For a) we definitely need the mapping to be the same size as the addressable storage of the SPI-NOR (or other device that fits into the cmd/addr/data pattern supported by the hardware)

Quoted 8 lines

- For b) the address is ignored and at most 32 bytes are transfered, so whatever the smallest MBUS mapping window size is would certainly be enough.

As a side-note, I see that you use a regular devm_ioremap_resource(), which I assume prevents the transfers from ever being more than a single 4-byte word, while burst mode with up to 32 bytes likely requires a write-combining mapping.

Quoted 5 lines

Thanks for the hint. I've tested again with devm_ioremap_wc() and the resulting SPI TX time is identical to the one using the driver with the uncached (non write-combined) mapping. This is most likely a result of the already fully saturated SPI bus speed being the bottle-neck here.

Ok

Quoted 5 lines

A bit more testing has shown, that using devm_ioremap_wc() leads to sporadic failures in the FPGA image downloading though. Now I'm unsure, if its really worth to add some cache flushing after the memcpy() call here (flush_cache_vmap ?) or better keep the uncached ioremap in place.

The cache is not involved here, as devm_ioremap_wc is still uncached. However it's possible that memcpy has other issues here. If you don't gain anything from sending more than 32 bits at a time, writesl() might be a better alternative, but it requires the data to be 32-bit aligned in RAM. If the buffer is not aligned, the memcpy() might already cause problems in case the kernel implementation does not send the data in the correct order.

memcpy_toio() could also be helpful here.

Quoted 14 lines

Quoted 7 lines

Quoted 6 lines

Quoted 3 lines

This means that the 1MB window is probably a reasonable size (provided that the (most importantly) the SPI-NOR driver can guarantee that it never exceeds this).

I have no problem switching from 1MiB to using 0xffffffff in the SPI controller 'reg' node to allow even bigger windows in v6.

Quoted 6 lines

How do you decide how much of the window to map though? Using 0xffffffff is nice because it better represents what the hardware looks like and doesn't limit you from having arbitrarily large mbus windows based on your needs, but you'd probably have to try mapping various sizes then to find the maximum supported one, or you'd have to do some ugly parsing of the parent node ranges.

Quoted 5 lines

This is for the single-window mode, right? My comment above was still referring to the original implementation. And yes, this gets a bit complicated and is most likely not really necessary. So I would prefer to keep the mapping fixed to a max. of 1MiB for now.

No, I also meant the separate windows here. Based on the discussion above, I think the driver can ideally just ioremap 4KB for devices other than SPI-NOR, and then you can list the 0xffffffff length.

Quoted 6 lines

Quoted 5 lines

In this example, you have to configure the SPI controller to have 1MB per CS in single-window mode using the "SPI DecodeMode" and "SPI CS Adress Decode" registers. The mbus then contains two mappings, one 2MB window spanning CS3 and CS4 in a continuous CPU range, and one 64K window for a device at CS6, making it as small as possible to conserve CPU address space.

Quoted 4 lines

Thanks. I've prepared a new version implementing this single- window mode. Please let me know how you prefer the cache issue above to be handled (uncached mapping, flush or ?) and I'll update the patch accordingly.

If a write-combining mapping doesn't help you here, just use the normal ioremap as before.

The single-window mode won't really work with SPI-NOR unless you make each window large enough to hold the largest possible flash (128MB with single-window, 256MB with double-window), but that size makes it harder to gain much from saving mbus windows when you trade them for gigantic CPU address space consumption.

Therefore, we probably want the separate-window mode to allow the combination of large SPI flashes with other SPI devices in direct-access mode. We can also implement the single (and/or double) window mode in addition to that, but realistically we probably don't need that as all machines we support in the kernel today have at most one non-flash device.

Quoted 6 lines

The single-window mode won't really work with SPI-NOR unless you make each window large enough to hold the largest possible flash (128MB with single-window, 256MB with double-window), but that size makes it harder to gain much from saving mbus windows when you trade them for gigantic CPU address space consumption.

Quoted 6 lines

Therefore, we probably want the separate-window mode to allow the combination of large SPI flashes with other SPI devices in direct-access mode. We can also implement the single (and/or double) window mode in addition to that, but realistically we probably don't need that as all machines we support in the kernel today have at most one non-flash device.

Yes, this is also my feeling. While implementing this 0xffffffff size in the SPI controller 'reg' DT node, I just stumbled over the following problem: The resulting size of the area to map (of_address_to_resource) is not the minimum of the 'soc' 'ranges' entry and this 0xffffffff as I would have expected. But its always 0xffffffff. Do you know if this is general problem with this approach or perhaps a problem / bug in the DT address probing / translation?