spi: imx: native chip selects and devicetree

Some platform based configuration setup of spi-imx SPI devices does not set the "chip_select" to the actual hardware chip select used. This works because the cs_gpio mapping that is associated with this platform setup maps the chip_select offset used to the appropriate hardware chip select. The spi-imx driver uses the chip_select as an index into the cs_gpio array and ultimately gets the correct hardware chip select for its hardware setup.

The motivation is to be able to eventually modify the spi-imx code to use the "chip_select" directly for harwdare setup instead of indirectly via the cs_gpio mapping array.

This change only affects platforms using the hardware chip select addressing method for their SPI devices (sometimes called native chip select). The majority of devices using the spi-imx driver use the GPIO addressing method.

The change to use the correct "chip_select" is strait forward. But the cs_gpio mapping arrary also needs to be modifed to match that change. In simple terms the cs_gpio mapping should always have the hardware chip select number at its same index offset.

There is no functional change with these patches. The three affected platforms should work exactly as before. However I don't have any of these platforms (or access to them) and so I can't test them. So this patch is compile tested only.

Signed-off-by Greg Ungerer <[email protected]>

Quoted 27 lines · Greg Ungerer

Some platform based configuration setup of spi-imx SPI devices does not set the "chip_select" to the actual hardware chip select used. This works because the cs_gpio mapping that is associated with this platform setup maps the chip_select offset used to the appropriate hardware chip select. The spi-imx driver uses the chip_select as an index into the cs_gpio array and ultimately gets the correct hardware chip select for its hardware setup.

The motivation is to be able to eventually modify the spi-imx code to use the "chip_select" directly for harwdare setup instead of indirectly via the cs_gpio mapping array.

This change only affects platforms using the hardware chip select addressing method for their SPI devices (sometimes called native chip select). The majority of devices using the spi-imx driver use the GPIO addressing method.

The change to use the correct "chip_select" is strait forward. But the cs_gpio mapping arrary also needs to be modifed to match that change. In simple terms the cs_gpio mapping should always have the hardware chip select number at its same index offset.

There is no functional change with these patches. The three affected platforms should work exactly as before. However I don't have any of these platforms (or access to them) and so I can't test them. So this patch is compile tested only.

Quoted 1 line

Signed-off-by Greg Ungerer <[email protected]>

The subject prefix should be 'ARM: imx: ...'.

Since the patch can go independently, I fix up the prefix and applied the patch. Thanks.

The commonly used mechanism of specifying the hardware or native chip-select on an SPI device in devicetree (that is "cs-gpios = <0>") does not result in the native chip-select being configured for use. So external SPI devices that require use of the native chip-select will not work.

You can successfully specify native chip-selects if using a platform setup by specifying the cs-gpio as negative offset by 32. And that works correctly. You cannot use the same method in devicetree.

The logic in the spi-imx.c driver during probe uses core spi function of_spi_register_master() in spi.c to parse the "cs-gpios" devicetree tag. For valid GPIO values that will be recorded for use, all other entries in the cs_gpios list will be set to -ENOENT. So entries like "<0>" will be set to -ENOENT in the cs_gpios list.

When the SPI device registers are setup the code will use the GPIO listed in the cs_gpios list for the desired chip-select. If the cs_gpio is less then 0 then it is intended to be for a native chip-select, and its cs_gpio value is added to 32 to get the chipselect number to use. Problem is that with devicetree this can only ever be -ENOENT (which is -2), and that alone results in an invalid chip-select number. But also doesn't allow selection of the native chip-select at all.

To fix, if the cs_gpio specified for this spi device is not a valid GPIO then use the "chip_select" (that is the native chip-select number) for hardware setup.

Signed-off-by Greg Ungerer <[email protected]>

The commonly used mechanism of specifying the hardware or native chip-select on an SPI device in devicetree (that is "cs-gpios = <0>") does not result in the native chip-select being configured for use. So external SPI devices that require use of the native chip-select will not work.

You can successfully specify native chip-selects if using a platform setup by specifying the cs-gpio as negative offset by 32. And that works correctly. You cannot use the same method in devicetree.

The logic in the spi-imx.c driver during probe uses core spi function of_spi_register_master() in spi.c to parse the "cs-gpios" devicetree tag. For valid GPIO values that will be recorded for use, all other entries in the cs_gpios list will be set to -ENOENT. So entries like "<0>" will be set to -ENOENT in the cs_gpios list.

When the SPI device registers are setup the code will use the GPIO listed in the cs_gpios list for the desired chip-select. If the cs_gpio is less then 0 then it is intended to be for a native chip-select, and its cs_gpio value is added to 32 to get the chipselect number to use. Problem is that with devicetree this can only ever be -ENOENT (which is -2), and that alone results in an invalid chip-select number. But also doesn't allow selection of the native chip-select at all.

To fix, if the cs_gpio specified for this spi device is not a valid GPIO then use the "chip_select" (that is the native chip-select number) for hardware setup.

Quoted 24 lines · Greg Ungerer

The commonly used mechanism of specifying the hardware or native chip-select on an SPI device in devicetree (that is "cs-gpios = <0>") does not result in the native chip-select being configured for use. So external SPI devices that require use of the native chip-select will not work.

You can successfully specify native chip-selects if using a platform setup by specifying the cs-gpio as negative offset by 32. And that works correctly. You cannot use the same method in devicetree.

The logic in the spi-imx.c driver during probe uses core spi function of_spi_register_master() in spi.c to parse the "cs-gpios" devicetree tag. For valid GPIO values that will be recorded for use, all other entries in the cs_gpios list will be set to -ENOENT. So entries like "<0>" will be set to -ENOENT in the cs_gpios list.

When the SPI device registers are setup the code will use the GPIO listed in the cs_gpios list for the desired chip-select. If the cs_gpio is less then 0 then it is intended to be for a native chip-select, and its cs_gpio value is added to 32 to get the chipselect number to use. Problem is that with devicetree this can only ever be -ENOENT (which is -2), and that alone results in an invalid chip-select number. But also doesn't allow selection of the native chip-select at all.

Quoted 4 lines

To fix, if the cs_gpio specified for this spi device is not a valid GPIO then use the "chip_select" (that is the native chip-select number) for hardware setup.

Can you please share an example of "cs-gpios" property for a particular case, which I'm able to test? It is an Atmel AT93C66A EEPROM connected to CSPI1 and sitting on chip select 2 (= selected by CSPI1_SS2), there is no other SPI devices on CSPI1.

Since I raise the question, please add the correspondent updates and and an example to Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt

In fact my expectation would be to have a device description like one below:

&spi1 {
	status = "okay";
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_cspi1>;

	eeprom at 2 {
		compatible = "atmel,at93c66a";
		reg = <2>;
		spi-cs-high;
		spi-max-frequency = <1000000>;
	};
};

Note, that there is no cs-gpios property at all, which is mandatory at the moment, and unit address / reg property defines chip select number.

For that type of bindings locally I have a hackish spi-imx driver change, which supports this option, but I'm unsure if it is universal enough.

Quoted 2 lines · Vladimir Zapolskiy

For that type of bindings locally I have a hackish spi-imx driver change, which supports this option, but I'm unsure if it is universal enough.

Do you mean supporting no cs-gpios tag?
That would be nice, but it would seem not many users of this are using native chip selects.

Quoted 7 lines · Greg Ungerer

Quoted 2 lines · Vladimir Zapolskiy

For that type of bindings locally I have a hackish spi-imx driver change, which supports this option, but I'm unsure if it is universal enough.

Do you mean supporting no cs-gpios tag?
That would be nice, but it would seem not many users of this are using native chip selects.

The reason for this is that the native chip selects are less flexible than gpios because you cannot control when they deassert. IIRC they do it too much for some chips. So the only reason to stick to them is that on some SoCs not all pins have a GPIO function. Not sure if transfer speed is another reason, but I would expect that the gain isn't that big.

Quoted 9 lines · Uwe Kleine-K?nig

Quoted 7 lines · Greg Ungerer

Quoted 2 lines · Vladimir Zapolskiy

For that type of bindings locally I have a hackish spi-imx driver change, which supports this option, but I'm unsure if it is universal enough.

Do you mean supporting no cs-gpios tag?
That would be nice, but it would seem not many users of this are using native chip selects.

Quoted 6 lines

The reason for this is that the native chip selects are less flexible than gpios because you cannot control when they deassert. IIRC they do it too much for some chips. So the only reason to stick to them is that on some SoCs not all pins have a GPIO function. Not sure if transfer speed is another reason, but I would expect that the gain isn't that big.

For the particular SPI device I am using, a Silicon Labs 32260, it actually wants the assertion and de-assertion of the chip-select between each byte. So it is the only way it can work for me.

Quoted 17 lines · Greg Ungerer

Quoted 15 lines · Uwe Kleine-K?nig

Quoted 7 lines · Greg Ungerer

Quoted 2 lines · Vladimir Zapolskiy

For that type of bindings locally I have a hackish spi-imx driver change, which supports this option, but I'm unsure if it is universal enough.

Do you mean supporting no cs-gpios tag?
That would be nice, but it would seem not many users of this are using native chip selects.

The reason for this is that the native chip selects are less flexible than gpios because you cannot control when they deassert. IIRC they do it too much for some chips. So the only reason to stick to them is that on some SoCs not all pins have a GPIO function. Not sure if transfer speed is another reason, but I would expect that the gain isn't that big.

Quoted 3 lines

For the particular SPI device I am using, a Silicon Labs 32260, it actually wants the assertion and de-assertion of the chip-select between each byte. So it is the only way it can work for me.

That should be doable with gpio-cs, too. You just need the right flags in your spi transfer IIRC.

Hi Uwe,

Quoted 18 lines · Uwe Kleine-K?nig

Quoted 17 lines · Greg Ungerer

Quoted 15 lines · Uwe Kleine-K?nig

Quoted 7 lines · Greg Ungerer

Quoted 2 lines · Vladimir Zapolskiy

For that type of bindings locally I have a hackish spi-imx driver change, which supports this option, but I'm unsure if it is universal enough.

Do you mean supporting no cs-gpios tag?
That would be nice, but it would seem not many users of this are using native chip selects.

The reason for this is that the native chip selects are less flexible than gpios because you cannot control when they deassert. IIRC they do it too much for some chips. So the only reason to stick to them is that on some SoCs not all pins have a GPIO function. Not sure if transfer speed is another reason, but I would expect that the gain isn't that big.

Quoted 6 lines

Quoted 3 lines

For the particular SPI device I am using, a Silicon Labs 32260, it actually wants the assertion and de-assertion of the chip-select between each byte. So it is the only way it can work for me.

That should be doable with gpio-cs, too. You just need the right flags in your spi transfer IIRC.

Do you know which flag(s) do that?

Quoted 23 lines · Greg Ungerer

Quoted 22 lines · Uwe Kleine-K?nig

Quoted 20 lines · Greg Ungerer

Quoted 9 lines · Uwe Kleine-K?nig

Quoted 7 lines · Greg Ungerer

Quoted 2 lines · Vladimir Zapolskiy

For that type of bindings locally I have a hackish spi-imx driver change, which supports this option, but I'm unsure if it is universal enough.

Do you mean supporting no cs-gpios tag?
That would be nice, but it would seem not many users of this are using native chip selects.

Quoted 6 lines

The reason for this is that the native chip selects are less flexible than gpios because you cannot control when they deassert. IIRC they do it too much for some chips. So the only reason to stick to them is that on some SoCs not all pins have a GPIO function. Not sure if transfer speed is another reason, but I would expect that the gain isn't that big.

For the particular SPI device I am using, a Silicon Labs 32260, it actually wants the assertion and de-assertion of the chip-select between each byte. So it is the only way it can work for me.

Quoted 4 lines

Quoted 2 lines

That should be doable with gpio-cs, too. You just need the right flags in your spi transfer IIRC.

Do you know which flag(s) do that?

Looking at the source it's not about flags, but you have to split your transfer into several messages.

AFAICT that's how the spi stuff is
supposed to work. That is, at the start of a message CS is asserted and (only) at it's end CS is deasserted. So the imx core with native chip select actually misbehaves by toggling CS between each word.

Quoted 27 lines · Uwe Kleine-K?nig

Quoted 26 lines · Greg Ungerer

Quoted 24 lines · Uwe Kleine-K?nig

Quoted 17 lines · Greg Ungerer

Quoted 9 lines · Uwe Kleine-K?nig

Quoted 7 lines · Greg Ungerer

Quoted 2 lines · Vladimir Zapolskiy

For that type of bindings locally I have a hackish spi-imx driver change, which supports this option, but I'm unsure if it is universal enough.

Do you mean supporting no cs-gpios tag?
That would be nice, but it would seem not many users of this are using native chip selects.

Quoted 6 lines

The reason for this is that the native chip selects are less flexible than gpios because you cannot control when they deassert. IIRC they do it too much for some chips. So the only reason to stick to them is that on some SoCs not all pins have a GPIO function. Not sure if transfer speed is another reason, but I would expect that the gain isn't that big.

Quoted 3 lines

For the particular SPI device I am using, a Silicon Labs 32260, it actually wants the assertion and de-assertion of the chip-select between each byte. So it is the only way it can work for me.

That should be doable with gpio-cs, too. You just need the right flags in your spi transfer IIRC.

Quoted 4 lines

Quoted 1 line

Do you know which flag(s) do that?

Looking at the source it's not about flags, but you have to split your transfer into several messages.

... and set spi_transfer.cs_change.

Quoted 4 lines

AFAICT that's how the spi stuff is
supposed to work. That is, at the start of a message CS is asserted and (only) at it's end CS is deasserted. So the imx core with native chip select actually misbehaves by toggling CS between each word.

Indeed.

Quoted 31 lines · Geert Uytterhoeven

Quoted 29 lines · Uwe Kleine-K?nig

Quoted 27 lines · Greg Ungerer

Quoted 18 lines · Uwe Kleine-K?nig

Quoted 17 lines · Greg Ungerer

Quoted 15 lines · Uwe Kleine-K?nig

Quoted 7 lines · Greg Ungerer

Quoted 2 lines · Vladimir Zapolskiy

For that type of bindings locally I have a hackish spi-imx driver change, which supports this option, but I'm unsure if it is universal enough.

Do you mean supporting no cs-gpios tag?
That would be nice, but it would seem not many users of this are using native chip selects.

The reason for this is that the native chip selects are less flexible than gpios because you cannot control when they deassert. IIRC they do it too much for some chips. So the only reason to stick to them is that on some SoCs not all pins have a GPIO function. Not sure if transfer speed is another reason, but I would expect that the gain isn't that big.

Quoted 6 lines

Quoted 3 lines

For the particular SPI device I am using, a Silicon Labs 32260, it actually wants the assertion and de-assertion of the chip-select between each byte. So it is the only way it can work for me.

That should be doable with gpio-cs, too. You just need the right flags in your spi transfer IIRC.

Do you know which flag(s) do that?

Quoted 4 lines

Quoted 2 lines

Looking at the source it's not about flags, but you have to split your transfer into several messages.

... and set spi_transfer.cs_change.

Yep, that looks like the one. Though it would appear not all spi drivers support it. The spi-imx driver for one doesn't look at it at all.

Quoted 6 lines

Quoted 4 lines

AFAICT that's how the spi stuff is
supposed to work. That is, at the start of a message CS is asserted and (only) at it's end CS is deasserted. So the imx core with native chip select actually misbehaves by toggling CS between each word.

Indeed.

If you look around the kernel source for cs_change there is a number of devices that use it. A bunch od ADC/DACs in particular (including in backlight support).

So I don't know that I would characterize the iMX one as misbehaving so much as only natively supporting the model where chip select is asserted/deasserted per burst. It is strait forward to implement the GPIO method instead of the native chip select with the iMX pin multiplexing.

From 602c8f4485cd1e6de67e41c78db96fa4f6808e53 Mon Sep 17 00:00:00 2001

You can successfully specify native chip-selects if using a platform setup by specifying the cs-gpio as negative offset by 32. And that works correctly. You cannot use the same method in devicetree.

Quoted 3 lines · Greg Ungerer

You can successfully specify native chip-selects if using a platform setup by specifying the cs-gpio as negative offset by 32. And that works correctly. You cannot use the same method in devicetree.

I came across some spi-imx bugs in an older kernel and in checking the latest kernel, found this patch. It fixed the issue with dt not working for older spi imx devices (the imx51+ config function never used the platform data 32 offset thing), but fails to address non-dt platform data based users of the spi imx driver.

For instance, include/linux/platform_data/spi-imx.h still refers to the offset by 32 for native cs:

* @chipselect: Array of chipselects for this master. Numbers >= 0 mean gpio
* pins, numbers < 0 mean internal CSPI chipselects according
* to MXC_SPI_CS(). Normally you want to use gpio based chip

#define MXC_SPI_CS(no) ((no) - 32)

After this patch, for no-DT users, any negative (or rather not a valid gpio) value in chipselect will cause the native chipselect corresponding to the spi slave's chipselect. It's no longer possible to specify which native cs to use in the spi-imx platform data. So in something like:

arch/arm/mach-imx/mach-mx31moboard.c-static int moboard_spi1_cs[] = {
arch/arm/mach-imx/mach-mx31moboard.c: MXC_SPI_CS(0),
arch/arm/mach-imx/mach-mx31moboard.c: MXC_SPI_CS(2),
arch/arm/mach-imx/mach-mx31moboard.c-};

The spi device configured to use chip select 1 will no longer use native chip select line 2.

Looking at all in-tree users of spi imx platform data, it appears that this board is the only one in which the cs selected does not match the array index. Though there are still several which still use MSC_SPI_CS() even though it doesn't really work anymore.

Quoted 6 lines · Trent Piepho

arch/arm/mach-imx/mach-mx31moboard.c-static int moboard_spi1_cs[] = {
arch/arm/mach-imx/mach-mx31moboard.c: MXC_SPI_CS(0),
arch/arm/mach-imx/mach-mx31moboard.c: MXC_SPI_CS(2),
arch/arm/mach-imx/mach-mx31moboard.c-};

Quoted 7 lines

The spi device configured to use chip select 1 will no longer use native chip select line 2.

Looking at all in-tree users of spi imx platform data, it appears that this board is the only one in which the cs selected does not match the array index. Though there are still several which still use MSC_SPI_CS() even though it doesn't really work anymore.

Commit 901f26bce64a ("ARM: imx: set correct chip_select in platform setup") fixes this issue. It specifically makes sure that the board setup chip select is set correctly (so that spi-imx can then use it). It also pads out the chipselect arrays so that the chip select and indexes match.

Is this not working for you?

Selecting native chip selects for iMX SPI devices in a devicetree configuration does not work. That is the case for imx25 based SoC parts, and I think for imx31 SoC as well. There is no problem with configuring SPI ports to use a GPIO as chip select.

Selecting native chip selects via the old platform setup code will work, it is only devicetree configurations that are broken.

With platform configuration you specify a native chip select by setting the cs_gpio for the SPI device to be "32 - chipselect", This will be a negative number - and thus not a valid GPIO number. And that "chipselect" is the actual hardware native chip select number.

You cannot specify the cs_gpio in dvicetree as a negative number, so this whole scheme does not work. The common method in devicetree is to set the cs_gpio entry for your device to be "<0>". If you do this to configure your SPI device to use a native chip select it is valid, but the SPI device cannot be accessed (no valid read data returned from it).

The problem lies in the way the spi-imx.c driver sets up the controlling registers of the iMX SPI block. It doesn't have the correct logic for using nativce chip selects in the devicetree case.

The best fix is to use the "chip_select" associated with the SPI device as passed in from the devicetree (as the "reg" tag) or directly in the platform data structure. The hitch with that is that field is not set correctly by some platform device code.

The following patches fix the platform setup code to correctly set the "chip_select" for the SPI device and to set the SPI registers correctly based on that.

These changes affect the iMX machine code and drivers/spi subsystem so I have sent this to both lists for feedback.

Signed-off-by Greg Ungerer <[email protected]>