[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201124144353.7c759cae@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date: Tue, 24 Nov 2020 14:43:53 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Bongsu Jeon <bongsu.jeon@...sung.com>
Cc: "davem@...emloft.net" <davem@...emloft.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
linux-nfc@...ts.01.org
Subject: Re: [PATCH net-next v2] net/nfc/nci: Support NCI 2.x initial
sequence
On Mon, 23 Nov 2020 19:12:08 +0900 Bongsu Jeon wrote:
> implement the NCI 2.x initial sequence to support NCI 2.x NFCC.
> Since NCI 2.0, CORE_RESET and CORE_INIT sequence have been changed.
> If NFCEE supports NCI 2.x, then NCI 2.x initial sequence will work.
>
> In NCI 1.0, Initial sequence and payloads are as below:
> (DH) (NFCC)
> | -- CORE_RESET_CMD --> |
> | <-- CORE_RESET_RSP -- |
> | -- CORE_INIT_CMD --> |
> | <-- CORE_INIT_RSP -- |
> CORE_RESET_RSP payloads are Status, NCI version, Configuration Status.
> CORE_INIT_CMD payloads are empty.
> CORE_INIT_RSP payloads are Status, NFCC Features,
> Number of Supported RF Interfaces, Supported RF Interface,
> Max Logical Connections, Max Routing table Size,
> Max Control Packet Payload Size, Max Size for Large Parameters,
> Manufacturer ID, Manufacturer Specific Information.
>
> In NCI 2.0, Initial Sequence and Parameters are as below:
> (DH) (NFCC)
> | -- CORE_RESET_CMD --> |
> | <-- CORE_RESET_RSP -- |
> | <-- CORE_RESET_NTF -- |
> | -- CORE_INIT_CMD --> |
> | <-- CORE_INIT_RSP -- |
> CORE_RESET_RSP payloads are Status.
> CORE_RESET_NTF payloads are Reset Trigger,
> Configuration Status, NCI Version, Manufacturer ID,
> Manufacturer Specific Information Length,
> Manufacturer Specific Information.
> CORE_INIT_CMD payloads are Feature1, Feature2.
> CORE_INIT_RSP payloads are Status, NFCC Features,
> Max Logical Connections, Max Routing Table Size,
> Max Control Packet Payload Size,
> Max Data Packet Payload Size of the Static HCI Connection,
> Number of Credits of the Static HCI Connection,
> Max NFC-V RF Frame Size, Number of Supported RF Interfaces,
> Supported RF Interfaces.
>
> Signed-off-by: Bongsu Jeon <bongsu.jeon@...sung.com>
NFC folks, looks like when the NFC core got orphaned it lost all links
in MAINTAINERS. Should we add the L: linux-nfc@...ts.01.org so that
there is a better chance that someone knowledgeable will provide
reviews?
Also if anyone is up for it feel free to add your M: or R: entries!
> #define NCI_OP_CORE_INIT_CMD nci_opcode_pack(NCI_GID_CORE, 0x01)
> +/* To support NCI 2.x */
> +struct nci_core_init_v2_cmd {
> + unsigned char feature1;
> + unsigned char feature2;
> +} __packed;
No need for this to be packed.
> #define NCI_OP_CORE_SET_CONFIG_CMD nci_opcode_pack(NCI_GID_CORE, 0x02)
> struct set_config_param {
> @@ -316,6 +326,11 @@ struct nci_core_reset_rsp {
> __u8 config_status;
> } __packed;
>
> +/* To support NCI ver 2.x */
> +struct nci_core_reset_rsp_nci_ver2 {
> + unsigned char status;
> +} __packed;
ditto
> #define NCI_OP_CORE_INIT_RSP nci_opcode_pack(NCI_GID_CORE, 0x01)
> struct nci_core_init_rsp_1 {
> __u8 status;
> @@ -334,6 +349,20 @@ struct nci_core_init_rsp_2 {
> __le32 manufact_specific_info;
> } __packed;
>
> +/* To support NCI ver 2.x */
> +struct nci_core_init_rsp_nci_ver2 {
> + unsigned char status;
> + __le32 nfcc_features;
> + unsigned char max_logical_connections;
> + __le16 max_routing_table_size;
> + unsigned char max_ctrl_pkt_payload_len;
> + unsigned char max_data_pkt_hci_payload_len;
> + unsigned char number_of_hci_credit;
> + __le16 max_nfc_v_frame_size;
> + unsigned char num_supported_rf_interfaces;
> + unsigned char supported_rf_interfaces[];
> +} __packed;
> +
> #define NCI_OP_CORE_SET_CONFIG_RSP nci_opcode_pack(NCI_GID_CORE, 0x02)
> struct nci_core_set_config_rsp {
> __u8 status;
> @@ -372,6 +401,16 @@ struct nci_nfcee_discover_rsp {
> /* --------------------------- */
> /* ---- NCI Notifications ---- */
> /* --------------------------- */
> +#define NCI_OP_CORE_RESET_NTF nci_opcode_pack(NCI_GID_CORE, 0x00)
> +struct nci_core_reset_ntf {
> + unsigned char reset_trigger;
> + unsigned char config_status;
> + unsigned char nci_ver;
> + unsigned char manufact_id;
> + unsigned char manufacturer_specific_len;
> + __le32 manufact_specific_info;
> +} __packed;
> +
> #define NCI_OP_CORE_CONN_CREDITS_NTF nci_opcode_pack(NCI_GID_CORE, 0x06)
> struct conn_credit_entry {
> __u8 conn_id;
> diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
> index 4953ee5146e1..68889faadda2 100644
> --- a/net/nfc/nci/core.c
> +++ b/net/nfc/nci/core.c
> @@ -165,7 +165,14 @@ static void nci_reset_req(struct nci_dev *ndev, unsigned long opt)
>
> static void nci_init_req(struct nci_dev *ndev, unsigned long opt)
> {
> - nci_send_cmd(ndev, NCI_OP_CORE_INIT_CMD, 0, NULL);
> + struct nci_core_init_v2_cmd *cmd = (struct nci_core_init_v2_cmd *)opt;
> +
> + if (!cmd) {
> + nci_send_cmd(ndev, NCI_OP_CORE_INIT_CMD, 0, NULL);
> + } else {
> + /* if nci version is 2.0, then use the feature parameters */
> + nci_send_cmd(ndev, NCI_OP_CORE_INIT_CMD, sizeof(struct nci_core_init_v2_cmd), cmd);
Please wrap this line.
> + }
Parenthesis unnecessary.
> }
>
> static void nci_init_complete_req(struct nci_dev *ndev, unsigned long opt)
> +static unsigned char nci_core_init_rsp_packet_v2(struct nci_dev *ndev, struct sk_buff *skb)
> +{
> + struct nci_core_init_rsp_nci_ver2 *rsp = (void *)skb->data;
> + unsigned char rf_interface_idx = 0;
> + unsigned char rf_extension_cnt = 0;
> + unsigned char *supported_rf_interface = rsp->supported_rf_interfaces;
> +
> + pr_debug("status %x\n", rsp->status);
> +
> + if (rsp->status != NCI_STATUS_OK)
> + return rsp->status;
> +
> + ndev->nfcc_features = __le32_to_cpu(rsp->nfcc_features);
> + ndev->num_supported_rf_interfaces = rsp->num_supported_rf_interfaces;
> +
> + if (ndev->num_supported_rf_interfaces >
> + NCI_MAX_SUPPORTED_RF_INTERFACES) {
> + ndev->num_supported_rf_interfaces =
> + NCI_MAX_SUPPORTED_RF_INTERFACES;
> + }
brackets unnecessary unnecessary
also:
ndev->num_supported_rf_interfaces =
min(ndev->num_supported_rf_interfaces,
NCI_MAX_SUPPORTED_RF_INTERFACES);
> + while (rf_interface_idx < ndev->num_supported_rf_interfaces) {
> + ndev->supported_rf_interfaces[rf_interface_idx++] = *supported_rf_interface++;
> +
> + /* skip rf extension parameters */
> + rf_extension_cnt = *supported_rf_interface++;
> + supported_rf_interface += rf_extension_cnt;
> + }
Powered by blists - more mailing lists