At Thu, 30 Aug 2007 03:21:52 -0700 (PDT),
Trent Piepho wrote:
>> After applying this patch, the bt87x.patch file in alsa-driver doesn't apply.
> I'm not sure what I'm supposed to do about that?
Would be helpful, of course :) But I can do it myself, too. Don't
worry about that.
>> # HG changeset patch
> # User Trent Piepho <xyzzy at speakeasy.org>
> # Date 1188469025 25200
> # Node ID 33d453db23d246dade155a6fc3b91d8437a4b7f5
> # Parent 52dfc5244360d2b0b119786596962ff5d0c9f338
> snd-bt87x: Improve support for different board types
>> Different cards have different audio configurations, but the driver didn't
> support this. The only setting it had was the digital rate.
>> This patch adds a board configuration list. Currently, configurable items are
> the digital rate and the digital data format (for cards with an external ADC),
> a flag for the absence of an external ADC, and a flag for no connection to the
> Bt87x internal ADC.
>> This allows cards that don't use the internal ADC to omit the ALSA "Bt87x
> analog" device and related controls. Cards without an external ADC can omit
> the "Bt87x digital" device.
>> In order to support the CS5331A ADC used on the Osprey 440 and 2x0 cards, the
> digital format needs to be different than the default.
>> Support could be added for defining:
> The connections or lack of them to the Bt87x's internal ADC mux
> Multiple sample rates for an external ADC (e.g. Osprey)
> Control of an external mux for an external ADC (e.g. Osprey)
>> The card definitions for cards other than the Ospreys are kept equivalent to
> their old values. This is likely inaccurate for most cards, as it is doubtful
> that both an external and the internal ADC would be used. Lacking information
> on those cards, the behavior is left unchanged.
>> Signed-off-by: Trent Piepho <xyzzy at speakeasy.org>
The patch looks fine, but checkpatch.pl complains about some coding
style issuse.
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.
Thanks!
Takashi