linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 1/3] spi: uapi: unify SPI modes into a single spi.h header
@ 2020-12-21 14:19 Alexandru Ardelean
  2020-12-21 14:19 ` [PATCH v5 2/3] spi: Add SPI_NO_TX/RX support Alexandru Ardelean
  2020-12-21 14:19 ` [PATCH v5 3/3] spi: dt-bindings: document zero value for spi-{rx,tx}-bus-width properties Alexandru Ardelean
  0 siblings, 2 replies; 7+ messages in thread
From: Alexandru Ardelean @ 2020-12-21 14:19 UTC (permalink / raw)
  To: linux-spi, devicetree, linux-kernel
  Cc: dragos.bogdan, broonie, robh+dt, andy.shevchenko, 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.

This change also does a conversion of these bitfields to _BITUL() macro.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changelog v4 -> v5:
* https://lore.kernel.org/linux-spi/20201203140531.74470-1-alexandru.ardelean@analog.com/
* added 'Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>'

 include/linux/spi/spi.h         | 23 ++---------------------
 include/uapi/linux/spi/spi.h    | 31 +++++++++++++++++++++++++++++++
 include/uapi/linux/spi/spidev.h | 30 +-----------------------------
 3 files changed, 34 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..a08c3f37e202 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -15,6 +15,8 @@
 #include <linux/gpio/consumer.h>
 #include <linux/ptp_clock_kernel.h>
 
+#include <uapi/linux/spi/spi.h>
+
 struct dma_chan;
 struct property_entry;
 struct spi_controller;
@@ -165,27 +167,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..703b586f35df
--- /dev/null
+++ b/include/uapi/linux/spi/spi.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+#ifndef _UAPI_SPI_H
+#define _UAPI_SPI_H
+
+#include <linux/const.h>
+
+#define	SPI_CPHA		_BITUL(0)	/* clock phase */
+#define	SPI_CPOL		_BITUL(1)	/* 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		_BITUL(2)	/* chipselect active high? */
+#define	SPI_LSB_FIRST		_BITUL(3)	/* per-word bits-on-wire */
+#define	SPI_3WIRE		_BITUL(4)	/* SI/SO signals shared */
+#define	SPI_LOOP		_BITUL(5)	/* loopback mode */
+#define	SPI_NO_CS		_BITUL(6)	/* 1 dev/bus, no chipselect */
+#define	SPI_READY		_BITUL(7)	/* slave pulls low to pause */
+#define	SPI_TX_DUAL		_BITUL(8)	/* transmit with 2 wires */
+#define	SPI_TX_QUAD		_BITUL(9)	/* transmit with 4 wires */
+#define	SPI_RX_DUAL		_BITUL(10)	/* receive with 2 wires */
+#define	SPI_RX_QUAD		_BITUL(11)	/* receive with 4 wires */
+#define	SPI_CS_WORD		_BITUL(12)	/* toggle cs after each word */
+#define	SPI_TX_OCTAL		_BITUL(13)	/* transmit with 8 wires */
+#define	SPI_RX_OCTAL		_BITUL(14)	/* receive with 8 wires */
+#define	SPI_3WIRE_HIZ		_BITUL(15)	/* 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.17.1


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

* [PATCH v5 2/3] spi: Add SPI_NO_TX/RX support
  2020-12-21 14:19 [PATCH v5 1/3] spi: uapi: unify SPI modes into a single spi.h header Alexandru Ardelean
@ 2020-12-21 14:19 ` Alexandru Ardelean
  2020-12-21 14:34   ` Andy Shevchenko
  2020-12-21 14:19 ` [PATCH v5 3/3] spi: dt-bindings: document zero value for spi-{rx,tx}-bus-width properties Alexandru Ardelean
  1 sibling, 1 reply; 7+ messages in thread
From: Alexandru Ardelean @ 2020-12-21 14:19 UTC (permalink / raw)
  To: linux-spi, devicetree, linux-kernel
  Cc: dragos.bogdan, broonie, robh+dt, andy.shevchenko, 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>
---

Changelog v4 -> v5:
* https://lore.kernel.org/linux-spi/20201203140531.74470-2-alexandru.ardelean@analog.com/
* using static_assert() vs BUILD_BUG_ON()
* added comments for SPI_MODE_KERNEL_MASK & SPI_MODE_USER_MASK
* using shorter form defining SPI_MODE_KERNEL_MASK & SPI_MODE_USER_MASK

 drivers/spi/spi.c            | 26 +++++++++++++++++++++-----
 include/linux/spi/spi.h      | 17 +++++++++++++++++
 include/uapi/linux/spi/spi.h | 10 ++++++++++
 3 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 51d7c004fbab..0b69934698c3 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.
 	 */
@@ -3610,6 +3622,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)
@@ -3623,6 +3637,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/linux/spi/spi.h b/include/linux/spi/spi.h
index a08c3f37e202..9bfdfaf286eb 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -6,6 +6,7 @@
 #ifndef __LINUX_SPI_H
 #define __LINUX_SPI_H
 
+#include <linux/bits.h>
 #include <linux/device.h>
 #include <linux/mod_devicetable.h>
 #include <linux/slab.h>
@@ -166,6 +167,18 @@ struct spi_device {
 	u8			chip_select;
 	u8			bits_per_word;
 	bool			rt;
+#define SPI_NO_TX	BIT(31)		/* no transmit wire */
+#define SPI_NO_RX	BIT(30)		/* no receive wire */
+	/*
+	 * All bits defined above should be covered by SPI_MODE_KERNEL_MASK.
+	 * The SPI_MODE_KERNEL_MASK has the SPI_MODE_USER_MASK counterpart,
+	 * which is defined in 'include/uapi/linux/spi/spi.h'.
+	 * The bits defined here are from bit 31 downwards, while in
+	 * SPI_MODE_USER_MASK are from 0 upwards.
+	 * These bits must not overlap. A static assert check should make sure of that.
+	 * If adding extra bits, make sure to decrease the bit index below as well.
+	 */
+#define SPI_MODE_KERNEL_MASK	(~(BIT(30) - 1))
 	u32			mode;
 	int			irq;
 	void			*controller_state;
@@ -189,6 +202,10 @@ struct spi_device {
 	 */
 };
 
+/* Make sure that SPI_MODE_KERNEL_MASK & SPI_MODE_USER_MASK don't overlap */
+static_assert((SPI_MODE_KERNEL_MASK & SPI_MODE_USER_MASK) == 0,
+	      "SPI_MODE_USER_MASK & SPI_MODE_KERNEL_MASK must not overlap");
+
 static inline struct spi_device *to_spi_device(struct device *dev)
 {
 	return dev ? container_of(dev, struct spi_device, dev) : NULL;
diff --git a/include/uapi/linux/spi/spi.h b/include/uapi/linux/spi/spi.h
index 703b586f35df..236a85f08ded 100644
--- a/include/uapi/linux/spi/spi.h
+++ b/include/uapi/linux/spi/spi.h
@@ -28,4 +28,14 @@
 #define	SPI_RX_OCTAL		_BITUL(14)	/* receive with 8 wires */
 #define	SPI_3WIRE_HIZ		_BITUL(15)	/* high impedance turnaround */
 
+/*
+ * All the bits defined above should be covered by SPI_MODE_USER_MASK.
+ * The SPI_MODE_USER_MASK has the SPI_MODE_KERNEL_MASK counterpart in
+ * 'include/linux/spi/spi.h'. The bits defined here are from bit 0 upwards
+ * while in SPI_MODE_KERNEL_MASK they are from the other end downwards.
+ * 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(16) - 1)
+
 #endif /* _UAPI_SPI_H */
-- 
2.17.1


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

* [PATCH v5 3/3] spi: dt-bindings: document zero value for spi-{rx,tx}-bus-width properties
  2020-12-21 14:19 [PATCH v5 1/3] spi: uapi: unify SPI modes into a single spi.h header Alexandru Ardelean
  2020-12-21 14:19 ` [PATCH v5 2/3] spi: Add SPI_NO_TX/RX support Alexandru Ardelean
@ 2020-12-21 14:19 ` Alexandru Ardelean
  1 sibling, 0 replies; 7+ messages in thread
From: Alexandru Ardelean @ 2020-12-21 14:19 UTC (permalink / raw)
  To: linux-spi, devicetree, linux-kernel
  Cc: dragos.bogdan, broonie, robh+dt, andy.shevchenko, 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 that no RX or TX is allowed.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changelog v4 -> v5:
* https://lore.kernel.org/linux-spi/20201203140531.74470-3-alexandru.ardelean@analog.com/
* added 'Reviewed-by: Rob Herring <robh@kernel.org>'

 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 5f505810104d..06786f1b43d2 100644
--- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
@@ -152,8 +152,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 device.
         $ref: /schemas/types.yaml#/definitions/uint32
-        enum: [1, 2, 4, 8]
+        enum: [0, 1, 2, 4, 8]
         default: 1
 
       spi-rx-delay-us:
@@ -163,8 +164,9 @@ patternProperties:
       spi-tx-bus-width:
         description:
           Bus width to the SPI bus used for write transfers.
+          If 0 is provided, then no TX will be possible on this device.
         $ref: /schemas/types.yaml#/definitions/uint32
-        enum: [1, 2, 4, 8]
+        enum: [0, 1, 2, 4, 8]
         default: 1
 
       spi-tx-delay-us:
-- 
2.17.1


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

* Re: [PATCH v5 2/3] spi: Add SPI_NO_TX/RX support
  2020-12-21 14:19 ` [PATCH v5 2/3] spi: Add SPI_NO_TX/RX support Alexandru Ardelean
@ 2020-12-21 14:34   ` Andy Shevchenko
  2020-12-21 14:36     ` Andy Shevchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2020-12-21 14:34 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-spi, devicetree, Linux Kernel Mailing List, Bogdan, Dragos,
	Mark Brown, Rob Herring

On Mon, Dec 21, 2020 at 4:15 PM Alexandru Ardelean
<alexandru.ardelean@analog.com> wrote:
>
> 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>

Missed Co-developed-by: Alexandru ... ?

Anyway, looks good to me,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
>
> Changelog v4 -> v5:
> * https://lore.kernel.org/linux-spi/20201203140531.74470-2-alexandru.ardelean@analog.com/
> * using static_assert() vs BUILD_BUG_ON()
> * added comments for SPI_MODE_KERNEL_MASK & SPI_MODE_USER_MASK
> * using shorter form defining SPI_MODE_KERNEL_MASK & SPI_MODE_USER_MASK
>
>  drivers/spi/spi.c            | 26 +++++++++++++++++++++-----
>  include/linux/spi/spi.h      | 17 +++++++++++++++++
>  include/uapi/linux/spi/spi.h | 10 ++++++++++
>  3 files changed, 48 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 51d7c004fbab..0b69934698c3 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.
>          */
> @@ -3610,6 +3622,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)
> @@ -3623,6 +3637,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/linux/spi/spi.h b/include/linux/spi/spi.h
> index a08c3f37e202..9bfdfaf286eb 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -6,6 +6,7 @@
>  #ifndef __LINUX_SPI_H
>  #define __LINUX_SPI_H
>
> +#include <linux/bits.h>
>  #include <linux/device.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/slab.h>
> @@ -166,6 +167,18 @@ struct spi_device {
>         u8                      chip_select;
>         u8                      bits_per_word;
>         bool                    rt;
> +#define SPI_NO_TX      BIT(31)         /* no transmit wire */
> +#define SPI_NO_RX      BIT(30)         /* no receive wire */
> +       /*
> +        * All bits defined above should be covered by SPI_MODE_KERNEL_MASK.
> +        * The SPI_MODE_KERNEL_MASK has the SPI_MODE_USER_MASK counterpart,
> +        * which is defined in 'include/uapi/linux/spi/spi.h'.
> +        * The bits defined here are from bit 31 downwards, while in
> +        * SPI_MODE_USER_MASK are from 0 upwards.
> +        * These bits must not overlap. A static assert check should make sure of that.
> +        * If adding extra bits, make sure to decrease the bit index below as well.
> +        */
> +#define SPI_MODE_KERNEL_MASK   (~(BIT(30) - 1))
>         u32                     mode;
>         int                     irq;
>         void                    *controller_state;
> @@ -189,6 +202,10 @@ struct spi_device {
>          */
>  };
>
> +/* Make sure that SPI_MODE_KERNEL_MASK & SPI_MODE_USER_MASK don't overlap */
> +static_assert((SPI_MODE_KERNEL_MASK & SPI_MODE_USER_MASK) == 0,
> +             "SPI_MODE_USER_MASK & SPI_MODE_KERNEL_MASK must not overlap");
> +
>  static inline struct spi_device *to_spi_device(struct device *dev)
>  {
>         return dev ? container_of(dev, struct spi_device, dev) : NULL;
> diff --git a/include/uapi/linux/spi/spi.h b/include/uapi/linux/spi/spi.h
> index 703b586f35df..236a85f08ded 100644
> --- a/include/uapi/linux/spi/spi.h
> +++ b/include/uapi/linux/spi/spi.h
> @@ -28,4 +28,14 @@
>  #define        SPI_RX_OCTAL            _BITUL(14)      /* receive with 8 wires */
>  #define        SPI_3WIRE_HIZ           _BITUL(15)      /* high impedance turnaround */
>
> +/*
> + * All the bits defined above should be covered by SPI_MODE_USER_MASK.
> + * The SPI_MODE_USER_MASK has the SPI_MODE_KERNEL_MASK counterpart in
> + * 'include/linux/spi/spi.h'. The bits defined here are from bit 0 upwards
> + * while in SPI_MODE_KERNEL_MASK they are from the other end downwards.
> + * 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(16) - 1)
> +
>  #endif /* _UAPI_SPI_H */
> --
> 2.17.1
>


-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v5 2/3] spi: Add SPI_NO_TX/RX support
  2020-12-21 14:34   ` Andy Shevchenko
@ 2020-12-21 14:36     ` Andy Shevchenko
  2020-12-21 15:19       ` Ardelean, Alexandru
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2020-12-21 14:36 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-spi, devicetree, Linux Kernel Mailing List, Bogdan, Dragos,
	Mark Brown, Rob Herring

On Mon, Dec 21, 2020 at 4:34 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Dec 21, 2020 at 4:15 PM Alexandru Ardelean
> <alexandru.ardelean@analog.com> wrote:
> >
> > 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>
>
> Missed Co-developed-by: Alexandru ... ?
>
> Anyway, looks good to me,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

One nit, though...

> > -               "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");

Can we avoid splitting string literals which are assumed to be on one
line when printed?

-- 
With Best Regards,
Andy Shevchenko

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

* RE: [PATCH v5 2/3] spi: Add SPI_NO_TX/RX support
  2020-12-21 14:36     ` Andy Shevchenko
@ 2020-12-21 15:19       ` Ardelean, Alexandru
  2020-12-21 17:24         ` Andy Shevchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Ardelean, Alexandru @ 2020-12-21 15:19 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-spi, devicetree, Linux Kernel Mailing List, Bogdan, Dragos,
	Mark Brown, Rob Herring



> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> Sent: Monday, December 21, 2020 4:37 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>; Bogdan, Dragos <Dragos.Bogdan@analog.com>;
> Mark Brown <broonie@kernel.org>; Rob Herring <robh+dt@kernel.org>
> Subject: Re: [PATCH v5 2/3] spi: Add SPI_NO_TX/RX support
> 
> On Mon, Dec 21, 2020 at 4:34 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, Dec 21, 2020 at 4:15 PM Alexandru Ardelean
> > <alexandru.ardelean@analog.com> wrote:
> > >
> > > 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>
> >
> > Missed Co-developed-by: Alexandru ... ?

Nah, that's fine from my side without that tag.

> >
> > Anyway, looks good to me,
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> One nit, though...
> 
> > > -               "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");
> 
> Can we avoid splitting string literals which are assumed to be on one line when
> printed?

It ends up at about 96 cols, but it's within limits.
The patch may have been written before the new 100 col-width limit.

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

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

* Re: [PATCH v5 2/3] spi: Add SPI_NO_TX/RX support
  2020-12-21 15:19       ` Ardelean, Alexandru
@ 2020-12-21 17:24         ` Andy Shevchenko
  0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2020-12-21 17:24 UTC (permalink / raw)
  To: Ardelean, Alexandru
  Cc: linux-spi, devicetree, Linux Kernel Mailing List, Bogdan, Dragos,
	Mark Brown, Rob Herring

On Mon, Dec 21, 2020 at 5:19 PM Ardelean, Alexandru
<alexandru.Ardelean@analog.com> wrote:
> > -----Original Message-----
> > From: Andy Shevchenko <andy.shevchenko@gmail.com>
> > Sent: Monday, December 21, 2020 4:37 PM
> > On Mon, Dec 21, 2020 at 4:34 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:

...

> > One nit, though...
> >
> > > > -               "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");
> >
> > Can we avoid splitting string literals which are assumed to be on one line when
> > printed?
>
> It ends up at about 96 cols, but it's within limits.
> The patch may have been written before the new 100 col-width limit.

JFYI: string literals do not have limits (neither 80, nor 100).
It's a special category.

-- 
With Best Regards,
Andy Shevchenko

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

end of thread, other threads:[~2020-12-21 18:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-21 14:19 [PATCH v5 1/3] spi: uapi: unify SPI modes into a single spi.h header Alexandru Ardelean
2020-12-21 14:19 ` [PATCH v5 2/3] spi: Add SPI_NO_TX/RX support Alexandru Ardelean
2020-12-21 14:34   ` Andy Shevchenko
2020-12-21 14:36     ` Andy Shevchenko
2020-12-21 15:19       ` Ardelean, Alexandru
2020-12-21 17:24         ` Andy Shevchenko
2020-12-21 14:19 ` [PATCH v5 3/3] spi: dt-bindings: document zero value for spi-{rx,tx}-bus-width properties Alexandru Ardelean

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