From: Greg KH on
On Thu, Jul 15, 2010 at 08:52:37PM +0200, Peter Huewe wrote:
> From: Peter Huewe <peterhuewe(a)gmx.de>
>
> This patch converts pci_table entries, where .subvendor=PCI_ANY_ID and
> .subdevice=PCI_ANY_ID, .class=0 and .class_mask=0, to use the
> PCI_VDEVICE macro, and thus improves readability.
>
> Signed-off-by: Peter Huewe <peterhuewe(a)gmx.de>
> ---
> drivers/char/epca.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/char/epca.c b/drivers/char/epca.c
> index d9df46a..5dafcdb 100644
> --- a/drivers/char/epca.c
> +++ b/drivers/char/epca.c
> @@ -2762,10 +2762,10 @@ err_out:
>
>
> static struct pci_device_id epca_pci_tbl[] = {
> - { PCI_VENDOR_DIGI, PCI_DEVICE_XR, PCI_ANY_ID, PCI_ANY_ID, 0, 0, brd_xr },
> - { PCI_VENDOR_DIGI, PCI_DEVICE_XEM, PCI_ANY_ID, PCI_ANY_ID, 0, 0, brd_xem },
> - { PCI_VENDOR_DIGI, PCI_DEVICE_CX, PCI_ANY_ID, PCI_ANY_ID, 0, 0, brd_cx },
> - { PCI_VENDOR_DIGI, PCI_DEVICE_XRJ, PCI_ANY_ID, PCI_ANY_ID, 0, 0, brd_xrj },
> + { PCI_VDEVICE(DIGI, PCI_DEVICE_XR), brd_xr },
> + { PCI_VDEVICE(DIGI, PCI_DEVICE_XEM), brd_xem },
> + { PCI_VDEVICE(DIGI, PCI_DEVICE_CX), brd_cx },
> + { PCI_VDEVICE(DIGI, PCI_DEVICE_XRJ), brd_xrj },

The main reason I hate this macro, is that it now makes it almost
impossible to grep for any users of the PCI_VENDOR_DIGI pci vendor id.
I much prefer the PCI_DEVICE() macro instead, and as such, I'm not
willing to take any of these patches, sorry.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on
On Thu, Jul 15, 2010 at 02:00:15PM -0700, Joe Perches wrote:
> On Thu, 2010-07-15 at 13:45 -0700, Greg KH wrote:
> > I much prefer the PCI_DEVICE() macro instead, and as such, I'm not
> > willing to take any of these patches, sorry.
>
> grepping for pci device ids using constants and
> expecting the result to be comprehensive isn't
> sensible.

But it's a nice goal :)

> $ grep -rwP --include=*.[ch] -w PCI_VDEVICE drivers/char | wc -l
> 32

drivers/char is not exactly a large collection of PCI drivers, only some
old serial port ones.

> The current drivers/ use of PCI_VDEVICE to PCI_DEVICE is ~50/50
>
> $ grep --include=*.[ch] -rwP PCI_DEVICE drivers | wc -l
> 866
> $ grep --include=*.[ch] -rwP PCI_VDEVICE drivers | wc -l
> 768

Hey, anything to increase that ratio is good :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on
On Thu, Jul 15, 2010 at 09:44:23PM -0700, Joe Perches wrote:
> On Thu, 2010-07-15 at 21:29 -0700, Greg KH wrote:
> > On Thu, Jul 15, 2010 at 02:00:15PM -0700, Joe Perches wrote:
> > > On Thu, 2010-07-15 at 13:45 -0700, Greg KH wrote:
> > > > I much prefer the PCI_DEVICE() macro instead, and as such, I'm not
> > > > willing to take any of these patches, sorry.
> > > grepping for pci device ids using constants and
> > > expecting the result to be comprehensive isn't
> > > sensible.
> > But it's a nice goal :)
>
> I think your goal is not a good one.
>
> For instance:
>
> $ grep -rP --include=*.[ch] "\bPCI_VDEVICE\s*\(\s*INTEL" drivers | wc -l
> 201
> $ grep -rP --include=*.[ch] "\bPCI_DEVICE\s*\(PCI_VENDOR_ID_INTEL" drivers | wc -l
> 45
> $ grep -rP --include=*.[ch] "\bPCI_DEVICE\s*\(\s*0x8086" drivers | wc -l
> 38
>
> I'd much rather do a search for "PCI_VDEVICE.*INTEL"

I'd much rather use 'cscope' or 'ctags' than trying to remember regular
expressions like the above.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on
On Thu, Jul 15, 2010 at 10:37:20PM -0700, Joe Perches wrote:
> On Thu, 2010-07-15 at 22:29 -0700, Greg KH wrote:
> > On Thu, Jul 15, 2010 at 09:44:23PM -0700, Joe Perches wrote:
> > > On Thu, 2010-07-15 at 21:29 -0700, Greg KH wrote:
> > > > On Thu, Jul 15, 2010 at 02:00:15PM -0700, Joe Perches wrote:
> > > > > On Thu, 2010-07-15 at 13:45 -0700, Greg KH wrote:
> > > > > > I much prefer the PCI_DEVICE() macro instead, and as such, I'm not
> > > > > > willing to take any of these patches, sorry.
> > > > > grepping for pci device ids using constants and
> > > > > expecting the result to be comprehensive isn't
> > > > > sensible.
> > > > But it's a nice goal :)
> > > I think your goal is not a good one.
> > >
> > > For instance:
> > >
> > > $ grep -rP --include=*.[ch] "\bPCI_VDEVICE\s*\(\s*INTEL" drivers | wc -l
> > > 201
> > > $ grep -rP --include=*.[ch] "\bPCI_DEVICE\s*\(PCI_VENDOR_ID_INTEL" drivers | wc -l
> > > 45
> > > $ grep -rP --include=*.[ch] "\bPCI_DEVICE\s*\(\s*0x8086" drivers | wc -l
> > > 38
> > > I'd much rather do a search for "PCI_VDEVICE.*INTEL"
> > I'd much rather use 'cscope' or 'ctags' than trying to remember regular
> > expressions like the above.
>
> Then it appears your original argument doesn't have much merit.

I'd rather people not use PCI_VDEVICE() as then you can't easily scan
for the PCI_VENDOR_ID_FOO value usage with a tool like cscope, so I
think my original point stands.

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