Hi Waldemar,
On 09/06/2012 12:22 PM, Waldemar Rymarkiewicz wrote:
> Checksum is specific for a chip spcification and it varies
> (in size and type) between different hardware. It should be
> handled in the driver then.
>
> Moreover, shdlc spec doesn't mention crc as a part of the frame.
>
> Update pn544_hci driver as well.
>
> Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@xxxxxxxxx>
> ---
> drivers/nfc/pn544_hci.c | 19 ++++++++++++++++++-
> net/nfc/hci/shdlc.c | 27 ++-------------------------
> 2 files changed, 20 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/nfc/pn544_hci.c b/drivers/nfc/pn544_hci.c
> index 9458d53..4e0716c 100644
> --- a/drivers/nfc/pn544_hci.c
> +++ b/drivers/nfc/pn544_hci.c
> @@ -128,6 +128,7 @@ static struct nfc_hci_gate pn544_gates[] = {
>
> /* Largest headroom needed for outgoing custom commands */
> #define PN544_CMDS_HEADROOM 2
> +#define PN544_CMDS_TAILROOM 2
>
> struct pn544_hci_info {
> struct i2c_client *i2c_dev;
> @@ -576,6 +577,20 @@ static int pn544_hci_ready(struct nfc_shdlc *shdlc)
> return 0;
> }
>
> +static void pn544_hci_add_len_crc(struct sk_buff *skb)
> +{
> + u16 crc;
> + int len;
> +
> + len = skb->len + 2;
> + *skb_push(skb, 1) = len;
> +
> + crc = crc_ccitt(0xffff, skb->data, skb->len);
> + crc = ~crc;
> + *skb_put(skb, 1) = crc & 0xff;
> + *skb_put(skb, 1) = crc >> 8;
> +}
> +
> static int pn544_hci_xmit(struct nfc_shdlc *shdlc, struct sk_buff *skb)
> {
> struct pn544_hci_info *info = nfc_shdlc_get_clientdata(shdlc);
> @@ -584,6 +599,8 @@ static int pn544_hci_xmit(struct nfc_shdlc *shdlc, struct sk_buff *skb)
> if (info->hard_fault != 0)
> return info->hard_fault;
>
> + pn544_hci_add_len_crc(skb);
> +
> return pn544_hci_i2c_write(client, skb->data, skb->len);
> }
>
> @@ -874,7 +891,7 @@ static int __devinit pn544_hci_probe(struct i2c_client *client,
>
> info->shdlc = nfc_shdlc_allocate(&pn544_shdlc_ops,
> &init_data, protocols,
> - PN544_CMDS_HEADROOM, 0,
> + PN544_CMDS_HEADROOM, PN544_CMDS_TAILROOM,
> PN544_HCI_LLC_MAX_PAYLOAD,
> dev_name(&client->dev));
> if (!info->shdlc) {
> diff --git a/net/nfc/hci/shdlc.c b/net/nfc/hci/shdlc.c
> index d4fe5e9..8a5f034 100644
> --- a/net/nfc/hci/shdlc.c
> +++ b/net/nfc/hci/shdlc.c
> @@ -22,7 +22,6 @@
> #include <linux/sched.h>
> #include <linux/export.h>
> #include <linux/wait.h>
> -#include <linux/crc-ccitt.h>
> #include <linux/slab.h>
> #include <linux/skbuff.h>
>
> @@ -30,7 +29,6 @@
> #include <net/nfc/shdlc.h>
>
> #define SHDLC_LLC_HEAD_ROOM 2
> -#define SHDLC_LLC_TAIL_ROOM 2
>
> #define SHDLC_MAX_WINDOW 4
> #define SHDLC_SREJ_SUPPORT false
> @@ -94,28 +92,13 @@ static struct sk_buff *nfc_shdlc_alloc_skb(struct nfc_shdlc *shdlc,
> struct sk_buff *skb;
>
> skb = alloc_skb(shdlc->client_headroom + SHDLC_LLC_HEAD_ROOM +
> - shdlc->client_tailroom + SHDLC_LLC_TAIL_ROOM +
> - payload_len, GFP_KERNEL);
> + shdlc->client_tailroom + payload_len, GFP_KERNEL);
> if (skb)
> skb_reserve(skb, shdlc->client_headroom + SHDLC_LLC_HEAD_ROOM);
>
> return skb;
> }
>
> -static void nfc_shdlc_add_len_crc(struct sk_buff *skb)
> -{
> - u16 crc;
> - int len;
> -
> - len = skb->len + 2;
> - *skb_push(skb, 1) = len;
> -
> - crc = crc_ccitt(0xffff, skb->data, skb->len);
> - crc = ~crc;
> - *skb_put(skb, 1) = crc & 0xff;
> - *skb_put(skb, 1) = crc >> 8;
> -}
> -
> /* immediately sends an S frame. */
> static int nfc_shdlc_send_s_frame(struct nfc_shdlc *shdlc,
> enum sframe_type sframe_type, int nr)
> @@ -131,8 +114,6 @@ static int nfc_shdlc_send_s_frame(struct nfc_shdlc *shdlc,
>
> *skb_push(skb, 1) = SHDLC_CONTROL_HEAD_S | (sframe_type << 3) | nr;
>
> - nfc_shdlc_add_len_crc(skb);
> -
> r = shdlc->ops->xmit(shdlc, skb);
>
> kfree_skb(skb);
> @@ -151,8 +132,6 @@ static int nfc_shdlc_send_u_frame(struct nfc_shdlc *shdlc,
>
> *skb_push(skb, 1) = SHDLC_CONTROL_HEAD_U | uframe_modifier;
>
> - nfc_shdlc_add_len_crc(skb);
> -
> r = shdlc->ops->xmit(shdlc, skb);
>
> kfree_skb(skb);
> @@ -510,8 +489,6 @@ static void nfc_shdlc_handle_send_queue(struct nfc_shdlc *shdlc)
> shdlc->nr);
> /* SHDLC_DUMP_SKB("shdlc frame written", skb); */
>
> - nfc_shdlc_add_len_crc(skb);
> -
> r = shdlc->ops->xmit(shdlc, skb);
> if (r < 0) {
> shdlc->hard_fault = r;
> @@ -881,7 +858,7 @@ struct nfc_shdlc *nfc_shdlc_allocate(struct nfc_shdlc_ops *ops,
>
> shdlc->hdev = nfc_hci_allocate_device(&shdlc_ops, init_data, protocols,
> tx_headroom + SHDLC_LLC_HEAD_ROOM,
> - tx_tailroom + SHDLC_LLC_TAIL_ROOM,
> + tx_tailroom,
> max_link_payload);
> if (shdlc->hdev == NULL)
> goto err_allocdev;
>
You are certainly right that the CRC belongs to the driver.
Acked-by: Eric Lapuyade <eric.lapuyade@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
