| Message ID | 1692681973-20764-5-git-send-email-quic_taozha@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support to configure TPDM DSB subunit | expand |
Hi Tao, kernel test robot noticed the following build warnings: [auto build test WARNING on robh/for-next] [also build test WARNING on linus/master v6.5-rc7 next-20230822] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Tao-Zhang/coresight-tpdm-Remove-the-unnecessary-lock/20230822-132946 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next patch link: https://lore.kernel.org/r/1692681973-20764-5-git-send-email-quic_taozha%40quicinc.com patch subject: [PATCH v8 04/13] coresight-tpda: Add DSB dataset support config: arm64-randconfig-r031-20230823 (https://download.01.org/0day-ci/archive/20230823/202308230927.JBX1uKOE-lkp@intel.com/config) compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a) reproduce: (https://download.01.org/0day-ci/archive/20230823/202308230927.JBX1uKOE-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202308230927.JBX1uKOE-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/hwtracing/coresight/coresight-tpda.c:168:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] if (atomic_read(&in->dest_refcnt) == 0) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/hwtracing/coresight/coresight-tpda.c:177:9: note: uninitialized use occurs here return ret; ^~~ drivers/hwtracing/coresight/coresight-tpda.c:168:2: note: remove the 'if' if its condition is always true if (atomic_read(&in->dest_refcnt) == 0) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/hwtracing/coresight/coresight-tpda.c:165:9: note: initialize the variable 'ret' to silence this warning int ret; ^ = 0 1 warning generated. vim +168 drivers/hwtracing/coresight/coresight-tpda.c 159 160 static int tpda_enable(struct coresight_device *csdev, 161 struct coresight_connection *in, 162 struct coresight_connection *out) 163 { 164 struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); 165 int ret; 166 167 spin_lock(&drvdata->spinlock); > 168 if (atomic_read(&in->dest_refcnt) == 0) { 169 ret = __tpda_enable(drvdata, in->dest_port); 170 if (!ret) { 171 atomic_inc(&in->dest_refcnt); 172 dev_dbg(drvdata->dev, "TPDA inport %d enabled.\n", in->dest_port); 173 } 174 } 175 176 spin_unlock(&drvdata->spinlock); 177 return ret; 178 } 179
On 22/08/2023 06:26, Tao Zhang wrote: > Read the DSB element size from the device tree. Set the register > bit that controls the DSB element size of the corresponding port. > > Signed-off-by: Tao Zhang <quic_taozha@quicinc.com> > --- > drivers/hwtracing/coresight/coresight-tpda.c | 126 ++++++++++++++++++++++++--- > drivers/hwtracing/coresight/coresight-tpda.h | 2 + > 2 files changed, 118 insertions(+), 10 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c > index 8d2b9d2..0f21cd1 100644 > --- a/drivers/hwtracing/coresight/coresight-tpda.c > +++ b/drivers/hwtracing/coresight/coresight-tpda.c > @@ -21,6 +21,80 @@ > > DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda"); > > +static bool coresight_device_is_tpdm(struct coresight_device *csdev) > +{ > + return (csdev->type == CORESIGHT_DEV_TYPE_SOURCE) && > + (csdev->subtype.source_subtype == minor nit: Please align return (csdev->type == CORESIGHT_DEV_TYPE_SOURCE) && (csdev->subtype.source_subtype == > + CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM); > +} > + > +/* > + * Read the DSB element size from the TPDM device > + * Returns > + * The dsb element size read from the devicetree if available. > + * 0 - Otherwise, with a warning once. > + */ > +static int tpdm_read_dsb_element_size(struct coresight_device *csdev) > +{ > + int rc = 0; > + u8 size = 0; > + > + rc = fwnode_property_read_u8(dev_fwnode(csdev->dev.parent), > + "qcom,dsb-element-size", &size); > + if (rc) > + dev_warn_once(&csdev->dev, > + "Failed to read TPDM DSB Element size: %d\n", rc); > + > + return size; > +} > + > +/* > + * Search and read element data size from the TPDM node in > + * the devicetree. Each input port of TPDA is connected to > + * a TPDM. Different TPDM supports different types of dataset, > + * and some may support more than one type of dataset. > + * Parameter "inport" is used to pass in the input port number > + * of TPDA, and it is set to -1 in the recursize call. > + */ > +static int tpda_get_element_size(struct coresight_device *csdev, > + int inport) nit: Again, please align. static int tpda_get_element_size(struct coresight_device *csdev, int inport) { > +{ > + int dsb_size = -ENOENT; > + int i, size; > + struct coresight_device *in; > + > + for (i = 0; i < csdev->pdata->nr_inconns; i++) { > + in = csdev->pdata->in_conns[i]->src_dev; > + if (!in) > + continue; > + > + /* Ignore the paths that do not match port */ > + if (inport > 0 && > + (csdev->pdata->in_conns[i]->dest_port != inport)) Align please : if (inport > 0 && (csdev->pdata->in_conns[i]->dest_port != inport)) > + continue; > + > + if (coresight_device_is_tpdm(in)) { > + size = tpdm_read_dsb_element_size(in); > + } else { > + /* Recurse down the path */ > + size = tpda_get_element_size(in, -1); > + } > + > + if (size < 0) > + return size; > + > + if (dsb_size < 0) { > + /* Found a size, save it. */ > + dsb_size = size; > + } else { > + /* Found duplicate TPDMs */ > + return -EEXIST; > + } > + } > + > + return dsb_size; > +} > + > /* Settings pre enabling port control register */ > static void tpda_enable_pre_port(struct tpda_drvdata *drvdata) > { > @@ -32,26 +106,55 @@ static void tpda_enable_pre_port(struct tpda_drvdata *drvdata) > writel_relaxed(val, drvdata->base + TPDA_CR); > } > > -static void tpda_enable_port(struct tpda_drvdata *drvdata, int port) > +static int tpda_enable_port(struct tpda_drvdata *drvdata, int port) > { > u32 val; > + int size; > > val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port)); > + /* > + * Configure aggregator port n DSB data set element size > + * Set the bit to 0 if the size is 32 > + * Set the bit to 1 if the size is 64 > + */ > + size = tpda_get_element_size(drvdata->csdev, port); > + switch (size) { > + case 32: > + val &= ~TPDA_Pn_CR_DSBSIZE; > + break; > + case 64: > + val |= TPDA_Pn_CR_DSBSIZE; > + break; > + case 0: > + return -EEXIST; > + case -EEXIST: > + dev_warn_once(&drvdata->csdev->dev, > + "Detected multiple TPDMs on port %d", -EEXIST); > + return -EEXIST; > + default: > + return -EINVAL; > + } > + > /* Enable the port */ > val |= TPDA_Pn_CR_ENA; > writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port)); > + > + return 0; > } > > -static void __tpda_enable(struct tpda_drvdata *drvdata, int port) > +static int __tpda_enable(struct tpda_drvdata *drvdata, int port) > { > + int ret; > + > CS_UNLOCK(drvdata->base); > > if (!drvdata->csdev->enable) > tpda_enable_pre_port(drvdata); > > - tpda_enable_port(drvdata, port); > - > + ret = tpda_enable_port(drvdata, port); > CS_LOCK(drvdata->base); > + > + return ret; > } > > static int tpda_enable(struct coresight_device *csdev, > @@ -59,16 +162,19 @@ static int tpda_enable(struct coresight_device *csdev, > struct coresight_connection *out) > { > struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + int ret; As reported by the build robot, please initialise this to 0. > > spin_lock(&drvdata->spinlock); > - if (atomic_read(&in->dest_refcnt) == 0) > - __tpda_enable(drvdata, in->dest_port); > + if (atomic_read(&in->dest_refcnt) == 0) { > + ret = __tpda_enable(drvdata, in->dest_port); > + if (!ret) { > + atomic_inc(&in->dest_refcnt); > + dev_dbg(drvdata->dev, "TPDA inport %d enabled.\n", in->dest_port); > + } > + } > > - atomic_inc(&in->dest_refcnt); > spin_unlock(&drvdata->spinlock); > - > - dev_dbg(drvdata->dev, "TPDA inport %d enabled.\n", in->dest_port); > - return 0; > + return ret; > } > > static void __tpda_disable(struct tpda_drvdata *drvdata, int port) > diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h > index 0399678..b3b38fd 100644 > --- a/drivers/hwtracing/coresight/coresight-tpda.h > +++ b/drivers/hwtracing/coresight/coresight-tpda.h > @@ -10,6 +10,8 @@ > #define TPDA_Pn_CR(n) (0x004 + (n * 4)) > /* Aggregator port enable bit */ > #define TPDA_Pn_CR_ENA BIT(0) > +/* Aggregator port DSB data set element size bit */ > +#define TPDA_Pn_CR_DSBSIZE BIT(8) > > #define TPDA_MAX_INPORTS 32 > Rest looks fine to me. Suzuki
diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c index 8d2b9d2..0f21cd1 100644 --- a/drivers/hwtracing/coresight/coresight-tpda.c +++ b/drivers/hwtracing/coresight/coresight-tpda.c @@ -21,6 +21,80 @@ DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda"); +static bool coresight_device_is_tpdm(struct coresight_device *csdev) +{ + return (csdev->type == CORESIGHT_DEV_TYPE_SOURCE) && + (csdev->subtype.source_subtype == + CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM); +} + +/* + * Read the DSB element size from the TPDM device + * Returns + * The dsb element size read from the devicetree if available. + * 0 - Otherwise, with a warning once. + */ +static int tpdm_read_dsb_element_size(struct coresight_device *csdev) +{ + int rc = 0; + u8 size = 0; + + rc = fwnode_property_read_u8(dev_fwnode(csdev->dev.parent), + "qcom,dsb-element-size", &size); + if (rc) + dev_warn_once(&csdev->dev, + "Failed to read TPDM DSB Element size: %d\n", rc); + + return size; +} + +/* + * Search and read element data size from the TPDM node in + * the devicetree. Each input port of TPDA is connected to + * a TPDM. Different TPDM supports different types of dataset, + * and some may support more than one type of dataset. + * Parameter "inport" is used to pass in the input port number + * of TPDA, and it is set to -1 in the recursize call. + */ +static int tpda_get_element_size(struct coresight_device *csdev, + int inport) +{ + int dsb_size = -ENOENT; + int i, size; + struct coresight_device *in; + + for (i = 0; i < csdev->pdata->nr_inconns; i++) { + in = csdev->pdata->in_conns[i]->src_dev; + if (!in) + continue; + + /* Ignore the paths that do not match port */ + if (inport > 0 && + (csdev->pdata->in_conns[i]->dest_port != inport)) + continue; + + if (coresight_device_is_tpdm(in)) { + size = tpdm_read_dsb_element_size(in); + } else { + /* Recurse down the path */ + size = tpda_get_element_size(in, -1); + } + + if (size < 0) + return size; + + if (dsb_size < 0) { + /* Found a size, save it. */ + dsb_size = size; + } else { + /* Found duplicate TPDMs */ + return -EEXIST; + } + } + + return dsb_size; +} + /* Settings pre enabling port control register */ static void tpda_enable_pre_port(struct tpda_drvdata *drvdata) { @@ -32,26 +106,55 @@ static void tpda_enable_pre_port(struct tpda_drvdata *drvdata) writel_relaxed(val, drvdata->base + TPDA_CR); } -static void tpda_enable_port(struct tpda_drvdata *drvdata, int port) +static int tpda_enable_port(struct tpda_drvdata *drvdata, int port) { u32 val; + int size; val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port)); + /* + * Configure aggregator port n DSB data set element size + * Set the bit to 0 if the size is 32 + * Set the bit to 1 if the size is 64 + */ + size = tpda_get_element_size(drvdata->csdev, port); + switch (size) { + case 32: + val &= ~TPDA_Pn_CR_DSBSIZE; + break; + case 64: + val |= TPDA_Pn_CR_DSBSIZE; + break; + case 0: + return -EEXIST; + case -EEXIST: + dev_warn_once(&drvdata->csdev->dev, + "Detected multiple TPDMs on port %d", -EEXIST); + return -EEXIST; + default: + return -EINVAL; + } + /* Enable the port */ val |= TPDA_Pn_CR_ENA; writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port)); + + return 0; } -static void __tpda_enable(struct tpda_drvdata *drvdata, int port) +static int __tpda_enable(struct tpda_drvdata *drvdata, int port) { + int ret; + CS_UNLOCK(drvdata->base); if (!drvdata->csdev->enable) tpda_enable_pre_port(drvdata); - tpda_enable_port(drvdata, port); - + ret = tpda_enable_port(drvdata, port); CS_LOCK(drvdata->base); + + return ret; } static int tpda_enable(struct coresight_device *csdev, @@ -59,16 +162,19 @@ static int tpda_enable(struct coresight_device *csdev, struct coresight_connection *out) { struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + int ret; spin_lock(&drvdata->spinlock); - if (atomic_read(&in->dest_refcnt) == 0) - __tpda_enable(drvdata, in->dest_port); + if (atomic_read(&in->dest_refcnt) == 0) { + ret = __tpda_enable(drvdata, in->dest_port); + if (!ret) { + atomic_inc(&in->dest_refcnt); + dev_dbg(drvdata->dev, "TPDA inport %d enabled.\n", in->dest_port); + } + } - atomic_inc(&in->dest_refcnt); spin_unlock(&drvdata->spinlock); - - dev_dbg(drvdata->dev, "TPDA inport %d enabled.\n", in->dest_port); - return 0; + return ret; } static void __tpda_disable(struct tpda_drvdata *drvdata, int port) diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h index 0399678..b3b38fd 100644 --- a/drivers/hwtracing/coresight/coresight-tpda.h +++ b/drivers/hwtracing/coresight/coresight-tpda.h @@ -10,6 +10,8 @@ #define TPDA_Pn_CR(n) (0x004 + (n * 4)) /* Aggregator port enable bit */ #define TPDA_Pn_CR_ENA BIT(0) +/* Aggregator port DSB data set element size bit */ +#define TPDA_Pn_CR_DSBSIZE BIT(8) #define TPDA_MAX_INPORTS 32
Read the DSB element size from the device tree. Set the register bit that controls the DSB element size of the corresponding port. Signed-off-by: Tao Zhang <quic_taozha@quicinc.com> --- drivers/hwtracing/coresight/coresight-tpda.c | 126 ++++++++++++++++++++++++--- drivers/hwtracing/coresight/coresight-tpda.h | 2 + 2 files changed, 118 insertions(+), 10 deletions(-)