2023-01-09 22:51:49

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 2/6] rpmsg: glink: smem: Wrap driver context

The Glink SMEM driver allocates a struct device and hangs two
devres-allocated pipe objects thereon. To facilitate the move of
interrupt and mailbox handling to the driver, introduce a wrapper object
capturing the device, glink reference and remote processor id.

The type of the remoteproc reference is updated, as these are
specifically targetting the SMEM implementation.

Signed-off-by: Bjorn Andersson <[email protected]>
---
drivers/remoteproc/qcom_common.h | 3 +-
drivers/rpmsg/qcom_glink_smem.c | 76 ++++++++++++++++++++------------
include/linux/rpmsg/qcom_glink.h | 12 ++---
3 files changed, 55 insertions(+), 36 deletions(-)

diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
index c35adf730be0..2747c7d9ba44 100644
--- a/drivers/remoteproc/qcom_common.h
+++ b/drivers/remoteproc/qcom_common.h
@@ -6,6 +6,7 @@
#include "remoteproc_internal.h"
#include <linux/soc/qcom/qmi.h>

+struct qcom_glink_smem;
struct qcom_sysmon;

struct qcom_rproc_glink {
@@ -15,7 +16,7 @@ struct qcom_rproc_glink {

struct device *dev;
struct device_node *node;
- struct qcom_glink *edge;
+ struct qcom_glink_smem *edge;
};

struct qcom_rproc_subdev {
diff --git a/drivers/rpmsg/qcom_glink_smem.c b/drivers/rpmsg/qcom_glink_smem.c
index 579bc4443f6d..703e63fa5a86 100644
--- a/drivers/rpmsg/qcom_glink_smem.c
+++ b/drivers/rpmsg/qcom_glink_smem.c
@@ -33,6 +33,14 @@
#define SMEM_GLINK_NATIVE_XPRT_FIFO_0 479
#define SMEM_GLINK_NATIVE_XPRT_FIFO_1 480

+struct qcom_glink_smem {
+ struct device dev;
+
+ struct qcom_glink *glink;
+
+ u32 remote_pid;
+};
+
struct glink_smem_pipe {
struct qcom_glink_pipe native;

@@ -41,7 +49,7 @@ struct glink_smem_pipe {

void *fifo;

- int remote_pid;
+ struct qcom_glink_smem *smem;
};

#define to_smem_pipe(p) container_of(p, struct glink_smem_pipe, native)
@@ -49,13 +57,14 @@ struct glink_smem_pipe {
static size_t glink_smem_rx_avail(struct qcom_glink_pipe *np)
{
struct glink_smem_pipe *pipe = to_smem_pipe(np);
+ struct qcom_glink_smem *smem = pipe->smem;
size_t len;
void *fifo;
u32 head;
u32 tail;

if (!pipe->fifo) {
- fifo = qcom_smem_get(pipe->remote_pid,
+ fifo = qcom_smem_get(smem->remote_pid,
SMEM_GLINK_NATIVE_XPRT_FIFO_1, &len);
if (IS_ERR(fifo)) {
pr_err("failed to acquire RX fifo handle: %ld\n",
@@ -179,45 +188,49 @@ static void glink_smem_tx_write(struct qcom_glink_pipe *glink_pipe,

static void qcom_glink_smem_release(struct device *dev)
{
- kfree(dev);
+ struct qcom_glink_smem *smem = container_of(dev, struct qcom_glink_smem, dev);
+
+ kfree(smem);
}

-struct qcom_glink *qcom_glink_smem_register(struct device *parent,
- struct device_node *node)
+struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
+ struct device_node *node)
{
struct glink_smem_pipe *rx_pipe;
struct glink_smem_pipe *tx_pipe;
struct qcom_glink *glink;
- struct device *dev;
+ struct qcom_glink_smem *smem;
u32 remote_pid;
__le32 *descs;
size_t size;
int ret;

- dev = kzalloc(sizeof(*dev), GFP_KERNEL);
- if (!dev)
+ smem = kzalloc(sizeof(*smem), GFP_KERNEL);
+ if (!smem)
return ERR_PTR(-ENOMEM);

- dev->parent = parent;
- dev->of_node = node;
- dev->release = qcom_glink_smem_release;
- dev_set_name(dev, "%s:%pOFn", dev_name(parent->parent), node);
- ret = device_register(dev);
+ smem->dev.parent = parent;
+ smem->dev.of_node = node;
+ smem->dev.release = qcom_glink_smem_release;
+ dev_set_name(&smem->dev, "%s:%pOFn", dev_name(parent->parent), node);
+ ret = device_register(&smem->dev);
if (ret) {
pr_err("failed to register glink edge\n");
- put_device(dev);
+ put_device(&smem->dev);
return ERR_PTR(ret);
}

- ret = of_property_read_u32(dev->of_node, "qcom,remote-pid",
+ ret = of_property_read_u32(smem->dev.of_node, "qcom,remote-pid",
&remote_pid);
if (ret) {
- dev_err(dev, "failed to parse qcom,remote-pid\n");
+ dev_err(&smem->dev, "failed to parse qcom,remote-pid\n");
goto err_put_dev;
}

- rx_pipe = devm_kzalloc(dev, sizeof(*rx_pipe), GFP_KERNEL);
- tx_pipe = devm_kzalloc(dev, sizeof(*tx_pipe), GFP_KERNEL);
+ smem->remote_pid = remote_pid;
+
+ rx_pipe = devm_kzalloc(&smem->dev, sizeof(*rx_pipe), GFP_KERNEL);
+ tx_pipe = devm_kzalloc(&smem->dev, sizeof(*tx_pipe), GFP_KERNEL);
if (!rx_pipe || !tx_pipe) {
ret = -ENOMEM;
goto err_put_dev;
@@ -226,20 +239,20 @@ struct qcom_glink *qcom_glink_smem_register(struct device *parent,
ret = qcom_smem_alloc(remote_pid,
SMEM_GLINK_NATIVE_XPRT_DESCRIPTOR, 32);
if (ret && ret != -EEXIST) {
- dev_err(dev, "failed to allocate glink descriptors\n");
+ dev_err(&smem->dev, "failed to allocate glink descriptors\n");
goto err_put_dev;
}

descs = qcom_smem_get(remote_pid,
SMEM_GLINK_NATIVE_XPRT_DESCRIPTOR, &size);
if (IS_ERR(descs)) {
- dev_err(dev, "failed to acquire xprt descriptor\n");
+ dev_err(&smem->dev, "failed to acquire xprt descriptor\n");
ret = PTR_ERR(descs);
goto err_put_dev;
}

if (size != 32) {
- dev_err(dev, "glink descriptor of invalid size\n");
+ dev_err(&smem->dev, "glink descriptor of invalid size\n");
ret = -EINVAL;
goto err_put_dev;
}
@@ -252,31 +265,31 @@ struct qcom_glink *qcom_glink_smem_register(struct device *parent,
ret = qcom_smem_alloc(remote_pid, SMEM_GLINK_NATIVE_XPRT_FIFO_0,
SZ_16K);
if (ret && ret != -EEXIST) {
- dev_err(dev, "failed to allocate TX fifo\n");
+ dev_err(&smem->dev, "failed to allocate TX fifo\n");
goto err_put_dev;
}

tx_pipe->fifo = qcom_smem_get(remote_pid, SMEM_GLINK_NATIVE_XPRT_FIFO_0,
&tx_pipe->native.length);
if (IS_ERR(tx_pipe->fifo)) {
- dev_err(dev, "failed to acquire TX fifo\n");
+ dev_err(&smem->dev, "failed to acquire TX fifo\n");
ret = PTR_ERR(tx_pipe->fifo);
goto err_put_dev;
}

+ rx_pipe->smem = smem;
rx_pipe->native.avail = glink_smem_rx_avail;
rx_pipe->native.peak = glink_smem_rx_peak;
rx_pipe->native.advance = glink_smem_rx_advance;
- rx_pipe->remote_pid = remote_pid;

+ tx_pipe->smem = smem;
tx_pipe->native.avail = glink_smem_tx_avail;
tx_pipe->native.write = glink_smem_tx_write;
- tx_pipe->remote_pid = remote_pid;

*rx_pipe->tail = 0;
*tx_pipe->head = 0;

- glink = qcom_glink_native_probe(dev,
+ glink = qcom_glink_native_probe(&smem->dev,
GLINK_FEATURE_INTENT_REUSE,
&rx_pipe->native, &tx_pipe->native,
false);
@@ -285,17 +298,22 @@ struct qcom_glink *qcom_glink_smem_register(struct device *parent,
goto err_put_dev;
}

- return glink;
+ smem->glink = glink;
+
+ return smem;
+

err_put_dev:
- device_unregister(dev);
+ device_unregister(&smem->dev);

return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(qcom_glink_smem_register);

-void qcom_glink_smem_unregister(struct qcom_glink *glink)
+void qcom_glink_smem_unregister(struct qcom_glink_smem *smem)
{
+ struct qcom_glink *glink = smem->glink;
+
qcom_glink_native_remove(glink);
qcom_glink_native_unregister(glink);
}
diff --git a/include/linux/rpmsg/qcom_glink.h b/include/linux/rpmsg/qcom_glink.h
index 22fc3a69b683..bfbd48f435fa 100644
--- a/include/linux/rpmsg/qcom_glink.h
+++ b/include/linux/rpmsg/qcom_glink.h
@@ -5,7 +5,7 @@

#include <linux/device.h>

-struct qcom_glink;
+struct qcom_glink_smem;

#if IS_ENABLED(CONFIG_RPMSG_QCOM_GLINK)
void qcom_glink_ssr_notify(const char *ssr_name);
@@ -15,20 +15,20 @@ static inline void qcom_glink_ssr_notify(const char *ssr_name) {}

#if IS_ENABLED(CONFIG_RPMSG_QCOM_GLINK_SMEM)

-struct qcom_glink *qcom_glink_smem_register(struct device *parent,
- struct device_node *node);
-void qcom_glink_smem_unregister(struct qcom_glink *glink);
+struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
+ struct device_node *node);
+void qcom_glink_smem_unregister(struct qcom_glink_smem *glink);

#else

-static inline struct qcom_glink *
+static inline struct qcom_glink_smem *
qcom_glink_smem_register(struct device *parent,
struct device_node *node)
{
return NULL;
}

-static inline void qcom_glink_smem_unregister(struct qcom_glink *glink) {}
+static inline void qcom_glink_smem_unregister(struct qcom_glink_smem *glink) {}
#endif

#endif
--
2.37.3


2023-01-25 06:31:10

by Chris Lew

[permalink] [raw]
Subject: Re: [PATCH 2/6] rpmsg: glink: smem: Wrap driver context



On 1/9/2023 2:39 PM, Bjorn Andersson wrote:
> The Glink SMEM driver allocates a struct device and hangs two
> devres-allocated pipe objects thereon. To facilitate the move of
> interrupt and mailbox handling to the driver, introduce a wrapper object
> capturing the device, glink reference and remote processor id.
>
> The type of the remoteproc reference is updated, as these are
> specifically targetting the SMEM implementation.

s/targetting/targeting

>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
> drivers/remoteproc/qcom_common.h | 3 +-
> drivers/rpmsg/qcom_glink_smem.c | 76 ++++++++++++++++++++------------
> include/linux/rpmsg/qcom_glink.h | 12 ++---
> 3 files changed, 55 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
> index c35adf730be0..2747c7d9ba44 100644
> --- a/drivers/remoteproc/qcom_common.h
> +++ b/drivers/remoteproc/qcom_common.h
> @@ -6,6 +6,7 @@
> #include "remoteproc_internal.h"
> #include <linux/soc/qcom/qmi.h>
>
> +struct qcom_glink_smem;
> struct qcom_sysmon;
>
> struct qcom_rproc_glink {
> @@ -15,7 +16,7 @@ struct qcom_rproc_glink {
>
> struct device *dev;
> struct device_node *node;
> - struct qcom_glink *edge;
> + struct qcom_glink_smem *edge;
> };
>
> struct qcom_rproc_subdev {
> diff --git a/drivers/rpmsg/qcom_glink_smem.c b/drivers/rpmsg/qcom_glink_smem.c
> index 579bc4443f6d..703e63fa5a86 100644
> --- a/drivers/rpmsg/qcom_glink_smem.c
> +++ b/drivers/rpmsg/qcom_glink_smem.c
> @@ -33,6 +33,14 @@
> #define SMEM_GLINK_NATIVE_XPRT_FIFO_0 479
> #define SMEM_GLINK_NATIVE_XPRT_FIFO_1 480
>
> +struct qcom_glink_smem {
> + struct device dev;
> +
> + struct qcom_glink *glink;
> +
> + u32 remote_pid;
> +};
> +
> struct glink_smem_pipe {
> struct qcom_glink_pipe native;
>
> @@ -41,7 +49,7 @@ struct glink_smem_pipe {
>
> void *fifo;
>
> - int remote_pid;
> + struct qcom_glink_smem *smem;
> };
>
> #define to_smem_pipe(p) container_of(p, struct glink_smem_pipe, native)
> @@ -49,13 +57,14 @@ struct glink_smem_pipe {
> static size_t glink_smem_rx_avail(struct qcom_glink_pipe *np)
> {
> struct glink_smem_pipe *pipe = to_smem_pipe(np);
> + struct qcom_glink_smem *smem = pipe->smem;
> size_t len;
> void *fifo;
> u32 head;
> u32 tail;
>
> if (!pipe->fifo) {
> - fifo = qcom_smem_get(pipe->remote_pid,
> + fifo = qcom_smem_get(smem->remote_pid,
> SMEM_GLINK_NATIVE_XPRT_FIFO_1, &len);
> if (IS_ERR(fifo)) {
> pr_err("failed to acquire RX fifo handle: %ld\n",
> @@ -179,45 +188,49 @@ static void glink_smem_tx_write(struct qcom_glink_pipe *glink_pipe,
>
> static void qcom_glink_smem_release(struct device *dev)
> {
> - kfree(dev);
> + struct qcom_glink_smem *smem = container_of(dev, struct qcom_glink_smem, dev);
> +
> + kfree(smem);
> }
>
> -struct qcom_glink *qcom_glink_smem_register(struct device *parent,
> - struct device_node *node)
> +struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
> + struct device_node *node)
> {
> struct glink_smem_pipe *rx_pipe;
> struct glink_smem_pipe *tx_pipe;
> struct qcom_glink *glink;
> - struct device *dev;
> + struct qcom_glink_smem *smem;

I think we're following reverse christmas tree in this file

> u32 remote_pid;
> __le32 *descs;
> size_t size;
> int ret;
>
> - dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> - if (!dev)
> + smem = kzalloc(sizeof(*smem), GFP_KERNEL);
> + if (!smem)
> return ERR_PTR(-ENOMEM);
>

Would it be proper to keep a pointer to dev and avoid all the changes to
smem->dev use?

dev = &smem->dev;

> - dev->parent = parent;
> - dev->of_node = node;
> - dev->release = qcom_glink_smem_release;
> - dev_set_name(dev, "%s:%pOFn", dev_name(parent->parent), node);
> - ret = device_register(dev);
> + smem->dev.parent = parent;
> + smem->dev.of_node = node;
> + smem->dev.release = qcom_glink_smem_release;
> + dev_set_name(&smem->dev, "%s:%pOFn", dev_name(parent->parent), node);
> + ret = device_register(&smem->dev);
> if (ret) {
> pr_err("failed to register glink edge\n");
> - put_device(dev);
> + put_device(&smem->dev);
> return ERR_PTR(ret);
> }
>
> - ret = of_property_read_u32(dev->of_node, "qcom,remote-pid",
> + ret = of_property_read_u32(smem->dev.of_node, "qcom,remote-pid",
> &remote_pid);
> if (ret) {
> - dev_err(dev, "failed to parse qcom,remote-pid\n");
> + dev_err(&smem->dev, "failed to parse qcom,remote-pid\n");
> goto err_put_dev;
> }
>
> - rx_pipe = devm_kzalloc(dev, sizeof(*rx_pipe), GFP_KERNEL);
> - tx_pipe = devm_kzalloc(dev, sizeof(*tx_pipe), GFP_KERNEL);
> + smem->remote_pid = remote_pid;
> +
> + rx_pipe = devm_kzalloc(&smem->dev, sizeof(*rx_pipe), GFP_KERNEL);
> + tx_pipe = devm_kzalloc(&smem->dev, sizeof(*tx_pipe), GFP_KERNEL);
> if (!rx_pipe || !tx_pipe) {
> ret = -ENOMEM;
> goto err_put_dev;
> @@ -226,20 +239,20 @@ struct qcom_glink *qcom_glink_smem_register(struct device *parent,
> ret = qcom_smem_alloc(remote_pid,
> SMEM_GLINK_NATIVE_XPRT_DESCRIPTOR, 32);
> if (ret && ret != -EEXIST) {
> - dev_err(dev, "failed to allocate glink descriptors\n");
> + dev_err(&smem->dev, "failed to allocate glink descriptors\n");
> goto err_put_dev;
> }
>
> descs = qcom_smem_get(remote_pid,
> SMEM_GLINK_NATIVE_XPRT_DESCRIPTOR, &size);
> if (IS_ERR(descs)) {
> - dev_err(dev, "failed to acquire xprt descriptor\n");
> + dev_err(&smem->dev, "failed to acquire xprt descriptor\n");
> ret = PTR_ERR(descs);
> goto err_put_dev;
> }
>
> if (size != 32) {
> - dev_err(dev, "glink descriptor of invalid size\n");
> + dev_err(&smem->dev, "glink descriptor of invalid size\n");
> ret = -EINVAL;
> goto err_put_dev;
> }
> @@ -252,31 +265,31 @@ struct qcom_glink *qcom_glink_smem_register(struct device *parent,
> ret = qcom_smem_alloc(remote_pid, SMEM_GLINK_NATIVE_XPRT_FIFO_0,
> SZ_16K);
> if (ret && ret != -EEXIST) {
> - dev_err(dev, "failed to allocate TX fifo\n");
> + dev_err(&smem->dev, "failed to allocate TX fifo\n");
> goto err_put_dev;
> }
>
> tx_pipe->fifo = qcom_smem_get(remote_pid, SMEM_GLINK_NATIVE_XPRT_FIFO_0,
> &tx_pipe->native.length);
> if (IS_ERR(tx_pipe->fifo)) {
> - dev_err(dev, "failed to acquire TX fifo\n");
> + dev_err(&smem->dev, "failed to acquire TX fifo\n");
> ret = PTR_ERR(tx_pipe->fifo);
> goto err_put_dev;
> }
>
> + rx_pipe->smem = smem;
> rx_pipe->native.avail = glink_smem_rx_avail;
> rx_pipe->native.peak = glink_smem_rx_peak;
> rx_pipe->native.advance = glink_smem_rx_advance;
> - rx_pipe->remote_pid = remote_pid;
>
> + tx_pipe->smem = smem;
> tx_pipe->native.avail = glink_smem_tx_avail;
> tx_pipe->native.write = glink_smem_tx_write;
> - tx_pipe->remote_pid = remote_pid;
>
> *rx_pipe->tail = 0;
> *tx_pipe->head = 0;
>
> - glink = qcom_glink_native_probe(dev,
> + glink = qcom_glink_native_probe(&smem->dev,
> GLINK_FEATURE_INTENT_REUSE,
> &rx_pipe->native, &tx_pipe->native,
> false);
> @@ -285,17 +298,22 @@ struct qcom_glink *qcom_glink_smem_register(struct device *parent,
> goto err_put_dev;
> }
>
> - return glink;
> + smem->glink = glink;
> +
> + return smem;
> +
>
> err_put_dev:
> - device_unregister(dev);
> + device_unregister(&smem->dev);
>
> return ERR_PTR(ret);
> }
> EXPORT_SYMBOL_GPL(qcom_glink_smem_register);
>
> -void qcom_glink_smem_unregister(struct qcom_glink *glink)
> +void qcom_glink_smem_unregister(struct qcom_glink_smem *smem)
> {
> + struct qcom_glink *glink = smem->glink;
> +
> qcom_glink_native_remove(glink);
> qcom_glink_native_unregister(glink);
> }
> diff --git a/include/linux/rpmsg/qcom_glink.h b/include/linux/rpmsg/qcom_glink.h
> index 22fc3a69b683..bfbd48f435fa 100644
> --- a/include/linux/rpmsg/qcom_glink.h
> +++ b/include/linux/rpmsg/qcom_glink.h
> @@ -5,7 +5,7 @@
>
> #include <linux/device.h>
>
> -struct qcom_glink;
> +struct qcom_glink_smem;
>
> #if IS_ENABLED(CONFIG_RPMSG_QCOM_GLINK)
> void qcom_glink_ssr_notify(const char *ssr_name);
> @@ -15,20 +15,20 @@ static inline void qcom_glink_ssr_notify(const char *ssr_name) {}
>
> #if IS_ENABLED(CONFIG_RPMSG_QCOM_GLINK_SMEM)
>
> -struct qcom_glink *qcom_glink_smem_register(struct device *parent,
> - struct device_node *node);
> -void qcom_glink_smem_unregister(struct qcom_glink *glink);
> +struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
> + struct device_node *node);
> +void qcom_glink_smem_unregister(struct qcom_glink_smem *glink);
>
> #else
>
> -static inline struct qcom_glink *
> +static inline struct qcom_glink_smem *
> qcom_glink_smem_register(struct device *parent,
> struct device_node *node)
> {
> return NULL;
> }
>
> -static inline void qcom_glink_smem_unregister(struct qcom_glink *glink) {}
> +static inline void qcom_glink_smem_unregister(struct qcom_glink_smem *glink) {}
> #endif
>
> #endif

2023-01-25 18:56:02

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 2/6] rpmsg: glink: smem: Wrap driver context

On Tue, Jan 24, 2023 at 10:30:42PM -0800, Chris Lew wrote:
>
>
> On 1/9/2023 2:39 PM, Bjorn Andersson wrote:
> > The Glink SMEM driver allocates a struct device and hangs two
> > devres-allocated pipe objects thereon. To facilitate the move of
> > interrupt and mailbox handling to the driver, introduce a wrapper object
> > capturing the device, glink reference and remote processor id.
> >
> > The type of the remoteproc reference is updated, as these are
> > specifically targetting the SMEM implementation.
>
> s/targetting/targeting
>

Thank you.

> >
> > Signed-off-by: Bjorn Andersson <[email protected]>
> > ---
> > drivers/remoteproc/qcom_common.h | 3 +-
> > drivers/rpmsg/qcom_glink_smem.c | 76 ++++++++++++++++++++------------
> > include/linux/rpmsg/qcom_glink.h | 12 ++---
> > 3 files changed, 55 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
> > index c35adf730be0..2747c7d9ba44 100644
> > --- a/drivers/remoteproc/qcom_common.h
> > +++ b/drivers/remoteproc/qcom_common.h
> > @@ -6,6 +6,7 @@
> > #include "remoteproc_internal.h"
> > #include <linux/soc/qcom/qmi.h>
> > +struct qcom_glink_smem;
> > struct qcom_sysmon;
> > struct qcom_rproc_glink {
> > @@ -15,7 +16,7 @@ struct qcom_rproc_glink {
> > struct device *dev;
> > struct device_node *node;
> > - struct qcom_glink *edge;
> > + struct qcom_glink_smem *edge;
> > };
> > struct qcom_rproc_subdev {
> > diff --git a/drivers/rpmsg/qcom_glink_smem.c b/drivers/rpmsg/qcom_glink_smem.c
> > index 579bc4443f6d..703e63fa5a86 100644
> > --- a/drivers/rpmsg/qcom_glink_smem.c
> > +++ b/drivers/rpmsg/qcom_glink_smem.c
> > @@ -33,6 +33,14 @@
> > #define SMEM_GLINK_NATIVE_XPRT_FIFO_0 479
> > #define SMEM_GLINK_NATIVE_XPRT_FIFO_1 480
> > +struct qcom_glink_smem {
> > + struct device dev;
> > +
> > + struct qcom_glink *glink;
> > +
> > + u32 remote_pid;
> > +};
> > +
> > struct glink_smem_pipe {
> > struct qcom_glink_pipe native;
> > @@ -41,7 +49,7 @@ struct glink_smem_pipe {
> > void *fifo;
> > - int remote_pid;
> > + struct qcom_glink_smem *smem;
> > };
> > #define to_smem_pipe(p) container_of(p, struct glink_smem_pipe, native)
> > @@ -49,13 +57,14 @@ struct glink_smem_pipe {
> > static size_t glink_smem_rx_avail(struct qcom_glink_pipe *np)
> > {
> > struct glink_smem_pipe *pipe = to_smem_pipe(np);
> > + struct qcom_glink_smem *smem = pipe->smem;
> > size_t len;
> > void *fifo;
> > u32 head;
> > u32 tail;
> > if (!pipe->fifo) {
> > - fifo = qcom_smem_get(pipe->remote_pid,
> > + fifo = qcom_smem_get(smem->remote_pid,
> > SMEM_GLINK_NATIVE_XPRT_FIFO_1, &len);
> > if (IS_ERR(fifo)) {
> > pr_err("failed to acquire RX fifo handle: %ld\n",
> > @@ -179,45 +188,49 @@ static void glink_smem_tx_write(struct qcom_glink_pipe *glink_pipe,
> > static void qcom_glink_smem_release(struct device *dev)
> > {
> > - kfree(dev);
> > + struct qcom_glink_smem *smem = container_of(dev, struct qcom_glink_smem, dev);
> > +
> > + kfree(smem);
> > }
> > -struct qcom_glink *qcom_glink_smem_register(struct device *parent,
> > - struct device_node *node)
> > +struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
> > + struct device_node *node)
> > {
> > struct glink_smem_pipe *rx_pipe;
> > struct glink_smem_pipe *tx_pipe;
> > struct qcom_glink *glink;
> > - struct device *dev;
> > + struct qcom_glink_smem *smem;
>
> I think we're following reverse christmas tree in this file
>
> > u32 remote_pid;
> > __le32 *descs;
> > size_t size;
> > int ret;
> > - dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> > - if (!dev)
> > + smem = kzalloc(sizeof(*smem), GFP_KERNEL);
> > + if (!smem)
> > return ERR_PTR(-ENOMEM);
> >
>
> Would it be proper to keep a pointer to dev and avoid all the changes to
> smem->dev use?
>
> dev = &smem->dev;
>

That seems reasonable. Will respin accordingly.

Thanks,
Bjorn