[PATCH V2] spi: dual and quad support(add single macro)

[PATCH V2] spi: dual and quad support(add single macro)

[wangyuhang]

fix two things in patch:
commit id:f477b7fb13df2b843997559ff34e87d054ba6538

1.Add SPI_TX_SINGLE and SPI_RX_SINGLE to specify SINGLE mode.
 Instead of using default value in mode.
2.Delete a "return" when commit the patch to a new kernel version
 by mistake. So recover it.

Signed-off-by: wangyuhang <wangyuhang2014@gmail.com>
---
 drivers/spi/spi.c       |    5 +++++
 include/linux/spi/spi.h |    4 ++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 8d191f2..81bdc96 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -873,9 +873,11 @@ static void of_register_spi_devices(struct spi_master *master)
 		prop = of_get_property(nc, "spi-tmax-nbits", &len);
 		if (!prop || len < sizeof(*prop)) {
 			/* set tx mode in SINGLE as default */
+			spi->mode &= SPI_TX_SINGLE;
 		} else {
 			switch (be32_to_cpup(prop)) {
 			case SPI_NBITS_SINGLE:
+				spi->mode &= SPI_TX_SINGLE;
 				break;
 			case SPI_NBITS_DUAL:
 				spi->mode |= SPI_TX_DUAL;
@@ -893,9 +895,11 @@ static void of_register_spi_devices(struct spi_master *master)
 		prop = of_get_property(nc, "spi-rmax-nbits", &len);
 		if (!prop || len < sizeof(*prop)) {
 			/* set rx mode in SINGLE as default */
+			spi->mode &= SPI_RX_SINGLE;
 		} else {
 			switch (be32_to_cpup(prop)) {
 			case SPI_NBITS_SINGLE:
+				spi->mode &= SPI_RX_SINGLE;
 				break;
 			case SPI_NBITS_DUAL:
 				spi->mode |= SPI_RX_DUAL;
@@ -1459,6 +1463,7 @@ static int __spi_async(struct spi_device *spi, struct spi_message *message)
 			return -EINVAL;
 		if (xfer->speed_hz && master->max_speed_hz &&
 		    xfer->speed_hz > master->max_speed_hz)
+			return -EINVAL;
 
 		if (xfer->tx_buf && !xfer->tx_nbits)
 			xfer->tx_nbits = SPI_NBITS_SINGLE;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index ccd7840..faa138c 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -89,8 +89,12 @@ struct spi_device {
 #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 */
+/* transmit with 1 wire(not using dual-tx and quad-tx) */
+#define	SPI_TX_SINGLE	~(SPI_TX_DUAL | SPI_TX_QUAD)
 #define	SPI_RX_DUAL	0x400			/* receive with 2 wires */
 #define	SPI_RX_QUAD	0x800			/* receive with 4 wires */
+/* receive with 1 wire(not using dual-rx and quad-rx) */
+#define	SPI_RX_SINGLE	~(SPI_RX_DUAL | SPI_RX_QUAD)
 	u8			bits_per_word;
 	int			irq;
 	void			*controller_state;
-- 
1.7.9.5

Re: [PATCH V2] spi: dual and quad support(add single macro)

[Mark Brown]

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

On Thu, Aug 29, 2013 at 09:01:50AM +0800, wangyuhang wrote:

> fix two things in patch:
> commit id:f477b7fb13df2b843997559ff34e87d054ba6538

> 1.Add SPI_TX_SINGLE and SPI_RX_SINGLE to specify SINGLE mode.
>  Instead of using default value in mode.
> 2.Delete a "return" when commit the patch to a new kernel version
>  by mistake. So recover it.

These two changes aren't related to each other so should be sent as two
separate patches.

> @@ -89,8 +89,12 @@ struct spi_device {
>  #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 */
> +/* transmit with 1 wire(not using dual-tx and quad-tx) */
> +#define	SPI_TX_SINGLE	~(SPI_TX_DUAL | SPI_TX_QUAD)
>  #define	SPI_RX_DUAL	0x400			/* receive with 2 wires */
>  #define	SPI_RX_QUAD	0x800			/* receive with 4 wires */
> +/* receive with 1 wire(not using dual-rx and quad-rx) */
> +#define	SPI_RX_SINGLE	~(SPI_RX_DUAL | SPI_RX_QUAD)

These don't look right, they're going to set all bits which is going to
interfere with all the other flags.  Why do we need this define at all?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V2] spi: dual and quad support(add single macro)

[yuhang wang]

Hi. Mark

2013/8/29 Mark Brown <broonie@kernel.org>:
> On Thu, Aug 29, 2013 at 09:01:50AM +0800, wangyuhang wrote:
>
>> fix two things in patch:
>> commit id:f477b7fb13df2b843997559ff34e87d054ba6538
>
>> 1.Add SPI_TX_SINGLE and SPI_RX_SINGLE to specify SINGLE mode.
>>  Instead of using default value in mode.
>> 2.Delete a "return" when commit the patch to a new kernel version
>>  by mistake. So recover it.
>
> These two changes aren't related to each other so should be sent as two
> separate patches.
>
Got it. Thanks.

>> @@ -89,8 +89,12 @@ struct spi_device {
>>  #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 */
>> +/* transmit with 1 wire(not using dual-tx and quad-tx) */
>> +#define      SPI_TX_SINGLE   ~(SPI_TX_DUAL | SPI_TX_QUAD)
>>  #define      SPI_RX_DUAL     0x400                   /* receive with 2 wires */
>>  #define      SPI_RX_QUAD     0x800                   /* receive with 4 wires */
>> +/* receive with 1 wire(not using dual-rx and quad-rx) */
>> +#define      SPI_RX_SINGLE   ~(SPI_RX_DUAL | SPI_RX_QUAD)
>
> These don't look right, they're going to set all bits which is going to
> interfere with all the other flags.  Why do we need this define at all?

Well, it is not necessary. Pekon thought to set SINGLE just using the
default value in mode seems not reliable. But the flag is not
consistent with others really seems a little dangerous. So drop it.

Best regards.