[PATCH v2 0/6] Add chrdev and name query support for GLINK

[PATCH v2 0/6] Add chrdev and name query support for GLINK

[Chris Lew]

Add support for the GLINK rpmsg transport to register a rpmsg chrdev.
This will create the rpmsg_ctrl nodes for userspace clients to open 
rpmsg epts. Create a label property that will help userspace clients
distinguish between the different GLINK links. The rpmsg chrdev 
allocation is done by allocating a local channel which also allocates
an ept. We need to add some guards against edge cases for this chrdev
because it will never fully open.

Changes since v1:
- Add explanation to dt-bindings commit message
- Add patch complete_all the open_req/ack variables
- Add patch to prevent null pointer dereference in chrdev channel release
- Change chrdev allocation to use glink channel allocation
- Change glink attr struct to const

Chris Lew (6):
  dt-bindings: soc: qcom: Add label for GLINK bindings
  rpmsg: glink: Store edge name for glink device
  rpmsg: glink: Use complete_all for open states
  rpmsg: Guard against null endpoint ops in destroy
  rpmsg: glink: Add support for rpmsg glink chrdev
  rpmsg: glink: Expose rpmsg name attr for glink

 .../devicetree/bindings/soc/qcom/qcom,glink.txt    |  5 ++
 drivers/rpmsg/qcom_glink_native.c                  | 68 +++++++++++++++++++++-
 drivers/rpmsg/rpmsg_core.c                         |  2 +-
 3 files changed, 71 insertions(+), 4 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 1/6] dt-bindings: soc: qcom: Add label for GLINK bindings

[Chris Lew]

There are GLINK clients who open the same channel on multiple GLINK
links. These clients need a way to distinguish which remoteproc they
are communicating to. Add a label property to identify the edge this
node represents.

Signed-off-by: Chris Lew <clew@codeaurora.org>
---

Changes since v1:
- Add explanation for label in commit message

 Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt
index 9663cab52246..0b8cc533ca83 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt
@@ -10,6 +10,11 @@ edge.
 	Value type: <stringlist>
 	Definition: must be "qcom,glink-rpm"
 
+- label:
+	Usage: optional
+	Value type: <string>
+	Definition: should specify the subsystem name this edge corresponds to.
+
 - interrupts:
 	Usage: required
 	Value type: <prop-encoded-array>
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 2/6] rpmsg: glink: Store edge name for glink device

[Chris Lew]

Channels may need to identify the edge their channel was probed for.
Store the edge name by reading the label property from device tree or
default to the node name.

Signed-off-by: Chris Lew <clew@codeaurora.org>
---

Changes since v1:
- None

 drivers/rpmsg/qcom_glink_native.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 768ef542a841..c3f548e2ff20 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -101,6 +101,8 @@ struct glink_core_rx_intent {
 struct qcom_glink {
 	struct device *dev;
 
+	const char *name;
+
 	struct mbox_client mbox_client;
 	struct mbox_chan *mbox_chan;
 
@@ -1580,6 +1582,10 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev,
 	idr_init(&glink->lcids);
 	idr_init(&glink->rcids);
 
+	ret = of_property_read_string(dev->of_node, "label", &glink->name);
+	if (ret < 0)
+		glink->name = dev->of_node->name;
+
 	glink->mbox_client.dev = dev;
 	glink->mbox_client.knows_txdone = true;
 	glink->mbox_chan = mbox_request_channel(&glink->mbox_client, 0);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 3/6] rpmsg: glink: Use complete_all for open states

[Chris Lew]

The open_req and open_ack completion variables are the state variables
to represet a remote channel as open. Use complete_all so there are no
races with waiters and using completion_done.

Signed-off-by: Chris Lew <clew@codeaurora.org>
---

Changes since v1:
- New change

 drivers/rpmsg/qcom_glink_native.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index c3f548e2ff20..120d6b9bb9f3 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -957,7 +957,7 @@ static int qcom_glink_rx_open_ack(struct qcom_glink *glink, unsigned int lcid)
 		return -EINVAL;
 	}
 
-	complete(&channel->open_ack);
+	complete_all(&channel->open_ack);
 
 	return 0;
 }
@@ -1401,7 +1401,7 @@ static int qcom_glink_rx_open(struct qcom_glink *glink, unsigned int rcid,
 	channel->rcid = ret;
 	spin_unlock_irqrestore(&glink->idr_lock, flags);
 
-	complete(&channel->open_req);
+	complete_all(&channel->open_req);
 
 	if (create_device) {
 		rpdev = kzalloc(sizeof(*rpdev), GFP_KERNEL);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 4/6] rpmsg: Guard against null endpoint ops in destroy

[Chris Lew]

In RPMSG GLINK the chrdev device will allocate an ept as part of the
rpdev creation. This device will not register endpoint ops even though
it has an allocated ept. Protect against the case where the device is
being destroyed.

Signed-off-by: Chris Lew <clew@codeaurora.org>
---

Changes since v1:
- New change

 drivers/rpmsg/rpmsg_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index 920a02f0462c..7bfe36afccc5 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -88,7 +88,7 @@ struct rpmsg_endpoint *rpmsg_create_ept(struct rpmsg_device *rpdev,
  */
 void rpmsg_destroy_ept(struct rpmsg_endpoint *ept)
 {
-	if (ept)
+	if (ept && ept->ops)
 		ept->ops->destroy_ept(ept);
 }
 EXPORT_SYMBOL(rpmsg_destroy_ept);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 5/6] rpmsg: glink: Add support for rpmsg glink chrdev

[Chris Lew]

RPMSG provides a char device interface to userspace. Probe the rpmsg
chrdev channel to enable the rpmsg_ctrl device creation on glink
transports.

Signed-off-by: Chris Lew <clew@codeaurora.org>
---

Changes since v1:
- Use qcom_glink_alloc_channel to create the chrdev rpmsg device

 drivers/rpmsg/qcom_glink_native.c | 40 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 120d6b9bb9f3..1418926f6110 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -1166,7 +1166,7 @@ static int qcom_glink_announce_create(struct rpmsg_device *rpdev)
 	__be32 *val = defaults;
 	int size;
 
-	if (glink->intentless)
+	if (glink->intentless || !completion_done(&channel->open_ack))
 		return 0;
 
 	prop = of_find_property(np, "qcom,intents", NULL);
@@ -1552,6 +1552,40 @@ static void qcom_glink_work(struct work_struct *work)
 	}
 }
 
+static void qcom_glink_device_release(struct device *dev)
+{
+	struct rpmsg_device *rpdev = to_rpmsg_device(dev);
+	struct glink_channel *channel = to_glink_channel(rpdev->ept);
+
+	/* Release qcom_glink_alloc_channel() reference */
+	kref_put(&channel->refcount, qcom_glink_channel_release);
+	kfree(rpdev);
+}
+
+static int qcom_glink_create_chrdev(struct qcom_glink *glink)
+{
+	struct rpmsg_device *rpdev;
+	struct glink_channel *channel;
+
+	rpdev = kzalloc(sizeof(*rpdev), GFP_KERNEL);
+	if (!rpdev)
+		return -ENOMEM;
+
+	channel = qcom_glink_alloc_channel(glink, "rpmsg_chrdev");
+	if (IS_ERR(channel)) {
+		kfree(rpdev);
+		return PTR_ERR(channel);
+	}
+	channel->rpdev = rpdev;
+
+	rpdev->ept = &channel->ept;
+	rpdev->ops = &glink_device_ops;
+	rpdev->dev.parent = glink->dev;
+	rpdev->dev.release = qcom_glink_device_release;
+
+	return rpmsg_chrdev_register_device(rpdev);
+}
+
 struct qcom_glink *qcom_glink_native_probe(struct device *dev,
 					   unsigned long features,
 					   struct qcom_glink_pipe *rx,
@@ -1611,6 +1645,10 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev,
 	if (ret)
 		return ERR_PTR(ret);
 
+	ret = qcom_glink_create_chrdev(glink);
+	if (ret)
+		dev_err(glink->dev, "failed to register chrdev\n");
+
 	return glink;
 }
 EXPORT_SYMBOL_GPL(qcom_glink_native_probe);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 6/6] rpmsg: glink: Expose rpmsg name attr for glink

[Chris Lew]

Expose the name field as an attr so clients listening to uevents for
rpmsg can identify the edge the events correspond to.

Signed-off-by: Chris Lew <clew@codeaurora.org>
---

Changes since v1:
- Add const to attribute struct
- Get name from glink channel

 drivers/rpmsg/qcom_glink_native.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 1418926f6110..72240614d4ca 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -1552,6 +1552,22 @@ static void qcom_glink_work(struct work_struct *work)
 	}
 }
 
+static ssize_t rpmsg_name_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	struct rpmsg_device *rpdev = to_rpmsg_device(dev);
+	struct glink_channel *channel = to_glink_channel(rpdev->ept);
+
+	return snprintf(buf, RPMSG_NAME_SIZE, "%s\n", channel->glink->name);
+}
+static DEVICE_ATTR_RO(rpmsg_name);
+
+static const struct attribute *qcom_glink_attrs[] = {
+	&dev_attr_rpmsg_name.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(qcom_glink);
+
 static void qcom_glink_device_release(struct device *dev)
 {
 	struct rpmsg_device *rpdev = to_rpmsg_device(dev);
@@ -1601,6 +1617,8 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev,
 		return ERR_PTR(-ENOMEM);
 
 	glink->dev = dev;
+	glink->dev->groups = qcom_glink_groups;
+
 	glink->tx_pipe = tx;
 	glink->rx_pipe = rx;
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 6/6] rpmsg: glink: Expose rpmsg name attr for glink

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2119 bytes --]

Hi Chris,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.17-rc2 next-20180426]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chris-Lew/Add-chrdev-and-name-query-support-for-GLINK/20180427-023202
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: i386-randconfig-x016-201816 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from include/linux/kobject.h:20:0,
                    from include/linux/irqdesc.h:6,
                    from include/linux/irq.h:517,
                    from arch/x86/include/asm/hardirq.h:6,
                    from include/linux/hardirq.h:9,
                    from include/linux/interrupt.h:11,
                    from drivers/rpmsg/qcom_glink_native.c:15:
>> drivers/rpmsg/qcom_glink_native.c:1569:18: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
    ATTRIBUTE_GROUPS(qcom_glink);
                     ^
   include/linux/sysfs.h:154:11: note: in definition of macro 'ATTRIBUTE_GROUPS'
     .attrs = _name##_attrs,     \
              ^~~~~
   drivers/rpmsg/qcom_glink_native.c:1569:18: note: (near initialization for 'qcom_glink_group.attrs')
    ATTRIBUTE_GROUPS(qcom_glink);
                     ^
   include/linux/sysfs.h:154:11: note: in definition of macro 'ATTRIBUTE_GROUPS'
     .attrs = _name##_attrs,     \
              ^~~~~
   cc1: some warnings being treated as errors

vim +1569 drivers/rpmsg/qcom_glink_native.c

  1564	
  1565	static const struct attribute *qcom_glink_attrs[] = {
  1566		&dev_attr_rpmsg_name.attr,
  1567		NULL
  1568	};
> 1569	ATTRIBUTE_GROUPS(qcom_glink);
  1570	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29152 bytes --]

Re: [PATCH v2 6/6] rpmsg: glink: Expose rpmsg name attr for glink

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3108 bytes --]

Hi Chris,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.17-rc2 next-20180426]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chris-Lew/Add-chrdev-and-name-query-support-for-GLINK/20180427-023202
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: i386-randconfig-b0-04270034 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kobject.h:20:0,
                    from include/linux/irqdesc.h:6,
                    from include/linux/irq.h:517,
                    from arch/x86/include/asm/hardirq.h:6,
                    from include/linux/hardirq.h:9,
                    from include/linux/interrupt.h:11,
                    from drivers/rpmsg/qcom_glink_native.c:15:
>> include/linux/sysfs.h:153:21: warning: initialization from incompatible pointer type
    static const struct attribute_group _name##_group = {  \
                        ^
>> drivers/rpmsg/qcom_glink_native.c:1569:1: note: in expansion of macro 'ATTRIBUTE_GROUPS'
    ATTRIBUTE_GROUPS(qcom_glink);
    ^
   include/linux/sysfs.h:153:21: warning: (near initialization for 'qcom_glink_group.attrs')
    static const struct attribute_group _name##_group = {  \
                        ^
>> drivers/rpmsg/qcom_glink_native.c:1569:1: note: in expansion of macro 'ATTRIBUTE_GROUPS'
    ATTRIBUTE_GROUPS(qcom_glink);
    ^
--
   In file included from include/linux/kobject.h:20:0,
                    from include/linux/irqdesc.h:6,
                    from include/linux/irq.h:517,
                    from arch/x86/include/asm/hardirq.h:6,
                    from include/linux/hardirq.h:9,
                    from include/linux/interrupt.h:11,
                    from drivers//rpmsg/qcom_glink_native.c:15:
>> include/linux/sysfs.h:153:21: warning: initialization from incompatible pointer type
    static const struct attribute_group _name##_group = {  \
                        ^
   drivers//rpmsg/qcom_glink_native.c:1569:1: note: in expansion of macro 'ATTRIBUTE_GROUPS'
    ATTRIBUTE_GROUPS(qcom_glink);
    ^
   include/linux/sysfs.h:153:21: warning: (near initialization for 'qcom_glink_group.attrs')
    static const struct attribute_group _name##_group = {  \
                        ^
   drivers//rpmsg/qcom_glink_native.c:1569:1: note: in expansion of macro 'ATTRIBUTE_GROUPS'
    ATTRIBUTE_GROUPS(qcom_glink);
    ^

vim +/ATTRIBUTE_GROUPS +1569 drivers/rpmsg/qcom_glink_native.c

  1564	
  1565	static const struct attribute *qcom_glink_attrs[] = {
  1566		&dev_attr_rpmsg_name.attr,
  1567		NULL
  1568	};
> 1569	ATTRIBUTE_GROUPS(qcom_glink);
  1570	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25198 bytes --]

Re: [PATCH v2 6/6] rpmsg: glink: Expose rpmsg name attr for glink

[kbuild test robot]

Hi Chris,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.17-rc2 next-20180426]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chris-Lew/Add-chrdev-and-name-query-support-for-GLINK/20180427-023202
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   drivers/rpmsg/qcom_glink_native.c:624:16: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [unsigned] [usertype] id @@    got  short [unsigned] [usertype] id @@
   drivers/rpmsg/qcom_glink_native.c:624:16:    expected unsigned short [unsigned] [usertype] id
   drivers/rpmsg/qcom_glink_native.c:624:16:    got restricted __le16 [usertype] <noident>
   drivers/rpmsg/qcom_glink_native.c:625:18: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [unsigned] [usertype] lcid @@    got  short [unsigned] [usertype] lcid @@
   drivers/rpmsg/qcom_glink_native.c:625:18:    expected unsigned short [unsigned] [usertype] lcid
   drivers/rpmsg/qcom_glink_native.c:625:18:    got restricted __le16 [usertype] <noident>
   drivers/rpmsg/qcom_glink_native.c:626:19: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [unsigned] [usertype] count @@    got ed int [unsigned] [usertype] count @@
   drivers/rpmsg/qcom_glink_native.c:626:19:    expected unsigned int [unsigned] [usertype] count
   drivers/rpmsg/qcom_glink_native.c:626:19:    got restricted __le32 [usertype] <noident>
   drivers/rpmsg/qcom_glink_native.c:627:18: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [unsigned] [usertype] size @@    got ed int [unsigned] [usertype] size @@
   drivers/rpmsg/qcom_glink_native.c:627:18:    expected unsigned int [unsigned] [usertype] size
   drivers/rpmsg/qcom_glink_native.c:627:18:    got restricted __le32 [usertype] <noident>
   drivers/rpmsg/qcom_glink_native.c:628:18: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [unsigned] [usertype] liid @@    got ed int [unsigned] [usertype] liid @@
   drivers/rpmsg/qcom_glink_native.c:628:18:    expected unsigned int [unsigned] [usertype] liid
   drivers/rpmsg/qcom_glink_native.c:628:18:    got restricted __le32 [usertype] <noident>
>> drivers/rpmsg/qcom_glink_native.c:1569:1: sparse: incorrect type in initializer (different modifiers) @@    expected struct attribute **attrs @@    got structstruct attribute **attrs @@
   drivers/rpmsg/qcom_glink_native.c:1569:1:    expected struct attribute **attrs
   drivers/rpmsg/qcom_glink_native.c:1569:1:    got struct attribute const **<noident>
   In file included from include/linux/kobject.h:20:0,
                    from include/linux/irqdesc.h:6,
                    from include/linux/irq.h:517,
                    from arch/x86/include/asm/hardirq.h:6,
                    from include/linux/hardirq.h:9,
                    from include/linux/interrupt.h:11,
                    from drivers/rpmsg/qcom_glink_native.c:15:
   drivers/rpmsg/qcom_glink_native.c:1569:18: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
    ATTRIBUTE_GROUPS(qcom_glink);
                     ^
   include/linux/sysfs.h:154:11: note: in definition of macro 'ATTRIBUTE_GROUPS'
     .attrs = _name##_attrs,     31-           ^~~~~
   drivers/rpmsg/qcom_glink_native.c:1569:18: note: (near initialization for 'qcom_glink_group.attrs')
    ATTRIBUTE_GROUPS(qcom_glink);
                     ^
   include/linux/sysfs.h:154:11: note: in definition of macro 'ATTRIBUTE_GROUPS'
     .attrs = _name##_attrs,     37-           ^~~~~
   cc1: some warnings being treated as errors

vim +1569 drivers/rpmsg/qcom_glink_native.c

  1564	
  1565	static const struct attribute *qcom_glink_attrs[] = {
  1566		&dev_attr_rpmsg_name.attr,
  1567		NULL
  1568	};
> 1569	ATTRIBUTE_GROUPS(qcom_glink);
  1570	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation