* [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro @ 2013-11-28 1:55 Jingoo Han 2013-11-28 1:57 ` [PATCH 2/5] serial: icom: " Jingoo Han ` (4 more replies) 0 siblings, 5 replies; 27+ messages in thread From: Jingoo Han @ 2013-11-28 1:55 UTC (permalink / raw) To: 'Greg Kroah-Hartman' Cc: linux-serial, 'Jingoo Han', 'Angelo Butti', 'Ian Abbott', 'Heikki Krogerus' 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/5] serial: icom: use DEFINE_PCI_DEVICE_TABLE macro 2013-11-28 1:55 [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro Jingoo Han @ 2013-11-28 1:57 ` Jingoo Han 2013-11-28 1:58 ` [PATCH 3/5] serial: jsm: " Jingoo Han ` (3 subsequent siblings) 4 siblings, 0 replies; 27+ messages in thread From: Jingoo Han @ 2013-11-28 1:57 UTC (permalink / raw) To: 'Greg Kroah-Hartman'; +Cc: linux-serial, '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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/5] serial: jsm: use DEFINE_PCI_DEVICE_TABLE macro 2013-11-28 1:55 [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro Jingoo Han 2013-11-28 1:57 ` [PATCH 2/5] serial: icom: " Jingoo Han @ 2013-11-28 1:58 ` Jingoo Han 2013-11-28 11:35 ` Thadeu Lima de Souza Cascardo 2013-11-28 1:59 ` [PATCH 4/5] serial: mfd: " Jingoo Han ` (2 subsequent siblings) 4 siblings, 1 reply; 27+ messages in thread From: Jingoo Han @ 2013-11-28 1:58 UTC (permalink / raw) To: 'Greg Kroah-Hartman' Cc: linux-serial, 'Jingoo Han', 'Thadeu Lima de Souza Cascardo' 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 3/5] serial: jsm: use DEFINE_PCI_DEVICE_TABLE macro 2013-11-28 1:58 ` [PATCH 3/5] serial: jsm: " Jingoo Han @ 2013-11-28 11:35 ` Thadeu Lima de Souza Cascardo 2013-11-29 0:53 ` Jingoo Han 0 siblings, 1 reply; 27+ messages in thread From: Thadeu Lima de Souza Cascardo @ 2013-11-28 11:35 UTC (permalink / raw) To: Jingoo Han; +Cc: 'Greg Kroah-Hartman', linux-serial 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> ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/5] serial: jsm: use DEFINE_PCI_DEVICE_TABLE macro 2013-11-28 11:35 ` Thadeu Lima de Souza Cascardo @ 2013-11-29 0:53 ` Jingoo Han 0 siblings, 0 replies; 27+ messages in thread From: Jingoo Han @ 2013-11-29 0:53 UTC (permalink / raw) To: 'Thadeu Lima de Souza Cascardo' Cc: 'Greg Kroah-Hartman', linux-serial, '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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 4/5] serial: mfd: use DEFINE_PCI_DEVICE_TABLE macro 2013-11-28 1:55 [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro Jingoo Han 2013-11-28 1:57 ` [PATCH 2/5] serial: icom: " Jingoo Han 2013-11-28 1:58 ` [PATCH 3/5] serial: jsm: " Jingoo Han @ 2013-11-28 1:59 ` Jingoo Han 2013-11-28 2:00 ` [PATCH 5/5] serial: txx9: " Jingoo Han 2013-11-28 4:07 ` [PATCH 1/5] serial: 8250_pci: " 'Greg Kroah-Hartman' 4 siblings, 0 replies; 27+ messages in thread From: Jingoo Han @ 2013-11-28 1:59 UTC (permalink / raw) To: 'Greg Kroah-Hartman'; +Cc: linux-serial, '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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 5/5] serial: txx9: use DEFINE_PCI_DEVICE_TABLE macro 2013-11-28 1:55 [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro Jingoo Han ` (2 preceding siblings ...) 2013-11-28 1:59 ` [PATCH 4/5] serial: mfd: " Jingoo Han @ 2013-11-28 2:00 ` Jingoo Han 2013-11-28 4:07 ` [PATCH 1/5] serial: 8250_pci: " 'Greg Kroah-Hartman' 4 siblings, 0 replies; 27+ messages in thread From: Jingoo Han @ 2013-11-28 2:00 UTC (permalink / raw) To: 'Greg Kroah-Hartman'; +Cc: linux-serial, '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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro 2013-11-28 1:55 [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro Jingoo Han ` (3 preceding siblings ...) 2013-11-28 2:00 ` [PATCH 5/5] serial: txx9: " Jingoo Han @ 2013-11-28 4:07 ` 'Greg Kroah-Hartman' 2013-11-28 4:32 ` Jingoo Han 2013-11-28 5:29 ` Jingoo Han 4 siblings, 2 replies; 27+ messages in thread From: 'Greg Kroah-Hartman' @ 2013-11-28 4:07 UTC (permalink / raw) To: Jingoo Han Cc: linux-serial, 'Angelo Butti', 'Ian Abbott', 'Heikki Krogerus' 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro 2013-11-28 4:07 ` [PATCH 1/5] serial: 8250_pci: " 'Greg Kroah-Hartman' @ 2013-11-28 4:32 ` Jingoo Han 2013-11-28 5:29 ` Jingoo Han 1 sibling, 0 replies; 27+ messages in thread From: Jingoo Han @ 2013-11-28 4:32 UTC (permalink / raw) To: 'Greg Kroah-Hartman' Cc: linux-serial, 'Angelo Butti', 'Ian Abbott', 'Heikki Krogerus', '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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro 2013-11-28 4:07 ` [PATCH 1/5] serial: 8250_pci: " 'Greg Kroah-Hartman' 2013-11-28 4:32 ` Jingoo Han @ 2013-11-28 5:29 ` Jingoo Han 2013-11-28 5:40 ` Joe Perches 1 sibling, 1 reply; 27+ messages in thread From: Jingoo Han @ 2013-11-28 5:29 UTC (permalink / raw) To: 'Joe Perches' Cc: linux-kernel, 'Andrew Morton', 'Andy Whitcroft', linux-serial, 'Greg Kroah-Hartman' 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro 2013-11-28 5:29 ` Jingoo Han @ 2013-11-28 5:40 ` Joe Perches 2013-11-28 5:53 ` 'Greg Kroah-Hartman' 0 siblings, 1 reply; 27+ messages in thread From: Joe Perches @ 2013-11-28 5:40 UTC (permalink / raw) To: Jingoo Han Cc: linux-kernel, 'Andrew Morton', 'Andy Whitcroft', linux-serial, 'Greg Kroah-Hartman' 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[] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro 2013-11-28 5:40 ` Joe Perches @ 2013-11-28 5:53 ` 'Greg Kroah-Hartman' 2013-11-28 6:24 ` Joe Perches 0 siblings, 1 reply; 27+ messages in thread From: 'Greg Kroah-Hartman' @ 2013-11-28 5:53 UTC (permalink / raw) To: Joe Perches Cc: Jingoo Han, linux-kernel, 'Andrew Morton', 'Andy Whitcroft', linux-serial 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro 2013-11-28 5:53 ` 'Greg Kroah-Hartman' @ 2013-11-28 6:24 ` Joe Perches 2013-11-29 1:33 ` Jingoo Han 0 siblings, 1 reply; 27+ messages in thread From: Joe Perches @ 2013-11-28 6:24 UTC (permalink / raw) To: 'Greg Kroah-Hartman' Cc: Jingoo Han, linux-kernel, 'Andrew Morton', 'Andy Whitcroft', linux-serial 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro 2013-11-28 6:24 ` Joe Perches @ 2013-11-29 1:33 ` Jingoo Han 2013-12-02 0:07 ` Jingoo Han 0 siblings, 1 reply; 27+ messages in thread From: Jingoo Han @ 2013-11-29 1:33 UTC (permalink / raw) To: 'Joe Perches', 'Greg Kroah-Hartman', 'Bjorn Helgaas' Cc: linux-kernel, linux-pci, 'Andrew Morton', 'Andy Whitcroft', linux-serial, '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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro 2013-11-29 1:33 ` Jingoo Han @ 2013-12-02 0:07 ` Jingoo Han 0 siblings, 0 replies; 27+ messages in thread From: Jingoo Han @ 2013-12-02 0:07 UTC (permalink / raw) To: 'Jingoo Han', 'Joe Perches', 'Greg Kroah-Hartman', 'Bjorn Helgaas' Cc: linux-kernel, linux-pci, 'Andrew Morton', 'Andy Whitcroft', linux-serial, '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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro @ 2013-12-02 0:07 ` Jingoo Han 0 siblings, 0 replies; 27+ messages in thread From: Jingoo Han @ 2013-12-02 0:07 UTC (permalink / raw) To: 'Joe Perches', 'Greg Kroah-Hartman', 'Bjorn Helgaas' Cc: linux-kernel, linux-pci, 'Andrew Morton', 'Andy Whitcroft', linux-serial, '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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro 2013-12-02 0:07 ` Jingoo Han (?) @ 2013-12-02 3:45 ` Guenter Roeck 2013-12-02 3:50 ` Jingoo Han -1 siblings, 1 reply; 27+ messages in thread From: Guenter Roeck @ 2013-12-02 3:45 UTC (permalink / raw) To: Jingoo Han, 'Joe Perches', 'Greg Kroah-Hartman', 'Bjorn Helgaas' Cc: linux-kernel, linux-pci, 'Andrew Morton', 'Andy Whitcroft', linux-serial 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro 2013-12-02 3:45 ` Guenter Roeck @ 2013-12-02 3:50 ` Jingoo Han 2013-12-02 3:55 ` Guenter Roeck 0 siblings, 1 reply; 27+ messages in thread From: Jingoo Han @ 2013-12-02 3:50 UTC (permalink / raw) To: 'Guenter Roeck' Cc: 'Joe Perches', 'Greg Kroah-Hartman', 'Bjorn Helgaas', linux-kernel, linux-pci, 'Andrew Morton', 'Andy Whitcroft', linux-serial, '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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro 2013-12-02 3:50 ` Jingoo Han @ 2013-12-02 3:55 ` Guenter Roeck 2013-12-02 4:03 ` Jingoo Han 0 siblings, 1 reply; 27+ messages in thread From: Guenter Roeck @ 2013-12-02 3:55 UTC (permalink / raw) To: Jingoo Han Cc: 'Joe Perches', 'Greg Kroah-Hartman', 'Bjorn Helgaas', linux-kernel, linux-pci, 'Andrew Morton', 'Andy Whitcroft', linux-serial 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro 2013-12-02 3:55 ` Guenter Roeck @ 2013-12-02 4:03 ` Jingoo Han 2013-12-02 5:48 ` Joe Perches 0 siblings, 1 reply; 27+ messages in thread From: Jingoo Han @ 2013-12-02 4:03 UTC (permalink / raw) To: 'Guenter Roeck', 'Joe Perches' Cc: 'Greg Kroah-Hartman', 'Bjorn Helgaas', linux-kernel, linux-pci, 'Andrew Morton', 'Andy Whitcroft', linux-serial, '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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro 2013-12-02 4:03 ` Jingoo Han @ 2013-12-02 5:48 ` Joe Perches 2013-12-02 10:44 ` Jonas Bonn 0 siblings, 1 reply; 27+ messages in thread From: Joe Perches @ 2013-12-02 5:48 UTC (permalink / raw) To: Jingoo Han, Jonas Bonn Cc: 'Guenter Roeck', 'Greg Kroah-Hartman', 'Bjorn Helgaas', linux-kernel, linux-pci, 'Andrew Morton', 'Andy Whitcroft', linux-serial (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 > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro 2013-12-02 5:48 ` Joe Perches @ 2013-12-02 10:44 ` Jonas Bonn 2013-12-02 17:43 ` [PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE Joe Perches 0 siblings, 1 reply; 27+ messages in thread From: Jonas Bonn @ 2013-12-02 10:44 UTC (permalink / raw) To: Joe Perches Cc: Jingoo Han, 'Guenter Roeck', 'Greg Kroah-Hartman', 'Bjorn Helgaas', linux-kernel, linux-pci, 'Andrew Morton', 'Andy Whitcroft', linux-serial 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE 2013-12-02 10:44 ` Jonas Bonn @ 2013-12-02 17:43 ` Joe Perches 2013-12-02 18:01 ` Guenter Roeck ` (2 more replies) 0 siblings, 3 replies; 27+ messages in thread From: Joe Perches @ 2013-12-02 17:43 UTC (permalink / raw) To: Jonas Bonn Cc: Jingoo Han, 'Guenter Roeck', 'Greg Kroah-Hartman', 'Bjorn Helgaas', linux-kernel, linux-pci, 'Andrew Morton', 'Andy Whitcroft', linux-serial 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE 2013-12-02 17:43 ` [PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE Joe Perches @ 2013-12-02 18:01 ` Guenter Roeck 2013-12-02 18:07 ` Joe Perches 2013-12-03 1:52 ` Jingoo Han 2013-12-13 18:39 ` Bjorn Helgaas 2 siblings, 1 reply; 27+ messages in thread From: Guenter Roeck @ 2013-12-02 18:01 UTC (permalink / raw) To: Joe Perches Cc: Jonas Bonn, Jingoo Han, 'Greg Kroah-Hartman', 'Bjorn Helgaas', linux-kernel, linux-pci, 'Andrew Morton', 'Andy Whitcroft', linux-serial 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE 2013-12-02 18:01 ` Guenter Roeck @ 2013-12-02 18:07 ` Joe Perches 0 siblings, 0 replies; 27+ messages in thread From: Joe Perches @ 2013-12-02 18:07 UTC (permalink / raw) To: Guenter Roeck Cc: Jonas Bonn, Jingoo Han, 'Greg Kroah-Hartman', 'Bjorn Helgaas', linux-kernel, linux-pci, 'Andrew Morton', 'Andy Whitcroft', linux-serial 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. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE 2013-12-02 17:43 ` [PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE Joe Perches 2013-12-02 18:01 ` Guenter Roeck @ 2013-12-03 1:52 ` Jingoo Han 2013-12-13 18:39 ` Bjorn Helgaas 2 siblings, 0 replies; 27+ messages in thread From: Jingoo Han @ 2013-12-03 1:52 UTC (permalink / raw) To: 'Joe Perches' Cc: 'Jonas Bonn', 'Guenter Roeck', 'Greg Kroah-Hartman', 'Bjorn Helgaas', linux-kernel, linux-pci, 'Andrew Morton', 'Andy Whitcroft', linux-serial, '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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE 2013-12-02 17:43 ` [PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE Joe Perches 2013-12-02 18:01 ` Guenter Roeck 2013-12-03 1:52 ` Jingoo Han @ 2013-12-13 18:39 ` Bjorn Helgaas 2 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2013-12-13 18:39 UTC (permalink / raw) To: Joe Perches Cc: Jonas Bonn, Jingoo Han, 'Guenter Roeck', 'Greg Kroah-Hartman', linux-kernel, linux-pci, 'Andrew Morton', 'Andy Whitcroft', linux-serial 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 > > ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2013-12-13 18:39 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-11-28 1:55 [PATCH 1/5] serial: 8250_pci: use DEFINE_PCI_DEVICE_TABLE macro Jingoo Han 2013-11-28 1:57 ` [PATCH 2/5] serial: icom: " Jingoo Han 2013-11-28 1:58 ` [PATCH 3/5] serial: jsm: " Jingoo Han 2013-11-28 11:35 ` Thadeu Lima de Souza Cascardo 2013-11-29 0:53 ` Jingoo Han 2013-11-28 1:59 ` [PATCH 4/5] serial: mfd: " Jingoo Han 2013-11-28 2:00 ` [PATCH 5/5] serial: txx9: " Jingoo Han 2013-11-28 4:07 ` [PATCH 1/5] serial: 8250_pci: " 'Greg Kroah-Hartman' 2013-11-28 4:32 ` Jingoo Han 2013-11-28 5:29 ` Jingoo Han 2013-11-28 5:40 ` Joe Perches 2013-11-28 5:53 ` 'Greg Kroah-Hartman' 2013-11-28 6:24 ` Joe Perches 2013-11-29 1:33 ` Jingoo Han 2013-12-02 0:07 ` Jingoo Han 2013-12-02 0:07 ` Jingoo Han 2013-12-02 3:45 ` Guenter Roeck 2013-12-02 3:50 ` Jingoo Han 2013-12-02 3:55 ` Guenter Roeck 2013-12-02 4:03 ` Jingoo Han 2013-12-02 5:48 ` Joe Perches 2013-12-02 10:44 ` Jonas Bonn 2013-12-02 17:43 ` [PATCH] pci/checkpatch: Deprecate DEFINE_PCI_DEVICE_TABLE Joe Perches 2013-12-02 18:01 ` Guenter Roeck 2013-12-02 18:07 ` Joe Perches 2013-12-03 1:52 ` Jingoo Han 2013-12-13 18:39 ` Bjorn Helgaas
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.