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@xxxxxxxxxxxxxxx> wrote:
> From: Matthew Gerlach <matthew.gerlach@xxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
> ---
>  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