Re: Aureal ALSA-kernel migration.

[Takashi Iwai <tiwai@suse.de>]

At Tue, 03 Feb 2004 12:40:20 -0300,
Manuel Jander wrote:
> 
> Hi Takashi,
> 
> > but a serious one is 
> > 
> > - au88x0_*.c have global functions vortex_*().
> > 
> > when you build multiple au88x0 drivers into kernel statically, they
> > will conflict with each other, because each au88[123]0_* includes the
> > same au88x0_* file.
> > 
> > i haven't notice this because alsa-drivers are supposed to be
> > modules...
> > 
> > 
> > to solve this situation, we'll need either:
> > 
> > - rename each function to a unique name for the card (maybe with the
> >   help of macro?)
> 
> I guess this can be done like the DRM kernel modules do:
> 
> #define DRM(x) snd_ati_##x
> 
> snd_ati_malloc(size_t size, int type)
> 
> p = DRM(malloc)(size, DRM_MEM_DRIVER)
> 
> That would be pretty easy to implement.
 
yes, that is also what i thought of.
but the resultant code looks a bit ugly.

> > - make them all static and implement as callbacks
> 
> As callbacks ? Hmm, do you mean some sort of function pointers that are
> assigned when the driver starts up to the corresponding function set ?
> I know what callbacks are, but i don't understand how callbacks fit in
> here (?).
 
well, this option isn't easy.  proposed just as a possible solution.

> > - make the h/w specific functions as callback functions (i.e. hwread,
> >   hwrite, etc), and each function calls these callbacks.
> > 
> > the last case would be the most elegant, but the module structure will
> > be changed in this way, namely, each snd-au88[123]0 and a core
> > (common) module snd-au88x0-lib.
> 
> Well, i will study the case, but whatever solution i take, i would not
> like to have to copy paste any code. That would be too bad.
> 
> All 3 aureal cards have the same architecture, the only things that
> change are the MMIO register offsets (but some feature relative offset
> are the same), and some of them have some features that others dont
> have.
> 
> Some time ago i thought about using dynamic offsets for the MMIO
> registers, and dynamic hardware function support, but i feared that the
> overhead could too bad. Im mean to have the offset addresses and
> "hardware feature is present" flags loaded into the vortex_t struct at
> runtime. But maybe its not too bad after all ? What do you think ?
> That way we would have one single driver for all 3 cards.

i vote for this one.

you can forget the overhead of offset calculation.  it must not be too
much (as long as you write the code rationally :)
an example implementation would be like this:

- prepare a static (constant) callback/data record for each chip type,

  struct au88x0_hw {
	int offset;	/* constant h/w offset */
	...
	int (*codec_read)(au88x0_t *chip, int reg); /* some lowlevel callback */
	...
	int (*build_eq)(au88x0_t *chip);	/* h/w specific constructor */
  };

  static struct au88x0_hw au8810_hw = {
	.offset = AU8810_OFFSET,
	...
  };
 
  static struct au88x0_hw au8820_hw = {
	...
  };

and you pass an enum (or the pointer of this record above) for each
type in the pci id list,

  static struct pci_device_id snd_vortex_ids[] = {
      {PCI_VENDOR_ID_AUREAL, PCI_DEVICE_ID_AU8810,
       PCI_ANY_ID, PCI_ANY_ID, 0, 0, TYPE_AU8810,},
      {PCI_VENDOR_ID_AUREAL, PCI_DEVICE_ID_AU8820,
       PCI_ANY_ID, PCI_ANY_ID, 0, 0, TYPE_AU8820,},
      {PCI_VENDOR_ID_AUREAL, PCI_DEVICE_ID_AU8830,
       PCI_ANY_ID, PCI_ANY_ID, 0, 0, TYPE_AU8830,},
    {0,}
  };

the au88x0 chip record will have the pointer of this h/w information,

  struct snd_au88x0 {
	...
	const struct au88x0_hw *hw;
	...
  };

"the hardware feature present" can be determined simply by checking
whether the corresponding callback pointer exists.


Takashi


-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn