| Message ID | 1513634534-22861-2-git-send-email-clew@codeaurora.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Mon, Dec 18, 2017 at 02:02:09PM -0800, Chris Lew wrote: > Add a label property to identify the edge this node represents. Why does a user need to know this? > > Signed-off-by: Chris Lew <clew@codeaurora.org> > --- > 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed 20 Dec 10:30 PST 2017, Rob Herring wrote: > On Mon, Dec 18, 2017 at 02:02:09PM -0800, Chris Lew wrote: > > Add a label property to identify the edge this node represents. > > Why does a user need to know this? > We have multiple remoteproc instances, each one having one or more associated SMD or GLINK links (this node), exposing logical communication channels. Some of these logical channels are exposed to user space and we need a way to distinguish them there. In the current implementation of SMD this value goes straight into an sysfs attribute that we can use when writing udev rules and for the DIAG implementation to pair up channels related to the same remoteproc. This adds the equivalent information for glink-backed channels. I'm therefor in favor of picking this patch. Regards, Bjorn > > > > Signed-off-by: Chris Lew <clew@codeaurora.org> > > --- > > 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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/20/2017 05:35 PM, Bjorn Andersson wrote: > On Wed 20 Dec 10:30 PST 2017, Rob Herring wrote: > >> On Mon, Dec 18, 2017 at 02:02:09PM -0800, Chris Lew wrote: >>> Add a label property to identify the edge this node represents. >> Why does a user need to know this? >> > We have multiple remoteproc instances, each one having one or more > associated SMD or GLINK links (this node), exposing logical > communication channels. Some of these logical channels are exposed to > user space and we need a way to distinguish them there. > > In the current implementation of SMD this value goes straight into an > sysfs attribute that we can use when writing udev rules and for the DIAG > implementation to pair up channels related to the same remoteproc. This > adds the equivalent information for glink-backed channels. > > > I'm therefor in favor of picking this patch. Please add these details to the commit log. Just writing what the patch is doing isn't very helpful.
On Wed, Dec 20, 2017 at 7:35 PM, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > On Wed 20 Dec 10:30 PST 2017, Rob Herring wrote: > >> On Mon, Dec 18, 2017 at 02:02:09PM -0800, Chris Lew wrote: >> > Add a label property to identify the edge this node represents. >> >> Why does a user need to know this? >> > > We have multiple remoteproc instances, each one having one or more > associated SMD or GLINK links (this node), exposing logical > communication channels. Some of these logical channels are exposed to > user space and we need a way to distinguish them there. They should have some sort of discoverable property. There must be some reason why you need a specific channel. I think of label as being a sticker or silkscreen on a device which doesn't seem to be the case here. This seems more like wanting fixed device numbering like disks and I assume you know the position on fixed device numbers. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed 20 Dec 10:30 PST 2017, Rob Herring wrote: > On Mon, Dec 18, 2017 at 02:02:09PM -0800, Chris Lew wrote: > > Add a label property to identify the edge this node represents. > > Why does a user need to know this? > Each SMD or GLINK edge will expose a dev node as /dev/rpmsg_ctrlX that can be used create dev nodes for channels that user space wants to access directly (without a specific kernel driver). The problem is that there's currently nothing in the device hierarchy that identifies which edge, and hence remoteproc, these channels are associated with. So for SMD I added the ability to specify a "label" on the SMD edge, which is propagated in sysfs and allows me to write udev rules creating symlinks based on the edge name (e.g. /dev/modem/DIAG vs /dev/rpmsg4) and it also allows user space tools to figure out which channels comes from the same edge/remoteproc (e.g. which DIAG control channel is related to which DIAG data channel). This patch attempts to add the equivalent property on GLINK edges. As SMD and GLINK edges can exist without a parent remoteproc (e.g. for the always-on subsystems) and it's possible for a single remoteproc to have multiple edges, it's inadequate to put this label on the remoteproc. PS. Downstream solutions of remoteproc + virtio-rpmsg has "solved" this problem by using aliases for remoteprocs and then only allow one virtio-rpmsg node per remoteproc, to get deterministic identification of channels. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/21/2017 11:36 AM, Stephen Boyd wrote: > On 12/20/2017 05:35 PM, Bjorn Andersson wrote: >> On Wed 20 Dec 10:30 PST 2017, Rob Herring wrote: >> >>> On Mon, Dec 18, 2017 at 02:02:09PM -0800, Chris Lew wrote: >>>> Add a label property to identify the edge this node represents. >>> Why does a user need to know this? >>> >> We have multiple remoteproc instances, each one having one or more >> associated SMD or GLINK links (this node), exposing logical >> communication channels. Some of these logical channels are exposed to >> user space and we need a way to distinguish them there. >> >> In the current implementation of SMD this value goes straight into an >> sysfs attribute that we can use when writing udev rules and for the DIAG >> implementation to pair up channels related to the same remoteproc. This >> adds the equivalent information for glink-backed channels. >> >> >> I'm therefor in favor of picking this patch. > > Please add these details to the commit log. Just writing what the patch > is doing isn't very helpful. > Ok, will do.
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>
Add a label property to identify the edge this node represents. Signed-off-by: Chris Lew <clew@codeaurora.org> --- Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt | 5 +++++ 1 file changed, 5 insertions(+)