* [PATCH V1 1/1] soc: qcom: smp2p: add feature negotiation and ssr ack feature support
@ 2021-09-30 16:25 Deepak Kumar Singh
2021-09-30 17:29 ` Bjorn Andersson
0 siblings, 1 reply; 2+ messages in thread
From: Deepak Kumar Singh @ 2021-09-30 16:25 UTC (permalink / raw)
To: bjorn.andersson, clew
Cc: linux-kernel, linux-arm-msm, linux-remoteproc,
Deepak Kumar Singh, Andy Gross
This patch adds feature negotiation and ssr ack feature between
local and remote host. Local host can negotiate on common features
supported with remote host.
Signed-off-by: Chris Lew <clew@codeaurora.org>
Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
---
drivers/soc/qcom/smp2p.c | 128 ++++++++++++++++++++++++++++++++++++++---------
1 file changed, 103 insertions(+), 25 deletions(-)
diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
index 38585a7..c1a60016 100644
--- a/drivers/soc/qcom/smp2p.c
+++ b/drivers/soc/qcom/smp2p.c
@@ -41,8 +41,11 @@
#define SMP2P_MAX_ENTRY_NAME 16
#define SMP2P_FEATURE_SSR_ACK 0x1
+#define SMP2P_FLAGS_RESTART_DONE_BIT 0
+#define SMP2P_FLAGS_RESTART_ACK_BIT 1
#define SMP2P_MAGIC 0x504d5324
+#define SMP2P_FEATURES SMP2P_FEATURE_SSR_ACK
/**
* struct smp2p_smem_item - in memory communication structure
@@ -136,6 +139,10 @@ struct qcom_smp2p {
unsigned valid_entries;
+ bool ssr_ack_enabled;
+ bool ssr_ack;
+ bool open;
+
unsigned local_pid;
unsigned remote_pid;
@@ -163,22 +170,59 @@ static void qcom_smp2p_kick(struct qcom_smp2p *smp2p)
}
}
-/**
- * qcom_smp2p_intr() - interrupt handler for incoming notifications
- * @irq: unused
- * @data: smp2p driver context
- *
- * Handle notifications from the remote side to handle newly allocated entries
- * or any changes to the state bits of existing entries.
- */
-static irqreturn_t qcom_smp2p_intr(int irq, void *data)
+static bool qcom_smp2p_check_ssr(struct qcom_smp2p *smp2p)
+{
+ struct smp2p_smem_item *in = smp2p->in;
+ bool restart;
+
+ if (!smp2p->ssr_ack_enabled)
+ return false;
+
+ restart = in->flags & BIT(SMP2P_FLAGS_RESTART_DONE_BIT);
+ if (restart == smp2p->ssr_ack)
+ return false;
+
+ return true;
+}
+
+static void qcom_smp2p_do_ssr_ack(struct qcom_smp2p *smp2p)
+{
+ struct smp2p_smem_item *out = smp2p->out;
+ u32 ack;
+ u32 val;
+
+ ack = !smp2p->ssr_ack;
+ smp2p->ssr_ack = ack;
+ ack = ack << SMP2P_FLAGS_RESTART_ACK_BIT;
+
+ val = out->flags & ~BIT(SMP2P_FLAGS_RESTART_ACK_BIT);
+ val |= ack;
+ out->flags = val;
+
+ qcom_smp2p_kick(smp2p);
+}
+
+static void qcom_smp2p_negotiate(struct qcom_smp2p *smp2p)
+{
+ struct smp2p_smem_item *out = smp2p->out;
+ struct smp2p_smem_item *in = smp2p->in;
+ u32 features;
+
+ if (in->version == out->version) {
+ features = in->features & out->features;
+ out->features = features;
+
+ if (features & SMP2P_FEATURE_SSR_ACK)
+ smp2p->ssr_ack_enabled = true;
+
+ smp2p->open = true;
+ }
+}
+
+static void qcom_smp2p_notify_in(struct qcom_smp2p *smp2p)
{
struct smp2p_smem_item *in;
struct smp2p_entry *entry;
- struct qcom_smp2p *smp2p = data;
- unsigned smem_id = smp2p->smem_items[SMP2P_INBOUND];
- unsigned pid = smp2p->remote_pid;
- size_t size;
int irq_pin;
u32 status;
char buf[SMP2P_MAX_ENTRY_NAME];
@@ -187,18 +231,6 @@ static irqreturn_t qcom_smp2p_intr(int irq, void *data)
in = smp2p->in;
- /* Acquire smem item, if not already found */
- if (!in) {
- in = qcom_smem_get(pid, smem_id, &size);
- if (IS_ERR(in)) {
- dev_err(smp2p->dev,
- "Unable to acquire remote smp2p item\n");
- return IRQ_HANDLED;
- }
-
- smp2p->in = in;
- }
-
/* Match newly created entries */
for (i = smp2p->valid_entries; i < in->valid_entries; i++) {
list_for_each_entry(entry, &smp2p->inbound, node) {
@@ -237,7 +269,52 @@ static irqreturn_t qcom_smp2p_intr(int irq, void *data)
}
}
}
+}
+
+/**
+ * qcom_smp2p_intr() - interrupt handler for incoming notifications
+ * @irq: unused
+ * @data: smp2p driver context
+ *
+ * Handle notifications from the remote side to handle newly allocated entries
+ * or any changes to the state bits of existing entries.
+ */
+static irqreturn_t qcom_smp2p_intr(int irq, void *data)
+{
+ struct smp2p_smem_item *in;
+ struct qcom_smp2p *smp2p = data;
+ unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
+ unsigned int pid = smp2p->remote_pid;
+ size_t size;
+
+ in = smp2p->in;
+
+ /* Acquire smem item, if not already found */
+ if (!in) {
+ in = qcom_smem_get(pid, smem_id, &size);
+ if (IS_ERR(in)) {
+ dev_err(smp2p->dev,
+ "Unable to acquire remote smp2p item\n");
+ goto out;
+ }
+
+ smp2p->in = in;
+ }
+
+ if (!smp2p->open)
+ qcom_smp2p_negotiate(smp2p);
+
+ if (smp2p->open) {
+ bool do_restart;
+
+ do_restart = qcom_smp2p_check_ssr(smp2p);
+ qcom_smp2p_notify_in(smp2p);
+
+ if (do_restart)
+ qcom_smp2p_do_ssr_ack(smp2p);
+ }
+out:
return IRQ_HANDLED;
}
@@ -393,6 +470,7 @@ static int qcom_smp2p_alloc_outbound_item(struct qcom_smp2p *smp2p)
out->remote_pid = smp2p->remote_pid;
out->total_entries = SMP2P_MAX_ENTRY;
out->valid_entries = 0;
+ out->features = SMP2P_FEATURES;
/*
* Make sure the rest of the header is written before we validate the
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH V1 1/1] soc: qcom: smp2p: add feature negotiation and ssr ack feature support
2021-09-30 16:25 [PATCH V1 1/1] soc: qcom: smp2p: add feature negotiation and ssr ack feature support Deepak Kumar Singh
@ 2021-09-30 17:29 ` Bjorn Andersson
0 siblings, 0 replies; 2+ messages in thread
From: Bjorn Andersson @ 2021-09-30 17:29 UTC (permalink / raw)
To: Deepak Kumar Singh
Cc: clew, linux-kernel, linux-arm-msm, linux-remoteproc, Andy Gross
On Thu 30 Sep 09:25 PDT 2021, Deepak Kumar Singh wrote:
> This patch adds feature negotiation and ssr ack feature between
> local and remote host. Local host can negotiate on common features
> supported with remote host.
>
This states that you're negotiating features, but doesn't capture the
actual ssr ack; why it's there and how it works.
> Signed-off-by: Chris Lew <clew@codeaurora.org>
Author of the patch should be Chris, please commit with --author
"Chris.."
> Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
> ---
> drivers/soc/qcom/smp2p.c | 128 ++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 103 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
> index 38585a7..c1a60016 100644
> --- a/drivers/soc/qcom/smp2p.c
> +++ b/drivers/soc/qcom/smp2p.c
> @@ -41,8 +41,11 @@
> #define SMP2P_MAX_ENTRY_NAME 16
>
> #define SMP2P_FEATURE_SSR_ACK 0x1
> +#define SMP2P_FLAGS_RESTART_DONE_BIT 0
> +#define SMP2P_FLAGS_RESTART_ACK_BIT 1
>
> #define SMP2P_MAGIC 0x504d5324
> +#define SMP2P_FEATURES SMP2P_FEATURE_SSR_ACK
Rename this SMP2P_ALL_FEATURES?
>
> /**
> * struct smp2p_smem_item - in memory communication structure
> @@ -136,6 +139,10 @@ struct qcom_smp2p {
>
> unsigned valid_entries;
>
> + bool ssr_ack_enabled;
> + bool ssr_ack;
> + bool open;
How about renaming this "negotiation_done"?
> +
> unsigned local_pid;
> unsigned remote_pid;
>
> @@ -163,22 +170,59 @@ static void qcom_smp2p_kick(struct qcom_smp2p *smp2p)
> }
> }
>
> -/**
> - * qcom_smp2p_intr() - interrupt handler for incoming notifications
> - * @irq: unused
> - * @data: smp2p driver context
> - *
> - * Handle notifications from the remote side to handle newly allocated entries
> - * or any changes to the state bits of existing entries.
> - */
> -static irqreturn_t qcom_smp2p_intr(int irq, void *data)
> +static bool qcom_smp2p_check_ssr(struct qcom_smp2p *smp2p)
> +{
> + struct smp2p_smem_item *in = smp2p->in;
> + bool restart;
> +
> + if (!smp2p->ssr_ack_enabled)
> + return false;
> +
> + restart = in->flags & BIT(SMP2P_FLAGS_RESTART_DONE_BIT);
return restart != smp2p->ssr_ack;
> + if (restart == smp2p->ssr_ack)
> + return false;
> +
> + return true;
> +}
> +
> +static void qcom_smp2p_do_ssr_ack(struct qcom_smp2p *smp2p)
> +{
> + struct smp2p_smem_item *out = smp2p->out;
> + u32 ack;
> + u32 val;
> +
> + ack = !smp2p->ssr_ack;
> + smp2p->ssr_ack = ack;
> + ack = ack << SMP2P_FLAGS_RESTART_ACK_BIT;
> +
> + val = out->flags & ~BIT(SMP2P_FLAGS_RESTART_ACK_BIT);
> + val |= ack;
> + out->flags = val;
I think this would be cleaner as:
smp2p->ssr_ack = !smp2p->ssr_ack;
val = out->flags & ~BIT(SMP2P_FLAGS_RESTART_ACK_BIT);
if (smp2p->ssr_ack)
val |= BIT(SMP2P_FLAGS_RESTART_ACK_BIT);
out->flags = val;
> +
> + qcom_smp2p_kick(smp2p);
> +}
> +
> +static void qcom_smp2p_negotiate(struct qcom_smp2p *smp2p)
> +{
> + struct smp2p_smem_item *out = smp2p->out;
> + struct smp2p_smem_item *in = smp2p->in;
> + u32 features;
> +
> + if (in->version == out->version) {
> + features = in->features & out->features;
> + out->features = features;
out->features &= in->features;
> +
> + if (features & SMP2P_FEATURE_SSR_ACK)
if (out->features & SMP2P_FEATURE_SSR_ACK)
> + smp2p->ssr_ack_enabled = true;
> +
> + smp2p->open = true;
> + }
> +}
> +
> +static void qcom_smp2p_notify_in(struct qcom_smp2p *smp2p)
> {
> struct smp2p_smem_item *in;
> struct smp2p_entry *entry;
> - struct qcom_smp2p *smp2p = data;
> - unsigned smem_id = smp2p->smem_items[SMP2P_INBOUND];
> - unsigned pid = smp2p->remote_pid;
> - size_t size;
> int irq_pin;
> u32 status;
> char buf[SMP2P_MAX_ENTRY_NAME];
> @@ -187,18 +231,6 @@ static irqreturn_t qcom_smp2p_intr(int irq, void *data)
>
> in = smp2p->in;
>
> - /* Acquire smem item, if not already found */
> - if (!in) {
> - in = qcom_smem_get(pid, smem_id, &size);
> - if (IS_ERR(in)) {
> - dev_err(smp2p->dev,
> - "Unable to acquire remote smp2p item\n");
> - return IRQ_HANDLED;
> - }
> -
> - smp2p->in = in;
> - }
> -
> /* Match newly created entries */
> for (i = smp2p->valid_entries; i < in->valid_entries; i++) {
> list_for_each_entry(entry, &smp2p->inbound, node) {
> @@ -237,7 +269,52 @@ static irqreturn_t qcom_smp2p_intr(int irq, void *data)
> }
> }
> }
> +}
> +
> +/**
> + * qcom_smp2p_intr() - interrupt handler for incoming notifications
> + * @irq: unused
> + * @data: smp2p driver context
> + *
> + * Handle notifications from the remote side to handle newly allocated entries
> + * or any changes to the state bits of existing entries.
> + */
> +static irqreturn_t qcom_smp2p_intr(int irq, void *data)
> +{
> + struct smp2p_smem_item *in;
> + struct qcom_smp2p *smp2p = data;
> + unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
> + unsigned int pid = smp2p->remote_pid;
> + size_t size;
> +
> + in = smp2p->in;
> +
> + /* Acquire smem item, if not already found */
> + if (!in) {
> + in = qcom_smem_get(pid, smem_id, &size);
> + if (IS_ERR(in)) {
> + dev_err(smp2p->dev,
> + "Unable to acquire remote smp2p item\n");
> + goto out;
> + }
> +
> + smp2p->in = in;
> + }
> +
> + if (!smp2p->open)
> + qcom_smp2p_negotiate(smp2p);
> +
> + if (smp2p->open) {
> + bool do_restart;
How about "ack_restart" or "need_ack"?
While valid, can you please move the declaration to the top of the
function, to follow the style.
Regards,
Bjorn
> +
> + do_restart = qcom_smp2p_check_ssr(smp2p);
> + qcom_smp2p_notify_in(smp2p);
> +
> + if (do_restart)
> + qcom_smp2p_do_ssr_ack(smp2p);
> + }
>
> +out:
> return IRQ_HANDLED;
> }
>
> @@ -393,6 +470,7 @@ static int qcom_smp2p_alloc_outbound_item(struct qcom_smp2p *smp2p)
> out->remote_pid = smp2p->remote_pid;
> out->total_entries = SMP2P_MAX_ENTRY;
> out->valid_entries = 0;
> + out->features = SMP2P_FEATURES;
>
> /*
> * Make sure the rest of the header is written before we validate the
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-09-30 17:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-30 16:25 [PATCH V1 1/1] soc: qcom: smp2p: add feature negotiation and ssr ack feature support Deepak Kumar Singh
2021-09-30 17:29 ` Bjorn Andersson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).