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

From: Jingoo Han
Date: Sun Dec 01 2013 - 19:07:20 EST


On Friday, November 29, 2013 10:34 AM, Jingoo Han wrote:
> On Thursday, November 28, 2013 3:24 PM, Joe Perches wrote:
> >On Wed, 2013-11-27 at 21:53 -0800, 'Greg Kroah-Hartman' wrote:
> >>On Wed, Nov 27, 2013 at 09:40:13PM -0800, Joe Perches wrote:
> >>>On Thu, 2013-11-28 at 14:29 +0900, Jingoo Han wrote:
> >>>>On Thursday, November 28, 2013 1:08 PM, Greg Kroah-Hartman wrote:
> >>>>>On Thu, Nov 28, 2013 at 10:55:35AM +0900, Jingoo Han wrote:
> >>>>>> This macro is used to create a struct pci_device_id array.
> >>>>>
> >>>>> Yeah, and it's a horrid macro that deserves to be removed, please don't
> >>>>> use it in more places.
> >>>>>
> >>>>> Actually, if you could just remove it, that would be best, sorry, I'm
> >>>>> not going to take these patches.
> >>>>
> >>>> (+cc Joe Perches, Andrew Morton, Andy Whitcroft)
> >>>>
> >>>> Hi Joe Perches,
> >>>>
> >>>> Would you fix checkpatch.pl about DEFINE_PCI_DEVICE_TABLE?
> >>>> Currently, checkpatch.pl guides to use DEFINE_PCI_DEVICE_TABLE
> >>>> as below.
> >>>>
> >>>> WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id
> >>>> #331: FILE: drivers/usb/host/ehci-pci.c:331:
> >>>> +static const struct pci_device_id pci_ids [] = { {
> >>>>
> >>>> However, Greg Kroah-Hartman mentioned that DEFINE_PCI_DEVICE_TABLE
> >>>> shouldn't be used anymore.
> >>>>
> >>>> So, would you change checkpatch.pl in order to guide to use
> >>>> struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE?
> >>>>
> >>>> For example,
> >>>> WARNING: Use struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE
> >>>
> >>> The documentation doesn't agree with Greg.
> >[]
> >> I say just remove it, I should have done that years ago when I was the
> >> PCI maintainer, just never got around to it. No other bus has something
> >> like this for their device ids, why should PCI be "special"?
> >
> >Anyone else have an opinion?
> >
> >I don't care one way or another, but please, one way
> >not two.
>
> (+cc Bjorn Helgaas, linux-pci)
>
> Then, how about the following steps?
>
> 1. Fix ./Documentation/PCI/pci.txt as below.
> (Jingoo Han)
> The ID table is an array of struct pci_device_id entries ending with an
> -all-zero entry; use of the macro DEFINE_PCI_DEVICE_TABLE is the preferred
> -method of declaring the table. Each entry consists of:
> +all-zero entry; Each entry consists of:
>
> 2. Fix ./scripts/checkpatch.pl in order to guide to use
> struct pci_device_id instead of DEFINE_PCI_DEVICE_TABLE.
> (Joe Perches)

If all DEFINE_PCI_DEVICE_TABLEs are replaced with 'const struct pci_device_id'
and these patches are merged through 'driver-core.git', it will be not
necessary to fix ./scripts/checkpatch.pl.

Then, I will send patches to Greg Kroah-Hartman, with CC'ing each subsystem
maintainer.

Best regards,
Jingoo Han

>
> 3. Replace DEFINE_PCI_DEVICE_TABLE of ./drivers/*
> with 'const struct pci_device_id'.
> (Jingoo Han)
>
> 4. These patches will be merged through 'driver-core.git'
> with 'Acked-by' of each subsystem maintainer.
> (Greg Kroah-Hartman)
>
> Best regards,
> Jingoo Han
>
> >Changing checkpatch is a trifle, but there are a _lot_
> >of maintainers to work through if it's to be removed.
> >
> >It'll probably take several releases.
> >
> >$ git grep --name-only -w DEFINE_PCI_DEVICE_TABLE | \
> > cut -f1,2 -d/ | uniq -c
> > 1 Documentation/PCI
> > 1 arch/x86
> > 1 drivers/bcma
> > 3 drivers/block
> > 1 drivers/char
> > 1 drivers/cpufreq
> > 2 drivers/dma
> > 18 drivers/edac
> > 6 drivers/gpio
> > 6 drivers/gpu
> > 6 drivers/hwmon
> > 20 drivers/i2c
> > 2 drivers/infiniband
> > 1 drivers/ipack
> > 1 drivers/leds
> > 3 drivers/media
> > 10 drivers/mfd
> > 2 drivers/misc
> > 1 drivers/mmc
> > 1 drivers/mtd
> > 132 drivers/net
> > 1 drivers/ntb
> > 1 drivers/pci
> > 5 drivers/pcmcia
> > 2 drivers/platform
> > 1 drivers/ptp
> > 1 drivers/rapidio
> > 7 drivers/scsi
> > 3 drivers/spi
> > 65 drivers/staging
> > 3 drivers/tty
> > 1 drivers/uio
> > 5 drivers/usb
> > 1 drivers/video
> > 1 drivers/virtio
> > 3 drivers/vme
> > 9 drivers/watchdog
> > 1 drivers/xen
> > 1 include/linux
> > 1 scripts/checkpatch.pl
> > 1 scripts/tags.sh
> > 1 sound/oss
> > 67 sound/pci

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/