Re: [PATCH v2] tag: Implement readout of tag UID via DBus interface
by Frieder Schrempf
Hi Fabian,
On 16.03.21 19:49, Gottstein, Fabian wrote:
> Hi Frieder,
>
> thanks for the patch.
thanks for your feedback.
>
> Could you please also consider the following situation:
> In the case of a NFC Tag Type 1, the identifier is delivered via the RID command (see NFC Digital Protocol). Thus, the Tag's nfcid property is updated in a later step.
> To inform the neard users, a property changed signal has to be emitted when nfcid has changed (in near_tag_set_nfcid). Also, a exists() handler for the new DBus property should be implemented.
I'm new to NFC and D-Bus, so I don't know much about what use-cases and
requirements there are.
Your request sounds reasonable and I think I have a rough understanding
of what is probably needed to implement this. Still to actually do this
I need to look at the specifications and the code more closely and I
don't know if/when I will find time to do this.
Also I don't have any hardware to test this with NFC type 1 tags.
>
> Another thing regarding building the response message:
> The following code snippet could simplify and improve the readability of the usage of the dbus message builder:
>
> dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE_AS_STRING, &entry);
> dbus_message_iter_append_fixed_array(&entry, DBUS_TYPE_BYTE, &uid, len);
> dbus_message_iter_close_container(iter, &entry);
>
Thanks for the improved code, I will use this instead.
Frieder
>
>
> -----Original Message-----
> From: Schrempf Frieder <frieder.schrempf(a)kontron.de>
> Sent: Dienstag, 16. März 2021 12:22
> To: Samuel Ortiz <sameo(a)linux.intel.com>; linux-nfc(a)lists.01.org
> Cc: Frieder Schrempf <frieder.schrempf(a)kontron.de>
> Subject: [linux-nfc] [PATCH v2] tag: Implement readout of tag UID via DBus interface
>
> Caution: This e-mail originated from outside of Philips, be careful for phishing.
>
>
> From: Frieder Schrempf <frieder.schrempf(a)kontron.de>
>
> NFC tags usually provide an unique identifier. Neard already checks if one of the two types of identifiers is available, reads them from tags and stores them in near_tag.nfcid or near_tag.iso15693_uid respectively.
>
> Though currently it is not possible for any client application to get this information via the D-Bus interface as no property for the UID is implemented.
>
> This adds a 'Uid' property to the D-Bus interface for tags, which exposes the UID of the tag as byte array. If nfcid is available this is returned as UID, otherwise if iso15693_uid is available this is returned. If no UID is available, no 'Uid' property is exposed.
>
> Signed-off-by: Frieder Schrempf <frieder.schrempf(a)kontron.de>
> ---
> Changes in v2:
> * Add whitespaces after 'for' statements
> * Add more details to the commit message
> ---
> src/tag.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 54 insertions(+), 3 deletions(-)
>
> diff --git a/src/tag.c b/src/tag.c
> index 9eba4ee..d530893 100644
> --- a/src/tag.c
> +++ b/src/tag.c
> @@ -53,6 +53,7 @@ struct near_tag {
> uint8_t nfcid_len;
>
> uint8_t iso15693_dsfid;
> + uint8_t iso15693_uid_len;
> uint8_t iso15693_uid[NFC_MAX_ISO15693_UID_LEN];
>
> size_t data_length;
> @@ -168,6 +169,29 @@ static const char *type_string(struct near_tag *tag)
> return type;
> }
>
> +static const uint8_t uid_array(struct near_tag *tag, uint8_t **uid) {
> + if (tag->nfcid_len) {
> + DBG("NFCID: ");
> + for (int i = 0; i < tag->nfcid_len; i++)
> + DBG("%x", tag->nfcid[i]);
> +
> + *uid = tag->nfcid;
> +
> + return tag->nfcid_len;
> + } else if (tag->iso15693_uid_len) {
> + DBG("ISO-UID: ");
> + for (int i = 0; i < tag->iso15693_uid_len; i++)
> + DBG("%x", tag->iso15693_uid[i]);
> +
> + *uid = tag->iso15693_uid;
> +
> + return tag->iso15693_uid_len;
> + }
> +
> + return 0;
> +}
> +
> static const char *protocol_string(struct near_tag *tag) {
> const char *protocol;
> @@ -219,6 +243,30 @@ static gboolean property_get_type(const GDBusPropertyTable *property,
> return TRUE;
> }
>
> +static gboolean property_get_uid(const GDBusPropertyTable *property,
> + DBusMessageIter *iter, void
> +*user_data) {
> + struct near_tag *tag = user_data;
> + DBusMessageIter entry;
> + uint8_t *uid;
> + uint8_t len;
> +
> + len = uid_array(tag, &uid);
> + if (!uid || !len)
> + return FALSE;
> +
> + dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
> + "{y}", &entry);
> +
> + for (int i = 0; i < len; i++)
> + dbus_message_iter_append_basic(&entry, DBUS_TYPE_BYTE,
> + (void *)&uid[i]);
> +
> + dbus_message_iter_close_container(iter, &entry);
> +
> + return TRUE;
> +}
> +
> static gboolean property_get_protocol(const GDBusPropertyTable *property,
> DBusMessageIter *iter, void *user_data) { @@ -526,6 +574,7 @@ static const GDBusPropertyTable tag_properties[] = {
> { "Protocol", "s", property_get_protocol },
> { "ReadOnly", "b", property_get_readonly },
> { "Adapter", "o", property_get_adapter },
> + { "Uid", "ay", property_get_uid },
>
> { }
> };
> @@ -671,8 +720,10 @@ static int tag_initialize(struct near_tag *tag,
> if (nfcid_len && nfcid_len <= NFC_MAX_NFCID1_LEN) {
> tag->nfcid_len = nfcid_len;
> memcpy(tag->nfcid, nfcid, nfcid_len);
> - } else if (iso15693_uid_len) {
> + } else if (iso15693_uid_len &&
> + iso15693_uid_len <= NFC_MAX_ISO15693_UID_LEN) {
> tag->iso15693_dsfid = iso15693_dsfid;
> + tag->iso15693_uid_len = iso15693_uid_len;
> memcpy(tag->iso15693_uid, iso15693_uid, iso15693_uid_len);
> }
>
> @@ -837,11 +888,11 @@ uint8_t *near_tag_get_iso15693_uid(uint32_t adapter_idx, uint32_t target_idx)
> if (!tag)
> goto fail;
>
> - iso15693_uid = g_try_malloc0(NFC_MAX_ISO15693_UID_LEN);
> + iso15693_uid = g_try_malloc0(tag->iso15693_uid_len);
> if (!iso15693_uid)
> goto fail;
>
> - memcpy(iso15693_uid, tag->iso15693_uid, NFC_MAX_ISO15693_UID_LEN);
> + memcpy(iso15693_uid, tag->iso15693_uid, tag->iso15693_uid_len);
>
> return iso15693_uid;
>
> --
> 2.25.1
> _______________________________________________
> Linux-nfc mailing list -- linux-nfc(a)lists.01.org To unsubscribe send an email to linux-nfc-leave(a)lists.01.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>
> ________________________________
> The information contained in this message may be confidential and legally protected under applicable law. The message is intended solely for the addressee(s). If you are not the intended recipient, you are hereby notified that any use, forwarding, dissemination, or reproduction of this message is strictly prohibited and may be unlawful. If you are not the intended recipient, please contact the sender by return e-mail and destroy all copies of the original message.
>
10 months, 1 week
[PATCH 1/2] MAINTAINERS: nfc: add Krzysztof Kozlowski as maintainer
by Krzysztof Kozlowski
The NFC subsystem is orphaned. I am happy to spend some cycles to
review the patches, send pull requests and in general keep the NFC
subsystem running.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
---
I admit I don't have big experience in NFC part but this will be nice
opportunity to learn something new. I am already maintainer of few
other parts: memory controller drivers, Samsung ARM/ARM64 SoC and some
drviers. I have a kernel.org account and my GPG key is:
https://git.kernel.org/pub/scm/docs/kernel/pgpkeys.git/tree/keys/1B93437D...
Best regards,
Krzysztof
---
MAINTAINERS | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index cc81667e8bab..adc6cbe29f78 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12899,8 +12899,9 @@ F: include/uapi/linux/nexthop.h
F: net/ipv4/nexthop.c
NFC SUBSYSTEM
+M: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
L: netdev(a)vger.kernel.org
-S: Orphan
+S: Maintained
F: Documentation/devicetree/bindings/net/nfc/
F: drivers/nfc/
F: include/linux/platform_data/nfcmrvl.h
--
2.25.1
11 months, 2 weeks
[RESEND PATCH 01/11] nfc: fdp: drop ftrace-like debugging messages
by Krzysztof Kozlowski
Now that the kernel has ftrace, any debugging calls that just do "made
it to this function!" and "leaving this function!" can be removed.
Better to use standard debugging tools.
This allows also to remove several local variables and entire
fdp_nci_recv_frame() function (whose purpose was only to log).
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
---
drivers/nfc/fdp/fdp.c | 31 -------------------------------
drivers/nfc/fdp/fdp.h | 1 -
drivers/nfc/fdp/i2c.c | 12 +-----------
3 files changed, 1 insertion(+), 43 deletions(-)
diff --git a/drivers/nfc/fdp/fdp.c b/drivers/nfc/fdp/fdp.c
index 125d71c27b8b..7863b2536999 100644
--- a/drivers/nfc/fdp/fdp.c
+++ b/drivers/nfc/fdp/fdp.c
@@ -237,28 +237,18 @@ static int fdp_nci_send_patch(struct nci_dev *ndev, u8 conn_id, u8 type)
static int fdp_nci_open(struct nci_dev *ndev)
{
struct fdp_nci_info *info = nci_get_drvdata(ndev);
- struct device *dev = &info->phy->i2c_dev->dev;
-
- dev_dbg(dev, "%s\n", __func__);
return info->phy_ops->enable(info->phy);
}
static int fdp_nci_close(struct nci_dev *ndev)
{
- struct fdp_nci_info *info = nci_get_drvdata(ndev);
- struct device *dev = &info->phy->i2c_dev->dev;
-
- dev_dbg(dev, "%s\n", __func__);
return 0;
}
static int fdp_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
{
struct fdp_nci_info *info = nci_get_drvdata(ndev);
- struct device *dev = &info->phy->i2c_dev->dev;
-
- dev_dbg(dev, "%s\n", __func__);
if (atomic_dec_and_test(&info->data_pkt_counter))
info->data_pkt_counter_cb(ndev);
@@ -266,16 +256,6 @@ static int fdp_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
return info->phy_ops->write(info->phy, skb);
}
-int fdp_nci_recv_frame(struct nci_dev *ndev, struct sk_buff *skb)
-{
- struct fdp_nci_info *info = nci_get_drvdata(ndev);
- struct device *dev = &info->phy->i2c_dev->dev;
-
- dev_dbg(dev, "%s\n", __func__);
- return nci_recv_frame(ndev, skb);
-}
-EXPORT_SYMBOL(fdp_nci_recv_frame);
-
static int fdp_nci_request_firmware(struct nci_dev *ndev)
{
struct fdp_nci_info *info = nci_get_drvdata(ndev);
@@ -476,8 +456,6 @@ static int fdp_nci_setup(struct nci_dev *ndev)
int r;
u8 patched = 0;
- dev_dbg(dev, "%s\n", __func__);
-
r = nci_core_init(ndev);
if (r)
goto error;
@@ -585,9 +563,7 @@ static int fdp_nci_core_reset_ntf_packet(struct nci_dev *ndev,
struct sk_buff *skb)
{
struct fdp_nci_info *info = nci_get_drvdata(ndev);
- struct device *dev = &info->phy->i2c_dev->dev;
- dev_dbg(dev, "%s\n", __func__);
info->setup_reset_ntf = 1;
wake_up(&info->setup_wq);
@@ -598,9 +574,7 @@ static int fdp_nci_prop_patch_ntf_packet(struct nci_dev *ndev,
struct sk_buff *skb)
{
struct fdp_nci_info *info = nci_get_drvdata(ndev);
- struct device *dev = &info->phy->i2c_dev->dev;
- dev_dbg(dev, "%s\n", __func__);
info->setup_patch_ntf = 1;
info->setup_patch_status = skb->data[0];
wake_up(&info->setup_wq);
@@ -773,11 +747,6 @@ EXPORT_SYMBOL(fdp_nci_probe);
void fdp_nci_remove(struct nci_dev *ndev)
{
- struct fdp_nci_info *info = nci_get_drvdata(ndev);
- struct device *dev = &info->phy->i2c_dev->dev;
-
- dev_dbg(dev, "%s\n", __func__);
-
nci_unregister_device(ndev);
nci_free_device(ndev);
}
diff --git a/drivers/nfc/fdp/fdp.h b/drivers/nfc/fdp/fdp.h
index 9bd1f3f23e2d..ead3b21ccae6 100644
--- a/drivers/nfc/fdp/fdp.h
+++ b/drivers/nfc/fdp/fdp.h
@@ -25,6 +25,5 @@ int fdp_nci_probe(struct fdp_i2c_phy *phy, struct nfc_phy_ops *phy_ops,
struct nci_dev **ndev, int tx_headroom, int tx_tailroom,
u8 clock_type, u32 clock_freq, u8 *fw_vsc_cfg);
void fdp_nci_remove(struct nci_dev *ndev);
-int fdp_nci_recv_frame(struct nci_dev *ndev, struct sk_buff *skb);
#endif /* __LOCAL_FDP_H_ */
diff --git a/drivers/nfc/fdp/i2c.c b/drivers/nfc/fdp/i2c.c
index 997e0806821a..c5596e514648 100644
--- a/drivers/nfc/fdp/i2c.c
+++ b/drivers/nfc/fdp/i2c.c
@@ -49,7 +49,6 @@ static int fdp_nci_i2c_enable(void *phy_id)
{
struct fdp_i2c_phy *phy = phy_id;
- dev_dbg(&phy->i2c_dev->dev, "%s\n", __func__);
fdp_nci_i2c_reset(phy);
return 0;
@@ -59,7 +58,6 @@ static void fdp_nci_i2c_disable(void *phy_id)
{
struct fdp_i2c_phy *phy = phy_id;
- dev_dbg(&phy->i2c_dev->dev, "%s\n", __func__);
fdp_nci_i2c_reset(phy);
}
@@ -197,7 +195,6 @@ static int fdp_nci_i2c_read(struct fdp_i2c_phy *phy, struct sk_buff **skb)
static irqreturn_t fdp_nci_i2c_irq_thread_fn(int irq, void *phy_id)
{
struct fdp_i2c_phy *phy = phy_id;
- struct i2c_client *client;
struct sk_buff *skb;
int r;
@@ -206,9 +203,6 @@ static irqreturn_t fdp_nci_i2c_irq_thread_fn(int irq, void *phy_id)
return IRQ_NONE;
}
- client = phy->i2c_dev;
- dev_dbg(&client->dev, "%s\n", __func__);
-
r = fdp_nci_i2c_read(phy, &skb);
if (r == -EREMOTEIO)
@@ -217,7 +211,7 @@ static irqreturn_t fdp_nci_i2c_irq_thread_fn(int irq, void *phy_id)
return IRQ_HANDLED;
if (skb != NULL)
- fdp_nci_recv_frame(phy->ndev, skb);
+ nci_recv_frame(phy->ndev, skb);
return IRQ_HANDLED;
}
@@ -288,8 +282,6 @@ static int fdp_nci_i2c_probe(struct i2c_client *client)
u32 clock_freq;
int r = 0;
- dev_dbg(dev, "%s\n", __func__);
-
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
nfc_err(dev, "No I2C_FUNC_I2C support\n");
return -ENODEV;
@@ -351,8 +343,6 @@ static int fdp_nci_i2c_remove(struct i2c_client *client)
{
struct fdp_i2c_phy *phy = i2c_get_clientdata(client);
- dev_dbg(&client->dev, "%s\n", __func__);
-
fdp_nci_remove(phy->ndev);
fdp_nci_i2c_disable(phy);
--
2.27.0
1 year
Re: [PATCH 01/11] nfc: fdp: drop ftrace-like debugging messages
by Krzysztof Kozlowski
On 29/05/2021 00:13, Jakub Kicinski wrote:
> On Fri, 28 May 2021 10:53:20 -0400 Krzysztof Kozlowski wrote:
>> Now that the kernel has ftrace, any debugging calls that just do "made
>> it to this function!" and "leaving this function!" can be removed.
>> Better to use standard debugging tools.
>>
>> This allows also to remove several local variables and entire
>> fdp_nci_recv_frame() function (whose purpose was only to log).
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
>
> Hi! Would you mind reposting these 11 fixes? build bots don't
> understand series dependencies and the last patch here depends
> on the previous clean up set.
Sure, no problem.
Best regards,
Krzysztof
1 year
[RESEND PATCH] nfc: fix NULL ptr dereference in llcp_sock_getname() after failed connect
by Krzysztof Kozlowski
It's possible to trigger NULL pointer dereference by local unprivileged
user, when calling getsockname() after failed bind() (e.g. the bind
fails because LLCP_SAP_MAX used as SAP):
BUG: kernel NULL pointer dereference, address: 0000000000000000
CPU: 1 PID: 426 Comm: llcp_sock_getna Not tainted 5.13.0-rc2-next-20210521+ #9
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
Call Trace:
llcp_sock_getname+0xb1/0xe0
__sys_getpeername+0x95/0xc0
? lockdep_hardirqs_on_prepare+0xd5/0x180
? syscall_enter_from_user_mode+0x1c/0x40
__x64_sys_getpeername+0x11/0x20
do_syscall_64+0x36/0x70
entry_SYSCALL_64_after_hwframe+0x44/0xae
This can be reproduced with Syzkaller C repro (bind followed by
getpeername):
https://syzkaller.appspot.com/x/repro.c?x=14def446e00000
Cc: <stable(a)vger.kernel.org>
Fixes: d646960f7986 ("NFC: Initial LLCP support")
Reported-by: syzbot+80fb126e7f7d8b1a5914(a)syzkaller.appspotmail.com
Reported-by: butt3rflyh4ck <butterflyhuangxx(a)gmail.com>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
---
Not extensively tested yet but fixes this particular issue.
Reason for resend:
1. Keep it public.
---
net/nfc/llcp_sock.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index 53dbe733f998..6cfd30fc0798 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -110,6 +110,7 @@ static int llcp_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
if (!llcp_sock->service_name) {
nfc_llcp_local_put(llcp_sock->local);
llcp_sock->local = NULL;
+ llcp_sock->dev = NULL;
ret = -ENOMEM;
goto put_dev;
}
@@ -119,6 +120,7 @@ static int llcp_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
llcp_sock->local = NULL;
kfree(llcp_sock->service_name);
llcp_sock->service_name = NULL;
+ llcp_sock->dev = NULL;
ret = -EADDRINUSE;
goto put_dev;
}
--
2.27.0
1 year
[PATCH 01/11] nfc: fdp: drop ftrace-like debugging messages
by Krzysztof Kozlowski
Now that the kernel has ftrace, any debugging calls that just do "made
it to this function!" and "leaving this function!" can be removed.
Better to use standard debugging tools.
This allows also to remove several local variables and entire
fdp_nci_recv_frame() function (whose purpose was only to log).
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
---
drivers/nfc/fdp/fdp.c | 31 -------------------------------
drivers/nfc/fdp/fdp.h | 1 -
drivers/nfc/fdp/i2c.c | 12 +-----------
3 files changed, 1 insertion(+), 43 deletions(-)
diff --git a/drivers/nfc/fdp/fdp.c b/drivers/nfc/fdp/fdp.c
index 125d71c27b8b..7863b2536999 100644
--- a/drivers/nfc/fdp/fdp.c
+++ b/drivers/nfc/fdp/fdp.c
@@ -237,28 +237,18 @@ static int fdp_nci_send_patch(struct nci_dev *ndev, u8 conn_id, u8 type)
static int fdp_nci_open(struct nci_dev *ndev)
{
struct fdp_nci_info *info = nci_get_drvdata(ndev);
- struct device *dev = &info->phy->i2c_dev->dev;
-
- dev_dbg(dev, "%s\n", __func__);
return info->phy_ops->enable(info->phy);
}
static int fdp_nci_close(struct nci_dev *ndev)
{
- struct fdp_nci_info *info = nci_get_drvdata(ndev);
- struct device *dev = &info->phy->i2c_dev->dev;
-
- dev_dbg(dev, "%s\n", __func__);
return 0;
}
static int fdp_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
{
struct fdp_nci_info *info = nci_get_drvdata(ndev);
- struct device *dev = &info->phy->i2c_dev->dev;
-
- dev_dbg(dev, "%s\n", __func__);
if (atomic_dec_and_test(&info->data_pkt_counter))
info->data_pkt_counter_cb(ndev);
@@ -266,16 +256,6 @@ static int fdp_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
return info->phy_ops->write(info->phy, skb);
}
-int fdp_nci_recv_frame(struct nci_dev *ndev, struct sk_buff *skb)
-{
- struct fdp_nci_info *info = nci_get_drvdata(ndev);
- struct device *dev = &info->phy->i2c_dev->dev;
-
- dev_dbg(dev, "%s\n", __func__);
- return nci_recv_frame(ndev, skb);
-}
-EXPORT_SYMBOL(fdp_nci_recv_frame);
-
static int fdp_nci_request_firmware(struct nci_dev *ndev)
{
struct fdp_nci_info *info = nci_get_drvdata(ndev);
@@ -476,8 +456,6 @@ static int fdp_nci_setup(struct nci_dev *ndev)
int r;
u8 patched = 0;
- dev_dbg(dev, "%s\n", __func__);
-
r = nci_core_init(ndev);
if (r)
goto error;
@@ -585,9 +563,7 @@ static int fdp_nci_core_reset_ntf_packet(struct nci_dev *ndev,
struct sk_buff *skb)
{
struct fdp_nci_info *info = nci_get_drvdata(ndev);
- struct device *dev = &info->phy->i2c_dev->dev;
- dev_dbg(dev, "%s\n", __func__);
info->setup_reset_ntf = 1;
wake_up(&info->setup_wq);
@@ -598,9 +574,7 @@ static int fdp_nci_prop_patch_ntf_packet(struct nci_dev *ndev,
struct sk_buff *skb)
{
struct fdp_nci_info *info = nci_get_drvdata(ndev);
- struct device *dev = &info->phy->i2c_dev->dev;
- dev_dbg(dev, "%s\n", __func__);
info->setup_patch_ntf = 1;
info->setup_patch_status = skb->data[0];
wake_up(&info->setup_wq);
@@ -773,11 +747,6 @@ EXPORT_SYMBOL(fdp_nci_probe);
void fdp_nci_remove(struct nci_dev *ndev)
{
- struct fdp_nci_info *info = nci_get_drvdata(ndev);
- struct device *dev = &info->phy->i2c_dev->dev;
-
- dev_dbg(dev, "%s\n", __func__);
-
nci_unregister_device(ndev);
nci_free_device(ndev);
}
diff --git a/drivers/nfc/fdp/fdp.h b/drivers/nfc/fdp/fdp.h
index 9bd1f3f23e2d..ead3b21ccae6 100644
--- a/drivers/nfc/fdp/fdp.h
+++ b/drivers/nfc/fdp/fdp.h
@@ -25,6 +25,5 @@ int fdp_nci_probe(struct fdp_i2c_phy *phy, struct nfc_phy_ops *phy_ops,
struct nci_dev **ndev, int tx_headroom, int tx_tailroom,
u8 clock_type, u32 clock_freq, u8 *fw_vsc_cfg);
void fdp_nci_remove(struct nci_dev *ndev);
-int fdp_nci_recv_frame(struct nci_dev *ndev, struct sk_buff *skb);
#endif /* __LOCAL_FDP_H_ */
diff --git a/drivers/nfc/fdp/i2c.c b/drivers/nfc/fdp/i2c.c
index 997e0806821a..c5596e514648 100644
--- a/drivers/nfc/fdp/i2c.c
+++ b/drivers/nfc/fdp/i2c.c
@@ -49,7 +49,6 @@ static int fdp_nci_i2c_enable(void *phy_id)
{
struct fdp_i2c_phy *phy = phy_id;
- dev_dbg(&phy->i2c_dev->dev, "%s\n", __func__);
fdp_nci_i2c_reset(phy);
return 0;
@@ -59,7 +58,6 @@ static void fdp_nci_i2c_disable(void *phy_id)
{
struct fdp_i2c_phy *phy = phy_id;
- dev_dbg(&phy->i2c_dev->dev, "%s\n", __func__);
fdp_nci_i2c_reset(phy);
}
@@ -197,7 +195,6 @@ static int fdp_nci_i2c_read(struct fdp_i2c_phy *phy, struct sk_buff **skb)
static irqreturn_t fdp_nci_i2c_irq_thread_fn(int irq, void *phy_id)
{
struct fdp_i2c_phy *phy = phy_id;
- struct i2c_client *client;
struct sk_buff *skb;
int r;
@@ -206,9 +203,6 @@ static irqreturn_t fdp_nci_i2c_irq_thread_fn(int irq, void *phy_id)
return IRQ_NONE;
}
- client = phy->i2c_dev;
- dev_dbg(&client->dev, "%s\n", __func__);
-
r = fdp_nci_i2c_read(phy, &skb);
if (r == -EREMOTEIO)
@@ -217,7 +211,7 @@ static irqreturn_t fdp_nci_i2c_irq_thread_fn(int irq, void *phy_id)
return IRQ_HANDLED;
if (skb != NULL)
- fdp_nci_recv_frame(phy->ndev, skb);
+ nci_recv_frame(phy->ndev, skb);
return IRQ_HANDLED;
}
@@ -288,8 +282,6 @@ static int fdp_nci_i2c_probe(struct i2c_client *client)
u32 clock_freq;
int r = 0;
- dev_dbg(dev, "%s\n", __func__);
-
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
nfc_err(dev, "No I2C_FUNC_I2C support\n");
return -ENODEV;
@@ -351,8 +343,6 @@ static int fdp_nci_i2c_remove(struct i2c_client *client)
{
struct fdp_i2c_phy *phy = i2c_get_clientdata(client);
- dev_dbg(&client->dev, "%s\n", __func__);
-
fdp_nci_remove(phy->ndev);
fdp_nci_i2c_disable(phy);
--
2.27.0
1 year
[PATCH 01/12] nfc: fdp: correct kerneldoc for structure
by Krzysztof Kozlowski
Since structure comments are not kerneldoc, remove the double ** to fix
W=1 warnings:
warning: This comment starts with '/**', but isn't a kernel-doc comment.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
---
drivers/nfc/fdp/fdp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/nfc/fdp/fdp.c b/drivers/nfc/fdp/fdp.c
index fe0719ed81a0..125d71c27b8b 100644
--- a/drivers/nfc/fdp/fdp.c
+++ b/drivers/nfc/fdp/fdp.c
@@ -149,7 +149,7 @@ static void fdp_nci_send_patch_cb(struct nci_dev *ndev)
wake_up(&info->setup_wq);
}
-/**
+/*
* Register a packet sent counter and a callback
*
* We have no other way of knowing when all firmware packets were sent out
@@ -167,7 +167,7 @@ static void fdp_nci_set_data_pkt_counter(struct nci_dev *ndev,
info->data_pkt_counter_cb = cb;
}
-/**
+/*
* The device is expecting a stream of packets. All packets need to
* have the PBF flag set to 0x0 (last packet) even if the firmware
* file is segmented and there are multiple packets. If we give the
--
2.27.0
1 year
Re: [PATCH v2] nfc: st-nci: remove unnecessary labels
by Krzysztof Kozlowski
On 25/05/2021 21:16, samirweng1979 wrote:
> From: wengjianfeng <wengjianfeng(a)yulong.com>
>
> Some labels are only used once, so we delete them and use the
> return statement instead of the goto statement.
>
> Signed-off-by: wengjianfeng <wengjianfeng(a)yulong.com>
> ---
> drivers/nfc/st-nci/vendor_cmds.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
Best regards,
Krzysztof
1 year, 1 month
Re: [PATCH v2] nfc: st95hf: remove unnecessary assignment and label
by Krzysztof Kozlowski
On 25/05/2021 20:56, samirweng1979 wrote:
> From: wengjianfeng <wengjianfeng(a)yulong.com>
>
> In function st95hf_in_send_cmd, the variable rc is assigned then goto
> error label, which just returns rc, so we use return to replace it.
> Since error label only used once in the function, so we remove error label.
>
> Signed-off-by: wengjianfeng <wengjianfeng(a)yulong.com>
> ---
> drivers/nfc/st95hf/core.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
Best regards,
Krzysztof
1 year, 1 month
Re: [PATCH] nfc: st-nci: remove unnecessary labels
by Krzysztof Kozlowski
On 24/05/2021 23:12, samirweng1979 wrote:
> From: wengjianfeng <wengjianfeng(a)yulong.com>
>
> In some functions, use goto exit statement, which just return r.
This is not parseable. Imperative "use goto" suggest that it's the
change you make.
> and exit label only used once in each funciton, so we use return
Every sentence in English starts with capital letter. They however
cannto start with "and" because it joins two parts of sentences.
s/funciton/use the spellcheck please/
The code looks good, but please spend some time to make the description
understandable. You can also make it very short, to avoid complicated
sentences. Look for examples in Linux kernel how other people wrote it.
Best regards,
Krzysztof
1 year, 1 month