Re: [PATCH 10/18] rpmsg: glink: Add support for TX intents

[Sricharan R]

Hi,

<snip>

> 
> spinlock intent_lock initialization is missed ?

 right, should add that.

>> ÂÂÂÂÂ kref_init(&channel->refcount);
>> Â ÂÂÂÂÂ return channel;
>> @@ -187,6 +215,7 @@ static void qcom_glink_channel_release(struct kref *ref)
>> ÂÂÂÂÂ struct glink_channel *channel = container_of(ref, struct glink_channel,
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ refcount);
>> Â +ÂÂÂ idr_destroy(&channel->liids);
> 
> idr_destroy shouldn`t be covered by intent_lock ?
> 

 hmm, will fix this.

>> ÂÂÂÂÂ kfree(channel->name);
>> ÂÂÂÂÂ kfree(channel);
>> Â }
>> @@ -423,6 +452,130 @@ static void qcom_glink_receive_version_ack(struct qcom_glink *glink,
>> ÂÂÂÂÂ }
>> Â }
>> Â +/**
>> + * qcom_glink_send_intent_req_ack() - convert an rx intent request ack cmd to
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ wire format and transmit
>> + * @glink:ÂÂÂ The transport to transmit on.
>> + * @channel:ÂÂÂ The glink channel
>> + * @granted:ÂÂÂ The request response to encode.
>> + *
>> + * Return: 0 on success or standard Linux error code.
>> + */
>> +static int qcom_glink_send_intent_req_ack(struct qcom_glink *glink,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct glink_channel *channel,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ bool granted)
>> +{
>> +ÂÂÂ struct glink_msg msg;
>> +
>> +ÂÂÂ msg.cmd = cpu_to_le16(RPM_CMD_RX_INTENT_REQ_ACK);
>> +ÂÂÂ msg.param1 = cpu_to_le16(channel->lcid);
>> +ÂÂÂ msg.param2 = cpu_to_le32(granted);
>> +
>> +ÂÂÂ qcom_glink_tx(glink, &msg, sizeof(msg), NULL, 0, true);
>> +
>> +ÂÂÂ return 0;
>> +}
>> +
>> +/**
>> + * tx_cmd_local_rx_intent() - convert an rx intent cmd to wire format and
>> + *ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ transmit
> 
> copy-paste mistake

 ok.

>> + * @glink:ÂÂÂ The transport to transmit on.
>> + * @channel:ÂÂÂ The local channel
>> + * @size:ÂÂÂ The intent to pass on to remote.
>> + *
>> + * Return: 0 on success or standard Linux error code.
>> + */
>> +static int qcom_glink_advertise_intent(struct qcom_glink *glink,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct glink_channel *channel,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct glink_core_rx_intent *intent)
>> +{
>> +ÂÂÂ struct command {
>> +ÂÂÂÂÂÂÂ u16 id;
>> +ÂÂÂÂÂÂÂ u16 lcid;
>> +ÂÂÂÂÂÂÂ u32 count;
>> +ÂÂÂÂÂÂÂ u32 size;
>> +ÂÂÂÂÂÂÂ u32 liid;
>> +ÂÂÂ } __packed;
>> +ÂÂÂ struct command cmd;
>> +
>> +ÂÂÂ cmd.id = cpu_to_le16(RPM_CMD_INTENT);
>> +ÂÂÂ cmd.lcid = cpu_to_le16(channel->lcid);
>> +ÂÂÂ cmd.count = cpu_to_le32(1);
>> +ÂÂÂ cmd.size = cpu_to_le32(intent->size);
>> +ÂÂÂ cmd.liid = cpu_to_le32(intent->id);
>> +
>> +ÂÂÂ qcom_glink_tx(glink, &cmd, sizeof(cmd), NULL, 0, true);
>> +
>> +ÂÂÂ return 0;
>> +}
>> +
>> +static struct glink_core_rx_intent *
>> +qcom_glink_alloc_intent(struct qcom_glink *glink,
>> +ÂÂÂÂÂÂÂÂÂÂÂ struct glink_channel *channel,
>> +ÂÂÂÂÂÂÂÂÂÂÂ size_t size,
>> +ÂÂÂÂÂÂÂÂÂÂÂ bool reuseable)
>> +{
>> +ÂÂÂ struct glink_core_rx_intent *intent;
>> +ÂÂÂ int ret;
>> +ÂÂÂ unsigned long flags;
>> +
>> +ÂÂÂ intent = kzalloc(sizeof(*intent), GFP_KERNEL);
>> +
>> +ÂÂÂ if (!intent)
>> +ÂÂÂÂÂÂÂ return NULL;
>> +
>> +ÂÂÂ intent->data = kzalloc(size, GFP_KERNEL);
>> +ÂÂÂ if (!intent->data)
>> +ÂÂÂÂÂÂÂ return NULL;
>> +
>> +ÂÂÂ spin_lock_irqsave(&channel->intent_lock, flags);
>> +ÂÂÂ ret = idr_alloc_cyclic(&channel->liids, intent, 1, -1, GFP_ATOMIC);
>> +ÂÂÂ if (ret < 0) {
>> +ÂÂÂÂÂÂÂ spin_unlock_irqrestore(&channel->intent_lock, flags);
>> +ÂÂÂÂÂÂÂ return NULL;
>> +ÂÂÂ }
>> +ÂÂÂ spin_unlock_irqrestore(&channel->intent_lock, flags);
>> +
>> +ÂÂÂ intent->id = ret;
>> +ÂÂÂ intent->size = size;
>> +ÂÂÂ intent->reuse = reuseable;
>> +
>> +ÂÂÂ return intent;
>> +}
>> +
>> +/**
>> + * glink_core_rx_cmd_remote_rx_intent_req() - Receive a request for rx_intent
>> + *ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ from remote side
> 
> copy-paste mistake

 ok.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus