| Message ID | 1489726983-17706-3-git-send-email-gerg@linux-m68k.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>
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
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
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
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
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
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
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
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
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
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
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.
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
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.
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);
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(-)