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

[PATCH 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 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 <gerg@linux-m68k.org>
---
 arch/arm/mach-imx/mach-mx31_3ds.c    |    7 +++++--
 arch/arm/mach-imx/mach-mx31moboard.c |    4 ++--
 arch/arm/mach-imx/mach-pcm037_eet.c  |    4 ++--
 drivers/spi/spi-imx.c                |    8 ++++----
 4 files changed, 13 insertions(+), 10 deletions(-)

[PATCH 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 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 <gerg@linux-m68k.org>
---
 arch/arm/mach-imx/mach-mx31_3ds.c    |    7 +++++--
 arch/arm/mach-imx/mach-mx31moboard.c |    4 ++--
 arch/arm/mach-imx/mach-pcm037_eet.c  |    4 ++--
 drivers/spi/spi-imx.c                |    8 ++++----
 4 files changed, 13 insertions(+), 10 deletions(-)

[PATCH 1/2] spi: imx: set correct chip_select in platform setup

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

Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
---
 arch/arm/mach-imx/mach-mx31_3ds.c    | 7 +++++--
 arch/arm/mach-imx/mach-mx31moboard.c | 4 ++--
 arch/arm/mach-imx/mach-pcm037_eet.c  | 4 ++--
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-imx/mach-mx31_3ds.c b/arch/arm/mach-imx/mach-mx31_3ds.c
index 558e5f8..68c3f07 100644
--- a/arch/arm/mach-imx/mach-mx31_3ds.c
+++ b/arch/arm/mach-imx/mach-mx31_3ds.c
@@ -375,6 +375,8 @@ static void mx31_3ds_sdhc1_setpower(struct device *dev, unsigned int vdd)
 
 /* SPI */
 static int spi0_internal_chipselect[] = {
+	MXC_SPI_CS(0),
+	MXC_SPI_CS(1),
 	MXC_SPI_CS(2),
 };
 
@@ -385,6 +387,7 @@ static void mx31_3ds_sdhc1_setpower(struct device *dev, unsigned int vdd)
 
 static int spi1_internal_chipselect[] = {
 	MXC_SPI_CS(0),
+	MXC_SPI_CS(1),
 	MXC_SPI_CS(2),
 };
 
@@ -398,7 +401,7 @@ static void mx31_3ds_sdhc1_setpower(struct device *dev, unsigned int vdd)
 		.modalias	= "mc13783",
 		.max_speed_hz	= 1000000,
 		.bus_num	= 1,
-		.chip_select	= 1, /* SS2 */
+		.chip_select	= 2, /* SS2 */
 		.platform_data	= &mc13783_pdata,
 		/* irq number is run-time assigned */
 		.mode = SPI_CS_HIGH,
@@ -406,7 +409,7 @@ static void mx31_3ds_sdhc1_setpower(struct device *dev, unsigned int vdd)
 		.modalias	= "l4f00242t03",
 		.max_speed_hz	= 5000000,
 		.bus_num	= 0,
-		.chip_select	= 0, /* SS2 */
+		.chip_select	= 2, /* SS2 */
 		.platform_data	= &mx31_3ds_l4f00242t03_pdata,
 	},
 };
diff --git a/arch/arm/mach-imx/mach-mx31moboard.c b/arch/arm/mach-imx/mach-mx31moboard.c
index cc86768..bde9a9a 100644
--- a/arch/arm/mach-imx/mach-mx31moboard.c
+++ b/arch/arm/mach-imx/mach-mx31moboard.c
@@ -296,14 +296,14 @@ static void __init moboard_uart0_init(void)
 		/* irq number is run-time assigned */
 		.max_speed_hz = 300000,
 		.bus_num = 1,
-		.chip_select = 0,
+		.chip_select = 1,
 		.platform_data = &moboard_pmic,
 		.mode = SPI_CS_HIGH,
 	},
 };
 
 static int moboard_spi2_cs[] = {
-	MXC_SPI_CS(1),
+	MXC_SPI_CS(0), MXC_SPI_CS(1),
 };
 
 static const struct spi_imx_master moboard_spi2_pdata __initconst = {
diff --git a/arch/arm/mach-imx/mach-pcm037_eet.c b/arch/arm/mach-imx/mach-pcm037_eet.c
index 8fd8255..95bd977 100644
--- a/arch/arm/mach-imx/mach-pcm037_eet.c
+++ b/arch/arm/mach-imx/mach-pcm037_eet.c
@@ -50,13 +50,13 @@
 		.modalias	= "dac124s085",
 		.max_speed_hz	= 400000,
 		.bus_num	= 0,
-		.chip_select	= 0,		/* Index in pcm037_spi1_cs[] */
+		.chip_select	= 1,		/* Index in pcm037_spi1_cs[] */
 		.mode		= SPI_CPHA,
 	},
 };
 
 /* Platform Data for MXC CSPI */
-static int pcm037_spi1_cs[] = {MXC_SPI_CS(1), IOMUX_TO_GPIO(MX31_PIN_KEY_COL7)};
+static int pcm037_spi1_cs[] = { MXC_SPI_CS(0), MXC_SPI_CS(1), };
 
 static const struct spi_imx_master pcm037_spi1_pdata __initconst = {
 	.chipselect = pcm037_spi1_cs,
-- 
1.9.1

[PATCH 1/2] spi: imx: set correct chip_select in platform setup

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

Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
---
 arch/arm/mach-imx/mach-mx31_3ds.c    | 7 +++++--
 arch/arm/mach-imx/mach-mx31moboard.c | 4 ++--
 arch/arm/mach-imx/mach-pcm037_eet.c  | 4 ++--
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-imx/mach-mx31_3ds.c b/arch/arm/mach-imx/mach-mx31_3ds.c
index 558e5f8..68c3f07 100644
--- a/arch/arm/mach-imx/mach-mx31_3ds.c
+++ b/arch/arm/mach-imx/mach-mx31_3ds.c
@@ -375,6 +375,8 @@ static void mx31_3ds_sdhc1_setpower(struct device *dev, unsigned int vdd)
 
 /* SPI */
 static int spi0_internal_chipselect[] = {
+	MXC_SPI_CS(0),
+	MXC_SPI_CS(1),
 	MXC_SPI_CS(2),
 };
 
@@ -385,6 +387,7 @@ static void mx31_3ds_sdhc1_setpower(struct device *dev, unsigned int vdd)
 
 static int spi1_internal_chipselect[] = {
 	MXC_SPI_CS(0),
+	MXC_SPI_CS(1),
 	MXC_SPI_CS(2),
 };
 
@@ -398,7 +401,7 @@ static void mx31_3ds_sdhc1_setpower(struct device *dev, unsigned int vdd)
 		.modalias	= "mc13783",
 		.max_speed_hz	= 1000000,
 		.bus_num	= 1,
-		.chip_select	= 1, /* SS2 */
+		.chip_select	= 2, /* SS2 */
 		.platform_data	= &mc13783_pdata,
 		/* irq number is run-time assigned */
 		.mode = SPI_CS_HIGH,
@@ -406,7 +409,7 @@ static void mx31_3ds_sdhc1_setpower(struct device *dev, unsigned int vdd)
 		.modalias	= "l4f00242t03",
 		.max_speed_hz	= 5000000,
 		.bus_num	= 0,
-		.chip_select	= 0, /* SS2 */
+		.chip_select	= 2, /* SS2 */
 		.platform_data	= &mx31_3ds_l4f00242t03_pdata,
 	},
 };
diff --git a/arch/arm/mach-imx/mach-mx31moboard.c b/arch/arm/mach-imx/mach-mx31moboard.c
index cc86768..bde9a9a 100644
--- a/arch/arm/mach-imx/mach-mx31moboard.c
+++ b/arch/arm/mach-imx/mach-mx31moboard.c
@@ -296,14 +296,14 @@ static void __init moboard_uart0_init(void)
 		/* irq number is run-time assigned */
 		.max_speed_hz = 300000,
 		.bus_num = 1,
-		.chip_select = 0,
+		.chip_select = 1,
 		.platform_data = &moboard_pmic,
 		.mode = SPI_CS_HIGH,
 	},
 };
 
 static int moboard_spi2_cs[] = {
-	MXC_SPI_CS(1),
+	MXC_SPI_CS(0), MXC_SPI_CS(1),
 };
 
 static const struct spi_imx_master moboard_spi2_pdata __initconst = {
diff --git a/arch/arm/mach-imx/mach-pcm037_eet.c b/arch/arm/mach-imx/mach-pcm037_eet.c
index 8fd8255..95bd977 100644
--- a/arch/arm/mach-imx/mach-pcm037_eet.c
+++ b/arch/arm/mach-imx/mach-pcm037_eet.c
@@ -50,13 +50,13 @@
 		.modalias	= "dac124s085",
 		.max_speed_hz	= 400000,
 		.bus_num	= 0,
-		.chip_select	= 0,		/* Index in pcm037_spi1_cs[] */
+		.chip_select	= 1,		/* Index in pcm037_spi1_cs[] */
 		.mode		= SPI_CPHA,
 	},
 };
 
 /* Platform Data for MXC CSPI */
-static int pcm037_spi1_cs[] = {MXC_SPI_CS(1), IOMUX_TO_GPIO(MX31_PIN_KEY_COL7)};
+static int pcm037_spi1_cs[] = { MXC_SPI_CS(0), MXC_SPI_CS(1), };
 
 static const struct spi_imx_master pcm037_spi1_pdata __initconst = {
 	.chipselect = pcm037_spi1_cs,
-- 
1.9.1

Re: [PATCH 1/2] spi: imx: set correct chip_select in platform setup

[Shawn Guo]

On Fri, Mar 17, 2017 at 03:03:02PM +1000, Greg Ungerer wrote:
> 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 <gerg-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>

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

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

Shawn
--
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

[PATCH 1/2] spi: imx: set correct chip_select in platform setup

[Shawn Guo]

On Fri, Mar 17, 2017 at 03:03:02PM +1000, Greg Ungerer wrote:
> 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 <gerg@linux-m68k.org>

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

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

Shawn

Re: [PATCH 1/2] spi: imx: set correct chip_select in platform setup

[Greg Ungerer]

On 20/03/17 17:38, Shawn Guo wrote:
> On Fri, Mar 17, 2017 at 03:03:02PM +1000, Greg Ungerer wrote:
>> 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 <gerg-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
>
> The subject prefix should be 'ARM: imx: ...'.
>
> Since the patch can go independently, I fix up the prefix and applied
> the patch.  Thanks.

Thanks Shawn.

Regards
Greg



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

[PATCH 1/2] spi: imx: set correct chip_select in platform setup

[Greg Ungerer]

On 20/03/17 17:38, Shawn Guo wrote:
> On Fri, Mar 17, 2017 at 03:03:02PM +1000, Greg Ungerer wrote:
>> 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 <gerg@linux-m68k.org>
>
> The subject prefix should be 'ARM: imx: ...'.
>
> Since the patch can go independently, I fix up the prefix and applied
> the patch.  Thanks.

Thanks Shawn.

Regards
Greg

[PATCH 2/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(-)

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

[PATCH 2/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@linux-m68k.org>
---
 drivers/spi/spi-imx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

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

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

[Shawn Guo]

On Fri, Mar 17, 2017 at 03:03:03PM +1000, Greg Ungerer wrote:
> 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@linux-m68k.org>

Acked-by: Shawn Guo <shawnguo@kernel.org>

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

[Shawn Guo]

On Fri, Mar 17, 2017 at 03:03:03PM +1000, Greg Ungerer wrote:
> 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@linux-m68k.org>

Acked-by: Shawn Guo <shawnguo@kernel.org>

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

[Vladimir Zapolskiy]

Hi Greg,

On 03/17/2017 07:03 AM, Greg Ungerer wrote:
> 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.
> 

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

--
With best wishes,
Vladimir
--
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

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

[Vladimir Zapolskiy]

Hi Greg,

On 03/17/2017 07:03 AM, Greg Ungerer wrote:
> 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.
> 

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.

--
With best wishes,
Vladimir

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

[Greg Ungerer]

Hi Vladimir,

On 20/03/17 23:22, Vladimir Zapolskiy wrote:
> On 03/17/2017 07:03 AM, Greg Ungerer wrote:
>> 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.
>>
> 
> 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.

Is the AT93C66A going to work if you use native chip selects?

I have an iMX25 based board with a NOR flash on SPI1 using
a GPIO chip select, and on SPI2 a Silicon Labs 32260 SLIC
using the native chip select 1. It requires the chip select line
to be asserted and de-asserted between each byte, as done when
using the native chip select setup of the iMX SPI module.

Anyway, the cs-gpio I use is:

  cs-gpios = <0>, <0>, <0>, <0>;


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

Ok.

There is a few other devicetrees in arc/arm/boot/dts that use the "<0>"
notation for cs-gpios. Documentation/devicetree/bindings/spi/spi-davinci.txt
does describe its meaning.


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

Ideally it would be allowed to have no cs-gpios at all. But currently
the spi-imx driver explicitly fails on this:

  spi.1: No CS GPIOs available

As an example my devicetree entry is similar to your above, but
it has "cs-gpios = <0>, <0>, <0>, <0>;" as well. Thats all.


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

Regards
Greg

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

[Greg Ungerer]

Hi Vladimir,

On 20/03/17 23:22, Vladimir Zapolskiy wrote:
> On 03/17/2017 07:03 AM, Greg Ungerer wrote:
>> 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.
>>
> 
> 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.

Is the AT93C66A going to work if you use native chip selects?

I have an iMX25 based board with a NOR flash on SPI1 using
a GPIO chip select, and on SPI2 a Silicon Labs 32260 SLIC
using the native chip select 1. It requires the chip select line
to be asserted and de-asserted between each byte, as done when
using the native chip select setup of the iMX SPI module.

Anyway, the cs-gpio I use is:

  cs-gpios = <0>, <0>, <0>, <0>;


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

Ok.

There is a few other devicetrees in arc/arm/boot/dts that use the "<0>"
notation for cs-gpios. Documentation/devicetree/bindings/spi/spi-davinci.txt
does describe its meaning.


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

Ideally it would be allowed to have no cs-gpios at all. But currently
the spi-imx driver explicitly fails on this:

  spi.1: No CS GPIOs available

As an example my devicetree entry is similar to your above, but
it has "cs-gpios = <0>, <0>, <0>, <0>;" as well. Thats all.


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

Regards
Greg

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

[Uwe Kleine-König]

Hello,

On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
> On 20/03/17 23:22, Vladimir Zapolskiy wrote:
> > 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.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
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

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

[Uwe Kleine-König]

Hello,

On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
> On 20/03/17 23:22, Vladimir Zapolskiy wrote:
> > 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.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

[Greg Ungerer]

On 21/03/17 18:05, Uwe Kleine-König wrote:
> On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
>> On 20/03/17 23:22, Vladimir Zapolskiy wrote:
>>> 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.

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.

Regards
Greg

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

[Greg Ungerer]

On 21/03/17 18:05, Uwe Kleine-K?nig wrote:
> On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
>> On 20/03/17 23:22, Vladimir Zapolskiy wrote:
>>> 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.

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.

Regards
Greg

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

[Uwe Kleine-König]

On Tue, Mar 21, 2017 at 09:53:52PM +1000, Greg Ungerer wrote:
> On 21/03/17 18:05, Uwe Kleine-König wrote:
> > On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
> > > On 20/03/17 23:22, Vladimir Zapolskiy wrote:
> > > > 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.
> 
> 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.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
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

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

[Uwe Kleine-König]

On Tue, Mar 21, 2017 at 09:53:52PM +1000, Greg Ungerer wrote:
> On 21/03/17 18:05, Uwe Kleine-K?nig wrote:
> > On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
> > > On 20/03/17 23:22, Vladimir Zapolskiy wrote:
> > > > 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.
> 
> 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.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

[Greg Ungerer]

Hi Uwe,

On 21/03/17 22:11, Uwe Kleine-König wrote:
> On Tue, Mar 21, 2017 at 09:53:52PM +1000, Greg Ungerer wrote:
>> On 21/03/17 18:05, Uwe Kleine-König wrote:
>>> On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
>>>> On 20/03/17 23:22, Vladimir Zapolskiy wrote:
>>>>> 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.
>>
>> 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?

Regards
Greg



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

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

[Greg Ungerer]

Hi Uwe,

On 21/03/17 22:11, Uwe Kleine-K?nig wrote:
> On Tue, Mar 21, 2017 at 09:53:52PM +1000, Greg Ungerer wrote:
>> On 21/03/17 18:05, Uwe Kleine-K?nig wrote:
>>> On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
>>>> On 20/03/17 23:22, Vladimir Zapolskiy wrote:
>>>>> 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.
>>
>> 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?

Regards
Greg

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

[Uwe Kleine-König]

On Tue, Mar 21, 2017 at 11:22:27PM +1000, Greg Ungerer wrote:
> Hi Uwe,
> 
> On 21/03/17 22:11, Uwe Kleine-König wrote:
> > On Tue, Mar 21, 2017 at 09:53:52PM +1000, Greg Ungerer wrote:
> > > On 21/03/17 18:05, Uwe Kleine-König wrote:
> > > > On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
> > > > > On 20/03/17 23:22, Vladimir Zapolskiy wrote:
> > > > > > 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.
> > > 
> > > 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?

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.

@broonie: Can you confirm or contradict please?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
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

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

[Uwe Kleine-König]

On Tue, Mar 21, 2017 at 11:22:27PM +1000, Greg Ungerer wrote:
> Hi Uwe,
> 
> On 21/03/17 22:11, Uwe Kleine-K?nig wrote:
> > On Tue, Mar 21, 2017 at 09:53:52PM +1000, Greg Ungerer wrote:
> > > On 21/03/17 18:05, Uwe Kleine-K?nig wrote:
> > > > On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
> > > > > On 20/03/17 23:22, Vladimir Zapolskiy wrote:
> > > > > > 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.
> > > 
> > > 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?

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.

@broonie: Can you confirm or contradict please?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

[Geert Uytterhoeven]

Hi Uwe,

On Tue, Mar 21, 2017 at 8:23 PM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Tue, Mar 21, 2017 at 11:22:27PM +1000, Greg Ungerer wrote:
>> On 21/03/17 22:11, Uwe Kleine-König wrote:
>> > On Tue, Mar 21, 2017 at 09:53:52PM +1000, Greg Ungerer wrote:
>> > > On 21/03/17 18:05, Uwe Kleine-König wrote:
>> > > > On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
>> > > > > On 20/03/17 23:22, Vladimir Zapolskiy wrote:
>> > > > > > 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.
>> > >
>> > > 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?
>
> 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.

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

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Geert Uytterhoeven]

Hi Uwe,

On Tue, Mar 21, 2017 at 8:23 PM, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:
> On Tue, Mar 21, 2017 at 11:22:27PM +1000, Greg Ungerer wrote:
>> On 21/03/17 22:11, Uwe Kleine-K?nig wrote:
>> > On Tue, Mar 21, 2017 at 09:53:52PM +1000, Greg Ungerer wrote:
>> > > On 21/03/17 18:05, Uwe Kleine-K?nig wrote:
>> > > > On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
>> > > > > On 20/03/17 23:22, Vladimir Zapolskiy wrote:
>> > > > > > 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.
>> > >
>> > > 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?
>
> 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.

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

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Greg Ungerer]

Hi Geert, Uwe,

On 22/03/17 06:15, Geert Uytterhoeven wrote:
> On Tue, Mar 21, 2017 at 8:23 PM, Uwe Kleine-König
> <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
>> On Tue, Mar 21, 2017 at 11:22:27PM +1000, Greg Ungerer wrote:
>>> On 21/03/17 22:11, Uwe Kleine-König wrote:
>>>> On Tue, Mar 21, 2017 at 09:53:52PM +1000, Greg Ungerer wrote:
>>>>> On 21/03/17 18:05, Uwe Kleine-König wrote:
>>>>>> On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
>>>>>>> On 20/03/17 23:22, Vladimir Zapolskiy wrote:
>>>>>>>> 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.
>>>>>
>>>>> 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?
>>
>> 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.


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

Regards
Greg


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

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

[Greg Ungerer]

Hi Geert, Uwe,

On 22/03/17 06:15, Geert Uytterhoeven wrote:
> On Tue, Mar 21, 2017 at 8:23 PM, Uwe Kleine-K?nig
> <u.kleine-koenig@pengutronix.de> wrote:
>> On Tue, Mar 21, 2017 at 11:22:27PM +1000, Greg Ungerer wrote:
>>> On 21/03/17 22:11, Uwe Kleine-K?nig wrote:
>>>> On Tue, Mar 21, 2017 at 09:53:52PM +1000, Greg Ungerer wrote:
>>>>> On 21/03/17 18:05, Uwe Kleine-K?nig wrote:
>>>>>> On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
>>>>>>> On 20/03/17 23:22, Vladimir Zapolskiy wrote:
>>>>>>>> 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.
>>>>>
>>>>> 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?
>>
>> 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.


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

Regards
Greg

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

[Geert Uytterhoeven]

Hi Greg,

On Wed, Mar 22, 2017 at 1:50 AM, Greg Ungerer <gerg@linux-m68k.org> wrote:
> On 22/03/17 06:15, Geert Uytterhoeven wrote:
>> On Tue, Mar 21, 2017 at 8:23 PM, Uwe Kleine-König
>> <u.kleine-koenig@pengutronix.de> wrote:
>>> On Tue, Mar 21, 2017 at 11:22:27PM +1000, Greg Ungerer wrote:
>>>> On 21/03/17 22:11, Uwe Kleine-König wrote:
>>>>> On Tue, Mar 21, 2017 at 09:53:52PM +1000, Greg Ungerer wrote:
>>>>>> On 21/03/17 18:05, Uwe Kleine-König wrote:
>>>>>>> On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
>>>>>>>> On 20/03/17 23:22, Vladimir Zapolskiy wrote:
>>>>>>>>> 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.
>>>>>>
>>>>>> 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?
>>>
>>> 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.

Correct. With many drivers, you must use cs-gpios to support that feature.
Remember, SPI is a mixed bag of features, and not all of them are supported
on all controllers.

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

It's up to the system integrator to specify cs-gpios when needed.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Geert Uytterhoeven]

Hi Greg,

On Wed, Mar 22, 2017 at 1:50 AM, Greg Ungerer <gerg@linux-m68k.org> wrote:
> On 22/03/17 06:15, Geert Uytterhoeven wrote:
>> On Tue, Mar 21, 2017 at 8:23 PM, Uwe Kleine-K?nig
>> <u.kleine-koenig@pengutronix.de> wrote:
>>> On Tue, Mar 21, 2017 at 11:22:27PM +1000, Greg Ungerer wrote:
>>>> On 21/03/17 22:11, Uwe Kleine-K?nig wrote:
>>>>> On Tue, Mar 21, 2017 at 09:53:52PM +1000, Greg Ungerer wrote:
>>>>>> On 21/03/17 18:05, Uwe Kleine-K?nig wrote:
>>>>>>> On Tue, Mar 21, 2017 at 12:05:20PM +1000, Greg Ungerer wrote:
>>>>>>>> On 20/03/17 23:22, Vladimir Zapolskiy wrote:
>>>>>>>>> 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.
>>>>>>
>>>>>> 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?
>>>
>>> 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.

Correct. With many drivers, you must use cs-gpios to support that feature.
Remember, SPI is a mixed bag of features, and not all of them are supported
on all controllers.

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

It's up to the system integrator to specify cs-gpios when needed.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Applied "spi: imx: fix use of native chip-selects with devicetree" to the spi tree

[Mark Brown]

The patch

   spi: imx: fix use of native chip-selects with devicetree

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 602c8f4485cd1e6de67e41c78db96fa4f6808e53 Mon Sep 17 00:00:00 2001
From: Greg Ungerer <gerg-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
Date: Tue, 11 Jul 2017 14:22:11 +1000
Subject: [PATCH] spi: imx: fix use of native chip-selects with devicetree

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>
Reviewed-by: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
Tested-by: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-imx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 6fcb6adf9565..babb15f07995 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -622,8 +622,8 @@ static int mx31_config(struct spi_device *spi)
 		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);
 
@@ -714,8 +714,8 @@ static int mx21_config(struct spi_device *spi)
 		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);
 
-- 
2.13.2

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

Applied "spi: imx: fix use of native chip-selects with devicetree" to the spi tree

[Mark Brown]

The patch

   spi: imx: fix use of native chip-selects with devicetree

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 602c8f4485cd1e6de67e41c78db96fa4f6808e53 Mon Sep 17 00:00:00 2001
From: Greg Ungerer <gerg@linux-m68k.org>
Date: Tue, 11 Jul 2017 14:22:11 +1000
Subject: [PATCH] spi: imx: fix use of native chip-selects with devicetree

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@linux-m68k.org>
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
Tested-by: Vladimir Zapolskiy <vz@mleia.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-imx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 6fcb6adf9565..babb15f07995 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -622,8 +622,8 @@ static int mx31_config(struct spi_device *spi)
 		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);
 
@@ -714,8 +714,8 @@ static int mx21_config(struct spi_device *spi)
 		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);
 
-- 
2.13.2

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

[Trent Piepho]

On Fri, 2017-03-17 at 15:03 +1000, Greg Ungerer wrote:
> 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.

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

[Trent Piepho]

On Fri, 2017-03-17 at 15:03 +1000, Greg Ungerer wrote:
> 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@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.

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

[Greg Ungerer]

Hi Trent,

On 11/10/17 06:38, Trent Piepho wrote:
> On Fri, 2017-03-17 at 15:03 +1000, Greg Ungerer wrote:
>> 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.

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?

Regards
Greg

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

[Greg Ungerer]

Hi Trent,

On 11/10/17 06:38, Trent Piepho wrote:
> On Fri, 2017-03-17 at 15:03 +1000, Greg Ungerer wrote:
>> 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.

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?

Regards
Greg

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

[Trent Piepho]

On Thu, 2017-10-12 at 16:26 +1000, Greg Ungerer wrote:
> On 11/10/17 06:38, Trent Piepho wrote:
> > 
> > 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.
> 
> 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?

It looks like that patch missed one spi bus in mx31moboard.c, quoted
above.

I have a patch series that should fix that and also the documentation. 
Need to test it a bit more.

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

[Trent Piepho]

On Thu, 2017-10-12 at 16:26 +1000, Greg Ungerer wrote:
> On 11/10/17 06:38, Trent Piepho wrote:
> > 
> > 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.
> 
> 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?

It looks like that patch missed one spi bus in mx31moboard.c, quoted
above.

I have a patch series that should fix that and also the documentation. 
Need to test it a bit more.

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

[Shawn Guo]

On Fri, Mar 17, 2017 at 03:03:01PM +1000, Greg Ungerer wrote:
> 
> 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 <gerg-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>

The device tree support was added for imx spi driver with only the case
of GPIO being chip select in consideration, as that's the case for all
recent i.MX SoCs.  So, yes, native chip select never worked for device
tree case before.  Thanks for getting it work.

Shawn
--
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

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

[Shawn Guo]

On Fri, Mar 17, 2017 at 03:03:01PM +1000, Greg Ungerer wrote:
> 
> 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 <gerg@linux-m68k.org>

The device tree support was added for imx spi driver with only the case
of GPIO being chip select in consideration, as that's the case for all
recent i.MX SoCs.  So, yes, native chip select never worked for device
tree case before.  Thanks for getting it work.

Shawn