Re: [PATCH] em28xx: Remove useless runtime-private_data usage
Mauro, On Thu, Jul 5, 2012 at 2:22 PM, Ezequiel Garcia 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. This should be: I'm almost sure :-) Anyway, probably the patch should have a more verbose commit message, right? Do you want to do drop it entirely? Regards, Ezequiel. -- 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] em28xx: Remove useless runtime-private_data usage
Em 06-07-2012 11:33, Ezequiel Garcia escreveu: Mauro, On Thu, Jul 5, 2012 at 2:22 PM, Ezequiel Garcia 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. This should be: I'm almost sure :-) Anyway, probably the patch should have a more verbose commit message, right? Yeah, that would be good. Do you want to do drop it entirely? No, but, as I'm taking a 2-week vacations starting next week, I'll postpone those compiled-only cleanup patches to apply after my return, probably holding them to be applied on 3.6. Regards, Mauro -- 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] em28xx: Remove useless runtime-private_data usage
On Fri, Jul 6, 2012 at 12:12 PM, Mauro Carvalho Chehab mche...@redhat.com wrote: Em 06-07-2012 11:33, Ezequiel Garcia escreveu: Mauro, On Thu, Jul 5, 2012 at 2:22 PM, Ezequiel Garcia 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. This should be: I'm almost sure :-) Anyway, probably the patch should have a more verbose commit message, right? Yeah, that would be good. Do you want to do drop it entirely? No, but, as I'm taking a 2-week vacations starting next week, I'll postpone those compiled-only cleanup patches to apply after my return, probably holding them to be applied on 3.6. Okey. Have a nice holiday! Ezequiel. -- 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] em28xx: Remove useless runtime-private_data usage
Em 12-06-2012 10:53, Ezequiel Garcia escreveu: Signed-off-by: Ezequiel Garcia elezegar...@gmail.com --- drivers/media/video/em28xx/em28xx-audio.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/drivers/media/video/em28xx/em28xx-audio.c b/drivers/media/video/em28xx/em28xx-audio.c index d7e2a3d..2fcddae 100644 --- a/drivers/media/video/em28xx/em28xx-audio.c +++ b/drivers/media/video/em28xx/em28xx-audio.c @@ -305,7 +305,6 @@ static int snd_em28xx_capture_open(struct snd_pcm_substream *substream) 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. Had you test em28xx audio with this change? Regards, Mauro -- 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] em28xx: Remove useless runtime-private_data usage
Hi Mauro,
On Thu, Jul 5, 2012 at 1:57 PM, Mauro Carvalho Chehab
mche...@redhat.com 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] em28xx: Remove useless runtime-private_data usage
Signed-off-by: Ezequiel Garcia elezegar...@gmail.com --- drivers/media/video/em28xx/em28xx-audio.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/drivers/media/video/em28xx/em28xx-audio.c b/drivers/media/video/em28xx/em28xx-audio.c index d7e2a3d..2fcddae 100644 --- a/drivers/media/video/em28xx/em28xx-audio.c +++ b/drivers/media/video/em28xx/em28xx-audio.c @@ -305,7 +305,6 @@ static int snd_em28xx_capture_open(struct snd_pcm_substream *substream) snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS); dev-adev.capture_pcm_substream = substream; - runtime-private_data = dev; return 0; err: -- 1.7.3.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
