diff mbox series

[2/2] NFC: nxp-nci: Don't issue a zero length i2c_master_read()

Message ID 20220626194243.4059870-2-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

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1 this patch: 1
netdev/cc_maintainers fail 1 blamed authors not CCed: sameo@linux.intel.com; 1 maintainers not CCed: sameo@linux.intel.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1 this patch: 1
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 9 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Michael Walle June 26, 2022, 7:42 p.m. UTC
There are packets which doesn't have a payload. In that case, the second
i2c_master_read() will have a zero length. But because the NFC
controller doesn't have any data left, it will NACK the I2C read and
-ENXIO will be returned. In case there is no payload, just skip the
second i2c master read.

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 | 3 +++
 1 file changed, 3 insertions(+)

Comments

Krzysztof Kozlowski June 26, 2022, 7:51 p.m. UTC | #1
On 26/06/2022 21:42, Michael Walle wrote:
> There are packets which doesn't have a payload. In that case, the second
> i2c_master_read() will have a zero length. But because the NFC
> controller doesn't have any data left, it will NACK the I2C read and
> -ENXIO will be returned. In case there is no payload, just skip the
> second i2c master read.
> 
> Fixes: 6be88670fc59 ("NFC: nxp-nci_i2c: Add I2C support to NXP NCI driver")
> Signed-off-by: Michael Walle <michael@walle.cc>

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof
Michael Walle June 26, 2022, 7:56 p.m. UTC | #2
Am 2022-06-26 21:51, schrieb Krzysztof Kozlowski:
> On 26/06/2022 21:42, Michael Walle wrote:
>> There are packets which doesn't have a payload. In that case, the 
>> second
>> i2c_master_read() will have a zero length. But because the NFC
>> controller doesn't have any data left, it will NACK the I2C read and
>> -ENXIO will be returned. In case there is no payload, just skip the
>> second i2c master read.
>> 
>> Fixes: 6be88670fc59 ("NFC: nxp-nci_i2c: Add I2C support to NXP NCI 
>> driver")
>> Signed-off-by: Michael Walle <michael@walle.cc>
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Thanks, I'll reorder the patches in the next version otherwise
there will likely be a conflict. That should work with any patch
tools (i.e. b4), shouldn't it?

-michael
Krzysztof Kozlowski June 26, 2022, 8:02 p.m. UTC | #3
On 26/06/2022 21:56, Michael Walle wrote:
> Am 2022-06-26 21:51, schrieb Krzysztof Kozlowski:
>> On 26/06/2022 21:42, Michael Walle wrote:
>>> There are packets which doesn't have a payload. In that case, the 
>>> second
>>> i2c_master_read() will have a zero length. But because the NFC
>>> controller doesn't have any data left, it will NACK the I2C read and
>>> -ENXIO will be returned. In case there is no payload, just skip the
>>> second i2c master read.
>>>
>>> Fixes: 6be88670fc59 ("NFC: nxp-nci_i2c: Add I2C support to NXP NCI 
>>> driver")
>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Thanks, I'll reorder the patches in the next version otherwise
> there will likely be a conflict.

Yes.

> That should work with any patch
> tools (i.e. b4), shouldn't it?

You mean - re-ordering should work? Yes, no problem with that.

Best regards,
Krzysztof
Jakub Kicinski June 28, 2022, 5:03 a.m. UTC | #4
On Sun, 26 Jun 2022 21:42:43 +0200 Michael Walle wrote:
> There are packets which doesn't have a payload. In that case, the second
> i2c_master_read() will have a zero length. But because the NFC
> controller doesn't have any data left, it will NACK the I2C read and
> -ENXIO will be returned. In case there is no payload, just skip the
> second i2c master read.

Whoa, are you using this code or just found the problem thru code
inspection? NFC is notorious for having no known users.
Michael Walle June 28, 2022, 6:42 a.m. UTC | #5
Am 2022-06-28 07:03, schrieb Jakub Kicinski:
> On Sun, 26 Jun 2022 21:42:43 +0200 Michael Walle wrote:
>> There are packets which doesn't have a payload. In that case, the 
>> second
>> i2c_master_read() will have a zero length. But because the NFC
>> controller doesn't have any data left, it will NACK the I2C read and
>> -ENXIO will be returned. In case there is no payload, just skip the
>> second i2c master read.
> 
> Whoa, are you using this code or just found the problem thru code
> inspection? NFC is notorious for having no known users.

Ha! Well, I *try* to use it with a PN7160. No luck so far, we'll see.
At least the communication with the chip works now. I was also kinda
tricked by the Supported status ;)

-michael
diff mbox series

Patch

diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 9c80d5a6d56b..c9361b5249b7 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -162,6 +162,9 @@  static int nxp_nci_i2c_nci_read(struct nxp_nci_i2c_phy *phy,
 
 	skb_put_data(*skb, (void *)&header, NCI_CTRL_HDR_SIZE);
 
+	if (!header.plen)
+		return 0;
+
 	r = i2c_master_recv(client, skb_put(*skb, header.plen), header.plen);
 	if (r < 0) {
 		nfc_err(&client->dev, "I2C receive error %pe\n", ERR_PTR(r));