diff mbox

[2/2] fpga: Add support for Xilinx LogiCORE PR Decoupler

Message ID 1489181435-3151-2-git-send-email-mdf@kernel.org (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Moritz Fischer March 10, 2017, 9:30 p.m. UTC
This adds support for the Xilinx LogiCORE PR Decoupler
soft-ip that does decoupling of PR regions in the FPGA
fabric during partial reconfiguration.

Signed-off-by: Moritz Fischer <mdf@kernel.org>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
 drivers/fpga/Kconfig               |   9 +++
 drivers/fpga/Makefile              |   1 +
 drivers/fpga/xilinx-pr-decoupler.c | 156 +++++++++++++++++++++++++++++++++++++
 3 files changed, 166 insertions(+)
 create mode 100644 drivers/fpga/xilinx-pr-decoupler.c

Comments

Moritz Fischer March 10, 2017, 10:42 p.m. UTC | #1
On Fri, Mar 10, 2017 at 1:30 PM, Moritz Fischer <mdf@kernel.org> wrote:
> This adds support for the Xilinx LogiCORE PR Decoupler
> soft-ip that does decoupling of PR regions in the FPGA
> fabric during partial reconfiguration.
>
> Signed-off-by: Moritz Fischer <mdf@kernel.org>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> ---
>  drivers/fpga/Kconfig               |   9 +++
>  drivers/fpga/Makefile              |   1 +
>  drivers/fpga/xilinx-pr-decoupler.c | 156 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 166 insertions(+)
>  create mode 100644 drivers/fpga/xilinx-pr-decoupler.c
>
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 967cda4..e42c7dc 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -69,6 +69,15 @@ config ALTERA_FREEZE_BRIDGE
>           isolate one region of the FPGA from the busses while that
>           region is being reprogrammed.
>
> +config XILINX_PR_DECOUPLER
> +       tristate "Xilinx LogiCORE PR Decoupler"
> +       depends on FPGA_BRIDGE
> +       help
> +         Say Y to enable drivers for Xilinx LogiCORE PR Decoupler.
> +         The PR Decoupler exists in the FPGA fabric to isolate one
> +         region of the FPGA from the busses while that region is
> +         being reprogrammed during partial reconfig.
> +
>  endif # FPGA
>
>  endmenu
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index cc0d364..3f04bcf 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)      += zynq-fpga.o
>  obj-$(CONFIG_FPGA_BRIDGE)              += fpga-bridge.o
>  obj-$(CONFIG_SOCFPGA_FPGA_BRIDGE)      += altera-hps2fpga.o altera-fpga2sdram.o
>  obj-$(CONFIG_ALTERA_FREEZE_BRIDGE)     += altera-freeze-bridge.o
> +obj-$(CONFIG_XILINX_PR_DECOUPLER)      += xilinx-pr-decoupler.o
>
>  # High Level Interfaces
>  obj-$(CONFIG_FPGA_REGION)              += fpga-region.o
> diff --git a/drivers/fpga/xilinx-pr-decoupler.c b/drivers/fpga/xilinx-pr-decoupler.c
> new file mode 100644
> index 0000000..07afdd6
> --- /dev/null
> +++ b/drivers/fpga/xilinx-pr-decoupler.c
> @@ -0,0 +1,156 @@
> +/*
> + * Copyright (c) 2017, National Instruments Corp.
> + *
> + * FPGA Bridge Driver for the Xilinx LogiCORE Partial Reconfiguration
> + * Decoupler IP Core.
> + *
> + * 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; version 2 of the License.
> + *
> + * 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/clk.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
> +#include <linux/fpga/fpga-bridge.h>
> +
> +#define CTRL_OFFSET 0x00
> +#define STATUS_OFFSET 0x00
> +
> +#define CTRL_CMD_MASK          BIT(0)
> +#define CTRL_CMD_DECOUPLE      BIT(0)
> +#define CTRL_CMD_COUPLE                ~BIT(0)
> +
> +#define STATUS_STATE_MASK      BIT(0)
> +#define STATUS_STATE_COUPLED   ~BIT(0)
> +#define STATUS_STATE_DECOUPLED BIT(0)
> +
> +struct xlnx_pr_decoupler_data {
> +       void __iomem *io_base;
> +       struct clk *clk;
> +       bool enable;
> +};
> +
> +static inline void xlnx_pr_decoupler_write(struct xlnx_pr_decoupler_data *p,
> +                                          u32 offset, u32 val)
> +{
> +       writel(val, p->io_base + offset);
> +}
> +
> +static inline u32 xlnx_pr_decoupler_read(const struct xlnx_pr_decoupler_data *p,
> +                                        u32 offset)
> +{
> +       return readl(p->io_base + offset);
> +}
> +
> +static int xlnx_pr_decoupler_enable_set(struct fpga_bridge *bridge, bool enable)
> +{
> +       int err;
> +       struct xlnx_pr_decoupler_data *priv = bridge->priv;
> +
> +       if (priv->enable != enable) {
> +               err = clk_enable(priv->clk);
> +               if (err)
> +                       return err;
> +
> +               xlnx_pr_decoupler_write(priv, CTRL_OFFSET,
> +                                       enable ? CTRL_CMD_COUPLE :
> +                                       CTRL_CMD_DECOUPLE);
Needs:

+ priv->enable = enable;

This is obviously garbage, priv->enable doesn't get set ...

> +               clk_disable(priv->clk);
> +       }
> +
> +       return 0;
> +}
> +
> +static int xlnx_pr_decoupler_enable_show(struct fpga_bridge *bridge)
> +{
> +       const struct xlnx_pr_decoupler_data *p = bridge->priv;
> +
> +       return p->enable;
> +}
> +
> +static struct fpga_bridge_ops xlnx_pr_decoupler_br_ops = {
> +       .enable_set = xlnx_pr_decoupler_enable_set,
> +       .enable_show = xlnx_pr_decoupler_enable_show,
> +};
> +
> +static const struct of_device_id xlnx_pr_decoupler_of_match[] = {
> +       { .compatible = "xlnx,pr-decoupler-1.00", },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, xlnx_pr_decoupler_of_match);
> +
> +static int xlnx_pr_decoupler_probe(struct platform_device *pdev)
> +{
> +       struct xlnx_pr_decoupler_data *priv;
> +       u32 status;
> +       int err;
> +       struct resource *res;
> +
> +       priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       priv->io_base = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(priv->io_base))
> +               return PTR_ERR(priv->io_base);
> +
> +       priv->clk = devm_clk_get(&pdev->dev, "ref_clk");
> +       if (IS_ERR(priv->clk)) {
> +               dev_err(&pdev->dev, "input clock not found\n");
> +               return PTR_ERR(priv->clk);
> +       }
> +
> +       err = clk_prepare_enable(priv->clk);
> +       if (err) {
> +               dev_err(&pdev->dev, "unable to enable clock\n");
> +               return err;
> +       }
> +
> +       status = xlnx_pr_decoupler_read(priv, STATUS_OFFSET);
> +       priv->enable = !!(status & STATUS_STATE_MASK);
> +
> +       clk_disable(priv->clk);
> +
> +       err = fpga_bridge_register(&pdev->dev, "Xilinx PR Decoupler",
> +                                  &xlnx_pr_decoupler_br_ops, priv);
> +
> +       if (err) {
> +               dev_err(&pdev->dev, "unable to register Xilinx PR Decoupler");
> +               clk_unprepare(priv->clk);
> +               return err;
> +       }
> +
> +       return 0;
> +}
> +
> +static int xlnx_pr_decoupler_remove(struct platform_device *pdev)
> +{
> +       fpga_bridge_unregister(&pdev->dev);
> +
> +       return 0;
> +}
> +
> +static struct platform_driver xlnx_pr_decoupler_driver = {
> +       .probe = xlnx_pr_decoupler_probe,
> +       .remove = xlnx_pr_decoupler_remove,
> +       .driver = {
> +               .name = "xlnx_pr_decoupler",
> +               .of_match_table = of_match_ptr(xlnx_pr_decoupler_of_match),
> +       },
> +};
> +
> +module_platform_driver(xlnx_pr_decoupler_driver);
> +
> +MODULE_DESCRIPTION("Xilinx Partial Reconfiguration Decoupler");
> +MODULE_AUTHOR("Moritz Fischer <mdf@kernel.org>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Simek March 13, 2017, 10:27 a.m. UTC | #2
Hi Moritz,

On 10.3.2017 23:42, Moritz Fischer wrote:
> On Fri, Mar 10, 2017 at 1:30 PM, Moritz Fischer <mdf@kernel.org> wrote:
>> This adds support for the Xilinx LogiCORE PR Decoupler
>> soft-ip that does decoupling of PR regions in the FPGA
>> fabric during partial reconfiguration.
>>
>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>> Cc: linux-kernel@vger.kernel.org
>> Cc: devicetree@vger.kernel.org
>> ---
>>  drivers/fpga/Kconfig               |   9 +++
>>  drivers/fpga/Makefile              |   1 +
>>  drivers/fpga/xilinx-pr-decoupler.c | 156 +++++++++++++++++++++++++++++++++++++
>>  3 files changed, 166 insertions(+)
>>  create mode 100644 drivers/fpga/xilinx-pr-decoupler.c

I have written very similar driver some week ago but didn't sent it out.

Here it is.
https://github.com/Xilinx/linux-xlnx/blob/master/drivers/fpga/xilinx-pr-decoupler.c

Your clk handling is better and my enable_show is better.
You shouldn't rely on setting status before. It is better to read that
reg again. The reason is you can connect status signal from one PR
decoupler to decouple input which can change status

There is another topic I wanted to open in connection to this. There
should be gpio based bridge because this pr decoupler can be without axi
interface and for that gpio driver would be useful.

Thanks,
Michal


--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Moritz Fischer March 13, 2017, 4:18 p.m. UTC | #3
On Mon, Mar 13, 2017 at 3:27 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> Hi Moritz,
>
> On 10.3.2017 23:42, Moritz Fischer wrote:
>> On Fri, Mar 10, 2017 at 1:30 PM, Moritz Fischer <mdf@kernel.org> wrote:
>>> This adds support for the Xilinx LogiCORE PR Decoupler
>>> soft-ip that does decoupling of PR regions in the FPGA
>>> fabric during partial reconfiguration.
>>>
>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>> Cc: linux-kernel@vger.kernel.org
>>> Cc: devicetree@vger.kernel.org
>>> ---
>>>  drivers/fpga/Kconfig               |   9 +++
>>>  drivers/fpga/Makefile              |   1 +
>>>  drivers/fpga/xilinx-pr-decoupler.c | 156 +++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 166 insertions(+)
>>>  create mode 100644 drivers/fpga/xilinx-pr-decoupler.c
>
> I have written very similar driver some week ago but didn't sent it out.

Hah. I'll take a look.

> Here it is.
> https://github.com/Xilinx/linux-xlnx/blob/master/drivers/fpga/xilinx-pr-decoupler.c
>
> Your clk handling is better and my enable_show is better.
> You shouldn't rely on setting status before. It is better to read that
> reg again. The reason is you can connect status signal from one PR
> decoupler to decouple input which can change status

I will just merge them together and add you to author's list if that's
fine with you?

> There is another topic I wanted to open in connection to this. There
> should be gpio based bridge because this pr decoupler can be without axi
> interface and for that gpio driver would be useful.

That's a good idea. I can look at that. This can be pretty generic
code I suppose.

Cheers,

Moritz
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Simek March 14, 2017, 6:40 a.m. UTC | #4
On 13.3.2017 17:18, Moritz Fischer wrote:
> On Mon, Mar 13, 2017 at 3:27 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>> Hi Moritz,
>>
>> On 10.3.2017 23:42, Moritz Fischer wrote:
>>> On Fri, Mar 10, 2017 at 1:30 PM, Moritz Fischer <mdf@kernel.org> wrote:
>>>> This adds support for the Xilinx LogiCORE PR Decoupler
>>>> soft-ip that does decoupling of PR regions in the FPGA
>>>> fabric during partial reconfiguration.
>>>>
>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Cc: devicetree@vger.kernel.org
>>>> ---
>>>>  drivers/fpga/Kconfig               |   9 +++
>>>>  drivers/fpga/Makefile              |   1 +
>>>>  drivers/fpga/xilinx-pr-decoupler.c | 156 +++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 166 insertions(+)
>>>>  create mode 100644 drivers/fpga/xilinx-pr-decoupler.c
>>
>> I have written very similar driver some week ago but didn't sent it out.
> 
> Hah. I'll take a look.
> 
>> Here it is.
>> https://github.com/Xilinx/linux-xlnx/blob/master/drivers/fpga/xilinx-pr-decoupler.c
>>
>> Your clk handling is better and my enable_show is better.
>> You shouldn't rely on setting status before. It is better to read that
>> reg again. The reason is you can connect status signal from one PR
>> decoupler to decouple input which can change status
> 
> I will just merge them together and add you to author's list if that's
> fine with you?

sure. Go ahead.

> 
>> There is another topic I wanted to open in connection to this. There
>> should be gpio based bridge because this pr decoupler can be without axi
>> interface and for that gpio driver would be useful.
> 
> That's a good idea. I can look at that. This can be pretty generic
> code I suppose.

yes - it should be. Simple gpio driver with polarity support should be
enough.

Thanks,
Michal



--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 967cda4..e42c7dc 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -69,6 +69,15 @@  config ALTERA_FREEZE_BRIDGE
 	  isolate one region of the FPGA from the busses while that
 	  region is being reprogrammed.
 
+config XILINX_PR_DECOUPLER
+	tristate "Xilinx LogiCORE PR Decoupler"
+	depends on FPGA_BRIDGE
+	help
+	  Say Y to enable drivers for Xilinx LogiCORE PR Decoupler.
+	  The PR Decoupler exists in the FPGA fabric to isolate one
+	  region of the FPGA from the busses while that region is
+	  being reprogrammed during partial reconfig.
+
 endif # FPGA
 
 endmenu
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index cc0d364..3f04bcf 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -15,6 +15,7 @@  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
 obj-$(CONFIG_FPGA_BRIDGE)		+= fpga-bridge.o
 obj-$(CONFIG_SOCFPGA_FPGA_BRIDGE)	+= altera-hps2fpga.o altera-fpga2sdram.o
 obj-$(CONFIG_ALTERA_FREEZE_BRIDGE)	+= altera-freeze-bridge.o
+obj-$(CONFIG_XILINX_PR_DECOUPLER)	+= xilinx-pr-decoupler.o
 
 # High Level Interfaces
 obj-$(CONFIG_FPGA_REGION)		+= fpga-region.o
diff --git a/drivers/fpga/xilinx-pr-decoupler.c b/drivers/fpga/xilinx-pr-decoupler.c
new file mode 100644
index 0000000..07afdd6
--- /dev/null
+++ b/drivers/fpga/xilinx-pr-decoupler.c
@@ -0,0 +1,156 @@ 
+/*
+ * Copyright (c) 2017, National Instruments Corp.
+ *
+ * FPGA Bridge Driver for the Xilinx LogiCORE Partial Reconfiguration
+ * Decoupler IP Core.
+ *
+ * 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; version 2 of the License.
+ *
+ * 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/clk.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+#include <linux/fpga/fpga-bridge.h>
+
+#define CTRL_OFFSET 0x00
+#define STATUS_OFFSET 0x00
+
+#define CTRL_CMD_MASK		BIT(0)
+#define CTRL_CMD_DECOUPLE	BIT(0)
+#define CTRL_CMD_COUPLE		~BIT(0)
+
+#define STATUS_STATE_MASK	BIT(0)
+#define STATUS_STATE_COUPLED	~BIT(0)
+#define STATUS_STATE_DECOUPLED	BIT(0)
+
+struct xlnx_pr_decoupler_data {
+	void __iomem *io_base;
+	struct clk *clk;
+	bool enable;
+};
+
+static inline void xlnx_pr_decoupler_write(struct xlnx_pr_decoupler_data *p,
+					   u32 offset, u32 val)
+{
+	writel(val, p->io_base + offset);
+}
+
+static inline u32 xlnx_pr_decoupler_read(const struct xlnx_pr_decoupler_data *p,
+					 u32 offset)
+{
+	return readl(p->io_base + offset);
+}
+
+static int xlnx_pr_decoupler_enable_set(struct fpga_bridge *bridge, bool enable)
+{
+	int err;
+	struct xlnx_pr_decoupler_data *priv = bridge->priv;
+
+	if (priv->enable != enable) {
+		err = clk_enable(priv->clk);
+		if (err)
+			return err;
+
+		xlnx_pr_decoupler_write(priv, CTRL_OFFSET,
+					enable ? CTRL_CMD_COUPLE :
+					CTRL_CMD_DECOUPLE);
+
+		clk_disable(priv->clk);
+	}
+
+	return 0;
+}
+
+static int xlnx_pr_decoupler_enable_show(struct fpga_bridge *bridge)
+{
+	const struct xlnx_pr_decoupler_data *p = bridge->priv;
+
+	return p->enable;
+}
+
+static struct fpga_bridge_ops xlnx_pr_decoupler_br_ops = {
+	.enable_set = xlnx_pr_decoupler_enable_set,
+	.enable_show = xlnx_pr_decoupler_enable_show,
+};
+
+static const struct of_device_id xlnx_pr_decoupler_of_match[] = {
+	{ .compatible = "xlnx,pr-decoupler-1.00", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, xlnx_pr_decoupler_of_match);
+
+static int xlnx_pr_decoupler_probe(struct platform_device *pdev)
+{
+	struct xlnx_pr_decoupler_data *priv;
+	u32 status;
+	int err;
+	struct resource *res;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->io_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->io_base))
+		return PTR_ERR(priv->io_base);
+
+	priv->clk = devm_clk_get(&pdev->dev, "ref_clk");
+	if (IS_ERR(priv->clk)) {
+		dev_err(&pdev->dev, "input clock not found\n");
+		return PTR_ERR(priv->clk);
+	}
+
+	err = clk_prepare_enable(priv->clk);
+	if (err) {
+		dev_err(&pdev->dev, "unable to enable clock\n");
+		return err;
+	}
+
+	status = xlnx_pr_decoupler_read(priv, STATUS_OFFSET);
+	priv->enable = !!(status & STATUS_STATE_MASK);
+
+	clk_disable(priv->clk);
+
+	err = fpga_bridge_register(&pdev->dev, "Xilinx PR Decoupler",
+				   &xlnx_pr_decoupler_br_ops, priv);
+
+	if (err) {
+		dev_err(&pdev->dev, "unable to register Xilinx PR Decoupler");
+		clk_unprepare(priv->clk);
+		return err;
+	}
+
+	return 0;
+}
+
+static int xlnx_pr_decoupler_remove(struct platform_device *pdev)
+{
+	fpga_bridge_unregister(&pdev->dev);
+
+	return 0;
+}
+
+static struct platform_driver xlnx_pr_decoupler_driver = {
+	.probe = xlnx_pr_decoupler_probe,
+	.remove = xlnx_pr_decoupler_remove,
+	.driver = {
+		.name = "xlnx_pr_decoupler",
+		.of_match_table = of_match_ptr(xlnx_pr_decoupler_of_match),
+	},
+};
+
+module_platform_driver(xlnx_pr_decoupler_driver);
+
+MODULE_DESCRIPTION("Xilinx Partial Reconfiguration Decoupler");
+MODULE_AUTHOR("Moritz Fischer <mdf@kernel.org>");
+MODULE_LICENSE("GPL v2");