DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CC2F721BCE
MIME-Version: 1.0
In-Reply-To: <20170914095037.GA10693@hao-dev>
References: <20170913204841.2730-1-atull@kernel.org> <20170913204841.2730-12-atull@kernel.org>
 <20170914095037.GA10693@hao-dev>
From: Alan Tull <atull@kernel.org>
Date: Thu, 14 Sep 2017 14:36:30 -0500
Message-ID: <CANk1AXQ0DSfU_FGxsDTotaOuPeJ8iieFa=fCrx7ii8NWJ=R6kw@mail.gmail.com>
Subject: Re: [PATCH v4 11/18] fpga: region: add fpga-region.h header
To: Wu Hao <hao.wu@intel.com>
Cc: Moritz Fischer <mdf@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-fpga@vger.kernel.org" <linux-fpga@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4681
Lines: 146

On Thu, Sep 14, 2017 at 4:50 AM, Wu Hao <hao.wu@intel.com> wrote:

Hi Hao,

> On Thu, Sep 14, 2017 at 04:48:34AM +0800, Alan Tull wrote:
>> * Create fpga-region.h.
>> * Export fpga_region_program_fpga.
>> * Move struct fpga_region and other things to the header.
>>
>> This is a step in separating FPGA region common code
>> from Device Tree support.
>>
>> Signed-off-by: Alan Tull <atull@kernel.org>
>> ---
>> v2: split out from another patch
>>     update author email
>> v3: changes due to fpga_region_program_fpga() removed info param
>> v4: no change to this patch in this version of patchset
>> ---
>>  drivers/fpga/fpga-region.c       | 24 ++++--------------------
>>  include/linux/fpga/fpga-region.h | 28 ++++++++++++++++++++++++++++
>>  2 files changed, 32 insertions(+), 20 deletions(-)
>>  create mode 100644 include/linux/fpga/fpga-region.h
>>
>> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
>> index 2a8621d..402d0b6 100644
>> --- a/drivers/fpga/fpga-region.c
>> +++ b/drivers/fpga/fpga-region.c
>> @@ -18,6 +18,7 @@
>>
>>  #include <linux/fpga/fpga-bridge.h>
>>  #include <linux/fpga/fpga-mgr.h>
>> +#include <linux/fpga/fpga-region.h>
>>  #include <linux/idr.h>
>>  #include <linux/kernel.h>
>>  #include <linux/list.h>
>> @@ -26,24 +27,6 @@
>>  #include <linux/slab.h>
>>  #include <linux/spinlock.h>
>>
>> -/**
>> - * struct fpga_region - FPGA Region structure
>> - * @dev: FPGA Region device
>> - * @mutex: enforces exclusive reference to region
>> - * @bridge_list: list of FPGA bridges specified in region
>> - * @mgr: FPGA manager
>> - * @info: fpga image specific information
>> - */
>> -struct fpga_region {
>> -     struct device dev;
>> -     struct mutex mutex; /* for exclusive reference to region */
>> -     struct list_head bridge_list;
>> -     struct fpga_manager *mgr;
>> -     struct fpga_image_info *info;
>> -};
>> -
>> -#define to_fpga_region(d) container_of(d, struct fpga_region, dev)
>> -
>>  static DEFINE_IDA(fpga_region_ida);
>>  static struct class *fpga_region_class;
>>
>> @@ -226,7 +209,7 @@ static int fpga_region_get_bridges(struct fpga_region *region,
>>   * Program an FPGA using fpga image info (region->info).
>>   * Return 0 for success or negative error code.
>>   */
>> -static int fpga_region_program_fpga(struct fpga_region *region)
>> +int fpga_region_program_fpga(struct fpga_region *region)
>>  {
>>       struct device *dev = &region->dev;
>>       struct fpga_image_info *info = region->info;
>> @@ -282,6 +265,7 @@ static int fpga_region_program_fpga(struct fpga_region *region)
>>
>>       return ret;
>>  }
>> +EXPORT_SYMBOL_GPL(fpga_region_program_fpga);
>>
>>  /**
>>   * child_regions_with_firmware
>> @@ -667,5 +651,5 @@ subsys_initcall(fpga_region_init);
>>  module_exit(fpga_region_exit);
>>
>>  MODULE_DESCRIPTION("FPGA Region");
>> -MODULE_AUTHOR("Alan Tull <atull@opensource.altera.com>");
>> +MODULE_AUTHOR("Alan Tull <atull@kernel.org>");
>>  MODULE_LICENSE("GPL v2");
>> diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h
>> new file mode 100644
>> index 0000000..f84a2e1
>> --- /dev/null
>> +++ b/include/linux/fpga/fpga-region.h
>> @@ -0,0 +1,28 @@
>> +#include <linux/device.h>
>> +#include <linux/fpga/fpga-mgr.h>
>> +#include <linux/fpga/fpga-bridge.h>
>> +
>> +#ifndef _FPGA_REGION_H
>> +#define _FPGA_REGION_H
>
> Hi Alan
>
> I think it may be better to move #ifndef check to the beginning of this
> header files (before include other header files). :)
>
> Thanks
> Hao

That's reasonable.  Will do!

Thanks,
Alan


>
>> +
>> +/**
>> + * struct fpga_region - FPGA Region structure
>> + * @dev: FPGA Region device
>> + * @mutex: enforces exclusive reference to region
>> + * @bridge_list: list of FPGA bridges specified in region
>> + * @mgr: FPGA manager
>> + * @info: FPGA image info
>> + */
>> +struct fpga_region {
>> +     struct device dev;
>> +     struct mutex mutex; /* for exclusive reference to region */
>> +     struct list_head bridge_list;
>> +     struct fpga_manager *mgr;
>> +     struct fpga_image_info *info;
>> +};
>> +
>> +#define to_fpga_region(d) container_of(d, struct fpga_region, dev)
>> +
>> +int fpga_region_program_fpga(struct fpga_region *region);
>> +
>> +#endif /* _FPGA_REGION_H */
>> --
>> 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