| Message ID | 20220626194243.4059870-1-michael@walle.cc (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [1/2] NFC: nxp-nci: check return code of i2c_master_recv() | expand |
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. > 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'. > + 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
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
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)); + 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);
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") Signed-off-by: Michael Walle <michael@walle.cc> --- drivers/nfc/nxp-nci/i2c.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)