Skip to content

An improved version of Pull Request #72 #130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lyakh wants to merge 2 commits into from
Closed

An improved version of Pull Request #72 #130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
30b905e
sound: snd_sgbuf_aligned_pages() doesn't need CONFIG_SND_DMA_SGBUF
lyakh Sep 7, 2018
9b1abcd
ASoC:topology:bug fix:check return value avoid oops.
Aug 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions include/sound/memalloc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,6 @@ struct snd_sg_buf {
struct device *dev;
};

/*
* return the pages matching with the given byte size
*/
static inline unsigned int snd_sgbuf_aligned_pages(size_t size)
{
return (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
}

/*
* return the physical address at the corresponding offset
*/
Expand Down Expand Up @@ -140,6 +132,14 @@ static inline void *snd_sgbuf_get_ptr(struct snd_dma_buffer *dmab,

#endif /* CONFIG_SND_DMA_SGBUF */

/*
* return the pages matching with the given byte size
*/
static inline unsigned int snd_sgbuf_aligned_pages(size_t size)
{
return (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
}

/* allocate/release a buffer */
int snd_dma_alloc_pages(int type, struct device *dev, size_t size,
struct snd_dma_buffer *dmab);
Expand Down
98 changes: 89 additions & 9 deletions sound/soc/soc-dapm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2410,12 +2410,11 @@ static void dapm_free_path(struct snd_soc_dapm_path *path)
kfree(path);
}

void snd_soc_dapm_free_widget(struct snd_soc_dapm_widget *w)
static void snd_soc_dapm_free_widget_data(struct snd_soc_dapm_widget *w)
{
struct snd_soc_dapm_path *p, *next_p;
enum snd_soc_dapm_direction dir;

list_del(&w->list);
/*
* remove source and sink paths associated to this widget.
* While removing the path, remove reference to it from both
Expand All @@ -2428,6 +2427,12 @@ void snd_soc_dapm_free_widget(struct snd_soc_dapm_widget *w)

kfree(w->kcontrols);
kfree_const(w->name);
}

void snd_soc_dapm_free_widget(struct snd_soc_dapm_widget *w)
{
list_del(&w->list);
snd_soc_dapm_free_widget_data(w);
kfree(w);
}

Expand Down Expand Up @@ -3046,11 +3051,16 @@ EXPORT_SYMBOL_GPL(snd_soc_dapm_weak_routes);
*/
int snd_soc_dapm_new_widgets(struct snd_soc_card *card)
{
struct snd_soc_dapm_widget *w;
struct snd_soc_dapm_widget *w, *last;
unsigned int val;
int ret = 0;

mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_INIT);

/*
* widgets with the snd_soc_dapm_kcontrol ID and .num_controls = 0 can
* be appended to the list while scanning it, this is safe.
*/
list_for_each_entry(w, &card->widgets, list)
{
if (w->new)
Expand All @@ -3061,33 +3071,39 @@ int snd_soc_dapm_new_widgets(struct snd_soc_card *card)
sizeof(struct snd_kcontrol *),
GFP_KERNEL);
if (!w->kcontrols) {
mutex_unlock(&card->dapm_mutex);
return -ENOMEM;
ret = -ENOMEM;
goto out_free;
}
}

switch(w->id) {
case snd_soc_dapm_switch:
case snd_soc_dapm_mixer:
case snd_soc_dapm_mixer_named_ctl:
dapm_new_mixer(w);
ret = dapm_new_mixer(w);
break;
case snd_soc_dapm_mux:
case snd_soc_dapm_demux:
dapm_new_mux(w);
ret = dapm_new_mux(w);
break;
case snd_soc_dapm_pga:
case snd_soc_dapm_effect:
case snd_soc_dapm_out_drv:
dapm_new_pga(w);
ret = dapm_new_pga(w);
break;
case snd_soc_dapm_dai_link:
dapm_new_dai_link(w);
ret = dapm_new_dai_link(w);
break;
default:
break;
Copy link

@ghost ghost Sep 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the root cause of the issue is:
One of the w->id in the link list is not processed here, it goes to the default case.
we have to handle this.

Copy link
Member

@lgirdwood lgirdwood Sep 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lyakh it seems to be failing checkpatch

Copy link
Collaborator Author

@lyakh lyakh Sep 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, 2 errors are about the opening brace after a loop statement "list_for_each_..." is on a separate line. I saw it, I know it isn't the kernel style, but I used that style to preserve the original file formatting. That should be a separate patch to fix that. Perhaps before this one to then not have to add more such formatting issues. Or I can just fix this patch and then a separate patch should fix remaining old locations. As for "over 80 characters" warnings - yeah, 89 characters is a bit on a high end, but I see even a 92-char line in that file already, and it isn't one of printk() strings:

static int soc_dapm_mux_update_power(struct snd_soc_card *card,
				struct snd_kcontrol *kcontrol, int mux, struct soc_enum *e)

but I can make those shorter, if needed.
I think I'll make a new version with braces and long lines fixed - if those are the only problems with this patch.

}

if (ret < 0) {
kfree(w->kcontrols);
w->kcontrols = NULL;
goto out_free;
}

/* Read the initial power state from the device */
if (w->reg >= 0) {
soc_dapm_read(w->dapm, w->reg, &val);
Expand All @@ -3096,6 +3112,12 @@ int snd_soc_dapm_new_widgets(struct snd_soc_card *card)
if (val == w->on_val)
w->power = 1;
}
}

list_for_each_entry(w, &card->widgets, list)
{
if (w->new)
continue;

w->new = 1;

Expand All @@ -3106,6 +3128,64 @@ int snd_soc_dapm_new_widgets(struct snd_soc_card *card)
dapm_power_widgets(card, SND_SOC_DAPM_STREAM_NOP);
mutex_unlock(&card->dapm_mutex);
return 0;

out_free:
last = w;

/*
* If any new widgets have been created above for .autodisable = 1
* controls, they are also on this list, but at its very end. We're
* processing an error case, so the loop above was interrupted before
* reaching dynamically added widgets, since the latter cannot fail -
* their .num_controls = 0, so allocation cannot fail, and their type is
* snd_soc_dapm_kcontrol, those cannot fail either. Therefore "last"
* points to a widget before the first dynamic one.
*/
list_for_each_entry(w, &card->widgets, list)
{
struct soc_mixer_control *mc;
struct soc_enum *e;
unsigned int i;

if (w->new)
continue;

if (w == last)
break;

switch (w->id) {
case snd_soc_dapm_switch:
case snd_soc_dapm_mixer:
case snd_soc_dapm_mixer_named_ctl:
for (i = 0; i < w->num_kcontrols; i++) {
struct snd_kcontrol *kcontrol = w->kcontrols[i];
struct dapm_kcontrol_data *data = kcontrol->private_data;

mc = (struct soc_mixer_control *)kcontrol->private_value;
if (mc->autodisable)
snd_soc_dapm_free_widget(data->widget);
}
break;
case snd_soc_dapm_demux:
case snd_soc_dapm_mux:
for (i = 0; i < w->num_kcontrols; i++) {
struct snd_kcontrol *kcontrol = w->kcontrols[i];
struct dapm_kcontrol_data *data = kcontrol->private_data;

e = (struct soc_enum *)kcontrol->private_value;
if (e->autodisable)
snd_soc_dapm_free_widget(data->widget);
}
break;
default:
break;
}

snd_soc_dapm_free_widget_data(w);
}

mutex_unlock(&card->dapm_mutex);
return ret;
}
EXPORT_SYMBOL_GPL(snd_soc_dapm_new_widgets);

Expand Down