Re: [PATCH v2 2/2] ALSA: Add new driver for Marian M2 sound card

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux