[PATCH] fpga fr br: separate freeze bridge driver code

[PATCH] fpga fr br: separate freeze bridge driver code

[matthew.gerlach]

From: Matthew Gerlach <matthew.gerlach@intel.com>

This patch separates the core Freeze Bridge
driver code from the platform driver code.
The intent is to allow the core driver code
to be used without requiring platform driver support.

Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
---
 drivers/fpga/Kconfig                     |  7 ++++
 drivers/fpga/Makefile                    |  1 +
 drivers/fpga/altera-freeze-bridge-plat.c | 65 ++++++++++++++++++++++++++++++++
 drivers/fpga/altera-freeze-bridge.c      | 41 ++++----------------
 drivers/fpga/altera-freeze-bridge.h      | 26 +++++++++++++
 5 files changed, 107 insertions(+), 33 deletions(-)
 create mode 100644 drivers/fpga/altera-freeze-bridge-plat.c
 create mode 100644 drivers/fpga/altera-freeze-bridge.h

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index ce861a2..1a1fc47 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -63,6 +63,13 @@ config ALTERA_FREEZE_BRIDGE
 	  isolate one region of the FPGA from the busses while that
 	  region is being reprogrammed.
 
+config ALTERA_FREEZE_BRIDGE_PLAT
+	tristate "Platform support of Altera FPGA Freeze Bridge"
+	depends on ALTERA_FREEZE_BRIDGE && OF && HAS_IOMEM
+	help
+	  Say Y to enable platform driver support for Altera FPGA
+	  Freeze bridges.
+
 endif # FPGA
 
 endmenu
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 8df07bc..a270c00 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -14,6 +14,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_ALTERA_FREEZE_BRIDGE_PLAT)	+= altera-freeze-bridge-plat.o
 
 # High Level Interfaces
 obj-$(CONFIG_FPGA_REGION)		+= fpga-region.o
diff --git a/drivers/fpga/altera-freeze-bridge-plat.c b/drivers/fpga/altera-freeze-bridge-plat.c
new file mode 100644
index 0000000..44ce539
--- /dev/null
+++ b/drivers/fpga/altera-freeze-bridge-plat.c
@@ -0,0 +1,65 @@
+/*
+ * Platform Driver Support for FPGA Freeze Bridge Controller
+ *
+ *  Copyright (C) 2016 Altera Corporation. All rights reserved.
+ *  Copyright (C) 2016-2017 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include "altera-freeze-bridge.h"
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+
+static int altera_freeze_br_platform_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = pdev->dev.of_node;
+	struct resource *res;
+	void __iomem *reg_base;
+
+	if (!np)
+		return -ENODEV;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	reg_base = devm_ioremap_resource(dev, res);
+
+	if (IS_ERR(reg_base))
+		return PTR_ERR(reg_base);
+
+	return altera_freeze_br_probe(dev, reg_base);
+}
+
+static int altera_freeze_br_platform_remove(struct platform_device *pdev)
+{
+	return altera_freeze_br_remove(&pdev->dev);
+}
+
+static const struct of_device_id altera_freeze_br_of_match[] = {
+	{ .compatible = "altr,freeze-bridge-controller", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, altera_freeze_br_of_match);
+
+static struct platform_driver altera_freeze_br_driver = {
+	.probe = altera_freeze_br_platform_probe,
+	.remove = altera_freeze_br_platform_remove,
+	.driver = {
+		.name	= "altera_freeze_br",
+		.of_match_table = of_match_ptr(altera_freeze_br_of_match),
+	},
+};
+
+module_platform_driver(altera_freeze_br_driver);
diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c
index 8dcd9fb..2942459 100644
--- a/drivers/fpga/altera-freeze-bridge.c
+++ b/drivers/fpga/altera-freeze-bridge.c
@@ -15,12 +15,12 @@
  * You should have received a copy of the GNU General Public License along with
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
+#include "altera-freeze-bridge.h"
 #include <linux/delay.h>
+#include <linux/fpga/fpga-bridge.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 FREEZE_CSR_STATUS_OFFSET		0
 #define FREEZE_CSR_CTRL_OFFSET			4
@@ -208,33 +208,17 @@ static struct fpga_bridge_ops altera_freeze_br_br_ops = {
 	.enable_show = altera_freeze_br_enable_show,
 };
 
-static const struct of_device_id altera_freeze_br_of_match[] = {
-	{ .compatible = "altr,freeze-bridge-controller", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, altera_freeze_br_of_match);
-
-static int altera_freeze_br_probe(struct platform_device *pdev)
+int altera_freeze_br_probe(struct device *dev, void __iomem *reg_base)
 {
-	struct device *dev = &pdev->dev;
-	struct device_node *np = pdev->dev.of_node;
 	struct altera_freeze_br_data *priv;
-	struct resource *res;
 	u32 status, revision;
 
-	if (!np)
-		return -ENODEV;
-
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
 	priv->dev = dev;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	priv->base_addr = devm_ioremap_resource(dev, res);
-	if (IS_ERR(priv->base_addr))
-		return PTR_ERR(priv->base_addr);
+	priv->base_addr = reg_base;
 
 	status = readl(priv->base_addr + FREEZE_CSR_STATUS_OFFSET);
 	if (status & FREEZE_CSR_STATUS_UNFREEZE_REQ_DONE)
@@ -249,24 +233,15 @@ static int altera_freeze_br_probe(struct platform_device *pdev)
 	return fpga_bridge_register(dev, FREEZE_BRIDGE_NAME,
 				    &altera_freeze_br_br_ops, priv);
 }
+EXPORT_SYMBOL_GPL(altera_freeze_br_probe);
 
-static int altera_freeze_br_remove(struct platform_device *pdev)
+int altera_freeze_br_remove(struct device *dev)
 {
-	fpga_bridge_unregister(&pdev->dev);
+	fpga_bridge_unregister(dev);
 
 	return 0;
 }
-
-static struct platform_driver altera_freeze_br_driver = {
-	.probe = altera_freeze_br_probe,
-	.remove = altera_freeze_br_remove,
-	.driver = {
-		.name	= "altera_freeze_br",
-		.of_match_table = of_match_ptr(altera_freeze_br_of_match),
-	},
-};
-
-module_platform_driver(altera_freeze_br_driver);
+EXPORT_SYMBOL_GPL(altera_freeze_br_remove);
 
 MODULE_DESCRIPTION("Altera Freeze Bridge");
 MODULE_AUTHOR("Alan Tull <atull@opensource.altera.com>");
diff --git a/drivers/fpga/altera-freeze-bridge.h b/drivers/fpga/altera-freeze-bridge.h
new file mode 100644
index 0000000..7d36edf
--- /dev/null
+++ b/drivers/fpga/altera-freeze-bridge.h
@@ -0,0 +1,26 @@
+/*
+ * FPGA Freeze Bridge Controller
+ *
+ * Copyright (C) 2016-2017 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _ALT_FRZ_BR_H
+#define _ALT_FRZ_BR_H
+#include <linux/io.h>
+
+int altera_freeze_br_probe(struct device *dev, void __iomem *reg_base);
+int altera_freeze_br_remove(struct device *dev);
+
+#endif /* _ALT_FRZ_BR_H */
-- 
2.7.4

Re: [PATCH] fpga fr br: separate freeze bridge driver code

[Moritz Fischer]

Hi Matthew,

small nit inline.

On Mon, Feb 27, 2017 at 12:03 PM,  <matthew.gerlach@linux.intel.com> wrote:
> From: Matthew Gerlach <matthew.gerlach@intel.com>
>
> This patch separates the core Freeze Bridge
> driver code from the platform driver code.
> The intent is to allow the core driver code
> to be used without requiring platform driver support.
>
> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> ---
>  drivers/fpga/Kconfig                     |  7 ++++
>  drivers/fpga/Makefile                    |  1 +
>  drivers/fpga/altera-freeze-bridge-plat.c | 65 ++++++++++++++++++++++++++++++++
>  drivers/fpga/altera-freeze-bridge.c      | 41 ++++----------------
>  drivers/fpga/altera-freeze-bridge.h      | 26 +++++++++++++
>  5 files changed, 107 insertions(+), 33 deletions(-)
>  create mode 100644 drivers/fpga/altera-freeze-bridge-plat.c
>  create mode 100644 drivers/fpga/altera-freeze-bridge.h
>
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index ce861a2..1a1fc47 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -63,6 +63,13 @@ config ALTERA_FREEZE_BRIDGE
>           isolate one region of the FPGA from the busses while that
>           region is being reprogrammed.
>
> +config ALTERA_FREEZE_BRIDGE_PLAT
> +       tristate "Platform support of Altera FPGA Freeze Bridge"
> +       depends on ALTERA_FREEZE_BRIDGE && OF && HAS_IOMEM
> +       help
> +         Say Y to enable platform driver support for Altera FPGA
> +         Freeze bridges.
> +
>  endif # FPGA
>
>  endmenu
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 8df07bc..a270c00 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -14,6 +14,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_ALTERA_FREEZE_BRIDGE_PLAT)        += altera-freeze-bridge-plat.o
>
>  # High Level Interfaces
>  obj-$(CONFIG_FPGA_REGION)              += fpga-region.o
> diff --git a/drivers/fpga/altera-freeze-bridge-plat.c b/drivers/fpga/altera-freeze-bridge-plat.c
> new file mode 100644
> index 0000000..44ce539
> --- /dev/null
> +++ b/drivers/fpga/altera-freeze-bridge-plat.c
> @@ -0,0 +1,65 @@
> +/*
> + * Platform Driver Support for FPGA Freeze Bridge Controller
> + *
> + *  Copyright (C) 2016 Altera Corporation. All rights reserved.
> + *  Copyright (C) 2016-2017 Intel Corporation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "altera-freeze-bridge.h"
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
> +
> +static int altera_freeze_br_platform_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct device_node *np = pdev->dev.of_node;
> +       struct resource *res;
> +       void __iomem *reg_base;
> +
> +       if (!np)
> +               return -ENODEV;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +       reg_base = devm_ioremap_resource(dev, res);
> +
> +       if (IS_ERR(reg_base))
> +               return PTR_ERR(reg_base);
> +
> +       return altera_freeze_br_probe(dev, reg_base);

Maybe you can call this 'register' instead of 'probe' since the
probing is basically
done at this point. The same comment basically applies to the pr-core I think.

Thanks,

Moritz

Re: [PATCH] fpga fr br: separate freeze bridge driver code

[matthew.gerlach]

On Mon, 27 Feb 2017, Moritz Fischer wrote:

> Hi Matthew,
>
> small nit inline.
>
> On Mon, Feb 27, 2017 at 12:03 PM,  <matthew.gerlach@linux.intel.com> wrote:
>> From: Matthew Gerlach <matthew.gerlach@intel.com>
>>
>> This patch separates the core Freeze Bridge
>> driver code from the platform driver code.
>> The intent is to allow the core driver code
>> to be used without requiring platform driver support.
>>
>> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
>> ---
>>  drivers/fpga/Kconfig                     |  7 ++++
>>  drivers/fpga/Makefile                    |  1 +
>>  drivers/fpga/altera-freeze-bridge-plat.c | 65 ++++++++++++++++++++++++++++++++
>>  drivers/fpga/altera-freeze-bridge.c      | 41 ++++----------------
>>  drivers/fpga/altera-freeze-bridge.h      | 26 +++++++++++++
>>  5 files changed, 107 insertions(+), 33 deletions(-)
>>  create mode 100644 drivers/fpga/altera-freeze-bridge-plat.c
>>  create mode 100644 drivers/fpga/altera-freeze-bridge.h
>>
>> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
>> index ce861a2..1a1fc47 100644
>> --- a/drivers/fpga/Kconfig
>> +++ b/drivers/fpga/Kconfig
>> @@ -63,6 +63,13 @@ config ALTERA_FREEZE_BRIDGE
>>           isolate one region of the FPGA from the busses while that
>>           region is being reprogrammed.
>>
>> +config ALTERA_FREEZE_BRIDGE_PLAT
>> +       tristate "Platform support of Altera FPGA Freeze Bridge"
>> +       depends on ALTERA_FREEZE_BRIDGE && OF && HAS_IOMEM
>> +       help
>> +         Say Y to enable platform driver support for Altera FPGA
>> +         Freeze bridges.
>> +
>>  endif # FPGA
>>
>>  endmenu
>> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
>> index 8df07bc..a270c00 100644
>> --- a/drivers/fpga/Makefile
>> +++ b/drivers/fpga/Makefile
>> @@ -14,6 +14,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_ALTERA_FREEZE_BRIDGE_PLAT)        += altera-freeze-bridge-plat.o
>>
>>  # High Level Interfaces
>>  obj-$(CONFIG_FPGA_REGION)              += fpga-region.o
>> diff --git a/drivers/fpga/altera-freeze-bridge-plat.c b/drivers/fpga/altera-freeze-bridge-plat.c
>> new file mode 100644
>> index 0000000..44ce539
>> --- /dev/null
>> +++ b/drivers/fpga/altera-freeze-bridge-plat.c
>> @@ -0,0 +1,65 @@
>> +/*
>> + * Platform Driver Support for FPGA Freeze Bridge Controller
>> + *
>> + *  Copyright (C) 2016 Altera Corporation. All rights reserved.
>> + *  Copyright (C) 2016-2017 Intel Corporation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope 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.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#include "altera-freeze-bridge.h"
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/of_device.h>
>> +#include <linux/module.h>
>> +
>> +static int altera_freeze_br_platform_probe(struct platform_device *pdev)
>> +{
>> +       struct device *dev = &pdev->dev;
>> +       struct device_node *np = pdev->dev.of_node;
>> +       struct resource *res;
>> +       void __iomem *reg_base;
>> +
>> +       if (!np)
>> +               return -ENODEV;
>> +
>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +
>> +       reg_base = devm_ioremap_resource(dev, res);
>> +
>> +       if (IS_ERR(reg_base))
>> +               return PTR_ERR(reg_base);
>> +
>> +       return altera_freeze_br_probe(dev, reg_base);
>
> Maybe you can call this 'register' instead of 'probe' since the
> probing is basically
> done at this point. The same comment basically applies to the pr-core I 
> think.
>
Hi Moritz,

This is very good suggestion. Having a "probe" function and a 
"platform_probe" function was confusing to some people.  'register' makes 
a lot of sense.

Matthew Gerlach


> Thanks,
>
> 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
>