Re: [virtio-dev] Re: [PATCH v2 4/9] ALSA: virtio: build PCM devices and substream hardware descriptors

[Anton Yakovlev]

On 25.01.2021 16:44, Guennadi Liakhovetski wrote:
> On Sun, 24 Jan 2021, Anton Yakovlev wrote:
>

...[snip]...

>>
>> diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
>> index 955eadc2d858..39fe13b43dd1 100644
>> --- a/sound/virtio/virtio_card.c
>> +++ b/sound/virtio/virtio_card.c
>> @@ -92,6 +92,17 @@ static void virtsnd_event_notify_cb(struct
>> virtqueue *vqueue)
>>                       if (!event)
>>                               break;
>>
>> +                     switch (le32_to_cpu(event->hdr.code)) {
>> +                     case VIRTIO_SND_EVT_PCM_PERIOD_ELAPSED:
>> +                     case VIRTIO_SND_EVT_PCM_XRUN: {
>
> In the previous patch you had a switch-case statement complying to the
> common kernel coding style. It isn't specified in coding-style.rst, but
> these superfluous braces really don't seem to be good for anything - in
> this and multiple other switch-case statements in the series.

I will fix this. Thanks!


...[snip]...


>> @@ -359,6 +384,8 @@ static int virtsnd_probe(struct virtio_device *vdev)
>> static void virtsnd_remove(struct virtio_device *vdev)
>> {
>>       struct virtio_snd *snd = vdev->priv;
>> +     struct virtio_pcm *pcm;
>> +     struct virtio_pcm *pcm_next;
>>
>>       if (!snd)
>>               return;
>> @@ -376,6 +403,24 @@ static void virtsnd_remove(struct virtio_device
>> *vdev)
>>       vdev->config->reset(vdev);
>>       vdev->config->del_vqs(vdev);
>>
>> +     list_for_each_entry_safe(pcm, pcm_next, &snd->pcm_list, list) {
>> +             unsigned int i;
>> +
>> +             list_del(&pcm->list);
>> +
>> +             for (i = 0; i < ARRAY_SIZE(pcm->streams); ++i) {
>> +                     struct virtio_pcm_stream *stream =
>> &pcm->streams[i];
>> +
>> +                     if (stream->substreams)
>> +                             devm_kfree(&vdev->dev, 
stream->substreams);
>> +             }
>> +
>> +             devm_kfree(&vdev->dev, pcm);
>
> Please double-check both devm_kfree() calls above. Probably they aren't
> needed in the .remove() method.

Then I will redo these parts, and the parts that you noticed in the rest
of the comments to this file.


...[snip]...


>
> Thanks
> Guennadi
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@xxxxxxxxxxxxxxxxxxxx
> For additional commands, e-mail: virtio-dev-help@xxxxxxxxxxxxxxxxxxxx
>
>
--
Anton Yakovlev
Senior Software Engineer

OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin

www.opensynergy.com