From: Basheer Ahmed Muddebihal <[email protected]>
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 <[email protected]>
Signed-off-by: Matthew Gerlach <[email protected]>
---
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 <[email protected]>
@@ -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
+
+#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
On Tue, Sep 06, 2022 at 12:04:23PM -0700, [email protected] wrote:
> From: Basheer Ahmed Muddebihal <[email protected]>
>
> Moving the DFH register offset and register definitions from
> drivers/fpga/dfl.h to include/linux/dfl.h. These definitions
Single 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".
--
With Best Regards,
Andy Shevchenko
On Tue, Sep 06, 2022 at 12:04:23PM -0700, [email protected] 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.
thanks,
greg k-h
On Tue, 6 Sep 2022, Andy Shevchenko wrote:
> On Tue, Sep 06, 2022 at 12:04:23PM -0700, [email protected] wrote:
>> From: Basheer Ahmed Muddebihal <[email protected]>
>>
>> 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 2022-09-06 at 12:04:23 -0700, [email protected] wrote:
> From: Basheer Ahmed Muddebihal <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Matthew Gerlach <[email protected]>
> ---
> 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 <[email protected]>
> @@ -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
> +
> +#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
>
On Wed, 7 Sep 2022, Greg KH wrote:
> On Tue, Sep 06, 2022 at 12:04:23PM -0700, [email protected] 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.
>
> thanks,
>
> greg k-h
>
Hi Greg,
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.
Best Regards,
Matthew Gerlach
On Sun, 11 Sep 2022, Xu Yilun wrote:
> On 2022-09-06 at 12:04:23 -0700, [email protected] wrote:
>> From: Basheer Ahmed Muddebihal <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Matthew Gerlach <[email protected]>
>> ---
>> 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 <[email protected]>
>> @@ -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 <[email protected]> wrote:
> On Wed, 7 Sep 2022, Greg KH wrote:
> > On Tue, Sep 06, 2022 at 12:04:23PM -0700, [email protected] 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 -- [email protected]
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