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

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(-)

On Tue, 6 Sep 2022, Andy Shevchenko wrote:

> On Tue, Sep 06, 2022 at 12:04:23PM -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
>
> Single space?

Sorry for the old habit. I will make it one space.

>
>> need to be accessed by dfl drivers that are outside of
>> drivers/fpga.
>
> ...
>
>> +#define DFH			0x0
>> +#define GUID_L			0x8
>> +#define GUID_H			0x10
>> +#define NEXT_AFU		0x18
>
> While at it, you may make them same width, like "0x08".

The same width does look better.  Thanks for the suggestion.

Matthew Gerlach
>
> -- 
> With Best Regards,
> Andy Shevchenko
>
>
>

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

Hi Matthew,

On Sun, Sep 11, 2022 at 5:40 PM <matthew.gerlach@linux.intel.com> wrote:
> On Wed, 7 Sep 2022, Greg KH wrote:
> > On Tue, Sep 06, 2022 at 12:04:23PM -0700, matthew.gerlach@linux.intel.com wrote:
> >> --- 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.
> >
> > I'm all for updated proper copyright dates, but in a patch that
> > _removes_ text from a file does not seem like the proper place for that,
> > right?  Please discuss with your corporate lawyers as to how to do this
> > properly and when to do it.

> I discussed how and when to do this properly with my corporate lawyers and
> confirmed this submission is consistent with their guidelines.
>
> You do raise an interesting point, though.  If you think this approach is
> improper, we should probably discuss it, including whether this
> restriction is already a condition for contributions or whether it should
> be.  It wouldn't be the first difference of opinion on the finer points of
> copyright law.

So each time code is removed from a file, its copyright year should
be updated? Eventually, we may end up with an empty file which
is copyrighted <this_year>? Do you think that makes sense?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds