diff mbox

[v2,01/12] reset: add the Berlin reset controller driver

Message ID 1403606121-6368-2-git-send-email-antoine.tenart@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Antoine Tenart June 24, 2014, 10:35 a.m. UTC
Add a reset controller for Marvell Berlin SoCs which is used by the
USB PHYs drivers (for now).

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
 drivers/reset/Makefile       |   1 +
 drivers/reset/reset-berlin.c | 131 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 132 insertions(+)
 create mode 100644 drivers/reset/reset-berlin.c

Comments

Philipp Zabel June 24, 2014, 11:13 a.m. UTC | #1
Hi Antoine,

Am Dienstag, den 24.06.2014, 12:35 +0200 schrieb Antoine Ténart:
[...]
> +++ b/drivers/reset/reset-berlin.c
> @@ -0,0 +1,131 @@
> +/*
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine Ténart <antoine.tenart@free-electrons.com>
> + * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#define to_berlin_reset_priv(p)		\
> +	container_of((p), struct berlin_reset_priv, rcdev)
> +
> +struct berlin_reset_priv {
> +	spinlock_t			lock;

lock is not used anymore.

[...]
> +static int berlin_reset_xlate(struct reset_controller_dev *rcdev,
> +			      const struct of_phandle_args *reset_spec)
> +{
> +	struct berlin_reset_priv *priv = to_berlin_reset_priv(rcdev);
> +	unsigned offset, bit;
> +
> +	if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
> +		return -EINVAL;
> +
> +	offset = reset_spec->args[0];
> +	bit = reset_spec->args[1];
> +
> +	if (offset >= priv->size)
> +		return -EINVAL;
> +
> +	if (bit >= rcdev->nr_resets)
> +		return -EINVAL;

This could be considered a misuse of nr_resets, even if the core only
ever uses nr_resets in the _xlate function. Maybe it would be more clear
to just leave nr_resets empty if you don't know the actual number and
hardcode 32 here.

[...]
> +static int __berlin_reset_init(struct device_node *np)
> +{
> +	struct berlin_reset_priv *priv;
> +	struct resource res;
> +	resource_size_t size;
> +	int ret;
> +
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	ret = of_address_to_resource(np, 0, &res);
> +	if (ret)
> +		goto err;
> +
> +	size = resource_size(&res);
> +	priv->base = ioremap(res.start, size);
> +	if (!priv->base) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +	priv->size = size;
> +
> +	priv->rcdev.owner = THIS_MODULE;
> +	priv->rcdev.nr_resets = 32;

Leave nr_resets empty, use the correct value, or at least add a comment
that this is not the number of resets in the rcdev as documented in the
structure documentation.

> +	priv->rcdev.ops = &berlin_reset_ops;
> +	priv->rcdev.of_node = np;
> +	priv->rcdev.of_reset_n_cells = 2;
> +	priv->rcdev.of_xlate = berlin_reset_xlate;
> +
> +	reset_controller_register(&priv->rcdev);
> +
> +	return 0;
> +
> +err:
> +	kfree(priv);
> +	return ret;
> +}
> +
> +static const struct of_device_id berlin_reset_of_match[] __initconst = {
> +	{ .compatible = "marvell,berlin2-chip-ctrl" },
> +	{ .compatible = "marvell,berlin2cd-chip-ctrl" },
> +	{ .compatible = "marvell,berlin2q-chip-ctrl" },
> +	{ },
> +};
> +
> +static int __init berlin_reset_init(void)
> +{
> +	struct device_node *np;
> +	int ret;
> +
> +	for_each_matching_node(np, berlin_reset_of_match) {
> +		ret = __berlin_reset_init(np);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +arch_initcall(berlin_reset_init);

Other than the above, and with the understanding that this is going to
be turned into a platform driver at some point in the future,

Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp
Antoine Tenart June 24, 2014, 12:05 p.m. UTC | #2
Hi Philipp,

On Tue, Jun 24, 2014 at 01:13:36PM +0200, Philipp Zabel wrote:
> Am Dienstag, den 24.06.2014, 12:35 +0200 schrieb Antoine Ténart:
> > +
> > +struct berlin_reset_priv {
> > +	spinlock_t			lock;
> 
> lock is not used anymore.

Oops. I'll remove it.

> 
> [...]
> > +static int berlin_reset_xlate(struct reset_controller_dev *rcdev,
> > +			      const struct of_phandle_args *reset_spec)
> > +{
> > +	struct berlin_reset_priv *priv = to_berlin_reset_priv(rcdev);
> > +	unsigned offset, bit;
> > +
> > +	if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
> > +		return -EINVAL;
> > +
> > +	offset = reset_spec->args[0];
> > +	bit = reset_spec->args[1];
> > +
> > +	if (offset >= priv->size)
> > +		return -EINVAL;
> > +
> > +	if (bit >= rcdev->nr_resets)
> > +		return -EINVAL;
> 
> This could be considered a misuse of nr_resets, even if the core only
> ever uses nr_resets in the _xlate function. Maybe it would be more clear
> to just leave nr_resets empty if you don't know the actual number and
> hardcode 32 here.
> 
> [...]
> > +static int __berlin_reset_init(struct device_node *np)
> > +{
> > +	struct berlin_reset_priv *priv;
> > +	struct resource res;
> > +	resource_size_t size;
> > +	int ret;
> > +
> > +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	ret = of_address_to_resource(np, 0, &res);
> > +	if (ret)
> > +		goto err;
> > +
> > +	size = resource_size(&res);
> > +	priv->base = ioremap(res.start, size);
> > +	if (!priv->base) {
> > +		ret = -ENOMEM;
> > +		goto err;
> > +	}
> > +	priv->size = size;
> > +
> > +	priv->rcdev.owner = THIS_MODULE;
> > +	priv->rcdev.nr_resets = 32;
> 
> Leave nr_resets empty, use the correct value, or at least add a comment
> that this is not the number of resets in the rcdev as documented in the
> structure documentation.

Ok. I'll hardcode the value in the driver then.

> 
> Other than the above, and with the understanding that this is going to
> be turned into a platform driver at some point in the future,
> 
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

Sure, that's the plan.

Antoine
diff mbox

Patch

diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 60fed3d7820b..157d421f755b 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -1,4 +1,5 @@ 
 obj-$(CONFIG_RESET_CONTROLLER) += core.o
 obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
+obj-$(CONFIG_ARCH_BERLIN) += reset-berlin.o
 obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_ARCH_STI) += sti/
diff --git a/drivers/reset/reset-berlin.c b/drivers/reset/reset-berlin.c
new file mode 100644
index 000000000000..c988c08825b4
--- /dev/null
+++ b/drivers/reset/reset-berlin.c
@@ -0,0 +1,131 @@ 
+/*
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine Ténart <antoine.tenart@free-electrons.com>
+ * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#define to_berlin_reset_priv(p)		\
+	container_of((p), struct berlin_reset_priv, rcdev)
+
+struct berlin_reset_priv {
+	spinlock_t			lock;
+	void __iomem			*base;
+	unsigned int			size;
+	struct reset_controller_dev	rcdev;
+};
+
+static int berlin_reset_reset(struct reset_controller_dev *rcdev,
+			      unsigned long id)
+{
+	struct berlin_reset_priv *priv = to_berlin_reset_priv(rcdev);
+	int offset = id >> 8;
+	int mask = BIT(id & 0xff);
+
+	writel(mask, priv->base + offset);
+
+	/* let the reset be effective */
+	udelay(10);
+
+	return 0;
+}
+
+static struct reset_control_ops berlin_reset_ops = {
+	.reset	= berlin_reset_reset,
+};
+
+static int berlin_reset_xlate(struct reset_controller_dev *rcdev,
+			      const struct of_phandle_args *reset_spec)
+{
+	struct berlin_reset_priv *priv = to_berlin_reset_priv(rcdev);
+	unsigned offset, bit;
+
+	if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
+		return -EINVAL;
+
+	offset = reset_spec->args[0];
+	bit = reset_spec->args[1];
+
+	if (offset >= priv->size)
+		return -EINVAL;
+
+	if (bit >= rcdev->nr_resets)
+		return -EINVAL;
+
+	return (offset << 8) | bit;
+}
+
+static int __berlin_reset_init(struct device_node *np)
+{
+	struct berlin_reset_priv *priv;
+	struct resource res;
+	resource_size_t size;
+	int ret;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	ret = of_address_to_resource(np, 0, &res);
+	if (ret)
+		goto err;
+
+	size = resource_size(&res);
+	priv->base = ioremap(res.start, size);
+	if (!priv->base) {
+		ret = -ENOMEM;
+		goto err;
+	}
+	priv->size = size;
+
+	priv->rcdev.owner = THIS_MODULE;
+	priv->rcdev.nr_resets = 32;
+	priv->rcdev.ops = &berlin_reset_ops;
+	priv->rcdev.of_node = np;
+	priv->rcdev.of_reset_n_cells = 2;
+	priv->rcdev.of_xlate = berlin_reset_xlate;
+
+	reset_controller_register(&priv->rcdev);
+
+	return 0;
+
+err:
+	kfree(priv);
+	return ret;
+}
+
+static const struct of_device_id berlin_reset_of_match[] __initconst = {
+	{ .compatible = "marvell,berlin2-chip-ctrl" },
+	{ .compatible = "marvell,berlin2cd-chip-ctrl" },
+	{ .compatible = "marvell,berlin2q-chip-ctrl" },
+	{ },
+};
+
+static int __init berlin_reset_init(void)
+{
+	struct device_node *np;
+	int ret;
+
+	for_each_matching_node(np, berlin_reset_of_match) {
+		ret = __berlin_reset_init(np);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+arch_initcall(berlin_reset_init);