[PATCHv2 0/2] spi: imx: native chip selects and devicetree

[PATCHv2 0/2] spi: imx: native chip selects and devicetree

[Greg Ungerer]

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 native chip selects in the devicetree case.

V1 of this patch set contained the fixes for some platform setups to
make sure that the correct "chip_select" is used that is associated with
the required native chip select. This is dropped in v2, since Shawn
picked that change up for arch/arm/imx.

The following patches fix use of native chip selects in the spi-imx
driver for the devicetree case, and also document the use of the "<0>"
notation in devicetree.

Signed-off-by: Greg Ungerer <gerg-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
---
 Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt |    4 +++-
 drivers/spi/spi-imx.c                                  |    8 ++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 1/2] spi: imx: fix use of native chip-selects with devicetree

[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.

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 <gerg-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
---
 drivers/spi/spi-imx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

v2: added documentation to patch set

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 9a7c62f..dbb482c 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -516,8 +516,8 @@ static int mx31_config(struct spi_device *spi, struct spi_imx_config *config)
 		reg |= MX31_CSPICTRL_POL;
 	if (spi->mode & SPI_CS_HIGH)
 		reg |= MX31_CSPICTRL_SSPOL;
-	if (spi->cs_gpio < 0)
-		reg |= (spi->cs_gpio + 32) <<
+	if (!gpio_is_valid(spi->cs_gpio))
+		reg |= (spi->chip_select) <<
 			(is_imx35_cspi(spi_imx) ? MX35_CSPICTRL_CS_SHIFT :
 						  MX31_CSPICTRL_CS_SHIFT);
 
@@ -608,8 +608,8 @@ static int mx21_config(struct spi_device *spi, struct spi_imx_config *config)
 		reg |= MX21_CSPICTRL_POL;
 	if (spi->mode & SPI_CS_HIGH)
 		reg |= MX21_CSPICTRL_SSPOL;
-	if (spi->cs_gpio < 0)
-		reg |= (spi->cs_gpio + 32) << MX21_CSPICTRL_CS_SHIFT;
+	if (!gpio_is_valid(spi->cs_gpio))
+		reg |= spi->chip_select << MX21_CSPICTRL_CS_SHIFT;
 
 	writel(reg, spi_imx->base + MXC_CSPICTRL);
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 2/2] spi: imx: document use of native chip-selects in devicetree

[Greg Ungerer]

Document the "<0>" notation for specifying use of a native chip-select
in the "cs-gpios" tag of an SPI device in devicetree. This isn't unique
to the spi-imx driver, but this clearly spells out that you can use it
with this driver.

Signed-off-by: Greg Ungerer <gerg-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
---
 Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

v2: added documentation to patch set

diff --git a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
index 8bc95e2..cfc60cb 100644
--- a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
+++ b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
@@ -12,6 +12,7 @@ Required properties:
 - reg : Offset and length of the register set for the device
 - interrupts : Should contain CSPI/eCSPI interrupt
 - cs-gpios : Specifies the gpio pins to be used for chipselects.
+	Specify use of native chipselects with "<0>" in place of a gpio.
 - clocks : Clock specifiers for both ipg and per clocks.
 - clock-names : Clock names should include both "ipg" and "per"
 See the clock consumer binding,
@@ -32,7 +33,8 @@ ecspi@70010000 {
 	reg = <0x70010000 0x4000>;
 	interrupts = <36>;
 	cs-gpios = <&gpio3 24 0>, /* GPIO3_24 */
-		   <&gpio3 25 0>; /* GPIO3_25 */
+		   <&gpio3 25 0>, /* GPIO3_25 */
+		   <0>;           /* Native chip select */
 	dmas = <&sdma 3 7 1>, <&sdma 4 7 2>;
 	dma-names = "rx", "tx";
 };
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html