| Message ID | 20190819205720.24457-6-mike.leach@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coresight: etm4x: Fixes and updates for sysfs API | expand |
On Mon, Aug 19, 2019 at 09:57:17PM +0100, Mike Leach wrote: > Some changes to make the sysfs programming more intuitive. > > 1) Setting include / exclude on a range had to be done by setting > the bit in 'mode' before setting the range. However, setting this > bit also had the effect of altering the current range as well. > > Changed to only set include / exclude setting of a range at the point of > setting that range. Either use a 3rd input parameter as the include exclude > value, or if not present use the current value of 'mode'. Do not change > current range when 'mode' changes. > > 2) Context ID and VM ID masks required 2 value inputs, even when the > second value is ignored as insufficient CID / VMID comparators are > implemented. > Permit a single value to be used if that is sufficient to cover all > implemented comparators. Please split in two patches. With that: Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > Signed-off-by: Mike Leach <mike.leach@linaro.org> > --- > .../coresight/coresight-etm4x-sysfs.c | 24 +++++++++++++------ > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > index 3bcc260c9e55..baac5b48b7ac 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > @@ -297,8 +297,6 @@ static ssize_t mode_store(struct device *dev, > > spin_lock(&drvdata->spinlock); > config->mode = val & ETMv4_MODE_ALL; > - etm4_set_mode_exclude(drvdata, > - config->mode & ETM_MODE_EXCLUDE ? true : false); > > if (drvdata->instrp0 == true) { > /* start by clearing instruction P0 field */ > @@ -972,8 +970,12 @@ static ssize_t addr_range_store(struct device *dev, > unsigned long val1, val2; > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > + int elements, exclude; > > - if (sscanf(buf, "%lx %lx", &val1, &val2) != 2) > + elements = sscanf(buf, "%lx %lx %x", &val1, &val2, &exclude); > + > + /* exclude is optional, but need at least two parameter */ > + if (elements < 2) > return -EINVAL; > /* lower address comparator cannot have a higher address value */ > if (val1 > val2) > @@ -1001,9 +1003,11 @@ static ssize_t addr_range_store(struct device *dev, > /* > * Program include or exclude control bits for vinst or vdata > * whenever we change addr comparators to ETM_ADDR_TYPE_RANGE > + * use supplied value, or default to bit set in 'mode' > */ > - etm4_set_mode_exclude(drvdata, > - config->mode & ETM_MODE_EXCLUDE ? true : false); > + if (elements != 3) > + exclude = config->mode & ETM_MODE_EXCLUDE; > + etm4_set_mode_exclude(drvdata, exclude ? true : false); > > spin_unlock(&drvdata->spinlock); > return size; > @@ -1787,6 +1791,7 @@ static ssize_t ctxid_masks_store(struct device *dev, > unsigned long val1, val2, mask; > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > + int nr_inputs; > > /* > * Don't use contextID tracing if coming from a PID namespace. See > @@ -1802,7 +1807,9 @@ static ssize_t ctxid_masks_store(struct device *dev, > */ > if (!drvdata->ctxid_size || !drvdata->numcidc) > return -EINVAL; > - if (sscanf(buf, "%lx %lx", &val1, &val2) != 2) > + /* one mask if < 4 comparators, two for up to 8 */ > + nr_inputs = sscanf(buf, "%lx %lx", &val1, &val2); > + if ((drvdata->numcidc > 4) && (nr_inputs != 2)) > return -EINVAL; > > spin_lock(&drvdata->spinlock); > @@ -1976,6 +1983,7 @@ static ssize_t vmid_masks_store(struct device *dev, > unsigned long val1, val2, mask; > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > + int nr_inputs; > > /* > * only implemented when vmid tracing is enabled, i.e. at least one > @@ -1983,7 +1991,9 @@ static ssize_t vmid_masks_store(struct device *dev, > */ > if (!drvdata->vmid_size || !drvdata->numvmidc) > return -EINVAL; > - if (sscanf(buf, "%lx %lx", &val1, &val2) != 2) > + /* one mask if < 4 comparators, two for up to 8 */ > + nr_inputs = sscanf(buf, "%lx %lx", &val1, &val2); > + if ((drvdata->numvmidc > 4) && (nr_inputs != 2)) > return -EINVAL; > > spin_lock(&drvdata->spinlock); > -- > 2.17.1 >
On Mon, Aug 19, 2019 at 09:57:17PM +0100, Mike Leach wrote: > Some changes to make the sysfs programming more intuitive. > > 1) Setting include / exclude on a range had to be done by setting > the bit in 'mode' before setting the range. However, setting this > bit also had the effect of altering the current range as well. > > Changed to only set include / exclude setting of a range at the point of > setting that range. Either use a 3rd input parameter as the include exclude > value, or if not present use the current value of 'mode'. Do not change > current range when 'mode' changes. > > 2) Context ID and VM ID masks required 2 value inputs, even when the > second value is ignored as insufficient CID / VMID comparators are > implemented. > Permit a single value to be used if that is sufficient to cover all > implemented comparators. > > Signed-off-by: Mike Leach <mike.leach@linaro.org> > --- > .../coresight/coresight-etm4x-sysfs.c | 24 +++++++++++++------ > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > index 3bcc260c9e55..baac5b48b7ac 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > @@ -297,8 +297,6 @@ static ssize_t mode_store(struct device *dev, > > spin_lock(&drvdata->spinlock); > config->mode = val & ETMv4_MODE_ALL; > - etm4_set_mode_exclude(drvdata, > - config->mode & ETM_MODE_EXCLUDE ? true : false); > > if (drvdata->instrp0 == true) { > /* start by clearing instruction P0 field */ > @@ -972,8 +970,12 @@ static ssize_t addr_range_store(struct device *dev, > unsigned long val1, val2; > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > + int elements, exclude; > > - if (sscanf(buf, "%lx %lx", &val1, &val2) != 2) > + elements = sscanf(buf, "%lx %lx %x", &val1, &val2, &exclude); > + > + /* exclude is optional, but need at least two parameter */ > + if (elements < 2) > return -EINVAL; > /* lower address comparator cannot have a higher address value */ > if (val1 > val2) > @@ -1001,9 +1003,11 @@ static ssize_t addr_range_store(struct device *dev, > /* > * Program include or exclude control bits for vinst or vdata > * whenever we change addr comparators to ETM_ADDR_TYPE_RANGE > + * use supplied value, or default to bit set in 'mode' > */ > - etm4_set_mode_exclude(drvdata, > - config->mode & ETM_MODE_EXCLUDE ? true : false); > + if (elements != 3) > + exclude = config->mode & ETM_MODE_EXCLUDE; > + etm4_set_mode_exclude(drvdata, exclude ? true : false); > > spin_unlock(&drvdata->spinlock); > return size; > @@ -1787,6 +1791,7 @@ static ssize_t ctxid_masks_store(struct device *dev, > unsigned long val1, val2, mask; > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > + int nr_inputs; > > /* > * Don't use contextID tracing if coming from a PID namespace. See > @@ -1802,7 +1807,9 @@ static ssize_t ctxid_masks_store(struct device *dev, > */ > if (!drvdata->ctxid_size || !drvdata->numcidc) > return -EINVAL; > - if (sscanf(buf, "%lx %lx", &val1, &val2) != 2) > + /* one mask if < 4 comparators, two for up to 8 */ One maks is <= 4 comparators. > + nr_inputs = sscanf(buf, "%lx %lx", &val1, &val2); > + if ((drvdata->numcidc > 4) && (nr_inputs != 2)) > return -EINVAL; > > spin_lock(&drvdata->spinlock); > @@ -1976,6 +1983,7 @@ static ssize_t vmid_masks_store(struct device *dev, > unsigned long val1, val2, mask; > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > + int nr_inputs; > > /* > * only implemented when vmid tracing is enabled, i.e. at least one > @@ -1983,7 +1991,9 @@ static ssize_t vmid_masks_store(struct device *dev, > */ > if (!drvdata->vmid_size || !drvdata->numvmidc) > return -EINVAL; > - if (sscanf(buf, "%lx %lx", &val1, &val2) != 2) > + /* one mask if < 4 comparators, two for up to 8 */ One maks is <= 4 comparators. > + nr_inputs = sscanf(buf, "%lx %lx", &val1, &val2); > + if ((drvdata->numvmidc > 4) && (nr_inputs != 2)) > return -EINVAL; > > spin_lock(&drvdata->spinlock); > -- > 2.17.1 > > _______________________________________________ > CoreSight mailing list > CoreSight@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/coresight
Hi Mathieu, Leo, Will split & fix comments. Thanks Mike On Wed, 28 Aug 2019 at 04:37, Leo Yan <leo.yan@linaro.org> wrote: > > On Mon, Aug 19, 2019 at 09:57:17PM +0100, Mike Leach wrote: > > Some changes to make the sysfs programming more intuitive. > > > > 1) Setting include / exclude on a range had to be done by setting > > the bit in 'mode' before setting the range. However, setting this > > bit also had the effect of altering the current range as well. > > > > Changed to only set include / exclude setting of a range at the point of > > setting that range. Either use a 3rd input parameter as the include exclude > > value, or if not present use the current value of 'mode'. Do not change > > current range when 'mode' changes. > > > > 2) Context ID and VM ID masks required 2 value inputs, even when the > > second value is ignored as insufficient CID / VMID comparators are > > implemented. > > Permit a single value to be used if that is sufficient to cover all > > implemented comparators. > > > > Signed-off-by: Mike Leach <mike.leach@linaro.org> > > --- > > .../coresight/coresight-etm4x-sysfs.c | 24 +++++++++++++------ > > 1 file changed, 17 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > > index 3bcc260c9e55..baac5b48b7ac 100644 > > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > > @@ -297,8 +297,6 @@ static ssize_t mode_store(struct device *dev, > > > > spin_lock(&drvdata->spinlock); > > config->mode = val & ETMv4_MODE_ALL; > > - etm4_set_mode_exclude(drvdata, > > - config->mode & ETM_MODE_EXCLUDE ? true : false); > > > > if (drvdata->instrp0 == true) { > > /* start by clearing instruction P0 field */ > > @@ -972,8 +970,12 @@ static ssize_t addr_range_store(struct device *dev, > > unsigned long val1, val2; > > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > > struct etmv4_config *config = &drvdata->config; > > + int elements, exclude; > > > > - if (sscanf(buf, "%lx %lx", &val1, &val2) != 2) > > + elements = sscanf(buf, "%lx %lx %x", &val1, &val2, &exclude); > > + > > + /* exclude is optional, but need at least two parameter */ > > + if (elements < 2) > > return -EINVAL; > > /* lower address comparator cannot have a higher address value */ > > if (val1 > val2) > > @@ -1001,9 +1003,11 @@ static ssize_t addr_range_store(struct device *dev, > > /* > > * Program include or exclude control bits for vinst or vdata > > * whenever we change addr comparators to ETM_ADDR_TYPE_RANGE > > + * use supplied value, or default to bit set in 'mode' > > */ > > - etm4_set_mode_exclude(drvdata, > > - config->mode & ETM_MODE_EXCLUDE ? true : false); > > + if (elements != 3) > > + exclude = config->mode & ETM_MODE_EXCLUDE; > > + etm4_set_mode_exclude(drvdata, exclude ? true : false); > > > > spin_unlock(&drvdata->spinlock); > > return size; > > @@ -1787,6 +1791,7 @@ static ssize_t ctxid_masks_store(struct device *dev, > > unsigned long val1, val2, mask; > > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > > struct etmv4_config *config = &drvdata->config; > > + int nr_inputs; > > > > /* > > * Don't use contextID tracing if coming from a PID namespace. See > > @@ -1802,7 +1807,9 @@ static ssize_t ctxid_masks_store(struct device *dev, > > */ > > if (!drvdata->ctxid_size || !drvdata->numcidc) > > return -EINVAL; > > - if (sscanf(buf, "%lx %lx", &val1, &val2) != 2) > > + /* one mask if < 4 comparators, two for up to 8 */ > > One maks is <= 4 comparators. > > > + nr_inputs = sscanf(buf, "%lx %lx", &val1, &val2); > > + if ((drvdata->numcidc > 4) && (nr_inputs != 2)) > > return -EINVAL; > > > > spin_lock(&drvdata->spinlock); > > @@ -1976,6 +1983,7 @@ static ssize_t vmid_masks_store(struct device *dev, > > unsigned long val1, val2, mask; > > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > > struct etmv4_config *config = &drvdata->config; > > + int nr_inputs; > > > > /* > > * only implemented when vmid tracing is enabled, i.e. at least one > > @@ -1983,7 +1991,9 @@ static ssize_t vmid_masks_store(struct device *dev, > > */ > > if (!drvdata->vmid_size || !drvdata->numvmidc) > > return -EINVAL; > > - if (sscanf(buf, "%lx %lx", &val1, &val2) != 2) > > + /* one mask if < 4 comparators, two for up to 8 */ > > One maks is <= 4 comparators. > > > + nr_inputs = sscanf(buf, "%lx %lx", &val1, &val2); > > + if ((drvdata->numvmidc > 4) && (nr_inputs != 2)) > > return -EINVAL; > > > > spin_lock(&drvdata->spinlock); > > -- > > 2.17.1 > > > > _______________________________________________ > > CoreSight mailing list > > CoreSight@lists.linaro.org > > https://lists.linaro.org/mailman/listinfo/coresight
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c index 3bcc260c9e55..baac5b48b7ac 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -297,8 +297,6 @@ static ssize_t mode_store(struct device *dev, spin_lock(&drvdata->spinlock); config->mode = val & ETMv4_MODE_ALL; - etm4_set_mode_exclude(drvdata, - config->mode & ETM_MODE_EXCLUDE ? true : false); if (drvdata->instrp0 == true) { /* start by clearing instruction P0 field */ @@ -972,8 +970,12 @@ static ssize_t addr_range_store(struct device *dev, unsigned long val1, val2; struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); struct etmv4_config *config = &drvdata->config; + int elements, exclude; - if (sscanf(buf, "%lx %lx", &val1, &val2) != 2) + elements = sscanf(buf, "%lx %lx %x", &val1, &val2, &exclude); + + /* exclude is optional, but need at least two parameter */ + if (elements < 2) return -EINVAL; /* lower address comparator cannot have a higher address value */ if (val1 > val2) @@ -1001,9 +1003,11 @@ static ssize_t addr_range_store(struct device *dev, /* * Program include or exclude control bits for vinst or vdata * whenever we change addr comparators to ETM_ADDR_TYPE_RANGE + * use supplied value, or default to bit set in 'mode' */ - etm4_set_mode_exclude(drvdata, - config->mode & ETM_MODE_EXCLUDE ? true : false); + if (elements != 3) + exclude = config->mode & ETM_MODE_EXCLUDE; + etm4_set_mode_exclude(drvdata, exclude ? true : false); spin_unlock(&drvdata->spinlock); return size; @@ -1787,6 +1791,7 @@ static ssize_t ctxid_masks_store(struct device *dev, unsigned long val1, val2, mask; struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); struct etmv4_config *config = &drvdata->config; + int nr_inputs; /* * Don't use contextID tracing if coming from a PID namespace. See @@ -1802,7 +1807,9 @@ static ssize_t ctxid_masks_store(struct device *dev, */ if (!drvdata->ctxid_size || !drvdata->numcidc) return -EINVAL; - if (sscanf(buf, "%lx %lx", &val1, &val2) != 2) + /* one mask if < 4 comparators, two for up to 8 */ + nr_inputs = sscanf(buf, "%lx %lx", &val1, &val2); + if ((drvdata->numcidc > 4) && (nr_inputs != 2)) return -EINVAL; spin_lock(&drvdata->spinlock); @@ -1976,6 +1983,7 @@ static ssize_t vmid_masks_store(struct device *dev, unsigned long val1, val2, mask; struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); struct etmv4_config *config = &drvdata->config; + int nr_inputs; /* * only implemented when vmid tracing is enabled, i.e. at least one @@ -1983,7 +1991,9 @@ static ssize_t vmid_masks_store(struct device *dev, */ if (!drvdata->vmid_size || !drvdata->numvmidc) return -EINVAL; - if (sscanf(buf, "%lx %lx", &val1, &val2) != 2) + /* one mask if < 4 comparators, two for up to 8 */ + nr_inputs = sscanf(buf, "%lx %lx", &val1, &val2); + if ((drvdata->numvmidc > 4) && (nr_inputs != 2)) return -EINVAL; spin_lock(&drvdata->spinlock);
Some changes to make the sysfs programming more intuitive. 1) Setting include / exclude on a range had to be done by setting the bit in 'mode' before setting the range. However, setting this bit also had the effect of altering the current range as well. Changed to only set include / exclude setting of a range at the point of setting that range. Either use a 3rd input parameter as the include exclude value, or if not present use the current value of 'mode'. Do not change current range when 'mode' changes. 2) Context ID and VM ID masks required 2 value inputs, even when the second value is ignored as insufficient CID / VMID comparators are implemented. Permit a single value to be used if that is sufficient to cover all implemented comparators. Signed-off-by: Mike Leach <mike.leach@linaro.org> --- .../coresight/coresight-etm4x-sysfs.c | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-)