From: Suzuki Kuruppassery Poulose <suzuki.poulose@arm.com> To: Mike Leach <mike.leach@linaro.org>, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, coresight@lists.linaro.org, linux-doc@vger.kernel.org Cc: linux-arm-msm@vger.kernel.org, mathieu.poirier@linaro.org, robh+dt@kernel.org, maxime@cerno.tech, liviu.dudau@arm.com, sudeep.holla@arm.com, lorenzo.pieralisi@arm.com, agross@kernel.org, corbet@lwn.net Subject: Re: [PATCH v8 08/15] coresight: cti: Enable CTI associated with devices. Date: Wed, 15 Jan 2020 11:14:39 +0000 [thread overview] Message-ID: <396fc3a2-083b-29ef-2bb6-2fca066ea0ef@arm.com> (raw) In-Reply-To: <20200113213149.25599-9-mike.leach@linaro.org> On 13/01/2020 21:31, Mike Leach wrote: > The CoreSight subsystem enables a path of devices from source to sink. > Any CTI devices associated with the path devices must be enabled at the > same time. > > This patch adds an associated coresight_device element to the main > coresight device structure, and uses this to create associations between > the CTI and other devices based on the device tree data. The associated > device element is used to enable CTI in conjunction with the path elements. > > CTI devices are reference counted so where a single CTI is associated with > multiple elements on the path, it will be enabled on the first associated > device enable, and disabled with the last associated device disable. > > Signed-off-by: Mike Leach <mike.leach@linaro.org> > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> > --- > drivers/hwtracing/coresight/coresight-cti.c | 129 ++++++++++++++++++ > drivers/hwtracing/coresight/coresight-cti.h | 1 + > .../hwtracing/coresight/coresight-platform.c | 1 + > drivers/hwtracing/coresight/coresight-priv.h | 12 ++ > drivers/hwtracing/coresight/coresight.c | 71 +++++++++- > include/linux/coresight.h | 4 + > 6 files changed, 213 insertions(+), 5 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c > index 77c2af247917..2be1b310e854 100644 > --- a/drivers/hwtracing/coresight/coresight-cti.c > +++ b/drivers/hwtracing/coresight/coresight-cti.c > @@ -4,6 +4,7 @@ > * Author: Mike Leach <mike.leach@linaro.org> > */ > > +#include <linux/property.h> > #include "coresight-cti.h" > > /** > @@ -440,6 +441,131 @@ int cti_channel_setop(struct device *dev, enum cti_chan_set_op op, > return err; > } > > +/* > + * Look for a matching connection device name in the list of connections. > + * If found then swap in the csdev name, set trig con association pointer > + * and return found. > + */ > +static bool > +cti_match_fixup_csdev(struct cti_device *ctidev, const char *node_name, > + struct coresight_device *csdev) > +{ > + struct cti_trig_con *trig_con; super minor nit: Please use "struct cti_trig_con *tc;" consistent with the naming everywhere else. Helps a lot in reading the code, especially which has a lot of different structures. > + const char *csdev_name; > + > + list_for_each_entry(trig_con, &ctidev->trig_cons, node) { > + if (trig_con->con_dev_name) { This was allocated via devm_* and ... > + if (!strcmp(node_name, trig_con->con_dev_name)) { > + /* match: so swap in csdev name & dev */ > + kfree(trig_con->con_dev_name); ... we free it here using kfree() without devm_ being aware. This could cause double-free when the device is removed. This should either be devm_kfree() or simply overwritten with the new string and leave the device cleanup to free it. > + csdev_name = dev_name(&csdev->dev); > + trig_con->con_dev_name = > + kstrdup(csdev_name, GFP_KERNEL); Please use devm_kstrdup() here on the CTI device to have a consistent allocation. > + trig_con->con_dev = csdev; > + return true; > + } > + } > + } > + return false; > +} > > +/* > + * Removing the associated devices is easier. > + * A CTI will not have a value for csdev->ect_dev. > + */ > +void cti_remove_assoc_from_csdev(struct coresight_device *csdev) > +{ > + struct cti_drvdata *ctidrv; > + struct cti_trig_con *tc; > + struct cti_device *ctidev; > + > + mutex_lock(&ect_mutex); > + if (csdev->ect_dev) { > + ctidrv = csdev_to_cti_drvdata(csdev->ect_dev); > + ctidev = &ctidrv->ctidev; > + list_for_each_entry(tc, &ctidev->trig_cons, node) { > + if (tc->con_dev == csdev->ect_dev) { > + tc->con_dev = NULL; Should we clear/free the name too ? > + break; > + } > + } > + csdev->ect_dev = NULL; > + } > + mutex_unlock(&ect_mutex); > +} > +EXPORT_SYMBOL_GPL(cti_remove_assoc_from_csdev); > + ... > diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h > index 469a06a1bb78..578d7e9ac67e 100644 > --- a/drivers/hwtracing/coresight/coresight-cti.h > +++ b/drivers/hwtracing/coresight/coresight-cti.h > @@ -216,6 +216,7 @@ int cti_channel_setop(struct device *dev, enum cti_chan_set_op op, > u32 channel_idx); > struct coresight_platform_data * > coresight_cti_get_platform_data(struct device *dev); > +const char *cti_plat_get_node_name(struct fwnode_handle *fwnode); > > /* cti powered and enabled */ > static inline bool cti_active(struct cti_config *cfg) > diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c > index 43418a2126ff..421d4fc95f41 100644 > --- a/drivers/hwtracing/coresight/coresight-platform.c > +++ b/drivers/hwtracing/coresight/coresight-platform.c > @@ -313,6 +313,7 @@ static int of_get_coresight_platform_data(struct device *dev, > > return 0; > } > + nit : spurious hunk ? > #else > static inline int > of_get_coresight_platform_data(struct device *dev, Otherwise looks good to me Suzuki
WARNING: multiple messages have this Message-ID (diff)
From: Suzuki Kuruppassery Poulose <suzuki.poulose@arm.com> To: Mike Leach <mike.leach@linaro.org>, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, coresight@lists.linaro.org, linux-doc@vger.kernel.org Cc: lorenzo.pieralisi@arm.com, mathieu.poirier@linaro.org, corbet@lwn.net, linux-arm-msm@vger.kernel.org, liviu.dudau@arm.com, agross@kernel.org, robh+dt@kernel.org, maxime@cerno.tech, sudeep.holla@arm.com Subject: Re: [PATCH v8 08/15] coresight: cti: Enable CTI associated with devices. Date: Wed, 15 Jan 2020 11:14:39 +0000 [thread overview] Message-ID: <396fc3a2-083b-29ef-2bb6-2fca066ea0ef@arm.com> (raw) In-Reply-To: <20200113213149.25599-9-mike.leach@linaro.org> On 13/01/2020 21:31, Mike Leach wrote: > The CoreSight subsystem enables a path of devices from source to sink. > Any CTI devices associated with the path devices must be enabled at the > same time. > > This patch adds an associated coresight_device element to the main > coresight device structure, and uses this to create associations between > the CTI and other devices based on the device tree data. The associated > device element is used to enable CTI in conjunction with the path elements. > > CTI devices are reference counted so where a single CTI is associated with > multiple elements on the path, it will be enabled on the first associated > device enable, and disabled with the last associated device disable. > > Signed-off-by: Mike Leach <mike.leach@linaro.org> > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> > --- > drivers/hwtracing/coresight/coresight-cti.c | 129 ++++++++++++++++++ > drivers/hwtracing/coresight/coresight-cti.h | 1 + > .../hwtracing/coresight/coresight-platform.c | 1 + > drivers/hwtracing/coresight/coresight-priv.h | 12 ++ > drivers/hwtracing/coresight/coresight.c | 71 +++++++++- > include/linux/coresight.h | 4 + > 6 files changed, 213 insertions(+), 5 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c > index 77c2af247917..2be1b310e854 100644 > --- a/drivers/hwtracing/coresight/coresight-cti.c > +++ b/drivers/hwtracing/coresight/coresight-cti.c > @@ -4,6 +4,7 @@ > * Author: Mike Leach <mike.leach@linaro.org> > */ > > +#include <linux/property.h> > #include "coresight-cti.h" > > /** > @@ -440,6 +441,131 @@ int cti_channel_setop(struct device *dev, enum cti_chan_set_op op, > return err; > } > > +/* > + * Look for a matching connection device name in the list of connections. > + * If found then swap in the csdev name, set trig con association pointer > + * and return found. > + */ > +static bool > +cti_match_fixup_csdev(struct cti_device *ctidev, const char *node_name, > + struct coresight_device *csdev) > +{ > + struct cti_trig_con *trig_con; super minor nit: Please use "struct cti_trig_con *tc;" consistent with the naming everywhere else. Helps a lot in reading the code, especially which has a lot of different structures. > + const char *csdev_name; > + > + list_for_each_entry(trig_con, &ctidev->trig_cons, node) { > + if (trig_con->con_dev_name) { This was allocated via devm_* and ... > + if (!strcmp(node_name, trig_con->con_dev_name)) { > + /* match: so swap in csdev name & dev */ > + kfree(trig_con->con_dev_name); ... we free it here using kfree() without devm_ being aware. This could cause double-free when the device is removed. This should either be devm_kfree() or simply overwritten with the new string and leave the device cleanup to free it. > + csdev_name = dev_name(&csdev->dev); > + trig_con->con_dev_name = > + kstrdup(csdev_name, GFP_KERNEL); Please use devm_kstrdup() here on the CTI device to have a consistent allocation. > + trig_con->con_dev = csdev; > + return true; > + } > + } > + } > + return false; > +} > > +/* > + * Removing the associated devices is easier. > + * A CTI will not have a value for csdev->ect_dev. > + */ > +void cti_remove_assoc_from_csdev(struct coresight_device *csdev) > +{ > + struct cti_drvdata *ctidrv; > + struct cti_trig_con *tc; > + struct cti_device *ctidev; > + > + mutex_lock(&ect_mutex); > + if (csdev->ect_dev) { > + ctidrv = csdev_to_cti_drvdata(csdev->ect_dev); > + ctidev = &ctidrv->ctidev; > + list_for_each_entry(tc, &ctidev->trig_cons, node) { > + if (tc->con_dev == csdev->ect_dev) { > + tc->con_dev = NULL; Should we clear/free the name too ? > + break; > + } > + } > + csdev->ect_dev = NULL; > + } > + mutex_unlock(&ect_mutex); > +} > +EXPORT_SYMBOL_GPL(cti_remove_assoc_from_csdev); > + ... > diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h > index 469a06a1bb78..578d7e9ac67e 100644 > --- a/drivers/hwtracing/coresight/coresight-cti.h > +++ b/drivers/hwtracing/coresight/coresight-cti.h > @@ -216,6 +216,7 @@ int cti_channel_setop(struct device *dev, enum cti_chan_set_op op, > u32 channel_idx); > struct coresight_platform_data * > coresight_cti_get_platform_data(struct device *dev); > +const char *cti_plat_get_node_name(struct fwnode_handle *fwnode); > > /* cti powered and enabled */ > static inline bool cti_active(struct cti_config *cfg) > diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c > index 43418a2126ff..421d4fc95f41 100644 > --- a/drivers/hwtracing/coresight/coresight-platform.c > +++ b/drivers/hwtracing/coresight/coresight-platform.c > @@ -313,6 +313,7 @@ static int of_get_coresight_platform_data(struct device *dev, > > return 0; > } > + nit : spurious hunk ? > #else > static inline int > of_get_coresight_platform_data(struct device *dev, Otherwise looks good to me Suzuki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-01-15 11:14 UTC|newest] Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-01-13 21:31 [PATCH v8 00/15] CoreSight CTI Driver Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-13 21:31 ` [PATCH v8 01/15] coresight: cti: Initial " Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-13 21:31 ` [PATCH v8 02/15] coresight: cti: Add sysfs coresight mgmt reg access Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-13 21:31 ` [PATCH v8 03/15] coresight: cti: Add sysfs access to program function regs Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-13 21:31 ` [PATCH v8 04/15] coresight: cti: Add sysfs trigger / channel programming API Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-15 9:42 ` Suzuki Kuruppassery Poulose 2020-01-15 9:42 ` Suzuki Kuruppassery Poulose 2020-01-13 21:31 ` [PATCH v8 05/15] dt-bindings: arm: Adds CoreSight CTI hardware definitions Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-14 22:21 ` Rob Herring 2020-01-14 22:21 ` Rob Herring 2020-01-13 21:31 ` [PATCH v8 06/15] coresight: cti: Add device tree support for v8 arch CTI Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-15 9:45 ` Suzuki Kuruppassery Poulose 2020-01-15 9:45 ` Suzuki Kuruppassery Poulose 2020-01-13 21:31 ` [PATCH v8 07/15] coresight: cti: Add device tree support for custom CTI Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-15 9:55 ` Suzuki Kuruppassery Poulose 2020-01-15 9:55 ` Suzuki Kuruppassery Poulose 2020-01-13 21:31 ` [PATCH v8 08/15] coresight: cti: Enable CTI associated with devices Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-15 11:14 ` Suzuki Kuruppassery Poulose [this message] 2020-01-15 11:14 ` Suzuki Kuruppassery Poulose 2020-01-28 19:32 ` Mike Leach 2020-01-28 19:32 ` Mike Leach 2020-01-13 21:31 ` [PATCH v8 09/15] coresight: cti: Add connection information to sysfs Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-15 11:34 ` Suzuki Kuruppassery Poulose 2020-01-15 11:34 ` Suzuki Kuruppassery Poulose 2020-01-13 21:31 ` [PATCH v8 10/15] dt-bindings: qcom: Add CTI options for qcom msm8916 Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-13 21:31 ` [PATCH v8 11/15] dt-bindings: arm: Juno platform - add CTI entries to device tree Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-13 21:31 ` [PATCH v8 12/15] dt-bindings: hisilicon: Add CTI bindings for hi-6220 Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-13 21:31 ` [PATCH v8 13/15] docs: coresight: Update documentation for CoreSight to cover CTI Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-13 21:31 ` [PATCH v8 14/15] docs: sysfs: coresight: Add sysfs ABI documentation for CTI Mike Leach 2020-01-13 21:31 ` Mike Leach 2020-01-13 21:31 ` [PATCH v8 15/15] Update MAINTAINERS to add reviewer for CoreSight Mike Leach 2020-01-13 21:31 ` Mike Leach
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=396fc3a2-083b-29ef-2bb6-2fca066ea0ef@arm.com \ --to=suzuki.poulose@arm.com \ --cc=agross@kernel.org \ --cc=corbet@lwn.net \ --cc=coresight@lists.linaro.org \ --cc=devicetree@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-doc@vger.kernel.org \ --cc=liviu.dudau@arm.com \ --cc=lorenzo.pieralisi@arm.com \ --cc=mathieu.poirier@linaro.org \ --cc=maxime@cerno.tech \ --cc=mike.leach@linaro.org \ --cc=robh+dt@kernel.org \ --cc=sudeep.holla@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.