Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro

Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro

[Jingoo Han]

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

Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro

[Joe Perches]

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

Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro

['Greg Kroah-Hartman']

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

Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro

[Joe Perches]

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

Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro

[Jingoo Han]

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

Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro

[Jingoo Han]

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

Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro

[Guenter Roeck]

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

Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro

[Jingoo Han]

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 {
 #define        to_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

Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro

[Guenter Roeck]

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 {
>   #define        to_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

Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro

[Jingoo Han]

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 {
> >   #define        to_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

Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro

[Joe Perches]

(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 {
> > >   #define        to_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
> 

Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro

[Jonas Bonn]

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

[PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE

[Joe Perches]

Prefer use of the direct definition of struct pci_device_id
instead of indirection via macro DEFINE_PCI_DEVICE_TABLE.

Update the PCI documentation to deprecate DEFINE_PCI_DEVICE_TABLE.
Update checkpatch adding --fix option.

Signed-off-by: Joe Perches <joe@perches.com>
---
 Documentation/PCI/pci.txt |  6 ++++--
 include/linux/pci.h       |  3 +--
 scripts/checkpatch.pl     | 11 +++++++----
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/Documentation/PCI/pci.txt b/Documentation/PCI/pci.txt
index 6f45856..9518006 100644
--- a/Documentation/PCI/pci.txt
+++ b/Documentation/PCI/pci.txt
@@ -123,8 +123,10 @@ initialization with a pointer to a structure describing the driver
 
 
 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.  Definitions with static const are generally preferred.
+Use of the deprecated macro DEFINE_PCI_DEVICE_TABLE should be avoided.
+
+Each entry consists of:
 
 	vendor,device	Vendor and device ID to match (or PCI_ANY_ID)
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1084a15..88674b0 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -634,8 +634,7 @@ struct pci_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.
+ * This macro is deprecated and should not be used in new code.
  */
 #define DEFINE_PCI_DEVICE_TABLE(_table) \
 	const struct pci_device_id _table[]
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 38be5d5..6f883f0 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2643,10 +2643,13 @@ sub process {
 			}
 		}
 
-# check for declarations of struct pci_device_id
-		if ($line =~ /\bstruct\s+pci_device_id\s+\w+\s*\[\s*\]\s*\=\s*\{/) {
-			WARN("DEFINE_PCI_DEVICE_TABLE",
-			     "Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id\n" . $herecurr);
+# check for uses of DEFINE_PCI_DEVICE_TABLE
+		if ($line =~ /\bDEFINE_PCI_DEVICE_TABLE\s*\(\s*(\w+)\s*\)\s*=/) {
+			if (WARN("DEFINE_PCI_DEVICE_TABLE",
+				 "Prefer struct pci_device_id over deprecated DEFINE_PCI_DEVICE_TABLE\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~ s/\b(?:static\s+|)DEFINE_PCI_DEVICE_TABLE\s*\(\s*(\w+)\s*\)\s*=\s*/static const struct pci_device_id $1\[\] = /;
+			}
 		}
 
 # check for new typedefs, only function parameters and sparse annotations

Re: [PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE

[Guenter Roeck]

On Mon, Dec 02, 2013 at 09:43:37AM -0800, Joe Perches wrote:
> Prefer use of the direct definition of struct pci_device_id

Add 'const' ?

> instead of indirection via macro DEFINE_PCI_DEVICE_TABLE.
> 
> Update the PCI documentation to deprecate DEFINE_PCI_DEVICE_TABLE.
> Update checkpatch adding --fix option.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  Documentation/PCI/pci.txt |  6 ++++--
>  include/linux/pci.h       |  3 +--
>  scripts/checkpatch.pl     | 11 +++++++----
>  3 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/PCI/pci.txt b/Documentation/PCI/pci.txt
> index 6f45856..9518006 100644
> --- a/Documentation/PCI/pci.txt
> +++ b/Documentation/PCI/pci.txt
> @@ -123,8 +123,10 @@ initialization with a pointer to a structure describing the driver
>  
>  
>  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.  Definitions with static const are generally preferred.
> +Use of the deprecated macro DEFINE_PCI_DEVICE_TABLE should be avoided.
> +
> +Each entry consists of:
>  
>  	vendor,device	Vendor and device ID to match (or PCI_ANY_ID)
>  
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 1084a15..88674b0 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -634,8 +634,7 @@ struct pci_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.
> + * This macro is deprecated and should not be used in new code.
>   */
>  #define DEFINE_PCI_DEVICE_TABLE(_table) \
>  	const struct pci_device_id _table[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 38be5d5..6f883f0 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2643,10 +2643,13 @@ sub process {
>  			}
>  		}
>  
> -# check for declarations of struct pci_device_id
> -		if ($line =~ /\bstruct\s+pci_device_id\s+\w+\s*\[\s*\]\s*\=\s*\{/) {
> -			WARN("DEFINE_PCI_DEVICE_TABLE",
> -			     "Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id\n" . $herecurr);
> +# check for uses of DEFINE_PCI_DEVICE_TABLE
> +		if ($line =~ /\bDEFINE_PCI_DEVICE_TABLE\s*\(\s*(\w+)\s*\)\s*=/) {
> +			if (WARN("DEFINE_PCI_DEVICE_TABLE",
> +				 "Prefer struct pci_device_id over deprecated DEFINE_PCI_DEVICE_TABLE\n" . $herecurr) &&

Add 'const' ?

Guenter

Re: [PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE

[Joe Perches]

On Mon, 2013-12-02 at 10:01 -0800, Guenter Roeck wrote:
> On Mon, Dec 02, 2013 at 09:43:37AM -0800, Joe Perches wrote:
> > Prefer use of the direct definition of struct pci_device_id
[]
> > +all-zero entry.  Definitions with static const are generally preferred.

see here:

> Add 'const' ?

Also see $fix.

Not all can be const as some codes actually modify
the tables for various quirks.

Re: [PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE

[Jingoo Han]

On Tuesday, December 03, 2013 2:44 AM, Joe Perches
> 
> Prefer use of the direct definition of struct pci_device_id
> instead of indirection via macro DEFINE_PCI_DEVICE_TABLE.
> 
> Update the PCI documentation to deprecate DEFINE_PCI_DEVICE_TABLE.
> Update checkpatch adding --fix option.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Reviewed-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  Documentation/PCI/pci.txt |  6 ++++--
>  include/linux/pci.h       |  3 +--
>  scripts/checkpatch.pl     | 11 +++++++----
>  3 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/PCI/pci.txt b/Documentation/PCI/pci.txt
> index 6f45856..9518006 100644
> --- a/Documentation/PCI/pci.txt
> +++ b/Documentation/PCI/pci.txt
> @@ -123,8 +123,10 @@ initialization with a pointer to a structure describing the driver
> 
> 
>  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.  Definitions with static const are generally preferred.
> +Use of the deprecated macro DEFINE_PCI_DEVICE_TABLE should be avoided.
> +
> +Each entry consists of:
> 
>  	vendor,device	Vendor and device ID to match (or PCI_ANY_ID)
> 
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 1084a15..88674b0 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -634,8 +634,7 @@ struct pci_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.
> + * This macro is deprecated and should not be used in new code.
>   */
>  #define DEFINE_PCI_DEVICE_TABLE(_table) \
>  	const struct pci_device_id _table[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 38be5d5..6f883f0 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2643,10 +2643,13 @@ sub process {
>  			}
>  		}
> 
> -# check for declarations of struct pci_device_id
> -		if ($line =~ /\bstruct\s+pci_device_id\s+\w+\s*\[\s*\]\s*\=\s*\{/) {
> -			WARN("DEFINE_PCI_DEVICE_TABLE",
> -			     "Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id\n" . $herecurr);
> +# check for uses of DEFINE_PCI_DEVICE_TABLE
> +		if ($line =~ /\bDEFINE_PCI_DEVICE_TABLE\s*\(\s*(\w+)\s*\)\s*=/) {
> +			if (WARN("DEFINE_PCI_DEVICE_TABLE",
> +				 "Prefer struct pci_device_id over deprecated DEFINE_PCI_DEVICE_TABLE\n" .
> $herecurr) &&
> +			    $fix) {
> +				$fixed[$linenr - 1] =~
> s/\b(?:static\s+|)DEFINE_PCI_DEVICE_TABLE\s*\(\s*(\w+)\s*\)\s*=\s*/static const struct pci_device_id
> $1\[\] = /;
> +			}
>  		}
> 
>  # check for new typedefs, only function parameters and sparse annotations

Re: [PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE

[Bjorn Helgaas]

On Mon, Dec 02, 2013 at 09:43:37AM -0800, Joe Perches wrote:
> Prefer use of the direct definition of struct pci_device_id
> instead of indirection via macro DEFINE_PCI_DEVICE_TABLE.
> 
> Update the PCI documentation to deprecate DEFINE_PCI_DEVICE_TABLE.
> Update checkpatch adding --fix option.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Applied with Jingoo's ack for v3.14, thanks!

Bjorn

> ---
>  Documentation/PCI/pci.txt |  6 ++++--
>  include/linux/pci.h       |  3 +--
>  scripts/checkpatch.pl     | 11 +++++++----
>  3 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/PCI/pci.txt b/Documentation/PCI/pci.txt
> index 6f45856..9518006 100644
> --- a/Documentation/PCI/pci.txt
> +++ b/Documentation/PCI/pci.txt
> @@ -123,8 +123,10 @@ initialization with a pointer to a structure describing the driver
>  
>  
>  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.  Definitions with static const are generally preferred.
> +Use of the deprecated macro DEFINE_PCI_DEVICE_TABLE should be avoided.
> +
> +Each entry consists of:
>  
>  	vendor,device	Vendor and device ID to match (or PCI_ANY_ID)
>  
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 1084a15..88674b0 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -634,8 +634,7 @@ struct pci_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.
> + * This macro is deprecated and should not be used in new code.
>   */
>  #define DEFINE_PCI_DEVICE_TABLE(_table) \
>  	const struct pci_device_id _table[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 38be5d5..6f883f0 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2643,10 +2643,13 @@ sub process {
>  			}
>  		}
>  
> -# check for declarations of struct pci_device_id
> -		if ($line =~ /\bstruct\s+pci_device_id\s+\w+\s*\[\s*\]\s*\=\s*\{/) {
> -			WARN("DEFINE_PCI_DEVICE_TABLE",
> -			     "Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id\n" . $herecurr);
> +# check for uses of DEFINE_PCI_DEVICE_TABLE
> +		if ($line =~ /\bDEFINE_PCI_DEVICE_TABLE\s*\(\s*(\w+)\s*\)\s*=/) {
> +			if (WARN("DEFINE_PCI_DEVICE_TABLE",
> +				 "Prefer struct pci_device_id over deprecated DEFINE_PCI_DEVICE_TABLE\n" . $herecurr) &&
> +			    $fix) {
> +				$fixed[$linenr - 1] =~ s/\b(?:static\s+|)DEFINE_PCI_DEVICE_TABLE\s*\(\s*(\w+)\s*\)\s*=\s*/static const struct pci_device_id $1\[\] = /;
> +			}
>  		}
>  
>  # check for new typedefs, only function parameters and sparse annotations
> 
>