Re: [PATCH] em28xx: Remove useless runtime->private_data usage

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

 



Hi Mauro,

On Thu, Jul 5, 2012 at 1:57 PM, Mauro Carvalho Chehab
<mchehab@xxxxxxxxxx> wrote:

>>
>>       snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
>>       dev->adev.capture_pcm_substream = substream;
>> -     runtime->private_data = dev;
>
>
> Are you sure that this can be removed? I think this is used internally
> by the alsa API, but maybe something has changed and this is not
> required anymore.

Yes, I'm sure.

>
> Had you test em28xx audio with this change?

No, I did not test it.

To make this patch, I've considered two things:

1. Alsa documentation [1]
This is from chapter 5, "Private Data" section.

---
You can allocate a record for the substream and store it in
runtime->private_data. Usually, this is done in the open callback.
Don't mix this with pcm->private_data. The pcm->private_data usually
points to the chip instance assigned statically at the creation of
PCM, while the runtime->private_data points to a dynamic data
structure created at the PCM open callback.

  static int snd_xxx_open(struct snd_pcm_substream *substream)
  {
          struct my_pcm_data *data;
          ....
          data = kmalloc(sizeof(*data), GFP_KERNEL);
          substream->runtime->private_data = data;
          ....
  }
---

I think the part "Don't mix this with pcm->private_data", is the one
related to this case.
Also, what alsa documentation calls "chip instance" is our em28xx
device structure.

2. Regular kernel practice:
Normally, private_data fields are suppose to be (private) data the
driver author wants
the core subsystem to pass him as callback parameter. The core
subsystem is not supposed
to use it in anyway (he wouldn't know how).
So, if you don't use it anywhere else in your code, it's safe to remove it.

If still in doubt, just don't apply it.

I'm not really concerned about one extra line,
rather about drivers doing unnecessary stuff,
and then others take these drivers as example
and spread the bloatness all over the place, so to speak.

[1] http://www.alsa-project.org/~tiwai/writing-an-alsa-driver/ch05s05.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux