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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.20.1702280742430.23635@mgerlach-VirtualBox>
Date:   Tue, 28 Feb 2017 07:44:47 -0800 (PST)
From:   matthew.gerlach@...ux.intel.com
To:     Moritz Fischer <moritz.fischer@...us.com>
cc:     Alan Tull <atull@...nsource.altera.com>,
        linux-fpga@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Matthew Gerlach <matthew.gerlach@...el.com>
Subject: Re: [PATCH] fpga fr br: separate freeze bridge driver code



On Mon, 27 Feb 2017, Moritz Fischer wrote:

> Hi Matthew,
>
> small nit inline.
>
> On Mon, Feb 27, 2017 at 12:03 PM,  <matthew.gerlach@...ux.intel.com> wrote:
>> From: Matthew Gerlach <matthew.gerlach@...el.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@...ux.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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>