Re: [PATCH v2 10/20] rpmsg: glink: Add support for transport version negotiation

Arun Kumar Neelakantam
2017-08-28T11:49:10Z

On 8/24/2017 12:51 PM, Sricharan R wrote:
> G-link supports a version number and feature flags for each transport.
> A combination of the version number and feature flags enable/disable:
>
> (*) G-Link software updates for each edge
> (*) Individual features for each edge
>
> Endpoints negotiate both the version and the supported flags when
> the transport is opened and they cannot be changed after negotiation has
> been completed.
>
> Each full implementation of G-Link must support a minimum of the current
> version, the previous version, and the base negotiation version called v0.
>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Arun Kumar Neelakantam <aneela@codeaurora.org>

Regards,
Arun N
> ---
> drivers/rpmsg/qcom_glink_native.c | 75 ++++++++++++++++++++++++++++++++++++---
> drivers/rpmsg/qcom_glink_native.h | 5 +++
> drivers/rpmsg/qcom_glink_rpm.c | 4 ++-
> drivers/rpmsg/qcom_glink_smem.c | 1 +
> 4 files changed, 79 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
> index 9a58925..777ac6b 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -31,6 +31,7 @@
> #include "qcom_glink_native.h"
>
> #define GLINK_NAME_SIZE 32
> +#define GLINK_VERSION_1 1
>
> #define RPM_GLINK_CID_MIN 1
> #define RPM_GLINK_CID_MAX 65536
> @@ -94,6 +95,9 @@ struct qcom_glink {
> struct mutex idr_lock;
> struct idr lcids;
> struct idr rcids;
> + unsigned long features;
> +
> + bool intentless;
> };
>
> enum {
> @@ -256,8 +260,8 @@ static int qcom_glink_send_version(struct qcom_glink *glink)
> struct glink_msg msg;
>
> msg.cmd = cpu_to_le16(RPM_CMD_VERSION);
> - msg.param1 = cpu_to_le16(1);
> - msg.param2 = cpu_to_le32(GLINK_FEATURE_INTENTLESS);
> + msg.param1 = cpu_to_le16(GLINK_VERSION_1);
> + msg.param2 = cpu_to_le32(glink->features);
>
> return qcom_glink_tx(glink, &msg, sizeof(msg), NULL, 0, true);
> }
> @@ -267,8 +271,8 @@ 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.param1 = cpu_to_le16(1);
> - msg.param2 = cpu_to_le32(0);
> + msg.param1 = cpu_to_le16(GLINK_VERSION_1);
> + msg.param2 = cpu_to_le32(glink->features);
>
> qcom_glink_tx(glink, &msg, sizeof(msg), NULL, 0, true);
> }
> @@ -362,6 +366,63 @@ static void qcom_glink_send_close_ack(struct qcom_glink *glink,
> qcom_glink_tx(glink, &req, sizeof(req), NULL, 0, true);
> }
>
> +/**
> + * qcom_glink_receive_version() - receive version/features from remote system
> + *
> + * @glink: pointer to transport interface
> + * @r_version: remote version
> + * @r_features: remote features
> + *
> + * This function is called in response to a remote-initiated version/feature
> + * negotiation sequence.
> + */
> +static void qcom_glink_receive_version(struct qcom_glink *glink,
> + u32 version,
> + u32 features)
> +{
> + switch (version) {
> + case 0:
> + break;
> + case GLINK_VERSION_1:
> + glink->features &= features;
> + /* FALLTHROUGH */
> + default:
> + qcom_glink_send_version_ack(glink);
> + break;
> + }
> +}
> +
> +/**
> + * qcom_glink_receive_version_ack() - receive negotiation ack from remote system
> + *
> + * @glink: pointer to transport interface
> + * @r_version: remote version response
> + * @r_features: remote features response
> + *
> + * This function is called in response to a local-initiated version/feature
> + * negotiation sequence and is the counter-offer from the remote side based
> + * upon the initial version and feature set requested.
> + */
> +static void qcom_glink_receive_version_ack(struct qcom_glink *glink,
> + u32 version,
> + u32 features)
> +{
> + switch (version) {
> + case 0:
> + /* Version negotiation failed */
> + break;
> + case GLINK_VERSION_1:
> + if (features == glink->features)
> + break;
> +
> + glink->features &= features;
> + /* FALLTHROUGH */
> + default:
> + qcom_glink_send_version(glink);
> + break;
> + }
> +}
> +
> static int qcom_glink_rx_defer(struct qcom_glink *glink, size_t extra)
> {
> struct glink_defer_cmd *dcmd;
> @@ -909,9 +970,10 @@ static void qcom_glink_work(struct work_struct *work)
>
> switch (cmd) {
> case RPM_CMD_VERSION:
> - qcom_glink_send_version_ack(glink);
> + qcom_glink_receive_version(glink, param1, param2);
> break;
> case RPM_CMD_VERSION_ACK:
> + qcom_glink_receive_version_ack(glink, param1, param2);
> break;
> case RPM_CMD_OPEN:
> qcom_glink_rx_open(glink, param1, msg->data);
> @@ -932,6 +994,7 @@ static void qcom_glink_work(struct work_struct *work)
> }
>
> struct qcom_glink *qcom_glink_native_probe(struct device *dev,
> + unsigned long features,
> struct qcom_glink_pipe *rx,
> struct qcom_glink_pipe *tx)
> {
> @@ -947,6 +1010,8 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev,
> glink->tx_pipe = tx;
> glink->rx_pipe = rx;
>
> + glink->features = features;
> +
> mutex_init(&glink->tx_lock);
> spin_lock_init(&glink->rx_lock);
> INIT_LIST_HEAD(&glink->rx_queue);
> diff --git a/drivers/rpmsg/qcom_glink_native.h b/drivers/rpmsg/qcom_glink_native.h
> index 197bb9d..456ec79 100644
> --- a/drivers/rpmsg/qcom_glink_native.h
> +++ b/drivers/rpmsg/qcom_glink_native.h
> @@ -14,6 +14,10 @@
> #ifndef __QCOM_GLINK_NATIVE_H__
> #define __QCOM_GLINK_NATIVE_H__
>
> +#define GLINK_FEATURE_INTENT_REUSE BIT(0)
> +#define GLINK_FEATURE_MIGRATION BIT(1)
> +#define GLINK_FEATURE_TRACER_PKT BIT(2)
> +
> struct qcom_glink_pipe {
> size_t length;
>
> @@ -31,6 +35,7 @@ struct qcom_glink_pipe {
> struct qcom_glink;
>
> struct qcom_glink *qcom_glink_native_probe(struct device *dev,
> + unsigned long features,
> struct qcom_glink_pipe *rx,
> struct qcom_glink_pipe *tx);
> void qcom_glink_native_remove(struct qcom_glink *glink);
> diff --git a/drivers/rpmsg/qcom_glink_rpm.c b/drivers/rpmsg/qcom_glink_rpm.c
> index cc73af0..7d039cd 100644
> --- a/drivers/rpmsg/qcom_glink_rpm.c
> +++ b/drivers/rpmsg/qcom_glink_rpm.c
> @@ -302,7 +302,9 @@ static int glink_rpm_probe(struct platform_device *pdev)
> writel(0, tx_pipe->head);
> writel(0, rx_pipe->tail);
>
> - glink = qcom_glink_native_probe(&pdev->dev, &rx_pipe->native,
> + glink = qcom_glink_native_probe(&pdev->dev,
> + 0,
> + &rx_pipe->native,
> &tx_pipe->native);
> if (IS_ERR(glink))
> return PTR_ERR(glink);
> diff --git a/drivers/rpmsg/qcom_glink_smem.c b/drivers/rpmsg/qcom_glink_smem.c
> index 19179a1..b78874b 100644
> --- a/drivers/rpmsg/qcom_glink_smem.c
> +++ b/drivers/rpmsg/qcom_glink_smem.c
> @@ -284,6 +284,7 @@ struct qcom_glink *qcom_glink_smem_register(struct device *parent,
> *tx_pipe->head = 0;
>
> glink = qcom_glink_native_probe(dev,
> + GLINK_FEATURE_TRACER_PKT,
> &rx_pipe->native, &tx_pipe->native);
> if (IS_ERR(glink)) {
> ret = PTR_ERR(glink);