Hi Ilan,
On Wed, 2012-01-11 at 17:40 +0200, ilanelias78@xxxxxxxxx wrote:
> From: Ilan Elias <ilane@xxxxxx>
>
> Download TI NFC init script during nfcwilink open operation,
> after the NFC channel is registered with TI shared transport.
> TI NFC init script is written in BTS format.
> First, read the chip version via a special vendor specific command.
> Second, we request the relevant BTS file from the user space, and
> then send the BTS commands to the chip.
Some nitpicks again:
> +static int nfcwilink_download_fw(struct nfcwilink *drv)
> +{
> + unsigned char file_name[BTS_FILE_NAME_MAX_SIZE];
> + const struct firmware *fw;
> + __u16 action_type, action_len;
> + __u8 *ptr;
> + int len, rc;
> +
> + nfc_dev_dbg(&drv->pdev->dev, "download_fw entry");
> +
> + set_bit(NFCWILINK_FW_DOWNLOAD, &drv->flags);
> +
> + rc = nfcwilink_get_bts_file_name(drv, file_name);
> + if (rc)
> + goto exit;
> +
> + rc = request_firmware(&fw, file_name, &drv->pdev->dev);
> + if (rc) {
> + nfc_dev_err(&drv->pdev->dev, "request_firmware failed %d", rc);
> +
> + /* if the file is not found, don't exit with failure */
> + if (rc == -ENOENT)
> + rc = 0;
> +
> + goto exit;
> + }
> +
> + len = fw->size;
> + ptr = (__u8 *)fw->data;
> +
> + if ((len == 0) || (ptr == NULL)) {
> + nfc_dev_dbg(&drv->pdev->dev,
> + "request_firmware returned size %d", len);
> + goto release_fw;
> + }
> +
> + if (__le32_to_cpu(((struct bts_file_hdr *)ptr)->magic) !=
> + BTS_FILE_HDR_MAGIC) {
> + nfc_dev_err(&drv->pdev->dev, "wrong bts magic number");
> + rc = -EINVAL;
> + goto release_fw;
> + }
> +
> + /* remove the BTS header */
> + len -= sizeof(struct bts_file_hdr);
> + ptr += sizeof(struct bts_file_hdr);
> +
> + while (len > 0) {
> + action_type =
> + __le16_to_cpu(((struct bts_file_action *)ptr)->type);
> + action_len =
> + __le16_to_cpu(((struct bts_file_action *)ptr)->len);
> +
> + nfc_dev_dbg(&drv->pdev->dev, "bts_file_action type %d, len %d",
> + action_type, action_len);
> +
> + switch (action_type) {
> + case BTS_FILE_ACTION_TYPE_SEND_CMD:
> + rc = nfcwilink_send_bts_cmd(drv,
> + ((struct bts_file_action *)ptr)->data,
> + action_len);
> + if (rc)
> + goto release_fw;
> + break;
> + }
> +
> + /* advance to the next action */
> + len -= (sizeof(struct bts_file_action) + action_len);
> + ptr += (sizeof(struct bts_file_action) + action_len);
> + }
I would somehow agree with Ohad here by saying that the BTS handling
could (even though this one seems to be a simplified one) be factorized
somehow. But I'm not going to comment any further on the ST code ;)
> @@ -208,11 +489,13 @@ static int nfcwilink_send(struct sk_buff *skb)
>
> nfc_dev_dbg(&drv->pdev->dev, "send entry, len %d", skb->len);
>
> - if (!test_bit(NFCWILINK_RUNNING, &drv->flags))
> - return -EBUSY;
> + if (!test_bit(NFCWILINK_RUNNING, &drv->flags)) {
> + kfree_skb(skb);
> + return -EINVAL;
> + }
This is not related to this patch.
> /* add the ST hdr to the start of the buffer */
> - hdr.len = skb->len;
> + hdr.len = cpu_to_le16(skb->len);
Same here.
> memcpy(skb_push(skb, NFCWILINK_HDR_LEN), &hdr, NFCWILINK_HDR_LEN);
>
> /* Insert skb to shared transport layer's transmit queue.
> @@ -239,7 +522,7 @@ static int nfcwilink_probe(struct platform_device *pdev)
> {
> static struct nfcwilink *drv;
> int rc;
> - u32 protocols;
> + __u32 protocols;
Ditto.
Cheers,
Samuel.
--
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
