Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro
Hi Joe, On 12/02/2013 06:48 AM, Joe Perches wrote: (Adding Jonas Bonn to list as he added the macro in the first place...) Thanks... ;) Actually, I think I submitted an even uglier macro called DECLARE_PCI_DEVICE_TABLE... might have been the first kernel patch I ever sent? In any case, it should certainly have been kindly rejected. After it hit mainline Andrew Morton just about choked on his tea and renamed it DEFINE_PCI_DEVICE_TABLE. On Mon, 2013-12-02 at 13:03 +0900, Jingoo Han wrote: On Monday, December 02, 2013 12:56 PM, Guenter Roeck wrote: On 12/01/2013 07:50 PM, Jingoo Han wrote: On Monday, December 02, 2013 12:46 PM, Guenter Roeck wrote: On 12/01/2013 04:07 PM, Jingoo Han wrote: On Friday, November 29, 2013 10:34 AM, Jingoo Han wrote: On Thursday, November 28, 2013 3:24 PM, Joe Perches wrote: On Wed, 2013-11-27 at 21:53 -0800, 'Greg Kroah-Hartman' wrote: On Wed, Nov 27, 2013 at 09:40:13PM -0800, Joe Perches wrote: On Thu, 2013-11-28 at 14:29 +0900, Jingoo Han wrote: On Thursday, November 28, 2013 1:08 PM, Greg Kroah-Hartman wrote: On Thu, Nov 28, 2013 at 10:55:35AM +0900, Jingoo Han wrote: This macro is used to create a struct pci_device_id array. Yeah, and it's a horrid macro that deserves to be removed, please don't use it in more places. Actually, if you could just remove it, that would be best, sorry, I'm not going to take these patches. Feel free to just remove the macro; it serves no purpose but to confuse. That said, the underlying issue that the macro was supposed to resolve (if I recall correctly) was to make sure that all the struct pci_device_id instances were marked as const, as per the PCI documentation; if there's something checkpatch should be warning for it's simply that the struct is const. /Jonas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro
(Adding Jonas Bonn to list as he added the macro in the first place...)
On Mon, 2013-12-02 at 13:03 +0900, Jingoo Han wrote:
> On Monday, December 02, 2013 12:56 PM, Guenter Roeck wrote:
> > On 12/01/2013 07:50 PM, Jingoo Han wrote:
> > > On Monday, December 02, 2013 12:46 PM, Guenter Roeck wrote:
> > >> On 12/01/2013 04:07 PM, Jingoo Han wrote:
> > >>> On Friday, November 29, 2013 10:34 AM, Jingoo Han wrote:
> > On Thursday, November 28, 2013 3:24 PM, Joe Perches wrote:
> > > On Wed, 2013-11-27 at 21:53 -0800, 'Greg Kroah-Hartman' wrote:
> > >> On Wed, Nov 27, 2013 at 09:40:13PM -0800, Joe Perches wrote:
> > >>> On Thu, 2013-11-28 at 14:29 +0900, Jingoo Han wrote:
> > On Thursday, November 28, 2013 1:08 PM, Greg Kroah-Hartman wrote:
> > > On Thu, Nov 28, 2013 at 10:55:35AM +0900, Jingoo Han wrote:
> > >> This macro is used to create a struct pci_device_id array.
> > >
> > > Yeah, and it's a horrid macro that deserves to be removed, please
> > > don't
> > > use it in more places.
> > >
> > > Actually, if you could just remove it, that would be best, sorry,
> > > I'm
> > > not going to take these patches.
> >
> > (+cc Joe Perches, Andrew Morton, Andy Whitcroft)
> >
> > Hi Joe Perches,
> >
> > Would you fix checkpatch.pl about DEFINE_PCI_DEVICE_TABLE?
> > Currently, checkpatch.pl guides to use DEFINE_PCI_DEVICE_TABLE
> > as below.
> >
> > WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id
> > #331: FILE: drivers/usb/host/ehci-pci.c:331:
> > +static const struct pci_device_id pci_ids [] = { {
> >
> > However, Greg Kroah-Hartman mentioned that DEFINE_PCI_DEVICE_TABLE
> > shouldn't be used anymore.
> >
> > So, would you change checkpatch.pl in order to guide to use
> > struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE?
> >
> > For example,
> > WARNING: Use struct pci_device_id instead of
> > DEFINE_PCI_DEVICE_TABLE
> > >>>
> > >>> The documentation doesn't agree with Greg.
> > > []
> > >> I say just remove it, I should have done that years ago when I was
> > >> the
> > >> PCI maintainer, just never got around to it. No other bus has
> > >> something
> > >> like this for their device ids, why should PCI be "special"?
> > >
> > > Anyone else have an opinion?
> > >
> > > I don't care one way or another, but please, one way
> > > not two.
> >
> > >>
> > >> Same here.
> > >>
> > (+cc Bjorn Helgaas, linux-pci)
> >
> > Then, how about the following steps?
> >
> > 1. Fix ./Documentation/PCI/pci.txt as below.
> > (Jingoo Han)
> > The ID table is an array of struct pci_device_id entries ending
> > with an
> > -all-zero entry; use of the macro DEFINE_PCI_DEVICE_TABLE is the
> > preferred
> > -method of declaring the table. Each entry consists of:
> > +all-zero entry; Each entry consists of:
> >
> > 2. Fix ./scripts/checkpatch.pl in order to guide to use
> > struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE.
> > (Joe Perches)
> > >>>
> > >>> If all DEFINE_PCI_DEVICE_TABLEs are replaced with 'const struct
> > >>> pci_device_id'
> > >>> and these patches are merged through 'driver-core.git', it will be not
> > >>> necessary to fix ./scripts/checkpatch.pl.
> > >>>
> > >> Why not ?
> > >
> > > I will replace all DEFINE_PCI_DEVICE_TABLEs with 'const struct
> > > pci_device_id',
> > > and remove the definition of DEFINE_PCI_DEVICE_TABLE macro.
> > >
> > > --- a/include/linux/pci.h
> > > +++ b/include/linux/pci.h
> > > @@ -631,16 +631,6 @@ struct pci_driver {
> > > #defineto_pci_driver(drv) container_of(drv, struct pci_driver,
> > > driver)
> > >
> > > /**
> > > - * DEFINE_PCI_DEVICE_TABLE - macro used to describe a pci device table
> > > - * @_table: device table name
> > > - *
> > > - * This macro is used to create a struct pci_device_id array (a device
> > > table)
> > > - * in a generic manner.
> > > - */
> > > -#define DEFINE_PCI_DEVICE_TABLE(_table) \
> > > - const struct pci_device_id _table[]
> > > -
> > > -/**
> > >
> > > In this case, there is no DEFINE_PCI_DEVICE_TABLE usage
> > > in the kernel. If someone uses DEFINE_PCI_DEVICE_TABLE macro,
> > > it will make build error.
> > >
> >
> > And that will make the checkpatch warning go away ?
> > That seems to be very unlikely.
>
> OK, I will ask Joe Perches to remove the following checkpatch
> warning.
>
> WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id
>
> Best regards,
> Jingoo Han
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel
Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro
On Monday, December 02, 2013 12:56 PM, Guenter Roeck wrote:
> On 12/01/2013 07:50 PM, Jingoo Han wrote:
> > On Monday, December 02, 2013 12:46 PM, Guenter Roeck wrote:
> >> On 12/01/2013 04:07 PM, Jingoo Han wrote:
> >>> On Friday, November 29, 2013 10:34 AM, Jingoo Han wrote:
> On Thursday, November 28, 2013 3:24 PM, Joe Perches wrote:
> > On Wed, 2013-11-27 at 21:53 -0800, 'Greg Kroah-Hartman' wrote:
> >> On Wed, Nov 27, 2013 at 09:40:13PM -0800, Joe Perches wrote:
> >>> On Thu, 2013-11-28 at 14:29 +0900, Jingoo Han wrote:
> On Thursday, November 28, 2013 1:08 PM, Greg Kroah-Hartman wrote:
> > On Thu, Nov 28, 2013 at 10:55:35AM +0900, Jingoo Han wrote:
> >> This macro is used to create a struct pci_device_id array.
> >
> > Yeah, and it's a horrid macro that deserves to be removed, please
> > don't
> > use it in more places.
> >
> > Actually, if you could just remove it, that would be best, sorry,
> > I'm
> > not going to take these patches.
>
> (+cc Joe Perches, Andrew Morton, Andy Whitcroft)
>
> Hi Joe Perches,
>
> Would you fix checkpatch.pl about DEFINE_PCI_DEVICE_TABLE?
> Currently, checkpatch.pl guides to use DEFINE_PCI_DEVICE_TABLE
> as below.
>
> WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id
> #331: FILE: drivers/usb/host/ehci-pci.c:331:
> +static const struct pci_device_id pci_ids [] = { {
>
> However, Greg Kroah-Hartman mentioned that DEFINE_PCI_DEVICE_TABLE
> shouldn't be used anymore.
>
> So, would you change checkpatch.pl in order to guide to use
> struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE?
>
> For example,
> WARNING: Use struct pci_device_id instead of
> DEFINE_PCI_DEVICE_TABLE
> >>>
> >>> The documentation doesn't agree with Greg.
> > []
> >> I say just remove it, I should have done that years ago when I was the
> >> PCI maintainer, just never got around to it. No other bus has
> >> something
> >> like this for their device ids, why should PCI be "special"?
> >
> > Anyone else have an opinion?
> >
> > I don't care one way or another, but please, one way
> > not two.
>
> >>
> >> Same here.
> >>
> (+cc Bjorn Helgaas, linux-pci)
>
> Then, how about the following steps?
>
> 1. Fix ./Documentation/PCI/pci.txt as below.
> (Jingoo Han)
> The ID table is an array of struct pci_device_id entries ending
> with an
> -all-zero entry; use of the macro DEFINE_PCI_DEVICE_TABLE is the
> preferred
> -method of declaring the table. Each entry consists of:
> +all-zero entry; Each entry consists of:
>
> 2. Fix ./scripts/checkpatch.pl in order to guide to use
> struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE.
> (Joe Perches)
> >>>
> >>> If all DEFINE_PCI_DEVICE_TABLEs are replaced with 'const struct
> >>> pci_device_id'
> >>> and these patches are merged through 'driver-core.git', it will be not
> >>> necessary to fix ./scripts/checkpatch.pl.
> >>>
> >> Why not ?
> >
> > I will replace all DEFINE_PCI_DEVICE_TABLEs with 'const struct
> > pci_device_id',
> > and remove the definition of DEFINE_PCI_DEVICE_TABLE macro.
> >
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -631,16 +631,6 @@ struct pci_driver {
> > #defineto_pci_driver(drv) container_of(drv, struct pci_driver,
> > driver)
> >
> > /**
> > - * DEFINE_PCI_DEVICE_TABLE - macro used to describe a pci device table
> > - * @_table: device table name
> > - *
> > - * This macro is used to create a struct pci_device_id array (a device
> > table)
> > - * in a generic manner.
> > - */
> > -#define DEFINE_PCI_DEVICE_TABLE(_table) \
> > - const struct pci_device_id _table[]
> > -
> > -/**
> >
> > In this case, there is no DEFINE_PCI_DEVICE_TABLE usage
> > in the kernel. If someone uses DEFINE_PCI_DEVICE_TABLE macro,
> > it will make build error.
> >
>
> And that will make the checkpatch warning go away ?
> That seems to be very unlikely.
OK, I will ask Joe Perches to remove the following checkpatch
warning.
WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id
Best regards,
Jingoo Han
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro
On 12/01/2013 07:50 PM, Jingoo Han wrote:
On Monday, December 02, 2013 12:46 PM, Guenter Roeck wrote:
On 12/01/2013 04:07 PM, Jingoo Han wrote:
On Friday, November 29, 2013 10:34 AM, Jingoo Han wrote:
On Thursday, November 28, 2013 3:24 PM, Joe Perches wrote:
On Wed, 2013-11-27 at 21:53 -0800, 'Greg Kroah-Hartman' wrote:
On Wed, Nov 27, 2013 at 09:40:13PM -0800, Joe Perches wrote:
On Thu, 2013-11-28 at 14:29 +0900, Jingoo Han wrote:
On Thursday, November 28, 2013 1:08 PM, Greg Kroah-Hartman wrote:
On Thu, Nov 28, 2013 at 10:55:35AM +0900, Jingoo Han wrote:
This macro is used to create a struct pci_device_id array.
Yeah, and it's a horrid macro that deserves to be removed, please don't
use it in more places.
Actually, if you could just remove it, that would be best, sorry, I'm
not going to take these patches.
(+cc Joe Perches, Andrew Morton, Andy Whitcroft)
Hi Joe Perches,
Would you fix checkpatch.pl about DEFINE_PCI_DEVICE_TABLE?
Currently, checkpatch.pl guides to use DEFINE_PCI_DEVICE_TABLE
as below.
WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id
#331: FILE: drivers/usb/host/ehci-pci.c:331:
+static const struct pci_device_id pci_ids [] = { {
However, Greg Kroah-Hartman mentioned that DEFINE_PCI_DEVICE_TABLE
shouldn't be used anymore.
So, would you change checkpatch.pl in order to guide to use
struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE?
For example,
WARNING: Use struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE
The documentation doesn't agree with Greg.
[]
I say just remove it, I should have done that years ago when I was the
PCI maintainer, just never got around to it. No other bus has something
like this for their device ids, why should PCI be "special"?
Anyone else have an opinion?
I don't care one way or another, but please, one way
not two.
Same here.
(+cc Bjorn Helgaas, linux-pci)
Then, how about the following steps?
1. Fix ./Documentation/PCI/pci.txt as below.
(Jingoo Han)
The ID table is an array of struct pci_device_id entries ending with an
-all-zero entry; use of the macro DEFINE_PCI_DEVICE_TABLE is the preferred
-method of declaring the table. Each entry consists of:
+all-zero entry; Each entry consists of:
2. Fix ./scripts/checkpatch.pl in order to guide to use
struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE.
(Joe Perches)
If all DEFINE_PCI_DEVICE_TABLEs are replaced with 'const struct pci_device_id'
and these patches are merged through 'driver-core.git', it will be not
necessary to fix ./scripts/checkpatch.pl.
Why not ?
I will replace all DEFINE_PCI_DEVICE_TABLEs with 'const struct pci_device_id',
and remove the definition of DEFINE_PCI_DEVICE_TABLE macro.
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -631,16 +631,6 @@ struct pci_driver {
#defineto_pci_driver(drv) container_of(drv, struct pci_driver, driver)
/**
- * DEFINE_PCI_DEVICE_TABLE - macro used to describe a pci device table
- * @_table: device table name
- *
- * This macro is used to create a struct pci_device_id array (a device table)
- * in a generic manner.
- */
-#define DEFINE_PCI_DEVICE_TABLE(_table) \
- const struct pci_device_id _table[]
-
-/**
In this case, there is no DEFINE_PCI_DEVICE_TABLE usage
in the kernel. If someone uses DEFINE_PCI_DEVICE_TABLE macro,
it will make build error.
And that will make the checkpatch warning go away ?
That seems to be very unlikely.
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro
On Monday, December 02, 2013 12:46 PM, Guenter Roeck wrote:
> On 12/01/2013 04:07 PM, Jingoo Han wrote:
> > On Friday, November 29, 2013 10:34 AM, Jingoo Han wrote:
> >> On Thursday, November 28, 2013 3:24 PM, Joe Perches wrote:
> >>> On Wed, 2013-11-27 at 21:53 -0800, 'Greg Kroah-Hartman' wrote:
> On Wed, Nov 27, 2013 at 09:40:13PM -0800, Joe Perches wrote:
> > On Thu, 2013-11-28 at 14:29 +0900, Jingoo Han wrote:
> >> On Thursday, November 28, 2013 1:08 PM, Greg Kroah-Hartman wrote:
> >>> On Thu, Nov 28, 2013 at 10:55:35AM +0900, Jingoo Han wrote:
> This macro is used to create a struct pci_device_id array.
> >>>
> >>> Yeah, and it's a horrid macro that deserves to be removed, please
> >>> don't
> >>> use it in more places.
> >>>
> >>> Actually, if you could just remove it, that would be best, sorry, I'm
> >>> not going to take these patches.
> >>
> >> (+cc Joe Perches, Andrew Morton, Andy Whitcroft)
> >>
> >> Hi Joe Perches,
> >>
> >> Would you fix checkpatch.pl about DEFINE_PCI_DEVICE_TABLE?
> >> Currently, checkpatch.pl guides to use DEFINE_PCI_DEVICE_TABLE
> >> as below.
> >>
> >>WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id
> >>#331: FILE: drivers/usb/host/ehci-pci.c:331:
> >>+static const struct pci_device_id pci_ids [] = { {
> >>
> >> However, Greg Kroah-Hartman mentioned that DEFINE_PCI_DEVICE_TABLE
> >> shouldn't be used anymore.
> >>
> >> So, would you change checkpatch.pl in order to guide to use
> >> struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE?
> >>
> >> For example,
> >>WARNING: Use struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE
> >
> > The documentation doesn't agree with Greg.
> >>> []
> I say just remove it, I should have done that years ago when I was the
> PCI maintainer, just never got around to it. No other bus has something
> like this for their device ids, why should PCI be "special"?
> >>>
> >>> Anyone else have an opinion?
> >>>
> >>> I don't care one way or another, but please, one way
> >>> not two.
> >>
>
> Same here.
>
> >> (+cc Bjorn Helgaas, linux-pci)
> >>
> >> Then, how about the following steps?
> >>
> >> 1. Fix ./Documentation/PCI/pci.txt as below.
> >> (Jingoo Han)
> >> The ID table is an array of struct pci_device_id entries ending with an
> >> -all-zero entry; use of the macro DEFINE_PCI_DEVICE_TABLE is the
> >> preferred
> >> -method of declaring the table. Each entry consists of:
> >> +all-zero entry; Each entry consists of:
> >>
> >> 2. Fix ./scripts/checkpatch.pl in order to guide to use
> >> struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE.
> >> (Joe Perches)
> >
> > If all DEFINE_PCI_DEVICE_TABLEs are replaced with 'const struct
> > pci_device_id'
> > and these patches are merged through 'driver-core.git', it will be not
> > necessary to fix ./scripts/checkpatch.pl.
> >
> Why not ?
I will replace all DEFINE_PCI_DEVICE_TABLEs with 'const struct pci_device_id',
and remove the definition of DEFINE_PCI_DEVICE_TABLE macro.
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -631,16 +631,6 @@ struct pci_driver {
#defineto_pci_driver(drv) container_of(drv, struct pci_driver, driver)
/**
- * DEFINE_PCI_DEVICE_TABLE - macro used to describe a pci device table
- * @_table: device table name
- *
- * This macro is used to create a struct pci_device_id array (a device table)
- * in a generic manner.
- */
-#define DEFINE_PCI_DEVICE_TABLE(_table) \
- const struct pci_device_id _table[]
-
-/**
In this case, there is no DEFINE_PCI_DEVICE_TABLE usage
in the kernel. If someone uses DEFINE_PCI_DEVICE_TABLE macro,
it will make build error.
Best regards,
Jingoo Han
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro
On 12/01/2013 04:07 PM, Jingoo Han wrote:
On Friday, November 29, 2013 10:34 AM, Jingoo Han wrote:
On Thursday, November 28, 2013 3:24 PM, Joe Perches wrote:
On Wed, 2013-11-27 at 21:53 -0800, 'Greg Kroah-Hartman' wrote:
On Wed, Nov 27, 2013 at 09:40:13PM -0800, Joe Perches wrote:
On Thu, 2013-11-28 at 14:29 +0900, Jingoo Han wrote:
On Thursday, November 28, 2013 1:08 PM, Greg Kroah-Hartman wrote:
On Thu, Nov 28, 2013 at 10:55:35AM +0900, Jingoo Han wrote:
This macro is used to create a struct pci_device_id array.
Yeah, and it's a horrid macro that deserves to be removed, please don't
use it in more places.
Actually, if you could just remove it, that would be best, sorry, I'm
not going to take these patches.
(+cc Joe Perches, Andrew Morton, Andy Whitcroft)
Hi Joe Perches,
Would you fix checkpatch.pl about DEFINE_PCI_DEVICE_TABLE?
Currently, checkpatch.pl guides to use DEFINE_PCI_DEVICE_TABLE
as below.
WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id
#331: FILE: drivers/usb/host/ehci-pci.c:331:
+static const struct pci_device_id pci_ids [] = { {
However, Greg Kroah-Hartman mentioned that DEFINE_PCI_DEVICE_TABLE
shouldn't be used anymore.
So, would you change checkpatch.pl in order to guide to use
struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE?
For example,
WARNING: Use struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE
The documentation doesn't agree with Greg.
[]
I say just remove it, I should have done that years ago when I was the
PCI maintainer, just never got around to it. No other bus has something
like this for their device ids, why should PCI be "special"?
Anyone else have an opinion?
I don't care one way or another, but please, one way
not two.
Same here.
(+cc Bjorn Helgaas, linux-pci)
Then, how about the following steps?
1. Fix ./Documentation/PCI/pci.txt as below.
(Jingoo Han)
The ID table is an array of struct pci_device_id entries ending with an
-all-zero entry; use of the macro DEFINE_PCI_DEVICE_TABLE is the preferred
-method of declaring the table. Each entry consists of:
+all-zero entry; Each entry consists of:
2. Fix ./scripts/checkpatch.pl in order to guide to use
struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE.
(Joe Perches)
If all DEFINE_PCI_DEVICE_TABLEs are replaced with 'const struct pci_device_id'
and these patches are merged through 'driver-core.git', it will be not
necessary to fix ./scripts/checkpatch.pl.
Why not ?
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro
On Friday, November 29, 2013 10:34 AM, Jingoo Han wrote:
> On Thursday, November 28, 2013 3:24 PM, Joe Perches wrote:
> >On Wed, 2013-11-27 at 21:53 -0800, 'Greg Kroah-Hartman' wrote:
> >>On Wed, Nov 27, 2013 at 09:40:13PM -0800, Joe Perches wrote:
> >>>On Thu, 2013-11-28 at 14:29 +0900, Jingoo Han wrote:
> On Thursday, November 28, 2013 1:08 PM, Greg Kroah-Hartman wrote:
> >On Thu, Nov 28, 2013 at 10:55:35AM +0900, Jingoo Han wrote:
> >> This macro is used to create a struct pci_device_id array.
> >
> > Yeah, and it's a horrid macro that deserves to be removed, please don't
> > use it in more places.
> >
> > Actually, if you could just remove it, that would be best, sorry, I'm
> > not going to take these patches.
>
> (+cc Joe Perches, Andrew Morton, Andy Whitcroft)
>
> Hi Joe Perches,
>
> Would you fix checkpatch.pl about DEFINE_PCI_DEVICE_TABLE?
> Currently, checkpatch.pl guides to use DEFINE_PCI_DEVICE_TABLE
> as below.
>
> WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id
> #331: FILE: drivers/usb/host/ehci-pci.c:331:
> +static const struct pci_device_id pci_ids [] = { {
>
> However, Greg Kroah-Hartman mentioned that DEFINE_PCI_DEVICE_TABLE
> shouldn't be used anymore.
>
> So, would you change checkpatch.pl in order to guide to use
> struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE?
>
> For example,
> WARNING: Use struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE
> >>>
> >>> The documentation doesn't agree with Greg.
> >[]
> >> I say just remove it, I should have done that years ago when I was the
> >> PCI maintainer, just never got around to it. No other bus has something
> >> like this for their device ids, why should PCI be "special"?
> >
> >Anyone else have an opinion?
> >
> >I don't care one way or another, but please, one way
> >not two.
>
> (+cc Bjorn Helgaas, linux-pci)
>
> Then, how about the following steps?
>
> 1. Fix ./Documentation/PCI/pci.txt as below.
>(Jingoo Han)
>The ID table is an array of struct pci_device_id entries ending with an
>-all-zero entry; use of the macro DEFINE_PCI_DEVICE_TABLE is the preferred
>-method of declaring the table. Each entry consists of:
>+all-zero entry; Each entry consists of:
>
> 2. Fix ./scripts/checkpatch.pl in order to guide to use
> struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE.
> (Joe Perches)
If all DEFINE_PCI_DEVICE_TABLEs are replaced with 'const struct pci_device_id'
and these patches are merged through 'driver-core.git', it will be not
necessary to fix ./scripts/checkpatch.pl.
Then, I will send patches to Greg Kroah-Hartman, with CC'ing each subsystem
maintainer.
Best regards,
Jingoo Han
>
> 3. Replace DEFINE_PCI_DEVICE_TABLE of ./drivers/*
> with 'const struct pci_device_id'.
>(Jingoo Han)
>
> 4. These patches will be merged through 'driver-core.git'
> with 'Acked-by' of each subsystem maintainer.
>(Greg Kroah-Hartman)
>
> Best regards,
> Jingoo Han
>
> >Changing checkpatch is a trifle, but there are a _lot_
> >of maintainers to work through if it's to be removed.
> >
> >It'll probably take several releases.
> >
> >$ git grep --name-only -w DEFINE_PCI_DEVICE_TABLE | \
> > cut -f1,2 -d/ | uniq -c
> > 1 Documentation/PCI
> > 1 arch/x86
> > 1 drivers/bcma
> > 3 drivers/block
> > 1 drivers/char
> > 1 drivers/cpufreq
> > 2 drivers/dma
> > 18 drivers/edac
> > 6 drivers/gpio
> > 6 drivers/gpu
> > 6 drivers/hwmon
> > 20 drivers/i2c
> > 2 drivers/infiniband
> > 1 drivers/ipack
> > 1 drivers/leds
> > 3 drivers/media
> > 10 drivers/mfd
> > 2 drivers/misc
> > 1 drivers/mmc
> > 1 drivers/mtd
> >132 drivers/net
> > 1 drivers/ntb
> > 1 drivers/pci
> > 5 drivers/pcmcia
> > 2 drivers/platform
> > 1 drivers/ptp
> > 1 drivers/rapidio
> > 7 drivers/scsi
> > 3 drivers/spi
> > 65 drivers/staging
> > 3 drivers/tty
> > 1 drivers/uio
> > 5 drivers/usb
> > 1 drivers/video
> > 1 drivers/virtio
> > 3 drivers/vme
> > 9 drivers/watchdog
> > 1 drivers/xen
> > 1 include/linux
> > 1 scripts/checkpatch.pl
> > 1 scripts/tags.sh
> > 1 sound/oss
> > 67 sound/pci
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro
On Thursday, November 28, 2013 3:24 PM, Joe Perches wrote:
>On Wed, 2013-11-27 at 21:53 -0800, 'Greg Kroah-Hartman' wrote:
>>On Wed, Nov 27, 2013 at 09:40:13PM -0800, Joe Perches wrote:
>>>On Thu, 2013-11-28 at 14:29 +0900, Jingoo Han wrote:
On Thursday, November 28, 2013 1:08 PM, Greg Kroah-Hartman wrote:
>On Thu, Nov 28, 2013 at 10:55:35AM +0900, Jingoo Han wrote:
>> This macro is used to create a struct pci_device_id array.
>
> Yeah, and it's a horrid macro that deserves to be removed, please don't
> use it in more places.
>
> Actually, if you could just remove it, that would be best, sorry, I'm
> not going to take these patches.
(+cc Joe Perches, Andrew Morton, Andy Whitcroft)
Hi Joe Perches,
Would you fix checkpatch.pl about DEFINE_PCI_DEVICE_TABLE?
Currently, checkpatch.pl guides to use DEFINE_PCI_DEVICE_TABLE
as below.
WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id
#331: FILE: drivers/usb/host/ehci-pci.c:331:
+static const struct pci_device_id pci_ids [] = { {
However, Greg Kroah-Hartman mentioned that DEFINE_PCI_DEVICE_TABLE
shouldn't be used anymore.
So, would you change checkpatch.pl in order to guide to use
struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE?
For example,
WARNING: Use struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE
>>>
>>> The documentation doesn't agree with Greg.
>[]
>> I say just remove it, I should have done that years ago when I was the
>> PCI maintainer, just never got around to it. No other bus has something
>> like this for their device ids, why should PCI be "special"?
>
>Anyone else have an opinion?
>
>I don't care one way or another, but please, one way
>not two.
(+cc Bjorn Helgaas, linux-pci)
Then, how about the following steps?
1. Fix ./Documentation/PCI/pci.txt as below.
(Jingoo Han)
The ID table is an array of struct pci_device_id entries ending with an
-all-zero entry; use of the macro DEFINE_PCI_DEVICE_TABLE is the preferred
-method of declaring the table. Each entry consists of:
+all-zero entry; Each entry consists of:
2. Fix ./scripts/checkpatch.pl in order to guide to use
struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE.
(Joe Perches)
3. Replace DEFINE_PCI_DEVICE_TABLE of ./drivers/*
with 'const struct pci_device_id'.
(Jingoo Han)
4. These patches will be merged through 'driver-core.git'
with 'Acked-by' of each subsystem maintainer.
(Greg Kroah-Hartman)
Best regards,
Jingoo Han
>Changing checkpatch is a trifle, but there are a _lot_
>of maintainers to work through if it's to be removed.
>
>It'll probably take several releases.
>
>$ git grep --name-only -w DEFINE_PCI_DEVICE_TABLE | \
> cut -f1,2 -d/ | uniq -c
> 1 Documentation/PCI
> 1 arch/x86
> 1 drivers/bcma
> 3 drivers/block
> 1 drivers/char
> 1 drivers/cpufreq
> 2 drivers/dma
> 18 drivers/edac
> 6 drivers/gpio
> 6 drivers/gpu
> 6 drivers/hwmon
> 20 drivers/i2c
> 2 drivers/infiniband
> 1 drivers/ipack
> 1 drivers/leds
> 3 drivers/media
> 10 drivers/mfd
> 2 drivers/misc
> 1 drivers/mmc
> 1 drivers/mtd
>132 drivers/net
> 1 drivers/ntb
> 1 drivers/pci
> 5 drivers/pcmcia
> 2 drivers/platform
> 1 drivers/ptp
> 1 drivers/rapidio
> 7 drivers/scsi
> 3 drivers/spi
> 65 drivers/staging
> 3 drivers/tty
> 1 drivers/uio
> 5 drivers/usb
> 1 drivers/video
> 1 drivers/virtio
> 3 drivers/vme
> 9 drivers/watchdog
> 1 drivers/xen
> 1 include/linux
> 1 scripts/checkpatch.pl
> 1 scripts/tags.sh
> 1 sound/oss
> 67 sound/pci
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro
On Wed, 2013-11-27 at 21:53 -0800, 'Greg Kroah-Hartman' wrote:
> On Wed, Nov 27, 2013 at 09:40:13PM -0800, Joe Perches wrote:
> > On Thu, 2013-11-28 at 14:29 +0900, Jingoo Han wrote:
> > > On Thursday, November 28, 2013 1:08 PM, Greg Kroah-Hartman wrote:
> > > > On Thu, Nov 28, 2013 at 10:55:35AM +0900, Jingoo Han wrote:
> > > > > This macro is used to create a struct pci_device_id array.
> > > >
> > > > Yeah, and it's a horrid macro that deserves to be removed, please don't
> > > > use it in more places.
> > > >
> > > > Actually, if you could just remove it, that would be best, sorry, I'm
> > > > not going to take these patches.
> > >
> > > (+cc Joe Perches, Andrew Morton, Andy Whitcroft)
> > >
> > > Hi Joe Perches,
> > >
> > > Would you fix checkpatch.pl about DEFINE_PCI_DEVICE_TABLE?
> > > Currently, checkpatch.pl guides to use DEFINE_PCI_DEVICE_TABLE
> > > as below.
> > >
> > > WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id
> > > #331: FILE: drivers/usb/host/ehci-pci.c:331:
> > > +static const struct pci_device_id pci_ids [] = { {
> > >
> > > However, Greg Kroah-Hartman mentioned that DEFINE_PCI_DEVICE_TABLE
> > > shouldn't be used anymore.
> > >
> > > So, would you change checkpatch.pl in order to guide to use
> > > struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE?
> > >
> > > For example,
> > > WARNING: Use struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE
> >
> > The documentation doesn't agree with Greg.
[]
> I say just remove it, I should have done that years ago when I was the
> PCI maintainer, just never got around to it. No other bus has something
> like this for their device ids, why should PCI be "special"?
Anyone else have an opinion?
I don't care one way or another, but please, one way
not two.
Changing checkpatch is a trifle, but there are a _lot_
of maintainers to work through if it's to be removed.
It'll probably take several releases.
$ git grep --name-only -w DEFINE_PCI_DEVICE_TABLE | \
cut -f1,2 -d/ | uniq -c
1 Documentation/PCI
1 arch/x86
1 drivers/bcma
3 drivers/block
1 drivers/char
1 drivers/cpufreq
2 drivers/dma
18 drivers/edac
6 drivers/gpio
6 drivers/gpu
6 drivers/hwmon
20 drivers/i2c
2 drivers/infiniband
1 drivers/ipack
1 drivers/leds
3 drivers/media
10 drivers/mfd
2 drivers/misc
1 drivers/mmc
1 drivers/mtd
132 drivers/net
1 drivers/ntb
1 drivers/pci
5 drivers/pcmcia
2 drivers/platform
1 drivers/ptp
1 drivers/rapidio
7 drivers/scsi
3 drivers/spi
65 drivers/staging
3 drivers/tty
1 drivers/uio
5 drivers/usb
1 drivers/video
1 drivers/virtio
3 drivers/vme
9 drivers/watchdog
1 drivers/xen
1 include/linux
1 scripts/checkpatch.pl
1 scripts/tags.sh
1 sound/oss
67 sound/pci
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro
On Wed, Nov 27, 2013 at 09:40:13PM -0800, Joe Perches wrote:
> On Thu, 2013-11-28 at 14:29 +0900, Jingoo Han wrote:
> > On Thursday, November 28, 2013 1:08 PM, Greg Kroah-Hartman wrote:
> > > On Thu, Nov 28, 2013 at 10:55:35AM +0900, Jingoo Han wrote:
> > > > This macro is used to create a struct pci_device_id array.
> > >
> > > Yeah, and it's a horrid macro that deserves to be removed, please don't
> > > use it in more places.
> > >
> > > Actually, if you could just remove it, that would be best, sorry, I'm
> > > not going to take these patches.
> >
> > (+cc Joe Perches, Andrew Morton, Andy Whitcroft)
> >
> > Hi Joe Perches,
> >
> > Would you fix checkpatch.pl about DEFINE_PCI_DEVICE_TABLE?
> > Currently, checkpatch.pl guides to use DEFINE_PCI_DEVICE_TABLE
> > as below.
> >
> > WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id
> > #331: FILE: drivers/usb/host/ehci-pci.c:331:
> > +static const struct pci_device_id pci_ids [] = { {
> >
> > However, Greg Kroah-Hartman mentioned that DEFINE_PCI_DEVICE_TABLE
> > shouldn't be used anymore.
> >
> > So, would you change checkpatch.pl in order to guide to use
> > struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE?
> >
> > For example,
> > WARNING: Use struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE
>
> The documentation doesn't agree with Greg.
>
> Documentation/PCI/pci.txt:
>
> The ID table is an array of struct pci_device_id entries ending with an
> all-zero entry; use of the macro DEFINE_PCI_DEVICE_TABLE is the preferred
> method of declaring the table.
Then it should be fixed.
> Neither does the kernel tree:
>
> $ git grep -w DEFINE_PCI_DEVICE_TABLE | wc -l
> 410
>
> $ git grep -E "\bstruct\s+pci_device_id\s+\w+\s*\[\s*\]\s*=" | wc -l
> 376
>
> Most of the 376 should be const and are not.
>
> $ git grep -E "\bconst\s+struct\s+pci_device_id\s+\w+\s*\[\s*\]\s*=" | wc -l
> 155
Then fix those and make them const.
Hiding structures behind an odd (and misnamed) macro isn't the best
thing.
> Everything that uses DEFINE_PCI_DEVICE_TABLE is const.
>
> $ git grep -A1 -E "define\s+DEFINE_PCI_DEVICE_TABLE"
> include/linux/pci.h:#define DEFINE_PCI_DEVICE_TABLE(_table) \
> include/linux/pci.h-const struct pci_device_id _table[]
I say just remove it, I should have done that years ago when I was the
PCI maintainer, just never got around to it. No other bus has something
like this for their device ids, why should PCI be "special"?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro
On Thu, 2013-11-28 at 14:29 +0900, Jingoo Han wrote:
> On Thursday, November 28, 2013 1:08 PM, Greg Kroah-Hartman wrote:
> > On Thu, Nov 28, 2013 at 10:55:35AM +0900, Jingoo Han wrote:
> > > This macro is used to create a struct pci_device_id array.
> >
> > Yeah, and it's a horrid macro that deserves to be removed, please don't
> > use it in more places.
> >
> > Actually, if you could just remove it, that would be best, sorry, I'm
> > not going to take these patches.
>
> (+cc Joe Perches, Andrew Morton, Andy Whitcroft)
>
> Hi Joe Perches,
>
> Would you fix checkpatch.pl about DEFINE_PCI_DEVICE_TABLE?
> Currently, checkpatch.pl guides to use DEFINE_PCI_DEVICE_TABLE
> as below.
>
> WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id
> #331: FILE: drivers/usb/host/ehci-pci.c:331:
> +static const struct pci_device_id pci_ids [] = { {
>
> However, Greg Kroah-Hartman mentioned that DEFINE_PCI_DEVICE_TABLE
> shouldn't be used anymore.
>
> So, would you change checkpatch.pl in order to guide to use
> struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE?
>
> For example,
> WARNING: Use struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE
The documentation doesn't agree with Greg.
Documentation/PCI/pci.txt:
The ID table is an array of struct pci_device_id entries ending with an
all-zero entry; use of the macro DEFINE_PCI_DEVICE_TABLE is the preferred
method of declaring the table.
Neither does the kernel tree:
$ git grep -w DEFINE_PCI_DEVICE_TABLE | wc -l
410
$ git grep -E "\bstruct\s+pci_device_id\s+\w+\s*\[\s*\]\s*=" | wc -l
376
Most of the 376 should be const and are not.
$ git grep -E "\bconst\s+struct\s+pci_device_id\s+\w+\s*\[\s*\]\s*=" | wc -l
155
Everything that uses DEFINE_PCI_DEVICE_TABLE is const.
$ git grep -A1 -E "define\s+DEFINE_PCI_DEVICE_TABLE"
include/linux/pci.h:#define DEFINE_PCI_DEVICE_TABLE(_table) \
include/linux/pci.h-const struct pci_device_id _table[]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro
On Thursday, November 28, 2013 1:08 PM, Greg Kroah-Hartman wrote:
> On Thu, Nov 28, 2013 at 10:55:35AM +0900, Jingoo Han wrote:
> > This macro is used to create a struct pci_device_id array.
>
> Yeah, and it's a horrid macro that deserves to be removed, please don't
> use it in more places.
>
> Actually, if you could just remove it, that would be best, sorry, I'm
> not going to take these patches.
(+cc Joe Perches, Andrew Morton, Andy Whitcroft)
Hi Joe Perches,
Would you fix checkpatch.pl about DEFINE_PCI_DEVICE_TABLE?
Currently, checkpatch.pl guides to use DEFINE_PCI_DEVICE_TABLE
as below.
WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id
#331: FILE: drivers/usb/host/ehci-pci.c:331:
+static const struct pci_device_id pci_ids [] = { {
However, Greg Kroah-Hartman mentioned that DEFINE_PCI_DEVICE_TABLE
shouldn't be used anymore.
So, would you change checkpatch.pl in order to guide to use
struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE?
For example,
WARNING: Use struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE
Thank you.
Best regards,
Jingoo Han
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
