Subject: Re: [PATCH] rpmsg: glink: Fix GLINK command prefix
To:     Bjorn Andersson <quic_bjorande@quicinc.com>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Mathieu Poirier <mathieu.poirier@linaro.org>
CC:     <linux-arm-msm@vger.kernel.org>,
        <linux-remoteproc@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20230214225933.2025595-1-quic_bjorande@quicinc.com>
From:   Chris Lew <quic_clew@quicinc.com>
Message-ID: <bffc0284-0a61-1292-c8be-da5eae675e7b@quicinc.com>
Date:   Tue, 14 Feb 2023 15:15:05 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
MIME-Version: 1.0
In-Reply-To: <20230214225933.2025595-1-quic_bjorande@quicinc.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 2/14/2023 2:59 PM, Bjorn Andersson wrote:
> The upstream GLINK driver was first introduced to communicate with the
> RPM on MSM8996, presumably as an artifact from that era the command
> defines was prefixed RPM_CMD, while they actually are GLINK_CMDs.
> 
> Let's rename these, to keep things tidy. No functional change.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>

Reviewed-by: Chris Lew <quic_clew@quicinc.com>

> ---
>   drivers/rpmsg/qcom_glink_native.c | 98 +++++++++++++++----------------
>   1 file changed, 49 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
> index 7a3cac62ecfa..89c3381f06c3 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -183,20 +183,20 @@ struct glink_channel {
>   
>   static const struct rpmsg_endpoint_ops glink_endpoint_ops;
>   
> -#define RPM_CMD_VERSION			0
> -#define RPM_CMD_VERSION_ACK		1
> -#define RPM_CMD_OPEN			2
> -#define RPM_CMD_CLOSE			3
> -#define RPM_CMD_OPEN_ACK		4
> -#define RPM_CMD_INTENT			5
> -#define RPM_CMD_RX_DONE			6
> -#define RPM_CMD_RX_INTENT_REQ		7
> -#define RPM_CMD_RX_INTENT_REQ_ACK	8
> -#define RPM_CMD_TX_DATA			9
> -#define RPM_CMD_CLOSE_ACK		11
> -#define RPM_CMD_TX_DATA_CONT		12
> -#define RPM_CMD_READ_NOTIF		13
> -#define RPM_CMD_RX_DONE_W_REUSE		14
> +#define GLINK_CMD_VERSION		0
> +#define GLINK_CMD_VERSION_ACK		1
> +#define GLINK_CMD_OPEN			2
> +#define GLINK_CMD_CLOSE			3
> +#define GLINK_CMD_OPEN_ACK		4
> +#define GLINK_CMD_INTENT		5
> +#define GLINK_CMD_RX_DONE		6
> +#define GLINK_CMD_RX_INTENT_REQ		7
> +#define GLINK_CMD_RX_INTENT_REQ_ACK	8
> +#define GLINK_CMD_TX_DATA		9
> +#define GLINK_CMD_CLOSE_ACK		11
> +#define GLINK_CMD_TX_DATA_CONT		12
> +#define GLINK_CMD_READ_NOTIF		13
> +#define GLINK_CMD_RX_DONE_W_REUSE	14
>   
>   #define GLINK_FEATURE_INTENTLESS	BIT(1)
>   
> @@ -306,7 +306,7 @@ static void qcom_glink_send_read_notify(struct qcom_glink *glink)
>   {
>   	struct glink_msg msg;
>   
> -	msg.cmd = cpu_to_le16(RPM_CMD_READ_NOTIF);
> +	msg.cmd = cpu_to_le16(GLINK_CMD_READ_NOTIF);
>   	msg.param1 = 0;
>   	msg.param2 = 0;
>   
> @@ -375,7 +375,7 @@ static int qcom_glink_send_version(struct qcom_glink *glink)
>   {
>   	struct glink_msg msg;
>   
> -	msg.cmd = cpu_to_le16(RPM_CMD_VERSION);
> +	msg.cmd = cpu_to_le16(GLINK_CMD_VERSION);
>   	msg.param1 = cpu_to_le16(GLINK_VERSION_1);
>   	msg.param2 = cpu_to_le32(glink->features);
>   
> @@ -386,7 +386,7 @@ static void qcom_glink_send_version_ack(struct qcom_glink *glink)
>   {
>   	struct glink_msg msg;
>   
> -	msg.cmd = cpu_to_le16(RPM_CMD_VERSION_ACK);
> +	msg.cmd = cpu_to_le16(GLINK_CMD_VERSION_ACK);
>   	msg.param1 = cpu_to_le16(GLINK_VERSION_1);
>   	msg.param2 = cpu_to_le32(glink->features);
>   
> @@ -398,7 +398,7 @@ static void qcom_glink_send_open_ack(struct qcom_glink *glink,
>   {
>   	struct glink_msg msg;
>   
> -	msg.cmd = cpu_to_le16(RPM_CMD_OPEN_ACK);
> +	msg.cmd = cpu_to_le16(GLINK_CMD_OPEN_ACK);
>   	msg.param1 = cpu_to_le16(channel->rcid);
>   	msg.param2 = cpu_to_le32(0);
>   
> @@ -430,11 +430,11 @@ static void qcom_glink_intent_req_abort(struct glink_channel *channel)
>   }
>   
>   /**
> - * qcom_glink_send_open_req() - send a RPM_CMD_OPEN request to the remote
> + * qcom_glink_send_open_req() - send a GLINK_CMD_OPEN request to the remote
>    * @glink: Ptr to the glink edge
>    * @channel: Ptr to the channel that the open req is sent
>    *
> - * Allocates a local channel id and sends a RPM_CMD_OPEN message to the remote.
> + * Allocates a local channel id and sends a GLINK_CMD_OPEN message to the remote.
>    * Will return with refcount held, regardless of outcome.
>    *
>    * Return: 0 on success, negative errno otherwise.
> @@ -463,7 +463,7 @@ static int qcom_glink_send_open_req(struct qcom_glink *glink,
>   
>   	channel->lcid = ret;
>   
> -	req.msg.cmd = cpu_to_le16(RPM_CMD_OPEN);
> +	req.msg.cmd = cpu_to_le16(GLINK_CMD_OPEN);
>   	req.msg.param1 = cpu_to_le16(channel->lcid);
>   	req.msg.param2 = cpu_to_le32(name_len);
>   	strcpy(req.name, channel->name);
> @@ -488,7 +488,7 @@ static void qcom_glink_send_close_req(struct qcom_glink *glink,
>   {
>   	struct glink_msg req;
>   
> -	req.cmd = cpu_to_le16(RPM_CMD_CLOSE);
> +	req.cmd = cpu_to_le16(GLINK_CMD_CLOSE);
>   	req.param1 = cpu_to_le16(channel->lcid);
>   	req.param2 = 0;
>   
> @@ -500,7 +500,7 @@ static void qcom_glink_send_close_ack(struct qcom_glink *glink,
>   {
>   	struct glink_msg req;
>   
> -	req.cmd = cpu_to_le16(RPM_CMD_CLOSE_ACK);
> +	req.cmd = cpu_to_le16(GLINK_CMD_CLOSE_ACK);
>   	req.param1 = cpu_to_le16(rcid);
>   	req.param2 = 0;
>   
> @@ -531,7 +531,7 @@ static void qcom_glink_rx_done_work(struct work_struct *work)
>   		iid = intent->id;
>   		reuse = intent->reuse;
>   
> -		cmd.id = reuse ? RPM_CMD_RX_DONE_W_REUSE : RPM_CMD_RX_DONE;
> +		cmd.id = reuse ? GLINK_CMD_RX_DONE_W_REUSE : GLINK_CMD_RX_DONE;
>   		cmd.lcid = cid;
>   		cmd.liid = iid;
>   
> @@ -643,7 +643,7 @@ static int qcom_glink_send_intent_req_ack(struct qcom_glink *glink,
>   {
>   	struct glink_msg msg;
>   
> -	msg.cmd = cpu_to_le16(RPM_CMD_RX_INTENT_REQ_ACK);
> +	msg.cmd = cpu_to_le16(GLINK_CMD_RX_INTENT_REQ_ACK);
>   	msg.param1 = cpu_to_le16(channel->lcid);
>   	msg.param2 = cpu_to_le32(granted);
>   
> @@ -674,7 +674,7 @@ static int qcom_glink_advertise_intent(struct qcom_glink *glink,
>   	} __packed;
>   	struct command cmd;
>   
> -	cmd.id = cpu_to_le16(RPM_CMD_INTENT);
> +	cmd.id = cpu_to_le16(GLINK_CMD_INTENT);
>   	cmd.lcid = cpu_to_le16(channel->lcid);
>   	cmd.count = cpu_to_le32(1);
>   	cmd.size = cpu_to_le32(intent->size);
> @@ -1037,40 +1037,40 @@ void qcom_glink_native_rx(struct qcom_glink *glink)
>   		param2 = le32_to_cpu(msg.param2);
>   
>   		switch (cmd) {
> -		case RPM_CMD_VERSION:
> -		case RPM_CMD_VERSION_ACK:
> -		case RPM_CMD_CLOSE:
> -		case RPM_CMD_CLOSE_ACK:
> -		case RPM_CMD_RX_INTENT_REQ:
> +		case GLINK_CMD_VERSION:
> +		case GLINK_CMD_VERSION_ACK:
> +		case GLINK_CMD_CLOSE:
> +		case GLINK_CMD_CLOSE_ACK:
> +		case GLINK_CMD_RX_INTENT_REQ:
>   			ret = qcom_glink_rx_defer(glink, 0);
>   			break;
> -		case RPM_CMD_OPEN_ACK:
> +		case GLINK_CMD_OPEN_ACK:
>   			ret = qcom_glink_rx_open_ack(glink, param1);
>   			qcom_glink_rx_advance(glink, ALIGN(sizeof(msg), 8));
>   			break;
> -		case RPM_CMD_OPEN:
> +		case GLINK_CMD_OPEN:
>   			ret = qcom_glink_rx_defer(glink, param2);
>   			break;
> -		case RPM_CMD_TX_DATA:
> -		case RPM_CMD_TX_DATA_CONT:
> +		case GLINK_CMD_TX_DATA:
> +		case GLINK_CMD_TX_DATA_CONT:
>   			ret = qcom_glink_rx_data(glink, avail);
>   			break;
> -		case RPM_CMD_READ_NOTIF:
> +		case GLINK_CMD_READ_NOTIF:
>   			qcom_glink_rx_advance(glink, ALIGN(sizeof(msg), 8));
>   			qcom_glink_tx_kick(glink);
>   			break;
> -		case RPM_CMD_INTENT:
> +		case GLINK_CMD_INTENT:
>   			qcom_glink_handle_intent(glink, param1, param2, avail);
>   			break;
> -		case RPM_CMD_RX_DONE:
> +		case GLINK_CMD_RX_DONE:
>   			qcom_glink_handle_rx_done(glink, param1, param2, false);
>   			qcom_glink_rx_advance(glink, ALIGN(sizeof(msg), 8));
>   			break;
> -		case RPM_CMD_RX_DONE_W_REUSE:
> +		case GLINK_CMD_RX_DONE_W_REUSE:
>   			qcom_glink_handle_rx_done(glink, param1, param2, true);
>   			qcom_glink_rx_advance(glink, ALIGN(sizeof(msg), 8));
>   			break;
> -		case RPM_CMD_RX_INTENT_REQ_ACK:
> +		case GLINK_CMD_RX_INTENT_REQ_ACK:
>   			qcom_glink_handle_intent_req_ack(glink, param1, param2);
>   			qcom_glink_rx_advance(glink, ALIGN(sizeof(msg), 8));
>   			break;
> @@ -1272,7 +1272,7 @@ static int qcom_glink_request_intent(struct qcom_glink *glink,
>   
>   	reinit_completion(&channel->intent_req_comp);
>   
> -	cmd.id = RPM_CMD_RX_INTENT_REQ;
> +	cmd.id = GLINK_CMD_RX_INTENT_REQ;
>   	cmd.cid = channel->lcid;
>   	cmd.size = size;
>   
> @@ -1346,7 +1346,7 @@ static int __qcom_glink_send(struct glink_channel *channel,
>   		chunk_size = SZ_8K;
>   		left_size = len - chunk_size;
>   	}
> -	req.msg.cmd = cpu_to_le16(RPM_CMD_TX_DATA);
> +	req.msg.cmd = cpu_to_le16(GLINK_CMD_TX_DATA);
>   	req.msg.param1 = cpu_to_le16(channel->lcid);
>   	req.msg.param2 = cpu_to_le32(iid);
>   	req.chunk_size = cpu_to_le32(chunk_size);
> @@ -1367,7 +1367,7 @@ static int __qcom_glink_send(struct glink_channel *channel,
>   			chunk_size = SZ_8K;
>   		left_size -= chunk_size;
>   
> -		req.msg.cmd = cpu_to_le16(RPM_CMD_TX_DATA_CONT);
> +		req.msg.cmd = cpu_to_le16(GLINK_CMD_TX_DATA_CONT);
>   		req.msg.param1 = cpu_to_le16(channel->lcid);
>   		req.msg.param2 = cpu_to_le32(iid);
>   		req.chunk_size = cpu_to_le32(chunk_size);
> @@ -1631,22 +1631,22 @@ static void qcom_glink_work(struct work_struct *work)
>   		param2 = le32_to_cpu(msg->param2);
>   
>   		switch (cmd) {
> -		case RPM_CMD_VERSION:
> +		case GLINK_CMD_VERSION:
>   			qcom_glink_receive_version(glink, param1, param2);
>   			break;
> -		case RPM_CMD_VERSION_ACK:
> +		case GLINK_CMD_VERSION_ACK:
>   			qcom_glink_receive_version_ack(glink, param1, param2);
>   			break;
> -		case RPM_CMD_OPEN:
> +		case GLINK_CMD_OPEN:
>   			qcom_glink_rx_open(glink, param1, msg->data);
>   			break;
> -		case RPM_CMD_CLOSE:
> +		case GLINK_CMD_CLOSE:
>   			qcom_glink_rx_close(glink, param1);
>   			break;
> -		case RPM_CMD_CLOSE_ACK:
> +		case GLINK_CMD_CLOSE_ACK:
>   			qcom_glink_rx_close_ack(glink, param1);
>   			break;
> -		case RPM_CMD_RX_INTENT_REQ:
> +		case GLINK_CMD_RX_INTENT_REQ:
>   			qcom_glink_handle_intent_req(glink, param1, param2);
>   			break;
>   		default:
>