linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/3] spi: uapi: unify SPI modes into a single spi.h header
@ 2020-11-27 13:08 Alexandru Ardelean
  2020-11-27 13:08 ` [PATCH v3 2/3] spi: Add SPI_NO_TX/RX support Alexandru Ardelean
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Alexandru Ardelean @ 2020-11-27 13:08 UTC (permalink / raw)
  To: linux-spi, devicetree, linux-kernel
  Cc: robh+dt, broonie, andy.shevchenko, dragos.bogdan, Alexandru Ardelean

This change moves all the SPI mode bits into a separate 'spi.h' header in
uapi. This is meant to re-use these definitions inside the kernel as well
as export them to userspace (via uapi).

The SPI mode definitions have usually been duplicated between between
'include/linux/spi/spi.h' and 'include/uapi/linux/spi/spidev.h', so
whenever adding a new entry, this would need to be put in both headers.

They've been moved from 'include/linux/spi/spi.h', since that seems a bit
more complete; the bits have descriptions and there is the SPI_MODE_X_MASK.

For now, this change does a simple move; no conversions to BIT() macros are
being done at this point. This can be done later, as that requires also
another header inclusion (the 'const.h' header).
The change as-is makes this 'spi.h' header more standalone.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Personally, I am not sure whether to convert the bitfield tos _BITUL()
macros or not. I feel that not-having these macros makes this uapi spi.h
header more standalone.
If there's a strong insistence to use those _BITUL() macros, I'll do it.
I'm hesitant now, because it requires that this spi.h includes the
'const.h' header.

Changelog v2 -> v3:
* https://lore.kernel.org/linux-spi/20201124102152.16548-1-alexandru.ardelean@analog.com/
* dropped 'spi: convert to BIT() all spi_device flags '
  added 'spi: uapi: unify SPI modes into a single spi.h header'

 include/linux/spi/spi.h         | 22 +--------------
 include/uapi/linux/spi/spi.h    | 47 +++++++++++++++++++++++++++++++++
 include/uapi/linux/spi/spidev.h | 30 +--------------------
 3 files changed, 49 insertions(+), 50 deletions(-)
 create mode 100644 include/uapi/linux/spi/spi.h

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index aa09fdc8042d..a4fedb33d34b 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -14,6 +14,7 @@
 #include <linux/scatterlist.h>
 #include <linux/gpio/consumer.h>
 #include <linux/ptp_clock_kernel.h>
+#include <uapi/linux/spi/spi.h>
 
 struct dma_chan;
 struct property_entry;
@@ -165,27 +166,6 @@ struct spi_device {
 	u8			bits_per_word;
 	bool			rt;
 	u32			mode;
-#define	SPI_CPHA	0x01			/* clock phase */
-#define	SPI_CPOL	0x02			/* clock polarity */
-#define	SPI_MODE_0	(0|0)			/* (original MicroWire) */
-#define	SPI_MODE_1	(0|SPI_CPHA)
-#define	SPI_MODE_2	(SPI_CPOL|0)
-#define	SPI_MODE_3	(SPI_CPOL|SPI_CPHA)
-#define	SPI_MODE_X_MASK	(SPI_CPOL|SPI_CPHA)
-#define	SPI_CS_HIGH	0x04			/* chipselect active high? */
-#define	SPI_LSB_FIRST	0x08			/* per-word bits-on-wire */
-#define	SPI_3WIRE	0x10			/* SI/SO signals shared */
-#define	SPI_LOOP	0x20			/* loopback mode */
-#define	SPI_NO_CS	0x40			/* 1 dev/bus, no chipselect */
-#define	SPI_READY	0x80			/* slave pulls low to pause */
-#define	SPI_TX_DUAL	0x100			/* transmit with 2 wires */
-#define	SPI_TX_QUAD	0x200			/* transmit with 4 wires */
-#define	SPI_RX_DUAL	0x400			/* receive with 2 wires */
-#define	SPI_RX_QUAD	0x800			/* receive with 4 wires */
-#define	SPI_CS_WORD	0x1000			/* toggle cs after each word */
-#define	SPI_TX_OCTAL	0x2000			/* transmit with 8 wires */
-#define	SPI_RX_OCTAL	0x4000			/* receive with 8 wires */
-#define	SPI_3WIRE_HIZ	0x8000			/* high impedance turnaround */
 	int			irq;
 	void			*controller_state;
 	void			*controller_data;
diff --git a/include/uapi/linux/spi/spi.h b/include/uapi/linux/spi/spi.h
new file mode 100644
index 000000000000..ae028d64c779
--- /dev/null
+++ b/include/uapi/linux/spi/spi.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/*
+ * include/linux/spi/spi.h
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+  */
+
+#ifndef _UAPI_SPI_H
+#define _UAPI_SPI_H
+
+#define	SPI_CPHA		0x01		/* clock phase */
+#define	SPI_CPOL		0x02		/* clock polarity */
+
+#define	SPI_MODE_0		(0|0)		/* (original MicroWire) */
+#define	SPI_MODE_1		(0|SPI_CPHA)
+#define	SPI_MODE_2		(SPI_CPOL|0)
+#define	SPI_MODE_3		(SPI_CPOL|SPI_CPHA)
+#define	SPI_MODE_X_MASK		(SPI_CPOL|SPI_CPHA)
+
+#define	SPI_CS_HIGH		0x04		/* chipselect active high? */
+#define	SPI_LSB_FIRST		0x08		/* per-word bits-on-wire */
+#define	SPI_3WIRE		0x10		/* SI/SO signals shared */
+#define	SPI_LOOP		0x20		/* loopback mode */
+#define	SPI_NO_CS		0x40		/* 1 dev/bus, no chipselect */
+#define	SPI_READY		0x80		/* slave pulls low to pause */
+#define	SPI_TX_DUAL		0x100		/* transmit with 2 wires */
+#define	SPI_TX_QUAD		0x200		/* transmit with 4 wires */
+#define	SPI_RX_DUAL		0x400		/* receive with 2 wires */
+#define	SPI_RX_QUAD		0x800		/* receive with 4 wires */
+#define	SPI_CS_WORD		0x1000		/* toggle cs after each word */
+#define	SPI_TX_OCTAL		0x2000		/* transmit with 8 wires */
+#define	SPI_RX_OCTAL		0x4000		/* receive with 8 wires */
+#define	SPI_3WIRE_HIZ		0x8000		/* high impedance turnaround */
+
+#endif /* _UAPI_SPI_H */
diff --git a/include/uapi/linux/spi/spidev.h b/include/uapi/linux/spi/spidev.h
index d56427c0b3e0..0c3da08f2aff 100644
--- a/include/uapi/linux/spi/spidev.h
+++ b/include/uapi/linux/spi/spidev.h
@@ -25,35 +25,7 @@
 
 #include <linux/types.h>
 #include <linux/ioctl.h>
-
-/* User space versions of kernel symbols for SPI clocking modes,
- * matching <linux/spi/spi.h>
- */
-
-#define SPI_CPHA		0x01
-#define SPI_CPOL		0x02
-
-#define SPI_MODE_0		(0|0)
-#define SPI_MODE_1		(0|SPI_CPHA)
-#define SPI_MODE_2		(SPI_CPOL|0)
-#define SPI_MODE_3		(SPI_CPOL|SPI_CPHA)
-
-#define SPI_CS_HIGH		0x04
-#define SPI_LSB_FIRST		0x08
-#define SPI_3WIRE		0x10
-#define SPI_LOOP		0x20
-#define SPI_NO_CS		0x40
-#define SPI_READY		0x80
-#define SPI_TX_DUAL		0x100
-#define SPI_TX_QUAD		0x200
-#define SPI_RX_DUAL		0x400
-#define SPI_RX_QUAD		0x800
-#define SPI_CS_WORD		0x1000
-#define SPI_TX_OCTAL		0x2000
-#define SPI_RX_OCTAL		0x4000
-#define SPI_3WIRE_HIZ		0x8000
-
-/*---------------------------------------------------------------------------*/
+#include <linux/spi/spi.h>
 
 /* IOCTL commands */
 
-- 
2.27.0


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

* [PATCH v3 2/3] spi: Add SPI_NO_TX/RX support
  2020-11-27 13:08 [PATCH v3 1/3] spi: uapi: unify SPI modes into a single spi.h header Alexandru Ardelean
@ 2020-11-27 13:08 ` Alexandru Ardelean
  2020-11-27 14:22   ` Andy Shevchenko
  2020-11-27 13:08 ` [PATCH v3 3/3] spi: dt-bindings: document zero value for spi-{rx,tx}-bus-width properties Alexandru Ardelean
  2020-11-27 14:12 ` [PATCH v3 1/3] spi: uapi: unify SPI modes into a single spi.h header Andy Shevchenko
  2 siblings, 1 reply; 11+ messages in thread
From: Alexandru Ardelean @ 2020-11-27 13:08 UTC (permalink / raw)
  To: linux-spi, devicetree, linux-kernel
  Cc: robh+dt, broonie, andy.shevchenko, dragos.bogdan, Alexandru Ardelean

From: Dragos Bogdan <dragos.bogdan@analog.com>

Transmit/receive only is a valid SPI mode. For example, the MOSI/TX line
might be missing from an ADC while for a DAC the MISO/RX line may be
optional. This patch adds these two new modes: SPI_NO_TX and
SPI_NO_RX. This way, the drivers will be able to identify if any of
these two lines is missing and to adjust the transfers accordingly.

Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/spi/spi.c            | 26 +++++++++++++++++++++-----
 include/uapi/linux/spi/spi.h |  2 ++
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index cd3c395b4e90..17d4004bd2c6 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1941,6 +1941,9 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
 	/* Device DUAL/QUAD mode */
 	if (!of_property_read_u32(nc, "spi-tx-bus-width", &value)) {
 		switch (value) {
+		case 0:
+			spi->mode |= SPI_NO_TX;
+			break;
 		case 1:
 			break;
 		case 2:
@@ -1962,6 +1965,9 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
 
 	if (!of_property_read_u32(nc, "spi-rx-bus-width", &value)) {
 		switch (value) {
+		case 0:
+			spi->mode |= SPI_NO_RX;
+			break;
 		case 1:
 			break;
 		case 2:
@@ -3329,12 +3335,17 @@ int spi_setup(struct spi_device *spi)
 	unsigned	bad_bits, ugly_bits;
 	int		status;
 
-	/* check mode to prevent that DUAL and QUAD set at the same time
+	/*
+	 * check mode to prevent that any two of DUAL, QUAD and NO_MOSI/MISO
+	 * are set at the same time
 	 */
-	if (((spi->mode & SPI_TX_DUAL) && (spi->mode & SPI_TX_QUAD)) ||
-		((spi->mode & SPI_RX_DUAL) && (spi->mode & SPI_RX_QUAD))) {
+	if ((hweight_long(spi->mode &
+		(SPI_TX_DUAL | SPI_TX_QUAD | SPI_NO_TX)) > 1) ||
+	    (hweight_long(spi->mode &
+		(SPI_RX_DUAL | SPI_RX_QUAD | SPI_NO_RX)) > 1)) {
 		dev_err(&spi->dev,
-		"setup: can not select dual and quad at the same time\n");
+		"setup: can not select any two of dual, quad and no-rx/tx "
+		"at the same time\n");
 		return -EINVAL;
 	}
 	/* if it is SPI_3WIRE mode, DUAL and QUAD should be forbidden
@@ -3348,7 +3359,8 @@ int spi_setup(struct spi_device *spi)
 	 * SPI_CS_WORD has a fallback software implementation,
 	 * so it is ignored here.
 	 */
-	bad_bits = spi->mode & ~(spi->controller->mode_bits | SPI_CS_WORD);
+	bad_bits = spi->mode & ~(spi->controller->mode_bits | SPI_CS_WORD |
+				 SPI_NO_TX | SPI_NO_RX);
 	/* nothing prevents from working with active-high CS in case if it
 	 * is driven by GPIO.
 	 */
@@ -3609,6 +3621,8 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
 		 * 2. check tx/rx_nbits match the mode in spi_device
 		 */
 		if (xfer->tx_buf) {
+			if (spi->mode & SPI_NO_TX)
+				return -EINVAL;
 			if (xfer->tx_nbits != SPI_NBITS_SINGLE &&
 				xfer->tx_nbits != SPI_NBITS_DUAL &&
 				xfer->tx_nbits != SPI_NBITS_QUAD)
@@ -3622,6 +3636,8 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
 		}
 		/* check transfer rx_nbits */
 		if (xfer->rx_buf) {
+			if (spi->mode & SPI_NO_RX)
+				return -EINVAL;
 			if (xfer->rx_nbits != SPI_NBITS_SINGLE &&
 				xfer->rx_nbits != SPI_NBITS_DUAL &&
 				xfer->rx_nbits != SPI_NBITS_QUAD)
diff --git a/include/uapi/linux/spi/spi.h b/include/uapi/linux/spi/spi.h
index ae028d64c779..b504e46a6f18 100644
--- a/include/uapi/linux/spi/spi.h
+++ b/include/uapi/linux/spi/spi.h
@@ -43,5 +43,7 @@
 #define	SPI_TX_OCTAL		0x2000		/* transmit with 8 wires */
 #define	SPI_RX_OCTAL		0x4000		/* receive with 8 wires */
 #define	SPI_3WIRE_HIZ		0x8000		/* high impedance turnaround */
+#define	SPI_NO_TX		0x10000		/* no transmit wire */
+#define	SPI_NO_RX		0x20000		/* no receive wire */
 
 #endif /* _UAPI_SPI_H */
-- 
2.27.0


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

* [PATCH v3 3/3] spi: dt-bindings: document zero value for spi-{rx,tx}-bus-width properties
  2020-11-27 13:08 [PATCH v3 1/3] spi: uapi: unify SPI modes into a single spi.h header Alexandru Ardelean
  2020-11-27 13:08 ` [PATCH v3 2/3] spi: Add SPI_NO_TX/RX support Alexandru Ardelean
@ 2020-11-27 13:08 ` Alexandru Ardelean
  2020-11-27 14:26   ` Andy Shevchenko
  2020-11-27 14:12 ` [PATCH v3 1/3] spi: uapi: unify SPI modes into a single spi.h header Andy Shevchenko
  2 siblings, 1 reply; 11+ messages in thread
From: Alexandru Ardelean @ 2020-11-27 13:08 UTC (permalink / raw)
  To: linux-spi, devicetree, linux-kernel
  Cc: robh+dt, broonie, andy.shevchenko, dragos.bogdan, Alexandru Ardelean

Following a change to the SPI framework, providing a value of zero for
'spi-rx-bus-width' and 'spi-tx-bus-width' is now possible and will
essentially mean than no RX or TX is allowed.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 Documentation/devicetree/bindings/spi/spi-controller.yaml | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml
index 1b56d5e40f1f..f1aaaf9b3709 100644
--- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
@@ -125,8 +125,9 @@ patternProperties:
       spi-rx-bus-width:
         description:
           Bus width to the SPI bus used for read transfers.
+          If 0 is provided, then no RX will be possible on this devices.
         $ref: /schemas/types.yaml#/definitions/uint32
-        enum: [1, 2, 4, 8]
+        enum: [0, 1, 2, 4, 8]
         default: 1
 
       spi-rx-delay-us:
@@ -136,8 +137,9 @@ patternProperties:
       spi-tx-bus-width:
         description:
           Bus width to the SPI bus used for write transfers.
+          If 0 is provided, then no RX will be possible on this devices.
         $ref: /schemas/types.yaml#/definitions/uint32
-        enum: [1, 2, 4, 8]
+        enum: [0, 1, 2, 4, 8]
         default: 1
 
       spi-tx-delay-us:
-- 
2.27.0


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

* Re: [PATCH v3 1/3] spi: uapi: unify SPI modes into a single spi.h header
  2020-11-27 13:08 [PATCH v3 1/3] spi: uapi: unify SPI modes into a single spi.h header Alexandru Ardelean
  2020-11-27 13:08 ` [PATCH v3 2/3] spi: Add SPI_NO_TX/RX support Alexandru Ardelean
  2020-11-27 13:08 ` [PATCH v3 3/3] spi: dt-bindings: document zero value for spi-{rx,tx}-bus-width properties Alexandru Ardelean
@ 2020-11-27 14:12 ` Andy Shevchenko
  2020-12-03  8:15   ` Ardelean, Alexandru
  2 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2020-11-27 14:12 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-spi, devicetree, Linux Kernel Mailing List, Rob Herring,
	Mark Brown, Bogdan, Dragos

On Fri, Nov 27, 2020 at 3:08 PM Alexandru Ardelean
<alexandru.ardelean@analog.com> wrote:
>
> This change moves all the SPI mode bits into a separate 'spi.h' header in
> uapi. This is meant to re-use these definitions inside the kernel as well
> as export them to userspace (via uapi).

uapi -> UAPI (or uAPI) here and everywhere else where it makes sense.

> The SPI mode definitions have usually been duplicated between between
> 'include/linux/spi/spi.h' and 'include/uapi/linux/spi/spidev.h', so
> whenever adding a new entry, this would need to be put in both headers.
>
> They've been moved from 'include/linux/spi/spi.h', since that seems a bit
> more complete; the bits have descriptions and there is the SPI_MODE_X_MASK.
>
> For now, this change does a simple move; no conversions to BIT() macros are
> being done at this point. This can be done later, as that requires also
> another header inclusion (the 'const.h' header).
> The change as-is makes this 'spi.h' header more standalone.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
>
> Personally, I am not sure whether to convert the bitfield tos _BITUL()
> macros or not. I feel that not-having these macros makes this uapi spi.h
> header more standalone.
> If there's a strong insistence to use those _BITUL() macros, I'll do it.
> I'm hesitant now, because it requires that this spi.h includes the
> 'const.h' header.

_BITUL is a part of uAPI, why not to use it?
In general BIT() type of macros makes values easier to read and less
error prone (in big numbers it's easy to miss 0).
It's not a strong opinion, it's just the rationale behind how I see it.

> Changelog v2 -> v3:
> * https://lore.kernel.org/linux-spi/20201124102152.16548-1-alexandru.ardelean@analog.com/
> * dropped 'spi: convert to BIT() all spi_device flags '
>   added 'spi: uapi: unify SPI modes into a single spi.h header'
>
>  include/linux/spi/spi.h         | 22 +--------------
>  include/uapi/linux/spi/spi.h    | 47 +++++++++++++++++++++++++++++++++
>  include/uapi/linux/spi/spidev.h | 30 +--------------------
>  3 files changed, 49 insertions(+), 50 deletions(-)
>  create mode 100644 include/uapi/linux/spi/spi.h
>
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index aa09fdc8042d..a4fedb33d34b 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -14,6 +14,7 @@
>  #include <linux/scatterlist.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/ptp_clock_kernel.h>
> +#include <uapi/linux/spi/spi.h>
>
>  struct dma_chan;
>  struct property_entry;
> @@ -165,27 +166,6 @@ struct spi_device {
>         u8                      bits_per_word;
>         bool                    rt;
>         u32                     mode;
> -#define        SPI_CPHA        0x01                    /* clock phase */
> -#define        SPI_CPOL        0x02                    /* clock polarity */
> -#define        SPI_MODE_0      (0|0)                   /* (original MicroWire) */
> -#define        SPI_MODE_1      (0|SPI_CPHA)
> -#define        SPI_MODE_2      (SPI_CPOL|0)
> -#define        SPI_MODE_3      (SPI_CPOL|SPI_CPHA)
> -#define        SPI_MODE_X_MASK (SPI_CPOL|SPI_CPHA)
> -#define        SPI_CS_HIGH     0x04                    /* chipselect active high? */
> -#define        SPI_LSB_FIRST   0x08                    /* per-word bits-on-wire */
> -#define        SPI_3WIRE       0x10                    /* SI/SO signals shared */
> -#define        SPI_LOOP        0x20                    /* loopback mode */
> -#define        SPI_NO_CS       0x40                    /* 1 dev/bus, no chipselect */
> -#define        SPI_READY       0x80                    /* slave pulls low to pause */
> -#define        SPI_TX_DUAL     0x100                   /* transmit with 2 wires */
> -#define        SPI_TX_QUAD     0x200                   /* transmit with 4 wires */
> -#define        SPI_RX_DUAL     0x400                   /* receive with 2 wires */
> -#define        SPI_RX_QUAD     0x800                   /* receive with 4 wires */
> -#define        SPI_CS_WORD     0x1000                  /* toggle cs after each word */
> -#define        SPI_TX_OCTAL    0x2000                  /* transmit with 8 wires */
> -#define        SPI_RX_OCTAL    0x4000                  /* receive with 8 wires */
> -#define        SPI_3WIRE_HIZ   0x8000                  /* high impedance turnaround */
>         int                     irq;
>         void                    *controller_state;
>         void                    *controller_data;
> diff --git a/include/uapi/linux/spi/spi.h b/include/uapi/linux/spi/spi.h
> new file mode 100644
> index 000000000000..ae028d64c779
> --- /dev/null
> +++ b/include/uapi/linux/spi/spi.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> +/*
> + * include/linux/spi/spi.h
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> +  */

Do we still need this boilerplate license header?

> +
> +#ifndef _UAPI_SPI_H
> +#define _UAPI_SPI_H
> +
> +#define        SPI_CPHA                0x01            /* clock phase */
> +#define        SPI_CPOL                0x02            /* clock polarity */
> +
> +#define        SPI_MODE_0              (0|0)           /* (original MicroWire) */
> +#define        SPI_MODE_1              (0|SPI_CPHA)
> +#define        SPI_MODE_2              (SPI_CPOL|0)
> +#define        SPI_MODE_3              (SPI_CPOL|SPI_CPHA)
> +#define        SPI_MODE_X_MASK         (SPI_CPOL|SPI_CPHA)
> +
> +#define        SPI_CS_HIGH             0x04            /* chipselect active high? */
> +#define        SPI_LSB_FIRST           0x08            /* per-word bits-on-wire */
> +#define        SPI_3WIRE               0x10            /* SI/SO signals shared */
> +#define        SPI_LOOP                0x20            /* loopback mode */
> +#define        SPI_NO_CS               0x40            /* 1 dev/bus, no chipselect */
> +#define        SPI_READY               0x80            /* slave pulls low to pause */
> +#define        SPI_TX_DUAL             0x100           /* transmit with 2 wires */
> +#define        SPI_TX_QUAD             0x200           /* transmit with 4 wires */
> +#define        SPI_RX_DUAL             0x400           /* receive with 2 wires */
> +#define        SPI_RX_QUAD             0x800           /* receive with 4 wires */
> +#define        SPI_CS_WORD             0x1000          /* toggle cs after each word */
> +#define        SPI_TX_OCTAL            0x2000          /* transmit with 8 wires */
> +#define        SPI_RX_OCTAL            0x4000          /* receive with 8 wires */
> +#define        SPI_3WIRE_HIZ           0x8000          /* high impedance turnaround */
> +
> +#endif /* _UAPI_SPI_H */
> diff --git a/include/uapi/linux/spi/spidev.h b/include/uapi/linux/spi/spidev.h
> index d56427c0b3e0..0c3da08f2aff 100644
> --- a/include/uapi/linux/spi/spidev.h
> +++ b/include/uapi/linux/spi/spidev.h
> @@ -25,35 +25,7 @@
>
>  #include <linux/types.h>
>  #include <linux/ioctl.h>
> -
> -/* User space versions of kernel symbols for SPI clocking modes,
> - * matching <linux/spi/spi.h>
> - */
> -
> -#define SPI_CPHA               0x01
> -#define SPI_CPOL               0x02
> -
> -#define SPI_MODE_0             (0|0)
> -#define SPI_MODE_1             (0|SPI_CPHA)
> -#define SPI_MODE_2             (SPI_CPOL|0)
> -#define SPI_MODE_3             (SPI_CPOL|SPI_CPHA)
> -
> -#define SPI_CS_HIGH            0x04
> -#define SPI_LSB_FIRST          0x08
> -#define SPI_3WIRE              0x10
> -#define SPI_LOOP               0x20
> -#define SPI_NO_CS              0x40
> -#define SPI_READY              0x80
> -#define SPI_TX_DUAL            0x100
> -#define SPI_TX_QUAD            0x200
> -#define SPI_RX_DUAL            0x400
> -#define SPI_RX_QUAD            0x800
> -#define SPI_CS_WORD            0x1000
> -#define SPI_TX_OCTAL           0x2000
> -#define SPI_RX_OCTAL           0x4000
> -#define SPI_3WIRE_HIZ          0x8000
> -
> -/*---------------------------------------------------------------------------*/
> +#include <linux/spi/spi.h>
>
>  /* IOCTL commands */
>
> --
> 2.27.0
>


-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v3 2/3] spi: Add SPI_NO_TX/RX support
  2020-11-27 13:08 ` [PATCH v3 2/3] spi: Add SPI_NO_TX/RX support Alexandru Ardelean
@ 2020-11-27 14:22   ` Andy Shevchenko
  2020-11-27 14:23     ` Andy Shevchenko
  0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2020-11-27 14:22 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-spi, devicetree, Linux Kernel Mailing List, Rob Herring,
	Mark Brown, Bogdan, Dragos

On Fri, Nov 27, 2020 at 3:08 PM Alexandru Ardelean
<alexandru.ardelean@analog.com> wrote:
> Transmit/receive only is a valid SPI mode. For example, the MOSI/TX line
> might be missing from an ADC while for a DAC the MISO/RX line may be
> optional. This patch adds these two new modes: SPI_NO_TX and
> SPI_NO_RX. This way, the drivers will be able to identify if any of
> these two lines is missing and to adjust the transfers accordingly.

...

> +       /*
> +        * check mode to prevent that any two of DUAL, QUAD and NO_MOSI/MISO
> +        * are set at the same time
>          */

Since you are here,
 check -> Check
 time -> time.

...

> +       if ((hweight_long(spi->mode &
> +               (SPI_TX_DUAL | SPI_TX_QUAD | SPI_NO_TX)) > 1) ||
> +           (hweight_long(spi->mode &
> +               (SPI_RX_DUAL | SPI_RX_QUAD | SPI_NO_RX)) > 1)) {
>                 dev_err(&spi->dev,
> -               "setup: can not select dual and quad at the same time\n");
> +               "setup: can not select any two of dual, quad and no-rx/tx "
> +               "at the same time\n");

Don't split literals, and probably rephrase (If I can't set 2, can I set 3?)
   "setup: can't select more than one out of dual, quad, and no-Rx /
no-Tx at the same time\n");

> --- a/include/uapi/linux/spi/spi.h
> +++ b/include/uapi/linux/spi/spi.h
> @@ -43,5 +43,7 @@
>  #define        SPI_TX_OCTAL            0x2000          /* transmit with 8 wires */
>  #define        SPI_RX_OCTAL            0x4000          /* receive with 8 wires */
>  #define        SPI_3WIRE_HIZ           0x8000          /* high impedance turnaround */
> +#define        SPI_NO_TX               0x10000         /* no transmit wire */
> +#define        SPI_NO_RX               0x20000         /* no receive wire */

Is it really material for uAPI?
Perhaps we may have something like
SPI_MODE_USER_MASK in uAPI and
in internal headers

SPI_MODE_KERNEL_MASK with
static_assert(_USER_MASK & _KERNEL_MASK); // check conditional

?

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v3 2/3] spi: Add SPI_NO_TX/RX support
  2020-11-27 14:22   ` Andy Shevchenko
@ 2020-11-27 14:23     ` Andy Shevchenko
  2020-12-03  8:20       ` Ardelean, Alexandru
  0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2020-11-27 14:23 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-spi, devicetree, Linux Kernel Mailing List, Rob Herring,
	Mark Brown, Bogdan, Dragos

On Fri, Nov 27, 2020 at 4:22 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Nov 27, 2020 at 3:08 PM Alexandru Ardelean
> <alexandru.ardelean@analog.com> wrote:

...

> > --- a/include/uapi/linux/spi/spi.h
> > +++ b/include/uapi/linux/spi/spi.h
> > @@ -43,5 +43,7 @@
> >  #define        SPI_TX_OCTAL            0x2000          /* transmit with 8 wires */
> >  #define        SPI_RX_OCTAL            0x4000          /* receive with 8 wires */
> >  #define        SPI_3WIRE_HIZ           0x8000          /* high impedance turnaround */
> > +#define        SPI_NO_TX               0x10000         /* no transmit wire */
> > +#define        SPI_NO_RX               0x20000         /* no receive wire */
>
> Is it really material for uAPI?
> Perhaps we may have something like
> SPI_MODE_USER_MASK in uAPI and
> in internal headers
>
> SPI_MODE_KERNEL_MASK with
> static_assert(_USER_MASK & _KERNEL_MASK); // check conditional
>
> ?

And logically start bits for the kernel from the end (31, 30, ...).

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v3 3/3] spi: dt-bindings: document zero value for spi-{rx,tx}-bus-width properties
  2020-11-27 13:08 ` [PATCH v3 3/3] spi: dt-bindings: document zero value for spi-{rx,tx}-bus-width properties Alexandru Ardelean
@ 2020-11-27 14:26   ` Andy Shevchenko
  2020-12-03 13:35     ` Ardelean, Alexandru
  0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2020-11-27 14:26 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-spi, devicetree, Linux Kernel Mailing List, Rob Herring,
	Mark Brown, Bogdan, Dragos

On Fri, Nov 27, 2020 at 3:08 PM Alexandru Ardelean
<alexandru.ardelean@analog.com> wrote:
>
> Following a change to the SPI framework, providing a value of zero for
> 'spi-rx-bus-width' and 'spi-tx-bus-width' is now possible and will
> essentially mean than no RX or TX is allowed.

than -> that

I'm wondering if we have a possibility to strict this for controllers
that always duplex (I assume that schema works in a way that the
generic bindings is most relaxed in comparison to the device specific
ones in case of the same binding in question).

-- 
With Best Regards,
Andy Shevchenko

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

* RE: [PATCH v3 1/3] spi: uapi: unify SPI modes into a single spi.h header
  2020-11-27 14:12 ` [PATCH v3 1/3] spi: uapi: unify SPI modes into a single spi.h header Andy Shevchenko
@ 2020-12-03  8:15   ` Ardelean, Alexandru
  0 siblings, 0 replies; 11+ messages in thread
From: Ardelean, Alexandru @ 2020-12-03  8:15 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-spi, devicetree, Linux Kernel Mailing List, Rob Herring,
	Mark Brown, Bogdan, Dragos



> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> Sent: Friday, November 27, 2020 4:13 PM
> To: Ardelean, Alexandru <alexandru.Ardelean@analog.com>
> Cc: linux-spi <linux-spi@vger.kernel.org>; devicetree
> <devicetree@vger.kernel.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; Rob Herring <robh+dt@kernel.org>; Mark Brown
> <broonie@kernel.org>; Bogdan, Dragos <Dragos.Bogdan@analog.com>
> Subject: Re: [PATCH v3 1/3] spi: uapi: unify SPI modes into a single spi.h header
> 
> On Fri, Nov 27, 2020 at 3:08 PM Alexandru Ardelean
> <alexandru.ardelean@analog.com> wrote:
> >
> > This change moves all the SPI mode bits into a separate 'spi.h' header
> > in uapi. This is meant to re-use these definitions inside the kernel
> > as well as export them to userspace (via uapi).
> 
> uapi -> UAPI (or uAPI) here and everywhere else where it makes sense.
> 
> > The SPI mode definitions have usually been duplicated between between
> > 'include/linux/spi/spi.h' and 'include/uapi/linux/spi/spidev.h', so
> > whenever adding a new entry, this would need to be put in both headers.
> >
> > They've been moved from 'include/linux/spi/spi.h', since that seems a
> > bit more complete; the bits have descriptions and there is the
> SPI_MODE_X_MASK.
> >
> > For now, this change does a simple move; no conversions to BIT()
> > macros are being done at this point. This can be done later, as that
> > requires also another header inclusion (the 'const.h' header).
> > The change as-is makes this 'spi.h' header more standalone.
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> >
> > Personally, I am not sure whether to convert the bitfield tos _BITUL()
> > macros or not. I feel that not-having these macros makes this uapi
> > spi.h header more standalone.
> > If there's a strong insistence to use those _BITUL() macros, I'll do it.
> > I'm hesitant now, because it requires that this spi.h includes the
> > 'const.h' header.
> 
> _BITUL is a part of uAPI, why not to use it?
> In general BIT() type of macros makes values easier to read and less error prone
> (in big numbers it's easy to miss 0).
> It's not a strong opinion, it's just the rationale behind how I see it.

Yeah I understand.
I guess since this is a new header in UAPI, I can use the _BITUL macro.
Sometimes I look at some code and remember how some people tend to use some things.
Like, I would expect some people to maybe just copy this header as-is in some "libspi".
So, then: do we make it easier for those people or not?
But again, since this is a new header, the concern may be reduced by just using _BITUL() from the start.
I sometimes have these arguments with myself whenever going into API code.
Kernel code is easy: all the users of that code in the kernel, and we try not to bother too much with out-of-tree drivers. Those get impossible to care about.

> 
> > Changelog v2 -> v3:
> > *
> > https://urldefense.com/v3/__https://lore.kernel.org/linux-spi/20201124
> > 102152.16548-1-
> alexandru.ardelean@analog.com/__;!!A3Ni8CS0y2Y!oNpULNID
> > JsvU1BX8CZgZYoS90mW3rZ2dHrZOwTRS4ndZ-_rLq3eJt22Rj1RQafWyw1-3YA$
> > * dropped 'spi: convert to BIT() all spi_device flags '
> >   added 'spi: uapi: unify SPI modes into a single spi.h header'
> >
> >  include/linux/spi/spi.h         | 22 +--------------
> >  include/uapi/linux/spi/spi.h    | 47 +++++++++++++++++++++++++++++++++
> >  include/uapi/linux/spi/spidev.h | 30 +--------------------
> >  3 files changed, 49 insertions(+), 50 deletions(-)  create mode
> > 100644 include/uapi/linux/spi/spi.h
> >
> > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index
> > aa09fdc8042d..a4fedb33d34b 100644
> > --- a/include/linux/spi/spi.h
> > +++ b/include/linux/spi/spi.h
> > @@ -14,6 +14,7 @@
> >  #include <linux/scatterlist.h>
> >  #include <linux/gpio/consumer.h>
> >  #include <linux/ptp_clock_kernel.h>
> > +#include <uapi/linux/spi/spi.h>
> >
> >  struct dma_chan;
> >  struct property_entry;
> > @@ -165,27 +166,6 @@ struct spi_device {
> >         u8                      bits_per_word;
> >         bool                    rt;
> >         u32                     mode;
> > -#define        SPI_CPHA        0x01                    /* clock phase */
> > -#define        SPI_CPOL        0x02                    /* clock polarity */
> > -#define        SPI_MODE_0      (0|0)                   /* (original MicroWire) */
> > -#define        SPI_MODE_1      (0|SPI_CPHA)
> > -#define        SPI_MODE_2      (SPI_CPOL|0)
> > -#define        SPI_MODE_3      (SPI_CPOL|SPI_CPHA)
> > -#define        SPI_MODE_X_MASK (SPI_CPOL|SPI_CPHA)
> > -#define        SPI_CS_HIGH     0x04                    /* chipselect active high? */
> > -#define        SPI_LSB_FIRST   0x08                    /* per-word bits-on-wire */
> > -#define        SPI_3WIRE       0x10                    /* SI/SO signals shared */
> > -#define        SPI_LOOP        0x20                    /* loopback mode */
> > -#define        SPI_NO_CS       0x40                    /* 1 dev/bus, no chipselect */
> > -#define        SPI_READY       0x80                    /* slave pulls low to pause */
> > -#define        SPI_TX_DUAL     0x100                   /* transmit with 2 wires */
> > -#define        SPI_TX_QUAD     0x200                   /* transmit with 4 wires */
> > -#define        SPI_RX_DUAL     0x400                   /* receive with 2 wires */
> > -#define        SPI_RX_QUAD     0x800                   /* receive with 4 wires */
> > -#define        SPI_CS_WORD     0x1000                  /* toggle cs after each word */
> > -#define        SPI_TX_OCTAL    0x2000                  /* transmit with 8 wires */
> > -#define        SPI_RX_OCTAL    0x4000                  /* receive with 8 wires */
> > -#define        SPI_3WIRE_HIZ   0x8000                  /* high impedance turnaround
> */
> >         int                     irq;
> >         void                    *controller_state;
> >         void                    *controller_data;
> > diff --git a/include/uapi/linux/spi/spi.h
> > b/include/uapi/linux/spi/spi.h new file mode 100644 index
> > 000000000000..ae028d64c779
> > --- /dev/null
> > +++ b/include/uapi/linux/spi/spi.h
> > @@ -0,0 +1,47 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> > +/*
> > + * include/linux/spi/spi.h
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify
> > + * it under the terms of the GNU General Public License as published
> > +by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> > +  */
> 
> Do we still need this boilerplate license header?

I'll remove this.
I'm still a n00b with all this licensing stuff.

> 
> > +
> > +#ifndef _UAPI_SPI_H
> > +#define _UAPI_SPI_H
> > +
> > +#define        SPI_CPHA                0x01            /* clock phase */
> > +#define        SPI_CPOL                0x02            /* clock polarity */
> > +
> > +#define        SPI_MODE_0              (0|0)           /* (original MicroWire) */
> > +#define        SPI_MODE_1              (0|SPI_CPHA)
> > +#define        SPI_MODE_2              (SPI_CPOL|0)
> > +#define        SPI_MODE_3              (SPI_CPOL|SPI_CPHA)
> > +#define        SPI_MODE_X_MASK         (SPI_CPOL|SPI_CPHA)
> > +
> > +#define        SPI_CS_HIGH             0x04            /* chipselect active high? */
> > +#define        SPI_LSB_FIRST           0x08            /* per-word bits-on-wire */
> > +#define        SPI_3WIRE               0x10            /* SI/SO signals shared */
> > +#define        SPI_LOOP                0x20            /* loopback mode */
> > +#define        SPI_NO_CS               0x40            /* 1 dev/bus, no chipselect */
> > +#define        SPI_READY               0x80            /* slave pulls low to pause */
> > +#define        SPI_TX_DUAL             0x100           /* transmit with 2 wires */
> > +#define        SPI_TX_QUAD             0x200           /* transmit with 4 wires */
> > +#define        SPI_RX_DUAL             0x400           /* receive with 2 wires */
> > +#define        SPI_RX_QUAD             0x800           /* receive with 4 wires */
> > +#define        SPI_CS_WORD             0x1000          /* toggle cs after each word */
> > +#define        SPI_TX_OCTAL            0x2000          /* transmit with 8 wires */
> > +#define        SPI_RX_OCTAL            0x4000          /* receive with 8 wires */
> > +#define        SPI_3WIRE_HIZ           0x8000          /* high impedance turnaround
> */
> > +
> > +#endif /* _UAPI_SPI_H */
> > diff --git a/include/uapi/linux/spi/spidev.h
> > b/include/uapi/linux/spi/spidev.h index d56427c0b3e0..0c3da08f2aff
> > 100644
> > --- a/include/uapi/linux/spi/spidev.h
> > +++ b/include/uapi/linux/spi/spidev.h
> > @@ -25,35 +25,7 @@
> >
> >  #include <linux/types.h>
> >  #include <linux/ioctl.h>
> > -
> > -/* User space versions of kernel symbols for SPI clocking modes,
> > - * matching <linux/spi/spi.h>
> > - */
> > -
> > -#define SPI_CPHA               0x01
> > -#define SPI_CPOL               0x02
> > -
> > -#define SPI_MODE_0             (0|0)
> > -#define SPI_MODE_1             (0|SPI_CPHA)
> > -#define SPI_MODE_2             (SPI_CPOL|0)
> > -#define SPI_MODE_3             (SPI_CPOL|SPI_CPHA)
> > -
> > -#define SPI_CS_HIGH            0x04
> > -#define SPI_LSB_FIRST          0x08
> > -#define SPI_3WIRE              0x10
> > -#define SPI_LOOP               0x20
> > -#define SPI_NO_CS              0x40
> > -#define SPI_READY              0x80
> > -#define SPI_TX_DUAL            0x100
> > -#define SPI_TX_QUAD            0x200
> > -#define SPI_RX_DUAL            0x400
> > -#define SPI_RX_QUAD            0x800
> > -#define SPI_CS_WORD            0x1000
> > -#define SPI_TX_OCTAL           0x2000
> > -#define SPI_RX_OCTAL           0x4000
> > -#define SPI_3WIRE_HIZ          0x8000
> > -
> > -/*-------------------------------------------------------------------
> > --------*/
> > +#include <linux/spi/spi.h>
> >
> >  /* IOCTL commands */
> >
> > --
> > 2.27.0
> >
> 
> 
> --
> With Best Regards,
> Andy Shevchenko

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

* RE: [PATCH v3 2/3] spi: Add SPI_NO_TX/RX support
  2020-11-27 14:23     ` Andy Shevchenko
@ 2020-12-03  8:20       ` Ardelean, Alexandru
  2020-12-03  9:47         ` Mark Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Ardelean, Alexandru @ 2020-12-03  8:20 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-spi, devicetree, Linux Kernel Mailing List, Rob Herring,
	Mark Brown, Bogdan, Dragos



> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> Sent: Friday, November 27, 2020 4:24 PM
> To: Ardelean, Alexandru <alexandru.Ardelean@analog.com>
> Cc: linux-spi <linux-spi@vger.kernel.org>; devicetree
> <devicetree@vger.kernel.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; Rob Herring <robh+dt@kernel.org>; Mark Brown
> <broonie@kernel.org>; Bogdan, Dragos <Dragos.Bogdan@analog.com>
> Subject: Re: [PATCH v3 2/3] spi: Add SPI_NO_TX/RX support
> 
> On Fri, Nov 27, 2020 at 4:22 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Fri, Nov 27, 2020 at 3:08 PM Alexandru Ardelean
> > <alexandru.ardelean@analog.com> wrote:
> 
> ...
> 
> > > --- a/include/uapi/linux/spi/spi.h
> > > +++ b/include/uapi/linux/spi/spi.h
> > > @@ -43,5 +43,7 @@
> > >  #define        SPI_TX_OCTAL            0x2000          /* transmit with 8 wires */
> > >  #define        SPI_RX_OCTAL            0x4000          /* receive with 8 wires */
> > >  #define        SPI_3WIRE_HIZ           0x8000          /* high impedance turnaround
> */
> > > +#define        SPI_NO_TX               0x10000         /* no transmit wire */
> > > +#define        SPI_NO_RX               0x20000         /* no receive wire */
> >
> > Is it really material for uAPI?
> > Perhaps we may have something like
> > SPI_MODE_USER_MASK in uAPI and
> > in internal headers

Hmm, in a way this could make sense for some SPIDEVs as well, to set these flags and get an error if they try to TX with the NO_TX flag set.
Not really sure about this.
Initially I mechanically added these here as an inertia to the previous patch which is just unifying the headers.

Any other opinions? Thoughts?
Mark?

> >
> > SPI_MODE_KERNEL_MASK with
> > static_assert(_USER_MASK & _KERNEL_MASK); // check conditional
> >
> > ?
> 
> And logically start bits for the kernel from the end (31, 30, ...).
> 
> --
> With Best Regards,
> Andy Shevchenko

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

* Re: [PATCH v3 2/3] spi: Add SPI_NO_TX/RX support
  2020-12-03  8:20       ` Ardelean, Alexandru
@ 2020-12-03  9:47         ` Mark Brown
  0 siblings, 0 replies; 11+ messages in thread
From: Mark Brown @ 2020-12-03  9:47 UTC (permalink / raw)
  To: Ardelean, Alexandru
  Cc: Andy Shevchenko, linux-spi, devicetree,
	Linux Kernel Mailing List, Rob Herring, Bogdan, Dragos

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

On Thu, Dec 03, 2020 at 08:20:57AM +0000, Ardelean, Alexandru wrote:

> > > > @@ -43,5 +43,7 @@
> > > >  #define        SPI_TX_OCTAL            0x2000          /* transmit with 8 wires */
> > > >  #define        SPI_RX_OCTAL            0x4000          /* receive with 8 wires */
> > > >  #define        SPI_3WIRE_HIZ           0x8000          /* high impedance turnaround
> > */
> > > > +#define        SPI_NO_TX               0x10000         /* no transmit wire */
> > > > +#define        SPI_NO_RX               0x20000         /* no receive wire */

> > > Is it really material for uAPI?
> > > Perhaps we may have something like
> > > SPI_MODE_USER_MASK in uAPI and
> > > in internal headers

> Hmm, in a way this could make sense for some SPIDEVs as well, to set these flags and get an error if they try to TX with the NO_TX flag set.
> Not really sure about this.
> Initially I mechanically added these here as an inertia to the previous patch which is just unifying the headers.

> Any other opinions? Thoughts?
> Mark?

spidev is hacky at the best of times...  It *is* probably better to only
have the usefully mainpulable modes exposed in uapi and then define the
rest internally though.

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

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

* RE: [PATCH v3 3/3] spi: dt-bindings: document zero value for spi-{rx,tx}-bus-width properties
  2020-11-27 14:26   ` Andy Shevchenko
@ 2020-12-03 13:35     ` Ardelean, Alexandru
  0 siblings, 0 replies; 11+ messages in thread
From: Ardelean, Alexandru @ 2020-12-03 13:35 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-spi, devicetree, Linux Kernel Mailing List, Rob Herring,
	Mark Brown, Bogdan, Dragos



> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> Sent: Friday, November 27, 2020 4:26 PM
> To: Ardelean, Alexandru <alexandru.Ardelean@analog.com>
> Cc: linux-spi <linux-spi@vger.kernel.org>; devicetree
> <devicetree@vger.kernel.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; Rob Herring <robh+dt@kernel.org>; Mark Brown
> <broonie@kernel.org>; Bogdan, Dragos <Dragos.Bogdan@analog.com>
> Subject: Re: [PATCH v3 3/3] spi: dt-bindings: document zero value for spi-{rx,tx}-
> bus-width properties
> 
> On Fri, Nov 27, 2020 at 3:08 PM Alexandru Ardelean
> <alexandru.ardelean@analog.com> wrote:
> >
> > Following a change to the SPI framework, providing a value of zero for
> > 'spi-rx-bus-width' and 'spi-tx-bus-width' is now possible and will
> > essentially mean than no RX or TX is allowed.
> 
> than -> that
> 
> I'm wondering if we have a possibility to strict this for controllers that always
> duplex (I assume that schema works in a way that the generic bindings is most
> relaxed in comparison to the device specific ones in case of the same binding in
> question).

No idea here.

> 
> --
> With Best Regards,
> Andy Shevchenko

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

end of thread, other threads:[~2020-12-03 13:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-27 13:08 [PATCH v3 1/3] spi: uapi: unify SPI modes into a single spi.h header Alexandru Ardelean
2020-11-27 13:08 ` [PATCH v3 2/3] spi: Add SPI_NO_TX/RX support Alexandru Ardelean
2020-11-27 14:22   ` Andy Shevchenko
2020-11-27 14:23     ` Andy Shevchenko
2020-12-03  8:20       ` Ardelean, Alexandru
2020-12-03  9:47         ` Mark Brown
2020-11-27 13:08 ` [PATCH v3 3/3] spi: dt-bindings: document zero value for spi-{rx,tx}-bus-width properties Alexandru Ardelean
2020-11-27 14:26   ` Andy Shevchenko
2020-12-03 13:35     ` Ardelean, Alexandru
2020-11-27 14:12 ` [PATCH v3 1/3] spi: uapi: unify SPI modes into a single spi.h header Andy Shevchenko
2020-12-03  8:15   ` Ardelean, Alexandru

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).