| Message ID | 20220906190426.3139760-3-matthew.gerlach@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enhance definition of DFH and use enhancements for uart driver | expand |
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? > 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".
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. thanks, greg k-h
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 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 > + > +#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, 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. > > 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, 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
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 + +#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 */