Messages in this thread | | | Date | Sun, 26 Jun 2022 21:55:16 +0200 | From | Michael Walle <> | Subject | Re: [PATCH 1/2] NFC: nxp-nci: check return code of i2c_master_recv() |
| |
Am 2022-06-26 21:49, schrieb Krzysztof Kozlowski: > On 26/06/2022 21:42, Michael Walle wrote: >> Check the return code of i2c_master_recv() for actual errors and >> propagate it to the caller. >> >> Fixes: 6be88670fc59 ("NFC: nxp-nci_i2c: Add I2C support to NXP NCI >> driver") > > The check was there, so I don't see here bug. The only thing missing > was > a bit more detailed error message (without cast to %u) and propagating > error code instead of EBADMSG, but these are not bugs. The commit msg > should sound different and Fixes tag is not appropriate.
Well one could argue the nfc_err() is very misleading as it prints an unreasonable large number. Will remove the Fixes tag and reword the commit message.
>> Signed-off-by: Michael Walle <michael@walle.cc> >> --- >> drivers/nfc/nxp-nci/i2c.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c >> index 7e451c10985d..9c80d5a6d56b 100644 >> --- a/drivers/nfc/nxp-nci/i2c.c >> +++ b/drivers/nfc/nxp-nci/i2c.c >> @@ -163,7 +163,10 @@ static int nxp_nci_i2c_nci_read(struct >> nxp_nci_i2c_phy *phy, >> skb_put_data(*skb, (void *)&header, NCI_CTRL_HDR_SIZE); >> >> r = i2c_master_recv(client, skb_put(*skb, header.plen), >> header.plen); >> - if (r != header.plen) { >> + if (r < 0) { >> + nfc_err(&client->dev, "I2C receive error %pe\n", ERR_PTR(r)); > > Print just 'r'.
Personally, I prefer seeing the actual error string and this idiom is also used in other drivers. But I wont insist, will change it.
> >> + goto nci_read_exit_free_skb; >> + } else if (r != header.plen) { >> nfc_err(&client->dev, >> "Invalid frame payload length: %u (expected %u)\n", >> r, header.plen); > > > Best regards, > Krzysztof
-michael
| |