[PATCH 1/2] reset: Add brcm,bcm63xx-reset device tree binding

[PATCH 1/2] reset: Add brcm,bcm63xx-reset device tree binding

[Simon Arlott]

Add device tree binding for the BCM63xx soft reset controller.

The BCM63xx contains a soft-reset controller activated by setting
a bit (that must previously have cleared).

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
 .../bindings/reset/brcm,bcm63xx-reset.txt          | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm63xx-reset.txt

diff --git a/Documentation/devicetree/bindings/reset/brcm,bcm63xx-reset.txt b/Documentation/devicetree/bindings/reset/brcm,bcm63xx-reset.txt
new file mode 100644
index 0000000..48e9daf
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/brcm,bcm63xx-reset.txt
@@ -0,0 +1,37 @@
+BCM63xx reset controller
+
+The BCM63xx contains a basic soft reset controller in the perf register
+set which resets components using a bit in a register.
+
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+Required properties:
+- compatible:	Should be "brcm,bcm<soc>-reset", "brcm,bcm63xx-reset"
+- regmap:	The register map phandle
+- offset:	Offset in the register map for the reset register (in bytes)
+- #reset-cells:	Must be set to 1
+
+Optional properties:
+- mask:		Mask of valid reset bits in the reset register (32 bit access)
+		(Defaults to all bits)
+
+Example:
+
+periph_soft_rst: reset-controller {
+	compatible = "brcm,bcm63168-reset", "brcm,bcm63xx-reset";
+	regmap = <&periph_cntl>;
+	offset = <0x10>;
+
+	#reset-cells = <1>;
+};
+
+usbh: usbphy@10002700 {
+	compatible = "brcm,bcm63168-usbh";
+	reg = <0x10002700 0x38>;
+	clocks = <&periph_clk 13>, <&timer_clk 18>;
+	resets = <&periph_soft_rst 6>;
+	power-supply = <&power_usbh>;
+	#phy-cells = <0>;
+};
+
-- 
2.1.4

-- 
Simon Arlott

[PATCH 2/2] reset: bcm63xx: Add support for the BCM63xx soft-reset controller

[Simon Arlott]

The BCM63xx contains a soft-reset controller activated by setting
a bit (that must previously have cleared).

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
 MAINTAINERS                   |   1 +
 drivers/reset/Kconfig         |   9 +++
 drivers/reset/Makefile        |   1 +
 drivers/reset/reset-bcm63xx.c | 134 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 145 insertions(+)
 create mode 100644 drivers/reset/reset-bcm63xx.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 07613dd..55e493a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2378,6 +2378,7 @@ F:	drivers/irqchip/irq-bcm63*
 F:	drivers/irqchip/irq-bcm7*
 F:	drivers/irqchip/irq-brcmstb*
 F:	drivers/regulator/bcm63*
+F:	drivers/reset/reset-bcm63*
 F:	include/linux/bcm63xx_wdt.h
 
 BROADCOM TG3 GIGABIT ETHERNET DRIVER
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 0615f50..064dad2 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -12,4 +12,13 @@ menuconfig RESET_CONTROLLER
 
 	  If unsure, say no.
 
+if RESET_CONTROLLER
+
+config RESET_BCM63XX
+	bool "BCM63xx Reset Controller"
+	help
+	  Support resetting of devices on BCM63xx boards.
+
+endif
+
 source "drivers/reset/sti/Kconfig"
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 85d5904..f6e2171 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_ARCH_STI) += sti/
 obj-$(CONFIG_ARCH_ZYNQ) += reset-zynq.o
 obj-$(CONFIG_ATH79) += reset-ath79.o
+obj-$(CONFIG_RESET_BCM63XX) += reset-bcm63xx.o
diff --git a/drivers/reset/reset-bcm63xx.c b/drivers/reset/reset-bcm63xx.c
new file mode 100644
index 0000000..46db57f
--- /dev/null
+++ b/drivers/reset/reset-bcm63xx.c
@@ -0,0 +1,134 @@
+/*
+ * Copyright 2015 Simon Arlott
+ *
+ * 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.
+ *
+ * Derived from reset-berlin.c:
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine Tenart <antoine.tenart@free-electrons.com>
+ * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#define to_bcm63xx_reset_priv(p) \
+		container_of((p), struct bcm63xx_reset_priv, rcdev)
+
+struct bcm63xx_reset_priv {
+	struct regmap *map;
+	u32 offset;
+	u32 mask; /* valid reset bits */
+	struct reset_controller_dev rcdev;
+	struct mutex mutex;
+};
+
+static int bcm63xx_reset_reset(struct reset_controller_dev *rcdev,
+	unsigned long id)
+{
+	struct bcm63xx_reset_priv *priv = to_bcm63xx_reset_priv(rcdev);
+
+	mutex_lock(&priv->mutex);
+	regmap_write_bits(priv->map, priv->offset, BIT(id), 0);
+	usleep_range(10000, 20000);
+	regmap_write_bits(priv->map, priv->offset, BIT(id), BIT(id));
+	usleep_range(10000, 20000);
+	mutex_unlock(&priv->mutex);
+
+	return 0;
+}
+
+static int bcm63xx_reset_xlate(struct reset_controller_dev *rcdev,
+	const struct of_phandle_args *reset_spec)
+{
+	struct bcm63xx_reset_priv *priv = to_bcm63xx_reset_priv(rcdev);
+
+	if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
+		return -EINVAL;
+
+	if (reset_spec->args[0] >= rcdev->nr_resets)
+		return -EINVAL;
+
+	if (!(BIT(reset_spec->args[0]) & priv->mask))
+		return -EINVAL;
+
+	return reset_spec->args[0];
+}
+
+
+static struct reset_control_ops bcm63xx_reset_ops = {
+	.reset = bcm63xx_reset_reset,
+};
+
+static int __init bcm63xx_reset_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct bcm63xx_reset_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	mutex_init(&priv->mutex);
+
+	priv->map = syscon_regmap_lookup_by_phandle(np, "regmap");
+	if (IS_ERR(priv->map))
+		return PTR_ERR(priv->map);
+
+	if (of_property_read_u32(np, "offset", &priv->offset))
+		return -EINVAL;
+
+	/* valid reset bits */
+	if (of_property_read_u32(np, "mask", &priv->mask))
+		priv->mask = 0xffffffff;
+
+	priv->rcdev.owner = THIS_MODULE;
+	priv->rcdev.ops = &bcm63xx_reset_ops;
+	priv->rcdev.nr_resets = 32;
+	priv->rcdev.of_node = pdev->dev.of_node;
+	priv->rcdev.of_reset_n_cells = 1;
+	priv->rcdev.of_xlate = bcm63xx_reset_xlate;
+
+	ret = reset_controller_register(&priv->rcdev);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"unable to register reset controller: %d\n", ret);
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "registered reset controller\n");
+	return 0;
+}
+
+static const struct of_device_id bcm63xx_reset_dt_match[] __initconst = {
+	{ .compatible = "brcm,bcm63xx-reset" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, bcm63xx_reset_dt_match);
+
+static struct platform_driver bcm63xx_reset_driver __refdata = {
+	.probe	= bcm63xx_reset_probe,
+	.driver	= {
+		.name = "bcm63xx-reset",
+		.of_match_table = bcm63xx_reset_dt_match,
+	},
+};
+module_platform_driver(bcm63xx_reset_driver);
+
+MODULE_DESCRIPTION("BCM63xx reset driver");
+MODULE_AUTHOR("Simon Arlott");
+MODULE_LICENSE("GPL");
-- 
2.1.4

-- 
Simon Arlott

Re: [PATCH 1/2] reset: Add brcm,bcm63xx-reset device tree binding

[Rob Herring]

On Mon, Nov 30, 2015 at 08:57:31PM +0000, Simon Arlott wrote:
> Add device tree binding for the BCM63xx soft reset controller.
> 
> The BCM63xx contains a soft-reset controller activated by setting
> a bit (that must previously have cleared).
> 
> Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
> ---
>  .../bindings/reset/brcm,bcm63xx-reset.txt          | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm63xx-reset.txt
> 
> diff --git a/Documentation/devicetree/bindings/reset/brcm,bcm63xx-reset.txt b/Documentation/devicetree/bindings/reset/brcm,bcm63xx-reset.txt
> new file mode 100644
> index 0000000..48e9daf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/brcm,bcm63xx-reset.txt
> @@ -0,0 +1,37 @@
> +BCM63xx reset controller
> +
> +The BCM63xx contains a basic soft reset controller in the perf register
> +set which resets components using a bit in a register.
> +
> +Please also refer to reset.txt in this directory for common reset
> +controller binding usage.
> +
> +Required properties:
> +- compatible:	Should be "brcm,bcm<soc>-reset", "brcm,bcm63xx-reset"
> +- regmap:	The register map phandle
> +- offset:	Offset in the register map for the reset register (in bytes)
> +- #reset-cells:	Must be set to 1
> +
> +Optional properties:
> +- mask:		Mask of valid reset bits in the reset register (32 bit access)
> +		(Defaults to all bits)

The bits correspond to the cell values in resets properties? If so then, 
is this really needed? If you are only validating cell values against 
this mask, then drop it (assume the DT does not have crap).

Rob

> +
> +Example:
> +
> +periph_soft_rst: reset-controller {
> +	compatible = "brcm,bcm63168-reset", "brcm,bcm63xx-reset";
> +	regmap = <&periph_cntl>;
> +	offset = <0x10>;
> +
> +	#reset-cells = <1>;
> +};
> +
> +usbh: usbphy@10002700 {
> +	compatible = "brcm,bcm63168-usbh";
> +	reg = <0x10002700 0x38>;
> +	clocks = <&periph_clk 13>, <&timer_clk 18>;
> +	resets = <&periph_soft_rst 6>;
> +	power-supply = <&power_usbh>;
> +	#phy-cells = <0>;
> +};
> +
> -- 
> 2.1.4
> 
> -- 
> Simon Arlott

Re: [PATCH 1/2] reset: Add brcm,bcm63xx-reset device tree binding

[Sergei Shtylyov]

Hello.

On 11/30/2015 11:57 PM, Simon Arlott wrote:

> Add device tree binding for the BCM63xx soft reset controller.
>
> The BCM63xx contains a soft-reset controller activated by setting
> a bit (that must previously have cleared).
>
> Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
> ---
>   .../bindings/reset/brcm,bcm63xx-reset.txt          | 37 ++++++++++++++++++++++
>   1 file changed, 37 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm63xx-reset.txt
>
> diff --git a/Documentation/devicetree/bindings/reset/brcm,bcm63xx-reset.txt b/Documentation/devicetree/bindings/reset/brcm,bcm63xx-reset.txt
> new file mode 100644
> index 0000000..48e9daf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/brcm,bcm63xx-reset.txt
> @@ -0,0 +1,37 @@
> +BCM63xx reset controller
> +
> +The BCM63xx contains a basic soft reset controller in the perf register
> +set which resets components using a bit in a register.
> +
> +Please also refer to reset.txt in this directory for common reset
> +controller binding usage.
> +
> +Required properties:
> +- compatible:	Should be "brcm,bcm<soc>-reset", "brcm,bcm63xx-reset"

     Wildcards (xx) are not allowed here. Please choose a "least common 
denominator" SoC and name the string after it.

[...]

MBR, Sergei

Re: [PATCH 2/2] reset: bcm63xx: Add support for the BCM63xx soft-reset controller

[Florian Fainelli]

2015-11-30 12:58 GMT-08:00 Simon Arlott <simon@fire.lp0.eu>:
> The BCM63xx contains a soft-reset controller activated by setting
> a bit (that must previously have cleared).
>
> Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
> ---
>  MAINTAINERS                   |   1 +
>  drivers/reset/Kconfig         |   9 +++
>  drivers/reset/Makefile        |   1 +
>  drivers/reset/reset-bcm63xx.c | 134 ++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 145 insertions(+)
>  create mode 100644 drivers/reset/reset-bcm63xx.c


Could you create a bcm directory and then add your reset-bcm63xx.c
file there? I have a pending submission for the BCM63138 reset
controller which is not at all using the same structure and will share
nothing with your driver.

[snip]

> +static int bcm63xx_reset_xlate(struct reset_controller_dev *rcdev,
> +       const struct of_phandle_args *reset_spec)
> +{
> +       struct bcm63xx_reset_priv *priv = to_bcm63xx_reset_priv(rcdev);
> +
> +       if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
> +               return -EINVAL;
> +
> +       if (reset_spec->args[0] >= rcdev->nr_resets)
> +               return -EINVAL;

Should not these two things be moved to the core reset controller code
based on the #reset-cells value?

[snip]

> +       if (of_property_read_u32(np, "offset", &priv->offset))
> +               return -EINVAL;
> +
> +       /* valid reset bits */
> +       if (of_property_read_u32(np, "mask", &priv->mask))
> +               priv->mask = 0xffffffff;
> +
> +       priv->rcdev.owner = THIS_MODULE;
> +       priv->rcdev.ops = &bcm63xx_reset_ops;
> +       priv->rcdev.nr_resets = 32;

Should not that come from Device Tree, or be computed based on the
mask property, like hweight_long() or something along these lines?

> +       priv->rcdev.of_node = pdev->dev.of_node;
> +       priv->rcdev.of_reset_n_cells = 1;
> +       priv->rcdev.of_xlate = bcm63xx_reset_xlate;

-- 
Florian

Re: [PATCH 2/2] reset: bcm63xx: Add support for the BCM63xx soft-reset controller

[Simon Arlott]

On 02/12/15 18:03, Florian Fainelli wrote:
> 2015-11-30 12:58 GMT-08:00 Simon Arlott <simon@fire.lp0.eu>:
>> The BCM63xx contains a soft-reset controller activated by setting
>> a bit (that must previously have cleared).
>>
>> Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
>> ---
>>  MAINTAINERS                   |   1 +
>>  drivers/reset/Kconfig         |   9 +++
>>  drivers/reset/Makefile        |   1 +
>>  drivers/reset/reset-bcm63xx.c | 134 ++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 145 insertions(+)
>>  create mode 100644 drivers/reset/reset-bcm63xx.c
> 
> 
> Could you create a bcm directory and then add your reset-bcm63xx.c
> file there? I have a pending submission for the BCM63138 reset
> controller which is not at all using the same structure and will share
> nothing with your driver.
> 

Ok, I'll call it reset-bcm6345.c to avoid confusion.

> 
>> +static int bcm63xx_reset_xlate(struct reset_controller_dev *rcdev,
>> +       const struct of_phandle_args *reset_spec)
>> +{
>> +       struct bcm63xx_reset_priv *priv = to_bcm63xx_reset_priv(rcdev);
>> +
>> +       if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
>> +               return -EINVAL;
>> +
>> +       if (reset_spec->args[0] >= rcdev->nr_resets)
>> +               return -EINVAL;
> 
> Should not these two things be moved to the core reset controller code
> based on the #reset-cells value?
> 

This has already been removed from the next version of the patch.

> 
>> +       if (of_property_read_u32(np, "offset", &priv->offset))
>> +               return -EINVAL;
>> +
>> +       /* valid reset bits */
>> +       if (of_property_read_u32(np, "mask", &priv->mask))
>> +               priv->mask = 0xffffffff;
>> +
>> +       priv->rcdev.owner = THIS_MODULE;
>> +       priv->rcdev.ops = &bcm63xx_reset_ops;
>> +       priv->rcdev.nr_resets = 32;
> 
> Should not that come from Device Tree, or be computed based on the
> mask property, like hweight_long() or something along these lines?

The "mask" property has been removed. It will assume 32 resets and rely
on the rest of the DT to only refer to valid bits.

-- 
Simon Arlott