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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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 <quic_bjorande@xxxxxxxxxxx>
---
  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



[Index of Archives]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Photo Sharing]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux