Re: [PATCH v1 2/5] fpga: dfl: Move the DFH definitions

matthew.gerlach
2022-09-11T16:13:10Z

On Sun, 11 Sep 2022, Xu Yilun wrote:

> On 2022-09-06 at 12:04:23 -0700, matthew.gerlach@linux.intel.com wrote:
>> From: Basheer Ahmed Muddebihal <basheer.ahmed.muddebihal@linux.intel.com>
>>
>> Moving the DFH register offset and register definitions from
>> drivers/fpga/dfl.h to include/linux/dfl.h. These definitions
>> need to be accessed by dfl drivers that are outside of
>> drivers/fpga.
>>
>> Signed-off-by: Basheer Ahmed Muddebihal <basheer.ahmed.muddebihal@linux.intel.com>
>> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
>> ---
>> drivers/fpga/dfl.h | 22 ++--------------------
>> include/linux/dfl.h | 23 ++++++++++++++++++++++-
>> 2 files changed, 24 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
>> index 06cfcd5e84bb..d4dfc03a0b61 100644
>> --- a/drivers/fpga/dfl.h
>> +++ b/drivers/fpga/dfl.h
>> @@ -2,7 +2,7 @@
>> /*
>> * Driver Header File for FPGA Device Feature List (DFL) Support
>> *
>> - * Copyright (C) 2017-2018 Intel Corporation, Inc.
>> + * Copyright (C) 2017-2022 Intel Corporation, Inc.
>> *
>> * Authors:
>> * Kang Luwei <luwei.kang@intel.com>
>> @@ -17,6 +17,7 @@
>> #include <linux/bitfield.h>
>> #include <linux/cdev.h>
>> #include <linux/delay.h>
>> +#include <linux/dfl.h>
>> #include <linux/eventfd.h>
>> #include <linux/fs.h>
>> #include <linux/interrupt.h>
>> @@ -53,28 +54,9 @@
>> #define PORT_FEATURE_ID_UINT 0x12
>> #define PORT_FEATURE_ID_STP 0x13
>>
>> -/*
>> - * Device Feature Header Register Set
>> - *
>> - * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
>> - * For AFUs, they have DFH + GUID as common header registers.
>> - * For private features, they only have DFH register as common header.
>> - */
>> -#define DFH 0x0
>> -#define GUID_L 0x8
>> -#define GUID_H 0x10
>> -#define NEXT_AFU 0x18
>> -
>> -#define DFH_SIZE 0x8
>> -
>> /* Device Feature Header Register Bitfield */
>> -#define DFH_ID GENMASK_ULL(11, 0) /* Feature ID */
>> #define DFH_ID_FIU_FME 0
>> #define DFH_ID_FIU_PORT 1
>> -#define DFH_REVISION GENMASK_ULL(15, 12) /* Feature revision */
>> -#define DFH_NEXT_HDR_OFST GENMASK_ULL(39, 16) /* Offset to next DFH */
>> -#define DFH_EOL BIT_ULL(40) /* End of list */
>> -#define DFH_TYPE GENMASK_ULL(63, 60) /* Feature type */
>> #define DFH_TYPE_AFU 1
>> #define DFH_TYPE_PRIVATE 3
>> #define DFH_TYPE_FIU 4
>> diff --git a/include/linux/dfl.h b/include/linux/dfl.h
>> index 431636a0dc78..b5accdcfa368 100644
>> --- a/include/linux/dfl.h
>> +++ b/include/linux/dfl.h
>> @@ -2,7 +2,7 @@
>> /*
>> * Header file for DFL driver and device API
>> *
>> - * Copyright (C) 2020 Intel Corporation, Inc.
>> + * Copyright (C) 2020-2022 Intel Corporation, Inc.
>> */
>>
>> #ifndef __LINUX_DFL_H
>> @@ -11,6 +11,27 @@
>> #include <linux/device.h>
>> #include <linux/mod_devicetable.h>
>>
>> +/*
>> + * Device Feature Header Register Set
>> + *
>> + * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
>> + * For AFUs, they have DFH + GUID as common header registers.
>> + * For private features, they only have DFH register as common header.
>> + */
>> +#define DFH 0x0
>> +#define GUID_L 0x8
>> +#define GUID_H 0x10
>> +#define NEXT_AFU 0x18
>
> Now these macros are accessible in global kernel, should we add the
> DFL_ or DFH_ prefix for them?
>
> Thanks,
> Yilun

It does make sense to a DFL_ or DFH_ to these globabl macros, but I'll
look again to see if the ones above really need to be global, where as the
macros below definitely need to be global. I also think a marco like
DFL_DFH might be a little strange.

Thanks,
Matthew Gerlach

8>
>> +
>> +#define DFH_SIZE 0x8
>> +
>> +/* Device Feature Header Register Bitfield */
>> +#define DFH_ID GENMASK_ULL(11, 0) /* Feature ID */
>> +#define DFH_REVISION GENMASK_ULL(15, 12) /* Feature revision */
>> +#define DFH_NEXT_HDR_OFST GENMASK_ULL(39, 16) /* Offset to next DFH */
>> +#define DFH_EOL BIT_ULL(40) /* End of list */
>> +#define DFH_TYPE GENMASK_ULL(63, 60) /* Feature type */
>> +
>> /**
>> * enum dfl_id_type - define the DFL FIU types
>> */
>> --
>> 2.25.1
>>
>