[PATCH v2 2/8] reset: Add reset controller API

On Wed, Feb 13, 2013 at 06:34:26PM +0100, Philipp Zabel wrote:

I see two kerneldoc styles in the file.  Some have new lines here while
others do not.

And @rcdev is missing.

+ * @id: ID of the reset controller in the reset
+ *      controller device
+ */
+struct reset_control {
+	struct reset_controller_dev *rcdev;
+	unsigned int id;
+};
+
+/**
+ * reset_controller_register - register a reset controller device
+ *
+ * @rcdev: a pointer to struct reset_controller_dev
+ */
+int reset_controller_register(struct reset_controller_dev *rcdev)
+{
+	mutex_lock(&reset_controller_list_mutex);
+	list_add(&rcdev->list, &reset_controller_list);
+	mutex_unlock(&reset_controller_list_mutex);
+
+	return 0;
+}

I think we need EXPORT_SYMBOL_GPL on it, as the gpio-reset driver
added in patch #8 which supports module build will need it.

+
+/**
+ * reset_controller_unregister - unregister a reset controller device
+ *
+ * @rcdev: a pointer to struct reset_controller_dev
+ */
+void reset_controller_unregister(struct reset_controller_dev *rcdev)
+{
+	mutex_lock(&reset_controller_list_mutex);
+	list_del(&rcdev->list);
+	mutex_unlock(&reset_controller_list_mutex);
+}

Ditto

+
+/**
+ * reset_control_reset - reset the controlled device
+ * @rstc: reset controller
+ */
+int reset_control_reset(struct reset_control *rstc)
+{
+	if (rstc->rcdev->ops->reset)
+		return rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
+
+	return -ENOSYS;
+}
+EXPORT_SYMBOL_GPL(reset_control_reset);
+
+/**
+ * reset_control_assert - asserts the reset line
+ * @rstc: reset controller
+ */
+int reset_control_assert(struct reset_control *rstc)
+{
+	if (rstc->rcdev->ops->assert)
+		return rstc->rcdev->ops->assert(rstc->rcdev, rstc->id);
+
+	return -ENOSYS;
+}
+EXPORT_SYMBOL_GPL(reset_control_assert);
+
+/**
+ * reset_control_deassert - deasserts the reset line
+ * @rstc: reset controller
+ */
+int reset_control_deassert(struct reset_control *rstc)
+{
+	if (rstc->rcdev->ops->deassert)
+		return rstc->rcdev->ops->deassert(rstc->rcdev, rstc->id);
+
+	return -ENOSYS;
+}
+EXPORT_SYMBOL_GPL(reset_control_deassert);
+
+/**
+ * reset_control_get - Lookup and obtain a reference to a reset controller.
+ * @dev: device to be reset by the controller
+ * @id: reset line name
+ *
+ * Returns a struct reset_control or IS_ERR() condition containing errno.
+ *
+ * Use of id names is optional.
+ */
+struct reset_control *reset_control_get(struct device *dev, const char *id)
+{
+	struct reset_control *rstc = ERR_PTR(-EPROBE_DEFER);
+	struct reset_controller_dev *r, *rcdev;
+	struct device_node *rcdev_node;
+	struct of_phandle_args args;
+	int rcdev_index;
+	int ret;
+	int i;
+
+	if (!dev)
+		return ERR_PTR(-EINVAL);
+
+	rcdev_node = NULL;
+	for (i = 0; rcdev_node == NULL; i++) {
+		ret = of_parse_phandle_with_args(dev->of_node, "resets",
+						 "#reset-cells", i, &args);
+		if (ret)
+			return ERR_PTR(ret);
+		of_node_put(args.np);
+		if (args.args_count <= 0)
+			return ERR_PTR(-EINVAL);
+
+		if (id) {
+			const char *reset_name;
+			ret = of_property_read_string_index(dev->of_node,
+							    "reset-names", i,
+							    &reset_name);
+			if (ret)
+				return ERR_PTR(ret);
+			if (strcmp(id, reset_name) != 0)
+				continue;
+		}
+
+		rcdev_index = args.args[0];
+		rcdev_node = args.np;
+		break;
+	}

I feel the block could be simplified a little bit by calling
of_property_match_string(dev->of_node, "reset-names", id) to find the
index that is needed by of_parse_phandle_with_args() call. 

You might want to take a look at of_clk_get_by_name() -
drivers/clk/clkdev.c for example.

+
+	mutex_lock(&reset_controller_list_mutex);
+	rcdev = NULL;
+	list_for_each_entry(r, &reset_controller_list, list) {
+		if (rcdev_node == r->of_node) {
+			rcdev = r;
+			break;
+		}
+	}
+	mutex_unlock(&reset_controller_list_mutex);
+
+	if (!rcdev)
+		return ERR_PTR(-ENODEV);
+
+	try_module_get(rcdev->owner);
+
+	rstc = kzalloc(sizeof(*rstc), GFP_KERNEL);
+	if (!rstc)
+		return ERR_PTR(-ENOMEM);
+
+	rstc->rcdev = rcdev;
+	rstc->id = rcdev_index;
+
+	return rstc;
+}
+EXPORT_SYMBOL_GPL(reset_control_get);
+
+/**
+ * reset_control_put - free the reset controller
+ * @reset: reset controller

@rstc, not @reset.

+ */
+
+void reset_control_put(struct reset_control *rstc)
+{
+	if (IS_ERR(rstc))
+		return;
+
+	module_put(rstc->rcdev->owner);
+	kfree(rstc);
+}
+EXPORT_SYMBOL_GPL(reset_control_put);
+
+static void devm_reset_control_release(struct device *dev, void *res)
+{
+	reset_control_put(*(struct reset_control **)res);
+}
+
+/**
+ * devm_reset_control_get - resource managed reset_control_get()
+ * @dev: device to be reset by the controller
+ * @id: reset line name
+ *
+ * Managed reset_control_get(). For reset controllers returned from this
+ * function, reset_control_put() is called automatically on driver detach.
+ * See reset_control_get() for more information.
+ */
+struct reset_control *devm_reset_control_get(struct device *dev, const char *id)
+{
+	struct reset_control **ptr, *rstc;
+
+	ptr = devres_alloc(devm_reset_control_release, sizeof(*ptr),
+			   GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	rstc = reset_control_get(dev, id);
+	if (!IS_ERR(rstc)) {
+		*ptr = rstc;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return rstc;
+}
+EXPORT_SYMBOL_GPL(devm_reset_control_get);
+
+/**
+ * device_reset - find reset controller associated with the device
+ *                and perform reset
+ * @dev: device to be reset by the controller
+ *
+ * Convenience wrapper for reset_control_get() and reset_control_reset().
+ * This is useful for the common case of devices with single, dedicated reset
+ * lines.
+ */
+int device_reset(struct device *dev)
+{
+	struct reset_control *rstc;
+	int ret;
+
+	rstc = reset_control_get(dev, NULL);
+	if (IS_ERR(rstc))
+		return PTR_ERR(rstc);
+
+	ret = reset_control_reset(rstc);
+
+	kfree(rstc);

Shouldn't reset_control_put() be called here instead?

s/reset_controller/reset_controller_dev

+ *                           provide multiple reset controls

Kerneldoc of parameters are missing. 

Shawn