Hi all,
while trying to get the devicetree overlays working using Alan's
simple-fpga-bus I couldn't find a way to independently reset
parts of the PL logic. I might have missed something and this
exists already somewhere, in that case, oh well ...
If Sören or Michael could take a look at this to let me know
if this is fundamentally wrong, that would be great.
Thanks,
Moritz
Moritz Fischer (3):
docs: dts: Added documentation for Xilinx Zynq PL Reset bindings.
dts: zynq: Add devicetree entry for PL reset controller.
reset: reset-zynq-pl: Adding support for Xilinx Zynq PL reset.
.../devicetree/bindings/reset/zynq-reset-pl.txt | 13 ++
arch/arm/boot/dts/zynq-7000.dtsi | 6 +
drivers/reset/Makefile | 1 +
drivers/reset/reset-zynq-pl.c | 142 +++++++++++++++++++++
4 files changed, 162 insertions(+)
create mode 100644 Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
create mode 100644 drivers/reset/reset-zynq-pl.c
--
2.4.3
Signed-off-by: Moritz Fischer <[email protected]>
---
Documentation/devicetree/bindings/reset/zynq-reset-pl.txt | 13 +++++++++++++
1 file changed, 13 insertions(+)
create mode 100644 Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
diff --git a/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
new file mode 100644
index 0000000..ac4499e
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
@@ -0,0 +1,13 @@
+Xilinx Zynq PL Reset Manager
+
+Required properties:
+- compatible: "xlnx,zynq-reset-pl"
+- syscon <&slcr>;
+- #reset-cells: 1
+
+Example:
+ rstc: rstc@240 {
+ #reset-cells = <1>;
+ compatible = "xlnx,zynq-reset-pl";
+ syscon = <&slcr>;
+ };
--
2.4.3
Signed-off-by: Moritz Fischer <[email protected]>
---
arch/arm/boot/dts/zynq-7000.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index b429e1d..a56fe11 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -258,6 +258,12 @@
reg = <0x100 0x100>;
};
+ rstc: rstc@240 {
+ #reset-cells = <1>;
+ compatible = "xlnx,zynq-reset-pl";
+ syscon = <&slcr>;
+ };
+
pinctrl0: pinctrl@700 {
compatible = "xlnx,pinctrl-zynq";
reg = <0x700 0x200>;
--
2.4.3
The Zynq PL reset controller allows to control the 4
FCLK{0..3}_RESETN signals that can be used to reset custom IP in
the PL.
Signed-off-by: Moritz Fischer <[email protected]>
---
drivers/reset/Makefile | 1 +
drivers/reset/reset-zynq-pl.c | 142 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 143 insertions(+)
create mode 100644 drivers/reset/reset-zynq-pl.c
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 157d421..5c86f92 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -3,3 +3,4 @@ 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/
+obj-$(CONFIG_ARCH_ZYNQ) += reset-zynq-pl.o
diff --git a/drivers/reset/reset-zynq-pl.c b/drivers/reset/reset-zynq-pl.c
new file mode 100644
index 0000000..3e04ab0
--- /dev/null
+++ b/drivers/reset/reset-zynq-pl.c
@@ -0,0 +1,142 @@
+/*
+ * Xilinx Zynq PL Reset Controller
+ *
+ * Copyright (c) 2015, National Instruments Corp.
+ * Author: Moritz Fischer <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+/* Offsets into SLCR regmap */
+#define SLCR_FPGA_RST_CTRL_OFFSET 0x240 /* FPGA Software Reset Control */
+
+struct zynq_pl_reset_data {
+ struct regmap *slcr;
+ struct reset_controller_dev rcdev;
+};
+
+static int zynq_pl_reset_assert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct zynq_pl_reset_data *priv = container_of(rcdev,
+ struct zynq_pl_reset_data,
+ rcdev);
+
+ int offset = id % BITS_PER_LONG;
+
+ regmap_update_bits(priv->slcr,
+ SLCR_FPGA_RST_CTRL_OFFSET,
+ BIT(offset),
+ BIT(offset));
+
+ return 0;
+}
+
+static int zynq_pl_reset_deassert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct zynq_pl_reset_data *priv = container_of(rcdev,
+ struct zynq_pl_reset_data,
+ rcdev);
+
+ int offset = id % BITS_PER_LONG;
+
+ regmap_update_bits(priv->slcr,
+ SLCR_FPGA_RST_CTRL_OFFSET,
+ BIT(offset),
+ ~BIT(offset));
+
+ return 0;
+}
+
+static int zynq_pl_reset_status(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct zynq_pl_reset_data *priv = container_of(rcdev,
+ struct zynq_pl_reset_data,
+ rcdev);
+ int offset = id % BITS_PER_LONG;
+ u32 reg;
+
+ regmap_read(priv->slcr, SLCR_FPGA_RST_CTRL_OFFSET, ®);
+
+ return !(reg & BIT(offset));
+}
+
+static const struct reset_control_ops zynq_pl_reset_ops = {
+ .assert = zynq_pl_reset_assert,
+ .deassert = zynq_pl_reset_deassert,
+ .status = zynq_pl_reset_status,
+};
+
+static int zynq_pl_reset_probe(struct platform_device *pdev)
+{
+ struct zynq_pl_reset_data *priv;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+ platform_set_drvdata(pdev, priv);
+
+ priv->slcr = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+ "syscon");
+ if (IS_ERR(priv->slcr)) {
+ dev_err(&pdev->dev, "unable to get zynq-slcr regmap");
+ return PTR_ERR(priv->slcr);
+ }
+
+ priv->rcdev.owner = THIS_MODULE;
+ priv->rcdev.nr_resets = BITS_PER_LONG;
+ priv->rcdev.ops = &zynq_pl_reset_ops;
+ priv->rcdev.of_node = pdev->dev.of_node;
+ reset_controller_register(&priv->rcdev);
+
+ return 0;
+}
+
+static int zynq_pl_reset_remove(struct platform_device *pdev)
+{
+ struct zynq_pl_reset_data *priv = platform_get_drvdata(pdev);
+
+ reset_controller_unregister(&priv->rcdev);
+
+ return 0;
+}
+
+static const struct of_device_id zynq_pl_reset_dt_ids[] = {
+ { .compatible = "xlnx,zynq-reset-pl", },
+ { /* sentinel */ },
+};
+
+static struct platform_driver zynq_pl_reset_driver = {
+ .probe = zynq_pl_reset_probe,
+ .remove = zynq_pl_reset_remove,
+ .driver = {
+ .name = "zynq-pl-reset",
+ .of_match_table = zynq_pl_reset_dt_ids,
+ },
+};
+module_platform_driver(zynq_pl_reset_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Moritz Fischer <[email protected]>");
+MODULE_DESCRIPTION("Zynq PL Reset Controller Driver");
+MODULE_ALIAS("reset:zynq-pl");
--
2.4.3
On 07/24/2015 12:51 AM, Moritz Fischer wrote:
> Signed-off-by: Moritz Fischer <[email protected]>
> ---
> Documentation/devicetree/bindings/reset/zynq-reset-pl.txt | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
>
> diff --git a/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
> new file mode 100644
> index 0000000..ac4499e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
> @@ -0,0 +1,13 @@
> +Xilinx Zynq PL Reset Manager
I think there is no reason to be just PL specific.
> +
> +Required properties:
> +- compatible: "xlnx,zynq-reset-pl"
> +- syscon <&slcr>;
> +- #reset-cells: 1
> +
> +Example:
> + rstc: rstc@240 {
@200 then here
> + #reset-cells = <1>;
> + compatible = "xlnx,zynq-reset-pl";
Compatible should be first below node name.
You should add also reg property 0x200 0x50
> + syscon = <&slcr>;
> + };
>
Thanks,
Michal
Am Donnerstag, den 23.07.2015, 15:51 -0700 schrieb Moritz Fischer:
> Signed-off-by: Moritz Fischer <[email protected]>
> ---
> Documentation/devicetree/bindings/reset/zynq-reset-pl.txt | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
>
> diff --git a/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
> new file mode 100644
> index 0000000..ac4499e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
> @@ -0,0 +1,13 @@
> +Xilinx Zynq PL Reset Manager
> +
> +Required properties:
> +- compatible: "xlnx,zynq-reset-pl"
> +- syscon <&slcr>;
> +- #reset-cells: 1
> +
> +Example:
> + rstc: rstc@240 {
> + #reset-cells = <1>;
> + compatible = "xlnx,zynq-reset-pl";
> + syscon = <&slcr>;
> + };
Instead of the syscon property, why not specify that the rstc node must
be a child of the slcr node?
regards
Philipp
On 07/24/2015 09:25 AM, Philipp Zabel wrote:
> Am Donnerstag, den 23.07.2015, 15:51 -0700 schrieb Moritz Fischer:
>> Signed-off-by: Moritz Fischer <[email protected]>
>> ---
>> Documentation/devicetree/bindings/reset/zynq-reset-pl.txt | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
>>
>> diff --git a/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
>> new file mode 100644
>> index 0000000..ac4499e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
>> @@ -0,0 +1,13 @@
>> +Xilinx Zynq PL Reset Manager
>> +
>> +Required properties:
>> +- compatible: "xlnx,zynq-reset-pl"
>> +- syscon <&slcr>;
>> +- #reset-cells: 1
>> +
>> +Example:
>> + rstc: rstc@240 {
>> + #reset-cells = <1>;
>> + compatible = "xlnx,zynq-reset-pl";
>> + syscon = <&slcr>;
>> + };
>
> Instead of the syscon property, why not specify that the rstc node must
> be a child of the slcr node?
FYI: It is already a child node of SLCR if you look at location in DTS.
Thanks,
Michal
On Fri, 2015-07-24 at 06:40AM +0200, Michal Simek wrote:
> On 07/24/2015 12:51 AM, Moritz Fischer wrote:
> > Signed-off-by: Moritz Fischer <[email protected]>
> > ---
> > Documentation/devicetree/bindings/reset/zynq-reset-pl.txt | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
> >
> > diff --git a/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
> > new file mode 100644
> > index 0000000..ac4499e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
> > @@ -0,0 +1,13 @@
> > +Xilinx Zynq PL Reset Manager
>
> I think there is no reason to be just PL specific.
That was my first thought too. Why not model all the resets in the SLCR?
Sören
Hi Moritz,
On Thu, 2015-07-23 at 03:51PM -0700, Moritz Fischer wrote:
> The Zynq PL reset controller allows to control the 4
> FCLK{0..3}_RESETN signals that can be used to reset custom IP in
> the PL.
>
> Signed-off-by: Moritz Fischer <[email protected]>
> ---
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-zynq-pl.c | 142 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 143 insertions(+)
> create mode 100644 drivers/reset/reset-zynq-pl.c
>
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 157d421..5c86f92 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -3,3 +3,4 @@ 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/
> +obj-$(CONFIG_ARCH_ZYNQ) += reset-zynq-pl.o
> diff --git a/drivers/reset/reset-zynq-pl.c b/drivers/reset/reset-zynq-pl.c
> new file mode 100644
> index 0000000..3e04ab0
> --- /dev/null
> +++ b/drivers/reset/reset-zynq-pl.c
> @@ -0,0 +1,142 @@
> +/*
> + * Xilinx Zynq PL Reset Controller
> + *
> + * Copyright (c) 2015, National Instruments Corp.
> + * Author: Moritz Fischer <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
Here you allow GPLv2 and later, whereas in the LICENSE macro you only
allow GPLv2. That should be in sync.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +
> +/* Offsets into SLCR regmap */
> +#define SLCR_FPGA_RST_CTRL_OFFSET 0x240 /* FPGA Software Reset Control */
> +
> +struct zynq_pl_reset_data {
> + struct regmap *slcr;
> + struct reset_controller_dev rcdev;
> +};
> +
> +static int zynq_pl_reset_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct zynq_pl_reset_data *priv = container_of(rcdev,
> + struct zynq_pl_reset_data,
> + rcdev);
> +
> + int offset = id % BITS_PER_LONG;
> +
> + regmap_update_bits(priv->slcr,
> + SLCR_FPGA_RST_CTRL_OFFSET,
> + BIT(offset),
> + BIT(offset));
> +
> + return 0;
> +}
> +
> +static int zynq_pl_reset_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct zynq_pl_reset_data *priv = container_of(rcdev,
> + struct zynq_pl_reset_data,
> + rcdev);
> +
> + int offset = id % BITS_PER_LONG;
> +
> + regmap_update_bits(priv->slcr,
> + SLCR_FPGA_RST_CTRL_OFFSET,
> + BIT(offset),
> + ~BIT(offset));
> +
> + return 0;
> +}
> +
> +static int zynq_pl_reset_status(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct zynq_pl_reset_data *priv = container_of(rcdev,
> + struct zynq_pl_reset_data,
> + rcdev);
> + int offset = id % BITS_PER_LONG;
> + u32 reg;
> +
> + regmap_read(priv->slcr, SLCR_FPGA_RST_CTRL_OFFSET, ®);
> +
> + return !(reg & BIT(offset));
> +}
> +
> +static const struct reset_control_ops zynq_pl_reset_ops = {
> + .assert = zynq_pl_reset_assert,
> + .deassert = zynq_pl_reset_deassert,
> + .status = zynq_pl_reset_status,
> +};
> +
> +static int zynq_pl_reset_probe(struct platform_device *pdev)
> +{
> + struct zynq_pl_reset_data *priv;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> + platform_set_drvdata(pdev, priv);
> +
> + priv->slcr = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> + "syscon");
> + if (IS_ERR(priv->slcr)) {
> + dev_err(&pdev->dev, "unable to get zynq-slcr regmap");
> + return PTR_ERR(priv->slcr);
> + }
> +
> + priv->rcdev.owner = THIS_MODULE;
> + priv->rcdev.nr_resets = BITS_PER_LONG;
> + priv->rcdev.ops = &zynq_pl_reset_ops;
> + priv->rcdev.of_node = pdev->dev.of_node;
> + reset_controller_register(&priv->rcdev);
> +
> + return 0;
> +}
> +
> +static int zynq_pl_reset_remove(struct platform_device *pdev)
> +{
> + struct zynq_pl_reset_data *priv = platform_get_drvdata(pdev);
> +
> + reset_controller_unregister(&priv->rcdev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id zynq_pl_reset_dt_ids[] = {
> + { .compatible = "xlnx,zynq-reset-pl", },
> + { /* sentinel */ },
> +};
> +
> +static struct platform_driver zynq_pl_reset_driver = {
> + .probe = zynq_pl_reset_probe,
> + .remove = zynq_pl_reset_remove,
> + .driver = {
> + .name = "zynq-pl-reset",
> + .of_match_table = zynq_pl_reset_dt_ids,
> + },
> +};
> +module_platform_driver(zynq_pl_reset_driver);
> +
> +MODULE_LICENSE("GPL v2");
GPLv2 or GPL?
> +MODULE_AUTHOR("Moritz Fischer <[email protected]>");
> +MODULE_DESCRIPTION("Zynq PL Reset Controller Driver");
> +MODULE_ALIAS("reset:zynq-pl");
There has been some discussion around the alias. I think in most cases
it turned out to not be necessary to have.
I think overall having a reset-manager is a good thing. But as already
mentioned in the other emails, why stop at the PL? I think this should
just become the Zynq reset controller and handle all the resets in the
SLCR.
Thanks,
Sören
Michal, Sören,
On Fri, Jul 24, 2015 at 7:50 AM, Sören Brinkmann
<[email protected]> wrote:
> On Fri, 2015-07-24 at 06:40AM +0200, Michal Simek wrote:
>> On 07/24/2015 12:51 AM, Moritz Fischer wrote:
>> > Signed-off-by: Moritz Fischer <[email protected]>
>> > ---
>> > Documentation/devicetree/bindings/reset/zynq-reset-pl.txt | 13 +++++++++++++
>> > 1 file changed, 13 insertions(+)
>> > create mode 100644 Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
>> > new file mode 100644
>> > index 0000000..ac4499e
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
>> > @@ -0,0 +1,13 @@
>> > +Xilinx Zynq PL Reset Manager
>>
>> I think there is no reason to be just PL specific.
>
> That was my first thought too. Why not model all the resets in the SLCR?
I only needed the ones for the PL for my fpga-mgr work and reading the
TRM had a hard time to decide which ones make sense,
and which ones don't make sense to expose to Linux. I'll look into
reworking it to support all the resets.
> Sören
Moritz