Re: [PATCH v5 04/14] coresight: cti: Add sysfs trigger / channel programming API

Suzuki Kuruppassery Poulose
2019-11-27T18:40:26Z

On 19/11/2019 23:19, Mike Leach wrote:
> Adds a user API to allow programming of CTI by trigger ID and
> channel number. This will take the channel and trigger ID supplied
> by the user and program the appropriate register values.
>
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---

> +
> +static ssize_t chan_xtrigs_view_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> + struct cti_config *cfg = &drvdata->config;
> + int used = 0, reg_idx;
> + int buf_sz = PAGE_SIZE;
> + u32 chan_mask = BIT(cfg->xtrig_rchan_sel);
> +
> + used += scnprintf(buf, buf_sz, "[%d] IN: ", cfg->xtrig_rchan_sel);
> + for (reg_idx = 0;
> + reg_idx < drvdata->config.nr_trig_max;
> + reg_idx++) {
> + if (chan_mask & cfg->ctiinen[reg_idx]) {
> + used += scnprintf(buf + used, buf_sz - used, "%d ",
> + reg_idx);
> + }
> + }

As a security measure, we must make sure that we have space left in the
buffer. We could end up passing "negative" numbers for the size
argument, in the worst case.

> +
> + used += scnprintf(buf + used, buf_sz - used, "OUT: ");
> + for (reg_idx = 0;
> + reg_idx < drvdata->config.nr_trig_max;
> + reg_idx++) {
> + if (chan_mask & cfg->ctiouten[reg_idx]) {
> + used += scnprintf(buf + used, buf_sz - used, "%d ",
> + reg_idx);
> + }
> + }
> + used += scnprintf(buf + used, buf_sz - used, "\n");
> + return used;
> +}
> +static DEVICE_ATTR_RW(chan_xtrigs_view);

The rest looks fine to me.

Suzuki