[PATCH v5 0/4] spi: add SPI_MOSI_IDLE_LOW mode bit

[PATCH v5 0/4] spi: add SPI_MOSI_IDLE_LOW mode bit

[Boerge Struempfel]

Some spi controller switch the mosi line to high, whenever they are
idle. This may not be desired in all use cases. For example neopixel
leds can get confused and flicker due to misinterpreting the idle state.
Therefore, we introduce a new spi-mode bit, with which the idle behaviour
can be overwritten on a per device basis.

---

Link for versions:
  v1 and v2: https://lore.kernel.org/linux-spi/20230511135632.78344-1-bstruempfel@ultratronik.de/
  v3: https://lore.kernel.org/linux-spi/20230517103007.26287-1-boerge.struempfel@gmail.com/T/#t
  v4: https://lore.kernel.org/linux-spi/CAEktqctboF3=ykVNtPsifcmHzF6dWwoEcVh+O4H1u-R=TT6gHg@mail.gmail.com/T/#t

Changes from V4:
  - Added the SPI_3WIRE_HIZ mode bit to spidev
  - Added the SPI_MOSI_IDLE_LOW mode bit to the spidev_test
    userspace tool and added the two other missing spi mode
    bits (SPI_3WIRE_HIZ and SPI_RX_CPHA_FLIP) to it as well.

Changes from V3:
  - Added missing paranthesis which caused builderrors

Changes from V2:
  - Removed the device-tree binding since this should not be managed by
    the DT but by the device itself.
  - Replaced all occurences of spi->chip_select with the corresponding
    macro spi_get_chipselect(spi,0)

Changes from V1:
  - Added patch, introducing the new devicetree binding flag
  - Split the generic spi part of the patch from the imx-spi specific
    part
  - Replaced SPI_CPOL and SPI_CPHA by the combined SPI_MODE_X_MASK bit
    in the imx-spi.c modebits.
  - Added the SPI_MOSI_IDLE_LOW bit to spidev

Boerge Struempfel (4):
  spi: add SPI_MOSI_IDLE_LOW mode bit
  spi: spi-imx: add support for SPI_MOSI_IDLE_LOW mode bit
  spi: spidev: add two new spi mode bits
  spi: spidev_test Add three missing spi mode bits

 drivers/spi/spi-imx.c        |   9 +++-
 drivers/spi/spidev.c         |   3 +-
 include/uapi/linux/spi/spi.h |   3 +-
 tools/spi/spidev_test.c      | 101 ++++++++++++++++++++---------------
 4 files changed, 70 insertions(+), 46 deletions(-)

-- 
2.25.1

[PATCH v5 1/4] spi: add SPI_MOSI_IDLE_LOW mode bit

[Boerge Struempfel]

Some spi controller switch the mosi line to high, whenever they are
idle. This may not be desired in all use cases. For example neopixel
leds can get confused and flicker due to misinterpreting the idle state.
Therefore, we introduce a new spi-mode bit, with which the idle behaviour
can be overwritten on a per device basis.

Signed-off-by: Boerge Struempfel <boerge.struempfel@gmail.com>
---
 include/uapi/linux/spi/spi.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/spi/spi.h b/include/uapi/linux/spi/spi.h
index 9d5f58059703..ca56e477d161 100644
--- a/include/uapi/linux/spi/spi.h
+++ b/include/uapi/linux/spi/spi.h
@@ -28,6 +28,7 @@
 #define	SPI_RX_OCTAL		_BITUL(14)	/* receive with 8 wires */
 #define	SPI_3WIRE_HIZ		_BITUL(15)	/* high impedance turnaround */
 #define	SPI_RX_CPHA_FLIP	_BITUL(16)	/* flip CPHA on Rx only xfer */
+#define SPI_MOSI_IDLE_LOW	_BITUL(17)	/* leave mosi line low when idle */
 
 /*
  * All the bits defined above should be covered by SPI_MODE_USER_MASK.
@@ -37,6 +38,6 @@
  * These bits must not overlap. A static assert check should make sure of that.
  * If adding extra bits, make sure to increase the bit index below as well.
  */
-#define SPI_MODE_USER_MASK	(_BITUL(17) - 1)
+#define SPI_MODE_USER_MASK	(_BITUL(18) - 1)
 
 #endif /* _UAPI_SPI_H */
-- 
2.25.1

[PATCH v5 2/4] spi: spi-imx: add support for SPI_MOSI_IDLE_LOW mode bit

[Boerge Struempfel]

By default, the spi-imx controller pulls the mosi line high, whenever it
is idle. This behaviour can be inverted per CS by setting the
corresponding DATA_CTL bit in the config register of the controller.

Also, since the controller mode-bits have to be touched anyways, the
SPI_CPOL and SPI_CPHA are replaced by the combined SPI_MODE_X_MASK flag.

Signed-off-by: Boerge Struempfel <boerge.struempfel@gmail.com>
---
 drivers/spi/spi-imx.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 34e5f81ec431..1c4172fcba2d 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -281,6 +281,7 @@ static bool spi_imx_can_dma(struct spi_controller *controller, struct spi_device
 #define MX51_ECSPI_CONFIG_SCLKPOL(cs)	(1 << ((cs & 3) +  4))
 #define MX51_ECSPI_CONFIG_SBBCTRL(cs)	(1 << ((cs & 3) +  8))
 #define MX51_ECSPI_CONFIG_SSBPOL(cs)	(1 << ((cs & 3) + 12))
+#define MX51_ECSPI_CONFIG_DATACTL(cs)	(1 << ((cs & 3) + 16))
 #define MX51_ECSPI_CONFIG_SCLKCTL(cs)	(1 << ((cs & 3) + 20))
 
 #define MX51_ECSPI_INT		0x10
@@ -573,6 +574,11 @@ static int mx51_ecspi_prepare_message(struct spi_imx_data *spi_imx,
 		cfg &= ~MX51_ECSPI_CONFIG_SCLKCTL(spi_get_chipselect(spi, 0));
 	}
 
+	if (spi->mode & SPI_MOSI_IDLE_LOW)
+		cfg |= MX51_ECSPI_CONFIG_DATACTL(spi_get_chipselect(spi, 0));
+	else
+		cfg &= ~MX51_ECSPI_CONFIG_DATACTL(spi_get_chipselect(spi, 0));
+
 	if (spi->mode & SPI_CS_HIGH)
 		cfg |= MX51_ECSPI_CONFIG_SSBPOL(spi_get_chipselect(spi, 0));
 	else
@@ -1743,7 +1749,8 @@ static int spi_imx_probe(struct platform_device *pdev)
 	spi_imx->controller->prepare_message = spi_imx_prepare_message;
 	spi_imx->controller->unprepare_message = spi_imx_unprepare_message;
 	spi_imx->controller->slave_abort = spi_imx_slave_abort;
-	spi_imx->controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_NO_CS;
+	spi_imx->controller->mode_bits = SPI_MODE_X_MASK | SPI_CS_HIGH | SPI_NO_CS |
+					 SPI_MOSI_IDLE_LOW;
 
 	if (is_imx35_cspi(spi_imx) || is_imx51_ecspi(spi_imx) ||
 	    is_imx53_ecspi(spi_imx))
-- 
2.25.1

[PATCH v5 3/4] spi: spidev: add two new spi mode bits

[Boerge Struempfel]

Allow userspace to set SPI_MOSI_IDLE_LOW and the SPI_3WIRE_HIZ mode bit
using the SPI_IOC_WR_MODE32 ioctl.

Signed-off-by: Boerge Struempfel <boerge.struempfel@gmail.com>
---
 drivers/spi/spidev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index 39d94c850839..c8b938e0f342 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -64,7 +64,8 @@ static_assert(N_SPI_MINORS > 0 && N_SPI_MINORS <= 256);
 				| SPI_NO_CS | SPI_READY | SPI_TX_DUAL \
 				| SPI_TX_QUAD | SPI_TX_OCTAL | SPI_RX_DUAL \
 				| SPI_RX_QUAD | SPI_RX_OCTAL \
-				| SPI_RX_CPHA_FLIP)
+				| SPI_RX_CPHA_FLIP | SPI_3WIRE_HIZ \
+				| SPI_MOSI_IDLE_LOW)
 
 struct spidev_data {
 	dev_t			devt;
-- 
2.25.1

[PATCH v5 4/4] spi: spidev_test Add three missing spi mode bits

[Boerge Struempfel]

Added the three missing spi mode bits SPI_3WIRE_HIZ, SPI_RX_CPHA_FLIP,
and SPI_MOSI_IDLE_LOW. Due to the length of the new options, the
indentation of the options in the help message was also adjusted for all
other options.

Signed-off-by: Boerge Struempfel <boerge.struempfel@gmail.com>
---
 tools/spi/spidev_test.c | 101 +++++++++++++++++++++++-----------------
 1 file changed, 58 insertions(+), 43 deletions(-)

diff --git a/tools/spi/spidev_test.c b/tools/spi/spidev_test.c
index b0ca44c70e83..66bfe90c541e 100644
--- a/tools/spi/spidev_test.c
+++ b/tools/spi/spidev_test.c
@@ -172,28 +172,31 @@
 
 static void print_usage(const char *prog)
 {
-	printf("Usage: %s [-DsbdlHOLC3vpNR24SI]\n", prog);
-	puts("  -D --device   device to use (default /dev/spidev1.1)\n"
-	     "  -s --speed    max speed (Hz)\n"
-	     "  -d --delay    delay (usec)\n"
-	     "  -b --bpw      bits per word\n"
-	     "  -i --input    input data from a file (e.g. \"test.bin\")\n"
-	     "  -o --output   output data to a file (e.g. \"results.bin\")\n"
-	     "  -l --loop     loopback\n"
-	     "  -H --cpha     clock phase\n"
-	     "  -O --cpol     clock polarity\n"
-	     "  -L --lsb      least significant bit first\n"
-	     "  -C --cs-high  chip select active high\n"
-	     "  -3 --3wire    SI/SO signals shared\n"
-	     "  -v --verbose  Verbose (show tx buffer)\n"
-	     "  -p            Send data (e.g. \"1234\\xde\\xad\")\n"
-	     "  -N --no-cs    no chip select\n"
-	     "  -R --ready    slave pulls low to pause\n"
-	     "  -2 --dual     dual transfer\n"
-	     "  -4 --quad     quad transfer\n"
-	     "  -8 --octal    octal transfer\n"
-	     "  -S --size     transfer size\n"
-	     "  -I --iter     iterations\n");
+	printf("Usage: %s [-DsbdlHOLC3ZFMvpNR24SI]\n", prog);
+	puts("  -D --device         device to use (default /dev/spidev1.1)\n"
+	     "  -s --speed          max speed (Hz)\n"
+	     "  -d --delay          delay (usec)\n"
+	     "  -b --bpw            bits per word\n"
+	     "  -i --input          input data from a file (e.g. \"test.bin\")\n"
+	     "  -o --output         output data to a file (e.g. \"results.bin\")\n"
+	     "  -l --loop           loopback\n"
+	     "  -H --cpha           clock phase\n"
+	     "  -O --cpol           clock polarity\n"
+	     "  -L --lsb            least significant bit first\n"
+	     "  -C --cs-high        chip select active high\n"
+	     "  -3 --3wire          SI/SO signals shared\n"
+		 "  -Z --3wire-hiz      high impedance turnaround\n"
+		 "  -F --rx-cpha-flip   flip CPHA on Rx only xfer\n"
+		 "  -M --mosi-idle-low  leave mosi line low when idle\n"
+	     "  -v --verbose        Verbose (show tx buffer)\n"
+	     "  -p                  Send data (e.g. \"1234\\xde\\xad\")\n"
+	     "  -N --no-cs          no chip select\n"
+	     "  -R --ready          slave pulls low to pause\n"
+	     "  -2 --dual           dual transfer\n"
+	     "  -4 --quad           quad transfer\n"
+	     "  -8 --octal          octal transfer\n"
+	     "  -S --size           transfer size\n"
+	     "  -I --iter           iterations\n");
 	exit(1);
 }
 
@@ -201,31 +204,34 @@ static void parse_opts(int argc, char *argv[])
 {
 	while (1) {
 		static const struct option lopts[] = {
-			{ "device",  1, 0, 'D' },
-			{ "speed",   1, 0, 's' },
-			{ "delay",   1, 0, 'd' },
-			{ "bpw",     1, 0, 'b' },
-			{ "input",   1, 0, 'i' },
-			{ "output",  1, 0, 'o' },
-			{ "loop",    0, 0, 'l' },
-			{ "cpha",    0, 0, 'H' },
-			{ "cpol",    0, 0, 'O' },
-			{ "lsb",     0, 0, 'L' },
-			{ "cs-high", 0, 0, 'C' },
-			{ "3wire",   0, 0, '3' },
-			{ "no-cs",   0, 0, 'N' },
-			{ "ready",   0, 0, 'R' },
-			{ "dual",    0, 0, '2' },
-			{ "verbose", 0, 0, 'v' },
-			{ "quad",    0, 0, '4' },
-			{ "octal",   0, 0, '8' },
-			{ "size",    1, 0, 'S' },
-			{ "iter",    1, 0, 'I' },
+			{ "device",        1, 0, 'D' },
+			{ "speed",         1, 0, 's' },
+			{ "delay",         1, 0, 'd' },
+			{ "bpw",           1, 0, 'b' },
+			{ "input",         1, 0, 'i' },
+			{ "output",        1, 0, 'o' },
+			{ "loop",          0, 0, 'l' },
+			{ "cpha",          0, 0, 'H' },
+			{ "cpol",          0, 0, 'O' },
+			{ "lsb",           0, 0, 'L' },
+			{ "cs-high",       0, 0, 'C' },
+			{ "3wire",         0, 0, '3' },
+			{ "3wire-hiz",     0, 0, 'Z' },
+			{ "rx-cpha-flip",  0, 0, 'F' },
+			{ "mosi-idle-low", 0, 0, 'M' },
+			{ "no-cs",         0, 0, 'N' },
+			{ "ready",         0, 0, 'R' },
+			{ "dual",          0, 0, '2' },
+			{ "verbose",       0, 0, 'v' },
+			{ "quad",          0, 0, '4' },
+			{ "octal",         0, 0, '8' },
+			{ "size",          1, 0, 'S' },
+			{ "iter",          1, 0, 'I' },
 			{ NULL, 0, 0, 0 },
 		};
 		int c;
 
-		c = getopt_long(argc, argv, "D:s:d:b:i:o:lHOLC3NR248p:vS:I:",
+		c = getopt_long(argc, argv, "D:s:d:b:i:o:lHOLC3ZFMNR248p:vS:I:",
 				lopts, NULL);
 
 		if (c == -1)
@@ -268,6 +274,15 @@ static void parse_opts(int argc, char *argv[])
 		case '3':
 			mode |= SPI_3WIRE;
 			break;
+		case 'Z':
+			mode |= SPI_3WIRE_HIZ;
+			break;
+		case 'F':
+			mode |= SPI_RX_CPHA_FLIP;
+			break;
+		case 'M':
+			mode |= SPI_MOSI_IDLE_LOW;
+			break;
 		case 'N':
 			mode |= SPI_NO_CS;
 			break;
-- 
2.25.1

Re: [PATCH v5 4/4] spi: spidev_test Add three missing spi mode bits

[Andy Shevchenko]

On Sat, May 20, 2023 at 10:09 PM Boerge Struempfel
<boerge.struempfel@gmail.com> wrote:
>
> Added the three missing spi mode bits SPI_3WIRE_HIZ, SPI_RX_CPHA_FLIP,
> and SPI_MOSI_IDLE_LOW. Due to the length of the new options, the
> indentation of the options in the help message was also adjusted for all
> other options.

Actually since you are touching all of them in the user-visible
output, you may also reshuffle them to be grouped logically. I'm not
sure if the switch-case ordering would be nice to have shuffled as
well. If so, in this case it might be better to have it as a
preparatory patch before you adding new options (and hence take care
of indentation in the first patch). That said, just think about it,
I'm not insisting.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 4/4] spi: spidev_test Add three missing spi mode bits

[Börge Strümpfel]

Am So., 21. Mai 2023 um 11:00 Uhr schrieb Andy Shevchenko
<andy.shevchenko@gmail.com>:
>
> On Sat, May 20, 2023 at 10:09 PM Boerge Struempfel
> <boerge.struempfel@gmail.com> wrote:
> >
> > Added the three missing spi mode bits SPI_3WIRE_HIZ, SPI_RX_CPHA_FLIP,
> > and SPI_MOSI_IDLE_LOW. Due to the length of the new options, the
> > indentation of the options in the help message was also adjusted for all
> > other options.
>
> Actually since you are touching all of them in the user-visible
> output, you may also reshuffle them to be grouped logically. I'm not
> sure if the switch-case ordering would be nice to have shuffled as
> well. If so, in this case it might be better to have it as a
> preparatory patch before you adding new options (and hence take care
> of indentation in the first patch). That said, just think about it,
> I'm not insisting.
>

Thanks for the suggestion. I tried coming up with a logical way of
ordering, but I am having some difficulties deciding. What do you
think of the following order?

general device settings
" -D --device device to use (default /dev/spidev1.1)\n"
" -s --speed max speed (Hz)\n"
" -d --delay delay (usec)\n"
" -l --loop loopback\n"

spi mode
" -H --cpha clock phase\n"
" -O --cpol clock polarity\n"
" -F --rx-cpha-flip flip CPHA on Rx only xfer\n"

number of wires for transmission
" -2 --dual dual transfer\n"
" -4 --quad quad transfer\n"
" -8 --octal octal transfer\n"
" -3 --3wire SI/SO signals shared\n"
" -Z --3wire-hiz high impedance turnaround\n"

additional parameters
" -b --bpw bits per word\n"
" -L --lsb least significant bit first\n"
" -C --cs-high chip select active high\n"
" -N --no-cs no chip select\n"
" -R --ready slave pulls low to pause\n"
" -M --mosi-idle-low leave mosi line low when idle\n"

data
" -i --input input data from a file (e.g. \"test.bin\")\n"
" -o --output output data to a file (e.g. \"results.bin\")\n"
" -p Send data (e.g. \"1234\\xde\\xad\")\n"
" -S --size transfer size\n"
" -I --iter iterations\n");

misc
" -v --verbose Verbose (show tx buffer)\n"

--
Kind regards,
Börge Strümpfel

Re: [PATCH v5 4/4] spi: spidev_test Add three missing spi mode bits

[Andy Shevchenko]

On Sun, May 21, 2023 at 2:35 PM Börge Strümpfel
<boerge.struempfel@gmail.com> wrote:
>
> Am So., 21. Mai 2023 um 11:00 Uhr schrieb Andy Shevchenko
> <andy.shevchenko@gmail.com>:
> >
> > On Sat, May 20, 2023 at 10:09 PM Boerge Struempfel
> > <boerge.struempfel@gmail.com> wrote:
> > >
> > > Added the three missing spi mode bits SPI_3WIRE_HIZ, SPI_RX_CPHA_FLIP,
> > > and SPI_MOSI_IDLE_LOW. Due to the length of the new options, the
> > > indentation of the options in the help message was also adjusted for all
> > > other options.
> >
> > Actually since you are touching all of them in the user-visible
> > output, you may also reshuffle them to be grouped logically. I'm not
> > sure if the switch-case ordering would be nice to have shuffled as
> > well. If so, in this case it might be better to have it as a
> > preparatory patch before you adding new options (and hence take care
> > of indentation in the first patch). That said, just think about it,
> > I'm not insisting.
> >
>
> Thanks for the suggestion. I tried coming up with a logical way of
> ordering, but I am having some difficulties deciding. What do you
> think of the following order?
>
> general device settings
> " -D --device device to use (default /dev/spidev1.1)\n"
> " -s --speed max speed (Hz)\n"
> " -d --delay delay (usec)\n"
> " -l --loop loopback\n"
>
> spi mode
> " -H --cpha clock phase\n"
> " -O --cpol clock polarity\n"
> " -F --rx-cpha-flip flip CPHA on Rx only xfer\n"
>
> number of wires for transmission
> " -2 --dual dual transfer\n"
> " -4 --quad quad transfer\n"
> " -8 --octal octal transfer\n"
> " -3 --3wire SI/SO signals shared\n"
> " -Z --3wire-hiz high impedance turnaround\n"
>
> additional parameters
> " -b --bpw bits per word\n"
> " -L --lsb least significant bit first\n"
> " -C --cs-high chip select active high\n"
> " -N --no-cs no chip select\n"
> " -R --ready slave pulls low to pause\n"
> " -M --mosi-idle-low leave mosi line low when idle\n"
>
> data
> " -i --input input data from a file (e.g. \"test.bin\")\n"
> " -o --output output data to a file (e.g. \"results.bin\")\n"
> " -p Send data (e.g. \"1234\\xde\\xad\")\n"
> " -S --size transfer size\n"
> " -I --iter iterations\n");
>
> misc
> " -v --verbose Verbose (show tx buffer)\n"

Looks great to me, thank you for doing that!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 4/4] spi: spidev_test Add three missing spi mode bits

[Börge Strümpfel]

Am So., 21. Mai 2023 um 21:26 Uhr schrieb Andy Shevchenko
<andy.shevchenko@gmail.com>:
>
> On Sun, May 21, 2023 at 2:35 PM Börge Strümpfel
> <boerge.struempfel@gmail.com> wrote:
> >
> > Am So., 21. Mai 2023 um 11:00 Uhr schrieb Andy Shevchenko
> > <andy.shevchenko@gmail.com>:
> > >
> > > On Sat, May 20, 2023 at 10:09 PM Boerge Struempfel
> > > <boerge.struempfel@gmail.com> wrote:
> > > >
> > > > Added the three missing spi mode bits SPI_3WIRE_HIZ, SPI_RX_CPHA_FLIP,
> > > > and SPI_MOSI_IDLE_LOW. Due to the length of the new options, the
> > > > indentation of the options in the help message was also adjusted for all
> > > > other options.
> > >
> > > Actually since you are touching all of them in the user-visible
> > > output, you may also reshuffle them to be grouped logically. I'm not
> > > sure if the switch-case ordering would be nice to have shuffled as
> > > well. If so, in this case it might be better to have it as a
> > > preparatory patch before you adding new options (and hence take care
> > > of indentation in the first patch). That said, just think about it,
> > > I'm not insisting.
> > >
> >
> > Thanks for the suggestion. I tried coming up with a logical way of
> > ordering, but I am having some difficulties deciding. What do you
> > think of the following order?
> >
> > general device settings
> > " -D --device device to use (default /dev/spidev1.1)\n"
> > " -s --speed max speed (Hz)\n"
> > " -d --delay delay (usec)\n"
> > " -l --loop loopback\n"
> >
> > spi mode
> > " -H --cpha clock phase\n"
> > " -O --cpol clock polarity\n"
> > " -F --rx-cpha-flip flip CPHA on Rx only xfer\n"
> >
> > number of wires for transmission
> > " -2 --dual dual transfer\n"
> > " -4 --quad quad transfer\n"
> > " -8 --octal octal transfer\n"
> > " -3 --3wire SI/SO signals shared\n"
> > " -Z --3wire-hiz high impedance turnaround\n"
> >
> > additional parameters
> > " -b --bpw bits per word\n"
> > " -L --lsb least significant bit first\n"
> > " -C --cs-high chip select active high\n"
> > " -N --no-cs no chip select\n"
> > " -R --ready slave pulls low to pause\n"
> > " -M --mosi-idle-low leave mosi line low when idle\n"
> >
> > data
> > " -i --input input data from a file (e.g. \"test.bin\")\n"
> > " -o --output output data to a file (e.g. \"results.bin\")\n"
> > " -p Send data (e.g. \"1234\\xde\\xad\")\n"
> > " -S --size transfer size\n"
> > " -I --iter iterations\n");
> >
> > misc
> > " -v --verbose Verbose (show tx buffer)\n"
>
> Looks great to me, thank you for doing that!
>
You are welcome.
Should I only reorder the flags, or actually introduce the
"group"-headers to visibly distinguish the options?
 --
With best regards,
Boerge Struempfel

Re: [PATCH v5 4/4] spi: spidev_test Add three missing spi mode bits

[Andy Shevchenko]

On Mon, May 22, 2023 at 10:23 AM Börge Strümpfel
<boerge.struempfel@gmail.com> wrote:
> Am So., 21. Mai 2023 um 21:26 Uhr schrieb Andy Shevchenko
> <andy.shevchenko@gmail.com>:
> > On Sun, May 21, 2023 at 2:35 PM Börge Strümpfel
> > <boerge.struempfel@gmail.com> wrote:
> > > Am So., 21. Mai 2023 um 11:00 Uhr schrieb Andy Shevchenko
> > > <andy.shevchenko@gmail.com>:

...

> > > Thanks for the suggestion. I tried coming up with a logical way of
> > > ordering, but I am having some difficulties deciding. What do you
> > > think of the following order?
> > >
> > > general device settings
> > > " -D --device device to use (default /dev/spidev1.1)\n"
> > > " -s --speed max speed (Hz)\n"
> > > " -d --delay delay (usec)\n"
> > > " -l --loop loopback\n"
> > >
> > > spi mode
> > > " -H --cpha clock phase\n"
> > > " -O --cpol clock polarity\n"
> > > " -F --rx-cpha-flip flip CPHA on Rx only xfer\n"
> > >
> > > number of wires for transmission
> > > " -2 --dual dual transfer\n"
> > > " -4 --quad quad transfer\n"
> > > " -8 --octal octal transfer\n"
> > > " -3 --3wire SI/SO signals shared\n"
> > > " -Z --3wire-hiz high impedance turnaround\n"
> > >
> > > additional parameters
> > > " -b --bpw bits per word\n"
> > > " -L --lsb least significant bit first\n"
> > > " -C --cs-high chip select active high\n"
> > > " -N --no-cs no chip select\n"
> > > " -R --ready slave pulls low to pause\n"
> > > " -M --mosi-idle-low leave mosi line low when idle\n"
> > >
> > > data
> > > " -i --input input data from a file (e.g. \"test.bin\")\n"
> > > " -o --output output data to a file (e.g. \"results.bin\")\n"
> > > " -p Send data (e.g. \"1234\\xde\\xad\")\n"
> > > " -S --size transfer size\n"
> > > " -I --iter iterations\n");
> > >
> > > misc
> > > " -v --verbose Verbose (show tx buffer)\n"
> >
> > Looks great to me, thank you for doing that!
> >
> You are welcome.
> Should I only reorder the flags, or actually introduce the
> "group"-headers to visibly distinguish the options?

Up to you. If you think it increases the usability I'm all for it.

-- 
With Best Regards,
Andy Shevchenko