[alsa-devel] [PATCH] Improved support for different bt87x board configurations

[Trent Piepho]

On Thu, 30 Aug 2007, Takashi Iwai wrote:
> One thin I don't like so much is:
>
> > +#define BT_DEVICE(chip, subvend, subdev, id) \
> >  	{ .vendor = PCI_VENDOR_ID_BROOKTREE, \
> > -	  .device = chip, \
> > +	  .device = PCI_DEVICE_ID_BROOKTREE ## chip, \
> >  	  .subvendor = subvend, .subdevice = subdev, \
> > -	  .driver_data = rate }
> > -
> > -/* driver_data is the default digital_rate value for that device */
> > +	  .driver_data = id }
> > +/* driver_data is the card id for that device */
> > +
> >  static struct pci_device_id snd_bt87x_ids[] = {
> >  	/* Hauppauge WinTV series */
> > -	BT_DEVICE(PCI_DEVICE_ID_BROOKTREE_878, 0x0070, 0x13eb, 32000),
> > +	BT_DEVICE(_878, 0x0070, 0x13eb, SND_BT87X_BOARD_HAUPPAUGE878),
> >  	/* Hauppauge WinTV series */
>
> The word _878 looks strange.  PCI_DEVICE_ID_BROOKTREE_878 is more
> obvious that it's a macro and the readability isn't so bad.
> I'd say, let it be.

The only problem is that PCI_DEVICE_ID_BROOKTREE_878 is so long that the
lines become longer than 80 columns.  They have to be wrapped, so it's no
longer one line, one card and less readable.  It's also easier to miss that
one line says PCI_DEVICE_ID_BROOKTREE_879, when you have such a long string
repeated many times, with only one digit different.

What if I used "BROOKTREE_878" or some suffix of that?