RE: [PATCH v42 5/7] firmware: imx: adds miscdev

[Pankaj Gupta (OSS)]

> se_msg_hdr *header,
> > +                       u32 tx_msg_sz)
> > +{
> > +   struct se_api_msg *msg = container_of(header, struct se_api_msg,
> header);
> > +   struct se_if_priv *priv = dev_ctx->priv;
> > +   bool is_cmd_receiver = false;
> > +
> > +   scoped_guard(mutex, &priv->modify_lock)
> > +           if (dev_ctx == priv->cmd_receiver_clbk_hdl.dev_ctx)
> > +                   is_cmd_receiver = true;
> > +
> > +   if (!is_cmd_receiver)
> > +           return -EINVAL;
>
> is_cmd_receiver only use once
>
>       if (dev_ctx != priv->cmd_receiver_clbk_hdl.dev_ctx)
>               return -EINVAL;
>
> so is_cmd_receiver can be removed.
Accepted will be removed in v43.

>
> > +
> > +   return se_cmd_receiver_allowed_rsp(dev_ctx, msg, tx_msg_sz); }
> > +
> ...
> > +int se_close_session(struct se_if_device_ctx *dev_ctx, u32
> > +session_hdl) {
> > +   struct se_api_msg *tx_msg __free(kfree) = NULL;
> > +   struct se_api_msg *rx_msg __free(kfree) = NULL;
> > +   struct se_if_priv *priv;
> > +   int ret;
> > +
> > +   if (!dev_ctx || !dev_ctx->priv)
> > +           return -EINVAL;
> > +
> > +   priv = dev_ctx->priv;
> > +
> > +   tx_msg = kzalloc(ELE_SESSION_CLOSE_REQ_SZ, GFP_KERNEL);
>
> cleanup prefer declear varible here
>
>       struct se_api_msg *tx_msg __free(kfree)  =
>               kzalloc(ELE_SESSION_CLOSE_REQ_SZ, GFP_KERNEL);
>
> > +   if (!tx_msg)
> > +           return -ENOMEM;
> > +
> > +   rx_msg = kzalloc(ELE_SESSION_CLOSE_RSP_SZ, GFP_KERNEL);
> > +   if (!rx_msg)
> > +           return -ENOMEM;
>
> the same here. Check other place.
>
Accepted will be removed in v43.

> > +
> > +   /*
> > +    * Session close is a FW-API command; format it with the FW API
> version
> > +    * so se_val_rsp_hdr_n_status() below (called with is_base_api =
> false,
> > +    * i.e. expecting fw_api_ver) does not reject the matching response
> and
> > +    * wrongly report the close as failed, which would leak the handle.
> > +    */
> > +   se_fill_cmd_msg_hdr(priv, (struct se_msg_hdr *)&tx_msg->header,
> > +                       ELE_SESSION_CLOSE_REQ,
> ELE_SESSION_CLOSE_REQ_SZ, false);
> > +
> > +   tx_msg->data[0] = session_hdl;
> > +
> > +   /*
> > +    * Transmit on the caller's own context. Using dev_ctx (rather than
> > +    * hardcoding priv->priv_dev_ctx) keeps a userspace close() subject
> to
> > +    * the going_away check in ele_msg_send_rcv(): if unbind has begun
> and
> > +    * freed priv->tx_chan, the send is rejected with -ENODEV instead of
> > +    * touching the freed mailbox channel. The teardown path passes
> > +    * priv_dev_ctx so its resync closes are still let through.
> > +    */
> > +   ret = ele_msg_send_rcv(dev_ctx,
> > +                          tx_msg,
> > +                          ELE_SESSION_CLOSE_REQ_SZ,
> > +                          rx_msg,
> > +                          ELE_SESSION_CLOSE_RSP_SZ);
> > +   if (ret < 0)
> > +           return ret;
> > +
> > +   ret = se_val_rsp_hdr_n_status(priv,
> > +                                 rx_msg,
> > +                                 ELE_SESSION_CLOSE_REQ,
> > +                                 ELE_SESSION_CLOSE_RSP_SZ,
> > +                                 false);
> > +   return ret;
> > +}
> > +
> ...
> > +int se_get_mem_pool_buf(struct se_if_device_ctx *dev_ctx, void **buf,
> > +                   dma_addr_t *daddr, u32 len)
> > +{
> > +   struct se_shared_mem_mgmt_info *se_shared_mem_mgmt =
> &dev_ctx->se_shared_mem_mgmt;
> > +   struct se_if_priv *priv = dev_ctx->priv;
> > +   struct se_buf_desc *b_desc = NULL;
> > +
> > +   lockdep_assert_held(&dev_ctx->fops_lock);
> > +
> > +   if (se_is_fw_busy_ctx(dev_ctx))
> > +           return -EBUSY;
> > +
> > +   b_desc = kzalloc_obj(*b_desc, GFP_KERNEL);
>
> GPF_KERNEL is default for *_obj alloc macro. Needn't set it
>
> kzalloc_obj(*b_desc), check others.
>
Accepted will be removed in v43.
> Frank


NXP Confidential