Re: [alsa-devel] [RFC/PATCH v6 03/12] media: Entities, pads and links
At Tue, 14 Dec 2010 14:31:55 +0100, Clemens Ladisch wrote: Should we have an AUDIO category ? Probably not, because there are combined audio/video jacks, any maybe other entities. Yes, nowadays HDMI / DP are pretty common, for example. Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG at mm/mmap.c:2309 when cx18.ko and cx18-alsa.ko loaded
At Fri, 04 Mar 2011 12:13:04 -0500, Andy Walls wrote: On Fri, 2011-03-04 at 10:50 -0500, Devin Heitmueller wrote: On Thu, Mar 3, 2011 at 9:06 PM, Andy Walls awa...@md.metrocast.net wrote: Hi, I got a BUG when loading the cx18.ko module (which in turn requests the cx18-alsa.ko module) on a kernel built from this repository http://git.linuxtv.org/media_tree.git staging/for_v2.6.39 which I beleive is based on 2.6.38-rc2. The BUG is mmap related and I'm almost certain it has to do with userspace accessing cx18-alsa.ko ALSA device nodes, since cx18.ko doesn't provide any mmap() related file ops. So here is my transcription of a fuzzy digital photo of the screen: snip I'm not very familiar with mmap() nor ALSA and I did not author the cx18-alsa part of the cx18 driver, so any hints at where to look for the problem are appreciated. Hi Andy, I'm traveling on business for about two weeks, so I won't be able to look into this right now. Any idea whether this is some new regression? I do not know. I normally don't let cx18-alsa.ko load, due to PulseAudio's persistence at keeping the device nodes open (which makes unloading the cx18.ko module for development a hassle.) I'm just trying to understand whether this is something that has always been there since I originally added the ALSA support to cx18 or whether it's something that is new, in which case it might make sense to drag the ALSA people into the conversation since there haven't been any changes in the cx18 driver lately. I can add some information about what is going on in userspace. This was on a Fedora 10 machine. When devices nodes show up, the HAL daemon and PulseAudio start using the device nodes right away. That activity triggers cx18.ko to do a firmware load which gets udevd running to satisfy firmware requests, and then the cx18 driver issues some simple commands to the CX23418 firmware, which will have acknowledgment interrupts coming back from the CX23418. I resolved the firmware race in cx18*.ko a while ago, so I'm confident its not an issue. The BUG looks like some sort of mmap() race or memory management problem outside of the cx18*.ko modules, given that mmput(), which appears to be an mm specific reference counting function, is involved. It could also be in ALSA I guess. There is no change in ALSA core regarding mmap for really long time. If it's a regression, it must be triggered by some other changes. Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] radio-maestro broken (conflicts with snd-es1968)
At Sat, 12 Mar 2011 19:19:00 +0100, Ondrej Zary wrote: Hello, the radio-maestro driver is badly broken. It's intended to drive the radio on MediaForte ESS Maestro-based sound cards with integrated radio (like SF64-PCE2-04). But it conflicts with snd_es1968, ALSA driver for the sound chip itself. If one driver is loaded, the other one does not work - because a driver is already registered for the PCI device (there is only one). This was probably broken by conversion of PCI probing in 2006: ttp://lkml.org/lkml/2005/12/31/93 How to fix it properly? Include radio functionality in snd-es1968 and delete radio-maestro? Yes. Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] radio-maestro broken (conflicts with snd-es1968)
At Sat, 12 Mar 2011 19:52:39 +0100, Hans Verkuil wrote: On Saturday, March 12, 2011 19:19:00 Ondrej Zary wrote: Hello, the radio-maestro driver is badly broken. It's intended to drive the radio on MediaForte ESS Maestro-based sound cards with integrated radio (like SF64-PCE2-04). But it conflicts with snd_es1968, ALSA driver for the sound chip itself. If one driver is loaded, the other one does not work - because a driver is already registered for the PCI device (there is only one). This was probably broken by conversion of PCI probing in 2006: ttp://lkml.org/lkml/2005/12/31/93 How to fix it properly? Include radio functionality in snd-es1968 and delete radio-maestro? Interesting. I don't know anyone among the video4linux developers who has this hardware, so the radio-maestro driver hasn't been tested in at least 6 or 7 years. The proper fix would be to do it like the fm801.c alsa driver does: have the radio functionality as an i2c driver. In fact, it would not surprise me at all if you could use the tea575x-tuner.c driver (in sound/i2c/other) for the es1968 and delete the radio-maestro altogether. I guess simply porting radio-maestro codes into snd-es1968 would work without much hustles, and it's a bit safe way to go for now; smaller changes have less chance for breakage, and as little people seem using this driver, it'd be better to take a safer option, IMO. If we have active testers for both devices, it's nicer to go forward to clean-up works indeed, though. thanks, Takashi Both are for the tea575x tuner, although radio-maestro seems to have better support for the g_tuner operation. It doesn't seem difficult to add that to tea575x-tuner.c. The fm801 code for driving the tea575x is pretty horrible and it should be possible to improve that. I suspect that those read/write/mute functions really belong in tea575x-tuner.c and that only the low-level gpio actions need to be in the fm801/es1968 drivers. Hope this helps. Regards, Hans BTW: if anyone has spare hardware for testing the radio-maestro/tea575x-tuner, then I'm interested. -- Hans Verkuil - video4linux developer - sponsored by Cisco ___ Alsa-devel mailing list alsa-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] radio-maestro broken (conflicts with snd-es1968)
At Mon, 14 Mar 2011 11:28:01 +0100, Ondrej Zary wrote: On Monday 14 March 2011, Hans Verkuil wrote: At Sat, 12 Mar 2011 19:52:39 +0100, Hans Verkuil wrote: On Saturday, March 12, 2011 19:19:00 Ondrej Zary wrote: Hello, the radio-maestro driver is badly broken. It's intended to drive the radio on MediaForte ESS Maestro-based sound cards with integrated radio (like SF64-PCE2-04). But it conflicts with snd_es1968, ALSA driver for the sound chip itself. If one driver is loaded, the other one does not work - because a driver is already registered for the PCI device (there is only one). This was probably broken by conversion of PCI probing in 2006: ttp://lkml.org/lkml/2005/12/31/93 How to fix it properly? Include radio functionality in snd-es1968 and delete radio-maestro? Interesting. I don't know anyone among the video4linux developers who has this hardware, so the radio-maestro driver hasn't been tested in at least 6 or 7 years. The proper fix would be to do it like the fm801.c alsa driver does: have the radio functionality as an i2c driver. In fact, it would not surprise me at all if you could use the tea575x-tuner.c driver (in sound/i2c/other) for the es1968 and delete the radio-maestro altogether. I guess simply porting radio-maestro codes into snd-es1968 would work without much hustles, and it's a bit safe way to go for now; smaller changes have less chance for breakage, and as little people seem using this driver, it'd be better to take a safer option, IMO. I assume someone has hardware since someone reported this breakage. So try to use tuner-tea575x for the es1968. It shouldn't be too difficult. Additional cleanup should probably wait until we find a tester for the fm801 as well. I have the hardware - both ES1968 (SF64-PCE2-04) and FM801 cards (SF64-PCR) with these tuners. I remember fixing mute in tea5757x-tuner back in 2009 (testing it on SF64-PCR). I don't like the idea to duplicate code. I don't like that either. I've done a quick hack - copied radio support from fm801 and radio_bits_get() and radio_bits_set() from radio-maestro to es1968 and it seems to basically work. Now I just need some more time to finish it, then move everything good from radio-maestro to tea575x-tuner and delete radio-maestro. Great! Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] tea575x-tuner: various improvements
At Sat, 19 Mar 2011 16:32:53 +0100, Ondrej Zary wrote: Improve tea575x-tuner with various good things from radio-maestro: - extend frequency range to 50-150MHz - fix querycap(): card name, CAP_RADIO - improve g_tuner(): CAP_STEREO, stereo and tuned indication - improve g_frequency(): tuner index checking and reading frequency from HW - improve s_frequency(): tuner index and type checking Signed-off-by: Ondrej Zary li...@rainbow-software.org Applied these 3 patches now to topic/misc branch of sound git tree (i.e. for 2.6.40 kernel). I leave the removal of radio-maestro to v4l guys, as this is fairly independent. thanks, Takashi --- linux-2.6.38-rc4-orig/sound/i2c/other/tea575x-tuner.c 2011-02-08 01:03:55.0 +0100 +++ linux-2.6.38-rc4/sound/i2c/other/tea575x-tuner.c 2011-03-19 15:40:14.0 +0100 @@ -37,8 +37,8 @@ static int radio_nr = -1; module_param(radio_nr, int, 0); #define RADIO_VERSION KERNEL_VERSION(0, 0, 2) -#define FREQ_LO (87 * 16000) -#define FREQ_HI (108 * 16000) +#define FREQ_LO (50UL * 16000) +#define FREQ_HI (150UL * 16000) /* * definitions @@ -77,15 +77,29 @@ static struct v4l2_queryctrl radio_qctrl * lowlevel part */ +static void snd_tea575x_get_freq(struct snd_tea575x *tea) +{ + unsigned long freq; + + freq = tea-ops-read(tea) TEA575X_BIT_FREQ_MASK; + /* freq *= 12.5 */ + freq *= 125; + freq /= 10; + /* crystal fixup */ + if (tea-tea5759) + freq += tea-freq_fixup; + else + freq -= tea-freq_fixup; + + tea-freq = freq * 16; /* from kHz */ +} + static void snd_tea575x_set_freq(struct snd_tea575x *tea) { unsigned long freq; - freq = tea-freq / 16; /* to kHz */ - if (freq 108000) - freq = 108000; - if (freq 87000) - freq = 87000; + freq = clamp(tea-freq, FREQ_LO, FREQ_HI); + freq /= 16; /* to kHz */ /* crystal fixup */ if (tea-tea5759) freq -= tea-freq_fixup; @@ -109,29 +123,33 @@ static int vidioc_querycap(struct file * { struct snd_tea575x *tea = video_drvdata(file); - strcpy(v-card, tea-tea5759 ? TEA5759 : TEA5757); strlcpy(v-driver, tea575x-tuner, sizeof(v-driver)); - strlcpy(v-card, Maestro Radio, sizeof(v-card)); + strlcpy(v-card, tea-tea5759 ? TEA5759 : TEA5757, sizeof(v-card)); sprintf(v-bus_info, PCI); v-version = RADIO_VERSION; - v-capabilities = V4L2_CAP_TUNER; + v-capabilities = V4L2_CAP_TUNER | V4L2_CAP_RADIO; return 0; } static int vidioc_g_tuner(struct file *file, void *priv, struct v4l2_tuner *v) { + struct snd_tea575x *tea = video_drvdata(file); + if (v-index 0) return -EINVAL; + tea-ops-read(tea); + strcpy(v-name, FM); v-type = V4L2_TUNER_RADIO; + v-capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO; v-rangelow = FREQ_LO; v-rangehigh = FREQ_HI; - v-rxsubchans = V4L2_TUNER_SUB_MONO|V4L2_TUNER_SUB_STEREO; - v-capability = V4L2_TUNER_CAP_LOW; - v-audmode = V4L2_TUNER_MODE_MONO; - v-signal = 0x; + v-rxsubchans = V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO; + v-audmode = tea-stereo ? V4L2_TUNER_MODE_STEREO : V4L2_TUNER_MODE_MONO; + v-signal = tea-tuned ? 0x : 0; + return 0; } @@ -148,7 +166,10 @@ static int vidioc_g_frequency(struct fil { struct snd_tea575x *tea = video_drvdata(file); + if (f-tuner != 0) + return -EINVAL; f-type = V4L2_TUNER_RADIO; + snd_tea575x_get_freq(tea); f-frequency = tea-freq; return 0; } @@ -158,6 +179,9 @@ static int vidioc_s_frequency(struct fil { struct snd_tea575x *tea = video_drvdata(file); + if (f-tuner != 0 || f-type != V4L2_TUNER_RADIO) + return -EINVAL; + if (f-frequency FREQ_LO || f-frequency FREQ_HI) return -EINVAL; --- linux-2.6.38-rc4-orig/include/sound/tea575x-tuner.h 2011-02-08 01:03:55.0 +0100 +++ linux-2.6.38-rc4/include/sound/tea575x-tuner.h2011-03-19 14:18:06.0 +0100 @@ -38,8 +38,10 @@ struct snd_tea575x { struct snd_card *card; struct video_device *vd;/* video device */ int dev_nr; /* requested device number + 1 */ - int tea5759;/* 5759 chip is present */ - int mute; /* Device is muted? */ + bool tea5759; /* 5759 chip is present */ + bool mute; /* Device is muted? */ + bool stereo;/* receiving stereo */ + bool tuned; /* tuned to a station */ unsigned int freq_fixup;/* crystal onboard */ unsigned int val;
Re: [RFC PATCH 4/3] remove radio-maestro
At Tue, 22 Mar 2011 15:44:05 -0300, Mauro Carvalho Chehab wrote: Hi Takashi, Em 19-03-2011 13:23, Ondrej Zary escreveu: Remove broken radio-maestro driver as the radio functionality is now integrated in the es1968 driver. I prefer if you could also add it on your tree, as we want to make sure that this patch will be applied together with the patches that moved Maestro support into the es1968 driver. I have no means to test if the es1968 changes work with a Maestro radio (as I don't have this hardware), but I'm OK with this change. So: Acked-by: Mauro Carvalho Chehab mche...@redhat.com OK, I removed it in my tree now. Thanks! Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 1/3] tea575x: unify read/write functions
At Mon, 9 May 2011 23:39:26 +0200, Ondrej Zary wrote: Implement generic read/write functions to access TEA575x tuners. They're now implemented 4 times (once in es1968 and 3 times in fm801). This also allows mute to work on all cards. Also improve tuner detection/initialization. Signed-off-by: Ondrej Zary li...@rainbow-software.org Applied all three patches to sound git tree. Thanks! Takashi --- linux-2.6.39-rc2-/include/sound/tea575x-tuner.h 2011-04-29 22:48:34.0 +0200 +++ linux-2.6.39-rc2/include/sound/tea575x-tuner.h2011-05-06 22:20:46.0 +0200 @@ -26,12 +26,17 @@ #include media/v4l2-dev.h #include media/v4l2-ioctl.h +#define TEA575X_DATA (1 0) +#define TEA575X_CLK (1 1) +#define TEA575X_WREN (1 2) +#define TEA575X_MOST (1 3) + struct snd_tea575x; struct snd_tea575x_ops { - void (*write)(struct snd_tea575x *tea, unsigned int val); - unsigned int (*read)(struct snd_tea575x *tea); - void (*mute)(struct snd_tea575x *tea, unsigned int mute); + void (*set_pins)(struct snd_tea575x *tea, u8 pins); + u8 (*get_pins)(struct snd_tea575x *tea); + void (*set_direction)(struct snd_tea575x *tea, bool output); }; struct snd_tea575x { @@ -49,7 +54,7 @@ struct snd_tea575x { void *private_data; }; -void snd_tea575x_init(struct snd_tea575x *tea); +int snd_tea575x_init(struct snd_tea575x *tea); void snd_tea575x_exit(struct snd_tea575x *tea); #endif /* __SOUND_TEA575X_TUNER_H */ --- linux-2.6.39-rc2-/sound/i2c/other/tea575x-tuner.c 2011-05-06 22:44:14.0 +0200 +++ linux-2.6.39-rc2/sound/i2c/other/tea575x-tuner.c 2011-05-06 22:21:09.0 +0200 @@ -77,11 +77,65 @@ static struct v4l2_queryctrl radio_qctrl * lowlevel part */ +static void snd_tea575x_write(struct snd_tea575x *tea, unsigned int val) +{ + u16 l; + u8 data; + + tea-ops-set_direction(tea, 1); + udelay(16); + + for (l = 25; l 0; l--) { + data = (val 24) TEA575X_DATA; + val = 1; /* shift data */ + tea-ops-set_pins(tea, data | TEA575X_WREN); + udelay(2); + tea-ops-set_pins(tea, data | TEA575X_WREN | TEA575X_CLK); + udelay(2); + tea-ops-set_pins(tea, data | TEA575X_WREN); + udelay(2); + } + + if (!tea-mute) + tea-ops-set_pins(tea, 0); +} + +static unsigned int snd_tea575x_read(struct snd_tea575x *tea) +{ + u16 l, rdata; + u32 data = 0; + + tea-ops-set_direction(tea, 0); + tea-ops-set_pins(tea, 0); + udelay(16); + + for (l = 24; l--;) { + tea-ops-set_pins(tea, TEA575X_CLK); + udelay(2); + if (!l) + tea-tuned = tea-ops-get_pins(tea) TEA575X_MOST ? 0 : 1; + tea-ops-set_pins(tea, 0); + udelay(2); + data = 1; /* shift data */ + rdata = tea-ops-get_pins(tea); + if (!l) + tea-stereo = (rdata TEA575X_MOST) ? 0 : 1; + if (rdata TEA575X_DATA) + data++; + udelay(2); + } + + if (tea-mute) + tea-ops-set_pins(tea, TEA575X_WREN); + + return data; +} + static void snd_tea575x_get_freq(struct snd_tea575x *tea) { unsigned long freq; - freq = tea-ops-read(tea) TEA575X_BIT_FREQ_MASK; + freq = snd_tea575x_read(tea) TEA575X_BIT_FREQ_MASK; /* freq *= 12.5 */ freq *= 125; freq /= 10; @@ -111,7 +165,7 @@ static void snd_tea575x_set_freq(struct tea-val = ~TEA575X_BIT_FREQ_MASK; tea-val |= freq TEA575X_BIT_FREQ_MASK; - tea-ops-write(tea, tea-val); + snd_tea575x_write(tea, tea-val); } /* @@ -139,7 +193,7 @@ static int vidioc_g_tuner(struct file *f if (v-index 0) return -EINVAL; - tea-ops-read(tea); + snd_tea575x_read(tea); strcpy(v-name, FM); v-type = V4L2_TUNER_RADIO; @@ -233,10 +287,8 @@ static int vidioc_g_ctrl(struct file *fi switch (ctrl-id) { case V4L2_CID_AUDIO_MUTE: - if (tea-ops-mute) { - ctrl-value = tea-mute; - return 0; - } + ctrl-value = tea-mute; + return 0; } return -EINVAL; } @@ -248,11 +300,11 @@ static int vidioc_s_ctrl(struct file *fi switch (ctrl-id) { case V4L2_CID_AUDIO_MUTE: - if (tea-ops-mute) { - tea-ops-mute(tea, ctrl-value); + if (tea-mute != ctrl-value) { tea-mute = ctrl-value; - return 0; + snd_tea575x_set_freq(tea); } + return 0; } return -EINVAL; } @@ -317,18 +369,16 @@ static struct video_device tea575x_radio /* *
Re: [alsa-devel] fm801: implement TEA575x tuner autodetection
At Tue, 10 May 2011 23:24:15 +0200, Ondrej Zary wrote: Autodetect TEA575x tuner connection type during init. This allows tuner to work out-of-the box. tea575x_tuner module parameter remains functional to force tuner type. Tested with SF256-PCP and SF64-PCR. Signed-off-by: Ondrej Zary li...@rainbow-software.org Applied now. Thanks. Takashi --- linux-2.6.39-rc2-/sound/pci/fm801.c 2011-05-10 22:31:45.0 +0200 +++ linux-2.6.39-rc2/sound/pci/fm801.c2011-05-10 23:21:42.0 +0200 @@ -53,7 +53,7 @@ static int enable[SNDRV_CARDS] = SNDRV_D /* * Enable TEA575x tuner *1 = MediaForte 256-PCS - *2 = MediaForte 256-PCPR + *2 = MediaForte 256-PCP *3 = MediaForte 64-PCR * 16 = setup tuner only (this is additional bit), i.e. SF64-PCR FM card * High 16-bits are video (radio) device number + 1 @@ -67,7 +67,7 @@ MODULE_PARM_DESC(id, ID string for the module_param_array(enable, bool, NULL, 0444); MODULE_PARM_DESC(enable, Enable FM801 soundcard.); module_param_array(tea575x_tuner, int, NULL, 0444); -MODULE_PARM_DESC(tea575x_tuner, TEA575x tuner access method (1 = SF256-PCS, 2=SF256-PCPR, 3=SF64-PCR, +16=tuner-only).); +MODULE_PARM_DESC(tea575x_tuner, TEA575x tuner access method (0 = auto, 1 = SF256-PCS, 2=SF256-PCP, 3=SF64-PCR, 8=disable, +16=tuner-only).); #define TUNER_ONLY (14) #define TUNER_TYPE_MASK (~TUNER_ONLY 0x) @@ -720,12 +720,13 @@ static int __devinit snd_fm801_pcm(struc /* GPIO to TEA575x maps */ struct snd_fm801_tea575x_gpio { u8 data, clk, wren, most; + char *name; }; static struct snd_fm801_tea575x_gpio snd_fm801_tea575x_gpios[] = { - { .data = 1, .clk = 3, .wren = 2, .most = 0 }, /* SF256-PCS */ - { .data = 1, .clk = 0, .wren = 2, .most = 3 }, /* SF256-PCP */ - { .data = 2, .clk = 0, .wren = 1, .most = 3 }, /* SF64-PCR */ + { .data = 1, .clk = 3, .wren = 2, .most = 0, .name = SF256-PCS }, + { .data = 1, .clk = 0, .wren = 2, .most = 3, .name = SF256-PCP }, + { .data = 2, .clk = 0, .wren = 1, .most = 3, .name = SF64-PCR }, }; static void snd_fm801_tea575x_set_pins(struct snd_tea575x *tea, u8 pins) @@ -1229,14 +1230,24 @@ static int __devinit snd_fm801_create(st snd_card_set_dev(card, pci-dev); #ifdef TEA575X_RADIO + chip-tea.card = card; + chip-tea.freq_fixup = 10700; + chip-tea.private_data = chip; + chip-tea.ops = snd_fm801_tea_ops; if ((tea575x_tuner TUNER_TYPE_MASK) 0 (tea575x_tuner TUNER_TYPE_MASK) 4) { - chip-tea.card = card; - chip-tea.freq_fixup = 10700; - chip-tea.private_data = chip; - chip-tea.ops = snd_fm801_tea_ops; - snd_tea575x_init(chip-tea); - } + if (snd_tea575x_init(chip-tea)) + snd_printk(KERN_ERR TEA575x radio not found\n); + } else if ((tea575x_tuner TUNER_TYPE_MASK) == 0) + /* autodetect tuner connection */ + for (tea575x_tuner = 1; tea575x_tuner = 3; tea575x_tuner++) { + chip-tea575x_tuner = tea575x_tuner; + if (!snd_tea575x_init(chip-tea)) { + snd_printk(KERN_INFO detected TEA575x radio type %s\n, + snd_fm801_tea575x_gpios[tea575x_tuner - 1].name); + break; + } + } #endif *rchip = chip; -- Ondrej Zary ___ Alsa-devel mailing list alsa-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH] fm801: clean-up radio-related Kconfig
At Fri, 13 May 2011 20:26:38 +0200, Ondrej Zary wrote: Change the weird SND_FM801_TEA575X_BOOL define in Kconfig to SND_FM801_RADIO and remove TEA575X_RADIO define from fm801.c. Well, this config is used over years, and if it's about only renaming, it won't be worth enough. Removing TEA575X_RADIO in fm801.c is fine, though. thanks, Takashi Also update help text to include all supported cards. Signed-off-by: Ondrej Zary li...@rainbow-software.org --- linux-2.6.39-rc2-/sound/pci/Kconfig 2011-05-13 19:36:27.0 +0200 +++ linux-2.6.39-rc2/sound/pci/Kconfig2011-05-13 19:23:00.0 +0200 @@ -554,18 +554,18 @@ config SND_FM801 To compile this driver as a module, choose M here: the module will be called snd-fm801. -config SND_FM801_TEA575X_BOOL +config SND_FM801_RADIO bool ForteMedia FM801 + TEA5757 tuner depends on SND_FM801 depends on VIDEO_V4L2=y || VIDEO_V4L2=SND_FM801 help Say Y here to include support for soundcards based on the ForteMedia - FM801 chip with a TEA5757 tuner connected to GPIO1-3 pins (Media - Forte SF256-PCS-02) into the snd-fm801 driver. + FM801 chip with a TEA5757 tuner (MediaForte SF256-PCS, SF256-PCP and + SF64-PCR) into the snd-fm801 driver. config SND_TEA575X tristate - depends on SND_FM801_TEA575X_BOOL || SND_ES1968_RADIO + depends on SND_FM801_RADIO || SND_ES1968_RADIO default SND_FM801 || SND_ES1968 source sound/pci/hda/Kconfig --- linux-2.6.39-rc2-/sound/pci/fm801.c 2011-05-13 19:39:23.0 +0200 +++ linux-2.6.39-rc2/sound/pci/fm801.c2011-05-13 19:22:20.0 +0200 @@ -36,9 +36,8 @@ #include asm/io.h -#ifdef CONFIG_SND_FM801_TEA575X_BOOL +#ifdef CONFIG_SND_FM801_RADIO #include sound/tea575x-tuner.h -#define TEA575X_RADIO 1 #endif MODULE_AUTHOR(Jaroslav Kysela pe...@perex.cz); @@ -196,7 +195,7 @@ struct fm801 { spinlock_t reg_lock; struct snd_info_entry *proc_entry; -#ifdef TEA575X_RADIO +#ifdef CONFIG_SND_FM801_RADIO struct snd_tea575x tea; #endif @@ -715,7 +714,7 @@ static int __devinit snd_fm801_pcm(struc * TEA5757 radio */ -#ifdef TEA575X_RADIO +#ifdef CONFIG_SND_FM801_RADIO /* GPIO to TEA575x maps */ struct snd_fm801_tea575x_gpio { @@ -1150,7 +1149,7 @@ static int snd_fm801_free(struct fm801 * outw(cmdw, FM801_REG(chip, IRQ_MASK)); __end_hw: -#ifdef TEA575X_RADIO +#ifdef CONFIG_SND_FM801_RADIO snd_tea575x_exit(chip-tea); #endif if (chip-irq = 0) @@ -1229,7 +1228,7 @@ static int __devinit snd_fm801_create(st snd_card_set_dev(card, pci-dev); -#ifdef TEA575X_RADIO +#ifdef CONFIG_SND_FM801_RADIO chip-tea.private_data = chip; chip-tea.ops = snd_fm801_tea_ops; sprintf(chip-tea.bus_info, PCI:%s, pci_name(pci)); -- Ondrej Zary ___ Alsa-devel mailing list alsa-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 1/3] tea575x: remove freq_fixup from struct
At Thu, 12 May 2011 22:17:56 +0200, Ondrej Zary wrote: freq_fixup is a constant, no need to hold it in struct snd_tea575x and set in each driver. Signed-off-by: Ondrej Zary li...@rainbow-software.org Thanks, applied all three patches. Takashi --- linux-2.6.39-rc2-/include/sound/tea575x-tuner.h 2011-05-10 22:31:40.0 +0200 +++ linux-2.6.39-rc2/include/sound/tea575x-tuner.h2011-05-12 21:00:50.0 +0200 @@ -26,6 +26,8 @@ #include media/v4l2-dev.h #include media/v4l2-ioctl.h +#define TEA575X_FMIF 10700 + #define TEA575X_DATA (1 0) #define TEA575X_CLK (1 1) #define TEA575X_WREN (1 2) @@ -46,7 +48,6 @@ struct snd_tea575x { bool mute; /* Device is muted? */ bool stereo;/* receiving stereo */ bool tuned; /* tuned to a station */ - unsigned int freq_fixup;/* crystal onboard */ unsigned int val; /* hw value */ unsigned long freq; /* frequency */ unsigned long in_use; /* set if the device is in use */ --- linux-2.6.39-rc2-/sound/i2c/other/tea575x-tuner.c 2011-05-10 22:31:40.0 +0200 +++ linux-2.6.39-rc2/sound/i2c/other/tea575x-tuner.c 2011-05-12 21:00:37.0 +0200 @@ -141,9 +141,9 @@ static void snd_tea575x_get_freq(struct freq /= 10; /* crystal fixup */ if (tea-tea5759) - freq += tea-freq_fixup; + freq += TEA575X_FMIF; else - freq -= tea-freq_fixup; + freq -= TEA575X_FMIF; tea-freq = freq * 16; /* from kHz */ } @@ -156,9 +156,9 @@ static void snd_tea575x_set_freq(struct freq /= 16; /* to kHz */ /* crystal fixup */ if (tea-tea5759) - freq -= tea-freq_fixup; + freq -= TEA575X_FMIF; else - freq += tea-freq_fixup; + freq += TEA575X_FMIF; /* freq /= 12.5 */ freq *= 10; freq /= 125; --- linux-2.6.39-rc2-/sound/pci/es1968.c 2011-05-10 22:31:43.0 +0200 +++ linux-2.6.39-rc2/sound/pci/es1968.c 2011-05-10 23:47:32.0 +0200 @@ -2794,7 +2794,6 @@ static int __devinit snd_es1968_create(s #ifdef CONFIG_SND_ES1968_RADIO chip-tea.card = card; - chip-tea.freq_fixup = 10700; chip-tea.private_data = chip; chip-tea.ops = snd_es1968_tea_ops; if (!snd_tea575x_init(chip-tea)) --- linux-2.6.39-rc2-/sound/pci/fm801.c 2011-05-10 23:24:39.0 +0200 +++ linux-2.6.39-rc2/sound/pci/fm801.c2011-05-10 23:47:43.0 +0200 @@ -1231,7 +1231,6 @@ static int __devinit snd_fm801_create(st #ifdef TEA575X_RADIO chip-tea.card = card; - chip-tea.freq_fixup = 10700; chip-tea.private_data = chip; chip-tea.ops = snd_fm801_tea_ops; if ((tea575x_tuner TUNER_TYPE_MASK) 0 -- Ondrej Zary ___ Alsa-devel mailing list alsa-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH RFC] radio-sf16fmr2: convert to generic TEA575x interface
At Sat, 14 May 2011 00:17:23 +0200, Ondrej Zary wrote: Hello, the SF16-FMR2 radio card is basically a TEA5757 tuner connected to ISA bus but the driver currently uses its own implementation. I converted the driver to use generic tea575x-tuner implementation (see patch below) and it works fine. But the tea575x-tuner module is located in /sound/i2c/other and the corresponding Kconfig in /sound/pci. Should it be moved to /drivers/media/radio or better leave it as is? That would be a feasible option. Actually no ALSA-specific data / function is used in tea575x-tuner.c, so it'd be more natural to put it to drivers/media/radio. (And remove the inclusion of sound/* from relevant files.) thanks, Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] fm801: clean-up radio-related Kconfig
At Sat, 14 May 2011 22:51:01 +0200, Ondrej Zary wrote: Remove TEA575X_RADIO define from fm801.c. Also update Kconfig help text to include all supported cards. Signed-off-by: Ondrej Zary li...@rainbow-software.org Thanks, applied now. Takashi --- linux-2.6.39-rc2-/sound/pci/Kconfig 2011-05-14 22:22:11.0 +0200 +++ linux-2.6.39-rc2/sound/pci/Kconfig2011-05-14 22:24:29.0 +0200 @@ -560,8 +560,8 @@ config SND_FM801_TEA575X_BOOL depends on VIDEO_V4L2=y || VIDEO_V4L2=SND_FM801 help Say Y here to include support for soundcards based on the ForteMedia - FM801 chip with a TEA5757 tuner connected to GPIO1-3 pins (Media - Forte SF256-PCS-02) into the snd-fm801 driver. + FM801 chip with a TEA5757 tuner (MediaForte SF256-PCS, SF256-PCP and + SF64-PCR) into the snd-fm801 driver. config SND_TEA575X tristate --- linux-2.6.39-rc2-/sound/pci/fm801.c 2011-05-14 22:22:11.0 +0200 +++ linux-2.6.39-rc2/sound/pci/fm801.c2011-05-14 22:26:01.0 +0200 @@ -38,7 +38,6 @@ #ifdef CONFIG_SND_FM801_TEA575X_BOOL #include sound/tea575x-tuner.h -#define TEA575X_RADIO 1 #endif MODULE_AUTHOR(Jaroslav Kysela pe...@perex.cz); @@ -196,7 +195,7 @@ struct fm801 { spinlock_t reg_lock; struct snd_info_entry *proc_entry; -#ifdef TEA575X_RADIO +#ifdef CONFIG_SND_FM801_TEA575X_BOOL struct snd_tea575x tea; #endif @@ -715,7 +714,7 @@ static int __devinit snd_fm801_pcm(struc * TEA5757 radio */ -#ifdef TEA575X_RADIO +#ifdef CONFIG_SND_FM801_TEA575X_BOOL /* GPIO to TEA575x maps */ struct snd_fm801_tea575x_gpio { @@ -1150,7 +1149,7 @@ static int snd_fm801_free(struct fm801 * outw(cmdw, FM801_REG(chip, IRQ_MASK)); __end_hw: -#ifdef TEA575X_RADIO +#ifdef CONFIG_SND_FM801_TEA575X_BOOL snd_tea575x_exit(chip-tea); #endif if (chip-irq = 0) @@ -1229,7 +1228,7 @@ static int __devinit snd_fm801_create(st snd_card_set_dev(card, pci-dev); -#ifdef TEA575X_RADIO +#ifdef CONFIG_SND_FM801_TEA575X_BOOL chip-tea.private_data = chip; chip-tea.ops = snd_fm801_tea_ops; sprintf(chip-tea.bus_info, PCI:%s, pci_name(pci)); -- Ondrej Zary -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH v5] [resend] radio-sf16fmr2: convert to generic TEA575x interface
At Wed, 25 May 2011 21:21:30 -0300, Mauro Carvalho Chehab wrote: Em 23-05-2011 09:17, Ondrej Zary escreveu: Convert radio-sf16fmr2 to use generic TEA575x implementation. Most of the driver code goes away as SF16-FMR2 is basically just a TEA5757 tuner connected to ISA bus. The card can optionally be equipped with PT2254A volume control (equivalent of TC9154AP) - the volume setting is completely reworked (with balance control added) and tested. Ondrej, As your first series went via alsa tree, and we are close to the end of the merge window, and assuming that Takashi didn't apply those patches on his tree, as you're re-sending it, I think that the better is to wait for the end of the merge window, in order to allow us to sync our development tree with 2.6.40-rc1, and then review and apply it on the top of it. Yeah, I didn't pick it up as the patches are rather V4L-side changes (although tea575x.c is in sound sub-directory). And I agree with Mauro - let's merge it after rc1, so that we stand on the same ground. This sort of cross-tree change is better done at the fixed point than in flux like during merge window. That being said, I don't mind that Mauro or Hans applies these through V4L tree. In that case, you can take my acks for both patches. Acked-by: Takashi Iwai ti...@suse.de Of if it's preferred through sound tree, I can take them later. thanks, Takashi Thanks, Mauro. Signed-off-by: Ondrej Zary li...@rainbow-software.org --- linux-2.6.39-rc2-/sound/pci/Kconfig 2011-05-15 18:50:18.0 +0200 +++ linux-2.6.39-rc2/sound/pci/Kconfig 2011-05-17 23:35:30.0 +0200 @@ -565,8 +565,8 @@ config SND_FM801_TEA575X_BOOL config SND_TEA575X tristate - depends on SND_FM801_TEA575X_BOOL || SND_ES1968_RADIO - default SND_FM801 || SND_ES1968 + depends on SND_FM801_TEA575X_BOOL || SND_ES1968_RADIO || RADIO_SF16FMR2 + default SND_FM801 || SND_ES1968 || RADIO_SF16FMR2 source sound/pci/hda/Kconfig --- linux-2.6.39-rc2-/drivers/media/radio/radio-sf16fmr2.c 2011-04-06 03:30:43.0 +0200 +++ linux-2.6.39-rc2/drivers/media/radio/radio-sf16fmr2.c 2011-05-19 17:56:08.0 +0200 @@ -1,441 +1,209 @@ -/* SF16FMR2 radio driver for Linux radio support - * heavily based on fmi driver... - * (c) 2000-2002 Ziglio Frediano, fredd...@angelfire.com +/* SF16-FMR2 radio driver for Linux + * Copyright (c) 2011 Ondrej Zary * - * Notes on the hardware - * - * Frequency control is done digitally -- ie out(port,encodefreq(95.8)); - * No volume control - only mute/unmute - you have to use line volume - * - * For read stereo/mono you must wait 0.1 sec after set frequency and - * card unmuted so I set frequency on unmute - * Signal handling seem to work only on autoscanning (not implemented) - * - * Converted to V4L2 API by Mauro Carvalho Chehab mche...@infradead.org + * Original driver was (c) 2000-2002 Ziglio Frediano, fredd...@angelfire.com + * but almost nothing remained here after conversion to generic TEA575x + * implementation */ +#include linux/delay.h #include linux/module.h /* Modules */ #include linux/init.h/* Initdata */ #include linux/ioport.h /* request_region */ -#include linux/delay.h /* udelay */ -#include linux/videodev2.h /* kernel radio structs */ -#include linux/mutex.h -#include linux/version.h /* for KERNEL_VERSION MACRO */ #include linux/io.h /* outb, outb_p */ -#include media/v4l2-device.h -#include media/v4l2-ioctl.h +#include sound/tea575x-tuner.h -MODULE_AUTHOR(Ziglio Frediano, fredd...@angelfire.com); -MODULE_DESCRIPTION(A driver for the SF16FMR2 radio.); +MODULE_AUTHOR(Ondrej Zary); +MODULE_DESCRIPTION(MediaForte SF16-FMR2 FM radio card driver); MODULE_LICENSE(GPL); -static int io = 0x384; -static int radio_nr = -1; - -module_param(io, int, 0); -MODULE_PARM_DESC(io, I/O address of the SF16FMR2 card (should be 0x384, if do not work try 0x284)); -module_param(radio_nr, int, 0); - -#define RADIO_VERSION KERNEL_VERSION(0,0,2) - -#define AUD_VOL_INDEX 1 - -#undef DEBUG -//#define DEBUG 1 - -#ifdef DEBUG -# define debug_print(s) printk s -#else -# define debug_print(s) -#endif - -/* this should be static vars for module size */ -struct fmr2 -{ - struct v4l2_device v4l2_dev; - struct video_device vdev; - struct mutex lock; +struct fmr2 { int io; - int curvol; /* 0-15 */ - int mute; - int stereo; /* card is producing stereo audio */ - unsigned long curfreq; /* freq in kHz */ - int card_type; + struct snd_tea575x tea; + struct v4l2_ctrl *volume; + struct v4l2_ctrl *balance; }; +/* the port is hardwired so no need to support
Re: [alsa-devel] [PATCH] tea575x: allow multiple opens
At Sat, 11 Jun 2011 15:36:39 +0200, Hans Verkuil wrote: On Saturday, June 11, 2011 15:28:59 Ondrej Zary wrote: Change locking to allow tea575x-radio device to be opened multiple times. Very nice! Signed-off-by: Hans Verkuil hverk...@xs4all.nl Hans, would you apply this and another one via v4l tree or shall I apply them via sound tree? Takashi Regards, Hans Signed-off-by: Ondrej Zary li...@rainbow-software.org --- linux-2.6.39-rc2-/include/sound/tea575x-tuner.h 2011-06-11 15:21:50.0 +0200 +++ linux-2.6.39-rc2/include/sound/tea575x-tuner.h 2011-06-11 14:50:55.0 +0200 @@ -49,7 +49,7 @@ struct snd_tea575x { bool tuned; /* tuned to a station */ unsigned int val; /* hw value */ unsigned long freq; /* frequency */ - unsigned long in_use; /* set if the device is in use */ + struct mutex mutex; struct snd_tea575x_ops *ops; void *private_data; u8 card[32]; --- linux-2.6.39-rc2-/sound/i2c/other/tea575x-tuner.c 2011-06-11 15:21:50.0 +0200 +++ linux-2.6.39-rc2/sound/i2c/other/tea575x-tuner.c2011-06-11 14:57:28.0 +0200 @@ -282,26 +282,9 @@ static int vidioc_s_input(struct file *f return 0; } -static int snd_tea575x_exclusive_open(struct file *file) -{ - struct snd_tea575x *tea = video_drvdata(file); - - return test_and_set_bit(0, tea-in_use) ? -EBUSY : 0; -} - -static int snd_tea575x_exclusive_release(struct file *file) -{ - struct snd_tea575x *tea = video_drvdata(file); - - clear_bit(0, tea-in_use); - return 0; -} - static const struct v4l2_file_operations tea575x_fops = { .owner = THIS_MODULE, - .open = snd_tea575x_exclusive_open, - .release= snd_tea575x_exclusive_release, - .ioctl = video_ioctl2, + .unlocked_ioctl = video_ioctl2, }; static const struct v4l2_ioctl_ops tea575x_ioctl_ops = { @@ -340,13 +323,14 @@ int snd_tea575x_init(struct snd_tea575x if (snd_tea575x_read(tea) != 0x55AA) return -ENODEV; - tea-in_use = 0; tea-val = TEA575X_BIT_BAND_FM | TEA575X_BIT_SEARCH_10_40; tea-freq = 90500 * 16; /* 90.5Mhz default */ snd_tea575x_set_freq(tea); tea-vd = tea575x_radio; video_set_drvdata(tea-vd, tea); + mutex_init(tea-mutex); + tea-vd.lock = tea-mutex; v4l2_ctrl_handler_init(tea-ctrl_handler, 1); tea-vd.ctrl_handler = tea-ctrl_handler; ___ Alsa-devel mailing list alsa-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH] tea575x: allow multiple opens
At Sun, 12 Jun 2011 19:48:47 +0200, Hans Verkuil wrote: On Sunday, June 12, 2011 18:52:56 Takashi Iwai wrote: At Sat, 11 Jun 2011 15:36:39 +0200, Hans Verkuil wrote: On Saturday, June 11, 2011 15:28:59 Ondrej Zary wrote: Change locking to allow tea575x-radio device to be opened multiple times. Very nice! Signed-off-by: Hans Verkuil hverk...@xs4all.nl Hans, would you apply this and another one via v4l tree or shall I apply them via sound tree? This is all V4L related, so I think Mauro can apply this patch and the other patch for v3.1. No need to go through an intermediate tree of mine. OK, good to know my work is shortened :) thanks, Takashi Regards, Hans Takashi Regards, Hans Signed-off-by: Ondrej Zary li...@rainbow-software.org --- linux-2.6.39-rc2-/include/sound/tea575x-tuner.h 2011-06-11 15:21:50.0 +0200 +++ linux-2.6.39-rc2/include/sound/tea575x-tuner.h 2011-06-11 14:50:55.0 +0200 @@ -49,7 +49,7 @@ struct snd_tea575x { bool tuned; /* tuned to a station */ unsigned int val; /* hw value */ unsigned long freq; /* frequency */ - unsigned long in_use; /* set if the device is in use */ + struct mutex mutex; struct snd_tea575x_ops *ops; void *private_data; u8 card[32]; --- linux-2.6.39-rc2-/sound/i2c/other/tea575x-tuner.c 2011-06-11 15:21:50.0 +0200 +++ linux-2.6.39-rc2/sound/i2c/other/tea575x-tuner.c2011-06-11 14:57:28.0 +0200 @@ -282,26 +282,9 @@ static int vidioc_s_input(struct file *f return 0; } -static int snd_tea575x_exclusive_open(struct file *file) -{ - struct snd_tea575x *tea = video_drvdata(file); - - return test_and_set_bit(0, tea-in_use) ? -EBUSY : 0; -} - -static int snd_tea575x_exclusive_release(struct file *file) -{ - struct snd_tea575x *tea = video_drvdata(file); - - clear_bit(0, tea-in_use); - return 0; -} - static const struct v4l2_file_operations tea575x_fops = { .owner = THIS_MODULE, - .open = snd_tea575x_exclusive_open, - .release= snd_tea575x_exclusive_release, - .ioctl = video_ioctl2, + .unlocked_ioctl = video_ioctl2, }; static const struct v4l2_ioctl_ops tea575x_ioctl_ops = { @@ -340,13 +323,14 @@ int snd_tea575x_init(struct snd_tea575x if (snd_tea575x_read(tea) != 0x55AA) return -ENODEV; - tea-in_use = 0; tea-val = TEA575X_BIT_BAND_FM | TEA575X_BIT_SEARCH_10_40; tea-freq = 90500 * 16; /* 90.5Mhz default */ snd_tea575x_set_freq(tea); tea-vd = tea575x_radio; video_set_drvdata(tea-vd, tea); + mutex_init(tea-mutex); + tea-vd.lock = tea-mutex; v4l2_ctrl_handler_init(tea-ctrl_handler, 1); tea-vd.ctrl_handler = tea-ctrl_handler; ___ Alsa-devel mailing list alsa-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SOUND: Fix non-ISA_DMA_API build failure
At Thu, 23 Jun 2011 15:47:50 +0100, Ralf Baechle wrote: A build with ISA ISA_DMA !ISA_DMA_API results in: CC sound/isa/es18xx.o sound/isa/es18xx.c: In function ‘snd_es18xx_playback1_prepare’: sound/isa/es18xx.c:501:9: error: implicit declaration of function ‘snd_dma_program’ [-Werror=implicit-function-declaration] sound/isa/es18xx.c: In function ‘snd_es18xx_playback_pointer’: sound/isa/es18xx.c:818:3: error: implicit declaration of function ‘snd_dma_pointer’ [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[2]: *** [sound/isa/es18xx.o] Error 1 CC sound/isa/sscape.o sound/isa/sscape.c: In function ‘upload_dma_data’: sound/isa/sscape.c:481:3: error: implicit declaration of function ‘snd_dma_program’ [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[2]: *** [sound/isa/sscape.o] Error 1 CC sound/isa/ad1816a/ad1816a_lib.o sound/isa/ad1816a/ad1816a_lib.c: In function ‘snd_ad1816a_playback_prepare’: sound/isa/ad1816a/ad1816a_lib.c:244:2: error: implicit declaration of function ‘snd_dma_program’ [-Werror=implicit-function-declaration] sound/isa/ad1816a/ad1816a_lib.c: In function ‘snd_ad1816a_playback_pointer’: sound/isa/ad1816a/ad1816a_lib.c:302:2: error: implicit declaration of function ‘snd_dma_pointer’ [-Werror=implicit-function-declaration] sound/isa/ad1816a/ad1816a_lib.c: In function ‘snd_ad1816a_free’: sound/isa/ad1816a/ad1816a_lib.c:544:3: error: implicit declaration of function ‘snd_dma_disable’ [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[3]: *** [sound/isa/ad1816a/ad1816a_lib.o] Error 1 make[3]: Target `__build' not remade because of errors. make[2]: *** [sound/isa/ad1816a] Error 2 CC sound/isa/es1688/es1688_lib.o sound/isa/es1688/es1688_lib.c: In function ‘snd_es1688_playback_prepare’: sound/isa/es1688/es1688_lib.c:417:2: error: implicit declaration of function ‘snd_dma_program’ [-Werror=implicit-function-declaration] sound/isa/es1688/es1688_lib.c: In function ‘snd_es1688_playback_pointer’: sound/isa/es1688/es1688_lib.c:509:2: error: implicit declaration of function ‘snd_dma_pointer’ [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[3]: *** [sound/isa/es1688/es1688_lib.o] Error 1 make[3]: Target `__build' not remade because of errors. make[2]: *** [sound/isa/es1688] Error 2 CC sound/isa/gus/gus_dma.o sound/isa/gus/gus_dma.c: In function ‘snd_gf1_dma_program’: sound/isa/gus/gus_dma.c:79:2: error: implicit declaration of function ‘snd_dma_program’ [-Werror=implicit-function-declaration] sound/isa/gus/gus_dma.c: In function ‘snd_gf1_dma_done’: sound/isa/gus/gus_dma.c:177:3: error: implicit declaration of function ‘snd_dma_disable’ [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[3]: *** [sound/isa/gus/gus_dma.o] Error 1 CC sound/isa/gus/gus_pcm.o sound/isa/gus/gus_pcm.c: In function ‘snd_gf1_pcm_capture_prepare’: sound/isa/gus/gus_pcm.c:591:2: error: implicit declaration of function ‘snd_dma_program’ [-Werror=implicit-function-declaration] sound/isa/gus/gus_pcm.c: In function ‘snd_gf1_pcm_capture_pointer’: sound/isa/gus/gus_pcm.c:619:2: error: implicit declaration of function ‘snd_dma_pointer’ [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[3]: *** [sound/isa/gus/gus_pcm.o] Error 1 make[3]: Target `__build' not remade because of errors. make[2]: *** [sound/isa/gus] Error 2 CC sound/isa/sb/sb16_csp.o sound/isa/sb/sb16_csp.c: In function ‘snd_sb_csp_ioctl’: sound/isa/sb/sb16_csp.c:228:227: error: case label does not reduce to an integer constant make[3]: *** [sound/isa/sb/sb16_csp.o] Error 1 CC sound/isa/sb/sb16_main.o sound/isa/sb/sb16_main.c: In function ‘snd_sb16_playback_prepare’: sound/isa/sb/sb16_main.c:276:2: error: implicit declaration of function ‘snd_dma_program’ [-Werror=implicit-function-declaration] sound/isa/sb/sb16_main.c: In function ‘snd_sb16_playback_pointer’: sound/isa/sb/sb16_main.c:456:2: error: implicit declaration of function ‘snd_dma_pointer’ [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[3]: *** [sound/isa/sb/sb16_main.o] Error 1 CC sound/isa/sb/sb8_main.o sound/isa/sb/sb8_main.c: In function ‘snd_sb8_playback_prepare’: sound/isa/sb/sb8_main.c:172:3: error: implicit declaration of function ‘snd_dma_program’ [-Werror=implicit-function-declaration] sound/isa/sb/sb8_main.c: In function ‘snd_sb8_playback_pointer’: sound/isa/sb/sb8_main.c:425:2: error: implicit declaration of function ‘snd_dma_pointer’ [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[3]: *** [sound/isa/sb/sb8_main.o] Error 1 make[3]: Target `__build' not remade because of errors. make[2]: *** [sound/isa/sb] Error 2 CC
Re: [PATCH] SOUND: Fix non-ISA_DMA_API build failure
At Fri, 24 Jun 2011 12:16:08 +0100, Ralf Baechle wrote: On Fri, Jun 24, 2011 at 10:26:13AM +0200, Takashi Iwai wrote: Hrm... I still don't understand why ES18XX or others were selected at the first place. Isn't it covered by the conditional in sound/isa/Kconfig like below? menuconfig SND_ISA bool ISA sound devices depends on ISA ISA_DMA_API ... if SND_ISA ... config SND_ES18XX tristate Generic ESS ES18xx driver ... endif # SND_ISA Isn't SND_ISA=n in your case although ISA_DMA_API=n? The answer is hidden in this Kconfig warning: warning: (RADIO_MIROPCM20) selects SND_ISA which has unmet direct dependencies (SOUND !M68K SND ISA ISA_DMA_API) This is due to the following in drivers/media/radio/Kconfig: config RADIO_MIROPCM20 tristate miroSOUND PCM20 radio depends on ISA VIDEO_V4L2 SND select SND_ISA select SND_MIRO So SND_ISA gets forced on even though the dependency on ISA_DMA_API is not fulfilled. That's solved by adding the dependency on ISA_DMA_API to RADIO_MIROPCM20. Ah, yeah, I see now. Also, adlib driver is really only for ISA, so I see no big reason to allow this built for non-ISA. With the patch applied: [...] menuconfig SND_ISA bool ISA sound devices depends on ISA [...] if SND_ISA config SND_ADLIB tristate AdLib FM card select SND_OPL3_LIB [...] So the Adlib driver will still only be built with ISA enabled. The only thing that makes the Adlib driver different from all the others in the ifdef SND_ISA ... endif bracket is that it does not directly or indirectly use the ISA DMA API and that's in the end the reason why sound/isa/Kconfig needs to be changed. I originally approach this a different way but now that I'm explaining the details I notice that it probably makes sense to split this patch into two: o The drivers/media/radio/Kconfig part should be applied for 3.0 and maybe -stable. Yes, this will be good. o The sound/isa/Kconfig part is basically only fixing the dependency for the Adlib driver allowing it to be built on non-ISA_DMA_API system and is material for the next release after 3.0. Any serious reason that snd-adlib must be built even with ISA=n? As the device is really present only for ISA, it doesn't make much sense to build this even though the driver itself doesn't need ISA_DMA_API. thanks, Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] SOUND: Fix non-ISA_DMA_API build failure
At Fri, 24 Jun 2011 14:08:18 +0100, Ralf Baechle wrote: On Fri, Jun 24, 2011 at 02:22:41PM +0200, Takashi Iwai wrote: o The drivers/media/radio/Kconfig part should be applied for 3.0 and maybe -stable. Yes, this will be good. I just tested that segment only and it works as expected. Will repost in a minute. Great, thanks. o The sound/isa/Kconfig part is basically only fixing the dependency for the Adlib driver allowing it to be built on non-ISA_DMA_API system and is material for the next release after 3.0. Any serious reason that snd-adlib must be built even with ISA=n? Definately not. As the device is really present only for ISA, it doesn't make much sense to build this even though the driver itself doesn't need ISA_DMA_API. I'm not aware of any systems that could use the Adlib in a ISA=n environment. That's why my patch left the dependency on ISA untouched. OK, but this would just make things complex, I'm afraid. Practically the only user of snd-adlib is the old x86 with ISA support, which implies ISA_DMA_API=y, too. Thus I don't want to touch sound/isa/Kconfig just for snd-adlib being built for some funky Kconfig setup :) thanks, Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH] MEDIA: Fix non-ISA_DMA_API link failure of sound code
sound/isa/wss/wss_lib.o sound/isa/wss/wss_lib.c: In function ‘snd_wss_playback_prepare’: sound/isa/wss/wss_lib.c:1025:2: error: implicit declaration of function ‘snd_dma_program’ [-Werror=implicit-function-declaration] sound/isa/wss/wss_lib.c: In function ‘snd_wss_playback_pointer’: sound/isa/wss/wss_lib.c:1160:2: error: implicit declaration of function ‘snd_dma_pointer’ [-Werror=implicit-function-declaration] sound/isa/wss/wss_lib.c: In function ‘snd_wss_free’: sound/isa/wss/wss_lib.c:1695:3: error: implicit declaration of function ‘snd_dma_disable’ [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[3]: *** [sound/isa/wss/wss_lib.o] Error 1 The root cause for this is hidden in this Kconfig warning: warning: (RADIO_MIROPCM20) selects SND_ISA which has unmet direct dependencies (SOUND !M68K SND ISA ISA_DMA_API) Adding a dependency on ISA_DMA_API to RADIO_MIROPCM20 fixes these issues. Signed-off-by: Ralf Baechle r...@linux-mips.org Acked-by: Takashi Iwai ti...@suse.de thanks, Takashi drivers/media/radio/Kconfig |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/radio/Kconfig b/drivers/media/radio/Kconfig index e4c97fd..0aeed28 100644 --- a/drivers/media/radio/Kconfig +++ b/drivers/media/radio/Kconfig @@ -168,7 +168,7 @@ config RADIO_MAXIRADIO config RADIO_MIROPCM20 tristate miroSOUND PCM20 radio - depends on ISA VIDEO_V4L2 SND + depends on ISA ISA_DMA_API VIDEO_V4L2 SND select SND_ISA select SND_MIRO ---help--- ___ Alsa-devel mailing list alsa-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: Tree for Aug 5 (media/radio/radio-sf16fmr2)
At Fri, 5 Aug 2011 16:56:11 -0700, Randy Dunlap wrote: On Fri, 5 Aug 2011 14:31:03 +1000 Stephen Rothwell wrote: Hi all, [The kernel.org mirroring is running slowly today] Is media/radio/radio-sf16fmr2 an ISA driver or a PCI driver? ugh. Or is it an I2C driver? linux-next fails with (this is not a new failure): ERROR: snd_tea575x_init [drivers/media/radio/radio-sf16fmr2.ko] undefined! ERROR: snd_tea575x_exit [drivers/media/radio/radio-sf16fmr2.ko] undefined! The Kconfig entry for RADIO_SF16FMR2 is: config RADIO_SF16FMR2 tristate SF16FMR2 Radio depends on ISA VIDEO_V4L2 SND and the Kconfig entry for SND_TEA575X is (not user visible): config SND_TEA575X tristate depends on SND_FM801_TEA575X_BOOL || SND_ES1968_RADIO || RADIO_SF16FMR2 default SND_FM801 || SND_ES1968 || RADIO_SF16FMR2 This latter entry is in sound/pci/Kconfig and is under: if SND_PCI so it depends on PCI and SND_PCI. This build fails when CONFIG_PCI is not enabled. tea575x-tuner is an i2c component (not meaning Linux i2c-subsystem), thus should be independent from the board bus type. Does a patch like below work? thanks, Takashi --- diff --git a/sound/pci/Kconfig b/sound/pci/Kconfig index 50abf5b..8816804 100644 --- a/sound/pci/Kconfig +++ b/sound/pci/Kconfig @@ -1,5 +1,10 @@ # ALSA PCI drivers +config SND_TEA575X + tristate + depends on SND_FM801_TEA575X_BOOL || SND_ES1968_RADIO || RADIO_SF16FMR2 + default SND_FM801 || SND_ES1968 || RADIO_SF16FMR2 + menuconfig SND_PCI bool PCI sound devices depends on PCI @@ -563,11 +568,6 @@ config SND_FM801_TEA575X_BOOL FM801 chip with a TEA5757 tuner (MediaForte SF256-PCS, SF256-PCP and SF64-PCR) into the snd-fm801 driver. -config SND_TEA575X - tristate - depends on SND_FM801_TEA575X_BOOL || SND_ES1968_RADIO || RADIO_SF16FMR2 - default SND_FM801 || SND_ES1968 || RADIO_SF16FMR2 - source sound/pci/hda/Kconfig config SND_HDSP -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: Tree for Aug 5 (media/radio/radio-sf16fmr2)
At Sat, 6 Aug 2011 09:50:04 -0700, Randy Dunlap wrote: On Sat, 06 Aug 2011 10:33:39 +0200 Takashi Iwai wrote: At Fri, 5 Aug 2011 16:56:11 -0700, Randy Dunlap wrote: On Fri, 5 Aug 2011 14:31:03 +1000 Stephen Rothwell wrote: Hi all, [The kernel.org mirroring is running slowly today] Is media/radio/radio-sf16fmr2 an ISA driver or a PCI driver? ugh. Or is it an I2C driver? linux-next fails with (this is not a new failure): ERROR: snd_tea575x_init [drivers/media/radio/radio-sf16fmr2.ko] undefined! ERROR: snd_tea575x_exit [drivers/media/radio/radio-sf16fmr2.ko] undefined! The Kconfig entry for RADIO_SF16FMR2 is: config RADIO_SF16FMR2 tristate SF16FMR2 Radio depends on ISA VIDEO_V4L2 SND and the Kconfig entry for SND_TEA575X is (not user visible): config SND_TEA575X tristate depends on SND_FM801_TEA575X_BOOL || SND_ES1968_RADIO || RADIO_SF16FMR2 default SND_FM801 || SND_ES1968 || RADIO_SF16FMR2 This latter entry is in sound/pci/Kconfig and is under: if SND_PCI so it depends on PCI and SND_PCI. This build fails when CONFIG_PCI is not enabled. tea575x-tuner is an i2c component (not meaning Linux i2c-subsystem), thus should be independent from the board bus type. Does a patch like below work? Yes, it does. Thanks. Reported-by: Randy Dunlap rdun...@xenotime.net Acked-by: Randy Dunlap rdun...@xenotime.net Thanks, now fixed in sound git tree. Takashi thanks, Takashi --- diff --git a/sound/pci/Kconfig b/sound/pci/Kconfig index 50abf5b..8816804 100644 --- a/sound/pci/Kconfig +++ b/sound/pci/Kconfig @@ -1,5 +1,10 @@ # ALSA PCI drivers +config SND_TEA575X + tristate + depends on SND_FM801_TEA575X_BOOL || SND_ES1968_RADIO || RADIO_SF16FMR2 + default SND_FM801 || SND_ES1968 || RADIO_SF16FMR2 + menuconfig SND_PCI bool PCI sound devices depends on PCI @@ -563,11 +568,6 @@ config SND_FM801_TEA575X_BOOL FM801 chip with a TEA5757 tuner (MediaForte SF256-PCS, SF256-PCP and SF64-PCR) into the snd-fm801 driver. -config SND_TEA575X - tristate - depends on SND_FM801_TEA575X_BOOL || SND_ES1968_RADIO || RADIO_SF16FMR2 - default SND_FM801 || SND_ES1968 || RADIO_SF16FMR2 - source sound/pci/hda/Kconfig config SND_HDSP -- To unsubscribe from this list: send the line unsubscribe linux-next in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] New driver for the radio FM module on Miro PCM20 sound card
At Tue, 24 Nov 2009 22:12:36 +0100, Krzysztof Helt wrote: From: Krzysztof Helt krzysztof...@wp.pl This is recreated driver for the FM module found on Miro PCM20 sound cards. This driver was removed around the 2.6.2x kernels because it relied on the removed OSS module. Now, it uses a current ALSA module (snd-miro) and is adapted to v4l2 layer. It provides only basic functionality: frequency changing and FM module muting. Signed-off-by: Krzysztof Helt krzysztof...@wp.pl --- This driver depends on changes to the snd-miro driver. These changes are already accepted to the ALSA tree, but these changes won't be pushed into the 2.6.33 kernel. Well, if we get ACK from V4L guys for this patch, I can merge it together with snd-miro changes for 2.6.33. But, it should be done ASAP (preferably in this week). drivers/media/radio/Kconfig | 17 ++ drivers/media/radio/Makefile |1 + drivers/media/radio/radio-miropcm20.c | 271 + 3 files changed, 289 insertions(+), 0 deletions(-) create mode 100644 drivers/media/radio/radio-miropcm20.c diff --git a/drivers/media/radio/Kconfig b/drivers/media/radio/Kconfig index a87a477..1c12f99 100644 --- a/drivers/media/radio/Kconfig +++ b/drivers/media/radio/Kconfig @@ -195,6 +195,23 @@ config RADIO_MAESTRO To compile this driver as a module, choose M here: the module will be called radio-maestro. +config RADIO_MIROPCM20 + tristate miroSOUND PCM20 radio + depends on ISA VIDEO_V4L2 + select SND_MIRO + ---help--- + Choose Y here if you have this FM radio card. You also need to select + the Miro miroSOUND PCM1pro/PCM12/PCM20radio driver ALSA sound card + driver for this to work. Better to rephrase This driver will select ? You cannot select it in practice but it's forcibly set. --- /dev/null +++ b/drivers/media/radio/radio-miropcm20.c (snip) + +static int radio_nr = -1; +module_param(radio_nr, int, 0); Missing MODULE_PARM_DESC(). thanks, Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] New driver for the radio FM module on Miro PCM20 sound card
At Wed, 25 Nov 2009 10:13:53 +0100, I wrote: At Tue, 24 Nov 2009 22:12:36 +0100, Krzysztof Helt wrote: From: Krzysztof Helt krzysztof...@wp.pl This is recreated driver for the FM module found on Miro PCM20 sound cards. This driver was removed around the 2.6.2x kernels because it relied on the removed OSS module. Now, it uses a current ALSA module (snd-miro) and is adapted to v4l2 layer. It provides only basic functionality: frequency changing and FM module muting. Signed-off-by: Krzysztof Helt krzysztof...@wp.pl --- This driver depends on changes to the snd-miro driver. These changes are already accepted to the ALSA tree, but these changes won't be pushed into the 2.6.33 kernel. Well, if we get ACK from V4L guys for this patch, I can merge it together with snd-miro changes for 2.6.33. But, it should be done ASAP (preferably in this week). BTW, the patches for snd-miro driver can be found in sound GIT tree next/isa or master branches git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git next/isa Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] New driver for the radio FM module on Miro PCM20 sound card
At Thu, 26 Nov 2009 13:50:17 +0100, Krzysztof Helt wrote: From: Krzysztof Helt krzysztof...@wp.pl This is recreated driver for the FM module found on Miro PCM20 sound cards. This driver was removed around the 2.6.2x kernels because it relied on the removed OSS module. Now, it uses a current ALSA module (snd-miro) and is adapted to v4l2 layer. It provides only basic functionality: frequency changing and FM module muting. Signed-off-by: Krzysztof Helt krzysztof...@wp.pl Reviewed-by: Hans Verkuil hverk...@xs4all.nl --- This is the second version of the patch with changes suggested by Hans Verkuil. Thanks. But you didn't check my review comment in my previous mail...? Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] New driver for the radio FM module on Miro PCM20 sound card
At Thu, 26 Nov 2009 18:38:27 +0100, Krzysztof Helt wrote: On Thu, 26 Nov 2009 16:43:15 +0100 Takashi Iwai ti...@suse.de wrote: At Thu, 26 Nov 2009 13:50:17 +0100, Krzysztof Helt wrote: From: Krzysztof Helt krzysztof...@wp.pl This is recreated driver for the FM module found on Miro PCM20 sound cards. This driver was removed around the 2.6.2x kernels because it relied on the removed OSS module. Now, it uses a current ALSA module (snd-miro) and is adapted to v4l2 layer. It provides only basic functionality: frequency changing and FM module muting. Signed-off-by: Krzysztof Helt krzysztof...@wp.pl Reviewed-by: Hans Verkuil hverk...@xs4all.nl --- This is the second version of the patch with changes suggested by Hans Verkuil. Thanks. But you didn't check my review comment in my previous mail...? I have missed it. I will redo the patch. OK, thanks ;) Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] New driver for the radio FM module on Miro PCM20 sound card
At Fri, 27 Nov 2009 11:24:13 +0100, Krzysztof Helt wrote: From: Krzysztof Helt krzysztof...@wp.pl This is recreated driver for the FM module found on Miro PCM20 sound cards. This driver was removed around the 2.6.2x kernels because it relied on the removed OSS module. Now, it uses a current ALSA module (snd-miro) and is adapted to v4l2 layer. It provides only basic functionality: frequency changing and FM module muting. Signed-off-by: Krzysztof Helt krzysztof...@wp.pl Reviewed-by: Hans Verkuil hverk...@xs4all.nl --- This is the third version of the patch with fixed issues pointed by Takashi Iwai. Thanks, I merged now this to sound tree exceptionally since it's purely depends on snd-miro driver. Will appear in the next linux-next. Takashi drivers/media/radio/Kconfig | 18 +++ drivers/media/radio/Makefile |1 + drivers/media/radio/radio-miropcm20.c | 270 + 3 files changed, 289 insertions(+), 0 deletions(-) create mode 100644 drivers/media/radio/radio-miropcm20.c diff --git a/drivers/media/radio/Kconfig b/drivers/media/radio/Kconfig index a87a477..b134553 100644 --- a/drivers/media/radio/Kconfig +++ b/drivers/media/radio/Kconfig @@ -195,6 +195,24 @@ config RADIO_MAESTRO To compile this driver as a module, choose M here: the module will be called radio-maestro. +config RADIO_MIROPCM20 + tristate miroSOUND PCM20 radio + depends on ISA VIDEO_V4L2 + select SND_MIRO + ---help--- + Choose Y here if you have this FM radio card. You also need to enable + the ALSA sound system. This choice automatically selects the ALSA + sound card driver Miro miroSOUND PCM1pro/PCM12/PCM20radio as this + is required for the radio-miropcm20. + + In order to control your radio card, you will need to use programs + that are compatible with the Video For Linux API. Information on + this API and pointers to v4l programs may be found at + file:Documentation/video4linux/API.html. + + To compile this driver as a module, choose M here: the + module will be called radio-miropcm20. + config RADIO_SF16FMI tristate SF16FMI Radio depends on ISA VIDEO_V4L2 diff --git a/drivers/media/radio/Makefile b/drivers/media/radio/Makefile index 2a1be3b..8a63d54 100644 --- a/drivers/media/radio/Makefile +++ b/drivers/media/radio/Makefile @@ -18,6 +18,7 @@ obj-$(CONFIG_RADIO_TRUST) += radio-trust.o obj-$(CONFIG_I2C_SI4713) += si4713-i2c.o obj-$(CONFIG_RADIO_SI4713) += radio-si4713.o obj-$(CONFIG_RADIO_MAESTRO) += radio-maestro.o +obj-$(CONFIG_RADIO_MIROPCM20) += radio-miropcm20.o obj-$(CONFIG_USB_DSBR) += dsbr100.o obj-$(CONFIG_RADIO_SI470X) += si470x/ obj-$(CONFIG_USB_MR800) += radio-mr800.o diff --git a/drivers/media/radio/radio-miropcm20.c b/drivers/media/radio/radio-miropcm20.c new file mode 100644 index 000..4ff8854 --- /dev/null +++ b/drivers/media/radio/radio-miropcm20.c @@ -0,0 +1,270 @@ +/* Miro PCM20 radio driver for Linux radio support + * (c) 1998 Ruurd Reitsma r.a.reit...@wbmt.tudelft.nl + * Thanks to Norberto Pellici for the ACI device interface specification + * The API part is based on the radiotrack driver by M. Kirkwood + * This driver relies on the aci mixer provided by the snd-miro + * ALSA driver. + * Look there for further info... + */ + +/* What ever you think about the ACI, version 0x07 is not very well! + * I can't get frequency, 'tuner status', 'tuner flags' or mute/mono + * conditions...Robert + */ + +#include linux/module.h +#include linux/init.h +#include linux/videodev2.h +#include media/v4l2-device.h +#include media/v4l2-ioctl.h +#include sound/aci.h + +static int radio_nr = -1; +module_param(radio_nr, int, 0); +MODULE_PARM_DESC(radio_nr, Set radio device number (/dev/radioX). Default: -1 (autodetect)); + +static int mono; +module_param(mono, bool, 0); +MODULE_PARM_DESC(mono, Force tuner into mono mode.); + +struct pcm20 { + struct v4l2_device v4l2_dev; + struct video_device vdev; + unsigned long freq; + int muted; + struct snd_miro_aci *aci; +}; + +static struct pcm20 pcm20_card = { + .freq = 87*16000, + .muted = 1, +}; + +static int pcm20_mute(struct pcm20 *dev, unsigned char mute) +{ + dev-muted = mute; + return snd_aci_cmd(dev-aci, ACI_SET_TUNERMUTE, mute, -1); +} + +static int pcm20_stereo(struct pcm20 *dev, unsigned char stereo) +{ + return snd_aci_cmd(dev-aci, ACI_SET_TUNERMONO, !stereo, -1); +} + +static int pcm20_setfreq(struct pcm20 *dev, unsigned long freq) +{ + unsigned char freql; + unsigned char freqh; + struct snd_miro_aci *aci = dev-aci; + + dev-freq = freq; + + freq /= 160; + if (!(aci-aci_version == 0x07 || aci-aci_version = 0xb0)) + freq /= 10; /* I don't know exactly which version
Re: [PULL] http://www.kernellabs.com/hg/~dheitmueller/hvr-1600-alsa-2
At Sun, 13 Dec 2009 10:22:44 -0500, Devin Heitmueller wrote: On Sat, Dec 12, 2009 at 5:34 PM, Mauro Carvalho Chehab mche...@infradead.org wrote: Hi Devin, It is better to submit the RFC patches to alsa ML for they to take a look. Cheers, Mauro Is there something specific you want the ALSA people to review? There are no changes to the ALSA core and this is a pretty simple/straightforward PCM capture device modelled after the existing cx88/saa7134/em28xx-alsa drivers. The driver is complete, tested by both KernelLabs and the customer. As far as I know, we generally haven't bothered the ALSA people with devices in the /media tree in the past unless there was some specific issue. If there is something here out of the ordinary here that you felt the ALSA people would be able to offer some insight on, I would be more than happy to send it to them. Just a very quick look through the files, here are some comments: - snd_card_free() can't be called with NULL, but the error path in snd_cx18_init() may reach that (when snd_card_create() returns an error). - Specifying both SNDRV_PCM_RATE_CONTINOUS and _KNOT to snd_pcm_hw_info.rates doesn't make sense. In your case, if it's only 48kHz, SNDRV_PCM_RATE_48000 there instead. - vfree() should be called in hw_free callback rather than close callback. And, there are already global vmalloc-buffer helper functions in the latest ALSA tree for 2.6.34... - It'd be nice if you give TLV information for the mixer dB mapping. thanks, Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://www.kernellabs.com/hg/~dheitmueller/hvr-1600-alsa-2
At Mon, 21 Dec 2009 12:30:48 -0500, Devin Heitmueller wrote: On Mon, Dec 21, 2009 at 12:21 PM, Takashi Iwai ti...@suse.de wrote: Just a very quick look through the files, here are some comments: - snd_card_free() can't be called with NULL, but the error path in snd_cx18_init() may reach that (when snd_card_create() returns an error). - Specifying both SNDRV_PCM_RATE_CONTINOUS and _KNOT to snd_pcm_hw_info.rates doesn't make sense. In your case, if it's only 48kHz, SNDRV_PCM_RATE_48000 there instead. - vfree() should be called in hw_free callback rather than close callback. And, there are already global vmalloc-buffer helper functions in the latest ALSA tree for 2.6.34... - It'd be nice if you give TLV information for the mixer dB mapping. Hello Takashi, Thanks for taking the time to provide feedback. I will make these changes and add them to the tree. Out of curiosity, is there any sort of tool/code which exercises the driver to verify the advertised capabilities. Part of the problem here is that it is not really easy to find example applications which use all the features, which I suspect is one of the big headaches that the PulseAudio people are suffering from in that they are finding bugs in ALSA drivers because they are the first ones to actually attempt to use some of these capabilities. For me the card works, but then some user complains that their PulseAudio is broken because I didn't setup the config structures properly or only half-implemented some capability used by PulseAudio but not arecord/aplay. Yeah, the test suite is one of the biggest missing pieces. Due to its nature, the automatic test is a bit hard to achieve with the real hardwares. Some tools are found, however, in alsa-lib/test directory. For example, pcm.c can test different PCM access methods. Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] media: Add stk1160 new driver
At Thu, 26 Jul 2012 21:15:22 -0300, Ezequiel Garcia wrote: This driver adds support for stk1160 usb bridge as used in some video/audio usb capture devices. It is a complete rewrite of staging/media/easycap driver and it's expected as a future replacement. Cc: Mauro Carvalho Chehab mche...@redhat.com Cc: Takashi Iwai ti...@suse.de Cc: Hans Verkuil hverk...@xs4all.nl Cc: Sylwester Nawrocki sylvester.nawro...@gmail.com Signed-off-by: Ezequiel Garcia elezegar...@gmail.com --- Here we are once again, As stk1160 allows communication with an ac97 codec chip, this driver registers a control-only sound card to allow the user to access ac97 controls. This is achieved through snd_ac97_codec/ac97_bus drivers. Mauro suggested that this ac97 handling should be put inside -alsa tree, but I'm still not sure about it. This approach is working well in practice, but I'm not 100% confident so feedback is welcome (in particular from you alsa guys). Well, it's a pretty small stuff and ac97 isn't developed so much any longer, so I'm fine to put your code in the video tree. Looking through your patch, a remaining problem is that the dependency on the sound core is missing. The select in Kconfig doesn't fulfill the dependencies automatically but forcibly sets the value. Selecting CONFIG_SND_AC97_CODEC will select most of other components but CONFIG_SND itself must be enabled beforehand. Thus, you need to wrap CONFIG_VIDEO_STK1160 with depends on SND. Or split the ac97 codec part and makes it depending on SND, and define dummy functions if not defined, e.g. #ifdef CONFIG_VIDEO_STK1160_AC97 int stk1160_ac97_register(struct stk1160 *dev); int stk1160_ac97_unregister(struct stk1160 *dev); #else static inline int stk1160_ac97_register(struct stk1160 *dev) { return 0; } static inline int stk1160_ac97_unregister(struct stk1160 *dev) { return 0; } #endif thanks, Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Ksummit-2012-discuss] [MINI SUMMIT] media mini-summit at KS/2012
At Wed, 01 Aug 2012 16:17:07 -0300, Mauro Carvalho Chehab wrote: After the discussions around the media mini-summit, this is the consolidated list of topics. We tried to give a fair time for each topic. With regards to the proposed topic Intel media SDK, nobody volunteered to give such a talk. So, if there is anyone interested to talk about it, it can either be part of SoC vendor's feedback (if it won't take too long), or it could be handled as a topic during LPC. So, let's put this one in the fridge. Topics for the media mini-summit - morning: V4L2 API ambiguities + v4l2 compliance tool: 60 min Media Controller library: 30 min ALSA and V4L/Media Controller: 30 min ARM and needed features for V4L/DVB: 30 min - after lunch: SoC Vendors feedback – how to help them to go upstream: 45 min V4L2/DVB issues from userspace perspective: 45 min Android's V4L2 cam library: 30 min HDMI CEC: 30 min Synchronization, shared resource and optimizations: 30 min The DT discussions will likely require more time and maybe a different audience. So, it seems better to handle it as either as a separate/LPC section or at DT mini-summit: Common device tree bindings for media devices: 45 min Attendants == Guennadi Liakhovetski g.liakhovet...@gmx.de Hans Verkuil hans.verk...@xs4all.nl Laurent Pinchart laurent.pinch...@ideasonboard.com Mauro Carvalho Chehab mche...@redhat.com Michael Krufky mkru...@linuxtv.org Naveen Krishnamurthy naveen.krishnamur...@st.com Palash Bandyopadhyay palash.bandyopadh...@conexant.com Pawel Osciak pa...@osciak.com Rémi Denis-Courmont r...@remlab.net Sakari Ailus sakari.ai...@iki.fi Sri Deevi srinivasa.de...@conexant.com Steven Toth st...@kernellabs.com Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com Marek Szyprowski m.szyprow...@samsung.com Kamil Debski k.deb...@samsung.com Naveen also asked another seat for ST, but didn't provide us yet the name of the other interested party. For the cross-system topics, it is important to have people from the other subsystems there. So, we're proposing to have some people there, but we need to double-check if they can really be there for the discussions. Of course, other Kernel developers from KS/LPC that work on those subsystems are welcome. The names that came up during our discussions for those panels are: Rob Clark rob.cl...@linaro.org (HDMI CEC, ALSA, ARM) Takashi Iwai ti...@suse.de (ALSA) Catalin Marinas catalin.mari...@arm.com (ARM) Mark Brown broo...@opensource.wolfsonmicro.com (DT) Not sure if they all will be there or if they'll have some time to discuss those things with us. We hope they will ;) I'll have a talk at Gstreamer conference on Tuesday afternoon, so I can't attend the conflicting slots, but other than that, I'll be likely available (e.g. before lunch). thanks, Takashi Thanks, Mauro Carvalho Chehab Hans Verkuil ___ Ksummit-2012-discuss mailing list ksummit-2012-disc...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/ksummit-2012-discuss -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] ALSA: ppc: keywest: Don't use i2c_client-driver
At Sun, 29 Sep 2013 10:51:04 +0200, Lars-Peter Clausen wrote: The 'driver' field of the i2c_client struct is redundant and is going to be removed. Use 'to_i2c_driver(client-dev.driver)' instead to get direct access to the i2c_driver struct. Signed-off-by: Lars-Peter Clausen l...@metafoo.de Acked-by: Takashi Iwai ti...@suse.de thanks, Takashi --- sound/ppc/keywest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/ppc/keywest.c b/sound/ppc/keywest.c index 01aecc2..0d1c27e 100644 --- a/sound/ppc/keywest.c +++ b/sound/ppc/keywest.c @@ -65,7 +65,7 @@ static int keywest_attach_adapter(struct i2c_adapter *adapter) * already bound. If not it means binding failed, and then there * is no point in keeping the device instantiated. */ - if (!keywest_ctx-client-driver) { + if (!keywest_ctx-client-dev.driver) { i2c_unregister_device(keywest_ctx-client); keywest_ctx-client = NULL; return -ENODEV; @@ -76,7 +76,7 @@ static int keywest_attach_adapter(struct i2c_adapter *adapter) * This is safe because i2c-core holds the core_lock mutex for us. */ list_add_tail(keywest_ctx-client-detected, - keywest_ctx-client-driver-clients); + to_i2c_driver(keywest_ctx-client-dev.driver)-clients); return 0; } -- 1.8.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RESEND 6/8] ALSA: memalloc: use gen_pool_dma_alloc() to allocate iram buffer
At Fri, 1 Nov 2013 19:48:19 +0800, Nicolin Chen wrote: Since gen_pool_dma_alloc() is introduced, we implement it to simplify code. Signed-off-by: Nicolin Chen b42...@freescale.com Acked-by: Takashi Iwai ti...@suse.de --- sound/core/memalloc.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c index 9d93f02..5e1c7bc 100644 --- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -184,11 +184,7 @@ static void snd_malloc_dev_iram(struct snd_dma_buffer *dmab, size_t size) /* Assign the pool into private_data field */ dmab-private_data = pool; - dmab-area = (void *)gen_pool_alloc(pool, size); - if (!dmab-area) - return; - - dmab-addr = gen_pool_virt_to_phys(pool, (unsigned long)dmab-area); + dmab-area = gen_pool_dma_alloc(pool, size, dmab-addr); } /** -- 1.8.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] cx231xx: card-driver Conexant cx231xx Audio too long
At Fri, 19 Mar 2010 14:49:57 +0300, Dan Carpenter wrote: card-driver is 15 characters and a NULL, the original code could cause a buffer overflow. Signed-off-by: Dan Carpenter erro...@gmail.com This should rather a string without spaces because this string is used as an identifier in alsa-lib. Better to use Cx231xx or Cx231xx-Audio. Takashi diff --git a/drivers/media/video/cx231xx/cx231xx-audio.c b/drivers/media/video/cx231xx/cx231xx-audio.c index 7793d60..b3282ae 100644 --- a/drivers/media/video/cx231xx/cx231xx-audio.c +++ b/drivers/media/video/cx231xx/cx231xx-audio.c @@ -495,7 +495,7 @@ static int cx231xx_audio_init(struct cx231xx *dev) pcm-info_flags = 0; pcm-private_data = dev; strcpy(pcm-name, Conexant cx231xx Capture); - strcpy(card-driver, Conexant cx231xx Audio); + strcpy(card-driver, Cx231xx Audio); strcpy(card-shortname, Cx231xx Audio); strcpy(card-longname, Conexant cx231xx Audio); -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch v2] cx231xx: card-driver Conexant cx231xx Audio too long
At Mon, 22 Mar 2010 08:43:47 -0700, Joe Perches wrote: On Mon, 2010-03-22 at 18:39 +0300, Dan Carpenter wrote: card-driver is 15 characters and a NULL, the original code could cause a buffer overflow. In version 2, I used a better name that Takashi Iwai suggested. Perhaps it's better to use strncpy as well. strlcpy() would be safer :) But, in such a case, we want rather that the error is notified at build time. Maybe a macro like below would be helpful to catch such bugs? #define COPY_STRING(buf, src) \ do {\ if (__builtin_constant_p(src)) \ BUILD_BUG_ON(strlen(src) = sizeof(buf)); \ strcpy(buf, src); \ } while (0) and used like: struct foo { char foo[5]; } x; COPY_STRING(x.foo, OK); // OK COPY_STRING(x.foo, 1234567890); // NG Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch v2] cx231xx: card-driver Conexant cx231xx Audio too long
At Mon, 22 Mar 2010 19:54:30 +0300, Dan Carpenter wrote: On Mon, Mar 22, 2010 at 05:04:55PM +0100, Takashi Iwai wrote: At Mon, 22 Mar 2010 08:43:47 -0700, Joe Perches wrote: On Mon, 2010-03-22 at 18:39 +0300, Dan Carpenter wrote: card-driver is 15 characters and a NULL, the original code could cause a buffer overflow. In version 2, I used a better name that Takashi Iwai suggested. Perhaps it's better to use strncpy as well. strlcpy() would be safer :) But, in such a case, we want rather that the error is notified at build time. Maybe a macro like below would be helpful to catch such bugs? #define COPY_STRING(buf, src) \ do {\ if (__builtin_constant_p(src)) \ BUILD_BUG_ON(strlen(src) = sizeof(buf)); \ strcpy(buf, src); \ } while (0) and used like: struct foo { char foo[5]; } x; COPY_STRING(x.foo, OK); // OK COPY_STRING(x.foo, 1234567890); // NG I can do the same thing with Smatch. The smatch check can also find bugs like this: buf = kmalloc(10, GFP_KERNEL); strcpy(buf, 1234567890); I used smatch to find this bug and 5 others on my allmodconfig w/ staging. I also found 19 other places that use strcpy() to copy from a large buffer into a smaller buffer. Ah, nice. Your idea is nice, but I think anyone who deliberately uses the new macro is not going to have the bug in the first place. ;) Yeah, in theory, such a code should be never committed because it can be caught at build time ;) Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch v2] cx231xx: card-driver Conexant cx231xx Audio too long
At Mon, 22 Mar 2010 18:39:09 +0300, Dan Carpenter wrote: card-driver is 15 characters and a NULL, the original code could cause a buffer overflow. Signed-off-by: Dan Carpenter erro...@gmail.com --- In version 2, I used a better name that Takashi Iwai suggested. Acked-by: Takashi Iwai ti...@suse.de Could you fix em28xx in the same way, too? thanks, Takashi diff --git a/drivers/media/video/cx231xx/cx231xx-audio.c b/drivers/media/video/cx231xx/cx231xx-audio.c index 7793d60..7cae95a 100644 --- a/drivers/media/video/cx231xx/cx231xx-audio.c +++ b/drivers/media/video/cx231xx/cx231xx-audio.c @@ -495,7 +495,7 @@ static int cx231xx_audio_init(struct cx231xx *dev) pcm-info_flags = 0; pcm-private_data = dev; strcpy(pcm-name, Conexant cx231xx Capture); - strcpy(card-driver, Conexant cx231xx Audio); + strcpy(card-driver, Cx231xx-Audio); strcpy(card-shortname, Cx231xx Audio); strcpy(card-longname, Conexant cx231xx Audio); -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] NULL pointer dereference in ALSA triggered through saa7134-alsa
At Mon, 10 Aug 2009 16:50:55 +0300, Ozan Çağlayan wrote: Hi, I've finally succesfully compiled and linked saa7134-alsa driver using an external alsa-driver and its Module.symvers file. Everything seems okay, no undefined symbol or something else: - An installed 2.6.30.4 kernel which only builds and brings soundcore and sound_firmware, - Latest alsa-driver built externally and installed, - Latest saa7134-alsa, cx88-alsa, etc. code from linus-2.6 (seen that they don't affected by some API/ABI changes) patched on top of the alsa-driver tarball, The external drivers using ALSA API have to be built with the newer ALSA header files from alsa-driver tree. It's not enough to change snd_card_new() with snd_card_create(). The core structure was changed, so the whole build has to be adjusted, too. Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] NULL pointer dereference in ALSA triggered through saa7134-alsa
At Mon, 10 Aug 2009 18:42:20 +0300, Ozan Çağlayan wrote: Takashi Iwai wrote: At Mon, 10 Aug 2009 16:50:55 +0300, Ozan Çağlayan wrote: Hi, I've finally succesfully compiled and linked saa7134-alsa driver using an external alsa-driver and its Module.symvers file. Everything seems okay, no undefined symbol or something else: - An installed 2.6.30.4 kernel which only builds and brings soundcore and sound_firmware, - Latest alsa-driver built externally and installed, - Latest saa7134-alsa, cx88-alsa, etc. code from linus-2.6 (seen that they don't affected by some API/ABI changes) patched on top of the alsa-driver tarball, The external drivers using ALSA API have to be built with the newer ALSA header files from alsa-driver tree. It's not enough to change snd_card_new() with snd_card_create(). The core structure was changed, so the whole build has to be adjusted, too. Actually that was the 0th step that I forgot to mention. I'm installing the headers from the alsa-driver snapshot into /usr/include/sound and then I build alsa-driver. Then I use the symvers from the alsa-driver build alltogether with the headers that I've already installed into /usr/include/sound to build the V4L ones. But /usr/include/sound isn't used for building kernel modules normally. Unless any hack is added, these files have to be installed to the kernel header directory. Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] NULL pointer dereference in ALSA triggered through saa7134-alsa
At Mon, 10 Aug 2009 19:06:29 +0300, Ozan Çağlayan wrote: Takashi Iwai wrote: But /usr/include/sound isn't used for building kernel modules normally. Unless any hack is added, these files have to be installed to the kernel header directory. Ah you're right, I totally missed that one. Thanks, will try to workaround that. Well, the best would be to make alsa-driver backward compatible. Basically we need to patch headers in alsa-kernel tree to be binary-compatible with older versions, i.e. adjusting the fields in struct snd_pcm or so, and supply the compatible function, snd_card_new() in addition. This could be done automatically in alsa-driver build process, but I have little time to work on it... Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 45/50] sound: usb: usx2y: spin_lock in complete() cleanup
At Thu, 11 Jul 2013 17:08:30 +0400, Sergei Shtylyov wrote: On 11-07-2013 13:06, Ming Lei wrote: Complete() will be run with interrupt enabled, so change to spin_lock_irqsave(). Changelog doesn't match the patch. Yep, but moreover... Cc: Jaroslav Kysela pe...@perex.cz Cc: Takashi Iwai ti...@suse.de Cc: alsa-de...@alsa-project.org Signed-off-by: Ming Lei ming@canonical.com --- sound/usb/usx2y/usbusx2yaudio.c |4 1 file changed, 4 insertions(+) diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c index 4967fe9..e2ee893 100644 --- a/sound/usb/usx2y/usbusx2yaudio.c +++ b/sound/usb/usx2y/usbusx2yaudio.c @@ -273,7 +273,11 @@ static void usX2Y_clients_stop(struct usX2Ydev *usX2Y) struct snd_usX2Y_substream *subs = usX2Y-subs[s]; if (subs) { if (atomic_read(subs-state) = state_PRERUNNING) { + unsigned long flags; + + local_irq_save(flags); snd_pcm_stop(subs-pcm_substream, SNDRV_PCM_STATE_XRUN); + local_irq_restore(flags); } ... actually this snd_pcm_stop() call should have been covered by snd_pcm_stream_lock(). Maybe it'd be enough to have a single patch together with the change, i.e. wrapping with snd_pcm_stream_lock_irqsave(). I'll prepare the patch for 3.11 independently from your patch series, so please drop this one. BTW, the word cleanup in the subject is inappropriate. This is rather a fix together with the core change. And, I wonder whether we can take a safer approach. When the caller condition changed, we often introduced either a different ops (e.g. ioctl case) or a flag for the new condition, at least during the transition period. Last but not least, is a conversion to tasklet really preferred? tasklet is rather an obsoleted infrastructure nowadays, and people don't recommend to use it any longer, AFAIK. thanks, Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 45/50] sound: usb: usx2y: spin_lock in complete() cleanup
At Thu, 11 Jul 2013 22:13:35 +0800, Ming Lei wrote: On Thu, Jul 11, 2013 at 9:50 PM, Takashi Iwai ti...@suse.de wrote: At Thu, 11 Jul 2013 17:08:30 +0400, Sergei Shtylyov wrote: On 11-07-2013 13:06, Ming Lei wrote: Complete() will be run with interrupt enabled, so change to spin_lock_irqsave(). Changelog doesn't match the patch. Yep, but moreover... Cc: Jaroslav Kysela pe...@perex.cz Cc: Takashi Iwai ti...@suse.de Cc: alsa-de...@alsa-project.org Signed-off-by: Ming Lei ming@canonical.com --- sound/usb/usx2y/usbusx2yaudio.c |4 1 file changed, 4 insertions(+) diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c index 4967fe9..e2ee893 100644 --- a/sound/usb/usx2y/usbusx2yaudio.c +++ b/sound/usb/usx2y/usbusx2yaudio.c @@ -273,7 +273,11 @@ static void usX2Y_clients_stop(struct usX2Ydev *usX2Y) struct snd_usX2Y_substream *subs = usX2Y-subs[s]; if (subs) { if (atomic_read(subs-state) = state_PRERUNNING) { + unsigned long flags; + + local_irq_save(flags); snd_pcm_stop(subs-pcm_substream, SNDRV_PCM_STATE_XRUN); + local_irq_restore(flags); } ... actually this snd_pcm_stop() call should have been covered by snd_pcm_stream_lock(). Maybe it'd be enough to have a single patch together with the change, i.e. wrapping with snd_pcm_stream_lock_irqsave(). Please use snd_pcm_stream_lock_irqsave() so that I can avoid sending out similar patch later, :-) I'll prepare the patch for 3.11 independently from your patch series, so please drop this one. OK, thanks for dealing with that. BTW, the word cleanup in the subject is inappropriate. This is rather a fix together with the core change. It is a cleanup since the patchset only addresses lock problem which is caused by the tasklet conversion. Well, the conversion to irqsave() is needed for the future drop of irq_save() in the caller, right? Then this isn't a cleanup but a preparation for movement ahead. And, I wonder whether we can take a safer approach. When the caller condition changed, we often introduced either a different ops (e.g. ioctl case) or a flag for the new condition, at least during the transition period. Interrupt is't enabled until all current drivers are cleaned up, so it is really safe, please see patch [2]. OK. Last but not least, is a conversion to tasklet really preferred? tasklet is rather an obsoleted infrastructure nowadays, and people don't recommend to use it any longer, AFAIK. We discussed the problem in the below link previously[1], Steven and Thomas suggested to use threaded irq handler, but which may degrade USB mass storage performance, so we have to take tasklet now until we rewrite transport part of USB mass storage driver. Also the conversion[2] has avoided the tasklet spin lock problem already. OK, good to know. thanks, Takashi [1], http://marc.info/?t=13707911921r=1w=2 [2], http://marc.info/?l=linux-usbm=137286326726326w=2 Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 0/2] tea575x: Move from sound to media
At Sun, 28 Jul 2013 22:01:42 +0200, Ondrej Zary wrote: Hello, TEA575x is neither a sound device nor an i2c device. Let's finally move it from sound/i2c/other to drivers/media/radio. Tested with snd-es1968, snd-fm801 and radio-sf16fmr2. Good to resolve messes there now. For both patches, Acked-by: Takashi Iwai ti...@suse.de Feel free to move them into media git tree for 3.12 kernel. thanks, Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] sound/pci/Kconfig: select RADIO_ADAPTERS if needed
At Sat, 24 Aug 2013 08:18:02 -0300, Mauro Carvalho Chehab wrote: As reported by kbuild test robot fengguang...@intel.com: warning: (SND_ES1968_RADIO SND_FM801_TEA575X_BOOL) selects RADIO_TEA575X which has unmet direct dependencies (MEDIA_SUPPORT RADIO_ADAPTERS VIDEO_V4L2) That happens because a radio driver is selected, without selecting the RADIO_ADAPTERS menu. Reported-by: kbuild test robot fengguang...@intel.com Cc: Takashi Iwai ti...@suse.de Feel free to take my ACK, if any. Acked-by: Takashi Iwai ti...@suse.de thanks, Takashi Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- sound/pci/Kconfig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/pci/Kconfig b/sound/pci/Kconfig index 9df80ef..46ed9e8 100644 --- a/sound/pci/Kconfig +++ b/sound/pci/Kconfig @@ -539,7 +539,9 @@ config SND_ES1968_RADIO depends on SND_ES1968 depends on MEDIA_RADIO_SUPPORT depends on VIDEO_V4L2=y || VIDEO_V4L2=SND_ES1968 + select RADIO_ADAPTERS select RADIO_TEA575X + help Say Y here to include support for TEA5757 radio tuner integrated on some MediaForte cards (e.g. SF64-PCE2). @@ -561,6 +563,7 @@ config SND_FM801_TEA575X_BOOL depends on SND_FM801 depends on MEDIA_RADIO_SUPPORT depends on VIDEO_V4L2=y || VIDEO_V4L2=SND_FM801 + select RADIO_ADAPTERS select RADIO_TEA575X help Say Y here to include support for soundcards based on the ForteMedia -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] ivtv: Fix Oops when no firmware is loaded
When ivtv PCM device is accessed at the state where no firmware is loaded, it oopses like: BUG: unable to handle kernel NULL pointer dereference at 0050 IP: [a049a881] try_mailbox.isra.0+0x11/0x50 [ivtv] Call Trace: [a049aa20] ivtv_api_call+0x160/0x6b0 [ivtv] [a049af86] ivtv_api+0x16/0x40 [ivtv] [a049b10c] ivtv_vapi+0xac/0xc0 [ivtv] [a049d40d] ivtv_start_v4l2_encode_stream+0x19d/0x630 [ivtv] [a0530653] snd_ivtv_pcm_capture_open+0x173/0x1c0 [ivtv_alsa] [a04526f1] snd_pcm_open_substream+0x51/0x100 [snd_pcm] [a0452853] snd_pcm_open+0xb3/0x260 [snd_pcm] [a0452a37] snd_pcm_capture_open+0x37/0x50 [snd_pcm] [a033f557] snd_open+0xa7/0x1e0 [snd] [8118a628] chrdev_open+0x88/0x1d0 [811840be] do_dentry_open+0x1de/0x270 [81193a73] do_last+0x1c3/0xec0 [81194826] path_openat+0xb6/0x670 [81195b65] do_filp_open+0x35/0x80 [81185449] do_sys_open+0x129/0x210 [815b782d] system_call_fastpath+0x1a/0x1f This patch adds the check of firmware at PCM open callback like other open callbacks of this driver. Bugzilla: https://apibugzilla.novell.com/show_bug.cgi?id=875440 Cc: sta...@vger.kernel.org Signed-off-by: Takashi Iwai ti...@suse.de --- drivers/media/pci/ivtv/ivtv-alsa-pcm.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/pci/ivtv/ivtv-alsa-pcm.c b/drivers/media/pci/ivtv/ivtv-alsa-pcm.c index e1863dbf4edc..7a9b98bc208b 100644 --- a/drivers/media/pci/ivtv/ivtv-alsa-pcm.c +++ b/drivers/media/pci/ivtv/ivtv-alsa-pcm.c @@ -159,6 +159,12 @@ static int snd_ivtv_pcm_capture_open(struct snd_pcm_substream *substream) /* Instruct the CX2341[56] to start sending packets */ snd_ivtv_lock(itvsc); + + if (ivtv_init_on_first_open(itv)) { + snd_ivtv_unlock(itvsc); + return -ENXIO; + } + s = itv-streams[IVTV_ENC_STREAM_TYPE_PCM]; v4l2_fh_init(item.fh, s-vdev); -- 1.9.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Linux Audio Mini Summit 2014, Oct. 14, Düsseldorf, Germany
Hi, like previous years, we're going to hold a meeting to discuss lowlevel audio on Linux. This time it's along with Linux Plumbers conference, but for avoiding the conflicts with other LPC mini conferences (including Media Mini-summit, of course :), it'll be held on Tuesday October 14, at the same venue as LPC, Congress Center Düsseldorf, Germany. If anyone is interested, feel free to sign up in the attendee list below. It's a public accessible Google doc, so you can edit freely. http://goo.gl/VbXLPW (or https://docs.google.com/spreadsheets/d/1wBAiAyZSnMjVFJbxdXE5JoVdKGIBIWt-4tuSsHzGw4w/edit?usp=sharing) The topics that have been currently raised are found in the URL below. If you have any more topics to discuss there, just put your favorite one in the URL below as well. http://goo.gl/JSasQ9 (or https://docs.google.com/spreadsheets/d/1cXzON65QYNrTXi31AIQHffUZ9NWwJuNEJAC0ubfKlZc/edit?usp=sharing) If you have any question or suggestion, drop me a mail. Thanks! Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/6] media token resource framework
At Tue, 14 Oct 2014 08:58:36 -0600, Shuah Khan wrote: Add media token device resource framework to allow sharing resources such as tuner, dma, audio etc. across media drivers and non-media sound drivers that control media hardware. The Media token resource is created at the main struct device that is common to all drivers that claim various pieces of the main media device, which allows them to find the resource using the main struct device. As an example, digital, analog, and snd-usb-audio drivers can use the media token resource API using the main struct device for the interface the media device is attached to. This patch series consists of media token resource framework and changes to use it in dvb-core, v4l2-core, au0828 driver, and snd-usb-audio driver. With these changes dvb and v4l2 can share the tuner without disrupting each other. Used tvtime, xawtv, kaffeine, and vlc, vlc audio capture option, arecord/aplay during development to identify v4l2 vb2 and vb1 ioctls and file operations that disrupt the digital stream and would require changes to check tuner ownership prior to changing the tuner configuration. vb2 changes are made in the v4l2-core and vb1 changes are made in the au0828 driver to encourage porting drivers to vb2 to advantage of the new media token resource framework with changes in the core. In this patch v2 series, fixed problems identified in the patch v1 series. Important ones are changing snd-usb-audio to use media tokens, holding tuner lock in VIDIOC_ENUMINPUT, and VIDIOC_QUERYSTD. Just took a quick glance over the patches, and my first concern is why this has to be lib/*. This means it's always built-in as long as this config is enabled (and will be so on distro kernel) even if it's not used at all. thanks, Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/6] media: add media token device resource framework
At Tue, 14 Oct 2014 08:58:37 -0600, Shuah Khan wrote: Add media token device resource framework to allow sharing resources such as tuner, dma, audio etc. across media drivers and non-media sound drivers that control media hardware. The Media token resource is created at the main struct device that is common to all drivers that claim various pieces of the main media device, which allows them to find the resource using the main struct device. As an example, digital, analog, and snd-usb-audio drivers can use the media token resource API using the main struct device for the interface the media device is attached to. A shared media tokens resource is created using devres framework for drivers to find and lock/unlock. Creating a shared devres helps avoid creating data structure dependencies between drivers. This media token resource contains media token for tuner, and audio. When tuner token is requested, audio token is issued. Subsequent token (for tuner and audio) gets from the same task and task in the same tgid succeed. This allows applications that make multiple v4l2 ioctls to work with the first call acquiring the token and applications that create separate threads to handle video and audio functions. Signed-off-by: Shuah Khan shua...@osg.samsung.com --- MAINTAINERS |2 + include/linux/media_tknres.h | 50 + lib/Makefile |2 + lib/media_tknres.c | 237 ++ 4 files changed, 291 insertions(+) create mode 100644 include/linux/media_tknres.h create mode 100644 lib/media_tknres.c diff --git a/MAINTAINERS b/MAINTAINERS index e80a275..9216179 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5864,6 +5864,8 @@ F: include/uapi/linux/v4l2-* F: include/uapi/linux/meye.h F: include/uapi/linux/ivtv* F: include/uapi/linux/uvcvideo.h +F: include/linux/media_tknres.h +F: lib/media_tknres.c MEDIAVISION PRO MOVIE STUDIO DRIVER M: Hans Verkuil hverk...@xs4all.nl diff --git a/include/linux/media_tknres.h b/include/linux/media_tknres.h new file mode 100644 index 000..6d37327 --- /dev/null +++ b/include/linux/media_tknres.h @@ -0,0 +1,50 @@ +/* + * media_tknres.h - managed media token resource + * + * Copyright (c) 2014 Shuah Khan shua...@osg.samsung.com + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * + * This file is released under the GPLv2. + */ +#ifndef __LINUX_MEDIA_TOKEN_H +#define __LINUX_MEDIA_TOKEN_H + +struct device; + +#if defined(CONFIG_MEDIA_SUPPORT) +extern int media_tknres_create(struct device *dev); +extern int media_tknres_destroy(struct device *dev); + +extern int media_get_tuner_tkn(struct device *dev); +extern int media_put_tuner_tkn(struct device *dev); + +extern int media_get_audio_tkn(struct device *dev); +extern int media_put_audio_tkn(struct device *dev); The words tknres and tkn (especially latter) look ugly and not clear to me. IMO, it should be token. +#else +static inline int media_tknres_create(struct device *dev) +{ + return 0; +} +static inline int media_tknres_destroy(struct device *dev) +{ + return 0; +} +static inline int media_get_tuner_tkn(struct device *dev) +{ + return 0; +} +static inline int media_put_tuner_tkn(struct device *dev) +{ + return 0; +} +static int media_get_audio_tkn(struct device *dev) +{ + return 0; +} +static int media_put_audio_tkn(struct device *dev) +{ + return 0; +} +#endif + +#endif /* __LINUX_MEDIA_TOKEN_H */ diff --git a/lib/Makefile b/lib/Makefile index d6b4bc4..6f21695 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -139,6 +139,8 @@ obj-$(CONFIG_DQL) += dynamic_queue_limits.o obj-$(CONFIG_GLOB) += glob.o +obj-$(CONFIG_MEDIA_SUPPORT) += media_tknres.o + obj-$(CONFIG_MPILIB) += mpi/ obj-$(CONFIG_SIGNATURE) += digsig.o diff --git a/lib/media_tknres.c b/lib/media_tknres.c new file mode 100644 index 000..e0a36cb --- /dev/null +++ b/lib/media_tknres.c @@ -0,0 +1,237 @@ +/* + * media_tknres.c - managed media token resource + * + * Copyright (c) 2014 Shuah Khan shua...@osg.samsung.com + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * + * This file is released under the GPLv2. + */ +/* + * Media devices often have hardware resources that are shared + * across several functions. For instance, TV tuner cards often + * have MUXes, converters, radios, tuners, etc. that are shared + * across various functions. However, v4l2, alsa, DVB, usbfs, and + * all other drivers have no knowledge of what resources are + * shared. For example, users can't access DVB and alsa at the same + * time, or the DVB and V4L analog API at the same time, since many + * only have one converter that can be in either analog or digital + * mode. Accessing and/or changing mode of a converter while it is + * in use by another function results in video stream error. + * +
Re: [PATCH v2 1/6] media: add media token device resource framework
At Wed, 15 Oct 2014 18:53:28 -0600, Shuah Khan wrote: On 10/15/2014 11:05 AM, Takashi Iwai wrote: +#if defined(CONFIG_MEDIA_SUPPORT) +extern int media_tknres_create(struct device *dev); +extern int media_tknres_destroy(struct device *dev); + +extern int media_get_tuner_tkn(struct device *dev); +extern int media_put_tuner_tkn(struct device *dev); + +extern int media_get_audio_tkn(struct device *dev); +extern int media_put_audio_tkn(struct device *dev); The words tknres and tkn (especially latter) look ugly and not clear to me. IMO, it should be token. No problem. I can change that to media_token_create/destroy() and expand token in other functions. +struct media_tkn { + spinlock_t lock; + unsigned int owner; /* owner task pid */ + unsigned int tgid; /* owner task gid */ + struct task_struct *task; +}; + +struct media_tknres { + struct media_tkn tuner; + struct media_tkn audio; +}; Why do you need to have both tuner and audio tokens? If I understand correctly, no matter whether it's tuner or audio, if it's being used, the open must fail, right? As it evolved during development, it turns out at the moment I don't have any use-cases that require holding audio and tuner separately. It probably could be collapsed into just a media token. I have to think about this some. + +static int __media_get_tkn(struct media_tkn *tkn, char *tkn_str) +{ + int rc = 0; + unsigned tpid; + unsigned tgid; + + spin_lock(tkn-lock); You should use spin_lock_irqsave() here and in all other places. The trigger callback in usb-audio, for example, may be called in irq context. ok. Good point, will change that. + + tpid = task_pid_nr(current); + tgid = task_tgid_nr(current); + + /* allow task in the same group id to release */ IMO, it's not release but steal... But what happens if the stolen owner calls put? Then it'll be released although the stealing owner still thinks it's being held. Yeah it could be called a steal. :) Essentially tgid happens to be the real owner. I am overwriting the pid with current pid when tgid is same. media dvb and v4l apps start two or more threads that all share the tgid and subsequent token gets should be allowed based on the tgid. Scenario 1: Please note that there are 3 device files in question and media token resource is the lock for all. Hence the changes to v4l-core, dvb-core, and snd-usb-audio to hold the token for exclusive access when the task or tgid don't match. program starts: - first thread opens device file in R/W mode - open gets the token or thread makes ioctls calls that clearly indicates intent to change tuner settings - creates one thread for audio - creates another for video or continues video function - video thread does a close and re-opens the device file In this case when thread tries to close, token put fails unless tasks with same tgid are allowed to release. ( I ran into this one of the media applications and it is a valid case to handle, thread can close the file and should be able to open again without running into token busy case ) - or continue to just use the device file - audio thread does snd_pcm_capture ops - trigger start program exits: - video thread closes the device file - audio thread does snd_pcm_capture ops - trigger stop This got me thinking about the scenario when an application does a fork() as opposed to create a thread. I have to think about this and see how I can address that. + if (tkn-task ((tkn-task != current) (tkn-tgid != tgid))) { Shouldn't the second be || instead? And too many parentheses there. Right - this is my bad. The comment right above this conditional is a give away that, at some point I did a copy and paste from _put to _get and only changed the first task null check. I am yelling at myself at the moment. + rc = -EBUSY; Wrong indentation. Yes. Will fix that. I have a feeling that the whole thing can be a bit more simplified in the end... Any ideas to simplify are welcome. There are a few points to make things more consistent and simplified, IMO. First of all, the whole concept can be more generalized. It's not necessarily only for media and audio. Rather we can make it a generic dev_token. Then it'll become more convincing to land into lib/*. The media-specific handling is only about the pid and gid checks. This can be implemented as a callback that is passed at creating a token, for example. This will reduce the core code in lib/*. Also, get and put should handle a proper refcount. The point I mentioned in my previous post is the possible unbalance, and you'll see unexpected unlock in the scenario. In addition, with use of kref, the lock can be removed. So, essentially the lib code would be just a devres_alloc() for an object with kref
Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api
At Thu, 16 Oct 2014 07:10:37 -0600, Shuah Khan wrote: On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote: On 10/14/2014 04:58 PM, Shuah Khan wrote: [...] switch (cmd) { case SNDRV_PCM_TRIGGER_START: +err = media_get_audio_tkn(subs-dev-dev); +if (err == -EBUSY) { +dev_info(subs-dev-dev, %s device is busy\n, +__func__); In my opinion this should not dev_info() as this is out of band error signaling and also as the potential to spam the log. The userspace application is already properly notified by the return code. Yes it has the potential to flood the dmesg especially with alsa, I will remove the dev_info(). Yes. And, I think doing this in the trigger isn't the best. Why not in open close? Also, is this token restriction needed only for PCM? No mixer or other controls? Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/6] media token resource framework
At Wed, 15 Oct 2014 14:21:34 -0600, Shuah Khan wrote: On 10/15/2014 10:48 AM, Takashi Iwai wrote: At Tue, 14 Oct 2014 08:58:36 -0600, Shuah Khan wrote: Add media token device resource framework to allow sharing resources such as tuner, dma, audio etc. across media drivers and non-media sound drivers that control media hardware. The Media token resource is created at the main struct device that is common to all drivers that claim various pieces of the main media device, which allows them to find the resource using the main struct device. As an example, digital, analog, and snd-usb-audio drivers can use the media token resource API using the main struct device for the interface the media device is attached to. This patch series consists of media token resource framework and changes to use it in dvb-core, v4l2-core, au0828 driver, and snd-usb-audio driver. With these changes dvb and v4l2 can share the tuner without disrupting each other. Used tvtime, xawtv, kaffeine, and vlc, vlc audio capture option, arecord/aplay during development to identify v4l2 vb2 and vb1 ioctls and file operations that disrupt the digital stream and would require changes to check tuner ownership prior to changing the tuner configuration. vb2 changes are made in the v4l2-core and vb1 changes are made in the au0828 driver to encourage porting drivers to vb2 to advantage of the new media token resource framework with changes in the core. In this patch v2 series, fixed problems identified in the patch v1 series. Important ones are changing snd-usb-audio to use media tokens, holding tuner lock in VIDIOC_ENUMINPUT, and VIDIOC_QUERYSTD. Just took a quick glance over the patches, and my first concern is why this has to be lib/*. This means it's always built-in as long as this config is enabled (and will be so on distro kernel) even if it's not used at all. Right this module gets built when CONFIG_MEDIA_SUPPORT is enabled and stubs are in place when it is not enabled. The intent is for this feature to be enabled by default when media support is enabled. When a driver doesn't create the resource, it will simply not find it and for drivers like snd-usb-audio that aren't tried to media support, the stubs are in place and feature is essentially disabled. I picked lib so this module can be included in non-media drivers e.g: snd-usb-audio. Does this help explain the design? I didn't want to introduce a new config for this feature. If lib isn't right place, could you recommend another one that makes this modules available to non-media drivers? moving isn't a problem. We can create a small module depending on CONFIG_MEDIA. But it'll be rather a question of the size. If it's reasonably small and generic enough, it's worth to put into lib/*, I think. Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api
At Thu, 16 Oct 2014 08:10:52 -0600, Shuah Khan wrote: On 10/16/2014 08:01 AM, Takashi Iwai wrote: At Thu, 16 Oct 2014 07:10:37 -0600, Shuah Khan wrote: On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote: On 10/14/2014 04:58 PM, Shuah Khan wrote: [...] switch (cmd) { case SNDRV_PCM_TRIGGER_START: +err = media_get_audio_tkn(subs-dev-dev); +if (err == -EBUSY) { +dev_info(subs-dev-dev, %s device is busy\n, +__func__); In my opinion this should not dev_info() as this is out of band error signaling and also as the potential to spam the log. The userspace application is already properly notified by the return code. Yes it has the potential to flood the dmesg especially with alsa, I will remove the dev_info(). Yes. And, I think doing this in the trigger isn't the best. Why not in open close? My first cut of this change was in open and close. I saw pulseaudio application go into this loop trying to open the device. To avoid such problems, I went with trigger stat and stop. That made all the pulseaudio continues attempts to open problems go away. But now starting the stream gives the error, and PA would loop it again, no? Also, is this token restriction needed only for PCM? No mixer or other controls? snd_pcm_ops are the only ones media drivers implement and use. So I don't think mixer is needed. If it is needed, it is not to hard to add for that case. Well, then I wonder what resource does actually conflict with usb-audio and media drivers at all...? Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api
At Thu, 16 Oct 2014 08:39:14 -0600, Shuah Khan wrote: On 10/16/2014 08:16 AM, Takashi Iwai wrote: At Thu, 16 Oct 2014 08:10:52 -0600, Shuah Khan wrote: On 10/16/2014 08:01 AM, Takashi Iwai wrote: At Thu, 16 Oct 2014 07:10:37 -0600, Shuah Khan wrote: On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote: On 10/14/2014 04:58 PM, Shuah Khan wrote: [...] switch (cmd) { case SNDRV_PCM_TRIGGER_START: +err = media_get_audio_tkn(subs-dev-dev); +if (err == -EBUSY) { +dev_info(subs-dev-dev, %s device is busy\n, +__func__); In my opinion this should not dev_info() as this is out of band error signaling and also as the potential to spam the log. The userspace application is already properly notified by the return code. Yes it has the potential to flood the dmesg especially with alsa, I will remove the dev_info(). Yes. And, I think doing this in the trigger isn't the best. Why not in open close? My first cut of this change was in open and close. I saw pulseaudio application go into this loop trying to open the device. To avoid such problems, I went with trigger stat and stop. That made all the pulseaudio continues attempts to open problems go away. But now starting the stream gives the error, and PA would loop it again, no? Also, is this token restriction needed only for PCM? No mixer or other controls? snd_pcm_ops are the only ones media drivers implement and use. So I don't think mixer is needed. If it is needed, it is not to hard to add for that case. Well, then I wonder what resource does actually conflict with usb-audio and media drivers at all...? audio for dvb/v4l apps gets disrupted when audio app starts. For example, dvb or v4l app tuned to a channel, and when an audio app starts. audio path needs protected to avoid conflicts between digital and analog applications as well. OK, then concentrating on only PCM is fine. But, I'm still not convinced about doing the token management in the trigger. The reason -EBUSY doesn't work is that it's the very same error code when a PCM device is blocked by other processes. And -EAGAIN is interpreted by PCM core to -EBUSY for historical reasons. How applications (e.g. PA) should behave if the token get fails? Shouldn't it retry or totally give up? Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api
At Sat, 18 Oct 2014 20:49:58 +0200, Mauro Carvalho Chehab wrote: Em Thu, 16 Oct 2014 08:59:29 -0600 Shuah Khan shua...@osg.samsung.com escreveu: On 10/16/2014 08:48 AM, Takashi Iwai wrote: At Thu, 16 Oct 2014 08:39:14 -0600, Shuah Khan wrote: On 10/16/2014 08:16 AM, Takashi Iwai wrote: At Thu, 16 Oct 2014 08:10:52 -0600, Shuah Khan wrote: On 10/16/2014 08:01 AM, Takashi Iwai wrote: At Thu, 16 Oct 2014 07:10:37 -0600, Shuah Khan wrote: On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote: On 10/14/2014 04:58 PM, Shuah Khan wrote: [...] switch (cmd) { case SNDRV_PCM_TRIGGER_START: +err = media_get_audio_tkn(subs-dev-dev); +if (err == -EBUSY) { +dev_info(subs-dev-dev, %s device is busy\n, +__func__); In my opinion this should not dev_info() as this is out of band error signaling and also as the potential to spam the log. The userspace application is already properly notified by the return code. Yes it has the potential to flood the dmesg especially with alsa, I will remove the dev_info(). Yes. And, I think doing this in the trigger isn't the best. Why not in open close? My first cut of this change was in open and close. I saw pulseaudio application go into this loop trying to open the device. To avoid such problems, I went with trigger stat and stop. That made all the pulseaudio continues attempts to open problems go away. But now starting the stream gives the error, and PA would loop it again, no? Also, is this token restriction needed only for PCM? No mixer or other controls? snd_pcm_ops are the only ones media drivers implement and use. So I don't think mixer is needed. If it is needed, it is not to hard to add for that case. Well, then I wonder what resource does actually conflict with usb-audio and media drivers at all...? audio for dvb/v4l apps gets disrupted when audio app starts. For example, dvb or v4l app tuned to a channel, and when an audio app starts. audio path needs protected to avoid conflicts between digital and analog applications as well. OK, then concentrating on only PCM is fine. But, I'm still not convinced about doing the token management in the trigger. The reason -EBUSY doesn't work is that it's the very same error code when a PCM device is blocked by other processes. And -EAGAIN is interpreted by PCM core to -EBUSY for historical reasons. ah. ok your recommendation is to go with open and close. Mauro has some reservations with holding at open when I discussed my observations with pulseaudio when I was holding token in open instead of trigger start. Maybe he can chime with his concerns. I think his concern was breaking applications if token is held in open(). Yes. My concern is that PA has weird behaviors, and it tries to open and keep opened all audio devices. PA usually closes the PCM devices when unused. If it doesn't, it must be a bug. Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/6] media: add media token device resource framework
At Tue, 21 Oct 2014 12:46:03 +0200, Hans Verkuil wrote: Hi Shuah, As promised, here is my review for this patch series. On 10/14/2014 04:58 PM, Shuah Khan wrote: Add media token device resource framework to allow sharing resources such as tuner, dma, audio etc. across media drivers and non-media sound drivers that control media hardware. The Media token resource is created at the main struct device that is common to all drivers that claim various pieces of the main media device, which allows them to find the resource using the main struct device. As an example, digital, analog, and snd-usb-audio drivers can use the media token resource API using the main struct device for the interface the media device is attached to. A shared media tokens resource is created using devres framework for drivers to find and lock/unlock. Creating a shared devres helps avoid creating data structure dependencies between drivers. This media token resource contains media token for tuner, and audio. When tuner token is requested, audio token is issued. Did you mean: 'tuner token is issued' instead of audio token? I also have the same question as Takashi: why do we have an audio token in the first place? While you are streaming audio over alsa the underlying tuner must be marked as being in use. It's all about the tuner, since that's the resource that is being shared, not about audio as such. For the remainder of my review I will ignore the audio-related code and concentrate only on the tuner part. Subsequent token (for tuner and audio) gets from the same task and task in the same tgid succeed. This allows applications that make multiple v4l2 ioctls to work with the first call acquiring the token and applications that create separate threads to handle video and audio functions. Signed-off-by: Shuah Khan shua...@osg.samsung.com --- MAINTAINERS |2 + include/linux/media_tknres.h | 50 + lib/Makefile |2 + lib/media_tknres.c | 237 ++ I am still not convinced myself that this should be a generic API. The only reason we need it today is for sharing tuners. Which is almost a purely media thing with USB audio as the single non-media driver that will be affected. Today I see no use case outside of tuners. I would probably want to keep this inside drivers/media. If this is going to be expanded it can always be moved to lib later. Well, my argument is that it should be more generic if it were intended to be put in lib. It'd be fine to put into drivers/media, but this code snippet must be a separate module. Otherwise usb-audio would grab the whole media stuff even if not needed at all. (snip) I also discovered that you are missing MODULE_AUTHOR, MODULE_DESCRIPTION and above all MODULE_LICENSE. Without the MODULE_LICENSE it won't link this module to the GPL devres_* functions. It took me some time to figure that out. It was a code in lib, so it cannot be a module at all :) Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/6] media: add media token device resource framework
At Tue, 21 Oct 2014 13:58:59 +0200, Hans Verkuil wrote: On 10/21/2014 01:51 PM, Takashi Iwai wrote: At Tue, 21 Oct 2014 12:46:03 +0200, Hans Verkuil wrote: Hi Shuah, As promised, here is my review for this patch series. On 10/14/2014 04:58 PM, Shuah Khan wrote: Add media token device resource framework to allow sharing resources such as tuner, dma, audio etc. across media drivers and non-media sound drivers that control media hardware. The Media token resource is created at the main struct device that is common to all drivers that claim various pieces of the main media device, which allows them to find the resource using the main struct device. As an example, digital, analog, and snd-usb-audio drivers can use the media token resource API using the main struct device for the interface the media device is attached to. A shared media tokens resource is created using devres framework for drivers to find and lock/unlock. Creating a shared devres helps avoid creating data structure dependencies between drivers. This media token resource contains media token for tuner, and audio. When tuner token is requested, audio token is issued. Did you mean: 'tuner token is issued' instead of audio token? I also have the same question as Takashi: why do we have an audio token in the first place? While you are streaming audio over alsa the underlying tuner must be marked as being in use. It's all about the tuner, since that's the resource that is being shared, not about audio as such. For the remainder of my review I will ignore the audio-related code and concentrate only on the tuner part. Subsequent token (for tuner and audio) gets from the same task and task in the same tgid succeed. This allows applications that make multiple v4l2 ioctls to work with the first call acquiring the token and applications that create separate threads to handle video and audio functions. Signed-off-by: Shuah Khan shua...@osg.samsung.com --- MAINTAINERS |2 + include/linux/media_tknres.h | 50 + lib/Makefile |2 + lib/media_tknres.c | 237 ++ I am still not convinced myself that this should be a generic API. The only reason we need it today is for sharing tuners. Which is almost a purely media thing with USB audio as the single non-media driver that will be affected. Today I see no use case outside of tuners. I would probably want to keep this inside drivers/media. If this is going to be expanded it can always be moved to lib later. Well, my argument is that it should be more generic if it were intended to be put in lib. It'd be fine to put into drivers/media, but this code snippet must be a separate module. Otherwise usb-audio would grab the whole media stuff even if not needed at all. Certainly. (snip) I also discovered that you are missing MODULE_AUTHOR, MODULE_DESCRIPTION and above all MODULE_LICENSE. Without the MODULE_LICENSE it won't link this module to the GPL devres_* functions. It took me some time to figure that out. It was a code in lib, so it cannot be a module at all :) Well, it depends on CONFIG_MEDIA_SUPPORT which is 'm' in my case, so it compiles as a module :-) Ah, I thought it was a boolean, but yes, this can be a module indeed. Then I don't think it's worth to put in lib. Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api
At Tue, 21 Oct 2014 17:42:51 +0200, Hans Verkuil wrote: On 10/16/2014 04:48 PM, Takashi Iwai wrote: At Thu, 16 Oct 2014 08:39:14 -0600, Shuah Khan wrote: On 10/16/2014 08:16 AM, Takashi Iwai wrote: At Thu, 16 Oct 2014 08:10:52 -0600, Shuah Khan wrote: On 10/16/2014 08:01 AM, Takashi Iwai wrote: At Thu, 16 Oct 2014 07:10:37 -0600, Shuah Khan wrote: On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote: On 10/14/2014 04:58 PM, Shuah Khan wrote: [...] switch (cmd) { case SNDRV_PCM_TRIGGER_START: +err = media_get_audio_tkn(subs-dev-dev); +if (err == -EBUSY) { +dev_info(subs-dev-dev, %s device is busy\n, +__func__); In my opinion this should not dev_info() as this is out of band error signaling and also as the potential to spam the log. The userspace application is already properly notified by the return code. Yes it has the potential to flood the dmesg especially with alsa, I will remove the dev_info(). Yes. And, I think doing this in the trigger isn't the best. Why not in open close? My first cut of this change was in open and close. I saw pulseaudio application go into this loop trying to open the device. To avoid such problems, I went with trigger stat and stop. That made all the pulseaudio continues attempts to open problems go away. But now starting the stream gives the error, and PA would loop it again, no? Also, is this token restriction needed only for PCM? No mixer or other controls? snd_pcm_ops are the only ones media drivers implement and use. So I don't think mixer is needed. If it is needed, it is not to hard to add for that case. Well, then I wonder what resource does actually conflict with usb-audio and media drivers at all...? audio for dvb/v4l apps gets disrupted when audio app starts. For example, dvb or v4l app tuned to a channel, and when an audio app starts. audio path needs protected to avoid conflicts between digital and analog applications as well. OK, then concentrating on only PCM is fine. But, I'm still not convinced about doing the token management in the trigger. The reason -EBUSY doesn't work is that it's the very same error code when a PCM device is blocked by other processes. And -EAGAIN is interpreted by PCM core to -EBUSY for historical reasons. How applications (e.g. PA) should behave if the token get fails? Shouldn't it retry or totally give up? Quite often media apps open the alsa device at the start and then switch between TV, radio or DVB mode. If the alsa device would claim the tuner just by being opened (as opposed to actually using the tuner, which happens when you start streaming), What about parameter changes? The sound devices have to be configured before using. Don't they influence on others at all, i.e. you can change the PCM sample rate etc during TV, radio or DVB is running? then that would make it impossible for the application to switch tuner mode. In general you want to avoid that open() will start configuring hardware since that can quite often be slow. Tuner configuration in particular can be slow since several common tuners need to load firmware over i2c. You only want to do that when it is really needed, and not when some application (udev!) opens the device just to examine what sort of device it is. But most apps close the device soon after that, no? Which programs keep the PCM device (not the control) opened without actually using? So claiming the tuner in the trigger seems to be the right place. If returning EBUSY is a poor error code for alsa, then we can use something else for that. EACCES perhaps? Sorry, I'm not convinced by that. If the device has to be controlled exclusively, the right position is the open/close. Otherwise, the program cannot know when it becomes inaccessible out of sudden during its operation. Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api
At Tue, 21 Oct 2014 12:08:59 -0400, Devin Heitmueller wrote: Sorry, I'm not convinced by that. If the device has to be controlled exclusively, the right position is the open/close. Otherwise, the program cannot know when it becomes inaccessible out of sudden during its operation. I can say that I've definitely seen cases where if you configure a device as the default capture device in PulseAudio, then pulse will continue to capture from it even if you're not actively capturing the audio from pulse. I only spotted this because I had a USB analyzer on the device and was dumbfounded when the ISOC packets kept arriving even after I had closed VLC. You might have had an input monitor active in some PA apps? PA shouldn't do it as default. If it does unintentionally, you should report it to PA guys. Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] DocBook: Reduce noise from make cleandocs
I've got a harmless warning when running make cleandocs on an already cleaned tree: Documentation/DocBook/media/Makefile:28: recipe for target 'cleanmediadocs' failed make[1]: [cleanmediadocs] Error 1 (ignored) Suppress this by passing -f to rm. Signed-off-by: Takashi Iwai ti...@suse.de --- Documentation/DocBook/media/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/DocBook/media/Makefile b/Documentation/DocBook/media/Makefile index df2962d9e11e..8bf7c6191296 100644 --- a/Documentation/DocBook/media/Makefile +++ b/Documentation/DocBook/media/Makefile @@ -25,7 +25,7 @@ GENFILES := $(addprefix $(MEDIA_OBJ_DIR)/, $(MEDIA_TEMP)) PHONY += cleanmediadocs cleanmediadocs: - -@rm `find $(MEDIA_OBJ_DIR) -type l` $(GENFILES) $(OBJIMGFILES) 2/dev/null + -@rm -f `find $(MEDIA_OBJ_DIR) -type l` $(GENFILES) $(OBJIMGFILES) 2/dev/null $(obj)/media_api.xml: $(GENFILES) FORCE -- 2.1.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] uvc: Fix destruction order in uvc_delete()
We've got a bug report at disconnecting a Webcam, where the kernel spews warnings like below: WARNING: CPU: 0 PID: 8385 at ../fs/sysfs/group.c:219 sysfs_remove_group+0x87/0x90() sysfs group c0b2350c not found for kobject 'event3' CPU: 0 PID: 8385 Comm: queue2:src Not tainted 3.16.2-1.gdcee397-default #1 Hardware name: ASUSTeK Computer INC. A7N8X-E/A7N8X-E, BIOS ASUS A7N8X-E Deluxe ACPI BIOS Rev 1013 11/12/2004 c08d0705 ddc75cbc c0718c5b ddc75ccc c024b654 c08c6d44 ddc75ce8 20c1 c08d0705 00db c03d1ec7 c03d1ec7 0009 c0b2350c d62c9064 ddc75cd4 c024b6a3 0009 ddc75ccc c08c6d44 ddc75ce8 ddc75cfc c03d1ec7 Call Trace: [c0205ba6] try_stack_unwind+0x156/0x170 [c02046f3] dump_trace+0x53/0x180 [c0205c06] show_trace_log_lvl+0x46/0x50 [c0204871] show_stack_log_lvl+0x51/0xe0 [c0205c67] show_stack+0x27/0x50 [c0718c5b] dump_stack+0x3e/0x4e [c024b654] warn_slowpath_common+0x84/0xa0 [c024b6a3] warn_slowpath_fmt+0x33/0x40 [c03d1ec7] sysfs_remove_group+0x87/0x90 [c05a2c54] device_del+0x34/0x180 [c05e3989] evdev_disconnect+0x19/0x50 [c05e06fa] __input_unregister_device+0x9a/0x140 [c05e0845] input_unregister_device+0x45/0x80 [f854b1d6] uvc_delete+0x26/0x110 [uvcvideo] [f84d66f8] v4l2_device_release+0x98/0xc0 [videodev] [c05a25bb] device_release+0x2b/0x90 [c04ad8bf] kobject_cleanup+0x6f/0x1a0 [f84d5453] v4l2_release+0x43/0x70 [videodev] [c0372f31] __fput+0xb1/0x1b0 [c02650c1] task_work_run+0x91/0xb0 [c024d845] do_exit+0x265/0x910 [c024df64] do_group_exit+0x34/0xa0 [c025a76f] get_signal_to_deliver+0x17f/0x590 [c0201b6a] do_signal+0x3a/0x960 [c02024f7] do_notify_resume+0x67/0x90 [c071ebb5] work_notifysig+0x30/0x3b [b7739e60] 0xb7739e5f ---[ end trace b1e56095a485b631 ]--- The cause is that uvc_status_cleanup() is called after usb_put_*() in uvc_delete(). usb_put_*() removes the sysfs parent and eventually removes the children recursively, so the later device_del() can't find its sysfs. The fix is simply rearrange the call orders in uvc_delete() so that the child is removed before the parent. Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=897736 Reported-and-tested-by: Martin Pluskal mplus...@suse.com Cc: sta...@vger.kernel.org Signed-off-by: Takashi Iwai ti...@suse.de --- drivers/media/usb/uvc/uvc_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 7c8322d4fc63..3c07af96b30f 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -1623,12 +1623,12 @@ static void uvc_delete(struct uvc_device *dev) { struct list_head *p, *n; - usb_put_intf(dev-intf); - usb_put_dev(dev-udev); - uvc_status_cleanup(dev); uvc_ctrl_cleanup_device(dev); + usb_put_intf(dev-intf); + usb_put_dev(dev-udev); + if (dev-vdev.dev) v4l2_device_unregister(dev-vdev); #ifdef CONFIG_MEDIA_CONTROLLER -- 2.1.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api
At Sat, 25 Oct 2014 11:41:15 -0200, Mauro Carvalho Chehab wrote: (re-sending from my third e-mail - somehow, the two emails I have at Samsung didn't seem to be delivering to vger.kernel.org today) Em Wed, 22 Oct 2014 14:26:41 -0500 Pierre-Louis Bossart pierre-louis.boss...@linux.intel.com escreveu: On 10/21/14, 11:08 AM, Devin Heitmueller wrote: Sorry, I'm not convinced by that. If the device has to be controlled exclusively, the right position is the open/close. Otherwise, the program cannot know when it becomes inaccessible out of sudden during its operation. I can say that I've definitely seen cases where if you configure a device as the default capture device in PulseAudio, then pulse will continue to capture from it even if you're not actively capturing the audio from pulse. I only spotted this because I had a USB analyzer on the device and was dumbfounded when the ISOC packets kept arriving even after I had closed VLC. this seems like a feature, not a bug. PulseAudio starts streaming before clients push any data and likewise keeps sources active even after for some time after clients stop recording. Closing VLC in your example doesn't immediately close the ALSA device. look for module-suspend-on-idle in your default.pa config file. This could be a feature for an audio capture device that is standalone, but for sure it isn't a feature for an audio capture device where the audio is only available if video is also being streamed. A V4L device with ALSA capture is a different beast than a standalone capture port. In a simplified way, it will basically follow the following state machine: +---+ | start | +---+ | | v ++ | idle | + ++ | |^ || || || v| || +---+ | || | configuration | | || +---+ | || || || || || v| || +---+ | || + | streaming | -+ || | +---+ || |||| |||| |vv| | +---+---++ | +- | 1 | suspended | 2 | -+ +---+---++ 1) start state This is when the V4L2 device gots probed. It checks if the hardware is present and initializes some vars/registers, turning off everything that can be powered down. The tuner on put in sleep mode, analog audio/video decoders and the dvb frontend and demux are also turned off. 2) idle state As the device is powered off, audio won't produce anything. Depending on the device, reading for audio may return a sequence of zeros, or may even fail, as the DMA engine is not ready yet for streaming. Also, the audio mixer is muted, but the audio input switch is on a random state. 2) configuration state When V4L2 node device is opened and configured, the audio mixer will be switched to input audio from the same source of the video stream. The corresponding audio input is also unmuted. Almost all devices have at least two audio/video inputs: TV TUNER and COMPOSITE. Other devices may also have S-VIDEO, COMPOSITE 2, RADIO TUNER, etc. If the device is set on TUNER mode, on modern devices, a tuner firmware will be loaded. That may require a long time. Typically, most devices take 1/2 seconds to load a firmware, but some devices may take up to 30 seconds. The firmware may depend on the TV standard that will be used, so this can't be loaded at driver warm up state. Also, the power consumption of the tuners is high (it can be ~100-200 mW or more when powered, and ~16mW when just I2C is powered). We don't want to keep it powered when the device is not used, as this spends battery. Also, the life of the device reduces a lot if we keep it always powered. During this stage, if an ALSA call is issued, it may interfere at the device settings and/or firmware load, with can cause the audio to fail. On such cases, applications might need to close the V4L2 node and re-open again. 3) streaming state The change to this staging requires a V4L2 ioctl. Please notice, however, that some apps will open the audio device before the V4L2 node, while others will open it after that. In any case, audio will only start to produce data after the V4L2 ioctl at V4L2 that starts the DMA engine there. After that ioctl: - Audio PCM capture will work; - The mixers will be in a good state: unmuted,
Re: [PATCH 1/2] [media] sound: simplify au0828 quirk table
At Thu, 30 Oct 2014 08:53:04 -0200, Mauro Carvalho Chehab wrote: From: Mauro Carvalho Chehab m.che...@samsung.com Add a macro to simplify au0828 quirk table. That makes easier to check it against the USB IDs at drivers/media/usb/au0828-card.c Cc: sta...@vger.kernel.org Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com Which sign-off should I take? Judging from the author line, the former one? The second patch had only s-o-b from @samsung.com. thanks, Takashi diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h index c657752a420c..5ae1d02d17a3 100644 --- a/sound/usb/quirks-table.h +++ b/sound/usb/quirks-table.h @@ -2804,133 +2804,37 @@ YAMAHA_DEVICE(0x7010, UB99), } }, -/* Hauppauge HVR-950Q and HVR-850 */ -{ - USB_DEVICE_VENDOR_SPEC(0x2040, 0x7200), - .match_flags = USB_DEVICE_ID_MATCH_DEVICE | -USB_DEVICE_ID_MATCH_INT_CLASS | -USB_DEVICE_ID_MATCH_INT_SUBCLASS, - .bInterfaceClass = USB_CLASS_AUDIO, - .bInterfaceSubClass = USB_SUBCLASS_AUDIOCONTROL, - .driver_info = (unsigned long) (const struct snd_usb_audio_quirk) { - .vendor_name = Hauppauge, - .product_name = HVR-950Q, - .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, - } -}, -{ - USB_DEVICE_VENDOR_SPEC(0x2040, 0x7210), - .match_flags = USB_DEVICE_ID_MATCH_DEVICE | -USB_DEVICE_ID_MATCH_INT_CLASS | -USB_DEVICE_ID_MATCH_INT_SUBCLASS, - .bInterfaceClass = USB_CLASS_AUDIO, - .bInterfaceSubClass = USB_SUBCLASS_AUDIOCONTROL, - .driver_info = (unsigned long) (const struct snd_usb_audio_quirk) { - .vendor_name = Hauppauge, - .product_name = HVR-950Q, - .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, - } -}, -{ - USB_DEVICE_VENDOR_SPEC(0x2040, 0x7217), - .match_flags = USB_DEVICE_ID_MATCH_DEVICE | -USB_DEVICE_ID_MATCH_INT_CLASS | -USB_DEVICE_ID_MATCH_INT_SUBCLASS, - .bInterfaceClass = USB_CLASS_AUDIO, - .bInterfaceSubClass = USB_SUBCLASS_AUDIOCONTROL, - .driver_info = (unsigned long) (const struct snd_usb_audio_quirk) { - .vendor_name = Hauppauge, - .product_name = HVR-950Q, - .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, - } -}, -{ - USB_DEVICE_VENDOR_SPEC(0x2040, 0x721b), - .match_flags = USB_DEVICE_ID_MATCH_DEVICE | -USB_DEVICE_ID_MATCH_INT_CLASS | -USB_DEVICE_ID_MATCH_INT_SUBCLASS, - .bInterfaceClass = USB_CLASS_AUDIO, - .bInterfaceSubClass = USB_SUBCLASS_AUDIOCONTROL, - .driver_info = (unsigned long) (const struct snd_usb_audio_quirk) { - .vendor_name = Hauppauge, - .product_name = HVR-950Q, - .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, - } -}, -{ - USB_DEVICE_VENDOR_SPEC(0x2040, 0x721e), - .match_flags = USB_DEVICE_ID_MATCH_DEVICE | -USB_DEVICE_ID_MATCH_INT_CLASS | -USB_DEVICE_ID_MATCH_INT_SUBCLASS, - .bInterfaceClass = USB_CLASS_AUDIO, - .bInterfaceSubClass = USB_SUBCLASS_AUDIOCONTROL, - .driver_info = (unsigned long) (const struct snd_usb_audio_quirk) { - .vendor_name = Hauppauge, - .product_name = HVR-950Q, - .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, - } -}, -{ - USB_DEVICE_VENDOR_SPEC(0x2040, 0x721f), - .match_flags = USB_DEVICE_ID_MATCH_DEVICE | -USB_DEVICE_ID_MATCH_INT_CLASS | -USB_DEVICE_ID_MATCH_INT_SUBCLASS, - .bInterfaceClass = USB_CLASS_AUDIO, - .bInterfaceSubClass = USB_SUBCLASS_AUDIOCONTROL, - .driver_info = (unsigned long) (const struct snd_usb_audio_quirk) { - .vendor_name = Hauppauge, - .product_name = HVR-950Q, - .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, - } -}, -{ - USB_DEVICE_VENDOR_SPEC(0x2040, 0x7240), - .match_flags = USB_DEVICE_ID_MATCH_DEVICE | -USB_DEVICE_ID_MATCH_INT_CLASS | -USB_DEVICE_ID_MATCH_INT_SUBCLASS, - .bInterfaceClass = USB_CLASS_AUDIO, - .bInterfaceSubClass = USB_SUBCLASS_AUDIOCONTROL, - .driver_info = (unsigned long) (const struct snd_usb_audio_quirk) { - .vendor_name = Hauppauge, - .product_name = HVR-850, - .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_AUDIO_ALIGN_TRANSFER, - } -}, -{ - USB_DEVICE_VENDOR_SPEC(0x2040, 0x7280), - .match_flags = USB_DEVICE_ID_MATCH_DEVICE | -
Re: DVB suspend/resume regression on 3.19
At Fri, 13 Feb 2015 16:12:40 +0100, Takashi Iwai wrote: At Fri, 13 Feb 2015 12:41:25 -0200, Mauro Carvalho Chehab wrote: Em Fri, 13 Feb 2015 15:02:42 +0100 Takashi Iwai ti...@suse.de escreveu: At Mon, 09 Feb 2015 11:59:07 +0100, Takashi Iwai wrote: Hi, we've got a bug report about the suspend/resume regression of DVB device with 3.19. The symptom is VLC doesn't work after S3 or S4 resume. strace shows that /dev/dvb/adaptor0/dvr returns -ENODEV. The reporter confirmed that 3.18 works fine, so the regression must be in 3.19. There is a relevant kernel warning while suspending: WARNING: CPU: 1 PID: 3603 at ../kernel/module.c:1001 module_put+0xc7/0xd0() Workqueue: events_unbound async_run_entry_fn 81a45779 81664f12 81062381 a051eea0 8800ca369278 a051a068 8800c0a18090 810dfb47 Call Trace: [810055ac] dump_trace+0x8c/0x340 [81005903] show_stack_log_lvl+0xa3/0x190 [81007061] show_stack+0x21/0x50 [81664f12] dump_stack+0x47/0x67 [81062381] warn_slowpath_common+0x81/0xb0 [810dfb47] module_put+0xc7/0xd0 [a04d98d1] dvb_usb_adapter_frontend_exit+0x41/0x60 [dvb_usb] [a04d8451] dvb_usb_exit+0x31/0xa0 [dvb_usb] [a04d84fb] dvb_usb_device_exit+0x3b/0x50 [dvb_usb] [814cefad] usb_unbind_interface+0x1ed/0x2c0 [8145ceae] __device_release_driver+0x7e/0x100 [8145cf52] device_release_driver+0x22/0x30 [814cf13d] usb_forced_unbind_intf+0x2d/0x60 [814cf3c3] usb_suspend+0x73/0x130 [814bd453] usb_dev_freeze+0x13/0x20 [81468fca] dpm_run_callback+0x4a/0x150 [81469c81] __device_suspend+0x121/0x350 [81469ece] async_suspend+0x1e/0xa0 [81081e63] async_run_entry_fn+0x43/0x150 [81079e72] process_one_work+0x142/0x3f0 [8107a234] worker_thread+0x114/0x460 [8107f3b1] kthread+0xc1/0xe0 [8166b77c] ret_from_fork+0x7c/0xb0 So something went wrong in module refcount, which likely leads to disabling the device and returning -ENODEV in the end. Does this ring a bell to you guys? The hardware details and logs are found in the URL below: https://bugzilla.novell.com/show_bug.cgi?id=916577 I wonder whether no one hits the same problem...? Hi Takashi, There were no recent changes at dvb-usb core or at the dtt200u.c driver. So, I don't think that the regression was caused by a change at the media subsystem. Yet, we've added some patches to fix suspend/resume at the DVB core, but they basically add a new set of optional callbacks. So, it should not cause any regression. The changeset in question is 59d7889ae49f6e3e9d9cff8c0de7ad95d9ca068b. Hm, is this commit really merged between 3.18 and 3.19? From those messages: 2015-02-06T15:30:47.468258+01:00 linux-z24t kernel: [ 150.552119] dvb-usb: downloading firmware from file 'dvb-usb-wt220u-fc03.fw' 2015-02-06T15:30:47.468827+01:00 linux-z24t mtp-probe: checking bus 1, device 5: /sys/devices/pci:00/:00:1d.7/usb1/1-4 2015-02-06T15:30:47.879878+01:00 linux-z24t mtp-probe: bus: 1, device: 5 was not an MTP device 2015-02-06T15:30:47.880192+01:00 linux-z24t kernel: [ 151.992993] usb 1-4: USB disconnect, device number 5 2015-02-06T15:30:47.880839+01:00 linux-z24t kernel: [ 151.993076] dvb-usb: generic DVB-USB module successfully deinitialized and disconnected. I _suspect_ that dvb_usb_download_firmware() failed to load the firmware file. That problem actually looks like a recently discovered issue:t request_firmware() fails during resume on some systems. What seems to happen in this case is that the media drivers are resumed _before_ mounting the partition where the firmware file is hosted. Yet, in this case, it should be printing: did not find the firmware file. Right. This makes me wonder, too. I would add a test patch in order to print the return code from dvb_usb_download_firmware(), in order to see if it succeeds or not, e. g. something like the patch below, and then try to narrow down where it is failing, assuming that the new message will be printed. If nothing gets printed, then I would try to discover why the USB stack disconnected the device. Perhaps an ehci/xhci bug? OK, I'll give a test kernel to report. The patched kernel didn't show the message. So it's not about the firmware load failure, apparently. Any other point to check? Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http
Re: DVB suspend/resume regression on 3.19
At Mon, 09 Feb 2015 11:59:07 +0100, Takashi Iwai wrote: Hi, we've got a bug report about the suspend/resume regression of DVB device with 3.19. The symptom is VLC doesn't work after S3 or S4 resume. strace shows that /dev/dvb/adaptor0/dvr returns -ENODEV. The reporter confirmed that 3.18 works fine, so the regression must be in 3.19. There is a relevant kernel warning while suspending: WARNING: CPU: 1 PID: 3603 at ../kernel/module.c:1001 module_put+0xc7/0xd0() Workqueue: events_unbound async_run_entry_fn 81a45779 81664f12 81062381 a051eea0 8800ca369278 a051a068 8800c0a18090 810dfb47 Call Trace: [810055ac] dump_trace+0x8c/0x340 [81005903] show_stack_log_lvl+0xa3/0x190 [81007061] show_stack+0x21/0x50 [81664f12] dump_stack+0x47/0x67 [81062381] warn_slowpath_common+0x81/0xb0 [810dfb47] module_put+0xc7/0xd0 [a04d98d1] dvb_usb_adapter_frontend_exit+0x41/0x60 [dvb_usb] [a04d8451] dvb_usb_exit+0x31/0xa0 [dvb_usb] [a04d84fb] dvb_usb_device_exit+0x3b/0x50 [dvb_usb] [814cefad] usb_unbind_interface+0x1ed/0x2c0 [8145ceae] __device_release_driver+0x7e/0x100 [8145cf52] device_release_driver+0x22/0x30 [814cf13d] usb_forced_unbind_intf+0x2d/0x60 [814cf3c3] usb_suspend+0x73/0x130 [814bd453] usb_dev_freeze+0x13/0x20 [81468fca] dpm_run_callback+0x4a/0x150 [81469c81] __device_suspend+0x121/0x350 [81469ece] async_suspend+0x1e/0xa0 [81081e63] async_run_entry_fn+0x43/0x150 [81079e72] process_one_work+0x142/0x3f0 [8107a234] worker_thread+0x114/0x460 [8107f3b1] kthread+0xc1/0xe0 [8166b77c] ret_from_fork+0x7c/0xb0 So something went wrong in module refcount, which likely leads to disabling the device and returning -ENODEV in the end. Does this ring a bell to you guys? The hardware details and logs are found in the URL below: https://bugzilla.novell.com/show_bug.cgi?id=916577 I wonder whether no one hits the same problem...? Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: DVB suspend/resume regression on 3.19
At Fri, 13 Feb 2015 12:41:25 -0200, Mauro Carvalho Chehab wrote: Em Fri, 13 Feb 2015 15:02:42 +0100 Takashi Iwai ti...@suse.de escreveu: At Mon, 09 Feb 2015 11:59:07 +0100, Takashi Iwai wrote: Hi, we've got a bug report about the suspend/resume regression of DVB device with 3.19. The symptom is VLC doesn't work after S3 or S4 resume. strace shows that /dev/dvb/adaptor0/dvr returns -ENODEV. The reporter confirmed that 3.18 works fine, so the regression must be in 3.19. There is a relevant kernel warning while suspending: WARNING: CPU: 1 PID: 3603 at ../kernel/module.c:1001 module_put+0xc7/0xd0() Workqueue: events_unbound async_run_entry_fn 81a45779 81664f12 81062381 a051eea0 8800ca369278 a051a068 8800c0a18090 810dfb47 Call Trace: [810055ac] dump_trace+0x8c/0x340 [81005903] show_stack_log_lvl+0xa3/0x190 [81007061] show_stack+0x21/0x50 [81664f12] dump_stack+0x47/0x67 [81062381] warn_slowpath_common+0x81/0xb0 [810dfb47] module_put+0xc7/0xd0 [a04d98d1] dvb_usb_adapter_frontend_exit+0x41/0x60 [dvb_usb] [a04d8451] dvb_usb_exit+0x31/0xa0 [dvb_usb] [a04d84fb] dvb_usb_device_exit+0x3b/0x50 [dvb_usb] [814cefad] usb_unbind_interface+0x1ed/0x2c0 [8145ceae] __device_release_driver+0x7e/0x100 [8145cf52] device_release_driver+0x22/0x30 [814cf13d] usb_forced_unbind_intf+0x2d/0x60 [814cf3c3] usb_suspend+0x73/0x130 [814bd453] usb_dev_freeze+0x13/0x20 [81468fca] dpm_run_callback+0x4a/0x150 [81469c81] __device_suspend+0x121/0x350 [81469ece] async_suspend+0x1e/0xa0 [81081e63] async_run_entry_fn+0x43/0x150 [81079e72] process_one_work+0x142/0x3f0 [8107a234] worker_thread+0x114/0x460 [8107f3b1] kthread+0xc1/0xe0 [8166b77c] ret_from_fork+0x7c/0xb0 So something went wrong in module refcount, which likely leads to disabling the device and returning -ENODEV in the end. Does this ring a bell to you guys? The hardware details and logs are found in the URL below: https://bugzilla.novell.com/show_bug.cgi?id=916577 I wonder whether no one hits the same problem...? Hi Takashi, There were no recent changes at dvb-usb core or at the dtt200u.c driver. So, I don't think that the regression was caused by a change at the media subsystem. Yet, we've added some patches to fix suspend/resume at the DVB core, but they basically add a new set of optional callbacks. So, it should not cause any regression. The changeset in question is 59d7889ae49f6e3e9d9cff8c0de7ad95d9ca068b. Hm, is this commit really merged between 3.18 and 3.19? From those messages: 2015-02-06T15:30:47.468258+01:00 linux-z24t kernel: [ 150.552119] dvb-usb: downloading firmware from file 'dvb-usb-wt220u-fc03.fw' 2015-02-06T15:30:47.468827+01:00 linux-z24t mtp-probe: checking bus 1, device 5: /sys/devices/pci:00/:00:1d.7/usb1/1-4 2015-02-06T15:30:47.879878+01:00 linux-z24t mtp-probe: bus: 1, device: 5 was not an MTP device 2015-02-06T15:30:47.880192+01:00 linux-z24t kernel: [ 151.992993] usb 1-4: USB disconnect, device number 5 2015-02-06T15:30:47.880839+01:00 linux-z24t kernel: [ 151.993076] dvb-usb: generic DVB-USB module successfully deinitialized and disconnected. I _suspect_ that dvb_usb_download_firmware() failed to load the firmware file. That problem actually looks like a recently discovered issue:t request_firmware() fails during resume on some systems. What seems to happen in this case is that the media drivers are resumed _before_ mounting the partition where the firmware file is hosted. Yet, in this case, it should be printing: did not find the firmware file. Right. This makes me wonder, too. I would add a test patch in order to print the return code from dvb_usb_download_firmware(), in order to see if it succeeds or not, e. g. something like the patch below, and then try to narrow down where it is failing, assuming that the new message will be printed. If nothing gets printed, then I would try to discover why the USB stack disconnected the device. Perhaps an ehci/xhci bug? OK, I'll give a test kernel to report. Thanks! Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
DVB suspend/resume regression on 3.19
Hi, we've got a bug report about the suspend/resume regression of DVB device with 3.19. The symptom is VLC doesn't work after S3 or S4 resume. strace shows that /dev/dvb/adaptor0/dvr returns -ENODEV. The reporter confirmed that 3.18 works fine, so the regression must be in 3.19. There is a relevant kernel warning while suspending: WARNING: CPU: 1 PID: 3603 at ../kernel/module.c:1001 module_put+0xc7/0xd0() Workqueue: events_unbound async_run_entry_fn 81a45779 81664f12 81062381 a051eea0 8800ca369278 a051a068 8800c0a18090 810dfb47 Call Trace: [810055ac] dump_trace+0x8c/0x340 [81005903] show_stack_log_lvl+0xa3/0x190 [81007061] show_stack+0x21/0x50 [81664f12] dump_stack+0x47/0x67 [81062381] warn_slowpath_common+0x81/0xb0 [810dfb47] module_put+0xc7/0xd0 [a04d98d1] dvb_usb_adapter_frontend_exit+0x41/0x60 [dvb_usb] [a04d8451] dvb_usb_exit+0x31/0xa0 [dvb_usb] [a04d84fb] dvb_usb_device_exit+0x3b/0x50 [dvb_usb] [814cefad] usb_unbind_interface+0x1ed/0x2c0 [8145ceae] __device_release_driver+0x7e/0x100 [8145cf52] device_release_driver+0x22/0x30 [814cf13d] usb_forced_unbind_intf+0x2d/0x60 [814cf3c3] usb_suspend+0x73/0x130 [814bd453] usb_dev_freeze+0x13/0x20 [81468fca] dpm_run_callback+0x4a/0x150 [81469c81] __device_suspend+0x121/0x350 [81469ece] async_suspend+0x1e/0xa0 [81081e63] async_run_entry_fn+0x43/0x150 [81079e72] process_one_work+0x142/0x3f0 [8107a234] worker_thread+0x114/0x460 [8107f3b1] kthread+0xc1/0xe0 [8166b77c] ret_from_fork+0x7c/0xb0 So something went wrong in module refcount, which likely leads to disabling the device and returning -ENODEV in the end. Does this ring a bell to you guys? The hardware details and logs are found in the URL below: https://bugzilla.novell.com/show_bug.cgi?id=916577 thanks, Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] sound/usb: Update ALSA driver to use media controller API
At Fri, 8 May 2015 13:31:31 -0600, Shuah Khan wrote: Change ALSA driver to use media controller API to share tuner with DVB and V4L2 drivers that control AU0828 media device. Media device is created based on a newly added field value in the struct snd_usb_audio_quirk. Using this approach, the media controller API usage can be added for a specific device. In this patch, media controller API is enabled for AU0828 hw. snd_usb_create_quirk() will check this new field, if set will create a media device using media_device_get_dr() interface. media_device_get_dr() will allocate a new media device device resource or return an existing one if it exists. During probe, media usb driver could have created the media device. It will then register the media device if it isn't already registered. Media device unregister is done from usb_audio_disconnect(). New fields to add support for media entity and links are added to struct snd_usb_substream. A new media entity for ALSA and a link from tuner entity to the newly registered ALSA entity are created from snd_usb_init_substream() and removed from free_substream(). The state is kept to indicate if tuner is linked. This is to account for case when tuner entity doesn't exist. Media pipeline gets started to mark the tuner busy from snd_usb_substream_capture_trigger in response to SNDRV_PCM_TRIGGER_START and pipeline is stopped in response to SNDRV_PCM_TRIGGER_STOP. snd_usb_pcm_close() stops pipeline to cover the case when SNDRV_PCM_TRIGGER_STOP isn't issued. Pipeline start and stop are done only when tuner_linked is set. Tested with and without CONFIG_MEDIA_CONTROLLER enabled. Tested tuner entity doesn't exist case as au0828 v4l2 driver is the one that will create the tuner when it gets updated to use media controller API. I guess it gets broken when CONFIG_MEDIA_SUPPORT=m while CONFIG_SND*=y. So, it should be compiled in only when the media support is built-in or both sound and media are module, i.e. #ifdef CONFIG_MEDIA_CONTROLLER #if defined(CONFIG_MEDIA_SUPPORT) || \ (defined(CONFIG_MEDIA_SUPPORT_MODULE) defined(MODULE)) #define I_CAN_USE_MEDIA_CONTROLLER #endif #endif Signed-off-by: Shuah Khan shua...@osg.samsung.com --- sound/usb/card.c | 5 + sound/usb/card.h | 12 ++ sound/usb/pcm.c | 23 ++- sound/usb/quirks-table.h | 1 + sound/usb/quirks.c | 58 +++- sound/usb/quirks.h | 6 + sound/usb/stream.c | 40 + sound/usb/usbaudio.h | 1 + 8 files changed, 144 insertions(+), 2 deletions(-) diff --git a/sound/usb/card.c b/sound/usb/card.c index 1fab977..587fc24 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -621,6 +621,11 @@ static void usb_audio_disconnect(struct usb_interface *intf) } } + /* Nice to check quirk quirk-media_device + * need some special handlings. Doesn't look like + * we have access to quirk here */ + media_device_delete(intf); This should be called once, so better in if (!was_shutdown) block, I guess. Apart from that, yes, a good way to call an optional destructor for quirks would be better. chip-num_interfaces--; if (chip-num_interfaces = 0) { usb_chip[chip-index] = NULL; diff --git a/sound/usb/card.h b/sound/usb/card.h index ef580b4..477bdcd 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -1,6 +1,11 @@ #ifndef __USBAUDIO_CARD_H #define __USBAUDIO_CARD_H +#ifdef CONFIG_MEDIA_CONTROLLER +#include media/media-device.h +#include media/media-entity.h +#endif + #define MAX_NR_RATES 1024 #define MAX_PACKS6 /* per URB */ #define MAX_PACKS_HS (MAX_PACKS * 8) /* in high speed mode */ @@ -155,6 +160,13 @@ struct snd_usb_substream { } dsd_dop; bool trigger_tstamp_pending_update; /* trigger timestamp being updated from initial estimate */ +#ifdef CONFIG_MEDIA_CONTROLLER + struct media_device *media_dev; + struct media_entity media_entity; + struct media_pad media_pads; + struct media_pipeline media_pipe; + bool tuner_linked; +#endif Maybe a slightly better idea is to allocate these in a new record and stores the pointer in stream-media_ctl or whatever new field. Then, a check like if (subs-tuner_linked) can be replaced with if (subs-media_ctl) The whole media-specific stuff can be gathered in a single file, e.g. sound/usb/media.c, and there you can define the struct. }; struct snd_usb_stream { diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index b4ef410..c2a40a9 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1225,6 +1225,10 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream, int direction) subs-pcm_substream = NULL; snd_usb_autosuspend(subs-stream-chip); +#ifdef CONFIG_MEDIA_CONTROLLER
Re: [PATCH 7/7] sound/usb: Update ALSA driver to use Managed Media Controller API
On Mon, 20 Jul 2015 10:47:46 +0200, Dan Carpenter wrote: On Tue, Jul 14, 2015 at 06:34:06PM -0600, Shuah Khan wrote: + ret = media_entity_setup_link(link, flags); + if (ret) { + dev_err(mctl-media_dev-dev, + Couldn't change tuner link, + %s-%s to %s. Error %d\n, Add a space after link. Couldn't change tuner link , %s-%s to %s. Error %d\n, Such a message string would be better to be in a single line even if it's over 80 chars. Otherwise it becomes hard to grep. thanks, Takashi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH MC Next Gen v2 2/3] sound/usb: Create media mixer function and control interface entities
On Wed, 21 Oct 2015 01:25:15 +0200, Shuah Khan wrote: > > Add support for creating MEDIA_ENT_F_AUDIO_MIXER entity for > each mixer and a MEDIA_INTF_T_ALSA_CONTROL control interface > entity that links to mixer entities. MEDIA_INTF_T_ALSA_CONTROL > entity corresponds to the control device for the card. > > Signed-off-by: Shuah Khan> --- > sound/usb/card.c | 5 +++ > sound/usb/media.c| 89 > > sound/usb/media.h| 20 > sound/usb/mixer.h| 1 + > sound/usb/usbaudio.h | 1 + > 5 files changed, 116 insertions(+) > > diff --git a/sound/usb/card.c b/sound/usb/card.c > index 469d2bf..d004cb4 100644 > --- a/sound/usb/card.c > +++ b/sound/usb/card.c > @@ -560,6 +560,9 @@ static int usb_audio_probe(struct usb_interface *intf, > if (err < 0) > goto __error; > > + /* Create media entities for mixer and control dev */ > + media_mixer_init(chip); > + > usb_chip[chip->index] = chip; > chip->num_interfaces++; > chip->probing = 0; > @@ -616,6 +619,8 @@ static void usb_audio_disconnect(struct usb_interface > *intf) > list_for_each(p, >midi_list) { > snd_usbmidi_disconnect(p); > } > + /* delete mixer media resources */ > + media_mixer_delete(chip); > /* release mixer resources */ > list_for_each_entry(mixer, >mixer_list, list) { > snd_usb_mixer_disconnect(mixer); > diff --git a/sound/usb/media.c b/sound/usb/media.c > index 0cbfee6..a26ea8b 100644 > --- a/sound/usb/media.c > +++ b/sound/usb/media.c > @@ -199,4 +199,93 @@ void media_stop_pipeline(struct snd_usb_substream *subs) > if (mctl) > media_disable_source(mctl); > } > + > +int media_mixer_init(struct snd_usb_audio *chip) > +{ > + struct device *ctl_dev = >card->ctl_dev; > + struct media_intf_devnode *ctl_intf; > + struct usb_mixer_interface *mixer; > + struct media_device *mdev; > + struct media_mixer_ctl *mctl; > + u32 intf_type = MEDIA_INTF_T_ALSA_CONTROL; > + int ret; > + > + mdev = media_device_find_devres(>dev->dev); > + if (!mdev) > + return -ENODEV; > + > + ctl_intf = (struct media_intf_devnode *) chip->ctl_intf_media_devnode; Why do we need cast? Can't chip->ctl_intf_media_devnode itself be struct media_intf_devndoe pointer? > + if (!ctl_intf) { > + ctl_intf = (void *) media_devnode_create(mdev, > + intf_type, 0, > + MAJOR(ctl_dev->devt), > + MINOR(ctl_dev->devt)); > + if (!ctl_intf) > + return -ENOMEM; > + } Is chip->ctl_intf_media_devnode assigned anywhere at all? Not directly related with this patchset: the above uses media_devnode_create(), which I assume it's a device-file (major:minor) specific. What if sound hardware with multiple codecs and so on in general? > + > + list_for_each_entry(mixer, >mixer_list, list) { > + > + if (mixer->media_mixer_ctl) > + continue; > + > + /* allocate media_ctl */ > + mctl = kzalloc(sizeof(struct media_ctl), GFP_KERNEL); > + if (!mctl) > + return -ENOMEM; > + > + mixer->media_mixer_ctl = (void *) mctl; > + mctl->media_dev = mdev; > + > + mctl->media_entity.function = MEDIA_ENT_F_AUDIO_MIXER; > + mctl->media_entity.name = chip->card->mixername; > + mctl->media_pad[0].flags = MEDIA_PAD_FL_SINK; > + mctl->media_pad[1].flags = MEDIA_PAD_FL_SOURCE; > + mctl->media_pad[2].flags = MEDIA_PAD_FL_SOURCE; > + media_entity_init(>media_entity, MEDIA_MIXER_PAD_MAX, > + mctl->media_pad); > + ret = media_device_register_entity(mctl->media_dev, > + >media_entity); > + if (ret) > + return ret; Will the last (unfinished) mctl be freed properly in the error path? > + mctl->intf_link = media_create_intf_link(>media_entity, > + _intf->intf, > + MEDIA_LNK_FL_ENABLED); > + if (!mctl->intf_link) { > + media_device_unregister_entity(>media_entity); > + return -ENOMEM; Ditto. > + } > + mctl->intf_devnode = ctl_intf; > + } > + return 0; > +} > + > +void media_mixer_delete(struct snd_usb_audio *chip) > +{ > + struct usb_mixer_interface *mixer; > + struct media_device *mdev; > + > + mdev = media_device_find_devres(>dev->dev); > + if (!mdev) > +
Re: [PATCH MC Next Gen v2 2/3] sound/usb: Create media mixer function and control interface entities
On Tue, 03 Nov 2015 17:06:45 +0100, Shuah Khan wrote: > > On 10/25/2015 03:37 PM, Shuah Khan wrote: > > On 10/22/2015 01:16 AM, Takashi Iwai wrote: > >> On Wed, 21 Oct 2015 01:25:15 +0200, > >> Shuah Khan wrote: > >>> > >>> Add support for creating MEDIA_ENT_F_AUDIO_MIXER entity for > >>> each mixer and a MEDIA_INTF_T_ALSA_CONTROL control interface > >>> entity that links to mixer entities. MEDIA_INTF_T_ALSA_CONTROL > >>> entity corresponds to the control device for the card. > >>> > >>> Signed-off-by: Shuah Khan <shua...@osg.samsung.com> > >>> --- > >>> sound/usb/card.c | 5 +++ > >>> sound/usb/media.c| 89 > >>> > >>> sound/usb/media.h| 20 > >>> sound/usb/mixer.h| 1 + > >>> sound/usb/usbaudio.h | 1 + > >>> 5 files changed, 116 insertions(+) > >>> > >>> diff --git a/sound/usb/card.c b/sound/usb/card.c > >>> index 469d2bf..d004cb4 100644 > >>> --- a/sound/usb/card.c > >>> +++ b/sound/usb/card.c > >>> @@ -560,6 +560,9 @@ static int usb_audio_probe(struct usb_interface > >>> *intf, > >>> if (err < 0) > >>> goto __error; > >>> > >>> +/* Create media entities for mixer and control dev */ > >>> +media_mixer_init(chip); > >>> + > >>> usb_chip[chip->index] = chip; > >>> chip->num_interfaces++; > >>> chip->probing = 0; > >>> @@ -616,6 +619,8 @@ static void usb_audio_disconnect(struct > >>> usb_interface *intf) > >>> list_for_each(p, >midi_list) { > >>> snd_usbmidi_disconnect(p); > >>> } > >>> +/* delete mixer media resources */ > >>> +media_mixer_delete(chip); > >>> /* release mixer resources */ > >>> list_for_each_entry(mixer, >mixer_list, list) { > >>> snd_usb_mixer_disconnect(mixer); > >>> diff --git a/sound/usb/media.c b/sound/usb/media.c > >>> index 0cbfee6..a26ea8b 100644 > >>> --- a/sound/usb/media.c > >>> +++ b/sound/usb/media.c > >>> @@ -199,4 +199,93 @@ void media_stop_pipeline(struct > >>> snd_usb_substream *subs) > >>> if (mctl) > >>> media_disable_source(mctl); > >>> } > >>> + > >>> +int media_mixer_init(struct snd_usb_audio *chip) > >>> +{ > >>> +struct device *ctl_dev = >card->ctl_dev; > >>> +struct media_intf_devnode *ctl_intf; > >>> +struct usb_mixer_interface *mixer; > >>> +struct media_device *mdev; > >>> +struct media_mixer_ctl *mctl; > >>> +u32 intf_type = MEDIA_INTF_T_ALSA_CONTROL; > >>> +int ret; > >>> + > >>> +mdev = media_device_find_devres(>dev->dev); > >>> +if (!mdev) > >>> +return -ENODEV; > >>> + > >>> +ctl_intf = (struct media_intf_devnode *) > >>> chip->ctl_intf_media_devnode; > >> > >> Why do we need cast? Can't chip->ctl_intf_media_devnode itself be > >> struct media_intf_devndoe pointer? > > > > Yeah. There is no need to cast here. I will fix it. > > Sorry I misspoke. The reason for this cast is ctl_intf_media_devnode > is void to avoid including media.h and other media files in usbaudio.h You can declare the struct without definition in each header file. So just declare it and use it in usbaudio.h like: struct media_intf_devnode; struct snd_usb_audio { struct media_intf_devnode *ctl_intf_media_devnode; And even if you're using a void pointer there instead of the explicit struct pointer, the cast is superfluous. The implicit cast between a void pointer and any other pointer is valid in plain C. Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] c8sectpfe: Remove select on CONFIG_FW_LOADER_USER_HELPER_FALLBACK
c8sectpfe driver selects CONFIG_FW_LOADER_USER_HELPER_FALLBACK by some reason, but this option is known to be harmful, leading to minutes of stalls at boot time. The option was intended for only compatibility for an old exotic system that mandates the udev interaction, and not a thing a driver selects by itself. Let's remove it. Fixes: 850a3f7d5911 ('[media] c8sectpfe: Add Kconfig and Makefile for the driver') Signed-off-by: Takashi Iwai <ti...@suse.de> --- drivers/media/platform/sti/c8sectpfe/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/media/platform/sti/c8sectpfe/Kconfig b/drivers/media/platform/sti/c8sectpfe/Kconfig index 641ad8f34956..7420a50572d3 100644 --- a/drivers/media/platform/sti/c8sectpfe/Kconfig +++ b/drivers/media/platform/sti/c8sectpfe/Kconfig @@ -3,7 +3,6 @@ config DVB_C8SECTPFE depends on PINCTRL && DVB_CORE && I2C depends on ARCH_STI || ARCH_MULTIPLATFORM || COMPILE_TEST select FW_LOADER - select FW_LOADER_USER_HELPER_FALLBACK select DEBUG_FS select DVB_LNBP21 if MEDIA_SUBDRV_AUTOSELECT select DVB_STV090x if MEDIA_SUBDRV_AUTOSELECT -- 2.6.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Linux 4.2 ALSA snd-usb-audio inconsistent lock state warn in PCM nonatomic mode
On Tue, 01 Sep 2015 00:48:30 +0200, Shuah Khan wrote: > > Hi Takashi, > > I am seeing the following inconsistent lock state warning when PCM > is run in nonatomic mode. This is on 4.2.0 and with the following > change to force PCM on nonatomic mode: > > diff --git a/sound/usb/stream.c b/sound/usb/stream.c > index 310a382..16bbb71 100644 > --- a/sound/usb/stream.c > +++ b/sound/usb/stream.c > @@ -370,6 +370,7 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip, > pcm->private_data = as; > pcm->private_free = snd_usb_audio_pcm_free; > pcm->info_flags = 0; > + pcm->nonatomic = true; > if (chip->pcm_devs > 0) > sprintf(pcm->name, "USB Audio #%d", chip->pcm_devs); > else > > The device Bus 003 Device 002: ID 2040:7200 Hauppauge > Please let me know if you need more information and any ideas on > how to fix this problem. The USB-audio can't be used in the non-atomic mode. snd_pcm_period_elapsed() is handled in the complete callback that is already atomic. Takashi > > thanks, > -- Shuah > > [ 120.283960] = > [ 120.283964] [ INFO: inconsistent lock state ] > [ 120.283968] 4.2.0+ #29 Not tainted > [ 120.283972] - > [ 120.283975] inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage. > [ 120.283980] swapper/1/0 [HC1[1]:SC0[0]:HE0:SE1] takes: > [ 120.283983] (&(>lock)->rlock){?.+...}, at: > [] retire_capture_urb+0x140/0x2b0 [snd_usb_audio] > [ 120.284005] {HARDIRQ-ON-W} state was registered at: > [ 120.284008] [] __lock_acquire+0xc50/0x2380 > [ 120.284016] [] lock_acquire+0xb1/0x130 > [ 120.284022] [] _raw_spin_lock+0x31/0x40 > [ 120.284028] [] snd_usb_pcm_pointer+0x5d/0xc0 > [snd_usb_audio] > [ 120.284040] [] snd_pcm_update_hw_ptr0+0x38/0x3a0 > [snd_pcm] > [ 120.284052] [] snd_pcm_update_hw_ptr+0x10/0x20 > [snd_pcm] > [ 120.284063] [] snd_pcm_hwsync+0x45/0xa0 [snd_pcm] > [ 120.284071] [] snd_pcm_common_ioctl1+0x277/0xce0 > [snd_pcm] > [ 120.284081] [] snd_pcm_capture_ioctl1+0x1be/0x2d0 > [snd_pcm] > [ 120.284090] [] snd_pcm_capture_ioctl+0x34/0x40 > [snd_pcm] > [ 120.284100] [] do_vfs_ioctl+0x301/0x560 > [ 120.284107] [] SyS_ioctl+0x79/0x90 > [ 120.284112] [] entry_SYSCALL_64_fastpath+0x12/0x6f > [ 120.284119] irq event stamp: 823304 > [ 120.284122] hardirqs last enabled at (823301): [] > cpuidle_enter_state+0xed/0x230 > [ 120.284129] hardirqs last disabled at (823302): [] > common_interrupt+0x68/0x6d > [ 120.284135] softirqs last enabled at (823304): [] > _local_bh_enable+0x21/0x50 > [ 120.284139] softirqs last disabled at (823303): [] > irq_enter+0x4c/0x70 > [ 120.284143] > other info that might help us debug this: > [ 120.284146] Possible unsafe locking scenario: > > [ 120.284149]CPU0 > [ 120.284150] > [ 120.284152] lock(&(>lock)->rlock); > [ 120.284155] > [ 120.284157] lock(&(>lock)->rlock); > [ 120.284160] > *** DEADLOCK *** > [ 120.284163] no locks held by swapper/1/0. > [ 120.284165] > stack backtrace: > [ 120.284170] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.2.0+ #29 > [ 120.284173] Hardware name: Hewlett-Packard HP ProBook 6475b/180F, > BIOS 68TTU Ver. F.04 08/03/2012 > [ 120.284176] 828d5630 88023ec83a68 817f4ea0 > 0007 > [ 120.284181] 880235a6a500 88023ec83ac8 810ad97f > > [ 120.284186] 88020001 810134df > 827c5390 > [ 120.284192] Call Trace: > [ 120.284194][] dump_stack+0x45/0x57 > [ 120.284203] [] print_usage_bug+0x1ff/0x210 > [ 120.284209] [] ? save_stack_trace+0x2f/0x50 > [ 120.284214] [] mark_lock+0x66e/0x6f0 > [ 120.284218] [] ? > print_shortest_lock_dependencies+0x1d0/0x1d0 > [ 120.284222] [] __lock_acquire+0xdcb/0x2380 > [ 120.284226] [] ? __enqueue_entity+0x6c/0x70 > [ 120.284230] [] ? __lock_is_held+0x4d/0x70 > [ 120.284234] [] ? __lock_is_held+0x4d/0x70 > [ 120.284238] [] ? __lock_is_held+0x4d/0x70 > [ 120.284242] [] ? __lock_is_held+0x4d/0x70 > [ 120.284246] [] lock_acquire+0xb1/0x130 > [ 120.284256] [] ? retire_capture_urb+0x140/0x2b0 > [snd_usb_audio] > [ 120.284261] [] _raw_spin_lock_irqsave+0x3c/0x50 > [ 120.284270] [] ? retire_capture_urb+0x140/0x2b0 > [snd_usb_audio] > [ 120.284276] [] ? usb_hcd_get_frame_number+0x25/0x30 > [ 120.284285] [] retire_capture_urb+0x140/0x2b0 > [snd_usb_audio] > [ 120.284294] [] snd_complete_urb+0x13c/0x250 > [snd_usb_audio] > [ 120.284298] [] __usb_hcd_giveback_urb+0x72/0x110 > [ 120.284303] [] usb_hcd_giveback_urb+0x43/0x140 > [ 120.284307] [] xhci_irq+0xd42/0x1fc0 > [ 120.284312] [] xhci_msi_irq+0x11/0x20 > [ 120.284317] [] handle_irq_event_percpu+0x80/0x1a0 > [ 120.284322] [] handle_irq_event+0x4a/0x70 > [ 120.284325] [] ? handle_edge_irq+0x24/0x150 > [ 120.284329] [] handle_edge_irq+0x81/0x150 > [ 120.284333] [] handle_irq+0x25/0x40 > [ 120.284337]
Re: [alsa-devel] [PATCH MC Next Gen] sound/usb: Fix out of bounds access in media_entity_init()
On Sat, 05 Dec 2015 01:00:29 +0100, Shuah Khan wrote: > > Fix the out of bounds access in media_entity_init() found > by KASan. This is a result of media_mixer_init() failing > to allocate memory for all 3 of its pads before calling > media_entity_init(). Fix it to allocate memory for the > right struct media_mixer_ctl instead of struct media_ctl. > > Signed-off-by: Shuah Khan> --- > > This patch fixes the mixer patch below: > https://patchwork.linuxtv.org/patch/31827/ > > sound/usb/media.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/sound/usb/media.c b/sound/usb/media.c > index bebe27b..0cb44b9 100644 > --- a/sound/usb/media.c > +++ b/sound/usb/media.c > @@ -233,8 +233,8 @@ int media_mixer_init(struct snd_usb_audio *chip) > if (mixer->media_mixer_ctl) > continue; > > - /* allocate media_ctl */ > - mctl = kzalloc(sizeof(struct media_ctl), GFP_KERNEL); > + /* allocate media_mixer_ctl */ > + mctl = kzalloc(sizeof(struct media_mixer_ctl), GFP_KERNEL); Isn't it better to use sizeof(*mctl)? Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 26/31] sound/usb: Update ALSA driver to use Managed Media Controller API
On Wed, 06 Jan 2016 22:05:35 +0100, Shuah Khan wrote: > > diff --git a/sound/usb/Makefile b/sound/usb/Makefile > index 2d2d122..665fdd9 100644 > --- a/sound/usb/Makefile > +++ b/sound/usb/Makefile > @@ -2,6 +2,18 @@ > # Makefile for ALSA > # > > +# Media Controller > +ifeq ($(CONFIG_MEDIA_CONTROLLER),y) > + ifeq ($(CONFIG_MEDIA_SUPPORT),y) > +KBUILD_CFLAGS += -DUSE_MEDIA_CONTROLLER > + endif > + ifeq ($(CONFIG_MEDIA_SUPPORT_MODULE),y) > +ifeq ($(MODULE),y) > + KBUILD_CFLAGS += -DUSE_MEDIA_CONTROLLER > +endif > + endif > +endif Can't we define this rather via Kconfig? Doing this in Makefile is way too tricky, and it's unclear to users whether MC is actually enabled or not. > diff --git a/sound/usb/media.c b/sound/usb/media.c > new file mode 100644 > index 000..747a66a > --- /dev/null > +++ b/sound/usb/media.c > @@ -0,0 +1,214 @@ > +/* > + * media.c - Media Controller specific ALSA driver code > + * > + * Copyright (c) 2015 Shuah Khan> + * Copyright (c) 2015 Samsung Electronics Co., Ltd. > + * > + * This file is released under the GPLv2. > + */ > + > +/* > + * This file adds Media Controller support to ALSA driver > + * to use the Media Controller API to share tuner with DVB > + * and V4L2 drivers that control media device. Media device > + * is created based on existing quirks framework. Using this > + * approach, the media controller API usage can be added for > + * a specific device. > +*/ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "usbaudio.h" > +#include "card.h" > +#include "midi.h" > +#include "mixer.h" > +#include "proc.h" > +#include "quirks.h" > +#include "endpoint.h" > +#include "helper.h" > +#include "debug.h" > +#include "pcm.h" > +#include "format.h" > +#include "power.h" > +#include "stream.h" > +#include "media.h" I believe we can get rid of many include files just for MC support... > +#ifdef USE_MEDIA_CONTROLLER This ifdef can be removed once if we build this object file conditionally in Makefile. > @@ -1232,7 +1244,10 @@ static int snd_usb_pcm_open(struct snd_pcm_substream > *substream, int direction) > subs->dsd_dop.channel = 0; > subs->dsd_dop.marker = 1; > > - return setup_hw_info(runtime, subs); > + ret = setup_hw_info(runtime, subs); > + if (ret == 0) > + ret = media_stream_init(subs, as->pcm, direction); Need to call snd_usb_autosuspend() in the error path. > --- a/sound/usb/quirks.c > +++ b/sound/usb/quirks.c > @@ -544,13 +545,19 @@ int snd_usb_create_quirk(struct snd_usb_audio *chip, > [QUIRK_AUDIO_ALIGN_TRANSFER] = create_align_transfer_quirk, > [QUIRK_AUDIO_STANDARD_MIXER] = create_standard_mixer_quirk, > }; > + int ret; > > + if (quirk->media_device) { > + /* don't want to fail when media_device_create() fails */ > + media_device_create(chip, iface); > + } So far, so good... > if (quirk->type < QUIRK_TYPE_COUNT) { > - return quirk_funcs[quirk->type](chip, iface, driver, quirk); > + ret = quirk_funcs[quirk->type](chip, iface, driver, quirk); > } else { > usb_audio_err(chip, "invalid quirk type %d\n", quirk->type); > return -ENXIO; > } > + return ret; Any reason to change this? Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media, sound: tea575x: constify snd_tea575x_ops structures
On Sun, 22 Nov 2015 11:32:53 +0100, Julia Lawall wrote: > > The snd_tea575x_ops structures are never modified, so declare them as > const. > > Done with the help of Coccinelle. > > Signed-off-by: Julia Lawall <julia.law...@lip6.fr> Reviewed-by: Takashi Iwai <ti...@suse.de> Feel free to take via media tree. thanks, Takashi > > --- > drivers/media/pci/bt8xx/bttv-cards.c |2 +- > drivers/media/radio/radio-maxiradio.c |2 +- > drivers/media/radio/radio-sf16fmr2.c |2 +- > drivers/media/radio/radio-shark.c |2 +- > include/media/drv-intf/tea575x.h |2 +- > sound/pci/es1968.c|2 +- > sound/pci/fm801.c |2 +- > 7 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/sound/pci/es1968.c b/sound/pci/es1968.c > index cb38cd1..514f260 100644 > --- a/sound/pci/es1968.c > +++ b/sound/pci/es1968.c > @@ -2605,7 +2605,7 @@ static void snd_es1968_tea575x_set_direction(struct > snd_tea575x *tea, bool outpu > } > } > > -static struct snd_tea575x_ops snd_es1968_tea_ops = { > +static const struct snd_tea575x_ops snd_es1968_tea_ops = { > .set_pins = snd_es1968_tea575x_set_pins, > .get_pins = snd_es1968_tea575x_get_pins, > .set_direction = snd_es1968_tea575x_set_direction, > diff --git a/sound/pci/fm801.c b/sound/pci/fm801.c > index 5144a7f..759295a 100644 > --- a/sound/pci/fm801.c > +++ b/sound/pci/fm801.c > @@ -815,7 +815,7 @@ static void snd_fm801_tea575x_set_direction(struct > snd_tea575x *tea, bool output > fm801_writew(chip, GPIO_CTRL, reg); > } > > -static struct snd_tea575x_ops snd_fm801_tea_ops = { > +static const struct snd_tea575x_ops snd_fm801_tea_ops = { > .set_pins = snd_fm801_tea575x_set_pins, > .get_pins = snd_fm801_tea575x_get_pins, > .set_direction = snd_fm801_tea575x_set_direction, > diff --git a/drivers/media/pci/bt8xx/bttv-cards.c > b/drivers/media/pci/bt8xx/bttv-cards.c > index 7a08102..8a17cc0 100644 > --- a/drivers/media/pci/bt8xx/bttv-cards.c > +++ b/drivers/media/pci/bt8xx/bttv-cards.c > @@ -3808,7 +3808,7 @@ static void bttv_tea575x_set_direction(struct > snd_tea575x *tea, bool output) > gpio_inout(mask, (1 << gpio.clk) | (1 << gpio.wren)); > } > > -static struct snd_tea575x_ops bttv_tea_ops = { > +static const struct snd_tea575x_ops bttv_tea_ops = { > .set_pins = bttv_tea575x_set_pins, > .get_pins = bttv_tea575x_get_pins, > .set_direction = bttv_tea575x_set_direction, > diff --git a/drivers/media/radio/radio-sf16fmr2.c > b/drivers/media/radio/radio-sf16fmr2.c > index 8e4f1d1..dc81d42 100644 > --- a/drivers/media/radio/radio-sf16fmr2.c > +++ b/drivers/media/radio/radio-sf16fmr2.c > @@ -82,7 +82,7 @@ static void fmr2_tea575x_set_direction(struct snd_tea575x > *tea, bool output) > { > } > > -static struct snd_tea575x_ops fmr2_tea_ops = { > +static const struct snd_tea575x_ops fmr2_tea_ops = { > .set_pins = fmr2_tea575x_set_pins, > .get_pins = fmr2_tea575x_get_pins, > .set_direction = fmr2_tea575x_set_direction, > diff --git a/include/media/drv-intf/tea575x.h > b/include/media/drv-intf/tea575x.h > index 5d09657..fb272d4 100644 > --- a/include/media/drv-intf/tea575x.h > +++ b/include/media/drv-intf/tea575x.h > @@ -63,7 +63,7 @@ struct snd_tea575x { > u32 band; /* 0: FM, 1: FM-Japan, 2: AM */ > u32 freq; /* frequency */ > struct mutex mutex; > - struct snd_tea575x_ops *ops; > + const struct snd_tea575x_ops *ops; > void *private_data; > u8 card[32]; > u8 bus_info[32]; > diff --git a/drivers/media/radio/radio-shark.c > b/drivers/media/radio/radio-shark.c > index 409fac1..85667a9 100644 > --- a/drivers/media/radio/radio-shark.c > +++ b/drivers/media/radio/radio-shark.c > @@ -150,7 +150,7 @@ static u32 shark_read_val(struct snd_tea575x *tea) > return val; > } > > -static struct snd_tea575x_ops shark_tea_ops = { > +static const struct snd_tea575x_ops shark_tea_ops = { > .write_val = shark_write_val, > .read_val = shark_read_val, > }; > diff --git a/drivers/media/radio/radio-maxiradio.c > b/drivers/media/radio/radio-maxiradio.c > index 41c1652..70fd8e8 100644 > --- a/drivers/media/radio/radio-maxiradio.c > +++ b/drivers/media/radio/radio-maxiradio.c > @@ -108,7 +108,7 @@ static void maxiradio_tea575x_set_direction(struct > snd_tea575x *tea, bool output > { > } > > -static struct snd_tea575x_ops maxiradio_tea_ops = { > +static const struct snd_tea575x_ops maxiradio_tea_ops = { > .set_pins = maxiradio_tea575x_set_pins, > .get_pins = maxiradio_tea575x_get_pins, > .set_direction = maxiradio_tea575x_set_direction, > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [very-RFC 0/8] TSN driver for the kernel
On Mon, 20 Jun 2016 17:21:26 +0200, Richard Cochran wrote: > > On Mon, Jun 20, 2016 at 02:31:48PM +0200, Richard Cochran wrote: > > Where is this "audio_time" program of which you speak? > > Never mind, found it in alsa-lib. > > I still would appreciate an answer to my other questions, though... Currently HD-audio (both ASoC and legacy ones) are the only drivers providing the link timestamp. In the recent code, it's PCM get_time_info ops, so you can easily grep it. HTH, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [very-RFC 0/8] TSN driver for the kernel
On Tue, 21 Jun 2016 08:38:57 +0200, Richard Cochran wrote: > > On Tue, Jun 21, 2016 at 07:54:32AM +0200, Takashi Iwai wrote: > > > I still would appreciate an answer to my other questions, though... > > > > Currently HD-audio (both ASoC and legacy ones) are the only drivers > > providing the link timestamp. In the recent code, it's PCM > > get_time_info ops, so you can easily grep it. > > Yes, I found that myself, thanks. > > > HTH, > > No it doesn't help me, because I asked three questions, and none were > about the link timestamp. ?? The extended audio timpestamp is essentially to return the link timestamp. Just the term has changed along time... Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: add GFP flag to media_*() that could get called in atomic context
On Mon, 14 Mar 2016 11:13:58 +0100, Mauro Carvalho Chehab wrote: > > Em Mon, 14 Mar 2016 09:22:37 +0200 > Sakari Ailusescreveu: > > > Hi Shuah, > > > > On Sat, Mar 12, 2016 at 06:48:09PM -0700, Shuah Khan wrote: > > > Add GFP flags to media_create_pad_link(), media_create_intf_link(), > > > media_devnode_create(), and media_add_link() that could get called > > > in atomic context to allow callers to pass in the right flags for > > > memory allocation. > > > > > > tree-wide driver changes for media_*() GFP flags change: > > > Change drivers to add gfpflags to interffaces, media_create_pad_link(), > > > media_create_intf_link() and media_devnode_create(). > > > > > > Signed-off-by: Shuah Khan > > > Suggested-by: Mauro Carvalho Chehab > > > > What's the use case for calling the above functions in an atomic context? > > > > ALSA code seems to do a lot of stuff at atomic context. That's what > happens on my test machine when au0828 gets probed before > snd-usb-audio: > http://pastebin.com/LEX5LD5K > > It seems that ALSA USB probe routine (usb_audio_probe) happens in > atomic context. No, usb_audio_probe() doesn't take a lock. But media_device_register_entity() seems taking a spinlock, instead. Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 0/5] Media Device Allocator API
On Tue, 05 Apr 2016 05:35:55 +0200, Shuah Khan wrote: > > There are known problems with media device life time management. When media > device is released while an media ioctl is in progress, ioctls fail with > use-after-free errors and kernel hangs in some cases. > > Media Device can be in any the following states: > > - Allocated > - Registered (could be tied to more than one driver) > - Unregistered, not in use (media device file is not open) > - Unregistered, in use (media device file is not open) > - Released > > When media device belongs to more than one driver, registrations should be > tracked to avoid unregistering when one of the drivers does unregister. A new > num_drivers field in the struct media_device covers this case. The media > device > should be unregistered only when the last unregister occurs with num_drivers > count zero. > > When a media device is in use when it is unregistered, it should not be > released until the application exits when it detects the unregistered > status. Media device that is in use when it is unregistered is moved to > to_delete_list. When the last unregister occurs, media device is unregistered > and becomes an unregistered, still allocated device. Unregister marks the > device to be deleted. > > When media device belongs to more than one driver, as both drivers could be > unbound/bound, driver should not end up getting stale media device that is > on its way out. Moving the unregistered media device to to_delete_list helps > this case as well. > > I ran bind/unbind loop tests on uvcvideo, au0828, and snd-usb-audio while > running application that does ioctls. Didn't see any use-after-free errors > on media device. A couple of known issues seen: > > 1. When application exits, cdev_put() gets called after media device is >released. This is a known issue to resolve and Media Device Allocator >can't solve this one. > 2. When au0828 module is removed and then ioctls fail when cdev_get() looks >for the owning module as au0828 is very often the module that owns the >media devnode. This is a cdev related issue that needs to be resolved and >Media Device Allocator can't solve this one. > > Shuah Khan (5): > media: Add Media Device Allocator API > media: Add driver count to keep track of media device registrations > media: uvcvideo change to use Media Device Allocator API > media: au0828 change to use Media Device Allocator API > sound/usb: Use Media Controller API to share media resources I don't think we need to include usb-audio patch at this stage yet. The most important thing for now is to improve / stabilize the API itself so that other drivers can use it as is. Once when the API is really stabilized, we create a solid git branch that may be based for multiple subsystems, and I'll merge usb-audio stuff through sound git tree. Also, the previous usb-audio MC implementation had a few serious bugs, including quirk NULL dereference. See the bugzilla below for some fix patches to 4.6-rc1: https://bugzilla.kernel.org/show_bug.cgi?id=115561 Feel free to fold them in, if they are still valid. thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH] sound/usb: Fix memory leak in media_snd_stream_delete() during unbind
On Thu, 17 Mar 2016 03:58:06 +0100, Shuah Khan wrote: > > media_snd_stream_delete() fails to release resources during unbind. This > leads to use-after-free in media_gobj_create() on a subsequent bind. > > [ 1445.086410] BUG: KASAN: use-after-free in media_gobj_create+0x3a1/0x470 > [media] at addr 8801ead49998 > > [ 1445.086771] Call Trace: > [ 1445.086779] [] dump_stack+0x67/0x94 > [ 1445.086785] [] print_trailer+0xf9/0x150 > [ 1445.086790] [] object_err+0x34/0x40 > [ 1445.086796] [] kasan_report_error+0x221/0x530 > [ 1445.086803] [] __asan_report_store8_noabort+0x43/0x50 > [ 1445.086813] [] ? media_gobj_create+0x3a1/0x470 [media] > [ 1445.086822] [] media_gobj_create+0x3a1/0x470 [media] > [ 1445.086831] [] media_device_register_entity+0x259/0x6f0 > [media] > [ 1445.086841] [] ? > media_device_unregister_entity_notify+0x100/0x100 [media] > [ 1445.086846] [] ? ___slab_alloc+0x172/0x500 > [ 1445.086854] [] ? mark_held_locks+0xc8/0x120 > [ 1445.086859] [] ? __slab_alloc+0x50/0x70 > [ 1445.086878] [] ? media_snd_mixer_init+0x16c/0x500 > [snd_usb_audio] > [ 1445.086884] [] ? kasan_unpoison_shadow+0x36/0x50 > [ 1445.086890] [] ? kasan_unpoison_shadow+0x36/0x50 > [ 1445.086895] [] ? kasan_kmalloc+0x5e/0x70 > > Signed-off-by: Shuah Khan <shua...@osg.samsung.com> Mauro, please take it through your tree. Acked-by: Takashi Iwai <ti...@suse.de> thanks, Takashi > --- > sound/usb/media.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sound/usb/media.c b/sound/usb/media.c > index 44a5de9..0d03773 100644 > --- a/sound/usb/media.c > +++ b/sound/usb/media.c > @@ -135,7 +135,7 @@ void media_snd_stream_delete(struct snd_usb_substream > *subs) > if (mctl && mctl->media_dev) { > struct media_device *mdev; > > - mdev = subs->stream->chip->media_dev; > + mdev = mctl->media_dev; > if (mdev && media_devnode_is_registered(>devnode)) { > media_devnode_remove(mctl->intf_devnode); > media_device_unregister_entity(>media_entity); > -- > 2.5.0 > > ___ > Alsa-devel mailing list > alsa-de...@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 0/2] ALSA: control: export all of TLV related macros to user land
On Sat, 10 Sep 2016 06:50:14 +0200, Takashi Sakamoto wrote: > > Hi, > > Currently, TLV related protocol is not shared to user land. This is not > good in a point of application interfaces, because application developers > can't realize the protocol just to see UAPI headers. > > For this purpose, this patchset moves all of macros related to TLV to UAPI > header. As a result, a header just for kernel land is obsoleted. When adding > new items to the protocol, it's added to the UAPI header. This change affects > some drivers in media subsystem. > > In my concern, this change can break applications. When these macros are > already defined in application side and they includes tlv UAPI header > directly, 'redefined' warning is generated at preprocess time. But the > compilation will be success itself. If these two macros have different > content, the result of preprocess is dominated to the order to define. > However, the most applications are assumed to use TLV feature via libraries > such as alsa-lib, thus I'm optimistic to this concern. > > As another my concern, the name of these macros are quite simple, as > 'TLV_XXX'. It might be help application developers to rename them with a > prefix, as 'SNDRV_CTL_TLV_XXX'. (But not yet. I'm a lazy guy.) The second patch does simply wrong. You must not obsolete include/sound/tlv.h. Even if it includes only uapi/*, it should be still there. So, just move the stuff to include/uapi/sound/tlv.h, and that's all. thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 0/2] ALSA: control: export all of TLV related macros to user land
On Sat, 10 Sep 2016 09:25:31 +0200, Takashi Sakamoto wrote: > > On Sep 10 2016 15:44, Takashi Iwai wrote: > > On Sat, 10 Sep 2016 06:50:14 +0200, > > Takashi Sakamoto wrote: > >> > >> Hi, > >> > >> Currently, TLV related protocol is not shared to user land. This is not > >> good in a point of application interfaces, because application developers > >> can't realize the protocol just to see UAPI headers. > >> > >> For this purpose, this patchset moves all of macros related to TLV to UAPI > >> header. As a result, a header just for kernel land is obsoleted. When > >> adding > >> new items to the protocol, it's added to the UAPI header. This change > >> affects > >> some drivers in media subsystem. > >> > >> In my concern, this change can break applications. When these macros are > >> already defined in application side and they includes tlv UAPI header > >> directly, 'redefined' warning is generated at preprocess time. But the > >> compilation will be success itself. If these two macros have different > >> content, the result of preprocess is dominated to the order to define. > >> However, the most applications are assumed to use TLV feature via libraries > >> such as alsa-lib, thus I'm optimistic to this concern. > >> > >> As another my concern, the name of these macros are quite simple, as > >> 'TLV_XXX'. It might be help application developers to rename them with a > >> prefix, as 'SNDRV_CTL_TLV_XXX'. (But not yet. I'm a lazy guy.) > > > > The second patch does simply wrong. You must not obsolete > > include/sound/tlv.h. Even if it includes only uapi/*, it should be > > still there. > > Any reasons? The concept and the design. Don't need to change the root inclusion, it's just to provide cleaner uapi header files, and not meant to be included directly -- it was the basic idea when uapi split was introduced. Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 0/2] ALSA: control: export all of TLV related macros to user land
On Sun, 11 Sep 2016 05:06:41 +0200, Takashi Sakamoto wrote: > > On Sep 10 2016 22:41, Takashi Iwai wrote: > > On Sat, 10 Sep 2016 09:25:31 +0200, > > Takashi Sakamoto wrote: > >> > >> On Sep 10 2016 15:44, Takashi Iwai wrote: > >>> On Sat, 10 Sep 2016 06:50:14 +0200, > >>> Takashi Sakamoto wrote: > >>>> > >>>> Hi, > >>>> > >>>> Currently, TLV related protocol is not shared to user land. This is not > >>>> good in a point of application interfaces, because application developers > >>>> can't realize the protocol just to see UAPI headers. > >>>> > >>>> For this purpose, this patchset moves all of macros related to TLV to > >>>> UAPI > >>>> header. As a result, a header just for kernel land is obsoleted. When > >>>> adding > >>>> new items to the protocol, it's added to the UAPI header. This change > >>>> affects > >>>> some drivers in media subsystem. > >>>> > >>>> In my concern, this change can break applications. When these macros are > >>>> already defined in application side and they includes tlv UAPI header > >>>> directly, 'redefined' warning is generated at preprocess time. But the > >>>> compilation will be success itself. If these two macros have different > >>>> content, the result of preprocess is dominated to the order to define. > >>>> However, the most applications are assumed to use TLV feature via > >>>> libraries > >>>> such as alsa-lib, thus I'm optimistic to this concern. > >>>> > >>>> As another my concern, the name of these macros are quite simple, as > >>>> 'TLV_XXX'. It might be help application developers to rename them with a > >>>> prefix, as 'SNDRV_CTL_TLV_XXX'. (But not yet. I'm a lazy guy.) > >>> > >>> The second patch does simply wrong. You must not obsolete > >>> include/sound/tlv.h. Even if it includes only uapi/*, it should be > >>> still there. > >> > >> Any reasons? > > > > The concept and the design. > > > > Don't need to change the root inclusion, it's just to provide cleaner > > uapi header files, and not meant to be included directly -- it was the > > basic idea when uapi split was introduced. > > OK. I can see what you indicated in this post for UAPI idea[0]. I'm > ready to drop the second patch. > > Well, how do you think about my concern of macro prefix? For example, we > can apply below step: > > Put substantial macros with renaming to 'include/uapi.sound/tlv.h': > #define SNDRV_CTL_TLV_DATA_LENGTH(...) \ >((unsigned int)sizeof((const unsigned int[]) { __VA_ARGS__ })) > > Then, put alias macros to 'include/sound/tlv.h': > #include > #define TLV_LENGTH SNDRV_CTL_TLV_DATA_LENGTH > ... > > Finally, applications can expand these macro with apparent names with > prefix of 'SNDRV_CTL_TLV_DATA_XXX'. I think the prefix prevent > application codes from name conflict by including 'uapi/sound/tlv.h'. Yes, that would work. Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 3/3] sound/usb: Use Media Controller API to share media resources
On Tue, 06 Dec 2016 19:41:37 +0100, Shuah Khan wrote: > > Hi Takashi, > > On 12/05/2016 11:50 PM, Takashi Iwai wrote: > > On Wed, 30 Nov 2016 23:01:16 +0100, > > Shuah Khan wrote: > >> > >> --- a/sound/usb/card.c > >> +++ b/sound/usb/card.c > > (snip) > >> @@ -616,6 +617,11 @@ static int usb_audio_probe(struct usb_interface *intf, > >>if (err < 0) > >>goto __error; > >> > >> + if (quirk && quirk->media_device) { > >> + /* don't want to fail when media_snd_device_create() fails */ > >> + media_snd_device_create(chip, intf); > > > > Note that the usb-audio driver is probed for each usb interface, and > > there are multiple interfaces per device. That is, usb_audio_probe() > > may be called multiple times per device, and at the second or the > > later calls, it appends the stuff onto the previously created card > > object. > > > > So, you'd have to make this call also conditional (e.g. check > > chip->num_interfaces == 0, indicating the very first one), or allow to > > be called multiple times. > > > > In the former case, it'd be better to split media_snd_device_create() > > and media_snd_mixer_init(). The *_device_create() needs to be called > > only once, while *_mixer_init() still has to be called for each time > > because the new mixer element may be added for each interface. > > > > Thanks for the catch. I am able to fix this in media_snd_device_create() > I made a change to do media_dev allocate only once. media_snd_mixer_init() > will get called every time media_snd_device_create() is called. This way > new mixers can be handled. media_snd_mixer_init() has logic to deal with > mixers that are already initialized. We are good here with the following > change: > > @@ -272,6 +258,14 @@ int media_snd_device_create(struct snd_usb_audio *chip, > struct usb_device *usbdev = interface_to_usbdev(iface); > int ret; > > + /* usb-audio driver is probed for each usb interface, and > +* there are multiple interfaces per device. Avoid calling > +* media_device_usb_allocate() each time usb_audio_probe() > +* is called. Do it only once. > +*/ > + if (chip->media_dev) > + goto snd_mixer_init; > + > mdev = media_device_usb_allocate(usbdev, KBUILD_MODNAME); > if (!mdev) > return -ENOMEM; > @@ -291,6 +285,7 @@ int media_snd_device_create(struct snd_usb_audio *chip, > /* save media device - avoid lookups */ > chip->media_dev = mdev; > > +snd_mixer_init: > /* Create media entities for mixer and control dev */ > ret = media_snd_mixer_init(chip); > if (ret) { This looks good enough, yes. > > > > >> + } > >> + > >>usb_chip[chip->index] = chip; > >>chip->num_interfaces++; > >>usb_set_intfdata(intf, chip); > >> @@ -672,6 +678,14 @@ static void usb_audio_disconnect(struct usb_interface > >> *intf) > >>list_for_each(p, >midi_list) { > >>snd_usbmidi_disconnect(p); > >>} > >> + /* > >> + * Nice to check quirk && quirk->media_device and > >> + * then call media_snd_device_delete(). Don't have > >> + * access to quirk here. media_snd_device_delete() > >> + * acceses mixer_list > >> + */ > >> + media_snd_device_delete(chip); > > > > ... meanwhile this is OK, as it's called only once. > > > > (BTW, is it OK to call media_* stuff while the device is still in use? > > The disconnect callback gets called for hot-unplug.) > > > > Yes. All of the media_* functions that get called during run-time check for > chip->media_dev or media_ctl and do nothing when these are null. > > media_device itself will not be free'd until all reference are gone. When > usb_audio_disconnect() happens via unbind snd_usb_audio or physical remove, > media_dev sticks around until au0828 (media driver) goes away. There is > handling for any user apps. that have /dev/mediaX open. > > Does this sound correct? Did I miss any of your concerns? That sounds all good, so it's safe to call there. thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 3/3] sound/usb: Use Media Controller API to share media resources
On Wed, 30 Nov 2016 23:01:16 +0100, Shuah Khan wrote: > > --- a/sound/usb/card.c > +++ b/sound/usb/card.c (snip) > @@ -616,6 +617,11 @@ static int usb_audio_probe(struct usb_interface *intf, > if (err < 0) > goto __error; > > + if (quirk && quirk->media_device) { > + /* don't want to fail when media_snd_device_create() fails */ > + media_snd_device_create(chip, intf); Note that the usb-audio driver is probed for each usb interface, and there are multiple interfaces per device. That is, usb_audio_probe() may be called multiple times per device, and at the second or the later calls, it appends the stuff onto the previously created card object. So, you'd have to make this call also conditional (e.g. check chip->num_interfaces == 0, indicating the very first one), or allow to be called multiple times. In the former case, it'd be better to split media_snd_device_create() and media_snd_mixer_init(). The *_device_create() needs to be called only once, while *_mixer_init() still has to be called for each time because the new mixer element may be added for each interface. > + } > + > usb_chip[chip->index] = chip; > chip->num_interfaces++; > usb_set_intfdata(intf, chip); > @@ -672,6 +678,14 @@ static void usb_audio_disconnect(struct usb_interface > *intf) > list_for_each(p, >midi_list) { > snd_usbmidi_disconnect(p); > } > + /* > + * Nice to check quirk && quirk->media_device and > + * then call media_snd_device_delete(). Don't have > + * access to quirk here. media_snd_device_delete() > + * acceses mixer_list > + */ > + media_snd_device_delete(chip); ... meanwhile this is OK, as it's called only once. (BTW, is it OK to call media_* stuff while the device is still in use? The disconnect callback gets called for hot-unplug.) > --- /dev/null > +++ b/sound/usb/media.c (snip) > +void media_snd_stream_delete(struct snd_usb_substream *subs) > +{ > + struct media_ctl *mctl = subs->media_ctl; > + > + if (mctl && mctl->media_dev) { mctl->media_dev NULL check here is superfluous, as it's checked mctl->below. > + struct media_device *mdev; > + > + mdev = mctl->media_dev; > + if (mdev && media_devnode_is_registered(mdev->devnode)) { > + media_devnode_remove(mctl->intf_devnode); > + media_device_unregister_entity(>media_entity); > + media_entity_cleanup(>media_entity); > + } > + kfree(mctl); > + subs->media_ctl = NULL; > + } > +} > + > +int media_snd_start_pipeline(struct snd_usb_substream *subs) > +{ > + struct media_ctl *mctl = subs->media_ctl; > + > + if (mctl) > + return media_snd_enable_source(mctl); > + return 0; > +} It's merely a wrapper, and media_snd_enable_source() itself checks NULL mctl. So we can replace media_snd_enable_source() with media_snd_start_pipeline(). > +void media_snd_stop_pipeline(struct snd_usb_substream *subs) > +{ > + struct media_ctl *mctl = subs->media_ctl; > + > + if (mctl) > + media_snd_disable_source(mctl); > +} Ditto. > diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c > index 44d178e..0e4e0640 100644 > --- a/sound/usb/pcm.c > +++ b/sound/usb/pcm.c (snip) > @@ -717,10 +718,14 @@ static int snd_usb_hw_params(struct snd_pcm_substream > *substream, > struct audioformat *fmt; > int ret; > > + ret = media_snd_start_pipeline(subs); > + if (ret) > + return ret; It's an open question at which timing we should call media_snd_start_pipeline(). The hw_params is mostly OK, while the real timing where the stream might be started is the prepare callback. I guess we can keep as is for now. Also, one more thing to be considered is whether media_snd_start_pipeline() can be called multiple times. hw_params and prepare callbacks may be called multiple times. I suppose it's OK, but just to be sure. > @@ -1234,7 +1246,12 @@ static int snd_usb_pcm_open(struct snd_pcm_substream > *substream, int direction) > subs->dsd_dop.channel = 0; > subs->dsd_dop.marker = 1; > > - return setup_hw_info(runtime, subs); > + ret = setup_hw_info(runtime, subs); > + if (ret == 0) > + ret = media_snd_stream_init(subs, as->pcm, direction); > + if (ret) > + snd_usb_autosuspend(subs->stream->chip); > + return ret; This leads to the PM refcount unbalance. The call of snd_usb_autosuspend() must be in the former if block, ret = setup_hw_info(runtime, subs); if (ret == 0) { ret = media_snd_stream_init(subs, as->pcm, direction); if (ret) snd_usb_autosuspend(subs->stream->chip); } return ret; thanks, Takashi -- To unsubscribe from this list:
Re: [PATCH v6 3/3] sound/usb: Use Media Controller API to share media resources
On Mon, 05 Dec 2016 23:44:30 +0100, Shuah Khan wrote: > > On 11/30/2016 03:01 PM, Shuah Khan wrote: > > Change ALSA driver to use Media Controller API to share media resources > > with DVB, and V4L2 drivers on a AU0828 media device. > > > > Media Controller specific initialization is done after sound card is > > registered. ALSA creates Media interface and entity function graph > > nodes for Control, Mixer, PCM Playback, and PCM Capture devices. > > > > snd_usb_hw_params() will call Media Controller enable source handler > > interface to request the media resource. If resource request is granted, > > it will release it from snd_usb_hw_free(). If resource is busy, -EBUSY is > > returned. > > > > Media specific cleanup is done in usb_audio_disconnect(). > > > > Signed-off-by: Shuah Khan> > Hi Takashi, > > If you are good with this patch, could you please Ack it, so Mauro > can pull it into media tree with the other two patches in this series, > when he is ready to do so. Sorry for the late review. The biggest concern right now is the way the driver is probed. As mentioned in the review reply, we need to consider the cases where the probe is called multiple times. Other things are minor. thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH] ALSA: hda - Fix applying MSI dual-codec mobo quirk
On Thu, 01 Jun 2017 22:58:24 +0200, Takashi Iwai wrote: > > The previous commit [63691587f7b0: ALSA: hda - Apply dual-codec quirk > for MSI Z270-Gaming mobo] attempted to apply the existing dual-codec > quirk for a MSI mobo. But it turned out that this isn't applied > properly due to the MSI-vendor quirk before this entry. I overlooked > such two MSI entries just because they were put in the wrong position, > although we have a list ordered by PCI SSID numbers. > > This patch fixes it by rearranging the unordered entries. > > Fixes: 63691587f7b0 ("ALSA: hda - Apply dual-codec quirk for MSI Z270-Gaming > mobo") > Reported-by: Rudolf Schmidt <i...@rudolfschmidt.com> > Signed-off-by: Takashi Iwai <ti...@suse.de> Gah, an irrelevant patch I wanted to submit slipped into the patch series. Please disregard one, sorry. Takashi > --- > sound/pci/hda/patch_realtek.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c > index 918e45268915..a57988d617e9 100644 > --- a/sound/pci/hda/patch_realtek.c > +++ b/sound/pci/hda/patch_realtek.c > @@ -2324,11 +2324,11 @@ static const struct snd_pci_quirk alc882_fixup_tbl[] > = { > SND_PCI_QUIRK(0x106b, 0x4a00, "Macbook 5,2", ALC889_FIXUP_MBA11_VREF), > > SND_PCI_QUIRK(0x1071, 0x8258, "Evesham Voyaeger", ALC882_FIXUP_EAPD), > - SND_PCI_QUIRK(0x1462, 0x7350, "MSI-7350", ALC889_FIXUP_CD), > - SND_PCI_QUIRK_VENDOR(0x1462, "MSI", ALC882_FIXUP_GPIO3), > SND_PCI_QUIRK(0x1458, 0xa002, "Gigabyte EP45-DS3/Z87X-UD3H", > ALC889_FIXUP_FRONT_HP_NO_PRESENCE), > SND_PCI_QUIRK(0x1458, 0xa0b8, "Gigabyte AZ370-Gaming", > ALC1220_FIXUP_GB_DUAL_CODECS), > + SND_PCI_QUIRK(0x1462, 0x7350, "MSI-7350", ALC889_FIXUP_CD), > SND_PCI_QUIRK(0x1462, 0xda57, "MSI Z270-Gaming", > ALC1220_FIXUP_GB_DUAL_CODECS), > + SND_PCI_QUIRK_VENDOR(0x1462, "MSI", ALC882_FIXUP_GPIO3), > SND_PCI_QUIRK(0x147b, 0x107a, "Abit AW9D-MAX", > ALC882_FIXUP_ABIT_AW9D_MAX), > SND_PCI_QUIRK_VENDOR(0x1558, "Clevo laptop", ALC882_FIXUP_EAPD), > SND_PCI_QUIRK(0x161f, 0x2054, "Medion laptop", ALC883_FIXUP_EAPD), > -- > 2.13.0 > > ___ > Alsa-devel mailing list > alsa-de...@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >
[PATCH v2 07/27] ALSA: rme32: Convert to the new PCM copy ops
Replace the copy and the silence ops with the new ops. The conversion is straightforward with standard helper functions, and now we can drop the bytes <-> frames conversions in callbacks. Signed-off-by: Takashi Iwai <ti...@suse.de> --- sound/pci/rme32.c | 65 --- 1 file changed, 43 insertions(+), 22 deletions(-) diff --git a/sound/pci/rme32.c b/sound/pci/rme32.c index 96d15db65dfd..ce438c62b0b3 100644 --- a/sound/pci/rme32.c +++ b/sound/pci/rme32.c @@ -254,39 +254,46 @@ static inline unsigned int snd_rme32_pcm_byteptr(struct rme32 * rme32) } /* silence callback for halfduplex mode */ -static int snd_rme32_playback_silence(struct snd_pcm_substream *substream, int channel,/* not used (interleaved data) */ - snd_pcm_uframes_t pos, - snd_pcm_uframes_t count) +static int snd_rme32_playback_silence(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + unsigned long count) { struct rme32 *rme32 = snd_pcm_substream_chip(substream); - count <<= rme32->playback_frlog; - pos <<= rme32->playback_frlog; + memset_io(rme32->iobase + RME32_IO_DATA_BUFFER + pos, 0, count); return 0; } /* copy callback for halfduplex mode */ -static int snd_rme32_playback_copy(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */ - snd_pcm_uframes_t pos, - void __user *src, snd_pcm_uframes_t count) +static int snd_rme32_playback_copy(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void __user *src, unsigned long count) { struct rme32 *rme32 = snd_pcm_substream_chip(substream); - count <<= rme32->playback_frlog; - pos <<= rme32->playback_frlog; + if (copy_from_user_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos, - src, count)) + src, count)) return -EFAULT; return 0; } +static int snd_rme32_playback_copy_kernel(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void *src, unsigned long count) +{ + struct rme32 *rme32 = snd_pcm_substream_chip(substream); + + memcpy_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos, src, count); + return 0; +} + /* copy callback for halfduplex mode */ -static int snd_rme32_capture_copy(struct snd_pcm_substream *substream, int channel,/* not used (interleaved data) */ - snd_pcm_uframes_t pos, - void __user *dst, snd_pcm_uframes_t count) +static int snd_rme32_capture_copy(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void __user *dst, unsigned long count) { struct rme32 *rme32 = snd_pcm_substream_chip(substream); - count <<= rme32->capture_frlog; - pos <<= rme32->capture_frlog; + if (copy_to_user_fromio(dst, rme32->iobase + RME32_IO_DATA_BUFFER + pos, count)) @@ -294,6 +301,16 @@ static int snd_rme32_capture_copy(struct snd_pcm_substream *substream, int chann return 0; } +static int snd_rme32_capture_copy_kernel(struct snd_pcm_substream *substream, +int channel, unsigned long pos, +void *dst, unsigned long count) +{ + struct rme32 *rme32 = snd_pcm_substream_chip(substream); + + memcpy_fromio(dst, rme32->iobase + RME32_IO_DATA_BUFFER + pos, count); + return 0; +} + /* * SPDIF I/O capabilities (half-duplex mode) */ @@ -1205,8 +1222,9 @@ static const struct snd_pcm_ops snd_rme32_playback_spdif_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer, - .copy = snd_rme32_playback_copy, - .silence = snd_rme32_playback_silence, + .copy_user =snd_rme32_playback_copy, + .copy_kernel = snd_rme32_playback_copy_kernel, + .fill_silence = snd_rme32_playback_silence, .mmap = snd_pcm_lib_mmap_iomem, }; @@ -1219,7 +1237,8 @@ static const struct snd_pcm_ops snd_rme32_capture_spdif_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer, - .copy = snd_rme32_capture_copy, + .copy_user =snd_rme32_c
[PATCH v2 08/27] ALSA: rme96: Convert to the new PCM ops
Replace the copy and the silence ops with the new PCM ops. The conversion is straightforward with standard helper functions, and now we can drop the bytes <-> frames conversions in callbacks. Signed-off-by: Takashi Iwai <ti...@suse.de> --- sound/pci/rme96.c | 70 ++- 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index 05b9da30990d..24f1349a8e1b 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -327,13 +327,10 @@ snd_rme96_capture_ptr(struct rme96 *rme96) static int snd_rme96_playback_silence(struct snd_pcm_substream *substream, - int channel, /* not used (interleaved data) */ - snd_pcm_uframes_t pos, - snd_pcm_uframes_t count) + int channel, unsigned long pos, unsigned long count) { struct rme96 *rme96 = snd_pcm_substream_chip(substream); - count <<= rme96->playback_frlog; - pos <<= rme96->playback_frlog; + memset_io(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, 0, count); return 0; @@ -341,32 +338,49 @@ snd_rme96_playback_silence(struct snd_pcm_substream *substream, static int snd_rme96_playback_copy(struct snd_pcm_substream *substream, - int channel, /* not used (interleaved data) */ - snd_pcm_uframes_t pos, - void __user *src, - snd_pcm_uframes_t count) + int channel, unsigned long pos, + void __user *src, unsigned long count) { struct rme96 *rme96 = snd_pcm_substream_chip(substream); - count <<= rme96->playback_frlog; - pos <<= rme96->playback_frlog; - return copy_from_user_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, src, - count); + + return copy_from_user_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, + src, count); +} + +static int +snd_rme96_playback_copy_kernel(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void *src, unsigned long count) +{ + struct rme96 *rme96 = snd_pcm_substream_chip(substream); + + memcpy_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, src, count); + return 0; } static int snd_rme96_capture_copy(struct snd_pcm_substream *substream, - int channel, /* not used (interleaved data) */ - snd_pcm_uframes_t pos, - void __user *dst, - snd_pcm_uframes_t count) + int channel, unsigned long pos, + void __user *dst, unsigned long count) { struct rme96 *rme96 = snd_pcm_substream_chip(substream); - count <<= rme96->capture_frlog; - pos <<= rme96->capture_frlog; - return copy_to_user_fromio(dst, rme96->iobase + RME96_IO_REC_BUFFER + pos, + + return copy_to_user_fromio(dst, + rme96->iobase + RME96_IO_REC_BUFFER + pos, count); } +static int +snd_rme96_capture_copy_kernel(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void *dst, unsigned long count) +{ + struct rme96 *rme96 = snd_pcm_substream_chip(substream); + + memcpy_fromio(dst, rme96->iobase + RME96_IO_REC_BUFFER + pos, count); + return 0; +} + /* * Digital output capabilities (S/PDIF) */ @@ -1513,8 +1527,9 @@ static const struct snd_pcm_ops snd_rme96_playback_spdif_ops = { .prepare = snd_rme96_playback_prepare, .trigger = snd_rme96_playback_trigger, .pointer = snd_rme96_playback_pointer, - .copy = snd_rme96_playback_copy, - .silence = snd_rme96_playback_silence, + .copy_user =snd_rme96_playback_copy, + .copy_kernel = snd_rme96_playback_copy_kernel, + .fill_silence = snd_rme96_playback_silence, .mmap = snd_pcm_lib_mmap_iomem, }; @@ -1526,7 +1541,8 @@ static const struct snd_pcm_ops snd_rme96_capture_spdif_ops = { .prepare = snd_rme96_capture_prepare, .trigger = snd_rme96_capture_trigger, .pointer = snd_rme96_capture_pointer, - .copy = snd_rme96_capture_copy, + .copy_user =snd_rme96_capture_copy, + .copy_kernel = snd_rme96_capture_copy_kernel, .mmap = snd_pcm_lib_mmap_iomem, }; @@ -1538,8 +1554,9 @@ static const struct snd_pcm_ops snd_rme96_playback_adat_ops = { .prepare = snd_rme96_playback_prepare, .trigger = snd_rme96_playback_trigger, .pointer = snd_rm
[PATCH v2 04/27] ALSA: es1938: Convert to the new PCM copy ops
Replace the copy ops with the new copy_user and copy_kernel ops. It's used only for a capture stream (for some hardware workaround), thus we need no silence operation. Signed-off-by: Takashi Iwai <ti...@suse.de> --- sound/pci/es1938.c | 33 + 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/sound/pci/es1938.c b/sound/pci/es1938.c index e8d943071a8c..a544cd52f73a 100644 --- a/sound/pci/es1938.c +++ b/sound/pci/es1938.c @@ -839,15 +839,12 @@ static snd_pcm_uframes_t snd_es1938_playback_pointer(struct snd_pcm_substream *s } static int snd_es1938_capture_copy(struct snd_pcm_substream *substream, - int channel, - snd_pcm_uframes_t pos, - void __user *dst, - snd_pcm_uframes_t count) + int channel, unsigned long pos, + void __user *dst, unsigned long count) { struct snd_pcm_runtime *runtime = substream->runtime; struct es1938 *chip = snd_pcm_substream_chip(substream); - pos <<= chip->dma1_shift; - count <<= chip->dma1_shift; + if (snd_BUG_ON(pos + count > chip->dma1_size)) return -EINVAL; if (pos + count < chip->dma1_size) { @@ -856,12 +853,31 @@ static int snd_es1938_capture_copy(struct snd_pcm_substream *substream, } else { if (copy_to_user(dst, runtime->dma_area + pos + 1, count - 1)) return -EFAULT; - if (put_user(runtime->dma_area[0], ((unsigned char __user *)dst) + count - 1)) + if (put_user(runtime->dma_area[0], +((unsigned char __user *)dst) + count - 1)) return -EFAULT; } return 0; } +static int snd_es1938_capture_copy_kernel(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void *dst, unsigned long count) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + struct es1938 *chip = snd_pcm_substream_chip(substream); + + if (snd_BUG_ON(pos + count > chip->dma1_size)) + return -EINVAL; + if (pos + count < chip->dma1_size) { + memcpy(dst, runtime->dma_area + pos + 1, count); + } else { + memcpy(dst, runtime->dma_area + pos + 1, count - 1); + runtime->dma_area[0] = *((unsigned char *)dst + count - 1); + } + return 0; +} + /* * buffer management */ @@ -1012,7 +1028,8 @@ static const struct snd_pcm_ops snd_es1938_capture_ops = { .prepare = snd_es1938_capture_prepare, .trigger = snd_es1938_capture_trigger, .pointer = snd_es1938_capture_pointer, - .copy = snd_es1938_capture_copy, + .copy_user =snd_es1938_capture_copy, + .copy_kernel = snd_es1938_capture_copy_kernel, }; static int snd_es1938_new_pcm(struct es1938 *chip, int device) -- 2.13.0
[PATCH v2 12/27] ALSA: sb: Convert to the new PCM ops
Replace the copy and the silence ops with the new PCM ops. For avoiding the code redundancy, slightly hackish macros are introduced. Signed-off-by: Takashi Iwai <ti...@suse.de> --- sound/isa/sb/emu8000_pcm.c | 190 ++--- 1 file changed, 109 insertions(+), 81 deletions(-) diff --git a/sound/isa/sb/emu8000_pcm.c b/sound/isa/sb/emu8000_pcm.c index c480024422af..2ee8d67871ec 100644 --- a/sound/isa/sb/emu8000_pcm.c +++ b/sound/isa/sb/emu8000_pcm.c @@ -422,121 +422,148 @@ do { \ return -EAGAIN;\ } while (0) +enum { + COPY_USER, COPY_KERNEL, FILL_SILENCE, +}; + +#define GET_VAL(sval, buf, mode) \ + do {\ + switch (mode) { \ + case FILL_SILENCE: \ + sval = 0; \ + break; \ + case COPY_KERNEL: \ + sval = *buf++; \ + break; \ + default:\ + if (get_user(sval, (unsigned short __user *)buf)) \ + return -EFAULT; \ + buf++; \ + break; \ + } \ + } while (0) #ifdef USE_NONINTERLEAVE -/* copy one channel block */ -static int emu8k_transfer_block(struct snd_emu8000 *emu, int offset, unsigned short *buf, int count) -{ - EMU8000_SMALW_WRITE(emu, offset); - while (count > 0) { - unsigned short sval; - CHECK_SCHEDULER(); - if (get_user(sval, buf)) - return -EFAULT; - EMU8000_SMLD_WRITE(emu, sval); - buf++; - count--; - } - return 0; -} +#define LOOP_WRITE(rec, offset, _buf, count, mode) \ + do {\ + struct snd_emu8000 *emu = (rec)->emu; \ + unsigned short *buf = (unsigned short *)(_buf); \ + snd_emu8000_write_wait(emu, 1); \ + EMU8000_SMALW_WRITE(emu, offset); \ + while (count > 0) { \ + unsigned short sval;\ + CHECK_SCHEDULER(); \ + GET_VAL(sval, buf, mode); \ + EMU8000_SMLD_WRITE(emu, sval); \ + count--;\ + } \ + } while (0) + +/* copy one channel block */ static int emu8k_pcm_copy(struct snd_pcm_substream *subs, - int voice, - snd_pcm_uframes_t pos, - void *src, - snd_pcm_uframes_t count) + int voice, unsigned long pos, + void __user *src, unsigned long count) { struct snd_emu8k_pcm *rec = subs->runtime->private_data; - struct snd_emu8000 *emu = rec->emu; - snd_emu8000_write_wait(emu, 1); - return emu8k_transfer_block(emu, pos + rec->loop_start[voice], src, - count); + /* convert to word unit */ + pos = (pos << 1) + rec->loop_start[voice]; + count <<= 1; + LOOP_WRITE(rec, pos, src, count, COPY_UESR); + return 0; } -/* make a channel block silence */ -static int emu8k_silence_block(struct snd_emu8000 *emu, int offset, int count) +static int emu8k_pcm_copy_kernel(struct snd_pcm_substream *subs, +int voice, unsigned long pos, +void *src, unsigned long count) { - EMU8000_SMALW_WRITE(emu, offset); - while (count > 0) { - CHECK_SCHEDULER(); - EMU8000_SMLD_WRITE(emu, 0); - count--; - } + struct snd_emu8k_pcm *rec = subs->runtime->private_data; + + /* convert to word unit */ + pos = (pos << 1) + rec->loop_start[voice]; + count <<= 1; + LOOP_WRITE(rec, pos, src, count, COPY_KERNEL); return 0; } +/* make a channel block silence */ static int emu8k_pcm_silence(struct snd_pcm_substream *subs, -int voice, -snd_pcm_u
[PATCH v2 19/27] ALSA: pcm: Call directly the common read/write helpers
Make snd_pcm_lib_read() and *_write() static inline functions that call the common helper functions directly. This reduces a slight amount of codes, and at the same time, it's a preparation for the further cleanups / fixes. Signed-off-by: Takashi Iwai <ti...@suse.de> --- include/sound/pcm.h | 43 +++--- sound/core/pcm_lib.c | 156 ++- 2 files changed, 89 insertions(+), 110 deletions(-) diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 953ebfc83184..cb4eff8ffd2e 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -1088,15 +1088,40 @@ int snd_pcm_update_state(struct snd_pcm_substream *substream, int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream); void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_uframes_t new_hw_ptr); void snd_pcm_period_elapsed(struct snd_pcm_substream *substream); -snd_pcm_sframes_t snd_pcm_lib_write(struct snd_pcm_substream *substream, - const void __user *buf, - snd_pcm_uframes_t frames); -snd_pcm_sframes_t snd_pcm_lib_read(struct snd_pcm_substream *substream, - void __user *buf, snd_pcm_uframes_t frames); -snd_pcm_sframes_t snd_pcm_lib_writev(struct snd_pcm_substream *substream, -void __user **bufs, snd_pcm_uframes_t frames); -snd_pcm_sframes_t snd_pcm_lib_readv(struct snd_pcm_substream *substream, - void __user **bufs, snd_pcm_uframes_t frames); +snd_pcm_sframes_t __snd_pcm_lib_write(struct snd_pcm_substream *substream, + void *buf, bool interleaved, + snd_pcm_uframes_t frames); +snd_pcm_sframes_t __snd_pcm_lib_read(struct snd_pcm_substream *substream, +void *buf, bool interleaved, +snd_pcm_uframes_t frames); + +static inline snd_pcm_sframes_t +snd_pcm_lib_write(struct snd_pcm_substream *substream, + const void __user *buf, snd_pcm_uframes_t frames) +{ + return __snd_pcm_lib_write(substream, (void *)buf, true, frames); +} + +static inline snd_pcm_sframes_t +snd_pcm_lib_read(struct snd_pcm_substream *substream, +void __user *buf, snd_pcm_uframes_t frames) +{ + return __snd_pcm_lib_read(substream, (void *)buf, true, frames); +} + +static inline snd_pcm_sframes_t +snd_pcm_lib_writev(struct snd_pcm_substream *substream, + void __user **bufs, snd_pcm_uframes_t frames) +{ + return __snd_pcm_lib_write(substream, (void *)bufs, false, frames); +} + +static inline snd_pcm_sframes_t +snd_pcm_lib_readv(struct snd_pcm_substream *substream, + void __user **bufs, snd_pcm_uframes_t frames) +{ + return __snd_pcm_lib_read(substream, (void *)bufs, false, frames); +} extern const struct snd_pcm_hw_constraint_list snd_pcm_known_rates; diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 1f5251cca607..1bd7244324d5 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -1992,12 +1992,12 @@ static int wait_for_avail(struct snd_pcm_substream *substream, } typedef int (*transfer_f)(struct snd_pcm_substream *substream, unsigned int hwoff, - unsigned long data, unsigned int off, + void *data, unsigned int off, snd_pcm_uframes_t size); static int snd_pcm_lib_write_transfer(struct snd_pcm_substream *substream, unsigned int hwoff, - unsigned long data, unsigned int off, + void *data, unsigned int off, snd_pcm_uframes_t frames) { struct snd_pcm_runtime *runtime = substream->runtime; @@ -2019,7 +2019,7 @@ static int snd_pcm_lib_write_transfer(struct snd_pcm_substream *substream, static int snd_pcm_lib_writev_transfer(struct snd_pcm_substream *substream, unsigned int hwoff, - unsigned long data, unsigned int off, + void *data, unsigned int off, snd_pcm_uframes_t frames) { struct snd_pcm_runtime *runtime = substream->runtime; @@ -2096,21 +2096,39 @@ static int pcm_accessible_state(struct snd_pcm_runtime *runtime) } } -static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream, - unsigned long data, - snd_pcm_uframes_t size, - int nonblock, - transfer_f transfer) +snd_pcm_sframes_t __snd_pcm_lib_write(struct snd_pcm_substrea
[PATCH v2 14/27] ASoC: blackfin: Convert to the new PCM ops
Replace the copy and the silence ops with the new PCM ops. In AC97 and I2S-TDM mode, we need to convert back to frames, but otherwise the conversion is pretty straightforward. Signed-off-by: Takashi Iwai <ti...@suse.de> --- sound/soc/blackfin/bf5xx-ac97-pcm.c | 27 +++ sound/soc/blackfin/bf5xx-i2s-pcm.c | 36 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/sound/soc/blackfin/bf5xx-ac97-pcm.c b/sound/soc/blackfin/bf5xx-ac97-pcm.c index 02ad2606fa19..913e29275f4e 100644 --- a/sound/soc/blackfin/bf5xx-ac97-pcm.c +++ b/sound/soc/blackfin/bf5xx-ac97-pcm.c @@ -279,23 +279,33 @@ static int bf5xx_pcm_mmap(struct snd_pcm_substream *substream, return 0 ; } #else -static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, - void __user *buf, snd_pcm_uframes_t count) +static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void *buf, unsigned long count) { struct snd_pcm_runtime *runtime = substream->runtime; unsigned int chan_mask = ac97_chan_mask[runtime->channels - 1]; + struct ac97_frame *dst; + pr_debug("%s copy pos:0x%lx count:0x%lx\n", substream->stream ? "Capture" : "Playback", pos, count); + dst = (struct ac97_frame *)runtime->dma_area + + bytes_to_frames(runtime, pos); + count = bytes_to_frames(runtime, count); if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - bf5xx_pcm_to_ac97((struct ac97_frame *)runtime->dma_area + pos, - (__u16 *)buf, count, chan_mask); + bf5xx_pcm_to_ac97(dst, buf, count, chan_mask); else - bf5xx_ac97_to_pcm((struct ac97_frame *)runtime->dma_area + pos, - (__u16 *)buf, count); + bf5xx_ac97_to_pcm(dst, buf, count); return 0; } + +static int bf5xx_pcm_copy_user(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void __user *buf, unsigned long count) +{ + return bf5xx_pcm_copy(substream, channel, pos, (void *)buf, count); +} #endif static struct snd_pcm_ops bf5xx_pcm_ac97_ops = { @@ -309,7 +319,8 @@ static struct snd_pcm_ops bf5xx_pcm_ac97_ops = { #if defined(CONFIG_SND_BF5XX_MMAP_SUPPORT) .mmap = bf5xx_pcm_mmap, #else - .copy = bf5xx_pcm_copy, + .copy_user = bf5xx_pcm_copy_user, + .copy_kernel= bf5xx_pcm_copy, #endif }; diff --git a/sound/soc/blackfin/bf5xx-i2s-pcm.c b/sound/soc/blackfin/bf5xx-i2s-pcm.c index 6cba211da32e..470d99abf6f6 100644 --- a/sound/soc/blackfin/bf5xx-i2s-pcm.c +++ b/sound/soc/blackfin/bf5xx-i2s-pcm.c @@ -225,8 +225,9 @@ static int bf5xx_pcm_mmap(struct snd_pcm_substream *substream, return 0 ; } -static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, void *buf, snd_pcm_uframes_t count) +static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void *buf, unsigned long count) { struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_pcm_runtime *runtime = substream->runtime; @@ -238,6 +239,8 @@ static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, int channel, dma_data = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream); if (dma_data->tdm_mode) { + pos = bytes_to_frames(runtime, pos); + count = bytes_to_frames(runtime, count); if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { src = buf; dst = runtime->dma_area; @@ -269,21 +272,29 @@ static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, int channel, if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { src = buf; dst = runtime->dma_area; - dst += frames_to_bytes(runtime, pos); + dst += pos; } else { src = runtime->dma_area; - src += frames_to_bytes(runtime, pos); + src += pos; dst = buf; } - memcpy(dst, src, frames_to_bytes(runtime, count)); + memcpy(dst, src, count); } return 0; } +static int bf5xx_pcm_copy_user(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void __user *buf, unsigned long count) +{ + return bf5xx_pcm_copy(sub
[PATCH v2 13/27] ALSA: sh: Convert to the new PCM ops
Replace the copy and the silence ops with the new PCM ops. Fixed also the user-space buffer copy with the proper copy_from_user*() variant. Signed-off-by: Takashi Iwai <ti...@suse.de> --- sound/sh/sh_dac_audio.c | 54 +++-- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/sound/sh/sh_dac_audio.c b/sound/sh/sh_dac_audio.c index 461b310c7872..c1e00ed715ee 100644 --- a/sound/sh/sh_dac_audio.c +++ b/sound/sh/sh_dac_audio.c @@ -184,23 +184,36 @@ static int snd_sh_dac_pcm_trigger(struct snd_pcm_substream *substream, int cmd) return 0; } -static int snd_sh_dac_pcm_copy(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, void __user *src, snd_pcm_uframes_t count) +static int snd_sh_dac_pcm_copy(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void __user *src, unsigned long count) { /* channel is not used (interleaved data) */ struct snd_sh_dac *chip = snd_pcm_substream_chip(substream); struct snd_pcm_runtime *runtime = substream->runtime; - ssize_t b_count = frames_to_bytes(runtime , count); - ssize_t b_pos = frames_to_bytes(runtime , pos); - if (count < 0) - return -EINVAL; + if (copy_from_user_toio(chip->data_buffer + pos, src, count)) + return -EFAULT; + chip->buffer_end = chip->data_buffer + pos + count; - if (!count) - return 0; + if (chip->empty) { + chip->empty = 0; + dac_audio_start_timer(chip); + } + + return 0; +} - memcpy_toio(chip->data_buffer + b_pos, src, b_count); - chip->buffer_end = chip->data_buffer + b_pos + b_count; +static int snd_sh_dac_pcm_copy_kernel(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void *src, unsigned long count) +{ + /* channel is not used (interleaved data) */ + struct snd_sh_dac *chip = snd_pcm_substream_chip(substream); + struct snd_pcm_runtime *runtime = substream->runtime; + + memcpy_toio(chip->data_buffer + pos, src, count); + chip->buffer_end = chip->data_buffer + pos + count; if (chip->empty) { chip->empty = 0; @@ -211,23 +224,15 @@ static int snd_sh_dac_pcm_copy(struct snd_pcm_substream *substream, int channel, } static int snd_sh_dac_pcm_silence(struct snd_pcm_substream *substream, - int channel, snd_pcm_uframes_t pos, - snd_pcm_uframes_t count) + int channel, unsigned long pos, + unsigned long count) { /* channel is not used (interleaved data) */ struct snd_sh_dac *chip = snd_pcm_substream_chip(substream); struct snd_pcm_runtime *runtime = substream->runtime; - ssize_t b_count = frames_to_bytes(runtime , count); - ssize_t b_pos = frames_to_bytes(runtime , pos); - - if (count < 0) - return -EINVAL; - - if (!count) - return 0; - memset_io(chip->data_buffer + b_pos, 0, b_count); - chip->buffer_end = chip->data_buffer + b_pos + b_count; + memset_io(chip->data_buffer + pos, 0, count); + chip->buffer_end = chip->data_buffer + pos + count; if (chip->empty) { chip->empty = 0; @@ -256,8 +261,9 @@ static struct snd_pcm_ops snd_sh_dac_pcm_ops = { .prepare= snd_sh_dac_pcm_prepare, .trigger= snd_sh_dac_pcm_trigger, .pointer= snd_sh_dac_pcm_pointer, - .copy = snd_sh_dac_pcm_copy, - .silence= snd_sh_dac_pcm_silence, + .copy_user = snd_sh_dac_pcm_copy, + .copy_kernel= snd_sh_dac_pcm_copy_kernel, + .fill_silence = snd_sh_dac_pcm_silence, .mmap = snd_pcm_lib_mmap_iomem, }; -- 2.13.0
[PATCH v2 11/27] ALSA: gus: Convert to the new PCM ops
Replace the copy and the silence ops with the new PCM ops. For simplifying the code a bit, two local helpers are introduced here: get_bpos() and playback_copy_ack(). Signed-off-by: Takashi Iwai <ti...@suse.de> --- sound/isa/gus/gus_pcm.c | 97 ++--- 1 file changed, 59 insertions(+), 38 deletions(-) diff --git a/sound/isa/gus/gus_pcm.c b/sound/isa/gus/gus_pcm.c index 0cc3f272edf1..b9f6dcbef889 100644 --- a/sound/isa/gus/gus_pcm.c +++ b/sound/isa/gus/gus_pcm.c @@ -353,26 +353,25 @@ static int snd_gf1_pcm_poke_block(struct snd_gus_card *gus, unsigned char *buf, return 0; } -static int snd_gf1_pcm_playback_copy(struct snd_pcm_substream *substream, -int voice, -snd_pcm_uframes_t pos, -void __user *src, -snd_pcm_uframes_t count) +static int get_bpos(struct gus_pcm_private *pcmp, int voice, unsigned int pos, + unsigned int len) { - struct snd_pcm_runtime *runtime = substream->runtime; - struct gus_pcm_private *pcmp = runtime->private_data; - struct snd_gus_card *gus = pcmp->gus; - unsigned int bpos, len; - int w16, invert; - - bpos = samples_to_bytes(runtime, pos) + (voice * (pcmp->dma_size / 2)); - len = samples_to_bytes(runtime, count); + unsigned int bpos = pos + (voice * (pcmp->dma_size / 2)); if (snd_BUG_ON(bpos > pcmp->dma_size)) return -EIO; if (snd_BUG_ON(bpos + len > pcmp->dma_size)) return -EIO; - if (copy_from_user(runtime->dma_area + bpos, src, len)) - return -EFAULT; + return bpos; +} + +static int playback_copy_ack(struct snd_pcm_substream *substream, +unsigned int bpos, unsigned int len) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + struct gus_pcm_private *pcmp = runtime->private_data; + struct snd_gus_card *gus = pcmp->gus; + int w16, invert; + if (len > 32) return snd_gf1_pcm_block_change(substream, bpos, pcmp->memory + bpos, len); @@ -383,33 +382,54 @@ static int snd_gf1_pcm_playback_copy(struct snd_pcm_substream *substream, pcmp->memory + bpos, len, w16, invert); } +static int snd_gf1_pcm_playback_copy(struct snd_pcm_substream *substream, +int voice, unsigned long pos, +void __user *src, unsigned long count) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + struct gus_pcm_private *pcmp = runtime->private_data; + unsigned int len = count; + int bpos; + + bpos = get_bpos(pcmp, voice, pos, len); + if (bpos < 0) + return pos; + if (copy_from_user(runtime->dma_area + bpos, src, len)) + return -EFAULT; + return playback_copy_ack(substream, bpos, len); +} + +static int snd_gf1_pcm_playback_copy_kernel(struct snd_pcm_substream *substream, + int voice, unsigned long pos, + void *src, unsigned long count) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + struct gus_pcm_private *pcmp = runtime->private_data; + unsigned int len = count; + int bpos; + + bpos = get_bpos(pcmp, voice, pos, len); + if (bpos < 0) + return pos; + memcpy(runtime->dma_area + bpos, src, len); + return playback_copy_ack(substream, bpos, len); +} + static int snd_gf1_pcm_playback_silence(struct snd_pcm_substream *substream, - int voice, - snd_pcm_uframes_t pos, - snd_pcm_uframes_t count) + int voice, unsigned long pos, + unsigned long count) { struct snd_pcm_runtime *runtime = substream->runtime; struct gus_pcm_private *pcmp = runtime->private_data; - struct snd_gus_card *gus = pcmp->gus; - unsigned int bpos, len; - int w16, invert; + unsigned int len = count; + int bpos; - bpos = samples_to_bytes(runtime, pos) + (voice * (pcmp->dma_size / 2)); - len = samples_to_bytes(runtime, count); - if (snd_BUG_ON(bpos > pcmp->dma_size)) - return -EIO; - if (snd_BUG_ON(bpos + len > pcmp->dma_size)) - return -EIO; + bpos = get_bpos(pcmp, voice, pos, len); + if (bpos < 0) + return pos; snd_pcm_format_set_silence(runtime->format, runtime->dma_area + bpos, -
[PATCH v2 00/27] Revised full patchset for PCM in-kernel copy support
Hi, this is a full patchset of what I sent previously, containing the all changes instead of the snippet. The main purpose of this patchset is to eliminate the remaining usages of set_fs(). They are basically used for in-kernel PCM data transfer, and this patch provides the new API functions and replaces the hackish set_fs() calls with them. Unlike the first patchset with the unified copy_silence ops, this adds a new copy_kernel ops instead. At the same time, copy/silence are changed to receive the position and size in bytes instead of frames. This allows us to simplify the PCM core code. As a result, a good amount of code could be removed from pcm_lib.c. The difference from the previous patchset is that this is a full patchset, i.e. all relevant drivers have been covered, and also some small issues have been addressed, in addition, the documentation update is provided, too. I'm Cc'ing the media and the USB people since it touches solo6x10 and usb-gadget drivers. The previous ACK was dropped as each patch was rewritten again. Sorry for the doubly patch-review labours. thanks, Takashi === Takashi Iwai (26): ALSA: pcm: Introduce copy_user, copy_kernel and fill_silence ops ALSA: dummy: Convert to new PCM copy ops ALSA: es1938: Convert to the new PCM copy ops ALSA: nm256: Convert to new PCM copy ops ALSA: korg1212: Convert to the new PCM ops ALSA: rme32: Convert to the new PCM copy ops ALSA: rme96: Convert to the new PCM ops ALSA: rme9652: Convert to the new PCM ops ALSA: hdsp: Convert to the new PCM ops ALSA: gus: Convert to the new PCM ops ALSA: sb: Convert to the new PCM ops ALSA: sh: Convert to the new PCM ops ASoC: blackfin: Convert to the new PCM ops [media] solo6x10: Convert to the new PCM ops ALSA: pcm: Drop the old copy and silence ops ALSA: pcm: Check PCM state by a common helper function ALSA: pcm: Shuffle codes ALSA: pcm: Call directly the common read/write helpers ALSA: pcm: More unification of PCM transfer codes ALSA: pcm: Unify read/write loop ALSA: pcm: Simplify snd_pcm_playback_silence() ALSA: pcm: Direct in-kernel read/write support usb: gadget: u_uac1: Kill set_fs() usage ALSA: pcm: Kill set_fs() in PCM OSS layer ALSA: pcm: Build OSS writev/readv helpers conditionally ALSA: doc: Update copy_user, copy_kernel and fill_silence PCM ops .../sound/kernel-api/writing-an-alsa-driver.rst| 111 ++-- drivers/media/pci/solo6x10/solo6x10-g723.c | 32 +- drivers/usb/gadget/function/u_uac1.c | 7 +- include/sound/pcm.h| 80 ++- sound/core/oss/io.c| 4 +- sound/core/oss/pcm_oss.c | 81 +-- sound/core/oss/pcm_plugin.h| 6 +- sound/core/pcm_lib.c | 564 - sound/drivers/dummy.c | 20 +- sound/isa/gus/gus_pcm.c| 97 ++-- sound/isa/sb/emu8000_pcm.c | 190 --- sound/pci/es1938.c | 33 +- sound/pci/korg1212/korg1212.c | 112 ++-- sound/pci/nm256/nm256.c| 57 ++- sound/pci/rme32.c | 65 ++- sound/pci/rme96.c | 70 ++- sound/pci/rme9652/hdsp.c | 67 ++- sound/pci/rme9652/rme9652.c| 71 ++- sound/sh/sh_dac_audio.c| 54 +- sound/soc/blackfin/bf5xx-ac97-pcm.c| 27 +- sound/soc/blackfin/bf5xx-i2s-pcm.c | 36 +- sound/soc/soc-pcm.c| 5 +- 22 files changed, 977 insertions(+), 812 deletions(-) -- 2.13.0
[PATCH v2 15/27] [media] solo6x10: Convert to the new PCM ops
Replace the copy and the silence ops with the new PCM ops. The device supports only 1 channel and 8bit sample, so it's always bytes=frames, and we need no conversion of unit in the callback. Also, it's a capture stream, thus no silence is needed. Signed-off-by: Takashi Iwai <ti...@suse.de> --- drivers/media/pci/solo6x10/solo6x10-g723.c | 32 ++ 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/drivers/media/pci/solo6x10/solo6x10-g723.c b/drivers/media/pci/solo6x10/solo6x10-g723.c index 36e93540bb49..3ca947092775 100644 --- a/drivers/media/pci/solo6x10/solo6x10-g723.c +++ b/drivers/media/pci/solo6x10/solo6x10-g723.c @@ -223,9 +223,9 @@ static snd_pcm_uframes_t snd_solo_pcm_pointer(struct snd_pcm_substream *ss) return idx * G723_FRAMES_PER_PAGE; } -static int snd_solo_pcm_copy(struct snd_pcm_substream *ss, int channel, -snd_pcm_uframes_t pos, void __user *dst, -snd_pcm_uframes_t count) +static int __snd_solo_pcm_copy(struct snd_pcm_substream *ss, + unsigned long pos, void *dst, + unsigned long count, bool in_kernel) { struct solo_snd_pcm *solo_pcm = snd_pcm_substream_chip(ss); struct solo_dev *solo_dev = solo_pcm->solo_dev; @@ -242,16 +242,31 @@ static int snd_solo_pcm_copy(struct snd_pcm_substream *ss, int channel, if (err) return err; - err = copy_to_user(dst + (i * G723_PERIOD_BYTES), - solo_pcm->g723_buf, G723_PERIOD_BYTES); - - if (err) + if (in_kernel) + memcpy(dst, solo_pcm->g723_buf, G723_PERIOD_BYTES); + else if (copy_to_user((void __user *)dst, + solo_pcm->g723_buf, G723_PERIOD_BYTES)) return -EFAULT; + dst += G723_PERIOD_BYTES; } return 0; } +static int snd_solo_pcm_copy_user(struct snd_pcm_substream *ss, int channel, + unsigned long pos, void __user *dst, + unsigned long count) +{ + return __snd_solo_pcm_copy(ss, pos, (void *)dst, count, false); +} + +static int snd_solo_pcm_copy_kernel(struct snd_pcm_substream *ss, int channel, + unsigned long pos, void *dst, + unsigned long count) +{ + return __snd_solo_pcm_copy(ss, pos, dst, count, true); +} + static const struct snd_pcm_ops snd_solo_pcm_ops = { .open = snd_solo_pcm_open, .close = snd_solo_pcm_close, @@ -261,7 +276,8 @@ static const struct snd_pcm_ops snd_solo_pcm_ops = { .prepare = snd_solo_pcm_prepare, .trigger = snd_solo_pcm_trigger, .pointer = snd_solo_pcm_pointer, - .copy = snd_solo_pcm_copy, + .copy_user = snd_solo_pcm_copy_user, + .copy_kernel = snd_solo_pcm_copy_kernel, }; static int snd_solo_capture_volume_info(struct snd_kcontrol *kcontrol, -- 2.13.0
[PATCH v2 17/27] ALSA: pcm: Check PCM state by a common helper function
Signed-off-by: Takashi Iwai <ti...@suse.de> --- sound/core/pcm_lib.c | 81 +++- 1 file changed, 29 insertions(+), 52 deletions(-) diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 0db8d4e0fca2..e4f5c43b6448 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -2017,6 +2017,22 @@ typedef int (*transfer_f)(struct snd_pcm_substream *substream, unsigned int hwof unsigned long data, unsigned int off, snd_pcm_uframes_t size); +static int pcm_accessible_state(struct snd_pcm_runtime *runtime) +{ + switch (runtime->status->state) { + case SNDRV_PCM_STATE_PREPARED: + case SNDRV_PCM_STATE_RUNNING: + case SNDRV_PCM_STATE_PAUSED: + return 0; + case SNDRV_PCM_STATE_XRUN: + return -EPIPE; + case SNDRV_PCM_STATE_SUSPENDED: + return -ESTRPIPE; + default: + return -EBADFD; + } +} + static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream, unsigned long data, snd_pcm_uframes_t size, @@ -2033,21 +2049,9 @@ static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream, return 0; snd_pcm_stream_lock_irq(substream); - switch (runtime->status->state) { - case SNDRV_PCM_STATE_PREPARED: - case SNDRV_PCM_STATE_RUNNING: - case SNDRV_PCM_STATE_PAUSED: - break; - case SNDRV_PCM_STATE_XRUN: - err = -EPIPE; - goto _end_unlock; - case SNDRV_PCM_STATE_SUSPENDED: - err = -ESTRPIPE; - goto _end_unlock; - default: - err = -EBADFD; + err = pcm_accessible_state(runtime); + if (err < 0) goto _end_unlock; - } runtime->twake = runtime->control->avail_min ? : 1; if (runtime->status->state == SNDRV_PCM_STATE_RUNNING) @@ -2083,16 +2087,9 @@ static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream, snd_pcm_stream_lock_irq(substream); if (err < 0) goto _end_unlock; - switch (runtime->status->state) { - case SNDRV_PCM_STATE_XRUN: - err = -EPIPE; - goto _end_unlock; - case SNDRV_PCM_STATE_SUSPENDED: - err = -ESTRPIPE; + err = pcm_accessible_state(runtime); + if (err < 0) goto _end_unlock; - default: - break; - } appl_ptr += frames; if (appl_ptr >= runtime->boundary) appl_ptr -= runtime->boundary; @@ -2263,27 +2260,14 @@ static snd_pcm_sframes_t snd_pcm_lib_read1(struct snd_pcm_substream *substream, return 0; snd_pcm_stream_lock_irq(substream); - switch (runtime->status->state) { - case SNDRV_PCM_STATE_PREPARED: - if (size >= runtime->start_threshold) { - err = snd_pcm_start(substream); - if (err < 0) - goto _end_unlock; - } - break; - case SNDRV_PCM_STATE_DRAINING: - case SNDRV_PCM_STATE_RUNNING: - case SNDRV_PCM_STATE_PAUSED: - break; - case SNDRV_PCM_STATE_XRUN: - err = -EPIPE; - goto _end_unlock; - case SNDRV_PCM_STATE_SUSPENDED: - err = -ESTRPIPE; - goto _end_unlock; - default: - err = -EBADFD; + err = pcm_accessible_state(runtime); + if (err < 0) goto _end_unlock; + if (runtime->status->state == SNDRV_PCM_STATE_PREPARED && + size >= runtime->start_threshold) { + err = snd_pcm_start(substream); + if (err < 0) + goto _end_unlock; } runtime->twake = runtime->control->avail_min ? : 1; @@ -2327,16 +2311,9 @@ static snd_pcm_sframes_t snd_pcm_lib_read1(struct snd_pcm_substream *substream, snd_pcm_stream_lock_irq(substream); if (err < 0) goto _end_unlock; - switch (runtime->status->state) { - case SNDRV_PCM_STATE_XRUN: - err = -EPIPE; - goto _end_unlock; - case SNDRV_PCM_STATE_SUSPENDED: - err = -ESTRPIPE; + err = pcm_accessible_state(runtime); + if (err < 0) goto _end_unlock; - default: -
[PATCH v2 03/27] ALSA: dummy: Convert to new PCM copy ops
It's a dummy ops, so just replacing it. Signed-off-by: Takashi Iwai <ti...@suse.de> --- sound/drivers/dummy.c | 20 ++-- 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c index 172dacd925f5..dd5ed037adf2 100644 --- a/sound/drivers/dummy.c +++ b/sound/drivers/dummy.c @@ -644,15 +644,22 @@ static int alloc_fake_buffer(void) } static int dummy_pcm_copy(struct snd_pcm_substream *substream, - int channel, snd_pcm_uframes_t pos, - void __user *dst, snd_pcm_uframes_t count) + int channel, unsigned long pos, + void __user *dst, unsigned long bytes) +{ + return 0; /* do nothing */ +} + +static int dummy_pcm_copy_kernel(struct snd_pcm_substream *substream, +int channel, unsigned long pos, +void *dst, unsigned long bytes) { return 0; /* do nothing */ } static int dummy_pcm_silence(struct snd_pcm_substream *substream, -int channel, snd_pcm_uframes_t pos, -snd_pcm_uframes_t count) +int channel, unsigned long pos, +unsigned long bytes) { return 0; /* do nothing */ } @@ -683,8 +690,9 @@ static struct snd_pcm_ops dummy_pcm_ops_no_buf = { .prepare = dummy_pcm_prepare, .trigger = dummy_pcm_trigger, .pointer = dummy_pcm_pointer, - .copy = dummy_pcm_copy, - .silence = dummy_pcm_silence, + .copy_user =dummy_pcm_copy, + .copy_kernel = dummy_pcm_copy_kernel, + .fill_silence = dummy_pcm_silence, .page = dummy_pcm_page, }; -- 2.13.0
[PATCH v2 16/27] ALSA: pcm: Drop the old copy and silence ops
Now that all users of old copy and silence ops have been converted to the new PCM ops, the old stuff can be retired and go away. Signed-off-by: Takashi Iwai <ti...@suse.de> --- include/sound/pcm.h | 5 - sound/core/pcm_lib.c | 38 +- sound/soc/soc-pcm.c | 2 -- 3 files changed, 1 insertion(+), 44 deletions(-) diff --git a/include/sound/pcm.h b/include/sound/pcm.h index a065415191d8..953ebfc83184 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -78,11 +78,6 @@ struct snd_pcm_ops { struct timespec *system_ts, struct timespec *audio_ts, struct snd_pcm_audio_tstamp_config *audio_tstamp_config, struct snd_pcm_audio_tstamp_report *audio_tstamp_report); - int (*copy)(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, - void __user *buf, snd_pcm_uframes_t count); - int (*silence)(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, snd_pcm_uframes_t count); int (*fill_silence)(struct snd_pcm_substream *substream, int channel, unsigned long pos, unsigned long bytes); int (*copy_user)(struct snd_pcm_substream *substream, int channel, diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 9334fc2c20c8..0db8d4e0fca2 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -116,9 +116,6 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram frames_to_bytes(runtime, ofs), frames_to_bytes(runtime, transfer)); snd_BUG_ON(err < 0); - } else if (substream->ops->silence) { - err = substream->ops->silence(substream, -1, ofs, transfer); - snd_BUG_ON(err < 0); } else { hwbuf = runtime->dma_area + frames_to_bytes(runtime, ofs); snd_pcm_format_set_silence(runtime->format, hwbuf, transfer * runtime->channels); @@ -133,11 +130,6 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram samples_to_bytes(runtime, transfer)); snd_BUG_ON(err < 0); } - } else if (substream->ops->silence) { - for (c = 0; c < channels; ++c) { - err = substream->ops->silence(substream, c, ofs, transfer); - snd_BUG_ON(err < 0); - } } else { size_t dma_csize = runtime->dma_bytes / channels; for (c = 0; c < channels; ++c) { @@ -2013,9 +2005,6 @@ static int snd_pcm_lib_write_transfer(struct snd_pcm_substream *substream, err = substream->ops->copy_user(substream, 0, hwoff, buf, frames); if (err < 0) return err; - } else if (substream->ops->copy) { - if ((err = substream->ops->copy(substream, -1, hwoff, buf, frames)) < 0) - return err; } else { char *hwbuf = runtime->dma_area + frames_to_bytes(runtime, hwoff); if (copy_from_user(hwbuf, buf, frames_to_bytes(runtime, frames))) @@ -2137,8 +2126,7 @@ static int pcm_sanity_check(struct snd_pcm_substream *substream) if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; runtime = substream->runtime; - if (snd_BUG_ON(!substream->ops->copy_user && !substream->ops->copy - && !runtime->dma_area)) + if (snd_BUG_ON(!substream->ops->copy_user && !runtime->dma_area)) return -EINVAL; if (runtime->status->state == SNDRV_PCM_STATE_OPEN) return -EBADFD; @@ -2198,19 +2186,6 @@ static int snd_pcm_lib_writev_transfer(struct snd_pcm_substream *substream, if (err < 0) return err; } - } else if (substream->ops->copy) { - if (snd_BUG_ON(!substream->ops->silence)) - return -EINVAL; - for (c = 0; c < channels; ++c, ++bufs) { - if (*bufs == NULL) { - if ((err = substream->ops->silence(substream, c, hwoff, frames)) < 0) - return err; -