All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] spi: spidev_test: Warn when the mode is not the requested mode
@ 2022-06-13  5:39 David Fries
  2022-06-13 12:08 ` Mark Brown
  2022-06-13 17:24 ` Mark Brown
  0 siblings, 2 replies; 4+ messages in thread
From: David Fries @ 2022-06-13  5:39 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel

Print a warning if the device mode doesn't match the requested mode.
The user doesn't enter the mode in hex so it isn't obvious when
setting the mode succeeds that the mode isn't the requested mode.  The
kernel logs a message, it will be more visible if the test also prints
a warning.  I was testing --quad, which is unsupported, but doesn't
cause the mode request to fail.

Signed-off-by: David Fries <David@Fries.net>
---
This is the kernel log, I just didn't see it at first.
spidev spi1.0: setup: ignoring unsupported mode bits 200

I was testing --quad on a Raspberry Pi where I didn't expect it to
work, which the ioctl didn't return an error, it just cleared the bits
it didn't support.

 tools/spi/spidev_test.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/spi/spidev_test.c b/tools/spi/spidev_test.c
index 83844f8b862a..5629387bfc56 100644
--- a/tools/spi/spidev_test.c
+++ b/tools/spi/spidev_test.c
@@ -417,6 +417,7 @@ int main(int argc, char *argv[])
 {
 	int ret = 0;
 	int fd;
+	uint32_t request;
 
 	parse_opts(argc, argv);
 
@@ -430,13 +431,23 @@ int main(int argc, char *argv[])
 	/*
 	 * spi mode
 	 */
+	/* WR is make a request to assign 'mode' */
+	request = mode;
 	ret = ioctl(fd, SPI_IOC_WR_MODE32, &mode);
 	if (ret == -1)
 		pabort("can't set spi mode");
 
+	/* RD is read what mode the device actually is in */
 	ret = ioctl(fd, SPI_IOC_RD_MODE32, &mode);
 	if (ret == -1)
 		pabort("can't get spi mode");
+	/* Drivers can reject some mode bits without returning an error.
+	 * Read the current value to identify what mode it is in, and if it
+	 * differs from the requested mode, warn the user.
+	 */
+	if(request != mode)
+		printf("WARNING device does not support requested mode 0x%x\n",
+			request);
 
 	/*
 	 * bits per word
-- 
2.30.2

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] spi: spidev_test: Warn when the mode is not the requested mode
  2022-06-13  5:39 [PATCH] spi: spidev_test: Warn when the mode is not the requested mode David Fries
@ 2022-06-13 12:08 ` Mark Brown
  2022-06-13 17:24 ` Mark Brown
  1 sibling, 0 replies; 4+ messages in thread
From: Mark Brown @ 2022-06-13 12:08 UTC (permalink / raw)
  To: David Fries; +Cc: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 374 bytes --]

On Mon, Jun 13, 2022 at 12:39:41AM -0500, David Fries wrote:

> +	/* Drivers can reject some mode bits without returning an error.
> +	 * Read the current value to identify what mode it is in, and if it
> +	 * differs from the requested mode, warn the user.
> +	 */
> +	if(request != mode)

Please try to follow the kernel coding style - both the if( and the
comment don't.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] spi: spidev_test: Warn when the mode is not the requested mode
  2022-06-13  5:39 [PATCH] spi: spidev_test: Warn when the mode is not the requested mode David Fries
  2022-06-13 12:08 ` Mark Brown
@ 2022-06-13 17:24 ` Mark Brown
  1 sibling, 0 replies; 4+ messages in thread
From: Mark Brown @ 2022-06-13 17:24 UTC (permalink / raw)
  To: David; +Cc: linux-kernel

On Mon, 13 Jun 2022 00:39:41 -0500, David Fries wrote:
> Print a warning if the device mode doesn't match the requested mode.
> The user doesn't enter the mode in hex so it isn't obvious when
> setting the mode succeeds that the mode isn't the requested mode.  The
> kernel logs a message, it will be more visible if the test also prints
> a warning.  I was testing --quad, which is unsupported, but doesn't
> cause the mode request to fail.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: spidev_test: Warn when the mode is not the requested mode
      commit: 41ecad2c3cce807abf32bff879cef5dfcacae363

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] spi: spidev_test: Warn when the mode is not the requested mode
@ 2022-06-13 14:35 David Fries
  0 siblings, 0 replies; 4+ messages in thread
From: David Fries @ 2022-06-13 14:35 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-kernel

Print a warning if the device mode doesn't match the requested mode.
The user doesn't enter the mode in hex so it isn't obvious when
setting the mode succeeds that the mode isn't the requested mode.  The
kernel logs a message, it will be more visible if the test also prints
a warning.  I was testing --quad, which is unsupported, but doesn't
cause the mode request to fail.

Signed-off-by: David Fries <David@Fries.net>
---
Sorry forgot to run checkpatch.pl though it was silent about the
comments, which I reworded.

 tools/spi/spidev_test.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/spi/spidev_test.c b/tools/spi/spidev_test.c
index 83844f8b862a..941d713d6f23 100644
--- a/tools/spi/spidev_test.c
+++ b/tools/spi/spidev_test.c
@@ -417,6 +417,7 @@ int main(int argc, char *argv[])
 {
 	int ret = 0;
 	int fd;
+	uint32_t request;
 
 	parse_opts(argc, argv);
 
@@ -429,7 +430,10 @@ int main(int argc, char *argv[])
 
 	/*
 	 * spi mode
+	 * WR is write 'mode' to request the transfer mode
+	 * RD is read what mode the device actually is
 	 */
+	request = mode;
 	ret = ioctl(fd, SPI_IOC_WR_MODE32, &mode);
 	if (ret == -1)
 		pabort("can't set spi mode");
@@ -437,6 +441,14 @@ int main(int argc, char *argv[])
 	ret = ioctl(fd, SPI_IOC_RD_MODE32, &mode);
 	if (ret == -1)
 		pabort("can't get spi mode");
+	/*
+	 * Drivers can reject some mode options without returning an error.
+	 * Warn in this case to avoid the user thinking the requested mode is
+	 * supported since it didn't give an error.
+	 */
+	if (request != mode)
+		printf("WARNING device does not support requested mode 0x%x\n",
+			request);
 
 	/*
 	 * bits per word
-- 
2.30.2

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-06-13 19:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-13  5:39 [PATCH] spi: spidev_test: Warn when the mode is not the requested mode David Fries
2022-06-13 12:08 ` Mark Brown
2022-06-13 17:24 ` Mark Brown
2022-06-13 14:35 David Fries

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.