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

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

[Jingoo Han]

This macro is used to create a struct pci_device_id array.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/tty/serial/8250/8250_pci.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 4697a51..7a605c6 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -3492,7 +3492,7 @@ static struct pciserial_board pci_boards[] = {
 	},
 };
 
-static const struct pci_device_id blacklist[] = {
+static DEFINE_PCI_DEVICE_TABLE(blacklist) = {
 	/* softmodems */
 	{ PCI_VDEVICE(AL, 0x5457), }, /* ALi Corporation M5457 AC'97 Modem */
 	{ PCI_VDEVICE(MOTOROLA, 0x3052), }, /* Motorola Si3052-based modem */
@@ -3849,7 +3849,7 @@ static int pciserial_resume_one(struct pci_dev *dev)
 }
 #endif
 
-static struct pci_device_id serial_pci_tbl[] = {
+static DEFINE_PCI_DEVICE_TABLE(serial_pci_tbl) = {
 	/* Advantech use PCI_DEVICE_ID_ADVANTECH_PCI3620 (0x3620) as 'PCI_SUBVENDOR_ID' */
 	{	PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
 		PCI_DEVICE_ID_ADVANTECH_PCI3620, 0x0001, 0, 0,
-- 
1.7.10.4

[PATCH 2/5] serial: icom: use DEFINE_PCI_DEVICE_TABLE macro

[Jingoo Han]

This macro is used to create a struct pci_device_id array.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/tty/serial/icom.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/icom.c b/drivers/tty/serial/icom.c
index d98e433..0126ba9 100644
--- a/drivers/tty/serial/icom.c
+++ b/drivers/tty/serial/icom.c
@@ -66,7 +66,7 @@
 #define ICOM_PORT ((struct icom_port *)port)
 #define to_icom_adapter(d) container_of(d, struct icom_adapter, kref)
 
-static const struct pci_device_id icom_pci_table[] = {
+static DEFINE_PCI_DEVICE_TABLE(icom_pci_table) = {
 	{
 		.vendor = PCI_VENDOR_ID_IBM,
 		.device = PCI_DEVICE_ID_IBM_ICOM_DEV_ID_1,
-- 
1.7.10.4

[PATCH 3/5] serial: jsm: use DEFINE_PCI_DEVICE_TABLE macro

[Jingoo Han]

This macro is used to create a struct pci_device_id array.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/tty/serial/jsm/jsm_driver.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/jsm/jsm_driver.c b/drivers/tty/serial/jsm/jsm_driver.c
index a47d882..4e4899b 100644
--- a/drivers/tty/serial/jsm/jsm_driver.c
+++ b/drivers/tty/serial/jsm/jsm_driver.c
@@ -202,7 +202,7 @@ static void jsm_remove_one(struct pci_dev *pdev)
 	kfree(brd);
 }
 
-static struct pci_device_id jsm_pci_tbl[] = {
+static DEFINE_PCI_DEVICE_TABLE(jsm_pci_tbl) = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2DB9), 0, 0, 0 },
 	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2DB9PRI), 0, 0, 1 },
 	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2RJ45), 0, 0, 2 },
-- 
1.7.10.4

[PATCH 4/5] serial: mfd: use DEFINE_PCI_DEVICE_TABLE macro

[Jingoo Han]

This macro is used to create a struct pci_device_id array.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/tty/serial/mfd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/mfd.c b/drivers/tty/serial/mfd.c
index 52c930f..158d79e 100644
--- a/drivers/tty/serial/mfd.c
+++ b/drivers/tty/serial/mfd.c
@@ -1458,7 +1458,7 @@ static void serial_hsu_remove(struct pci_dev *pdev)
 }
 
 /* First 3 are UART ports, and the 4th is the DMA */
-static const struct pci_device_id pci_ids[] = {
+static DEFINE_PCI_DEVICE_TABLE(pci_ids) = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x081B) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x081C) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x081D) },
-- 
1.7.10.4

[PATCH 5/5] serial: txx9: use DEFINE_PCI_DEVICE_TABLE macro

[Jingoo Han]

This macro is used to create a struct pci_device_id array.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/tty/serial/serial_txx9.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/serial_txx9.c b/drivers/tty/serial/serial_txx9.c
index 90a080b..6e5fad8 100644
--- a/drivers/tty/serial/serial_txx9.c
+++ b/drivers/tty/serial/serial_txx9.c
@@ -1250,7 +1250,7 @@ static int pciserial_txx9_resume_one(struct pci_dev *dev)
 }
 #endif
 
-static const struct pci_device_id serial_txx9_pci_tbl[] = {
+static DEFINE_PCI_DEVICE_TABLE(serial_txx9_pci_tbl) = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA_2, PCI_DEVICE_ID_TOSHIBA_TC86C001_MISC) },
 	{ 0, }
 };
-- 
1.7.10.4

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

['Greg Kroah-Hartman']

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.

greg k-h

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.

OK, I see. :-)
Then, I will send the patch to remove DEFINE_PCI_DEVICE_TABLE soon.
Thank you for your guidance.

Best regards,
Jingoo Han

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 3/5] serial: jsm: use DEFINE_PCI_DEVICE_TABLE macro

[Thadeu Lima de Souza Cascardo]

On Thu, Nov 28, 2013 at 10:58:39AM +0900, Jingoo Han wrote:
> This macro is used to create a struct pci_device_id array.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>  drivers/tty/serial/jsm/jsm_driver.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/jsm/jsm_driver.c b/drivers/tty/serial/jsm/jsm_driver.c
> index a47d882..4e4899b 100644
> --- a/drivers/tty/serial/jsm/jsm_driver.c
> +++ b/drivers/tty/serial/jsm/jsm_driver.c
> @@ -202,7 +202,7 @@ static void jsm_remove_one(struct pci_dev *pdev)
>  	kfree(brd);
>  }
> 
> -static struct pci_device_id jsm_pci_tbl[] = {
> +static DEFINE_PCI_DEVICE_TABLE(jsm_pci_tbl) = {
>  	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2DB9), 0, 0, 0 },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2DB9PRI), 0, 0, 1 },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2RJ45), 0, 0, 2 },
> -- 
> 1.7.10.4
> 
> 

Acked-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>

Re: [PATCH 3/5] serial: jsm: use DEFINE_PCI_DEVICE_TABLE macro

[Jingoo Han]

On Thursday, November 28, 2013 8:36 PM, Thadeu Lima de Souza Cascardo wrote:
> On Thu, Nov 28, 2013 at 10:58:39AM +0900, Jingoo Han wrote:
> > This macro is used to create a struct pci_device_id array.
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> >  drivers/tty/serial/jsm/jsm_driver.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)

[.....]

> 
> Acked-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>

Thank you for your review! :-)

However, these patches should NOT be merged.

According to the Greg Kroah-Hartman,
"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."

So, I will send the patch to remove 'DEFINE_PCI_DEVICE_TABLE' instead.
Sorry for annoying you. :-)

Best regards,
Jingoo Han

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

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