[bug report] tpm/tpm_i2c_stm_st33: Split tpm_i2c_tpm_st33 in 2 layers (core + phy)

[bug report] tpm/tpm_i2c_stm_st33: Split tpm_i2c_tpm_st33 in 2 layers (core + phy)

[Dan Carpenter]

[ Ancient code.  Never use your permanent Gmail address for kernel
  development if you want to have peace in your life.  - dan ]

Hello Christophe Ricard,

The patch bf38b8710892: "tpm/tpm_i2c_stm_st33: Split tpm_i2c_tpm_st33
in 2 layers (core + phy)" from Mar 8, 2015, leads to the following
Smatch static checker warning:

	drivers/char/tpm/st33zp24/st33zp24.c:117 check_locality()
	error: uninitialized symbol 'data'.

drivers/char/tpm/st33zp24/st33zp24.c
    110 static bool check_locality(struct tpm_chip *chip)
    111 {
    112         struct st33zp24_dev *tpm_dev = dev_get_drvdata(&chip->dev);
    113         u8 data;
    114         u8 status;
    115 
    116         status = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_ACCESS, &data, 1);
--> 117         if (status && (data &

The ->recv() functions, st33zp24_i2c_recv() and st33zp24_spi_recv(),
return negative error codes.

st33zp24_i2c_recv() is especially of problematic because theoretically
it could return 1 on error.  The i2c_master_recv() function is tricky.
It would be better if it returned negative and zero instead of returning
the number of bytes sent.

USB had this same issue so Greg added new helper functions in commit
719b8f2850d3 ("USB: add usb_control_msg_send() and usb_control_msg_recv()")

    118                 (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) ==
    119                 (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID))
    120                 return true;
    121 
    122         return false;
    123 } /* check_locality() */

regards,
dan carpenter