[PATCH v1] tpm_tis_spi_main: set cs_change = 0 when timesout

[PATCH v1] tpm_tis_spi_main: set cs_change = 0 when timesout

[wanghongzhe]

when i reach TPM_RETRY, the cs cannot  change back to 'high'.
So the TPM chips thinks this communication is not over.
And next times communication cannot be effective because
the communications mixed up with the last time.

Signed-off-by: wanghongzhe <wanghongzhe@huawei.com>
---
 drivers/char/tpm/tpm_tis_spi_main.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c
index 3856f6ebcb34..6c52cbb28881 100644
--- a/drivers/char/tpm/tpm_tis_spi_main.c
+++ b/drivers/char/tpm/tpm_tis_spi_main.c
@@ -64,8 +64,18 @@ static int tpm_tis_spi_flow_control(struct tpm_tis_spi_phy *phy,
 				break;
 		}
 
-		if (i == TPM_RETRY)
+		if (i == TPM_RETRY) {
+			/* change back to 'high',
+			 * So the TPM chips thinks the last communication
+			 * is done.
+			 */
+			spi_xfer.cs_change = 0;
+			spi_xfer->len = 1;
+			spi_message_init(&m);
+			spi_message_add_tail(spi_xfer, &m);
+			ret = spi_sync_locked(phy->spi_device, &m);
 			return -ETIMEDOUT;
+		}
 	}
 
 	return 0;
-- 
2.19.1

Re: [PATCH v1] tpm_tis_spi_main: set cs_change = 0 when timesout

[kernel test robot]

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

Hi wanghongzhe,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v5.11-rc6 next-20210125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/wanghongzhe/tpm_tis_spi_main-set-cs_change-0-when-timesout/20210205-140104
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 3a11b0b5d8d2b3f7d4b44945ef9226a3115bb15f
config: nios2-allyesconfig (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/42343d4238cfd18831ed331b153cc6d2c0a7bc33
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review wanghongzhe/tpm_tis_spi_main-set-cs_change-0-when-timesout/20210205-140104
        git checkout 42343d4238cfd18831ed331b153cc6d2c0a7bc33
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nios2 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/char/tpm/tpm_tis_spi_main.c: In function 'tpm_tis_spi_flow_control':
>> drivers/char/tpm/tpm_tis_spi_main.c:72:12: error: 'spi_xfer' is a pointer; did you mean to use '->'?
      72 |    spi_xfer.cs_change = 0;
         |            ^
         |            ->
   At top level:
   drivers/char/tpm/tpm_tis_spi_main.c:288:36: warning: 'acpi_tis_spi_match' defined but not used [-Wunused-const-variable=]
     288 | static const struct acpi_device_id acpi_tis_spi_match[] = {
         |                                    ^~~~~~~~~~~~~~~~~~


vim +72 drivers/char/tpm/tpm_tis_spi_main.c

    40	
    41	/*
    42	 * TCG SPI flow control is documented in section 6.4 of the spec[1]. In short,
    43	 * keep trying to read from the device until MISO goes high indicating the
    44	 * wait state has ended.
    45	 *
    46	 * [1] https://trustedcomputinggroup.org/resource/pc-client-platform-tpm-profile-ptp-specification/
    47	 */
    48	static int tpm_tis_spi_flow_control(struct tpm_tis_spi_phy *phy,
    49					    struct spi_transfer *spi_xfer)
    50	{
    51		struct spi_message m;
    52		int ret, i;
    53	
    54		if ((phy->iobuf[3] & 0x01) == 0) {
    55			// handle SPI wait states
    56			for (i = 0; i < TPM_RETRY; i++) {
    57				spi_xfer->len = 1;
    58				spi_message_init(&m);
    59				spi_message_add_tail(spi_xfer, &m);
    60				ret = spi_sync_locked(phy->spi_device, &m);
    61				if (ret < 0)
    62					return ret;
    63				if (phy->iobuf[0] & 0x01)
    64					break;
    65			}
    66	
    67			if (i == TPM_RETRY) {
    68				/* change back to 'high',
    69				 * So the TPM chips thinks the last communication
    70				 * is done.
    71				 */
  > 72				spi_xfer.cs_change = 0;
    73				spi_xfer->len = 1;
    74				spi_message_init(&m);
    75				spi_message_add_tail(spi_xfer, &m);
    76				ret = spi_sync_locked(phy->spi_device, &m);
    77				return -ETIMEDOUT;
    78			}
    79		}
    80	
    81		return 0;
    82	}
    83	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 58793 bytes --]

Re: [PATCH v1] tpm_tis_spi_main: set cs_change = 0 when timesout

[Jarkko Sakkinen]

On Fri, Feb 05, 2021 at 02:42:05PM +0800, wanghongzhe wrote:
> when i reach TPM_RETRY, the cs cannot  change back to 'high'.
> So the TPM chips thinks this communication is not over.
> And next times communication cannot be effective because
> the communications mixed up with the last time.
> 
> Signed-off-by: wanghongzhe <wanghongzhe@huawei.com>
                 ~~~~~~~~~~~
                 Firstname Lastname

Please write legit sentences starting with capital letters etc.

Please write the commit message in imperative form. E.g. "Do x because y
..". I presume that *you* are not an actor in the sequence.

You also would need to have a fixes tag and preferably some description
of the failing sequence if possible.

/Jarkko

> ---
>  drivers/char/tpm/tpm_tis_spi_main.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c
> index 3856f6ebcb34..6c52cbb28881 100644
> --- a/drivers/char/tpm/tpm_tis_spi_main.c
> +++ b/drivers/char/tpm/tpm_tis_spi_main.c
> @@ -64,8 +64,18 @@ static int tpm_tis_spi_flow_control(struct tpm_tis_spi_phy *phy,
>  				break;
>  		}
>  
> -		if (i == TPM_RETRY)
> +		if (i == TPM_RETRY) {
> +			/* change back to 'high',
> +			 * So the TPM chips thinks the last communication
> +			 * is done.
> +			 */
> +			spi_xfer.cs_change = 0;
> +			spi_xfer->len = 1;
> +			spi_message_init(&m);
> +			spi_message_add_tail(spi_xfer, &m);
> +			ret = spi_sync_locked(phy->spi_device, &m);
>  			return -ETIMEDOUT;
> +		}
>  	}
>  
>  	return 0;
> -- 
> 2.19.1
> 
> 

[PATCH v2] tpm_tis_spi_main: set cs_change = 0 when timesout

[wanghongzhe]

The cs cannot change back to 'high' when the count is 
TPM_RETRY. So the TPM chips thought this communication 
is not over. And next times communication cannot be 
effective because the communications mixed up with the 
last time.

v1 -> v2:
 - fix spi_xfer.cs_change to spi_xfer->cs_change

Signed-off-by: wanghongzhe <wanghongzhe@huawei.com>
---
 drivers/char/tpm/tpm_tis_spi_main.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c
index 3856f6ebcb34..6c52cbb28881 100644
--- a/drivers/char/tpm/tpm_tis_spi_main.c
+++ b/drivers/char/tpm/tpm_tis_spi_main.c
@@ -64,8 +64,18 @@ static int tpm_tis_spi_flow_control(struct tpm_tis_spi_phy *phy,
 				break;
 		}
 
-		if (i == TPM_RETRY)
+		if (i == TPM_RETRY) {
+			/* change back to 'high',
+			 * So the TPM chips thinks the last communication
+			 * is done.
+			 */
+			spi_xfer->cs_change = 0;
+			spi_xfer->len = 1;
+			spi_message_init(&m);
+			spi_message_add_tail(spi_xfer, &m);
+			ret = spi_sync_locked(phy->spi_device, &m);
 			return -ETIMEDOUT;
+		}
 	}
 
 	return 0;
-- 
2.19.1