On 9/18/23 03:26, Heikki Krogerus wrote:
On Thu, Sep 14, 2023 at 08:11:57PM +0800, Xu Yang wrote:
The return value from tcpci/regmap_read/write() must be checked to get
rid of the bad influence of other modules. This will add check code for
all of the rest read/write() callbacks and will show error when failed
to get ALERT register.
Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
---
Changes in v2:
- remove printing code
---
drivers/usb/typec/tcpm/tcpci.c | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 0ee3e6e29bb1..8ccc2d1a8ffc 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -657,21 +657,28 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
int ret;
unsigned int raw;
- tcpci_read16(tcpci, TCPC_ALERT, &status);
+ ret = tcpci_read16(tcpci, TCPC_ALERT, &status);
+ if (ret < 0)
+ return ret;
/*
* Clear alert status for everything except RX_STATUS, which shouldn't
* be cleared until we have successfully retrieved message.
*/
- if (status & ~TCPC_ALERT_RX_STATUS)
- tcpci_write16(tcpci, TCPC_ALERT,
+ if (status & ~TCPC_ALERT_RX_STATUS) {
+ ret = tcpci_write16(tcpci, TCPC_ALERT,
status & ~TCPC_ALERT_RX_STATUS);
+ if (ret < 0)
+ return ret;
+ }
if (status & TCPC_ALERT_CC_STATUS)
tcpm_cc_change(tcpci->port);
if (status & TCPC_ALERT_POWER_STATUS) {
- regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &raw);
+ ret = regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &raw);
+ if (ret < 0)
+ return ret;
/*
* If power status mask has been reset, then the TCPC
* has reset.
@@ -687,7 +694,9 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
unsigned int cnt, payload_cnt;
u16 header;
- regmap_read(tcpci->regmap, TCPC_RX_BYTE_CNT, &cnt);
+ ret = regmap_read(tcpci->regmap, TCPC_RX_BYTE_CNT, &cnt);
+ if (ret < 0)
+ return ret;
I think you still need to clear TCPC_ALERT_RX_STATUS in this case.
Guenter, can you check this?
If reading from or writing to the status register failed, we are pretty
much messed up anyway, so I don't think it really matters.
I think the more severe problem is that this is an interrupt handler,
which either handles the interrupt or it doesn't. It does not have the
option of returning an error (negative error code).
The submitter will have to decide what to do in the error case: Was
the interrupt handled or not ? I have no real answer to that question; all
answers seem wrong to me. I would tend to returning that the interrupt
was not handled. Most likely that would cause the handler to be called
again because the interrupt condition is not cleared. If this happens
repeatedly, the interrupt might end up being disabled, which would probably
be the best possible outcome.
Guenter
