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

[Takashi Iwai]

At Wed, 5 Sep 2007 02:09:45 -0700 (PDT),
Trent Piepho wrote:
> 
> On Wed, 5 Sep 2007, Mauro Carvalho Chehab wrote:
> > Em Ter, 2007-09-04 às 14:04 -0700, Trent Piepho escreveu:
> > > There are few drawbacks.
> > >
> > > First, there are like 20x more cards supported by the bttv driver than by
> > > the bt87x driver.  It's going to produce a very sparse card database in the
> > > ALSA driver.  Note that for the most part, __initdata doesn't work, so you
> > > can't say "it'll just be dropped after the driver loads."
> >
> > You should notice that even the way you've mapped is a a little sparse,
> > since just two boards use digital_fmt.
> >
> > Maybe one solution for that is to use a different code or data
> > structure.
> >
> > For example, instead of using a data struct, you could use some sort of
> > code like (*) (**):
> >
> > static void fill_board (struct snd_bt87x_board &board, int board_id)
> > {
> >         board->dig_rate = 32000;
> >         switch (board_id) {
> >         case BTTV_BOARD_AVPHONE98:
> >                 board.dig_rate = 48000,
> > 		break;
> 
> I don't think gcc will be smart enough to a bunch of cases with random numbers
> into a O(1) lookup, instead it will use an O(log n) branch chain.  With all
> the explicit assignments, I doubt it will be much smaller, if any, since there
> are just a couple board configurations.  Several board configurations are
> currently duplicates and I think I'll just merge them.
> 
> But code wise I think using a switch with manual assignments is ugly.  It's a
> lot more code to write and just doesn't have the regularity and elegance of a
> table.

I second for Trent.  The switch is uglier than the array for such a
purpose.


> > > And if both linuxtv and alsa use the same header, what happens if they
> > > don't push the changes to GIT at the exact same time?  You'll have patches
> > > in one subsystem that are dependant on the other subsystem.  How will they
> > > make the same merge window?
> >
> > This should be coordinated between me and Takashi. I have no objections
> > if he pull such patch, if he is willing to send the pull request before
> > myself.
> >
> > I don't see this as a big trouble. There are other cases like this
> > already (pci_ids.h, i2c_id.h). Also, currently, there are not much bttv
> > board additions anymore.
> 
> And pci_ids and i2d_id are pain to deal with.  Both v4l-dvb and ALSA have out
> of kernel tree build system that try to be backward compatible with older
> kernels.  When the IDs come from somewhere outside the repository, you need to
> have compat patches to deal with old id files that are missing ids.  Look at
> all the kernel-sync patches for those id files, all the compat code in the
> various drivers for non-defined PCI ids.  There's even something I had to add
> to compat.h for an i2c ID that went away in newer kernels.
> 
> It's a lot of extra work, and I just don't see the point.  To reuse a couple
> defines from a bttv header file?  They are just arbitrary numbers, it's not
> like there is any benefit from using the same value in both drivers.

Well, at first I misunderstood that we would share the PCI SSIDs
together with enum items.  Sharing only the enum items doesn't give
much improvements.  Sharing the card database may do, though.

So, I see Trent's point.  The merge work wouldn't pay for the benifit
by sharing only the enum id, not the card database.


thanks,

Takashi