lkml.org 
[lkml]   [2023]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 2/2] ALSA: Add new driver for Marian M2 sound card
From
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

\
 
 \ /
  Last update: 2023-09-19 10:51    [W:3.718 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site