Messages in this thread | | | Date | Tue, 19 Sep 2023 12:35:45 +0400 | Subject | Re: [PATCH v2 2/2] ALSA: Add new driver for Marian M2 sound card | From | Ivan Orlov <> |
| |
On 9/19/23 11:56, Takashi Iwai wrote: > On Mon, 18 Sep 2023 20:10:44 +0200, > Ivan Orlov wrote: >> >> +#include <sound/core.h> >> +#include <sound/control.h> >> +#include <sound/pcm.h> >> +#include <sound/pcm_params.h> >> +#include <sound/core.h> >> +#include <sound/pcm.h> >> +#include <sound/initval.h> >> +#include <sound/info.h> >> +#include <linux/delay.h> >> +#include <linux/module.h> >> +#include <linux/pci.h> >> +#include <linux/interrupt.h> > > We usually include linux/* at first, followed by sound/*. > > >> +#define DEBUG > > Any need to define this for the production system? > > >> +struct marian_card_descriptor; >> +struct marian_card; >> + >> +struct marian_card_descriptor { >> + char *name; >> + char *port_names; >> + unsigned int speedmode_max; >> + unsigned int ch_in; >> + unsigned int ch_out; >> + unsigned int midi_in; >> + unsigned int midi_out; >> + unsigned int serial_in; >> + unsigned int serial_out; >> + unsigned int wck_in; >> + unsigned int wck_out; >> + >> + unsigned int dma_bufsize; >> + >> + void (*hw_constraints_func)(struct marian_card *marian, >> + struct snd_pcm_substream *substream, >> + struct snd_pcm_hw_params *params); >> + /* custom function to set up ALSA controls */ >> + void (*create_controls)(struct marian_card *marian); >> + /* init is called after probing the card */ >> + int (*init_card)(struct marian_card *marian); >> + void (*free_card)(struct marian_card *marian); >> + /* prepare is called when ALSA is opening the card */ >> + void (*prepare)(struct marian_card *marian); >> + void (*set_speedmode)(struct marian_card *marian, unsigned int speedmode); >> + void (*proc_status)(struct marian_card *marian, struct snd_info_buffer *buffer); >> + void (*proc_ports)(struct marian_card *marian, struct snd_info_buffer *buffer, >> + unsigned int type); >> + >> + struct snd_pcm_hardware info_playback; >> + struct snd_pcm_hardware info_capture; > > Do we need this kind of abstraction inside the driver? > As far as I see, the driver supports only a single model, hence there > is no real merit of abstracted / indirect function calls. > > So I stop reading at this point. > > > thanks, > > Takashi
Hi Takashi,
Thank you for the review! I will send the next version after removing all indirections and cleaning the code again.
-- Kind regards, Ivan Orlov
| |