Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1582667 > unrolled thread

[PATCH 4/5] tpm_tis_spi: Remove limitation of transfers to MAX_SPI_FRAMESIZE bytes

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 4/5] tpm_tis_spi: Remove limitation of transfers to MAX_SPI_FRAMESIZE bytes Peter Huewe <peter.huewe@infineon.com> - 2017-02-16 17:10 +0100
    Re: [PATCH 4/5] tpm_tis_spi: Remove limitation of transfers to  MAX_SPI_FRAMESIZE bytes kbuild test robot <lkp@intel.com> - 2017-02-16 18:30 +0100
    Re: [PATCH 4/5] tpm_tis_spi: Remove limitation of transfers to  MAX_SPI_FRAMESIZE bytes Christophe Ricard <christophe.ricard@gmail.com> - 2017-02-17 06:20 +0100
      Re: [PATCH 4/5] tpm_tis_spi: Remove limitation of transfers to MAX_SPI_FRAMESIZE bytes Peter Huewe <peterhuewe@gmx.de> - 2017-02-17 08:30 +0100
    Re: [PATCH 4/5] tpm_tis_spi: Remove limitation of transfers to  MAX_SPI_FRAMESIZE bytes Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-02-24 13:00 +0100
    Re: [PATCH 4/5] tpm_tis_spi: Remove limitation of transfers to  MAX_SPI_FRAMESIZE bytes Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-02-24 14:10 +0100

#1582667 — [PATCH 4/5] tpm_tis_spi: Remove limitation of transfers to MAX_SPI_FRAMESIZE bytes

Limiting transfers to MAX_SPI_FRAMESIZE was not expected by the upper
layers, as tpm_tis has no such limitation. Add a loop to hide that
limitation.

Cc: <stable@vger.kernel.org>
Fixes: 0edbfea537d1 ("tpm/tpm_tis_spi: Add support for spi phy")
Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
Signed-off-by: Peter Huewe <peter.huewe@infineon.com>
---
 drivers/char/tpm/tpm_tis_spi.c | 108 ++++++++++++++++++++++-------------------
 1 file changed, 57 insertions(+), 51 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_spi.c b/drivers/char/tpm/tpm_tis_spi.c
index 16938e2253d2..b50c5b072df3 100644
--- a/drivers/char/tpm/tpm_tis_spi.c
+++ b/drivers/char/tpm/tpm_tis_spi.c
@@ -61,68 +61,74 @@ static int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u8 len,
 {
 	struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
 	int ret;
-	struct spi_message m;
-	struct spi_transfer spi_xfer = {
-		.tx_buf = phy->tx_buf,
-		.rx_buf = phy->rx_buf,
-		.len = 4,
-		.cs_change = 1,
-	};
-
-	if (len > MAX_SPI_FRAMESIZE)
-		return -ENOMEM;
 
-	phy->tx_buf[0] = direction | (len - 1);
-	phy->tx_buf[1] = 0xd4;
-	phy->tx_buf[2] = addr >> 8;
-	phy->tx_buf[3] = addr;
+	spi_bus_lock(phy->spi_device->master);
 
-	spi_message_init(&m);
-	spi_message_add_tail(&spi_xfer, &m);
+	while (len) {
+		struct spi_message m;
+		struct spi_transfer spi_xfer = {
+			.tx_buf = phy->tx_buf,
+			.rx_buf = phy->rx_buf,
+			.len = 4,
+			.cs_change = 1,
+		};
+		u8 transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);
+
+		phy->tx_buf[0] = direction | (transfer_len - 1);
+		phy->tx_buf[1] = 0xd4;
+		phy->tx_buf[2] = addr >> 8;
+		phy->tx_buf[3] = addr;
+
+		spi_message_init(&m);
+		spi_message_add_tail(&spi_xfer, &m);
+		ret = spi_sync_locked(phy->spi_device, &m);
+		if (ret < 0)
+			goto exit;
 
-	spi_bus_lock(phy->spi_device->master);
-	ret = spi_sync_locked(phy->spi_device, &m);
-	if (ret < 0)
-		goto exit;
-
-	if ((phy->rx_buf[3] & 0x01) == 0) {
-		// handle SPI wait states
-		int i;
-
-		phy->tx_buf[0] = 0;
-
-		for (i = 0; i < TPM_RETRY; i++) {
-			spi_xfer.len = 1;
-			spi_message_init(&m);
-			spi_message_add_tail(&spi_xfer, &m);
-			ret = spi_sync_locked(phy->spi_device, &m);
-			if (ret < 0)
+		if ((phy->rx_buf[3] & 0x01) == 0) {
+			// handle SPI wait states
+			int i;
+
+			phy->tx_buf[0] = 0;
+
+			for (i = 0; i < TPM_RETRY; i++) {
+				spi_xfer.len = 1;
+				spi_message_init(&m);
+				spi_message_add_tail(&spi_xfer, &m);
+				ret = spi_sync_locked(phy->spi_device, &m);
+				if (ret < 0)
+					goto exit;
+				if (phy->rx_buf[0] & 0x01)
+					break;
+			}
+
+			if (i == TPM_RETRY) {
+				ret = -ETIMEDOUT;
 				goto exit;
-			if (phy->rx_buf[0] & 0x01)
-				break;
+			}
 		}
 
-		if (i == TPM_RETRY) {
-			ret = -ETIMEDOUT;
-			goto exit;
+		spi_xfer.cs_change = 0;
+		spi_xfer.len = transfer_len;
+
+		if (direction) {
+			spi_xfer.tx_buf = NULL;
+			spi_xfer.rx_buf = buffer;
+		} else {
+			spi_xfer.tx_buf = buffer;
+			spi_xfer.rx_buf = NULL;
 		}
-	}
 
-	spi_xfer.cs_change = 0;
-	spi_xfer.len = len;
+		spi_message_init(&m);
+		spi_message_add_tail(&spi_xfer, &m);
+		ret = spi_sync_locked(phy->spi_device, &m);
+		if (ret < 0)
+			goto exit;
 
-	if (direction) {
-		spi_xfer.tx_buf = NULL;
-		spi_xfer.rx_buf = buffer;
-	} else {
-		spi_xfer.tx_buf = buffer;
-		spi_xfer.rx_buf = NULL;
+		len -= transfer_len;
+		buffer += transfer_len;
 	}
 
-	spi_message_init(&m);
-	spi_message_add_tail(&spi_xfer, &m);
-	ret = spi_sync_locked(phy->spi_device, &m);
-
 exit:
 	spi_bus_unlock(phy->spi_device->master);
 	return ret;
-- 
2.7.4

[toc] | [next] | [standalone]

#1582724 — Re: [PATCH 4/5] tpm_tis_spi: Remove limitation of transfers to MAX_SPI_FRAMESIZE bytes

[Multipart message — attachments visible in raw view] — view raw

Hi Peter,

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on v4.10-rc8 next-20170216]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Peter-Huewe/Fix-whole-native-SPI-TPM-driver/20170217-010419
config: x86_64-randconfig-x001-201707 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/char/tpm/tpm_tis_spi.c: In function 'tpm_tis_spi_transfer':
>> drivers/char/tpm/tpm_tis_spi.c:135:9: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return ret;
            ^~~

vim +/ret +135 drivers/char/tpm/tpm_tis_spi.c

dae9219c Peter Huewe       2017-02-16  119  			spi_xfer.tx_buf = buffer;
dae9219c Peter Huewe       2017-02-16  120  			spi_xfer.rx_buf = NULL;
dae9219c Peter Huewe       2017-02-16  121  		}
0edbfea5 Christophe Ricard 2016-05-19  122  
0edbfea5 Christophe Ricard 2016-05-19  123  		spi_message_init(&m);
0edbfea5 Christophe Ricard 2016-05-19  124  		spi_message_add_tail(&spi_xfer, &m);
0edbfea5 Christophe Ricard 2016-05-19  125  		ret = spi_sync_locked(phy->spi_device, &m);
4dea582e Peter Huewe       2017-02-16  126  		if (ret < 0)
4dea582e Peter Huewe       2017-02-16  127  			goto exit;
4dea582e Peter Huewe       2017-02-16  128  
4dea582e Peter Huewe       2017-02-16  129  		len -= transfer_len;
4dea582e Peter Huewe       2017-02-16  130  		buffer += transfer_len;
4dea582e Peter Huewe       2017-02-16  131  	}
0edbfea5 Christophe Ricard 2016-05-19  132  
0edbfea5 Christophe Ricard 2016-05-19  133  exit:
0edbfea5 Christophe Ricard 2016-05-19  134  	spi_bus_unlock(phy->spi_device->master);
0edbfea5 Christophe Ricard 2016-05-19 @135  	return ret;
0edbfea5 Christophe Ricard 2016-05-19  136  }
0edbfea5 Christophe Ricard 2016-05-19  137  
dae9219c Peter Huewe       2017-02-16  138  static int tpm_tis_spi_read_bytes(struct tpm_tis_data *data, u32 addr,
dae9219c Peter Huewe       2017-02-16  139  				  u16 len, u8 *result)
0edbfea5 Christophe Ricard 2016-05-19  140  {
dae9219c Peter Huewe       2017-02-16  141  	return tpm_tis_spi_transfer(data, addr, len, result, 0x80);
0edbfea5 Christophe Ricard 2016-05-19  142  }
0edbfea5 Christophe Ricard 2016-05-19  143  

:::::: The code at line 135 was first introduced by commit
:::::: 0edbfea537d10c0de5505d0413368aad71027663 tpm/tpm_tis_spi: Add support for spi phy

:::::: TO: Christophe Ricard <christophe.ricard@gmail.com>
:::::: CC: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1583099 — Re: [PATCH 4/5] tpm_tis_spi: Remove limitation of transfers to MAX_SPI_FRAMESIZE bytes

I am not sure i understand here, are you considering there could be 
burstcount > 64 with "TCG" TPM ?

Or is this because of TIS vs PTP differences ?

To be honest, this is a little behind me now :-)


On 16/02/2017 08:08, Peter Huewe wrote:
> Limiting transfers to MAX_SPI_FRAMESIZE was not expected by the upper
> layers, as tpm_tis has no such limitation. Add a loop to hide that
> limitation.
>
> Cc: <stable@vger.kernel.org>
> Fixes: 0edbfea537d1 ("tpm/tpm_tis_spi: Add support for spi phy")
> Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
> Signed-off-by: Peter Huewe <peter.huewe@infineon.com>
> ---
>   drivers/char/tpm/tpm_tis_spi.c | 108 ++++++++++++++++++++++-------------------
>   1 file changed, 57 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_spi.c b/drivers/char/tpm/tpm_tis_spi.c
> index 16938e2253d2..b50c5b072df3 100644
> --- a/drivers/char/tpm/tpm_tis_spi.c
> +++ b/drivers/char/tpm/tpm_tis_spi.c
> @@ -61,68 +61,74 @@ static int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u8 len,
>   {
>   	struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
>   	int ret;
> -	struct spi_message m;
> -	struct spi_transfer spi_xfer = {
> -		.tx_buf = phy->tx_buf,
> -		.rx_buf = phy->rx_buf,
> -		.len = 4,
> -		.cs_change = 1,
> -	};
> -
> -	if (len > MAX_SPI_FRAMESIZE)
> -		return -ENOMEM;
>   
> -	phy->tx_buf[0] = direction | (len - 1);
> -	phy->tx_buf[1] = 0xd4;
> -	phy->tx_buf[2] = addr >> 8;
> -	phy->tx_buf[3] = addr;
> +	spi_bus_lock(phy->spi_device->master);
>   
> -	spi_message_init(&m);
> -	spi_message_add_tail(&spi_xfer, &m);
> +	while (len) {
> +		struct spi_message m;
> +		struct spi_transfer spi_xfer = {
> +			.tx_buf = phy->tx_buf,
> +			.rx_buf = phy->rx_buf,
> +			.len = 4,
> +			.cs_change = 1,
> +		};
> +		u8 transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);
> +
> +		phy->tx_buf[0] = direction | (transfer_len - 1);
> +		phy->tx_buf[1] = 0xd4;
> +		phy->tx_buf[2] = addr >> 8;
> +		phy->tx_buf[3] = addr;
> +
> +		spi_message_init(&m);
> +		spi_message_add_tail(&spi_xfer, &m);
> +		ret = spi_sync_locked(phy->spi_device, &m);
> +		if (ret < 0)
> +			goto exit;
>   
> -	spi_bus_lock(phy->spi_device->master);
> -	ret = spi_sync_locked(phy->spi_device, &m);
> -	if (ret < 0)
> -		goto exit;
> -
> -	if ((phy->rx_buf[3] & 0x01) == 0) {
> -		// handle SPI wait states
> -		int i;
> -
> -		phy->tx_buf[0] = 0;
> -
> -		for (i = 0; i < TPM_RETRY; i++) {
> -			spi_xfer.len = 1;
> -			spi_message_init(&m);
> -			spi_message_add_tail(&spi_xfer, &m);
> -			ret = spi_sync_locked(phy->spi_device, &m);
> -			if (ret < 0)
> +		if ((phy->rx_buf[3] & 0x01) == 0) {
> +			// handle SPI wait states
> +			int i;
> +
> +			phy->tx_buf[0] = 0;
> +
> +			for (i = 0; i < TPM_RETRY; i++) {
> +				spi_xfer.len = 1;
> +				spi_message_init(&m);
> +				spi_message_add_tail(&spi_xfer, &m);
> +				ret = spi_sync_locked(phy->spi_device, &m);
> +				if (ret < 0)
> +					goto exit;
> +				if (phy->rx_buf[0] & 0x01)
> +					break;
> +			}
> +
> +			if (i == TPM_RETRY) {
> +				ret = -ETIMEDOUT;
>   				goto exit;
> -			if (phy->rx_buf[0] & 0x01)
> -				break;
> +			}
>   		}
>   
> -		if (i == TPM_RETRY) {
> -			ret = -ETIMEDOUT;
> -			goto exit;
> +		spi_xfer.cs_change = 0;
> +		spi_xfer.len = transfer_len;
> +
> +		if (direction) {
> +			spi_xfer.tx_buf = NULL;
> +			spi_xfer.rx_buf = buffer;
> +		} else {
> +			spi_xfer.tx_buf = buffer;
> +			spi_xfer.rx_buf = NULL;
>   		}
> -	}
>   
> -	spi_xfer.cs_change = 0;
> -	spi_xfer.len = len;
> +		spi_message_init(&m);
> +		spi_message_add_tail(&spi_xfer, &m);
> +		ret = spi_sync_locked(phy->spi_device, &m);
> +		if (ret < 0)
> +			goto exit;
>   
> -	if (direction) {
> -		spi_xfer.tx_buf = NULL;
> -		spi_xfer.rx_buf = buffer;
> -	} else {
> -		spi_xfer.tx_buf = buffer;
> -		spi_xfer.rx_buf = NULL;
> +		len -= transfer_len;
> +		buffer += transfer_len;
>   	}
>   
> -	spi_message_init(&m);
> -	spi_message_add_tail(&spi_xfer, &m);
> -	ret = spi_sync_locked(phy->spi_device, &m);
> -
>   exit:
>   	spi_bus_unlock(phy->spi_device->master);
>   	return ret;

[toc] | [prev] | [next] | [standalone]

#1583157

Am 17. Februar 2017 06:11:53 MEZ schrieb Christophe Ricard <christophe.ricard@gmail.com>:
>I am not sure i understand here, are you considering there could be 
>burstcount > 64 with "TCG" TPM ?
>
With the current upstream version, any command larger with a response of more than 64 byte is broken, since the splitting loop went missing during the merge.
At least with our tpms that's the case.
(I think we are signalling a higher burstcount value than 64, which is allowed)

>Or is this because of TIS vs PTP differences ?
>
>To be honest, this is a little behind me now :-)
>
>
>On 16/02/2017 08:08, Peter Huewe wrote:
>> Limiting transfers to MAX_SPI_FRAMESIZE was not expected by the upper
>> layers, as tpm_tis has no such limitation. Add a loop to hide that
>> limitation.
>>
>> Cc: <stable@vger.kernel.org>
>> Fixes: 0edbfea537d1 ("tpm/tpm_tis_spi: Add support for spi phy")
>> Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
>> Signed-off-by: Peter Huewe <peter.huewe@infineon.com>
>> ---
>>   drivers/char/tpm/tpm_tis_spi.c | 108
>++++++++++++++++++++++-------------------
>>   1 file changed, 57 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_spi.c
>b/drivers/char/tpm/tpm_tis_spi.c
>> index 16938e2253d2..b50c5b072df3 100644
>> --- a/drivers/char/tpm/tpm_tis_spi.c
>> +++ b/drivers/char/tpm/tpm_tis_spi.c
>> @@ -61,68 +61,74 @@ static int tpm_tis_spi_transfer(struct
>tpm_tis_data *data, u32 addr, u8 len,
>>   {
>>   	struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
>>   	int ret;
>> -	struct spi_message m;
>> -	struct spi_transfer spi_xfer = {
>> -		.tx_buf = phy->tx_buf,
>> -		.rx_buf = phy->rx_buf,
>> -		.len = 4,
>> -		.cs_change = 1,
>> -	};
>> -
>> -	if (len > MAX_SPI_FRAMESIZE)
>> -		return -ENOMEM;
>>   
>> -	phy->tx_buf[0] = direction | (len - 1);
>> -	phy->tx_buf[1] = 0xd4;
>> -	phy->tx_buf[2] = addr >> 8;
>> -	phy->tx_buf[3] = addr;
>> +	spi_bus_lock(phy->spi_device->master);
>>   
>> -	spi_message_init(&m);
>> -	spi_message_add_tail(&spi_xfer, &m);
>> +	while (len) {
>> +		struct spi_message m;
>> +		struct spi_transfer spi_xfer = {
>> +			.tx_buf = phy->tx_buf,
>> +			.rx_buf = phy->rx_buf,
>> +			.len = 4,
>> +			.cs_change = 1,
>> +		};
>> +		u8 transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);
>> +
>> +		phy->tx_buf[0] = direction | (transfer_len - 1);
>> +		phy->tx_buf[1] = 0xd4;
>> +		phy->tx_buf[2] = addr >> 8;
>> +		phy->tx_buf[3] = addr;
>> +
>> +		spi_message_init(&m);
>> +		spi_message_add_tail(&spi_xfer, &m);
>> +		ret = spi_sync_locked(phy->spi_device, &m);
>> +		if (ret < 0)
>> +			goto exit;
>>   
>> -	spi_bus_lock(phy->spi_device->master);
>> -	ret = spi_sync_locked(phy->spi_device, &m);
>> -	if (ret < 0)
>> -		goto exit;
>> -
>> -	if ((phy->rx_buf[3] & 0x01) == 0) {
>> -		// handle SPI wait states
>> -		int i;
>> -
>> -		phy->tx_buf[0] = 0;
>> -
>> -		for (i = 0; i < TPM_RETRY; i++) {
>> -			spi_xfer.len = 1;
>> -			spi_message_init(&m);
>> -			spi_message_add_tail(&spi_xfer, &m);
>> -			ret = spi_sync_locked(phy->spi_device, &m);
>> -			if (ret < 0)
>> +		if ((phy->rx_buf[3] & 0x01) == 0) {
>> +			// handle SPI wait states
>> +			int i;
>> +
>> +			phy->tx_buf[0] = 0;
>> +
>> +			for (i = 0; i < TPM_RETRY; i++) {
>> +				spi_xfer.len = 1;
>> +				spi_message_init(&m);
>> +				spi_message_add_tail(&spi_xfer, &m);
>> +				ret = spi_sync_locked(phy->spi_device, &m);
>> +				if (ret < 0)
>> +					goto exit;
>> +				if (phy->rx_buf[0] & 0x01)
>> +					break;
>> +			}
>> +
>> +			if (i == TPM_RETRY) {
>> +				ret = -ETIMEDOUT;
>>   				goto exit;
>> -			if (phy->rx_buf[0] & 0x01)
>> -				break;
>> +			}
>>   		}
>>   
>> -		if (i == TPM_RETRY) {
>> -			ret = -ETIMEDOUT;
>> -			goto exit;
>> +		spi_xfer.cs_change = 0;
>> +		spi_xfer.len = transfer_len;
>> +
>> +		if (direction) {
>> +			spi_xfer.tx_buf = NULL;
>> +			spi_xfer.rx_buf = buffer;
>> +		} else {
>> +			spi_xfer.tx_buf = buffer;
>> +			spi_xfer.rx_buf = NULL;
>>   		}
>> -	}
>>   
>> -	spi_xfer.cs_change = 0;
>> -	spi_xfer.len = len;
>> +		spi_message_init(&m);
>> +		spi_message_add_tail(&spi_xfer, &m);
>> +		ret = spi_sync_locked(phy->spi_device, &m);
>> +		if (ret < 0)
>> +			goto exit;
>>   
>> -	if (direction) {
>> -		spi_xfer.tx_buf = NULL;
>> -		spi_xfer.rx_buf = buffer;
>> -	} else {
>> -		spi_xfer.tx_buf = buffer;
>> -		spi_xfer.rx_buf = NULL;
>> +		len -= transfer_len;
>> +		buffer += transfer_len;
>>   	}
>>   
>> -	spi_message_init(&m);
>> -	spi_message_add_tail(&spi_xfer, &m);
>> -	ret = spi_sync_locked(phy->spi_device, &m);
>> -
>>   exit:
>>   	spi_bus_unlock(phy->spi_device->master);
>>   	return ret;

-- 
Sent from my mobile

[toc] | [prev] | [next] | [standalone]

#1587542 — Re: [PATCH 4/5] tpm_tis_spi: Remove limitation of transfers to MAX_SPI_FRAMESIZE bytes

On Thu, Feb 16, 2017 at 04:08:25PM +0000, Peter Huewe wrote:
> Limiting transfers to MAX_SPI_FRAMESIZE was not expected by the upper
> layers, as tpm_tis has no such limitation. Add a loop to hide that
> limitation.
> 
> Cc: <stable@vger.kernel.org>
> Fixes: 0edbfea537d1 ("tpm/tpm_tis_spi: Add support for spi phy")
> Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
> Signed-off-by: Peter Huewe <peter.huewe@infineon.com>
> ---
>  drivers/char/tpm/tpm_tis_spi.c | 108 ++++++++++++++++++++++-------------------
>  1 file changed, 57 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_tis_spi.c b/drivers/char/tpm/tpm_tis_spi.c
> index 16938e2253d2..b50c5b072df3 100644
> --- a/drivers/char/tpm/tpm_tis_spi.c
> +++ b/drivers/char/tpm/tpm_tis_spi.c
> @@ -61,68 +61,74 @@ static int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u8 len,
>  {
>  	struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
>  	int ret;
> -	struct spi_message m;
> -	struct spi_transfer spi_xfer = {
> -		.tx_buf = phy->tx_buf,
> -		.rx_buf = phy->rx_buf,
> -		.len = 4,
> -		.cs_change = 1,
> -	};
> -
> -	if (len > MAX_SPI_FRAMESIZE)
> -		return -ENOMEM;
>  
> -	phy->tx_buf[0] = direction | (len - 1);
> -	phy->tx_buf[1] = 0xd4;
> -	phy->tx_buf[2] = addr >> 8;
> -	phy->tx_buf[3] = addr;
> +	spi_bus_lock(phy->spi_device->master);
>  
> -	spi_message_init(&m);
> -	spi_message_add_tail(&spi_xfer, &m);
> +	while (len) {
> +		struct spi_message m;
> +		struct spi_transfer spi_xfer = {
> +			.tx_buf = phy->tx_buf,
> +			.rx_buf = phy->rx_buf,
> +			.len = 4,
> +			.cs_change = 1,
> +		};

I like the habbit of keeping all the declarations in the beginning of
the function as we mostly have in the subsystem. It gives a fast view
what the function eats and how much stack it uses.

If you really think that declaring spi_xfer here is a good idea please
create a separate "helper" function for it.

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1587607 — Re: [PATCH 4/5] tpm_tis_spi: Remove limitation of transfers to MAX_SPI_FRAMESIZE bytes

On Thu, Feb 16, 2017 at 04:08:25PM +0000, Peter Huewe wrote:
> Limiting transfers to MAX_SPI_FRAMESIZE was not expected by the upper
> layers, as tpm_tis has no such limitation. Add a loop to hide that
> limitation.
> 
> Cc: <stable@vger.kernel.org>
> Fixes: 0edbfea537d1 ("tpm/tpm_tis_spi: Add support for spi phy")
> Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
> Signed-off-by: Peter Huewe <peter.huewe@infineon.com>

This commit also had kbuild warning.

PS. When you send the next patch set version, please add this to
every commit:

  Tested-by: Benoit Houyere <benoit.houyere@st.com>

/Jarkko

> ---
>  drivers/char/tpm/tpm_tis_spi.c | 108 ++++++++++++++++++++++-------------------
>  1 file changed, 57 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_tis_spi.c b/drivers/char/tpm/tpm_tis_spi.c
> index 16938e2253d2..b50c5b072df3 100644
> --- a/drivers/char/tpm/tpm_tis_spi.c
> +++ b/drivers/char/tpm/tpm_tis_spi.c
> @@ -61,68 +61,74 @@ static int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u8 len,
>  {
>  	struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
>  	int ret;
> -	struct spi_message m;
> -	struct spi_transfer spi_xfer = {
> -		.tx_buf = phy->tx_buf,
> -		.rx_buf = phy->rx_buf,
> -		.len = 4,
> -		.cs_change = 1,
> -	};
> -
> -	if (len > MAX_SPI_FRAMESIZE)
> -		return -ENOMEM;
>  
> -	phy->tx_buf[0] = direction | (len - 1);
> -	phy->tx_buf[1] = 0xd4;
> -	phy->tx_buf[2] = addr >> 8;
> -	phy->tx_buf[3] = addr;
> +	spi_bus_lock(phy->spi_device->master);
>  
> -	spi_message_init(&m);
> -	spi_message_add_tail(&spi_xfer, &m);
> +	while (len) {
> +		struct spi_message m;
> +		struct spi_transfer spi_xfer = {
> +			.tx_buf = phy->tx_buf,
> +			.rx_buf = phy->rx_buf,
> +			.len = 4,
> +			.cs_change = 1,
> +		};
> +		u8 transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);
> +
> +		phy->tx_buf[0] = direction | (transfer_len - 1);
> +		phy->tx_buf[1] = 0xd4;
> +		phy->tx_buf[2] = addr >> 8;
> +		phy->tx_buf[3] = addr;
> +
> +		spi_message_init(&m);
> +		spi_message_add_tail(&spi_xfer, &m);
> +		ret = spi_sync_locked(phy->spi_device, &m);
> +		if (ret < 0)
> +			goto exit;
>  
> -	spi_bus_lock(phy->spi_device->master);
> -	ret = spi_sync_locked(phy->spi_device, &m);
> -	if (ret < 0)
> -		goto exit;
> -
> -	if ((phy->rx_buf[3] & 0x01) == 0) {
> -		// handle SPI wait states
> -		int i;
> -
> -		phy->tx_buf[0] = 0;
> -
> -		for (i = 0; i < TPM_RETRY; i++) {
> -			spi_xfer.len = 1;
> -			spi_message_init(&m);
> -			spi_message_add_tail(&spi_xfer, &m);
> -			ret = spi_sync_locked(phy->spi_device, &m);
> -			if (ret < 0)
> +		if ((phy->rx_buf[3] & 0x01) == 0) {
> +			// handle SPI wait states
> +			int i;
> +
> +			phy->tx_buf[0] = 0;
> +
> +			for (i = 0; i < TPM_RETRY; i++) {
> +				spi_xfer.len = 1;
> +				spi_message_init(&m);
> +				spi_message_add_tail(&spi_xfer, &m);
> +				ret = spi_sync_locked(phy->spi_device, &m);
> +				if (ret < 0)
> +					goto exit;
> +				if (phy->rx_buf[0] & 0x01)
> +					break;
> +			}
> +
> +			if (i == TPM_RETRY) {
> +				ret = -ETIMEDOUT;
>  				goto exit;
> -			if (phy->rx_buf[0] & 0x01)
> -				break;
> +			}
>  		}
>  
> -		if (i == TPM_RETRY) {
> -			ret = -ETIMEDOUT;
> -			goto exit;
> +		spi_xfer.cs_change = 0;
> +		spi_xfer.len = transfer_len;
> +
> +		if (direction) {
> +			spi_xfer.tx_buf = NULL;
> +			spi_xfer.rx_buf = buffer;
> +		} else {
> +			spi_xfer.tx_buf = buffer;
> +			spi_xfer.rx_buf = NULL;
>  		}
> -	}
>  
> -	spi_xfer.cs_change = 0;
> -	spi_xfer.len = len;
> +		spi_message_init(&m);
> +		spi_message_add_tail(&spi_xfer, &m);
> +		ret = spi_sync_locked(phy->spi_device, &m);
> +		if (ret < 0)
> +			goto exit;
>  
> -	if (direction) {
> -		spi_xfer.tx_buf = NULL;
> -		spi_xfer.rx_buf = buffer;
> -	} else {
> -		spi_xfer.tx_buf = buffer;
> -		spi_xfer.rx_buf = NULL;
> +		len -= transfer_len;
> +		buffer += transfer_len;
>  	}
>  
> -	spi_message_init(&m);
> -	spi_message_add_tail(&spi_xfer, &m);
> -	ret = spi_sync_locked(phy->spi_device, &m);
> -
>  exit:
>  	spi_bus_unlock(phy->spi_device->master);
>  	return ret;
> -- 
> 2.7.4
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web