Message ID | 20230523024536.4294-1-shaopeijie@cestc.cn (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v3] tpm_tis_spi: Release chip select when flow control fails | expand |
On Tue May 23, 2023 at 5:45 AM EEST, wrote: > From: Peijie Shao <shaopeijie@cestc.cn> > > The failure paths in tpm_tis_spi_transfer() do not deactivate > chip select. Send an empty message (cs_select == 0) to overcome > this. > > The patch is tested by two ways. > One way needs to touch hardware: > 1. force pull MISO pin down to GND, it emulates a forever > 'WAIT' timing. > 2. probe cs pin by an oscilloscope. > 3. load tpm_tis_spi.ko. > After loading, dmesg prints: > "probe of spi0.0 failed with error -110" > and oscilloscope shows cs pin goes high(deactivated) after > the failure. Before the patch, cs pin keeps low. > > Second way is by writing a fake spi controller. > 1. implement .transfer_one method, fill all rx buf with 0. > 2. implement .set_cs method, print the state of cs pin. > we can see cs goes high after the failure. > > Signed-off-by: Peijie Shao <shaopeijie@cestc.cn> Looks good to me + great explanation, thank you. Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> BR, Jarkko
On Tue May 23, 2023 at 5:45 AM EEST, wrote: > From: Peijie Shao <shaopeijie@cestc.cn> > > The failure paths in tpm_tis_spi_transfer() do not deactivate > chip select. Send an empty message (cs_select == 0) to overcome > this. > > The patch is tested by two ways. > One way needs to touch hardware: > 1. force pull MISO pin down to GND, it emulates a forever > 'WAIT' timing. > 2. probe cs pin by an oscilloscope. > 3. load tpm_tis_spi.ko. > After loading, dmesg prints: > "probe of spi0.0 failed with error -110" > and oscilloscope shows cs pin goes high(deactivated) after > the failure. Before the patch, cs pin keeps low. > > Second way is by writing a fake spi controller. > 1. implement .transfer_one method, fill all rx buf with 0. > 2. implement .set_cs method, print the state of cs pin. > we can see cs goes high after the failure. > > Signed-off-by: Peijie Shao <shaopeijie@cestc.cn> Thanks I substitute the patch that I applied with this. Again: Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> BR, Jarkko
diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c index 1f5207974..9bfaba092 100644 --- a/drivers/char/tpm/tpm_tis_spi_main.c +++ b/drivers/char/tpm/tpm_tis_spi_main.c @@ -136,6 +136,14 @@ int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len, } exit: + if (ret < 0) { + /* Deactivate chip select */ + memset(&spi_xfer, 0, sizeof(spi_xfer)); + spi_message_init(&m); + spi_message_add_tail(&spi_xfer, &m); + spi_sync_locked(phy->spi_device, &m); + } + spi_bus_unlock(phy->spi_device->master); return ret; }