Hi Clement,
On Thu, Jan 22, 2015 at 04:27:39PM +0100, clement.perrochaud@xxxxxxxxxxxx wrote:
> From: Clément Perrochaud <clement.perrochaud@xxxxxxx>
>
> Signed-off-by: Clément Perrochaud <clement.perrochaud@xxxxxxx>
> Signed-off-by: Clément Perrochaud <clement.perrochaud@xxxxxxxxxxxx>
Are you sure you want both S-O-B lines ?
> +static int nxp_nci_i2c_fw_read(struct nxp_nci_i2c_phy *phy,
> + struct sk_buff **skb)
> +{
> + struct i2c_client *client = phy->i2c_dev;
> + u16 header;
> + size_t frame_len;
> + int r;
> +
> + r = i2c_master_recv(client, (u8 *) &header, NXP_NCI_FW_HDR_LEN);
> + if (r < 0) {
> + goto fw_read_exit;
> + } else if (r != NXP_NCI_FW_HDR_LEN) {
> + nfc_err(&client->dev, "Incorrect header length: %u\n", r);
> + r = -EBADMSG;
> + goto fw_read_exit;
> + }
> +
> + frame_len = (get_unaligned_be16(&header) & NXP_NCI_FW_FRAME_LEN_MASK) +
> + NXP_NCI_FW_CRC_LEN;
> +
> + *skb = alloc_skb(NXP_NCI_FW_HDR_LEN + frame_len, GFP_KERNEL);
> + if (*skb == NULL) {
> + r = -ENOMEM;
> + goto fw_read_exit;
> + }
> +
> + memcpy(skb_put(*skb, NXP_NCI_FW_HDR_LEN), &header, NXP_NCI_FW_HDR_LEN);
> +
> + r = i2c_master_recv(client, skb_put(*skb, frame_len), frame_len);
> + if (r != frame_len) {
> + nfc_err(&client->dev,
> + "Invalid frame length: %u (expected %u)\n",
> + r, frame_len);
> + r = -EBADMSG;
> + goto fw_read_exit_free_skb;
> + }
> +
> + r = 0;
> + goto fw_read_exit;
return 0; is enough.
> +static int nxp_nci_i2c_nci_read(struct nxp_nci_i2c_phy *phy,
> + struct sk_buff **skb)
> +{
> + struct nci_ctrl_hdr header; /* May actually be a data header */
> + struct i2c_client *client = phy->i2c_dev;
> + int r;
> +
> + r = i2c_master_recv(client, (u8 *) &header, NCI_CTRL_HDR_SIZE);
> + if (r < 0) {
> + goto nci_read_exit;
> + } else if (r != NCI_CTRL_HDR_SIZE) {
> + nfc_err(&client->dev, "Incorrect header length: %u\n", r);
> + r = -EBADMSG;
> + goto nci_read_exit;
> + }
> +
> + *skb = alloc_skb(NCI_CTRL_HDR_SIZE + header.plen, GFP_KERNEL);
> + if (*skb == NULL) {
> + r = -ENOMEM;
> + goto nci_read_exit;
> + }
> +
> + memcpy(skb_put(*skb, NCI_CTRL_HDR_SIZE), (void *) &header,
> + NCI_CTRL_HDR_SIZE);
> +
> + r = i2c_master_recv(client, skb_put(*skb, header.plen), header.plen);
> + if (r != header.plen) {
> + nfc_err(&client->dev,
> + "Invalid frame payload length: %u (expected %u)\n",
> + r, header.plen);
> + r = -EBADMSG;
> + goto nci_read_exit_free_skb;
> + }
> +
> + r = 0;
> + goto nci_read_exit;
Ditto.
> +static irqreturn_t nxp_nci_i2c_irq_thread_fn(int irq, void *phy_id)
> +{
> + struct nxp_nci_i2c_phy *phy = phy_id;
> + struct i2c_client *client;
> + struct nxp_nci_info *info;
> +
> + struct sk_buff *skb = NULL;
> + int r = 0;
> +
> + if (!phy || !phy->ndev)
> + goto exit_irq_none;
> +
> + client = phy->i2c_dev;
> +
> + if (!client || irq != client->irq)
> + goto exit_irq_none;
> +
> + if (phy->hard_fault != 0)
> + goto exit_irq_handled;
> +
> + info = nci_get_drvdata(phy->ndev);
You probably want to take the info_lock mutex here.
> + switch (info->mode) {
> + case NXP_NCI_MODE_NCI:
> + r = nxp_nci_i2c_nci_read(phy, &skb);
> + break;
> + case NXP_NCI_MODE_FW:
> + r = nxp_nci_i2c_fw_read(phy, &skb);
> + break;
> + case NXP_NCI_MODE_COLD:
> + r = -EREMOTEIO;
> + break;
> + }
> +
> + if (r == -EREMOTEIO) {
> + phy->hard_fault = r;
> + skb = NULL;
> + } else if (r < 0) {
> + nfc_err(&client->dev, "Read failed with error %d\n", r);
> + goto exit_irq_handled;
> + }
> +
> + switch (info->mode) {
> + case NXP_NCI_MODE_NCI:
> + nci_recv_frame(phy->ndev, skb);
> + break;
> + case NXP_NCI_MODE_FW:
> + nxp_nci_fw_recv_frame(phy->ndev, skb);
> + break;
> + case NXP_NCI_MODE_COLD:
> + break;
> + }
> +
> +exit_irq_handled:
> + return IRQ_HANDLED;
> +exit_irq_none:
> + WARN_ON_ONCE(1);
> + return IRQ_NONE;
> +}
> +static int nxp_nci_i2c_request_gpios(struct nxp_nci_i2c_phy *phy)
> +{
> + int r;
> +
> + r = gpio_request(phy->gpio_en, "nxp_nci_en");
Please use the devm_gpiod_* API so that you don't need to free GPIOs
explicitely.
> + if (r < 0)
> + goto err_out;
> +
> + r = gpio_direction_output(phy->gpio_en, 0);
> + if (r < 0)
> + goto err_gpio_en;
> +
> + r = gpio_request(phy->gpio_fw, "nxp_nci_fw");
> + if (r < 0)
> + goto err_gpio_en;
> +
> + r = gpio_direction_output(phy->gpio_fw, 0);
> + if (r < 0)
> + goto err_gpio_fw;
> +
> + goto err_out;
> +
> +err_gpio_fw:
> + gpio_free(phy->gpio_fw);
> +err_gpio_en:
> + gpio_free(phy->gpio_en);
> +err_out:
> + return r;
> +}
> +
> +static void nxp_nci_i2c_free_gpios(struct nxp_nci_i2c_phy *phy)
> +{
> + gpio_free(phy->gpio_fw);
> + gpio_free(phy->gpio_en);
> +}
You would no longer need that one with devm_gpiod_*.
--
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
