Merge tag 'stm-for-greg-20160420' of git://git.kernel.org/pub/scm/linux/kernel/git/ash/stm into char-misc-next

Alexander writes:

stm class/intel_th: Updates for 4.7

These are:
* Intel TH/MSU: improved resource handling and releasing
* Intel TH/MSU: rehashed locking around buffer accesses
* Intel TH/outputs: better sysfs group handling
* Intel TH, STM: various bugfixes and smaller improvements
* Intel TH: added a PCI ID for Broxton-M SOC

MAINTAINERS

···
 SYSTEM TRACE MODULE CLASS
 M:	Alexander Shishkin <[e-mail]>
 S:	Maintained
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/ash/stm.git
 F:	Documentation/trace/stm.txt
 F:	drivers/hwtracing/stm/
 F:	include/linux/stm.h

drivers/hwtracing/intel_th/core.c

···
 	if (ret)
 		return ret;
 
+	if (thdrv->attr_group) {
+		ret = sysfs_create_group(&thdev->dev.kobj, thdrv->attr_group);
+		if (ret) {
+			thdrv->remove(thdev);
+
+			return ret;
+		}
+	}
+
 	if (thdev->type == INTEL_TH_OUTPUT &&
 	    !intel_th_output_assigned(thdev))
 		ret = hubdrv->assign(hub, thdev);
···
 		if (err)
 			return err;
 	}
+
+	if (thdrv->attr_group)
+		sysfs_remove_group(&thdev->dev.kobj, thdrv->attr_group);
 
 	thdrv->remove(thdev);
 
···
 
 static int intel_th_output_activate(struct intel_th_device *thdev)
 {
-	struct intel_th_driver *thdrv = to_intel_th_driver(thdev->dev.driver);
+	struct intel_th_driver *thdrv =
+		to_intel_th_driver_or_null(thdev->dev.driver);
+
+	if (!thdrv)
+		return -ENODEV;
+
+	if (!try_module_get(thdrv->driver.owner))
+		return -ENODEV;
 
 	if (thdrv->activate)
 		return thdrv->activate(thdev);
···
 
 static void intel_th_output_deactivate(struct intel_th_device *thdev)
 {
-	struct intel_th_driver *thdrv = to_intel_th_driver(thdev->dev.driver);
+	struct intel_th_driver *thdrv =
+		to_intel_th_driver_or_null(thdev->dev.driver);
+
+	if (!thdrv)
+		return;
 
 	if (thdrv->deactivate)
 		thdrv->deactivate(thdev);
 	else
 		intel_th_trace_disable(thdev);
+
+	module_put(thdrv->driver.owner);
 }
 
 static ssize_t active_show(struct device *dev, struct device_attribute *attr,

drivers/hwtracing/intel_th/intel_th.h

···
  * @enable:	enable tracing for a given output device
  * @disable:	disable tracing for a given output device
  * @fops:	file operations for device nodes
+ * @attr_group:	attributes provided by the driver
  *
  * Callbacks @probe and @remove are required for all device types.
  * Switch device driver needs to fill in @assign, @enable and @disable
···
 	void			(*deactivate)(struct intel_th_device *thdev);
 	/* file_operations for those who want a device node */
 	const struct file_operations *fops;
+	/* optional attributes */
+	struct attribute_group	*attr_group;
 
 	/* source ops */
 	int			(*set_output)(struct intel_th_device *thdev,
···
 
 #define to_intel_th_driver(_d)					\
 	container_of((_d), struct intel_th_driver, driver)
+
+#define to_intel_th_driver_or_null(_d)		\
+	((_d) ? to_intel_th_driver(_d) : NULL)
 
 static inline struct intel_th_device *
 to_intel_th_hub(struct intel_th_device *thdev)

drivers/hwtracing/intel_th/msu.c

···
 	atomic_t		mmap_count;
 	struct mutex		buf_mutex;
 
-	struct mutex		iter_mutex;
 	struct list_head	iter_list;
 
 	/* config */
···
 
 	iter = kzalloc(sizeof(*iter), GFP_KERNEL);
 	if (!iter)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
+
+	mutex_lock(&msc->buf_mutex);
+
+	/*
+	 * Reading and tracing are mutually exclusive; if msc is
+	 * enabled, open() will fail; otherwise existing readers
+	 * will prevent enabling the msc and the rest of fops don't
+	 * need to worry about it.
+	 */
+	if (msc->enabled) {
+		kfree(iter);
+		iter = ERR_PTR(-EBUSY);
+		goto unlock;
+	}
 
 	msc_iter_init(iter);
 	iter->msc = msc;
 
-	mutex_lock(&msc->iter_mutex);
 	list_add_tail(&iter->entry, &msc->iter_list);
-	mutex_unlock(&msc->iter_mutex);
+unlock:
+	mutex_unlock(&msc->buf_mutex);
 
 	return iter;
 }
 
 static void msc_iter_remove(struct msc_iter *iter, struct msc *msc)
 {
-	mutex_lock(&msc->iter_mutex);
+	mutex_lock(&msc->buf_mutex);
 	list_del(&iter->entry);
-	mutex_unlock(&msc->iter_mutex);
+	mutex_unlock(&msc->buf_mutex);
 
 	kfree(iter);
 }
···
 {
 	struct msc_window *win;
 
-	mutex_lock(&msc->buf_mutex);
 	list_for_each_entry(win, &msc->win_list, entry) {
 		unsigned int blk;
 		size_t hw_sz = sizeof(struct msc_block_desc) -
···
 			memset(&bdesc->hw_tag, 0, hw_sz);
 		}
 	}
-	mutex_unlock(&msc->buf_mutex);
 }
 
 /**
···
  * @msc:	the MSC device to configure
  *
  * Program storage mode, wrapping, burst length and trace buffer address
- * into a given MSC. If msc::enabled is set, enable the trace, too.
+ * into a given MSC. Then, enable tracing and set msc::enabled.
+ * The latter is serialized on msc::buf_mutex, so make sure to hold it.
  */
 static int msc_configure(struct msc *msc)
 {
 	u32 reg;
+
+	lockdep_assert_held(&msc->buf_mutex);
 
 	if (msc->mode > MSC_MODE_MULTI)
 		return -ENOTSUPP;
···
 	reg = ioread32(msc->reg_base + REG_MSU_MSC0CTL);
 	reg &= ~(MSC_MODE | MSC_WRAPEN | MSC_EN | MSC_RD_HDR_OVRD);
 
+	reg |= MSC_EN;
 	reg |= msc->mode << __ffs(MSC_MODE);
 	reg |= msc->burst_len << __ffs(MSC_LEN);
-	/*if (msc->mode == MSC_MODE_MULTI)
-	  reg |= MSC_RD_HDR_OVRD; */
+
 	if (msc->wrap)
 		reg |= MSC_WRAPEN;
-	if (msc->enabled)
-		reg |= MSC_EN;
 
 	iowrite32(reg, msc->reg_base + REG_MSU_MSC0CTL);
 
-	if (msc->enabled) {
-		msc->thdev->output.multiblock = msc->mode == MSC_MODE_MULTI;
-		intel_th_trace_enable(msc->thdev);
-	}
+	msc->thdev->output.multiblock = msc->mode == MSC_MODE_MULTI;
+	intel_th_trace_enable(msc->thdev);
+	msc->enabled = 1;
+
 
 	return 0;
 }
···
  * @msc:	MSC device to disable
  *
  * If @msc is enabled, disable tracing on the switch and then disable MSC
- * storage.
+ * storage. Caller must hold msc::buf_mutex.
  */
 static void msc_disable(struct msc *msc)
 {
 	unsigned long count;
 	u32 reg;
 
-	if (!msc->enabled)
-		return;
+	lockdep_assert_held(&msc->buf_mutex);
 
 	intel_th_trace_disable(msc->thdev);
 
···
 static int intel_th_msc_activate(struct intel_th_device *thdev)
 {
 	struct msc *msc = dev_get_drvdata(&thdev->dev);
-	int ret = 0;
+	int ret = -EBUSY;
 
 	if (!atomic_inc_unless_negative(&msc->user_count))
 		return -ENODEV;
 
-	mutex_lock(&msc->iter_mutex);
-	if (!list_empty(&msc->iter_list))
-		ret = -EBUSY;
-	mutex_unlock(&msc->iter_mutex);
+	mutex_lock(&msc->buf_mutex);
 
-	if (ret) {
+	/* if there are readers, refuse */
+	if (list_empty(&msc->iter_list))
+		ret = msc_configure(msc);
+
+	mutex_unlock(&msc->buf_mutex);
+
+	if (ret)
 		atomic_dec(&msc->user_count);
-		return ret;
-	}
 
-	msc->enabled = 1;
-
-	return msc_configure(msc);
+	return ret;
 }
 
 static void intel_th_msc_deactivate(struct intel_th_device *thdev)
 {
 	struct msc *msc = dev_get_drvdata(&thdev->dev);
 
-	msc_disable(msc);
-
-	atomic_dec(&msc->user_count);
+	mutex_lock(&msc->buf_mutex);
+	if (msc->enabled) {
+		msc_disable(msc);
+		atomic_dec(&msc->user_count);
+	}
+	mutex_unlock(&msc->buf_mutex);
 }
 
 /**
···
 		return -EPERM;
 
 	iter = msc_iter_install(msc);
-	if (!iter)
-		return -ENOMEM;
+	if (IS_ERR(iter))
+		return PTR_ERR(iter);
 
 	file->private_data = iter;
 
···
 
 	if (!atomic_inc_unless_negative(&msc->user_count))
 		return 0;
-
-	if (msc->enabled) {
-		ret = -EBUSY;
-		goto put_count;
-	}
 
 	if (msc->mode == MSC_MODE_SINGLE && !msc->single_wrap)
 		size = msc->single_sz;
···
 	.read		= intel_th_msc_read,
 	.mmap		= intel_th_msc_mmap,
 	.llseek		= no_llseek,
+	.owner		= THIS_MODULE,
 };
 
 static int intel_th_msc_init(struct msc *msc)
···
 	msc->mode = MSC_MODE_MULTI;
 	mutex_init(&msc->buf_mutex);
 	INIT_LIST_HEAD(&msc->win_list);
-
-	mutex_init(&msc->iter_mutex);
 	INIT_LIST_HEAD(&msc->iter_list);
 
 	msc->burst_len =
···
 	do {
 		end = memchr(p, ',', len);
 		s = kstrndup(p, end ? end - p : len, GFP_KERNEL);
+		if (!s) {
+			ret = -ENOMEM;
+			goto free_win;
+		}
+
 		ret = kstrtoul(s, 10, &val);
 		kfree(s);
 
···
 	if (err)
 		return err;
 
-	err = sysfs_create_group(&dev->kobj, &msc_output_group);
-	if (err)
-		return err;
-
 	dev_set_drvdata(dev, msc);
 
 	return 0;
···
 
 static void intel_th_msc_remove(struct intel_th_device *thdev)
 {
-	sysfs_remove_group(&thdev->dev.kobj, &msc_output_group);
+	struct msc *msc = dev_get_drvdata(&thdev->dev);
+	int ret;
+
+	intel_th_msc_deactivate(thdev);
+
+	/*
+	 * Buffers should not be used at this point except if the
+	 * output character device is still open and the parent
+	 * device gets detached from its bus, which is a FIXME.
+	 */
+	ret = msc_buffer_free_unless_used(msc);
+	WARN_ON_ONCE(ret);
 }
 
 static struct intel_th_driver intel_th_msc_driver = {
···
 	.activate	= intel_th_msc_activate,
 	.deactivate	= intel_th_msc_deactivate,
 	.fops	= &intel_th_msc_fops,
+	.attr_group	= &msc_output_group,
 	.driver	= {
 		.name	= "msc",
 		.owner	= THIS_MODULE,

drivers/hwtracing/intel_th/pci.c

···
 		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x0a80),
 		.driver_data = (kernel_ulong_t)0,
 	},
+	{
+		/* Broxton B-step */
+		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x1a8e),
+		.driver_data = (kernel_ulong_t)0,
+	},
 	{ 0 },
 };
 

drivers/hwtracing/intel_th/pti.c

···
 	struct resource *res;
 	struct pti_device *pti;
 	void __iomem *base;
-	int ret;
 
 	res = intel_th_device_get_resource(thdev, IORESOURCE_MEM, 0);
 	if (!res)
···
 
 	read_hw_config(pti);
 
-	ret = sysfs_create_group(&dev->kobj, &pti_output_group);
-	if (ret)
-		return ret;
-
 	dev_set_drvdata(dev, pti);
 
 	return 0;
···
 	.remove	= intel_th_pti_remove,
 	.activate	= intel_th_pti_activate,
 	.deactivate	= intel_th_pti_deactivate,
+	.attr_group	= &pti_output_group,
 	.driver	= {
 		.name	= "pti",
 		.owner	= THIS_MODULE,

drivers/hwtracing/stm/core.c

···
 	if (ret)
 		goto err_free;
 
-	ret = 0;
-
 	if (stm->data->link)
 		ret = stm->data->link(stm->data, stmf->output.master,
 				      stmf->output.channel);
···
 	stm->dev.parent = parent;
 	stm->dev.release = stm_device_release;
 
+	mutex_init(&stm->link_mutex);
+	spin_lock_init(&stm->link_lock);
+	INIT_LIST_HEAD(&stm->link_list);
+
+	/* initialize the object before it is accessible via sysfs */
+	spin_lock_init(&stm->mc_lock);
+	mutex_init(&stm->policy_mutex);
+	stm->sw_nmasters = nmasters;
+	stm->owner = owner;
+	stm->data = stm_data;
+	stm_data->stm = stm;
+
 	err = kobject_set_name(&stm->dev.kobj, "%s", stm_data->name);
 	if (err)
 		goto err_device;
···
 	if (err)
 		goto err_device;
 
-	mutex_init(&stm->link_mutex);
-	spin_lock_init(&stm->link_lock);
-	INIT_LIST_HEAD(&stm->link_list);
-
-	spin_lock_init(&stm->mc_lock);
-	mutex_init(&stm->policy_mutex);
-	stm->sw_nmasters = nmasters;
-	stm->owner = owner;
-	stm->data = stm_data;
-	stm_data->stm = stm;
-
 	return 0;
 
 err_device:
+	unregister_chrdev(stm->major, stm_data->name);
+
 	/* matches device_initialize() above */
 	put_device(&stm->dev);
 err_free:

drivers/hwtracing/stm/dummy_stm.c

···
 
 static int nr_dummies = 4;
 
-module_param(nr_dummies, int, 0600);
-
-static unsigned int dummy_stm_nr;
+module_param(nr_dummies, int, 0400);
 
 static unsigned int fail_mode;
 
···
 
 static int dummy_stm_init(void)
 {
-	int i, ret = -ENOMEM, __nr_dummies = ACCESS_ONCE(nr_dummies);
+	int i, ret = -ENOMEM;
 
-	if (__nr_dummies < 0 || __nr_dummies > DUMMY_STM_MAX)
+	if (nr_dummies < 0 || nr_dummies > DUMMY_STM_MAX)
 		return -EINVAL;
 
-	for (i = 0; i < __nr_dummies; i++) {
+	for (i = 0; i < nr_dummies; i++) {
 		dummy_stm[i].name = kasprintf(GFP_KERNEL, "dummy_stm.%d", i);
 		if (!dummy_stm[i].name)
 			goto fail_unregister;
···
 		if (ret)
 			goto fail_free;
 	}
-
-	dummy_stm_nr = __nr_dummies;
 
 	return 0;
 
···
 {
 	int i;
 
-	for (i = 0; i < dummy_stm_nr; i++) {
+	for (i = 0; i < nr_dummies; i++) {
 		stm_unregister_device(&dummy_stm[i]);
 		kfree(dummy_stm[i].name);
 	}

drivers/hwtracing/stm/heartbeat.c

···
 static int nr_devs = 4;
 static int interval_ms = 10;
 
-module_param(nr_devs, int, 0600);
+module_param(nr_devs, int, 0400);
 module_param(interval_ms, int, 0600);
 
 static struct stm_heartbeat {
···
 	struct hrtimer		hrtimer;
 	unsigned int		active;
 } stm_heartbeat[STM_HEARTBEAT_MAX];
-
-static unsigned int nr_instances;
 
 static const char str[] = "heartbeat stm source driver is here to serve you";
 
···
 
 static int stm_heartbeat_init(void)
 {
-	int i, ret = -ENOMEM, __nr_instances = ACCESS_ONCE(nr_devs);
+	int i, ret = -ENOMEM;
 
-	if (__nr_instances < 0 || __nr_instances > STM_HEARTBEAT_MAX)
+	if (nr_devs < 0 || nr_devs > STM_HEARTBEAT_MAX)
 		return -EINVAL;
 
-	for (i = 0; i < __nr_instances; i++) {
+	for (i = 0; i < nr_devs; i++) {
 		stm_heartbeat[i].data.name =
 			kasprintf(GFP_KERNEL, "heartbeat.%d", i);
 		if (!stm_heartbeat[i].data.name)
···
 			goto fail_free;
 	}
 
-	nr_instances = __nr_instances;
-
 	return 0;
 
 fail_unregister:
···
 {
 	int i;
 
-	for (i = 0; i < nr_instances; i++) {
+	for (i = 0; i < nr_devs; i++) {
 		stm_source_unregister_device(&stm_heartbeat[i].data);
 		kfree(stm_heartbeat[i].data.name);
 	}

drivers/hwtracing/stm/policy.c

···
 		goto unlock;
 
 	/* must be within [sw_start..sw_end], which is an inclusive range */
-	if (first > INT_MAX || last > INT_MAX || first > last ||
-	    first < stm->data->sw_start ||
+	if (first > last || first < stm->data->sw_start ||
 	    last > stm->data->sw_end) {
 		ret = -ERANGE;
 		goto unlock;
···
 		return ERR_PTR(-EINVAL);
 	}
 
-	*p++ = '\0';
+	*p = '\0';
 
 	stm = stm_find_device(devname);
 	kfree(devname);